code423n4 commented 3 years ago

Handle

ye0lde

Vulnerability details

Impact

Additional gas usage when an array's length value is used directly in a for loop.

Proof of Concept

The array's length value is used directly in a for loop here: https://github.com/tracer-protocol/perpetual-pools-contracts/blob/646360b0549962352fe0c3f5b214ff8b5f73ba51/contracts/implementation/PoolKeeper.sol#L77 https://github.com/tracer-protocol/perpetual-pools-contracts/blob/646360b0549962352fe0c3f5b214ff8b5f73ba51/contracts/implementation/PoolKeeper.sol#L125

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Change the loops above from: for (uint256 i=0; i < pools.length; i++)

to unit256 length = pools.length; for (uint256 i=0; i < length; i++)

When I tested these changes there was a small gas savings.

CalabashSquash commented 3 years ago

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

GalloDaSballo commented 3 years ago

The sponsor confirms However, this is not correct, as per another contest, reading from calldata (performing a CALLDATAREAD) has the same cost as reading from memory Adding the extra memory variable would cost 3 extra gas

Will set to invalid unless the sponsor implements the finding

The suggestion will cost an extra 3 gas though

Source: https://github.com/crytic/evm-opcodes