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 #149

Open
code423n4 opened this issue Aug 3, 2022 · 2 comments
Open

Gas Optimizations #149

code423n4 opened this issue Aug 3, 2022 · 2 comments

Comments

@code423n4
Copy link
Contributor

GAS

Constants expressions are expressions, not constants.

summary

Constant expressions are left as expressions, not constants.

details

Reference to this kind of issue: ethereum/solidity#9232

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L30-L43

mitigation

Use immutable for this expressions

Public function visibility can be made external

summary

Functions should have the strictest visibility possible. Public functions may lead to more gas usage by forcing the copy of their parameters to memory from calldata.

details

If a function is never called from the contract it should be marked as external. This will save gas.

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L241-L243

mitigation

Consider changing visibility from public to external.

usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

summary

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

details

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Use a larger size than downcast where needed

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L14

mitigation

Consider using some data type that uses 32 bytes, for example uint256

Explicit initialization

summary

It is not needed to initialize variables to the default value. Explicitly initializing it with its default value is an anti-pattern and wastes gas.

details

If a variable is not set/initialized, it is assumed to have the default value ( 0 for uint, false for bool, address(0) for address…).

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L94-L95
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L68

mitigation

Don't initialize variables to default value

Index initialized in for loop

summary

In for loops is not needed to initialize indexes to 0 as it is the default uint/int value. This saves gas.

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L98
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L69
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L207

mitigation

Don't initialize variables to default value

use of i++ in loop rather than ++i

summary

++i costs less gas than i++, especially when it's used in for loops

details

using ++i doesn't affect the flow of regular for loops and improves gas cost

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L123
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L204
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L168
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L114
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L207

mitigation

Substitute to ++i

increments can be unchecked in loops

summary

Unchecked operations as the ++i on for loops are cheaper than checked one.

details

In Solidity 0.8+, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline..

The code would go from:

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

to

for (uint256 i; i < numIterations;) {
  // ...
  unchecked { ++i; }
}
The risk of overflow is inexistent for a uint256 here.

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L123
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L195
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L207
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L292
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L17
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L69
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L116
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L98
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L114
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L168
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L123
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L204

mitigation

Add unchecked ++i at the end of all the for loop where it's not expected to overflow and remove them from the for header

.length should no be looked up in every loop of a for-loop

summary

In loops not assigning the length to a variable so memory accessed a lot (caching local variables)

details

The overheads outlined below are PER LOOP, excluding the first loop
storage arrays incur a Gwarmaccess (100 gas)
memory arrays use MLOAD (3 gas)
calldata arrays use CALLDATALOAD (3 gas)

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L123
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L204
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L168
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L114
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L116
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L98
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L17
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L207

mitigation

Assign the length of the array.length to a local variable in loops for gas savings

Variable should be declared out of a loop if it has always the same value

summary

If a variable is redeclare all the time in a loop, it will cost a lot of gas.

details

I think this variable has always the same value as the function method doesn't include any kind of argument and nothing is modified in the state between the iterations

github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L205

Mitigation

Move the declaration before the for loop as it is always use the same value

Variables should be cached when used several times

summary

loop length variables assigned to a local variable improves gas usage

details

In loops or state variables, this is even more gas saving

github permalinks

cache refundTokens[i]
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L118-L121
https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L208-L210

mitigation

Cache variables used more than one into a local variable.

abi.encode() is more efficient than abi.encodePacked()

Summary

In general, abi.encodePacked is more gas-efficient.

Details

Changing the abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient.

Github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L228

mitigation

Recommended Mitigation Steps
Change abi.encode to abi.encodePacked at line 355.

Functions guaranteed to revert when called by normal users can be marked payable

Summary

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function.

Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Details

The extra opcodes avoided are:
CALLVALUE (2), DUP1 (3), ISZERO (3), PUSH2 (3), JUMPI (10), PUSH1 (3), DUP1 (3), REVERT(0), JUMPDEST (1), POP (2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

Github permalinks

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/auth/AxelarAuthWeighted.sol#L47
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L120
https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L136

Mitigation

It's suggested to add payable to functions guaranteed to revert when called by normal users to improve gas costs

code423n4 added a commit that referenced this issue Aug 3, 2022
@re1ro
Copy link
Member

re1ro commented Aug 5, 2022

Dup #113 #2 #3 #18

@GalloDaSballo
Copy link
Collaborator

Constants expressions are expressions, not constants.

Debunked -> https://twitter.com/GalloDaSballo/status/1543729080926871557

Less than 500 gas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants