Gas Optimization: Pack struct in FeeSplitter.sol

[code423n4]

Handle

gzeon

Vulnerability details

Impact

Shareholder struct can be repacked to reduce 1 slot, since _sendFees is called on basically every interaction with the protocol the saving would be significant. (~3 SLOAD every interaction)

Proof of Concept

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/FeeSplitter.sol#L34
rewrite to

    struct Shareholder {
        address account;
        uint236 weight;
    }

with casting whenever Shareholder.weight is used.

[adrien-supizet]

I'm not sure I understand this, the code written in the recommendation is identical to the current code. Although it mentions uint236 which the compiler does not accept.

[alcueca]

I think the warden was suggesting using this struct, which would fit in 32 bytes and save 1 SLOAD each time a Shareholder struct is read. How he types 236 instead of 96 is anyone's guess, though.

    struct Shareholder {
        address account;
        uint96 weight;
    }

[adrien-supizet]

@alcueca Thank you for pointing it out.
cc @maximebrugel See the quick test I just did, there might be a way to make this very slightly more efficient
See sendFees and sendFeesWithRoyalties
Before:

|  Contract     ·  Method                 ·  Min        ·  Max        ·  Avg         ·  # calls      ·  usd (avg)  │
················|·························|·············|·············|··············|···············|··············
|  ERC20        ·  approve                ·      46153  ·      46227  ·       46209  ·           10  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  releaseETH             ·          -  ·          -  ·       99929  ·            2  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  releaseToken           ·      76322  ·     110522  ·      101972  ·            4  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  releaseTokens          ·          -  ·          -  ·      286166  ·            1  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  sendFees               ·      67403  ·     150803  ·      123515  ·            7  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  sendFeesWithRoyalties  ·      73743  ·     159243  ·      120768  ·            4  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  setRoyaltiesWeight     ·          -  ·          -  ·       33815  ·            1  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  updateShareholder      ·      36478  ·      36490  ·       36484  ·            2  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  Deployments                            ·                                          ·  % of limit   ·             │
··········································|·············|·············|··············|···············|··············
|  FeeSplitter                            ·    1868444  ·    1868456  ·     1868454  ·         15 %  ·          -  │
·-----------------------------------------|-------------|-------------|--------------|---------------|-------------·

After:

|  Contract     ·  Method                 ·  Min        ·  Max        ·  Avg         ·  # calls      ·  usd (avg)  │
················|·························|·············|·············|··············|···············|··············
|  ERC20        ·  approve                ·      46153  ·      46227  ·       46209  ·           10  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  releaseETH             ·          -  ·          -  ·       99929  ·            2  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  releaseToken           ·      76322  ·     110522  ·      101972  ·            4  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  releaseTokens          ·          -  ·          -  ·      286166  ·            1  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  sendFees               ·      63203  ·     146603  ·      119315  ·            7  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  sendFeesWithRoyalties  ·      69543  ·     155043  ·      116568  ·            4  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  setRoyaltiesWeight     ·          -  ·          -  ·       33815  ·            1  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  FeeSplitter  ·  updateShareholder      ·      36682  ·      36694  ·       36688  ·            2  ·          -  │
················|·························|·············|·············|··············|···············|··············
|  Deployments                            ·                                          ·  % of limit   ·             │
··········································|·············|·············|··············|···············|··············
|  FeeSplitter                            ·    1829618  ·    1829630  ·     1829628  ·       14.7 %  ·          -  │
·-----------------------------------------|-------------|-------------|--------------|---------------|-------------·