ERC725Alliance / ERC725

Repository for code and discussion around ERC725 and related standards
Apache License 2.0
127 stars 66 forks source link

perf: use directly `IERC165` in inheritance instead of `ERC165` #238

Closed CJ42 closed 1 year ago

CJ42 commented 1 year ago

What does this PR introduce?

Note: awaiting for #236 to be merged first.

  • [x] Change base branch to develop once #236 is merged.

♻️ Refactor

Remove IERC165 from the inheritance list of IERC725X and IERC725Y.

⚡️ Performance

Reduce deployment cost by an extra -45.814 gas compared to #236 by using directly IERC165 in the inheritance instead of ERC165 and calling super.supportsInterface.

The checks for interface ID ERC165 are directly inlined as a result inside the supportsInterface(bytes4) function of ERC725XCore, ERC725YCore and ERC725 contracts.

PR Checklist

Previous gas cost (develop branch)

···············|·······················································|·············|··············|·············|···············|··············
|  Contract    ·  Method                                               ·  Min        ·  Max         ·  Avg        ·  # calls      ·  usd (avg)  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  execute(uint256,address,uint256,bytes)               ·      34726  ·      127088  ·      47290  ·          114  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  executeBatch(uint256[],address[],uint256[],bytes[])  ·      60485  ·      155455  ·      92070  ·           16  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  renounceOwnership()                                  ·      31283  ·       31328  ·      31306  ·            4  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  setData(bytes32,bytes)                               ·      32983  ·     8244933  ·    1494244  ·           17  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  setDataBatch(bytes32[],bytes[])                      ·      35151  ·     8247117  ·    1417890  ·           18  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  transferOwnership(address)                           ·      32069  ·       32091  ·      32084  ·            6  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Init  ·  initialize(address)                                  ·      51749  ·       51771  ·      51749  ·          124  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  execute(uint256,address,uint256,bytes)               ·      31954  ·      124204  ·      44508  ·          114  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  executeBatch(uint256[],address[],uint256[],bytes[])  ·      57611  ·      152526  ·      89181  ·           16  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  renounceOwnership()                                  ·          -  ·           -  ·      23661  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  transferOwnership(address)                           ·          -  ·           -  ·      29219  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  renounceOwnership()                                  ·          -  ·           -  ·      23662  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  setData(bytes32,bytes)                               ·      30252  ·     8239813  ·    1491087  ·           17  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  setDataBatch(bytes32[],bytes[])                      ·      32396  ·     8241968  ·    1414731  ·           18  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  transferOwnership(address)                           ·          -  ·           -  ·      29197  ·            4  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  Deployments                                                         ·                                          ·  % of limit   ·             │
·······································································|·············|··············|·············|···············|··············
|  ERC725                                                              ·          -  ·           -  ·    2393988  ·          8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725Init                                                          ·          -  ·           -  ·    2600372  ·        8.7 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725X                                                             ·          -  ·           -  ·    1832301  ·        6.1 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725XInit                                                         ·          -  ·           -  ·    2044314  ·        6.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725Y                                                             ·          -  ·           -  ·    1134143  ·        3.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725YInit                                                         ·          -  ·           -  ·    1344141  ·        4.5 %  ·          -  │
·----------------------------------------------------------------------|-------------|--------------|-------------|---------------|-------------·

New Gas costs (this branch)

·----------------------------------------------------------------------|----------------------------|-------------|-----------------------------·
|                         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  ·      47290  ·          114  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  executeBatch(uint256[],address[],uint256[],bytes[])  ·      60485  ·      155455  ·      92070  ·           16  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  renounceOwnership()                                  ·      31283  ·       31328  ·      31306  ·            4  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  setData(bytes32,bytes)                               ·      32983  ·     8083629  ·    1465779  ·           17  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  setDataBatch(bytes32[],bytes[])                      ·      35151  ·     8085813  ·    1391006  ·           18  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725      ·  transferOwnership(address)                           ·      32069  ·       32091  ·      32084  ·            6  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Init  ·  initialize(address)                                  ·      51749  ·       51771  ·      51749  ·          124  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  execute(uint256,address,uint256,bytes)               ·      31954  ·      124204  ·      44508  ·          114  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  executeBatch(uint256[],address[],uint256[],bytes[])  ·      57611  ·      152526  ·      89181  ·           16  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  renounceOwnership()                                  ·          -  ·           -  ·      23661  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725X     ·  transferOwnership(address)                           ·          -  ·           -  ·      29219  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  renounceOwnership()                                  ·          -  ·           -  ·      23662  ·            2  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  setData(bytes32,bytes)                               ·      30252  ·     8078561  ·    1462631  ·           17  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  setDataBatch(bytes32[],bytes[])                      ·      32396  ·     8080716  ·    1387855  ·           18  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  ERC725Y     ·  transferOwnership(address)                           ·          -  ·           -  ·      29197  ·            4  ·          -  │
···············|·······················································|·············|··············|·············|···············|··············
|  Deployments                                                         ·                                          ·  % of limit   ·             │
·······································································|·············|··············|·············|···············|··············
|  ERC725                                                              ·          -  ·           -  ·    2348169  ·        7.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725Init                                                          ·          -  ·           -  ·    2554540  ·        8.5 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725X                                                             ·          -  ·           -  ·    1828454  ·        6.1 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725XInit                                                         ·          -  ·           -  ·    2040383  ·        6.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725Y                                                             ·          -  ·           -  ·    1130213  ·        3.8 %  ·          -  │
·······································································|·············|··············|·············|···············|··············
|  ERC725YInit                                                         ·          -  ·           -  ·    1340258  ·        4.5 %  ·          -  │
·----------------------------------------------------------------------|-------------|--------------|-------------|---------------|-------------·
CJ42 commented 1 year ago

We need to keep super.supportsInterface, otherwise we will stop the inheritance chain in supportsInterface of child contracts such as LSP9, and LSP0 for example.

Tested and this actually will break the inheritance call via super. indeed, if one of the ERC725 contract is in the middle of the inheritance hierarchy. I will close this PR then as we cannot implement this change.