improve(gas-golf): Minor gas tweaks #73

nicholaspai · 2022-03-07T12:39:16Z

Makes modest gas cost reductions, I think its easy to argue we don't need these optimizations but curious what you think.

Unit test gas report from running yarn test:gas-analytics

Stress testing results

BEFORE:
  Gas Analytics: HubPool Root Bundle Execution
    Pool Rebalance tree with 10 leaves, each containing refunds for 10 different tokens
proposeRootBundle-gasUsed: 110198
      ✔ Simple proposal
executeRootBundle-gasUsed: 714268
      ✔ Executing 1 leaf (220ms)
(average) executeRootBundle-gasUsed: 686317
      ✔ Executing all leaves (1838ms)
(average) executeRootBundle-gasUsed: 497583
      ✔ Executing all leaves using multicall (1506ms)
executeRootBundle-gasUsed: 10362678
      ✔ Stress Test: 1 leaf contains 100 L1 tokens with netSendAmounts > 0 (28541ms)

  Gas Analytics: SpokePool Deposits
    ERC20 Deposits
deposit-gasUsed: 70672
      ✔ 1 Deposit (39ms)
(average) deposit-gasUsed: 70672
      ✔ 10 deposits (264ms)
(average) deposit-gasUsed: 24380
      ✔ 10 deposits using multicall (117ms)
    WETH Deposits
deposit-gasUsed: 62709
      ✔ 1 ETH Deposit

  Gas Analytics: SpokePool Relays
    ERC20 Relays
fillRelay-gasUsed: 83390
      ✔ 1 Relay
(average) fillRelay-gasUsed: 83400
      ✔ 10 Relays (200ms)
(average) fillRelay-gasUsed: 47303
      ✔ 10 relays using multicall (110ms)
    WETH Relays
fillRelay-gasUsed: 98273
      ✔ 1 Relay

  Gas Analytics: SpokePool Relayer Refund Root Execution
    (ERC20) Tree with 10 leaves, each containing 10 refunds
relayRootBundle-gasUsed: 75995
      ✔ Relay proposal
executeRelayerRefundLeaf-gasUsed: 147265
      ✔ Executing 1 leaf (73ms)
(average) executeRelayerRefundLeaf-gasUsed: 131555
      ✔ Executing all leaves (522ms)
(average) executeRelayerRefundLeaf-gasUsed: 102826
      ✔ Executing all leaves using multicall (423ms)
    (WETH): Relayer Refund tree with 10 leaves, each containing 10 refunds
executeRelayerRefundLeaf-gasUsed: 165885
      ✔ Executing 1 leaf (74ms)

  Gas Analytics: SpokePool Slow Relay Root Execution
    (ERC20) Tree with 10 leaves
relayRootBundle-gasUsed: 75995
      ✔ Relay proposal
executeSlowRelayLeaf-gasUsed: 84868
      ✔ Executing 1 leaf
(average) executeSlowRelayLeaf-gasUsed: 99937
      ✔ Executing all leaves (211ms)
(average) executeSlowRelayLeaf-gasUsed: 75462
      ✔ Executing all leaves using multicall (135ms)
    (WETH) Tree with 10 leaves
executeSlowRelayLeaf-gasUsed: 92760
      ✔ Executing 1 leaf

AFTER
  Gas Analytics: HubPool Root Bundle Execution
    Pool Rebalance tree with 10 leaves, each containing refunds for 10 different tokens
proposeRootBundle-gasUsed: 106555
      ✓ Simple proposal
executeRootBundle-gasUsed: 712012
      ✓ Executing 1 leaf
(average) executeRootBundle-gasUsed: 684081
      ✓ Executing all leaves
(average) executeRootBundle-gasUsed: 495349
      ✓ Executing all leaves using multicall
executeRootBundle-gasUsed: 10348429
      ✓ Stress Test: 1 leaf contains 100 L1 tokens with netSendAmounts > 0

  Gas Analytics: SpokePool Deposits
    ERC20 Deposits
deposit-gasUsed: 70581
      ✓ 1 Deposit
(average) deposit-gasUsed: 70581
      ✓ 10 deposits
(average) deposit-gasUsed: 24289
      ✓ 10 deposits using multicall
    WETH Deposits
deposit-gasUsed: 62618
      ✓ 1 ETH Deposit

  Gas Analytics: SpokePool Relays
    ERC20 Relays
fillRelay-gasUsed: 83390
      ✓ 1 Relay
(average) fillRelay-gasUsed: 83400
      ✓ 10 Relays
(average) fillRelay-gasUsed: 47303
      ✓ 10 relays using multicall
    WETH Relays
fillRelay-gasUsed: 98273
      ✓ 1 Relay

  Gas Analytics: SpokePool Relayer Refund Leaf Execution
    (ERC20) Tree with 10 leaves, each containing 10 refunds
relayRootBundle-gasUsed: 75992
      ✓ Relay proposal
executeRelayerRefundLeaf-gasUsed: 146346
      ✓ Executing 1 leaf
(average) executeRelayerRefundLeaf-gasUsed: 130636
      ✓ Executing all leaves
(average) executeRelayerRefundLeaf-gasUsed: 101907
      ✓ Executing all leaves using multicall
    (WETH): Relayer Refund tree with 10 leaves, each containing 10 refunds
executeRelayerRefundLeaf-gasUsed: 164966
      ✓ Executing 1 leaf

  Gas Analytics: SpokePool Slow Relay Root Execution
    (ERC20) Tree with 10 leaves
relayRootBundle-gasUsed: 75992
      ✓ Relay proposal
executeSlowRelayLeaf-gasUsed: 84868
      ✓ Executing 1 leaf
(average) executeSlowRelayLeaf-gasUsed: 99937
      ✓ Executing all leaves
(average) executeSlowRelayLeaf-gasUsed: 75462
      ✓ Executing all leaves using multicall
    (WETH) Tree with 10 leaves
executeSlowRelayLeaf-gasUsed: 92760
      ✓ Executing 1 leaf

Changes

In for loops:
- Pre-increment saves a Store operation
- Cache length before loop
- Use unchecked to increment counter
Use calldata instead of memory for variables we don't change
Pack state variables smarter
Don't read from state when emitting variables

chrismaree

🏌️

nicholaspai · 2022-03-07T15:05:29Z

contracts/HubPool.sol

                        l1Token, // l1Token.
                        l2Token, // l2Token.
                        uint256(netSendAmounts[i]), // amount.
-                        crossChainContracts[chainId].spokePool // to. This should be the spokePool.


Loading this data in each loop adds gas, we should pre-load it instead

mrice32

As discussed IRL, I think we might want to hold off on merging this until we decide whether we think there's enough time for OZ to review this along with other fixes.

into npai/gas-golf

nicholaspai · 2022-03-12T16:55:25Z

contracts/HubPool.sol

        // The bond should be the passed in bondAmount + the final fee.
        bondToken = newBondToken;
        bondAmount = newBondAmount + _getBondTokenFinalFee();
-        emit BondSet(address(newBondToken), bondAmount);


Don't SLOAD unnecessarily

nicholaspai · 2022-03-12T16:56:10Z

contracts/HubPool.sol

+
+            // L1 tokens length won't be > types(uint32).length
+            unchecked {
+                ++i;


pre-increment saves a STORE vs post-increment

nicholaspai · 2022-03-12T16:56:24Z

contracts/Polygon_SpokePool.sol

    function setFxChild(address newFxChild) public onlyAdmin {
        fxChild = newFxChild;
-        emit SetFxChild(fxChild);
+        emit SetFxChild(newFxChild);


Don't SLOAD unnecessarily

nicholaspai · 2022-03-12T18:38:57Z

contracts/HubPool.sol

    // can be submitted.
    RootBundle public rootBundleProposal;

-    // Whether the bundle proposal process is paused.


Any changes in the order of state variables is done to take advantage of variable packing

nicholaspai · 2022-03-12T18:39:05Z

contracts/HubPool.sol

+
    // WETH contract for Ethereum.
-    WETH9 public weth;
+    WETH9 public immutable weth;


use immutable where possible

nicholaspai · 2022-03-12T18:39:46Z

contracts/HubPool.sol

-        uint256[] memory bundleLpFees,
-        int256[] memory netSendAmounts,
-        int256[] memory runningBalances,
+        uint256[] calldata bundleLpFees,


Use calldata for variables that will just be loaded into an in memory PoolRebalance struct

nicholaspai · 2022-03-12T18:39:57Z

contracts/HubPool.sol


        // Before interacting with a particular chain's adapter, ensure that the adapter is set.
-        require(address(crossChainContracts[chainId].adapter) != address(0), "No adapter for chain");
+        CrossChainContract memory _crossChainContracts = crossChainContracts[chainId];


Reduce one SLOAD

nicholaspai · 2022-03-12T18:40:13Z

contracts/HubPool.sol

        AdapterInterface adapter = crossChainContracts[chainId].adapter;

-        for (uint32 i = 0; i < l1Tokens.length; i++) {
+        uint32 length = uint32(l1Tokens.length);


cache loop length

into npai/gas-golf

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>

mrice32

LGTM!

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>

Use pre-increment

2f837a7

chrismaree approved these changes Mar 7, 2022

View reviewed changes

nicholaspai marked this pull request as ready for review March 7, 2022 14:25

nicholaspai requested review from chrismaree and mrice32 March 7, 2022 14:48

Update HubPool.sol

1076eda

nicholaspai commented Mar 7, 2022

View reviewed changes

mrice32 reviewed Mar 7, 2022

View reviewed changes

nicholaspai added 4 commits March 8, 2022 13:15

Merge branch 'master' of https://github.com/across-protocol/contracts-v2

a92d8a1

into npai/gas-golf

Update HubPool.RootExecution.ts

bf3dee5

emit calldata vars not storage

790c736

Merge branch 'master' of https://github.com/across-protocol/contracts-v2

4026145

into npai/gas-golf

nicholaspai commented Mar 12, 2022

View reviewed changes

nicholaspai added 5 commits March 12, 2022 12:13

adds

64b5bbf

fix

ad64ec1

more golf

7b08854

refactor

cc8c65b

Update HubPool.sol

e6ecec2

nicholaspai commented Mar 12, 2022

View reviewed changes

nicholaspai added 2 commits March 12, 2022 13:43

Update HubPool.sol

1afc3fc

Merge branch 'master' into npai/gas-golf

4089bfc

nicholaspai changed the title ~~improve(gas-golf): Use pre-increment and unchecked in loops~~ improve(gas-golf): Various gas golfing Mar 15, 2022

nicholaspai added 2 commits March 17, 2022 15:39

Merge branch 'master' of https://github.com/across-protocol/contracts-v2

7904973

into npai/gas-golf

Update HubPool.sol

4ead409

nicholaspai and others added 4 commits March 18, 2022 16:48

Merge branch 'master' into npai/gas-golf

b58787f

fix

b6a767a

add comments

f76f843

WIP

1c119d3

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>

mrice32 changed the title ~~improve(gas-golf): Various gas golfing~~ improve(gas-golf): Minor gas tweaks Mar 21, 2022

mrice32 approved these changes Mar 21, 2022

View reviewed changes

fix hardhat test issues

9823764

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>

nicholaspai merged commit 96b590e into master Mar 22, 2022

nicholaspai deleted the npai/gas-golf branch March 22, 2022 01:07

This was referenced Apr 6, 2022

feat: clean up some test related inconsistancies #130

Merged

feat: add rate model store on rinkeby #132

Merged

chrismaree mentioned this pull request Apr 20, 2022

chrismaree/add block number to export #143

Merged

This was referenced May 3, 2022

chrismaree/mainnet deployment #149

Merged

Latest kovan deployment #154

Merged

feat: add reverse look up for contract info from address #155

Merged

chrismaree mentioned this pull request May 10, 2022

chrisamree/mumbai deploy #157

Merged

This was referenced May 17, 2022

fix: address issue with weth on boba #159

Merged

fix: add haircut proposal #160

Merged

fix: address param ordering issue #163

Merged

improve(gas-golf): Minor gas tweaks #73

improve(gas-golf): Minor gas tweaks #73

Uh oh!

Conversation

nicholaspai commented Mar 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrismaree left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrice32 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrice32 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nicholaspai commented Mar 7, 2022 •

edited

Loading