openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

Reduce contract size through error messages normalization

Open bogdan opened this issue 3 years ago • 3 comments

🧐 Motivation

Contract size limit of 24KB on ETH mainnet is a big problem. We need to reduce contracts size as much as we can by optimizing error messages.

Using error codes is very inconvenient and backward incompatible option, so...

📝 Details

Error messages is a significant contributor to the contract size that can easily be optimized (with backward compatibility) by putting that as a part of separated contract. This contract can even be shared for a whole network.

Solution

Move all error messages to a separated contract that will be used in other contracts:

contract Errors {
  mapping (uint256 => string) public errorMessages

  addErrorMessage(message: string) {
     messages[keccak256(message)] = message;
  }
  addErrorMessages(messages: string[]) {
    ...
  }
  getManyByHash(hashes: uint256[]) {
   ...
  }
}

// Variant A: messages storage within the contract
contract ERC721 is Errors {
     function balanceOf(address owner) public view virtual override returns (uint256) {
        require(owner != address(0), errorMessages[0x388282]);
        return _balances[owner];
    }
}

// Variant B: shared messages storage for the whole network:
contract ERC721 {
     constructor(errorMessagesAddress) {
       errors = IErrors(errorMessagesAddress)
    }
     function balanceOf(address owner) public view virtual override returns (uint256) {
        require(owner != address(0), errors.errorMessages[0x388282]);
        return _balances[owner];
    }
}

Note that such a solution will not increase a gas cost of successful transaction but only the unsuccessful ones, which are pretty rare comparing to successful ones.

Shared contract approach can help people to safe gas when deploying to mainnet across the board.

Concerns

Such a solution will make deployment procedure more complex as now error messages list would need to be uploaded separately in case it is not reused from a shared contract.

Shared contract would need to be maintained and updated with each version of the library for all supported networks by uploading new list of error messages. Deleting the old messages or versioning for each library version is not required.

bogdan avatar Sep 30 '22 14:09 bogdan

Hello @bogdan

This approach needs storing error string in storage. This sound very expensive to set up. Considering custom errors are available since 0.8.4, I believe we should focus or effort in that direction. It both cheaper and more versatile than what your propose.

Amxx avatar Sep 30 '22 16:09 Amxx

@bogdan's proposal is to use a globally shared contract for error string decoding so the storage cost would be paid once upfront and not by the user.

While I sort of like this idea, it's going to run into several problems:

  • Big operational overhead to deploy the Errors contract on multiple networks
  • Usability issues around the need to deploy the Errors contract in local dev networks, or in networks not supported by us
  • Lack of tooling and usability issues around distribution of address of the Errors contract in each network, and the need to configure it properly at deployment

We are much more likely to migrate to custom Solidity custom errors (#2839). I'm interested in learning more why you say they're inconvenient.

frangio avatar Sep 30 '22 16:09 frangio

Balancer's approach to remove revert strings from the code is to use numeric codes along with a helper to generate a revert string with the ASCII number at runtime (BalancerErrors.sol) and documentation for the error codes online (https://dev.balancer.fi/references/error-codes).

I don't think we prefer this approach to Solidity Custom Errors either.

frangio avatar Sep 30 '22 16:09 frangio

We will be adopting Custom Errors for OpenZeppelin Contracts 5.0. See https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2839.

frangio avatar Dec 02 '22 00:12 frangio