explain to users why an operation failed through the use of custom errors. Until
now, you could already use strings to give more information about failures (e.g.,
revert("Insufficient funds.");), but they are rather expensive, especially when it
comes to deploy cost, and it is difficult to use dynamic information in them.
Custom errors are defined using the error statement, which can be used inside and
outside of contracts (including interfaces and libraries).
require(_locked == 0, "LOCKED");
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L133
There are several other places throughout the codebase where the same optimization
can be used.
require(hasRole(CHAIN_CONNECTOR_ROLE, msg.sender), "CHAIN_CONNECTOR_ROLE is
required");
Tools
Manual analysis
Recommended Mitigation Steps
Replace revert strings with custom errors.
7. Cache array length in for loops can save gas
Impact
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.
1.Not needed check for uint > 0
Impact
The following functions check that an uint > 0 but it's always true.
Proof of Concept
https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/
DepositBoxEth.sol#L122
There are several other places throughout the codebase where the same optimization
can be used.
Tools Used
Manual analysis
Recommended Mitigation Steps
Remove the checks.
2.Prefix increaments are cheaper than postfix increaments
Impact
The functions use prefix increaments (i ++) instead of postfix increaments (++ i). Prefix increaments are cheaper than postfix increaments.
Proof of Concept
https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/
DepositBoxERC721.sol#L76 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/
DepositBoxERC721.sol#L260 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/
DepositBoxERC1155.sol#L79 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/
DepositBoxERC1155.sol#L275
There are several other places throughout the codebase where the same optimization
can be used.
Tools Used
Manual analysis
Recommended Mitigation Steps
Change all prefix increaments to postfix increaments where it doesn't affects the
functionality.
3.Long Revert Strings
Impact
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and
will decrease runtime gas when the revert condition has been met. Revert strings that are longer than 32 bytes require at least one additional
mstore, along with additional overhead for computing memory offset, etc.
Proof of Concept
https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L152 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L171 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L186-L190 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L191 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L217 There are several other places throughout the codebase where the same optimization
can be used.
Tools Used
Manual analysis
Recommended Mitigation Steps
Shorten the revert strings to fit in 32 bytes. Or consider moving to solc 0.8.4 or
greater and use Custom Errors.
4.Unnecessary initialization of loop index variable
Impact
For loop indices across the contract functions use explicit 0 initializations which
are not required because the default value of uints is 0. Removing this explicit
unnecessary initialization will save a little gas.
Proof of Concept
https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/Linker.sol#L1
00 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/Linker.sol#L1
49 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/Linker.sol#L1
75 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L118 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyF
orMainnet.sol#L235 There are several other places throughout the codebase where the same optimization
can be used.
Tools Used
Manual analysis
Recommended Mitigation Steps
Remove unnecessary initialization of loop index variable
5.Change string to byteX if possible
Impact
Declaring in the string public schainName` with type bytes32 can save gas.
Proof of Concept
https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/extensions/ERC721Refe
renceMintAndMetadataMainnet.sol#L36
Tools
Manual analysis
6.Use Custom Errors to save Gas
Impact
Custom errors from Solidity 0.8.4 are cheaper than revert strings. Source: https://blog.soliditylang.org/2021/04/21/custom-errors/: Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to
explain to users why an operation failed through the use of custom errors. Until
now, you could already use strings to give more information about failures (e.g.,
revert("Insufficient funds.");), but they are rather expensive, especially when it
comes to deploy cost, and it is difficult to use dynamic information in them. Custom errors are defined using the error statement, which can be used inside and
outside of contracts (including interfaces and libraries). require(_locked == 0, "LOCKED");
Proof of Concept
https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L117 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L125 https://github.com/skalenetwork/ima-c4-
audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L133 There are several other places throughout the codebase where the same optimization
can be used.
Tools
Manual analysis
Recommended Mitigation Steps
Replace revert strings with custom errors.
7. Cache array length in for loops can save gas
Impact
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.
Proof of Concept
https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC1155.sol#L265 function getFunds( string calldata schainName, address erc1155OnMainnet, address receiver, uint256[] memory ids, uint256[] memory amounts ) external override onlySchainOwner(schainName) whenKilled(keccak256(abi.encodePacked(schainName))) { bytes32 schainHash = keccak256(abi.encodePacked(schainName)); require(ids.length == amounts.length, "Incorrect length of arrays"); for (uint256 i = 0; i < ids.length; i++) {
Tools Used
Manual analysis
Recommended Mitigation Steps
Consider to cache array length.