Gas Optimizations - Githubissues

Title: Rearrange state variables Severity: GAS

You can change the order of the storage variables to decrease memory uses.

In UniV3LpVault.sol,rearranging the storage fields can optimize to: 6 slots from: 7 slots. The new order of types (you choose the actual variables):

INonfungiblePositionManager
ISwapRouter
ComptrollerInterface
IFlashLoanReceiver
uint256
address
bool
bool
bool

Title: Unnecessary functions Severity: GAS

The following functions are not used at all. Therefore you can remove them to save deployment gas and improve code clearness.

    CToken.sol, liquidateBorrowInternal
    CToken.sol, liquidateBorrowUniV3Internal
    CToken.sol, borrowBehalfInternal
    CToken.sol, redeemBehalfInternal
    CToken.sol, mintBehalfInternal
    CToken.sol, repayBorrowBehalfInternal
    CToken.sol, mintInternal
    CToken.sol, _addReservesInternal
    CToken.sol, _functionCall
    CToken.sol, redeemUnderlyingBehalfInternal

Title: Use != 0 instead of > 0 Severity: GAS

Using != 0 is slightly cheaper than > 0. (see https://github.com/code-423n4/2021-12-maple-findings/issues/75 for similar issue)

    UniV3LpVault.sol, 145: change 'seizeLiquidity > 0' to 'seizeLiquidity != 0'
    CToken.sol, 1157: change 'balance > 0' to 'balance != 0'
    CToken.sol, 686: change 'redeemTokensIn > 0' to 'redeemTokensIn != 0'
    Comptroller.sol, 367: change 'redeemAmount > 0' to 'redeemAmount != 0'
    CToken.sol, 687: change 'redeemTokensIn > 0' to 'redeemTokensIn != 0'

Title: Caching array length can save gas Severity: GAS

Caching the array length is more gas efficient. This is because access to a local variable in solidity is more efficient than query storage / calldata / memory. We recommend to change from:

for (uint256 i=0; i<array.length; i++) { ... }

to:

uint len = array.length  
for (uint256 i=0; i<len; i++) { ... }

    Comptroller.sol, pools, 1471
    UniswapTwapOracle.sol, tokens, 209
    UniswapTwapOracle.sol, _tokens, 87
    Comptroller.sol, pools, 1490
    Comptroller.sol, assets, 846
    MasterPriceOracle.sol, underlyings, 68
    MasterPriceOracle.sol, underlyings, 51

Title: Inline one time use functions Severity: GAS

The following functions are used exactly once. Therefore you can inline them and save gas and improve code clearness.

    CToken.sol, borrowFresh
    CToken.sol, _setComptroller
    CToken.sol, finishInterestAccrual
    UniV3LpVault.sol, _mint
    UniV3LpVault.sol, _burn
    CToken.sol, _addReservesFresh
    CToken.sol, liquidateBorrowFresh
    CToken.sol, _reduceReservesFresh
    CToken.sol, liquidateBorrowUniV3Fresh
    Comptroller.sol, addNFTCollateral

Title: Unused state variables Severity: GAS

Unused state variables are gas consuming at deployment (since they are located in storage) and are a bad code practice. Removing those variables will decrease deployment gas cost and improve code quality. This is a full list of all the unused storage variables we found in your code base.

    Comptroller.sol, collateralFactorMaxMantissa
    Comptroller.sol, liquidationIncentiveMinMantissa
    Comptroller.sol, closeFactorMaxMantissa
    Comptroller.sol, liquidationIncentiveMaxMantissa
    UniswapTwapOracle.sol, ONE
    Comptroller.sol, closeFactorMinMantissa

Title: Public functions to external Severity: GAS

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

    Comptroller.sol, getAllBorrowers
    Comptroller.sol, _setPriceOracle
    Comptroller.sol, enterMarkets
    Comptroller.sol, _setMintPaused

Title: Do not cache msg.sender Severity: Gas

We recommend not to cache msg.sender since calling it is 2 gas while reading a variable is more.

    https://github.com/code-423n4/2022-04-dualityfocus/tree/main/contracts/compound_rari_fork/CToken.sol#L174

Title: Unnecessary equals boolean Severity: GAS

Boolean variables can be checked within conditionals directly without the use of equality operators to true/false.

    Comptroller.sol, 1410: require(msg.sender == admin || state == true, "only admin can unpause");
    Comptroller.sol, 1429: require(msg.sender == admin || state == true, "only admin can unpause");
    Comptroller.sol, 1438: require(msg.sender == admin || state == true, "only admin can unpause");
    Comptroller.sol, 1420: require(msg.sender == admin || state == true, "only admin can unpause");
    UniV3LpVault.sol, 854: require(msg.sender == comptroller.admin() || state == true, "only admin can unpause");
    Comptroller.sol, 142: if (marketToJoin.accountMembership[borrower] == true) {
    UniV3LpVault.sol, 838: require(msg.sender == comptroller.admin() || state == true, "only admin can unpause");
    Comptroller.sol, 1523: borrowGuardianPaused[address(cToken)] == true &&

Title: Consider inline the following functions to save gas Severity: GAS

You can inline the following functions instead of writing a specific function to save gas.
(see https://github.com/code-423n4/2021-11-nested-findings/issues/167 for a similar issue.)

    CToken.sol, getBlockNumber, { return block.number; }
    Comptroller.sol, getAccountLiquidityInternal, { return getHypotheticalAccountLiquidityInternal(account, CToken(0), 0, 0); }

Title: Use unchecked to save gas for certain additive calculations that cannot overflow Severity: GAS

You can use unchecked in the following calculations since there is no risk to overflow:

    UniV3LpVault.sol (L#675) - nonfungiblePositionManager.decreaseLiquidity( INonfungiblePositionManager.DecreaseLiquidityParams(tokenId, liquidity, 0, 0, block.timestamp + 200) );
    UniV3LpVault.sol (L#311) - INonfungiblePositionManager.MintParams memory mintParams = INonfungiblePositionManager.MintParams( token0, token1, fee, params.newTickLower, params.newTickUpper, amount0, amount1, params.amount0Min, params.amount1Min, msg.sender, block.timestamp + 200 ); 
    UniV3LpVault.sol (L#654) - (, amountOut0, amountOut1) = nonfungiblePositionManager.increaseLiquidity( INonfungiblePositionManager.IncreaseLiquidityParams( tokenId, amount0, amount1, amount0Min, amount1Min, block.timestamp + 200 ) ); 
    UniV3LpVault.sol (L#623) - amountOut = swapRouter.exactInput( ISwapRouter.ExactInputParams(swapPath, address(this), block.timestamp + 200, amount, 0) );

Title: Prefix increments are cheaper than postfix increments Severity: GAS

Prefix increments are cheaper than postfix increments. Further more, using unchecked {++x} is even more gas efficient, and the gas saving accumulates every iteration and can make a real change There is no risk of overflow caused by increamenting the iteration index in for loops (the ++i in for (uint256 i = 0; i < numIterations; ++i)). But increments perform overflow checks that are not necessary in this case. These functions use not using prefix increments (++x) or not using the unchecked keyword:

    change to prefix increment and unchecked: MasterPriceOracle.sol, i, 68
    change to prefix increment and unchecked: Comptroller.sol, i, 206
    change to prefix increment and unchecked: Comptroller.sol, i, 846
    change to prefix increment and unchecked: UniswapTwapOracle.sol, i, 209

Title: Internal functions to private Severity: GAS

The following functions could be set private to save gas and improve code quality:

    CToken.sol, _setComptroller
    Comptroller.sol, getAccountLiquidityInternal
    CToken.sol, getBlockNumber
    CToken.sol, exchangeRateStoredInternal
    CToken.sol, _functionCall
    CToken.sol, mintInternal

Title: Internal functions to private Severity: GAS

The following functions could be set private to save gas and improve code quality:

    CToken.sol, _setComptroller
    Comptroller.sol, getAccountLiquidityInternal
    CToken.sol, getBlockNumber
    CToken.sol, exchangeRateStoredInternal
    CToken.sol, _functionCall
    CToken.sol, mintInternal

Title: Short the following require messages Severity: GAS

The following require messages are of length more than 32 and we think are short enough to short them into exactly 32 characters such that it will be placed in one slot of memory and the require function will cost less gas. The list:

    Solidity file: UniV3LpVault.sol, In line 504, Require message length to shorten: 38, The message: swapPath1 did not pass integrity check
    Solidity file: UniV3LpVault.sol, In line 180, Require message length to shorten: 33, The message: periphery functionality is paused
    Solidity file: Comptroller.sol, In line 1373, Require message length to shorten: 38, The message: only admin can set borrow cap guardian
    Solidity file: Comptroller.sol, In line 1468, Require message length to shorten: 34, The message: only admin can set supported pools

Title: Upgrade pragma to at least 0.8.4 Severity: GAS

Using newer compiler versions and the optimizer gives gas optimizations and additional safety checks are available for free.

The advantages of versions 0.8.* over <0.8.0 are:

    1. Safemath by default from 0.8.0 (can be more gas efficient than library based safemath.)
    2. Low level inliner : from 0.8.2, leads to cheaper runtime gas. Especially relevant when the contract has small functions. For example, OpenZeppelin libraries typically have a lot of small helper functions and if they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.
    3. Optimizer improvements in packed structs: Before 0.8.3, storing packed structs, in some cases used an additional storage read operation. After EIP-2929, if the slot was already cold, this means unnecessary stack operations and extra deploy time costs. However, if the slot was already warm, this means additional cost of 100 gas alongside the same unnecessary stack operations and extra deploy time costs.
    4. Custom errors from 0.8.4, leads to cheaper deploy time cost and run time cost. Note: the run time cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

    FlashLoan.sol
    UniV3LpVault.sol
    Comptroller.sol
    UniswapTwapOracle.sol
    MasterPriceOracle.sol
    CToken.sol

code-423n4 / 2022-04-dualityfocus-findings

Gas Optimizations #5