Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Modifiers call internal functions #163

Merged
merged 6 commits into from
May 29, 2020
Merged

Modifiers call internal functions #163

merged 6 commits into from
May 29, 2020

Conversation

steviezhang
Copy link
Contributor

@steviezhang steviezhang commented May 27, 2020

Changes

  • modifiers onlyRelayers, onlyAdmin, onlyBridge, and onlyAdminOrRelayer now call equivalent internal functions
  • this reduces the amount of code inlined from the frequent use of these modifiers, reducing bytecode size for the bridge contract.
  • cuts deployment cost by ~185k gas for the bridge and ~100k for handlers while increasing cost of individual transactions involving functions with these modifiers marginally (~30 gas).

Benchmarks prior to this:

  Contract: Gas Benchmark - [contract deployments]
┌─────────┬───────────────────┬─────────┐
│ (index) │       type        │ gasUsed │
├─────────┼───────────────────┼─────────┤
│    0    │     'Bridge'      │ 4300541 │
│    1    │  'ERC20Handler'   │ 2223651 │
│    2    │  'ERC721Handler'  │ 2510589 │
│    3    │ 'GenericHandler'  │ 1583325 │
│    4    │ 'CentrifugeAsset' │ 197353  │
│    5    │ 'HandlerHelpers'  │ 588114  │
│    6    │    'ERC20Safe'    │ 370019  │
│    7    │   'ERC721Safe'    │ 369071  │
└─────────┴───────────────────┴─────────┘

  Contract: Gas Benchmark - [Deposits]
┌─────────┬──────────────────────────────┬─────────┐
│ (index) │             type             │ gasUsed │
├─────────┼──────────────────────────────┼─────────┤
│    0    │           'ERC20'            │ 318782  │
│    1    │           'ERC721'           │ 369065  │
│    2    │ 'Generic - Centrifuge Asset' │ 187755  │
│    3    │   'Generic - No Argument'    │ 131781  │
│    4    │   'Generic - One Argument'   │ 211133  │
│    5    │  'Generic - Two Arguments'   │ 415655  │
│    6    │ 'Generic - Three Arguments'  │ 374878  │
└─────────┴──────────────────────────────┴─────────┘

  Contract: Gas Benchmark - [Execute Proposal]
┌─────────┬──────────────────────────────┬─────────┐
│ (index) │             type             │ gasUsed │
├─────────┼──────────────────────────────┼─────────┤
│    0    │           'ERC20'            │  57053  │
│    1    │           'ERC721'           │ 105767  │
│    2    │ 'Generic - Centrifuge Asset' │  73050  │
│    3    │   'Generic - No Argument'    │  47137  │
│    4    │   'Generic - One Argument'   │  47313  │
│    5    │   'Generic - Two Argument'   │  48854  │
│    6    │  'Generic - Three Argument'  │  48570  │
└─────────┴──────────────────────────────┴─────────┘

  Contract: Gas Benchmark - [Vote Proposal]
┌─────────┬───────────────────────────────────────────────────────┬─────────┐
│ (index) │                         type                          │ gasUsed │
├─────────┼───────────────────────────────────────────────────────┼─────────┤
│    0    │ 'Vote Proposal - relayerThreshold = 2, Not Finalized' │ 210901  │
│    1    │   'Vote Proposal - relayerThreshold = 2, Finalized'   │  97093  │
│    2    │   'Vote Proposal - relayerThreshold = 1, Finalized'   │ 199139  │
└─────────┴───────────────────────────────────────────────────────┴─────────┘

Benchmarks after these changes:

  Contract: Gas Benchmark - [contract deployments]
┌─────────┬───────────────────┬─────────┐
│ (index) │       type        │ gasUsed │
├─────────┼───────────────────┼─────────┤
│    0    │     'Bridge'      │ 4116922 │
│    1    │  'ERC20Handler'   │ 2110472 │
│    2    │  'ERC721Handler'  │ 2397376 │
│    3    │ 'GenericHandler'  │ 1583325 │
│    4    │ 'CentrifugeAsset' │ 197353  │
│    5    │ 'HandlerHelpers'  │ 550475  │
│    6    │    'ERC20Safe'    │ 370019  │
│    7    │   'ERC721Safe'    │ 369071  │
└─────────┴───────────────────┴─────────┘

  Contract: Gas Benchmark - [Deposits]
┌─────────┬──────────────────────────────┬─────────┐
│ (index) │             type             │ gasUsed │
├─────────┼──────────────────────────────┼─────────┤
│    0    │           'ERC20'            │ 318806  │
│    1    │           'ERC721'           │ 369089  │
│    2    │ 'Generic - Centrifuge Asset' │ 187755  │
│    3    │   'Generic - No Argument'    │ 131781  │
│    4    │   'Generic - One Argument'   │ 211133  │
│    5    │  'Generic - Two Arguments'   │ 415655  │
│    6    │ 'Generic - Three Arguments'  │ 374878  │
└─────────┴──────────────────────────────┴─────────┘

  Contract: Gas Benchmark - [Execute Proposal]
┌─────────┬──────────────────────────────┬─────────┐
│ (index) │             type             │ gasUsed │
├─────────┼──────────────────────────────┼─────────┤
│    0    │           'ERC20'            │  57101  │
│    1    │           'ERC721'           │ 105815  │
│    2    │ 'Generic - Centrifuge Asset' │  73074  │
│    3    │   'Generic - No Argument'    │  47161  │
│    4    │   'Generic - One Argument'   │  47337  │
│    5    │   'Generic - Two Argument'   │  48878  │
│    6    │  'Generic - Three Argument'  │  48594  │
└─────────┴──────────────────────────────┴─────────┘

  Contract: Gas Benchmark - [Vote Proposal]
┌─────────┬───────────────────────────────────────────────────────┬─────────┐
│ (index) │                         type                          │ gasUsed │
├─────────┼───────────────────────────────────────────────────────┼─────────┤
│    0    │ 'Vote Proposal - relayerThreshold = 2, Not Finalized' │ 210925  │
│    1    │   'Vote Proposal - relayerThreshold = 2, Finalized'   │  97117  │
│    2    │   'Vote Proposal - relayerThreshold = 1, Finalized'   │ 199163  │
└─────────┴───────────────────────────────────────────────────────┴─────────┘


Related to: #162

@steviezhang steviezhang self-assigned this May 27, 2020
@ansermino ansermino merged commit 5382f0c into master May 29, 2020
@ansermino ansermino deleted the steph/no-inline branch May 29, 2020 13:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants