Gas Optimizations

[code423n4]

Gas opt

1 Using 1 modifier to check ROLE

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L116-L135 currently this contract is using 3 modifier to check which address is validated to run the function. Just use 1 modifier and pass which role will run the function then pass the address into the modifier argument:

modifier onlyRole(bytes32 _role) {
        require(hasRole(_role, msg.sender), "ROLE is required");
        _;
    }
//then run the function:
function foo(bytes) onlyRole(anyRole){}

2 Using && spend more gashttps://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/mainnet/DepositBox.sol#L77-L81

instead of using &&, using multiple require check can save execution gas fee:

require(
            schainHash != keccak256(abi.encodePacked("Mainnet")),
            "Receiver chain is incorrect"
        );
require(
            sender == schainLinks[schainHash],
            "Receiver chain is incorrect"
        );

3 Using new private function to validate connectedChains[value].inited

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L240 https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L258 https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L291 https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L315 create a new private function to run:

require(connectedChains[value].inited, "Error");

than call the code 4 times can save 18.018 gas when deploying the contract:

 function _foo(string memory _x)private view{
    require(connectedChains[_x].inited, "error");
}
 function removeConnectedChain(string memory schainName) public virtual override onlyChainConnector {
        bytes32 schainHash = keccak256(abi.encodePacked(schainName));
        _foo(schainName);
        delete connectedChains[schainHash];
    }    

4 better increment for saving more gas

Prefix increment are cheaper than postfix increment, its a common that using ++i instead i++ for compiler ^0.8.* .So this f() are not using prefix increments (++i) or using the unchecked (++i).

it can be seen from here : https://github.com/code-423n4/2022-01-xdefi-findings/issues/9 and

Occurance :

blob/main/contracts/schain/TokenManagers/TokenManagerERC1155.sol    #L500, #L514 
blob/main/contracts/schain/MessageProxyForSchain.sol            #L118, #L222 
blob/main/contracts/schain/TokenManagerLinker.sol           #L151, #L166, #L192     
blob/main/contracts/MessageProxy.sol                    #L221, #L515
blob/main/contracts/mainnet/Linker.sol                  #L100, #L149, #L175
blob/main/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol        #L76, #L276
blob/main/contracts/mainnet/DepositBoxes/DepositBoxERC721.sol       #L76, #L260
blob/main/contracts/mainnet/DepositBoxes/DepositBoxERC1155.sol      #L79, #L275, #L398, #L444, #L459

5 state variables that could be set as immutable

the erc721ContractOnSchain & receiverContractOnMainnet could be set as immutable to save more gas

Occurance :

blob/main/contracts/extensions/ERC721ReferenceMintAndMetadataSchain.sol #L34-35
blob/main/contracts/extensions/ERC721ReferenceMintAndMetadataMainnet.sol #L34

6 Custom errors from Solidity 0.8.4 are cheaper than revert strings.

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L173 instead of using string to revert the error message, use error to declare the custom error, then replace the revert string with it.

error errorMessage();

7 using ++var instead += 1

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/MessageProxy.sol#L302 change l 302 to:

++connectedChains[targetChainHash].outgoingMessageCounter;

8 Using storage can save gas

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/Messages.sol#L208 instead of caching message in memory, read it from storage can save gas. The message var is just called once inl L 213

 TransferEthMessage storage message = TransferEthMessage(
            BaseMessage(MessageType.TRANSFER_ETH),
            receiver,
            amount
        );

9 unnecessary length declaration

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/mainnet/Linker.sol#L173 length var is just used once in the L 175. Its unnecessary to chace it in memory. Remove the line and call _mainnetContracts.length() directly:

for (uint i = 0; connected && i <  _mainnetContracts.length(); i++)

10 Dont set uint i = 0

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/mainnet/MessageProxyForMainnet.sol#L118 by just declaring uint i; without set the value = 0 can save deployment gas fee. Do it on all for() loop

11 Set the default value on vars declaration

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/mainnet/MessageProxyForMainnet.sol#L73-L74 by setting the value of headerMessageGasCost and messageGasCost on their declaration, can save gas without writing storage in initialize(): https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/mainnet/MessageProxyForMainnet.sol#L298-L299

[yavrsky]

Only marginal gas improvements.

[GalloDaSballo]

Using 1 modifier to check ROLE

This does save gas on deploy but will not save gas on tx

Using && spend more

True, saved 3 gas

Using new private function to validate connectedChains[value].inited

Would save deployment cost but increase each call by 8 gas

better increment for saving more gas

21 * 3 = 63

state variables that could be set as immutable

Agree, that would save 2100 * 2 = 4200 gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings.

Agree but hard to quantify in lack of math

using ++var instead += 1

Agree, saves 3 gas

Using storage can save gas

Saves 6 gas

unnecessary length declaration

Disagree, length is cached which is necessary

Dont set uint i = 0

Costs an extra 3 gas

set the default value on vars declaration

Disagree as it changes the functionality

Total Gas saved: 4286