code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

Gas Optimizations #214

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

FINDINGS

Using unchecked blocks to save gas - Increments in for loop can be unchecked ( save 30-40 gas per loop iteration)

The majority of Solidity for loops increment a uint256 variable that starts at 0. These increment operations never need to be checked for over/underflow because the variable will never reach the max number of uint256 (will run out of gas long before that happens). The default over/underflow check wastes gas in every iteration of virtually every for loop . eg.

e.g Let's work with a sample loop below.

for(uint256 i; i < 10; i++){
//doSomething
}

can be written as shown below.

for(uint256 i; i < 10;) {
  // loop logic
  unchecked { i++; }
}

We can also write it as an inlined function like below.

function inc(i) internal pure returns (uint256) {
  unchecked { return i + 1; }
}
for(uint256 i; i < 10; i = inc(i)) {
  // doSomething
}

Affected code

File: AxelarGasService.sol line 123

    function collectFees(address payable receiver, address[] calldata tokens) external onlyOwner {
        if (receiver == address(0)) revert InvalidAddress();

        for (uint256 i; i < tokens.length; i++) {

Other Instances to modify File: AxelarDepositService.sol line 114

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarDepositService.sol line 168

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarDepositService.sol line 204

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarAuthWeighted.sol line 17

        for (uint256 i; i < recentOperators.length; ++i) {

File: AxelarAuthWeighted.sol line 69

        for (uint256 i = 0; i < weightsLength; ++i) {

File: AxelarAuthWeighted.sol line 98

        for (uint256 i = 0; i < signatures.length; ++i) {

File: AxelarAuthWeighted.sol line 116

        for (uint256 i; i < accounts.length - 1; ++i) {

see resource

Cache the length of arrays in loops (saves ~6 gas per iteration)

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

The solidity compiler will always read the length of the array during each iteration. That is,

1.if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929 2) for each iteration except for the first), 2.if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first), 3.if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)

This extra costs can be avoided by caching the array length (in stack): When reading the length of an array, sload or mload or calldataload operation is only called once and subsequently replaced by a cheap dupN instruction. Even though mload , calldataload and dupN have the same gas cost, mload and calldataload needs an additional dupN to put the offset in the stack, i.e., an extra 3 gas. which brings this to 6 gas

Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead:

File: AxelarGasService.sol line 123

    function collectFees(address payable receiver, address[] calldata tokens) external onlyOwner {
        if (receiver == address(0)) revert InvalidAddress();

        for (uint256 i; i < tokens.length; i++) {

The above should be modified to

    function collectFees(address payable receiver, address[] calldata tokens) external onlyOwner {
        if (receiver == address(0)) revert InvalidAddress();

                uint256 length = tokens.length;
        for (uint256 i; i < length; i++) {

Other instances to modify File: AxelarDepositService.sol line 114

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarDepositService.sol line 168

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarDepositService.sol line 204

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarAuthWeighted.sol line 17

        for (uint256 i; i < recentOperators.length; ++i) {

File: AxelarAuthWeighted.sol line 98

        for (uint256 i = 0; i < signatures.length; ++i) {

File: AxelarAuthWeighted.sol line 116

        for (uint256 i; i < accounts.length - 1; ++i) {

++i costs less gas compared to i++ or i += 1 (~5 gas per iteration)

++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2

Instances include: File: AxelarGasService.sol line 123

        for (uint256 i; i < tokens.length; i++) {

File: AxelarDepositService.sol line 114

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarDepositService.sol line 168

        for (uint256 i; i < refundTokens.length; i++) {

File: AxelarDepositService.sol line 204

        for (uint256 i; i < refundTokens.length; i++) {

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) Custom errors save ~50 gas each time they’re hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries). see Source

All the contracts in scope are already using custom errors apart from File: XC20Wrapper.sol in the following lines

File: XC20Wrapper.sol line 55

        if (axelarToken == address(0)) revert('NotAxelarToken()');

File: XC20Wrapper.sol line 56

        if (xc20Token.codehash != xc20Codehash) revert('NotXc20Token()');

File: XC20Wrapper.sol line 57

        if (wrapped[axelarToken] != address(0)) revert('AlreadyWrappingAxelarToken()');

File: XC20Wrapper.sol line 58

        if (unwrapped[xc20Token] != address(0)) revert('AlreadyWrappingXC20Token()');

File: XC20Wrapper.sol line 61

        if (!LocalAsset(xc20Token).set_team(address(this), address(this), address(this))) revert('NotOwner()');

File: XC20Wrapper.sol line 62

        if (!LocalAsset(xc20Token).set_metadata(newName, newSymbol, IERC20(axelarToken).decimals())) revert('CannotSetMetadata()');

More instances

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L68 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L70 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L78 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L79 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L84 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L85 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L86 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L98 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L111

GalloDaSballo commented 2 years ago

Less than 300 gas saved