code-423n4 / 2022-01-yield-findings

1 stars 0 forks source link

Use immutable variables if values don't change after the constructor #120

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

IllIllI

Vulnerability details

Impact

Gas is wasted accessing storage when a variable never changes after the constructor. The immutable keyword is used to designate such variables. Immutable variables are like constant variables, except that they can be assigned in the constructor.

Proof of Concept

ICauldron public cauldron;

https://github.com/code-423n4/2022-01-yield/blob/e946f40239b33812e54fafc700eb2298df1a2579/contracts/ConvexYieldWrapper.sol#L17

This variable is only ever assigned in the constructor, so it should be labeled immutable. Here are the bytecode differences between what is in the repository, and adding the single immutable keyword to the line, leading to large gas savings:

$diff good bad
771a772,773
> PUSH1(0x60)
> SLOAD(0x54)
773c775,777
< PUSH32(0x7f)
---
> SWAP1(0x90)
> PUSH20(0x73)
> AND(0x16)
1631a1636,1637
> SLOAD(0x54)
> PUSH1(0x60)
1637c1643
< DUP3(0x82)
---
> DUP4(0x83)
1644,1645c1650
< SWAP1(0x90)
< PUSH32(0x7f)
---
> SWAP2(0x91)
2527a2533,2534
> SLOAD(0x54)
> PUSH1(0x60)
2533c2540
< DUP4(0x83)
---
> DUP5(0x84)
2540,2541c2547
< SWAP1(0x90)
< PUSH32(0x7f)
---
> SWAP2(0x91)
5079c5085,5088
< DUP7(0x86)
---
> PUSH1(0x60)
> SLOAD(0x54)
> DUP6(0x85)
> MLOAD(0x51)
5080a5090,5091
> DUP1(0x80)
> DUP11(0x8a)
5082,5083c5093
< PUSH32(0x7f)
< PUSH20(0x73)
---
> SWAP3(0x92)
5084a5095
> SWAP1(0x90)
5086,5089c5097,5101
< DUP8(0x87)
< DUP5(0x84)
< DUP2(0x81)
< MLOAD(0x51)
---
> SWAP1(0x90)
> DUP9(0x88)
> SWAP1(0x90)
> DUP6(0x85)
> SWAP1(0x90)
5200c5212,5215
< PUSH32(0x7f)
---
> PUSH1(0x60)
> SLOAD(0x54)
> DUP6(0x85)
> MLOAD(0x51)
5201a5217,5218
> SWAP1(0x90)
> SWAP2(0x91)
5202a5220
> SWAP1(0x90)
5204,5207c5222,5226
< DUP7(0x86)
< DUP4(0x83)
< DUP2(0x81)
< MLOAD(0x51)
---
> SWAP1(0x90)
> DUP8(0x87)
> SWAP1(0x90)
> DUP5(0x84)
> SWAP1(0x90)

Tools Used

Hardhat npx @remix-project/remix-lib

Recommended Mitigation Steps

Add immutable to variables only set during the constructor

devtooligan commented 2 years ago

dup of #124