Caching the length in for loops

[code423n4]

Handle

hrkrshnn

Vulnerability details

Caching the length in for loops

Consider a generic example of an array arr and the following loop:

for (uint i = 0; i < arr.length; i++) {
    // do something that doesn't change arr.length
}

In the above case, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

This extra costs can be avoided by caching the array length (in stack):

uint length = arr.length;
for (uint i = 0; i < length; i++) {
    // do something that doesn't change arr.length
}

In the above example, the sload or mload operation is only done once and subsequently replaced by a cheap dupN instruction.

This optimization is especially important if it is a storage array or if it is a lengthy for loop.

Note that the Yul based optimizer (not enabled by default; only relevant if you are using --experimental-via-ir or the equivalent in standard JSON) can sometimes do this caching automatically. However, this is likely not the case in your project.

Examples

./contracts/contracts/Auction.sol:81:        for (uint256 i = 0; i < inputTokens.length; i++) {
./contracts/contracts/Auction.sol:85:        for (uint256 i = 0; i < outputTokens.length; i++) {
./contracts/contracts/Auction.sol:96:        for (uint256 i = 0; i < pendingWeights.length; i++) {
./contracts/contracts/Auction.sol:142:        for (uint256 i = 0; i < bountyIds.length; i++) {
./contracts/contracts/Factory.sol:103:        for (uint256 i = 0; i < bProposal.weights.length; i++) {
./contracts/contracts/Basket.sol:64:            for (uint256 x = 0; x < tokenList.length; x++) {
./contracts/contracts/Basket.sol:225:        for (uint256 i = 0; i < weights.length; i++) {
./contracts/contracts/Basket.sol:231:        for (uint256 i = 0; i < weights.length; i++) {
./contracts/contracts/Basket.sol:238:        for (uint256 i = 0; i < weights.length; i++) {

[GalloDaSballo]

Would not take the gas savings at face value as some of the numbers apply only for storage

That said, caching the length does actually save gas (3 gas from memory) because to retrieve array.length you need to perform a math operation to get the memory position in which the length is stored

[GalloDaSballo]

Duplicate of #230