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

refactor: use custom error for onlyOwner and address(0) check #236

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

CJ42
Copy link
Collaborator

@CJ42 CJ42 commented Sep 19, 2023

What does this PR introduce?

⚠️ BREAKING CHANGES

📦 Build

  • Increase minimum solc compiler version because of full usage of custom error (currently states pragma solidity ^0.8.0

⚡️ Performance

Replace revert reason strings with custom errors for OwnableUnset and address(0) check on transferOwnership(), constructor and initialize(address) functions.

This reduces the deployment cost of an ERC725 smart contract by -58,427 gas.

PR Checklist

  • Wrote Tests
  • Wrote Documentation
  • Ran npm run lint && npm run lint:solidity
  • Ran npm run format (prettier)
  • Ran npm run build
  • Ran npm run test

@CJ42
Copy link
Collaborator Author

CJ42 commented Sep 19, 2023

Before

·----------------------------------------------------------------------|----------------------------|-------------|-----------------------------·
| Solc version: 0.8.17 · Optimizer enabled: false · Runs: 200 · Block limit: 30000000 gas │
·······································································|····························|·············|······························
| Methods │
···············|·······················································|·············|··············|·············|···············|··············
| Contract · Method · Min · Max · Avg · # calls · usd (avg) │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725 · execute(uint256,address,uint256,bytes) · 34726 · 127088 · 47401 · 113 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725 · executeBatch(uint256[],address[],uint256[],bytes[]) · 60485 · 155455 · 92070 · 16 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725 · renounceOwnership() · 31296 · 31341 · 31319 · 4 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725 · setData(bytes32,bytes) · 32983 · 8566933 · 1551068 · 17 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725 · setDataBatch(bytes32[],bytes[]) · 35151 · 8569119 · 1471557 · 18 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725 · transferOwnership(address) · 32082 · 32104 · 32097 · 6 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725Init · initialize(address) · 51762 · 51784 · 51762 · 122 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725X · execute(uint256,address,uint256,bytes) · 31954 · 124204 · 44619 · 113 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725X · executeBatch(uint256[],address[],uint256[],bytes[]) · 57611 · 152526 · 89181 · 16 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725X · renounceOwnership() · - · - · 23677 · 2 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725X · transferOwnership(address) · - · - · 29235 · 2 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725Y · renounceOwnership() · - · - · 23678 · 2 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725Y · setData(bytes32,bytes) · 30252 · 8561709 · 1547892 · 17 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725Y · setDataBatch(bytes32[],bytes[]) · 32396 · 8563865 · 1468380 · 18 · - │
···············|·······················································|·············|··············|·············|···············|··············
| ERC725Y · transferOwnership(address) · - · - · 29213 · 4 · - │
···············|·······················································|·············|··············|·············|···············|··············
| Deployments · · % of limit · │
·······································································|·············|··············|·············|···············|··············
| ERC725 · - · - · 2454031 · 8.2 % · - │
·······································································|·············|··············|·············|···············|··············
| ERC725Init · - · - · 2659411 · 8.9 % · - │
·······································································|·············|··············|·············|···············|··············
| ERC725X · - · - · 1892377 · 6.3 % · - │
·······································································|·············|··············|·············|···············|··············
| ERC725XInit · - · - · 2103358 · 7 % · - │
·······································································|·············|··············|·············|···············|··············
| ERC725Y · - · - · 1197840 · 4 % · - │
·······································································|·············|··············|·············|···············|··············
| ERC725YInit · - · - · 1403187 · 4.7 % · - │
·----------------------------------------------------------------------|-------------|--------------|-------------|---------------|-------------·

After

·----------------------------------------------------------------------|----------------------------|-------------|-----------------------------·
|                         Solc version: 0.8.17                         ·  Optimizer enabled: false  ·  Runs: 200  ·  Block limit: 30000000 gas  │
·······································································|····························|·············|······························
|  Methods                                                                                                                                      │
···············|·······················································|·············|··············|·············|···············|··············
|  Contract    ·  Method                                               ·  Min        ·  Max         ·  Avg        ·  # calls      ·  usd (avg)  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  execute(uint256,address,uint256,bytes)               ·      34726  ·      127088  ·      47401  ·          113  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  executeBatch(uint256[],address[],uint256[],bytes[])  ·      60485  ·      155455  ·      92070  ·           16  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  renounceOwnership()                                  ·      31296  ·       31341  ·      31319  ·            4  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  setData(bytes32,bytes)                               ·      32983  ·     8267752  ·    1498271  ·           17  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  setDataBatch(bytes32[],bytes[])                      ·      35151  ·     8269937  ·    1421694  ·           18  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  transferOwnership(address)                           ·      32082  ·       32104  ·      32097  ·            6  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Init  ·  initialize(address)                                  ·      51762  ·       51784  ·      51762  ·          122  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  execute(uint256,address,uint256,bytes)               ·      31954  ·      124204  ·      44619  ·          113  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  executeBatch(uint256[],address[],uint256[],bytes[])  ·      57611  ·      152526  ·      89181  ·           16  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  renounceOwnership()                                  ·          -  ·           -  ·      23677  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  transferOwnership(address)                           ·          -  ·           -  ·      29235  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  renounceOwnership()                                  ·          -  ·           -  ·      23678  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  setData(bytes32,bytes)                               ·      30252  ·     8262625  ·    1495113  ·           17  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  setDataBatch(bytes32[],bytes[])                      ·      32396  ·     8264780  ·    1418533  ·           18  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  transferOwnership(address)                           ·          -  ·           -  ·      29213  ·            4  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  Deployments                                                         ·                                          ·  % of limit   ·             │
·······································································|·············|··············|·············|···············|··············
|  ERC725                                                              ·          -  ·           -  ·    2395604  ·          8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725Init                                                          ·          -  ·           -  ·    2601464  ·        8.7 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725X                                                             ·          -  ·           -  ·    1833965  ·        6.1 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725XInit                                                         ·          -  ·           -  ·    2045382  ·        6.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725Y                                                             ·          -  ·           -  ·    1135735  ·        3.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725YInit                                                         ·          -  ·           -  ·    1345233  ·        4.5 %  ·          -  │
·----------------------------------------------------------------------|-------------|--------------|-------------|---------------|-------------·

@CJ42 CJ42 force-pushed the refactor/custom-error branch 2 times, most recently from 68d6a83 to 5ca5fa0 Compare September 20, 2023 10:41
@CJ42 CJ42 changed the title refactor: use custom error for address(0) check refactor: use custom error for onlyOwner and address(0) check Sep 20, 2023
@CJ42 CJ42 force-pushed the refactor/custom-error branch 8 times, most recently from 3159683 to a0c1c1a Compare September 21, 2023 14:20
@CJ42 CJ42 merged commit d551e90 into develop Sep 27, 2023
18 checks passed
@CJ42 CJ42 deleted the refactor/custom-error branch September 27, 2023 08:52
@CJ42 CJ42 mentioned this pull request Oct 10, 2023
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

Successfully merging this pull request may close these issues.

3 participants