search

code-423n4 / 2022-05-bunker-findings

1 stars 0 forks source link

Gas Optimizations #124

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Issue: Require message is to long Explanation: The messages below can be shortened to 32 characters or fewer (as shown) to save gas

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L69

                    require(buyPunkSuccess, "CNFT: Calling buyPunk was unsuccessful");

Change message to CNFT: buyPunk call unsuccessful

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L93

        require(borrower != liquidator, "CNFT: Liquidator cannot be borrower");

Change message to CNFT: Liquidator cannot borrow

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L420

            require(borrowBalance >= repayAmount, "Can not repay more than the total borrow");

Change message to Cannot repay > than total borrow

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L702

        require(cNftCollateral == address(nftMarket), "cNFT is from the wrong comptroller");

Change message to cNFT is from wrong comptroller

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L960

        require(msg.sender == admin, "only admin can set borrow cap guardian");

Change message to admin sets borrow cap guardian

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1338

        require(address(nftMarket) == address(0), "nft collateral already initialized");

Change message to nft coll already initialized

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L32

        require(msg.sender == admin, "only admin may initialize the market");

Change message to only admin may initialize market

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L49

        require(err == uint(Error.NO_ERROR), "setting interest rate model failed");

Change message to interest rate model set failed

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L1093

        require(seizeTokens == 0, "LIQUIDATE_SEIZE_INCORRECT_NUM_NFTS");

Change message to LIQUIDATE_SEIZE_WRONG_NUM_NFTS

The same require message occurs in both lines referenced below: https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L995

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1009

        require(markets[address(cToken)].isListed, "cannot pause a market that is not listed");

Change message to can't pause a market not listed

The same require message occurs in both lines referenced below:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L128

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L149

        require(length > 0, "UniswapV2PriceOracle: No observations.");

Change message to UniswapV2PriceOracle: No obs.

Issue: Require message is to long but may be difficult to shorten Explanation: The long require messages below might be difficult to cut back while preserving their chosen format and/or meaning:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CErc20.sol#L136

        require(address(token) != underlying, "CErc20::sweepToken: can not sweep underlying token");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CErc20.sol#L234

        require(msg.sender == admin, "only the admin may set the comp-like delegate");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L24

        require(_underlying != address(0), "CNFT: Asset should not be address(0)");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L25

        require(ComptrollerInterface(_comptroller).isComptroller(), "_comptroller is not a Comptroller contract");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L69

                    require(buyPunkSuccess, "CNFT: Calling buyPunk was unsuccessful");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L148

                    require(transferPunkSuccess, "CNFT: Calling transferPunk was unsuccessful");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L208

        require(msg.sender == underlying, "CNFT: This contract can only receive the underlying NFT");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L209

        require(operator == address(this), "CNFT: Only the CNFT contract can be the operator");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L279

        require(to != underlying, "CNFT: Cannot make an arbitrary call to underlying NFT");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L171

        require(oErr == 0, "exitMarket: getAccountSnapshot failed"); // semi-opaque error code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L942

        require(msg.sender == admin || msg.sender == borrowCapGuardian, "only admin or borrow cap guardian can set borrow caps");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1037

        require(msg.sender == unitroller.admin(), "only unitroller admin can change brains");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1339

        require(address(cNft) != address(0), "cannot initialize nft market to the 0 address");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L37

        require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L271

        require(err == MathError.NO_ERROR, "borrowBalanceStored: borrowBalanceStoredInternal failed");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L328

        require(err == MathError.NO_ERROR, "exchangeRateStored: exchangeRateStoredInternal failed");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L542

        require(vars.mathErr == MathError.NO_ERROR, "MINT_NEW_TOTAL_SUPPLY_CALCULATION_FAILED");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L545

        require(vars.mathErr == MathError.NO_ERROR, "MINT_NEW_ACCOUNT_BALANCE_CALCULATION_FAILED");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L609

        require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L891

        require(vars.mathErr == MathError.NO_ERROR, "REPAY_BORROW_NEW_ACCOUNT_BORROW_BALANCE_CALCULATION_FAILED");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L894

        require(vars.mathErr == MathError.NO_ERROR, "REPAY_BORROW_NEW_TOTAL_BALANCE_CALCULATION_FAILED");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L1433

        require(totalReservesNew <= totalReserves, "reduce reserves unexpected underflow");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L1433

        require(totalReservesNew <= totalReserves, "reduce reserves unexpected underflow");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/CNftPriceOracle.sol#L70

                "CNftPriceOracle: Cannot overwrite existing address mappings."

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/CNftPriceOracle.sol#L90

            "CNftPriceOracle: No NFTX token for cNFT."

The long require message below appears in the following two locations:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L986

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L1083

        require(amountSeizeError == uint(Error.NO_ERROR), "LIQUIDATE_COMPTROLLER_CALCULATE_AMOUNT_SEIZE_FAILED");

The long require message below appears in the following two locations:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L52

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L100

                require(amounts[i] == 1, "CNFT: Amounts must be all 1s for non-ERC1155s.");

The long require message below appears in the following two locations:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L137

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L158

            "UniswapV2PriceOracle: Bad TWAP time."

The long require message below appears in the following two locations:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L131

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L152

            require(length > 1, "UniswapV2PriceOracle: Only one observation.");

The long require message below appears in the following three locations:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L68

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L92

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L116

                    "UniswapV2PriceOracle: Division by zero."

The long require message below appears in the following four locations:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L996

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1010

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1019

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L1028

        require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause");

Issue: Should use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint) Explanation: != 0 should be used where possible since > 0 costs more gas

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L37

        require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");

Change initialExchangeRateMantissa > 0 to require(initialExchangeRateMantissa != 0

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L90-L93

                require(
                    reserve0 > 0,
                    "UniswapV2PriceOracle: Division by zero."
                );

Change reserve0 > 0 to reserve0 != 0

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L114-L117

                require(
                    reserve1 > 0,
                    "UniswapV2PriceOracle: Division by zero."
                );

Change reserve1 > 0 to reserve1 != 0

Identical requires appear in both lines below:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L128

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L149

        require(length > 0, "UniswapV2PriceOracle: No observations.");

Change length > 0 to length != 0 in both cases

There are two opportunities to save gas in the require below:

  1. Shorten the require message to 32 characters or fewer
  2. Use != 0 instead of > 0 because > 0 costs more gas and initialExchangeRateMantissa cannot be less than zero

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L37

        require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");

Change to:

        require(initialExchangeRateMantissa != 0, "initial exchange rate must be >0");

Issue: Use of '&&' within a require function Explanation: Dividing the require into separate require messages instead of using '&&' will save gas

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L66

                    require(checkSuccess && nftOwner == msg.sender, "Not the NFT owner");

Recommendation:

                    require(checkSuccess, "Not the NFT owner");
                    require(nftOwner == msg.sender, "Not the NFT owner");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L947

        require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");

Recommendation:

        require(numMarkets != 0, "invalid input");
        require(numMarkets == numBorrowCaps, "invalid input");

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/CNftPriceOracle.sol#L62-L65

        require(
            cNfts.length > 0 && cNfts.length == nftxTokens.length,
            "CNftPriceOracle: `cNfts` and `nftxTokens` must have nonzero, equal lengths."
        );

Recommendation:

        require(cNfts.length > 0, "CNftPriceOracle: `cNfts` must have nonzero lengths");
        require(cNfts.length == nftxTokens.length, "CNftPriceOracle: `cNfts` and `nftxTokens` must have equal lengths");

There are two opportunities to save gas in the require below:

  1. Shorten the require message to 32 characters or fewer
  2. Divide the require into two separate requires instead of using '&&'

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L33

        require(accrualBlockNumber == 0 && borrowIndex == 0, "market may only be initialized once");

Recommendation:

require(accrualBlockNumber == 0, "accrualBlockNumber must equal 0");
require(borrowIndex == 0, "mkt may only be initialized once");

There are two opportunities to save gas in the require below:

  1. Use != 0 instead of > 0 because > 0 costs more gas and reserve0 and reserve1 cannot be < zero (since they are units)
  2. Divide the require into two separate requires instead of using '&&'

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L66-L69

                require(
                    reserve0 > 0 && reserve1 > 0,
                    "UniswapV2PriceOracle: Division by zero."
                );

Recommendation:

       require(reserve0 != 0, "UniswapV2PriceOracle: Division by zero.");
       require(reserve1 != 0, "UniswapV2PriceOracle: Division by zero.");

Issue: Variables should not be initialized to their default values Explanation: Initializing uint variables to their default value of 0 is unnecessary and costs gas uint256 totalAmount is initialized to zero three times in CNft.sol:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L49

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L97

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L119

        uint256 totalAmount = 0;

Recommendation:

        uint256 totalAmount;

The following also contain initializations of variables to their default values:

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CToken.sol#L81

        uint startingAllowance = 0;

Recommendation:

        uint startingAllowance;

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L41

        uint256 numberUpdated = 0;

Recommendation:

        uint256 numberUpdated;