search

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

0 stars 0 forks source link

array length can be cached to save gas #7

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

pants

Vulnerability details

At some places, array length is not cached. PrizePool.sol line 242, Ticket.sol line 253, PrizeDistributionHistory.sol line 69, DrawPrize.sol line 66, DrawCalculator.sol line 272, DrawCalculator.sol line 247, DrawCalculator.sol line 199, DrawCalculator.sol line 155, ObservationHaness.sol line 17, TwabLibraryExposed.sol line 32, ReserveHarness.sol line 27

At other cases it's cached.

PierrickGT commented 3 years ago

I've disputed the issue for the following reasons. My reasoning will be based on the following EVM opcodes table: https://github.com/crytic/evm-opcodes

PrizePool No real optimization here since _tokenIds is passed as a calldata, so each subsequent calls to _tokenIds.length will cost only 3 gas for a CALLDATALOAD. It would actually cost 3 more in gas if we store the length in a variable since MSTORE cost 3 in gas and each subsequent MLOAD calls would still cost 3 in gas.

Ticket Same reasoning here, _startTimes and _endTimes are passed as calldata.

PrizeDistributionBuffer Same reasoning, _drawIds is passed as calldata.

PrizeDistributor drawPayouts is stored in memory so each subsequent calls to length will only cost 3 in gas.

DrawCalculator _draws are passed in memory so each subsequent calls are only worth 3 in gas.

ObservationLibHarness This is a contract for testing purpose, we don't care about gas optimization.

TwabLibExposed Same remark. This is a contract for tests purpose.

ReserveHarness This is also a contract used for tests.

PierrickGT commented 3 years ago

Duplicate of https://github.com/code-423n4/2021-10-pooltogether-findings/issues/42