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

Gas Optimization: consider using block.chainid instead of inline assembly chainid #28

Open
naszam opened this issue May 16, 2023 · 0 comments

Comments

@naszam
Copy link
Collaborator

naszam commented May 16, 2023

Context: PermitERC721.sol#L119-L122

Description:
PermitERC721 makes use of chainid inline assembly to get the current chain id used in DOMAIN_SEPARATOR.

Recommendation:
Consider removing inline assembly chainid and the function _chainId in favour of passing directly the pre-defined global variable block.chainid, introduced in solidity version 0.8.0, that retrieves the current chain id from the block header of the current block.
The recommendation takes into account gas optimization, readability and complexity reduction benefits as well as maintainability.

diff --git a/src/base/PermitERC721.sol b/src/base/PermitERC721.sol
index a92acaf..0fcfc65 100644
--- a/src/base/PermitERC721.sol
+++ b/src/base/PermitERC721.sol
@@ -59,7 +59,7 @@ abstract contract PermitERC721 is ERC721, IPermit {
                     0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f,
                     _nameHash,
                     _versionHash,
-                    _chainId(),
+                    block.chainid,
                     address(this)
                 )
             );
@@ -112,14 +112,4 @@ abstract contract PermitERC721 is ERC721, IPermit {
         _approve(spender_, tokenId_);
     }
 
-    /**
-     *  @dev Gets the current chain id
-     *  @return chainId_ The current chain id
-     */
-    function _chainId() internal view returns (uint256 chainId_) {
-        assembly {
-            chainId_ := chainid()
-        }
-    }
-
 }
  • Tests PASS (removing _chainId function and passing block.chainid to DOMAIN_SEPARATOR)
  • Gas Report
| Function Name                    | min             | avg | median | max | # calls |
| chainId                          | 167             | 167 | 167    | 167 | 1       |
| chainIdOptimized                 | 145             | 145 | 145    | 145 | 1       |
  • Gas Report (applying the change)
| DOMAIN_SEPARATOR                                 | 512             | 512    | 512    | 512     | 7       |
| DOMAIN_SEPARATOR                                 | 509             | 509    | 509    | 509     | 7       |

Ajna:

Prototech:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant