Mimo August 2022 contest - durianSausage's results

low risk foundings

L01: UNUSED/EMPTY RECEIVE()/FALLBACK() FUNCTION

problem

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

prof

proxy/MIMOProxy.sol, 38, receive() external payable {}

###L02: address varible value should be checked before assigned.

problom

we should ensure the address varible not equal zero account.

prof

MIMOProxy.sol, 117-124

  function transferOwnership(address newOwner) external override {
    address oldOwner = owner;
    if (oldOwner != msg.sender) {
      revert CustomErrors.NOT_OWNER(oldOwner, msg.sender);
    }
    owner = newOwner;
    emit TransferOwnership(oldOwner, newOwner);
  }

we should using following code.

  function transferOwnership(address newOwner) external override {
    address oldOwner = owner;
    if (oldOwner != msg.sender) {
      revert CustomErrors.NOT_OWNER(oldOwner, msg.sender);
    }
    if(newOwner == address(0))
	{
		revert CustomErrors.ZeroAddress();
	}
    owner = newOwner;
    emit TransferOwnership(oldOwner, newOwner);
  }

---

MIMOProxyFactory.sol, 27, mimoProxyBase = _mimoProxyBase; we should use following code.

if(_mimoProxyBase == address(0))
{
	revert CustomErrors.ZeroAddress();
}
mimoProxyBase = _mimoProxyBase;

---

MIMOProxyRegistry.sol, 27, factory = factory_; we should use following code.

if(factory_ == address(0))
{
	revert CustomErrors.ZeroAddress();
}
factory = factory_;

L03:RETURN VALUES OF TRANSFER()/TRANSFERFROM() NOT CHECKED

prof

Not all IERC20 implementations revert() when there’s a failure in transfer()/transferFrom(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a

problem

MIMOAutoRebalance.sol, 78, 113 MIMOEmptyVault.sol, 82, 182 MIMOLeverage.sol, 51, 86, 137 MIMOManagedRebalance.sol, 79, 114 MIMORebalance.sol, 85,134,136 MIMOVaultActions.sol, 48, 71

none-critical issue foundings

###N01: TYPOS MIMOAutoAction.sol, L27,

    @notice Sets a vault automation parameters
	
	@notice Set a vault automation parameters
	

N02: NatSpec is incomplete

MIMOAutoAction.sol,88-91

N03: EVENT IS MISSING INDEXED FIELDS

MIMOProxy.sol, 32

N04: inconsistent solidity version usage.

MIMOProxyFactory.sol, pragma solidity >=0.8.4; MIMORebalance.sol, pragma solidity 0.8.10;

G01: COMPARISONS WITH ZERO FOR UNSIGNED INTEGERS

problem

0 is less gas efficient than !0 if you enable the optimizer at 10k AND you’re in a require statement. Detailed explanation with the opcodes https://twitter.com/gzeon/status/1485428085885640706

prof

actions/MIMOLeverage.sol, 50, if (depositAmount > 0) { actions/MIMORebalance.sol, 135, if (fee > 0) { actions/MIMOSwap.sol, 56, if (response.length > 0) { proxy/MIMOProxy.sol, 92, if (response.length > 0) { proxy/MIMOProxy.sol, 135, if (response.length > 0) {

G02: PREFIX INCREMENT SAVE MORE GAS

problem

prefix increment ++i is more cheaper than postfix i++

prof

proxy/MIMOProxy.sol, 132, for (uint256 i = 0; i < targets.length; i++) {

G03: ARRAY LENTH SHOULD USE MEMoRY VARIABLE LOAD IT

problem

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) Caching the length changes each of these to a DUP (3 gas), and gets rid of the extra DUP needed to store the stack offset

prof

proxy/MIMOProxy.sol, 132, for (uint256 i = 0; i < targets.length; i++) {

G04: resign the default value to the variables.

problem

resign the default value to the variables will cost more gas.

prof

proxy/MIMOProxy.sol, 132, for (uint256 i = 0; i < targets.length; i++) {

G05: USING BOOLS FOR STORAGE INCURS OVERHEAD

problem

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

prof

'MIMOManagedAction.sol', 16, ' mapping(address => bool) internal _managers; ‘MIMOProxy.sol', 23, ' mapping(address => mapping(address => mapping(bytes4 => bool))) internal _permissions; 'MIMOProxyFactory.sol', 23, ' mapping(address => bool) internal _proxies;

G06: Using calldata instead of memory for read-only arguments in external functions saves gas

problem

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * mem_array.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it’s still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

prof

MIMOEmptyVault.sol, 67,113, 114 MIMOAutoAction.sol, 32, 57, 64 MIMOAutoRebalance.sol, 94, 142, 54 MIMOFlashloan.sol, 42, MIMOLeverage.sol, 72, 114,115 MIMOManagedAction.sol, 33, 55, 70, 77, 84 MIMOProxy.sol, 105-108

G07: USE A MORE RECENT VERSION OF SOLIDITY

Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

G08: INTERNAL FUNCTIONS ONLY CALLED ONCE CAN BE INLINED TO SAVE GAS

MIMOManagedRebalance.sol, _postRebalanceChecks can be inlined