Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #214

Open
code423n4 opened this issue Aug 3, 2022 · 1 comment
Open

Gas Optimizations #214

code423n4 opened this issue Aug 3, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

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

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 3, 2022
code423n4 added a commit that referenced this issue Aug 3, 2022
@GalloDaSballo
Copy link
Collaborator

Less than 300 gas saved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants