Mimo August 2022 contest - simon135's results

dont use one letter variables its not very readible

ex: address a; instead make it into : address mimprovider https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/MIMOSwap.sol#L19

you can have a erc20 contract that is malious for executing flashloans and executing them for openvaluts and it wont pay msg.sender and this can only happen through lenderpool address

   IERC20 vaultCollateral = IERC20(assets[0]);
    uint256 amount = amounts[0];
    vaultCollateral.safeTransfer(address(mimoProxy), amounts[0]);
    uint256 flashloanRepayAmount = amount + premiums[0];

    IMIMOProxy(mimoProxy).execute(
      address(this),
      abi.encodeWithSignature(
        "emptyVaultOperation(address,uint256,uint256,(uint256,bytes))",
        vaultCollateral,
        vaultId,
        amount,
        swapData
      )
    );

that contract can get out of paying or increaseing allowance or transfering to msg.sender https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/MIMOEmptyVault.sol#L80

typos

instead of struc: make it struct https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/MIMOLeverage.sol#L42

make address zero if statement into a modifer for all code base to make it more readiable

   if (address(_core) == address(0) || address(_vaultsData) == address(0) || address(_stablex) == address(0)) {
      revert CustomErrors.CANNOT_SET_TO_ADDRESS_ZERO();
    }

instead make modifer like this ex:

modfier nonzero(address test) {
if( test == address(0) {
revert
}
_;

block.timestmap in 2016 will brake for a couple seconds

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/automated/MIMOAutoRebalance.sol#L76

no address zero check

 constructor(address _mimoProxyBase) {
    mimoProxyBase = _mimoProxyBase;
  }

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

There is no function to have the new owner approve to become owner which is best practice

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

make a timelock on importent functions like changing admin just incase there is malious admin you can prerpare to take out the funds

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

Using bools for storage incurs overhead

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

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

make address(0) into long version to save gas ex : 0x0000000000

MIMOSwap.sol:27:    if (address(_a) == address(0) || address(_dexAP) == address(0)) {
MIMOSwap.sol:47:    require(proxy != address(0), Errors.INVALID_AGGREGATOR);
MIMOSwap.sol:48:    require(router != address(0), Errors.INVALID_AGGREGATOR);
MIMORebalance.sol:31:    if (address(_proxyRegistry) == address(0)) {
MIMORebalance.sol:85:    fromCollateral.safeTransfer(address(mimoProxy), amounts[0]);
MIMOEmptyVault.sol:31:    if (address(_proxyRegistry) == address(0)) {
MIMOEmptyVault.sol:82:    vaultCollateral.safeTransfer(address(mimoProxy), amounts[0]);
MIMOVaultActions.sol:33:    if (address(_core) == address(0) || address(_vaultsData) == address(0) || address(_stablex) == address(0)) {
automated/MIMOAutoAction.sol:19:    if (address(_a) == address(0) || address(_proxyRegistry) == address(0)) {

use custom errors instead require to save gas

MIMOSwap.sol:47:    require(proxy != address(0), Errors.INVALID_AGGREGATOR);
MIMOSwap.sol:48:    require(router != address(0), Errors.INVALID_AGGREGATOR);
MIMORebalance.sol:129:    require(
MIMOEmptyVault.sol:96:    require(flashloanRepayAmount <= vaultCollateral.balanceOf(address(this)), Errors.CANNOT_REPAY_FLASHLOAN);
MIMOLeverage.sol:130:    require(collateralBalanceAfter >= flashloanRepayAmount, Errors.CANNOT_REPAY_FLASHLOAN);

USE ASSEMBLY TO CHECK FOR ADDRESS(0)

ex: iszero

MIMOSwap.sol:27:    if (address(_a) == address(0) || address(_dexAP) == address(0)) {
MIMOSwap.sol:47:    require(proxy != address(0), Errors.INVALID_AGGREGATOR);
MIMOSwap.sol:48:    require(router != address(0), Errors.INVALID_AGGREGATOR);
MIMORebalance.sol:31:    if (address(_proxyRegistry) == address(0)) {
MIMORebalance.sol:85:    fromCollateral.safeTransfer(address(mimoProxy), amounts[0]);
MIMOEmptyVault.sol:31:    if (address(_proxyRegistry) == address(0)) {
MIMOEmptyVault.sol:82:    vaultCollateral.safeTransfer(address(mimoProxy), amounts[0]);
MIMOVaultActions.sol:33:    if (address(_core) == address(0) || address(_vaultsData) == address(0) || address(_stablex) == address(0)) {
automated/MIMOAutoAction.sol:19:    if (address(_a) == address(0) || address(_proxyRegistry) == address(0)) {

to save gas make abi.encodepacked instead of abi.encode

automated/MIMOAutoRebalance.sol:73:    _takeFlashLoan(flData, abi.encode(vaultOwner, autoFee, rbData, swapData));
automated/MIMOAutoRebalance.sol:118:      abi.encodeWithSignature(
MIMOLeverage.sol:54:    bytes memory params = abi.encode(msg.sender, swapAmount, swapData);

FUNCTIONS GUARANTEED TO REVERT WHEN CALLED BY NORMAL USERS CAN BE MARKED PAYABLE

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. 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. https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/proxy/MIMOProxy.sol#L104-L117

make mappings into struct to save gas

https://github.com/code-423n4/2022-08-mimo/blob/9adf46f2efc61898247c719f2f948b41d5d62bbe/contracts/actions/managed/MIMOManagedAction.sol#L15-L17

make public constant private instead of public to save gas

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