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

0 stars 0 forks source link

Unnecessary `CALLDATALOAD`s in for-each loops #74

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pants

Vulnerability details

The for loop in Swap.sweepFees() follows this for-each pattern:

for (uint256 i = 0; i < array.length; i++) {
    // do something with `array[i]`
}

In such for loops, the array.length is read on every iteration, instead of caching it once in a local variable and read it again using the local variable.

Impact

Calldata reads are a bit more expensive than reading local variables.

Tool Used

Manual code review.

Recommended Mitigation Steps

Read these values from calldata once, cache them in local variables and then read them again using the local variables. For example:

uint256 length = array.length;
for (uint256 i = 0; i < length; i++) {
    // do something with `array[i]`
}
Shadowfiend commented 2 years ago

Duplicate of #18.

0xean commented 2 years ago

This is similar but a different recommendation to #18 as this is referring to the repeated calls to .length which 18 doesn't suggest. Leaving as unique and valid