AstrolabDAO / strats

Home of Astrolab strategies
Other
15 stars 11 forks source link

[ASS-02C] Inefficient Loop Limit Evaluations #35

Open pde-rent opened 7 months ago

pde-rent commented 7 months ago

Recommendation:

We advise the statements within the for loop limits to be relocated outside to a local variable declaration that is consequently utilized for the evaluations to significantly reduce the codebase's gas cost. We should note the same optimization is applicable for storage reads present in those limits as they are newly read on each iteration (i.e. length members of arrays in storage).

This seems to me like a basic compiler optimization, are you sure that solc+pragma >8.20 does not take care of it already?

omniscia-core commented 7 months ago

Sadly, the compiler is much less smarter than people think. You can attempt the optimization yourself and observe that all warm SLOAD operations are not present:

contract Foo {
    uint256[] public values;

    constructor() {
        for (uint256 i = 0; i< 100; i++) {
            values.push(i);
        }
    } 

    // 259524 w/ limit as values.length and optimizations on
    // 249525 w/ limit as 100 and optimizations on
    function test() external view returns (uint256 total) {
        for (uint256 i = 0; i < 100; i++) {
            total += values[i];
        }
    }
}

The compiler is unable to detect whether the dynamic type the length is evaluated from is mutated within its body.

pde-rent commented 6 months ago

Insightful, thank you @omniscia-core. I will make sure the team gets compiler-versed.