Unnecessary checked arithmetic in for loops

[code423n4]

Handle

WatchPug

Vulnerability details

There is no risk of overflow caused by increamenting the iteration index in for loops (the i++ in for for (uint256 i; i < _registry[product].length(); i++)).

Increments perform overflow checks that are not necessary in this case.

Recommendation

Surround the increment expressions with an unchecked { ... } block to avoid the default overflow checks. For example, change the for loop:

https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/incentivizer/Incentivizer.sol#L144-L151

for (uint256 i; i < _registry[product].length(); i++) {
    uint256 programId = _registry[product].at(i);

    if (_programs[programId].versionComplete != 0) continue;
    if (!_programInfos[programId].isComplete(currentTimestamp)) continue;

    completeInternal(programId);
}

to:

for (uint256 i; i < _registry[product].length();) {
    uint256 programId = _registry[product].at(i);

    if (_programs[programId].versionComplete != 0) continue;
    if (!_programInfos[programId].isComplete(currentTimestamp)) continue;

    completeInternal(programId);

    unchecked { ++i; }
}

[kbrizzle]

Good callout, we likely will not incorporate this since due to the readability hit versus the gas gains.

[GalloDaSballo]

Agree with finding, I believe pre-increment saves about 3 gas (MSTORE) and unchecked is about 30 / 40 gas per operation unchecked