Gas Optimization in `Most` contract

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x9e52ea7c8bb12aa9f406bb87cbe2f3373bfdd0c17a9dba37efb5ce4dee758dc4 Severity: minor

Description:

[G-01] State variable can be cached outside of the loop

Description

In _setCommittee function the state variable committeeId can be cache outside of the loop this can 1 SLOAD(~100 Gas) per loop iteration.

File : master/eth/contracts/Most.sol

105: function _setCommittee(
106:        address[] calldata _committee,
107:        uint256 _signatureThreshold
108:    ) internal {
109:        if (_signatureThreshold == 0) revert ZeroSignatureTreshold();
110:        if (_committee.length < _signatureThreshold)
111:            revert NotEnoughGuardians();
112:
113:        for (uint256 i; i < _committee.length; ++i) {
114:            bytes32 committeeMemberId = keccak256(
115:                abi.encodePacked(committeeId, _committee[i])
116:            );
117:            // avoid duplicates
118:            if (committee[committeeMemberId]) {
119:                revert DuplicateCommitteeMember();
120:            }
121:            committee[committeeMemberId] = true;
122:        }
123:
124:        committeeSize[committeeId] = _committee.length;
125:        signatureThreshold[committeeId] = _signatureThreshold;
126:    }

Most.sol#L105-L126

Proof Of Concept(POC):

Since committeeId is state variable it is reading in a loop but does not depend on the loop. So it can be cached in a stack variable outside of the loop saves 1 SLOAD per iteration

Recommended Mitigation Steps:

File : master/eth/contracts/Most.sol

105: function _setCommittee(
106:        address[] calldata _committee,
107:        uint256 _signatureThreshold
108:    ) internal {
109:        if (_signatureThreshold == 0) revert ZeroSignatureTreshold();
110:        if (_committee.length < _signatureThreshold)
111:            revert NotEnoughGuardians();
112:
+        // Cache committeeId outside of the loop
+           uint256 _committeeId = committeeId;
113:        for (uint256 i; i < _committee.length; ++i) {
114:            bytes32 committeeMemberId = keccak256(
-115:                abi.encodePacked(committeeId, _committee[i])
+                  abi.encodePacked(_committeeId, _committee[i])
116:            );
117:            // avoid duplicates
118:            if (committee[committeeMemberId]) {
119:                revert DuplicateCommitteeMember();
120:            }
121:            committee[committeeMemberId] = true;
122:        }
123:
124:        committeeSize[committeeId] = _committee.length;
125:        signatureThreshold[committeeId] = _signatureThreshold;
126:    }

[G-02] ++i/i++ should be unchecked when it is not possible for them to overflow

Description

When it is not possible for the loop counter i to overflow using unchecked for ++i can save gas. This recommendation applies to Solidity version 0.8.0 or higher. Applying unchecked to such instances saves approximately 30-40 gas per loop iteration.

File : master/eth/contracts/Most.sol

105: function _setCommittee(
106:        address[] calldata _committee,
107:        uint256 _signatureThreshold
108:    ) internal {
109:        if (_signatureThreshold == 0) revert ZeroSignatureTreshold();
110:        if (_committee.length < _signatureThreshold)
111:            revert NotEnoughGuardians();
112:
113:        for (uint256 i; i < _committee.length; ++i) {
114:            bytes32 committeeMemberId = keccak256(
115:                abi.encodePacked(committeeId, _committee[i])
116:            );
117:            // avoid duplicates
118:            if (committee[committeeMemberId]) {
119:                revert DuplicateCommitteeMember();
120:            }
121:            committee[committeeMemberId] = true;
122:        }
123:
124:        committeeSize[committeeId] = _committee.length;
125:        signatureThreshold[committeeId] = _signatureThreshold;
126:    }

Most.sol#L105-L126

Proof Of Concept(POC):

In this example the for loop post condition i.e. ++i involves checked arithmetic which is not required. This is because the value of i is always strictly less than _committee.length. Therefore, the theoretical maximum value of i to enter the for-loop body is _committee.length. This means that the ++i in the for loop can never overflow.

Recommended Mitigation Steps:

File : master/eth/contracts/Most.sol

105: function _setCommittee(
106:        address[] calldata _committee,
107:        uint256 _signatureThreshold
108:    ) internal {
109:        if (_signatureThreshold == 0) revert ZeroSignatureTreshold();
110:        if (_committee.length < _signatureThreshold)
111:            revert NotEnoughGuardians();
112:
-113:        for (uint256 i; i < _committee.length; ++i) {
+         for (uint256 i; i < _committee.length; unchecked{++i}) {
114:            bytes32 committeeMemberId = keccak256(
115:                abi.encodePacked(committeeId, _committee[i])
116:            );
117:            // avoid duplicates
118:            if (committee[committeeMemberId]) {
119:                revert DuplicateCommitteeMember();
120:            }
121:            committee[committeeMemberId] = true;
122:        }
123:
124:        committeeSize[committeeId] = _committee.length;
125:        signatureThreshold[committeeId] = _signatureThreshold;
126:    }

[G-03] Do not initialize state variable with default value

Description

Do not initialize committeeId with the default value of the uint and int type variables which is 0

File : master/eth/contracts/Most.sol

83:    function initialize(
84:        address[] calldata _committee,
85:        uint256 _signatureThreshold,
86:        address owner,
87:        address payable _wethAddress
88:    ) public initializer {
89:        committeeId = 0;

Most.sol#L83-L89

Proof Of Concept(POC):

Since the default value for uint/int variables is 0. So do not initialize a variable first time with 0 value

Recommended Mitigation Steps:

File : master/eth/contracts/Most.sol

83:    function initialize(
84:        address[] calldata _committee,
85:        uint256 _signatureThreshold,
86:        address owner,
87:        address payable _wethAddress
88:    ) public initializer {
-89:        committeeId = 0;

[krzysztofziobro]

Gas optimization is out of scope