OpenZeppelin / openzeppelin-contracts-upgradeable

Upgradeable variant of OpenZeppelin Contracts, meant for use in upgradeable contracts.
https://openzeppelin.com/contracts
MIT License
1k stars 436 forks source link

Reduce impact on contract size by modifying modifiers #159

Closed jnishiyama closed 2 years ago

jnishiyama commented 2 years ago

🧐 Motivation The idea here is to optimize contract size impact of modifiers by moving require statements to function that is separate from the modifier itself.

📝 Details I am fairly new to Web3 development, and I haven't checked throughout the repo, but in both OwnableUpgradeable and PausableUpgradeable the require statements are contained in the modifier. By simply creating a function that contains the require statement and calling it from the modifier, we can reduce the impact on the overall contract size of contracts using these.

e.g.

    /**
     * @dev Throws if called by any account other than the owner.
     */
    function _onlyOwner() private view {
        require(owner() == _msgSender(), "Ownable: caller is not the owner");
    }

    /**
     * @dev Throws if called by any account other than the owner.
     */
    modifier onlyOwner() {
        _onlyOwner();
        _;
    }

Depending on how much the modifier is used, the size reduction can be significant.

I'd be happy to make a PR if necessary.

Amxx commented 2 years ago

Hello @jnishiyama

The code in this repo is transpired from the "OpenZeppelin/OpenZeppelin-contracts" repository. This issue, and any corresponding PR should be submitted there.

Hope to see you soon there.

jnishiyama commented 2 years ago

Understood, leaving link to this issue on main repo for posterity: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3222