You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
1. Use EIP-1167 minimal proxies for 10x cheaper contract instantiation
When new contracts have to be instantiated frequently, it's much cheaper for it to be done via minimal proxies. The only downside is that they rely on delegatecall() calls for every function, which adds an overhead of ~800 gas, but this is multiple orders of magnitude less than the amount saved during deployment.
Due to the following note, this optimization seem relevant for DepositReceiver:
File: AxelarDepositService.sol
92: // NOTE: `DepositReceiver` is destroyed in the same runtime context that it is deployed.
Affected code:
deposit-service/AxelarDepositService.sol:93: new DepositReceiver{ salt: salt }(
deposit-service/AxelarDepositService.sol:123: new DepositReceiver{ salt: salt }(
deposit-service/AxelarDepositService.sol:145: new DepositReceiver{ salt: salt }(
deposit-service/AxelarDepositService.sol:171: new DepositReceiver{ salt: salt }(
deposit-service/AxelarDepositService.sol:191: new DepositReceiver{ salt: salt }(
deposit-service/AxelarDepositService.sol:212: new DepositReceiver{ salt: salt }(
2. Multiple accesses of a mapping/array should use a local variable cache
Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time.
The code can be optimized by minimizing the number of SLOADs (here, notably on the gateway state variable).
SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.
64: IERC20(wrappedTokenAddress).approve(gateway, amount); //@audit gas SLOAD (gateway)65: IAxelarGateway(gateway).sendToken(destinationChain, destinationAddress, wrappedSymbol(), amount); //@audit gas SLOAD (gateway)
4. Duplicated conditions should be refactored to a modifier or function to save deployment costs
deposit-service/ReceiverImplementation.sol:35: if (amount ==0) revertNothingDeposited();
deposit-service/ReceiverImplementation.sol:60: if (amount ==0) revertNothingDeposited();
deposit-service/ReceiverImplementation.sol:82: if (amount ==0) revertNothingDeposited();
gas-service/AxelarGasService.sol:69: if (msg.value==0) revertNothingReceived();
gas-service/AxelarGasService.sol:84: if (msg.value==0) revertNothingReceived();
gas-service/AxelarGasService.sol:115: if (msg.value==0) revertNothingReceived();
gas-service/AxelarGasService.sol:121: if (receiver ==address(0)) revertInvalidAddress();
gas-service/AxelarGasService.sol:141: if (receiver ==address(0)) revertInvalidAddress();
gas-service/AxelarGasService.sol:155: if (amount ==0) revertNothingReceived();
gas-service/AxelarGasService.sol:169: if (amount ==0) revertNothingReceived();
gas-service/AxelarGasService.sol:161: if (!transferred || tokenAddress.code.length==0) revertTransferFailed();
gas-service/AxelarGasService.sol:177: if (!transferred || tokenAddress.code.length==0) revertTransferFailed();
xc20/contracts/XC20Wrapper.sol:68: if (axelarToken ==address(0)) revert('NotAxelarToken()');
xc20/contracts/XC20Wrapper.sol:78: if (wrappedToken ==address(0)) revert('NotAxelarToken()');
xc20/contracts/XC20Wrapper.sol:98: if (!transferred || tokenAddress.code.length==0) revert('TransferFailed()');
xc20/contracts/XC20Wrapper.sol:111: if (!transferred || tokenAddress.code.length==0) revert('TransferFailed()');
AxelarGateway.sol:504: if (!burnSuccess) revertBurnFailed(symbol);
AxelarGateway.sol:515: if (!burnSuccess) revertBurnFailed(symbol);
5. <array>.length should not be looked up in every loop of a for-loop
Reading array length at each iteration of the loop consumes more gas than necessary.
In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration.
In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.
Here, consider storing the array's length in a variable before the for-loop, and use this new variable instead:
auth/AxelarAuthWeighted.sol:17: for (uint256 i; i < recentOperators.length; ++i) {
auth/AxelarAuthWeighted.sol:98: for (uint256 i =0; i < signatures.length; ++i) {
auth/AxelarAuthWeighted.sol:116: for (uint256 i; i < accounts.length-1; ++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++) {
gas-service/AxelarGasService.sol:123: for (uint256 i; i < tokens.length; i++) {
AxelarGateway.sol:207: for (uint256 i =0; i < symbols.length; i++) {
6. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
Pre-increments and pre-decrements are cheaper.
For a uint256 i variable, the following is true with the Optimizer enabled at 10k:
Increment:
i += 1 is the most expensive form
i++ costs 6 gas less than i += 1
++i costs 5 gas less than i++ (11 gas less than i += 1)
Decrement:
i -= 1 is the most expensive form
i-- costs 11 gas less than i -= 1
--i costs 5 gas less than i-- (16 gas less than i -= 1)
Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:
uint i =1;
uint j =2;
require(j == i++, "This will be false as i is incremented after the comparison");
However, pre-increments (or pre-decrements) return the new value:
uint i =1;
uint j =2;
require(j ==++i, "This will be true as i is incremented before the comparison");
In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.
Affected code:
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++) {
gas-service/AxelarGasService.sol:123: for (uint256 i; i < tokens.length; i++) {
AxelarGateway.sol:207: for (uint256 i =0; i < symbols.length; i++) {
Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).
7. Increments/decrements can be unchecked in for-loops
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.
Consider wrapping with an unchecked block here (around 25 gas saved per instance):
auth/AxelarAuthWeighted.sol:17: for (uint256 i; i < recentOperators.length; ++i) {
auth/AxelarAuthWeighted.sol:69: for (uint256 i =0; i < weightsLength; ++i) {
auth/AxelarAuthWeighted.sol:98: for (uint256 i =0; i < signatures.length; ++i) {
auth/AxelarAuthWeighted.sol:101: for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}
auth/AxelarAuthWeighted.sol:116: for (uint256 i; i < accounts.length-1; ++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++) {
gas-service/AxelarGasService.sol:123: for (uint256 i; i < tokens.length; i++) {
AxelarGateway.sol:195: for (uint256 i; i < adminCount; ++i) {
AxelarGateway.sol:207: for (uint256 i =0; i < symbols.length; i++) {
AxelarGateway.sol:292: for (uint256 i; i < commandsLength; ++i) {
The change would be:
- for (uint256 i; i < numIterations; i++) {+ for (uint256 i; i < numIterations;) {
// ...
+ unchecked { ++i; }
}
The same can be applied with decrements (which should use break when i == 0).
The risk of overflow is non-existant for uint256 here.
8. Upgrade pragma
Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.
The advantages here are:
Contract existence checks (>= 0.8.10): external calls skip contract existence checks if the external call has a return value
Consider upgrading from 0.8.9 to at least 0.8.10 in the solution.
9. (Not recommended, but true) Functions guaranteed to revert when called by normal users can be marked payable
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.
auth/AxelarAuthWeighted.sol:47: function transferOperatorship(bytescalldataparams) external onlyOwner {
gas-service/AxelarGasService.sol:120: function collectFees(address payablereceiver, address[] calldatatokens) external onlyOwner {
xc20/contracts/XC20Wrapper.sol:44: function setXc20Codehash(bytes32newCodehash) external onlyOwner {
xc20/contracts/XC20Wrapper.sol:66: function removeWrapping(stringcalldatasymbol) external onlyOwner {
AxelarGateway.sol:204: function setTokenDailyMintLimits(string[] calldatasymbols, uint256[] calldatalimits) externaloverride onlyAdmin {
AxelarGateway.sol:331: function deployToken(bytescalldataparams, bytes32) external onlySelf {
AxelarGateway.sol:367: function mintToken(bytescalldataparams, bytes32) external onlySelf {
AxelarGateway.sol:373: function burnToken(bytescalldataparams, bytes32) external onlySelf {
AxelarGateway.sol:397: function approveContractCall(bytescalldataparams, bytes32commandId) external onlySelf {
AxelarGateway.sol:411: function approveContractCallWithMint(bytescalldataparams, bytes32commandId) external onlySelf {
AxelarGateway.sol:437: function transferOperatorship(bytescalldatanewOperatorsData, bytes32) external onlySelf {
The text was updated successfully, but these errors were encountered:
2. Multiple accesses of a mapping/array should use a local variable cache
I seem to be getting between 20 and 40 gas savings in caching, I can't quite explain why this would happen for calldata.
For storage the position is recomputed, and that takes a keccak (30 gas) + costs for handling memory, but for calldata it's just an offset (I'd assume 3 gas) + calldata load (up to 12 gas on evm.codes)
Will give it 20 gas per instance per calldata * 2 =
40
40 for Storage * 5 =
200
3. Caching Storage into Memory
First time saves 94 gas (100 - 6 - SLOAD + MLOAD), each subsequent is 97 (100 - 3 - SLOAD)
94+94+97+94
5 + 6 + 7
Around 300 gas like other submissions
Upgrade pragma
Skipping external calls will save about 100 gas per instance, however no instances were listed, giving it 100 gas
In this case I don't think 1 is applicable, same for the last function as while it does save gas it's a trade-off
All in all way better than average report, highly recommend the sponsor to follow the advice
Overview
Table of Contents:
<array>.length
should not be looked up in every loop of afor-loop
++i
costs less gas compared toi++
ori += 1
(same for--i
vsi--
ori -= 1
)payable
1. Use EIP-1167 minimal proxies for 10x cheaper contract instantiation
When new contracts have to be instantiated frequently, it's much cheaper for it to be done via minimal proxies. The only downside is that they rely on
delegatecall()
calls for every function, which adds an overhead of ~800 gas, but this is multiple orders of magnitude less than the amount saved during deployment.Due to the following note, this optimization seem relevant for
DepositReceiver
:Affected code:
2. Multiple accesses of a mapping/array should use a local variable cache
Caching a mapping's value in a local
storage
orcalldata
variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time.Affected code:
refundTokens[i]
(calldata)wrapped[axelarToken]
andunwrapped[xc20Token]
(storage)3. Caching storage values in memory
The code can be optimized by minimizing the number of SLOADs (here, notably on the
gateway
state variable).SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.
4. Duplicated conditions should be refactored to a modifier or function to save deployment costs
5.
<array>.length
should not be looked up in every loop of afor-loop
Reading array length at each iteration of the loop consumes more gas than necessary.
In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration.
In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.
Here, consider storing the array's length in a variable before the for-loop, and use this new variable instead:
6.
++i
costs less gas compared toi++
ori += 1
(same for--i
vsi--
ori -= 1
)Pre-increments and pre-decrements are cheaper.
For a
uint256 i
variable, the following is true with the Optimizer enabled at 10k:Increment:
i += 1
is the most expensive formi++
costs 6 gas less thani += 1
++i
costs 5 gas less thani++
(11 gas less thani += 1
)Decrement:
i -= 1
is the most expensive formi--
costs 11 gas less thani -= 1
--i
costs 5 gas less thani--
(16 gas less thani -= 1
)Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:
However, pre-increments (or pre-decrements) return the new value:
In the pre-increment case, the compiler has to create a temporary variable (when used) for returning
1
instead of2
.Affected code:
Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).
7. Increments/decrements can be unchecked in for-loops
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.
ethereum/solidity#10695
Consider wrapping with an
unchecked
block here (around 25 gas saved per instance):The change would be:
The same can be applied with decrements (which should use
break
wheni == 0
).The risk of overflow is non-existant for
uint256
here.8. Upgrade pragma
Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.
The advantages here are:
Consider upgrading from
0.8.9
to at least0.8.10
in the solution.9. (Not recommended, but true) Functions guaranteed to revert when called by normal users can be marked
payable
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 aspayable
will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.The text was updated successfully, but these errors were encountered: