Axelar Network v2 contest - NoamYakov's results

[1] Array out-of-bounds

Array out-of-bounds access causes a revert without any informative error message (it raises a Panic(0x32) exception).

Recommendation

Check that an index which is used to access an array element fits the array's boundaries before accessing that array element.

Code References

• AxelarAuthWeighted.sol: line 105 (operatorIndex must be < weights.length)

[1] Unnecessary MLOADs and CALLDATALOADs in for-each loops

There are many for loops that 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 from there. Memory and calldata reads are a bit more expensive than reading local variables.

Recommendation

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

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

Code References

• AxelarGateway.sol: line 207
• AxelarAuthWeighted.sol: lines 17, 98, 116
• AxelarDepositService.sol: lines 114, 168, 204
• AxelarGasService.sol: line 123

[2] Default value initialization

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc. depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas. For example:

uint256 x = 0;

can be optimized to:

uint256 x;

Code References

• AxelarGateway.sol: line 207
• AxelarAuthWeighted.sol: lines 68, 69, 94, 95, 98

[3] Prefix increments are cheaper than postfix increments

Use prefix increments (++x) instead of postfix increments (x++).

Recommendation

Change all postfix increments to prefix increments.

Code References

• AxelarGateway.sol: line 207
• AxelarDepositService.sol: lines 114, 168, 204
• AxelarGasService.sol: line 123

[4] Unnecessary checked arithmetic in for loops

There is no risk of overflow caused by increments to the iteration index in for loops (the i++ in for (uint256 i = 0; i < numIterations; 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 loop

for (uint256 i = 0; i < numIterations; i++) {
      // ...
}

to

for (uint256 i = 0; i < numIterations;) {
      // ...
      unchecked { i++; }
}

It is a little less readable but it saves a significant amount of gas.

Code References

• AxelarGateway.sol: lines 195, 207, 292
• AxelarAuthWeighted.sol: lines 17, 69, 98, 101, 109, 116
• AxelarDepositService.sol: lines 114, 168, 204
• AxelarGasService.sol: line 123

[5] Non-zero tests for unsigned integers

Testing != 0 is cheaper than testing > 0 for unsigned integers.

Recommendation

Use != 0 instead of > 0 when testing unsigned integers.

Code References

• AxelarGateway.sol: lines 255, 613
• AxelarAuthWeighted.sol: line 76
• AxelarDepositService.sol: line 165
• ReceiverImplementation.sol: lines 23, 51, 71
• AxelarGasService.sol: lines 128, 131

[6] Unnecessary array boundaries check when loading an array element twice

Some functions loads the same array element more than once. In such cases, only one array boundaries check should take place, and the rest are unnecessary. Therefore, this array element should be cached in a local variable and then be loaded again using that local variable, skipping the redundant second array boundaries check and saving gas.

Recommendation

Load the array elements once, cache them in local variables and then read them again using the local variables. For example:

uint256 item = array[i];
// do something with `item`
// do some other thing with `item`

Code References

• AxelarDepositService.sol: lines 118+121 (refundTokens[i]), 208+210 (refundTokens[i])