Open code423n4 opened 2 years ago
Only marginal gas improvements.
Messages.length -> 5 * 3 = 15 gas as it's calldata
Will save 100 gas
Saves 3 gas
Saves 100 gas * 6 = 600
Saves 20 gas * 3 = 60
2500
3 * 8 = 24
100 gas per instance
Saves 3 * 6 = 18 gas
Saves 20 gas per instance
Total Gas Saved 3440
[S]: Suggested optimation, save a decent amount of gas without compromising readability;
[M]: Minor optimation, the amount of gas saved is minor, change when you see fit;
[N]: Non-preferred, the amount of gas saved is at cost of readability, only apply when gas saving is a top priority.
[S] Cache storage variables in the stack can save gas
For the storage variables that will be accessed multiple times, cache them in the stack can save ~100 gas from each extra read (
SLOAD
after Berlin).For example:
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L205-L253
MessageProxyForMainnet.sol#postIncomingMessages()
messages.length
can be cached as it will be read for more than 5 times;https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L282-L303
connectedChains[targetChainHash].outgoingMessageCounter
at L296 can be cached to be reused at L302.[S] Cache array length in for loops can save gas
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
Caching the array length in the stack saves around 3 gas per iteration.
Instances include:
MessageProxyForMainnet.sol#initializeAllRegisteredContracts()
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L118-L126
[S] Remove redundant contains check for SE can save gas
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L77-L79
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/node_modules/@openzeppelin/contracts-upgradeable/utils/structs/EnumerableSetUpgradeable.sol#L53-L63
The check of
!_schainToERC20[schainHash].contains(tokens[i])
is already included in functionadd()
.Therefore,
!_schainToERC20[schainHash].contains(tokens[i])
is redundant.Removing it will make the code simpler and save some gas.
Recommendation
The same issue also exists in:
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L76-L81
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L333-L344
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L367-L368
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC721.sol#L76-L81
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/TokenManagers/TokenManagerERC20.sol#L249-L251
[M] Adding unchecked directive can save gas
For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.
For example:
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/CommunityPool.sol#L98-L101
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/CommunityPool.sol#L171-L173
_userWallets[user][schainHash] - amount
will never underflow.https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/MessageProxyForSchain.sol#L400-L404
MINIMUM_BALANCE - balance
will never underflow.[M] Use short reason strings can save gas
Every reason string takes at least 32 bytes.
Use short reason strings that fits in 32 bytes or it will become more expensive.
Instances include:
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L220-L222
[M]
++i
is more efficient thani++
Using
++i
is more gas efficient thani++
, especially in for loops.For example:
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L215-L224
Change to:
Other examples:
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L515-L522
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L118-L126
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L235-L250
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/Linker.sol#L149-L151
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L76-L81
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC721.sol#L260-L262
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC1155.sol#L79-L87
[M] Changing bool to uint256 can save gas
Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.
See: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/ReentrancyGuard.sol#L23-L27
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L98-L103
Recommendation
[M] Setting
uint256
variables to0
is redundantSetting
uint256
variables to0
is redundant as they default to0
.https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L234-L234
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L118-L118
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L235-L235
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/Linker.sol#L149-L151
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L76-L81
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC1155.sol#L79-L87
[N] Unnecessary checked arithmetic in for loops
There is no risk of overflow caused by increamenting the iteration index in for loops (the
++i
in forfor (uint256 i; i < count; ++i)
).Increments perform overflow checks that are not necessary in this case.
Recommendation
Surround the increment expressions with an
unchecked { ... }
block to avoid the default overflow checks. For example, change the for loop: