search

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

0 stars 0 forks source link

Gas Optimizations #170

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[G-01] Cache storage values in memory

Impact

Anytime you are reading from storage more than once, it is cheaper to cache variables in memory. An SLOAD cost 100 GAS while MLOAD and MSTORE only cost 3 GAS.

This is especially true in for loops when using the length of a storage array as the condition being checked after each loop. Caching the array length in memory can yield significant gas savings when the array length is high.

// Before
for(uint 1; i < approvals.length; i++); // approvals is a storage variable

// After
uint memory numApprovals = approvals.length;
for(uint 1; i < numApprovals; i++);

POC

auth/AxelarAuthWeighted.sol:98:        for (uint256 i = 0; i < signatures.length; ++i) {
AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {
deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {

[G-02] No need to explicitly initialize variables with default values

Impact

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…). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

POC

auth/AxelarAuthWeighted.sol:69:        for (uint256 i = 0; i < weightsLength; ++i) {
auth/AxelarAuthWeighted.sol:98:        for (uint256 i = 0; i < signatures.length; ++i) {
AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {

Mitigation

As an example: for (uint256 i = 0; i < weightsLength;) { should be replaced with for (uint256 i; i < weightLength;) {

[G-03] Increments can be unchecked

Impact

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.

Instances include:

deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {

The code would go from:

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

to:

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

The risk of overflow is inexistant for a uint256 here.

[G-04] ++i costs less gas compared to i++ or i += 1

Impact

++i costs less gas compared to i++ or i += 1 for unsigned integers. This is because the pre-increment operation is cheaper (about 5 GAS per iteration). This is implemented in other sol files but looks like its missing from Vault.sol .

POC

deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {
re1ro commented 2 years ago

Dup #18 #2

GalloDaSballo commented 2 years ago

300 gas as other loop like reports