code-423n4 / 2021-10-pooltogether-findings

0 stars 0 forks source link

Cache array length in for loops can save gas #42

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

WatchPug

Vulnerability details

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Instances include:

https://github.com/pooltogether/v4-core/blob/055335bf9b09e3f4bbe11a788710dd04d827bf37/contracts/PrizeDistributor.sol#L68-L68

https://github.com/pooltogether/v4-core/blob/055335bf9b09e3f4bbe11a788710dd04d827bf37/contracts/DrawCalculator.sol#L196-L196

https://github.com/pooltogether/v4-core/blob/055335bf9b09e3f4bbe11a788710dd04d827bf37/contracts/PrizeDistributionBuffer.sol#L81-L81

https://github.com/pooltogether/v4-core/blob/055335bf9b09e3f4bbe11a788710dd04d827bf37/contracts/Ticket.sol#L260-L260

asselstine commented 3 years ago

https://github.com/pooltogether/v4-core/pull/235

PierrickGT commented 3 years ago

Duplicate of https://github.com/code-423n4/2021-10-pooltogether-findings/issues/7 Really not sure about these optimizations since these params are either calldata or memory, so it costs only 3 gas to access length. So storing in a variable will cost 3 more gas and it will still cost 3 gas per iteration.

GalloDaSballo commented 3 years ago

As the sponsor showed, by using https://github.com/crytic/evm-opcodes, there is no difference in gas cost when reading from memory or callData from the input vs caching it inside the function

The act of storing it locally via MSTORE will actually increase the gas cost by 3