code-423n4 / 2022-08-mimo-findings

0 stars 0 forks source link

Gas Optimizations #151

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

GAS

Using bools for storage incurs overhead

Summary

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.

Details

Here is one example of OpenZeppelin about this optimization https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/interfaces/IMIMOProxy.sol#L67 https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxyFactory.sol#L24 https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/managed/interfaces/IMIMOManagedAction.sol#L13 https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/automated/interfaces/IMIMOAutoAction.sol#L9 https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/managed/MIMOManagedAction.sol#L17

Mitigation

Using private rather than public for constants saves gas

Summary

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxyFactory.sol#L19

Mitigation

Consider replacing public for private in constants for gas saving.

Index initialized in for loop

Summary

In for loops is not needed to initialize indexes to 0 as it is the default uint/int value. This saves gas.

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L132

Mitigation

Don't initialize variables to default value

use of i++ in loop rather than ++i

Summary

++i costs less gas than i++, especially when it's used in for loops

Details

using ++i doesn't affect the flow of regular for loops and improves gas cost

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L132

Mitigation

Substitute to ++i

increments can be unchecked in loops

Summary

Unchecked operations as the ++i on for loops are cheaper than checked one.

Details

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..

The code would go from:

for (uint256 i; i < numIterations; i++) {
// ...
}

to

for (uint256 i; i < numIterations;) {
  // ...
  unchecked { ++i; }
}
The risk of overflow is inexistent for a uint256 here.

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L132

Mitigation

Add unchecked ++i at the end of all the for loop where it's not expected to overflow and remove them from the for header

.length should no be looked up in every loop of a for-loop

Summary

In loops not assigning the length to a variable so memory accessed a lot (caching local variables)

Details

The overheads outlined below are PER LOOP, excluding the first loop storage arrays incur a Gwarmaccess (100 gas) memory arrays use MLOAD (3 gas) calldata arrays use CALLDATALOAD (3 gas)

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L132

Mitigation

Assign the length of the array.length to a local variable in loops for gas savings

Functions guaranteed to revert when called by normal users can be marked payable

Summary

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.

Details

The extra opcodes avoided are: CALLVALUE (2), DUP1 (3), ISZERO (3), PUSH2 (3), JUMPI (10), PUSH1 (3), DUP1 (3), REVERT(0), JUMPDEST (1), POP (2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

Github Permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L117 https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L104

Mitigation

Consider adding payable to functions guaranteed to revert when called by normal users to improve gas costs

Variables should be cached when used several times

Summary

Caching variables improves gas usage

details

address owner_ = owner; can be cached at the beginning for the checks of that block, so it's improved the gas by not reading state variable owner 2 extra times.

Github permalinks

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L72

Mitigation

Cache variables used more than one into a local variable.

gzeoneth commented 2 years ago

https://github.com/code-423n4/2022-08-mimo/tree/main/docs#non-gas-optimized-syntax