Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce contract size through error messages normalization #3742

Closed
bogdan opened this issue Sep 30, 2022 · 4 comments
Closed

Reduce contract size through error messages normalization #3742

bogdan opened this issue Sep 30, 2022 · 4 comments

Comments

@bogdan
Copy link

bogdan commented Sep 30, 2022

🧐 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.

@Amxx
Copy link
Collaborator

Amxx commented Sep 30, 2022

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.

@frangio
Copy link
Contributor

frangio commented Sep 30, 2022

@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
Copy link
Contributor

frangio commented Sep 30, 2022

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
Copy link
Contributor

frangio commented Dec 2, 2022

We will be adopting Custom Errors for OpenZeppelin Contracts 5.0. See #2839.

@frangio frangio closed this as completed Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@bogdan @frangio @Amxx and others