QA Report

[code423n4]

Low Risk Issues

approve() return value not checked

Check the result, or use safeApprove()

contracts/vault_and_oracles/UniV3LpVault.sol:418:            IERC20Detailed(params.asset).approve(msg.sender, owedBack);
contracts/vault_and_oracles/UniV3LpVault.sol:533:        IERC20Detailed(params.underlying).approve(address(params.debtCToken), params.repayAmount);
contracts/vault_and_oracles/UniV3LpVault.sol:536:        IERC20Detailed(params.underlying).approve(address(params.debtCToken), 0);
contracts/vault_and_oracles/UniV3LpVault.sol:622:        IERC20Detailed(swapPath.toAddress(0)).approve(address(swapRouter), amount);
contracts/vault_and_oracles/UniV3LpVault.sol:626:        IERC20Detailed(swapPath.toAddress(0)).approve(address(swapRouter), 0);
contracts/vault_and_oracles/UniV3LpVault.sol:650:        IERC20Detailed(token0).approve(address(nonfungiblePositionManager), amount0);
contracts/vault_and_oracles/UniV3LpVault.sol:651:        IERC20Detailed(token1).approve(address(nonfungiblePositionManager), amount1);
contracts/vault_and_oracles/UniV3LpVault.sol:665:        IERC20Detailed(token0).approve(address(nonfungiblePositionManager), 0);
contracts/vault_and_oracles/UniV3LpVault.sol:666:        IERC20Detailed(token1).approve(address(nonfungiblePositionManager), 0);
contracts/vault_and_oracles/UniV3LpVault.sol:704:        IERC20Detailed(params.token0).approve(address(nonfungiblePositionManager), params.amount0Desired);
contracts/vault_and_oracles/UniV3LpVault.sol:705:        IERC20Detailed(params.token1).approve(address(nonfungiblePositionManager), params.amount1Desired);
contracts/vault_and_oracles/UniV3LpVault.sol:723:        IERC20Detailed(params.token0).approve(address(nonfungiblePositionManager), 0);
contracts/vault_and_oracles/UniV3LpVault.sol:724:        IERC20Detailed(params.token1).approve(address(nonfungiblePositionManager), 0);
contracts/vault_and_oracles/FlashLoan.sol:48:        IERC20(assets[0]).approve(address(LP_VAULT), amounts[0]);
contracts/vault_and_oracles/FlashLoan.sol:58:        IERC20(assets[0]).approve(address(LENDING_POOL), amountOwing);

require() should be used instead of assert()

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 778)

       assert(assetIndex < len);

2. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 196)

       assert(isSupportedPool[referencePools[token]]);

3. File: contracts/compound_rari_fork/Comptroller.sol (line 214)

       assert(assetIndex < len);

4. File: contracts/compound_rari_fork/Comptroller.sol (line 273)

           assert(markets[cToken].accountMembership[minter]);

5. File: contracts/compound_rari_fork/Comptroller.sol (line 403)

           assert(markets[cToken].accountMembership[borrower]);

6. File: contracts/compound_rari_fork/Comptroller.sol (line 1310)

       assert(assetIndex < len);

Missing checks for address(0x0) when assigning values to address state variables

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 65)

       factory = _factory;

2. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 93)

       WETH_ADDRESS = _wethAddress;

3. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 94)

       nfpManager = _nfpManager;

4. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 95)

       factory = _factory;

5. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 96)

       admin = _admin;

6. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 52)

       WETH_ADDRESS = _wethAddress;

7. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 53)

       admin = _admin;

8. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 83)

       admin = newAdmin;

Open TODOs

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 142)

       // TODO: do we want some Comptroller like error handling/messaging here?

2. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 166)

   // TODO: do we want a "decreaseLiquidityAndCollect" function?

3. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 53)

   // period used to calculate our TWAPs, in TODO units

4. File: contracts/compound_rari_fork/Comptroller.sol (line 1012)

       // TODO: custom liquidation incentive for LP shares

Non-critical Issues

public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

1. File: contracts/compound_rari_fork/CToken.sol (lines 26-33)

   function initialize(
       ComptrollerInterface comptroller_,
       InterestRateModel interestRateModel_,
       uint256 initialExchangeRateMantissa_,
       string memory name_,
       string memory symbol_,
       uint8 decimals_,
       uint256 reserveFactorMantissa_

2. File: contracts/compound_rari_fork/CToken.sol (line 1580)

   function _setInterestRateModel(InterestRateModel newInterestRateModel) public returns (uint256) {

3. File: contracts/compound_rari_fork/Comptroller.sol (line 115)

   function enterMarkets(address[] memory cTokens) public returns (uint256[] memory) {

4. File: contracts/compound_rari_fork/Comptroller.sol (lines 696-702)

   function getAccountLiquidity(address account)
       public
       view
       returns (
           uint256,
           uint256,
           uint256

5. File: contracts/compound_rari_fork/Comptroller.sol (lines 743-754)

   function getHypotheticalAccountLiquidity(
       address account,
       address cTokenModify,
       uint256 redeemTokens,
       uint256 borrowAmount
   )
       public
       view
       returns (
           uint256,
           uint256,
           uint256

6. File: contracts/compound_rari_fork/Comptroller.sol (line 1064)

   function _setPriceOracle(PriceOracle newOracle) public returns (uint256) {

7. File: contracts/compound_rari_fork/Comptroller.sol (line 1082)

   function _setTickOracle(TickOracle newTickOracle) public returns (uint256) {

8. File: contracts/compound_rari_fork/Comptroller.sol (line 1105)

   function _setUniV3LpVault(IUniV3LpVault newVault) public returns (uint256) {

9. File: contracts/compound_rari_fork/Comptroller.sol (line 1390)

   function _setPauseGuardian(address newPauseGuardian) public returns (uint256) {

10. File: contracts/compound_rari_fork/Comptroller.sol (line 1407)

    function _setMintPaused(CToken cToken, bool state) public returns (bool) {

11. File: contracts/compound_rari_fork/Comptroller.sol (line 1417)

    function _setBorrowPaused(CToken cToken, bool state) public returns (bool) {

12. File: contracts/compound_rari_fork/Comptroller.sol (line 1427)

    function _setTransferPaused(bool state) public returns (bool) {

13. File: contracts/compound_rari_fork/Comptroller.sol (line 1436)

    function _setSeizePaused(bool state) public returns (bool) {

14. File: contracts/compound_rari_fork/Comptroller.sol (line 1445)

    function _become(Unitroller unitroller) public {

15. File: contracts/compound_rari_fork/Comptroller.sol (line 1502)

    function getAllMarkets() public view returns (CToken[] memory) {

16. File: contracts/compound_rari_fork/Comptroller.sol (line 1511)

    function getAllBorrowers() public view returns (address[] memory) {

17. File: contracts/compound_rari_fork/CErc20.sol (lines 20-26)

    function initialize(
        address underlying_,
        ComptrollerInterface comptroller_,
        InterestRateModel interestRateModel_,
        string memory name_,
        string memory symbol_,
        uint256 reserveFactorMantissa_

constants should be defined rather than using magic numbers

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 322)

           block.timestamp + 200

2. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 624)

           ISwapRouter.ExactInputParams(swapPath, address(this), block.timestamp + 200, amount, 0)

3. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 661)

               block.timestamp + 200

4. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 676)

           INonfungiblePositionManager.DecreaseLiquidityParams(tokenId, liquidity, 0, 0, block.timestamp + 200)

5. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 810)

           (swapPath.toAddress(0) == tokenStart && swapPath.toAddress(swapPath.length - 20) == tokenEnd);

6. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 117)

       return (_price(underlying).mul(1e18)).div(10**uint256(IERC20Detailed(underlying).decimals()));

7. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 110)

       if (underlying == WETH_ADDRESS) return 1e18;

8. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 125)

       if (underlying == WETH_ADDRESS) return 1e18;

9. File: contracts/compound_rari_fork/Comptroller.sol (line 1524)

           cToken.reserveFactorMantissa() == 1e18;

10. File: contracts/compound_rari_fork/CErc20.sol (line 29)

        uint256 initialExchangeRateMantissa_ = 0.2e18;

Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

1. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (lines 57-60)

   mapping(address => address) public referencePools;
   
   // mapping from uni v3 pool to bool of whether or not they are being utilized
   mapping(address => bool) public isSupportedPool;

Non-library/interface files should use fixed compiler versions, not floating ones

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 1)

   pragma solidity ^0.7.6;

2. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 1)

   pragma solidity ^0.7.6;

3. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 2)

   pragma solidity ^0.7.6;

4. File: contracts/vault_and_oracles/FlashLoan.sol (line 1)

   pragma solidity ^0.7.6;

5. File: contracts/compound_rari_fork/CToken.sol (line 1)

   pragma solidity ^0.5.16;

File does not contain an SPDX Identifier

1. File: contracts/libs/LpBreakdownLibrary.sol (line 0)

   pragma solidity ^0.7.6;

2. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 0)

   pragma solidity ^0.7.6;

3. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 0)

   pragma solidity ^0.7.6;

4. File: contracts/vault_and_oracles/FlashLoan.sol (line 0)

   pragma solidity ^0.7.6;

5. File: contracts/compound_rari_fork/CToken.sol (line 0)

   pragma solidity ^0.5.16;

File is missing NatSpec

1. File: contracts/vault_and_oracles/FlashLoan.sol (line 0)

   pragma solidity ^0.7.6;

NatSpec is incomplete

1. File: contracts/libs/LpBreakdownLibrary.sol (lines 31-59)

   /**
    * @dev gets the breakdown of a LP Token in the pool's underlying tokens, utilizing an instantaneous or TWAP tick
    * @param tokenId The tokenId to get a breakdown of
    * @param isTwap Whether to use a TWAP or instantaneous tick for calculations
    * @return token0 The first token of tokenId's pool
    *         token1 The second token of tokenId's pool
    *         amountToken0Fees Amount of token0 in fees
    *         amountToken1Fees Amount of token1 in fees
    *         amountToken0Liquidity Amount of token0 contributing to liquidity value
    *         amountToken1Liquidity Amount of token1 contributing to liquidity value
    *         amountLiquidity Amount of liquidity that tokenId represents
    */
   function _getTokenBreakdown(
       uint256 tokenId,
       bool isTwap,
       address nfpManager,
       address factory,
       address tickOracle
   )
       internal
       view
       returns (
           address token0,
           address token1,
           uint256 amountToken0Fees,
           uint256 amountToken1Fees,
           uint256 amountToken0Liquidity,
           uint256 amountToken1Liquidity,
           uint256 amountLiquidity

   Missing: @param nfpManager @param factory @param tickOracle

2. File: contracts/libs/LpBreakdownLibrary.sol (lines 84-93)

   /**
    * @dev generates an in-memory struct of position information of the tokenId to be used for computations
    * @param tokenId The tokenId to generate position information of
    * @return vars The in-memory struct containing all most recently updated information about our tokenId
    */
   function _generateTokenValueLocalVars(
       uint256 tokenId,
       address nfpmManager,
       address factory
   ) internal view returns (TokenValueLocalVars memory vars) {

   Missing: @param nfpmManager @param factory

3. File: contracts/libs/LpBreakdownLibrary.sol (lines 121-130)

   /**
    * @dev calculates updated fees for a tokenId position given old stats
    * @param vars The in-memory struct containing all most recently updated information about our tokenId
    * @return tokensOwed0 The updated amount of token0 collected in fees
    *         tokensOwed1 The updated amount of token1 collected in fees
    */
   function _getTokensOwed(TokenValueLocalVars memory vars, int24 tick)
       internal
       view
       returns (uint256 tokensOwed0, uint256 tokensOwed1)

   Missing: @param tick

4. File: contracts/libs/UniswapTwapLibrary.sol (lines 26-41)

   /**
    * @notice get twap converted with base & quote token decimals
    * @dev if period is longer than the current timestamp - first timestamp stored in the pool, this will revert with "OLD"
    * @param _pool uniswap pool address
    * @param _base base currency. to get eth/usd price, eth is base token
    * @param _quote quote currency. to get eth/usd price, usd is the quote currency
    * @param _period number of seconds in the past to start calculating time-weighted average
    * @return price of 1 base currency in quote currency. scaled by 1e18
    */
   function getTwap(
       address _pool,
       address _base,
       address _quote,
       uint32 _period,
       bool _checkPeriod
   ) internal view returns (uint256) {

   Missing: @param _checkPeriod

5. File: contracts/libs/LiquidityLibrary.sol (lines 17-32)

   /**
    * @notice get balances of token0 / token1 in a uniswap position
    * @dev knowing liquidity, tick range, and current tick gives balances
    *      Opyn team (https://github.com/opynfinance/squeeth-monorepo/blob/main/packages/hardhat/contracts/libs/VaultLib.sol)
    * @param _tickLower address of the uniswap position manager
    * @param _tickUpper uniswap position token id
    * @param _tick current price tick used for calculation
    * @return amount0 the amount of token0 in the uniswap position token
    * @return amount1 the amount of token1 in the uniswap position token
    */
   function _getToken0Token1Balances(
       int24 _tickLower,
       int24 _tickUpper,
       int24 _tick,
       uint128 _liquidity
   ) internal pure returns (uint256 amount0, uint256 amount1) {

   Missing: @param _liquidity

6. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 619-621)

    * @param amount The amount of the first token to swap
    */
   function _swap(bytes memory swapPath, uint256 amount) internal returns (uint256 amountOut) {

   Missing: @return

7. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 682-684)

    * @param tokenId The tokenId of the NFT we are collecting fees from
    */
   function _collectMax(uint256 tokenId) internal returns (uint256 amount0, uint256 amount1) {

   Missing: @return

8. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 694-701)

    * @param params NonfungiblePositionManager's MintParams. Acts as a passthrough except for ownership management
    */
   function _mint(INonfungiblePositionManager.MintParams memory params)
       internal
       returns (
           uint256 tokenId,
           uint256 amount0,
           uint256 amount1

   Missing: @return

9. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 831-833)

    * @param state new value for whether deposits are paused
    */
   function _pauseDeposits(bool state) external override returns (bool) {

   Missing: @return

10. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 847-849)

     * @param state new value for whether periphery UX functions are paused
     */
    function _pausePeripheryFunctions(bool state) external override returns (bool) {

    Missing: @return

11. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 863-865)

     * @param _flashLoan the new flashLoan contract's address
     */
    function _setFlashLoan(address _flashLoan) external override returns (address) {

    Missing: @return

12. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 875-877)

     * @param _userTokensMax the new value for userTokensMax
     */
    function _setUserTokensMax(uint256 _userTokensMax) external override returns (uint256) {

    Missing: @return

13. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (lines 62-78)

    /**
     * @dev Constructor to initialize state variables.
     * @param _tokens The underlying ERC20 token addresses to link to `_pools`.
     * @param _pools The Uniswap V3 Pools to be assigned to `_tokens`.
     * @param _twapPeriod The period used to calculate our TWAP from the Uni V3 pool
     * @param _admin The admin who can assign pools to tokens.
     * @param _canAdminOverwrite Controls if `admin` can overwrite existing assignments of pools to tokens.
     */
    constructor(
        address[] memory _tokens,
        address[] memory _pools,
        uint32 _twapPeriod,
        address _wethAddress,
        address _nfpManager,
        address _factory,
        address _admin,
        bool _canAdminOverwrite

    Missing: @param _wethAddress @param _nfpManager @param _factory

14. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (lines 109-113)

    /**
     * @notice Returns the price in ETH of the token underlying `cToken`.
     * @return Price in ETH of the token underlying `cToken`, scaled by `10 ** (36 - underlyingDecimals)`.
     */
    function getUnderlyingPrice(CTokenInterface cToken) external view override returns (uint256) {

    Missing: @param cToken

15. File: contracts/vault_and_oracles/MasterPriceOracle.sol (lines 30-42)

    /**
     * @dev Constructor to initialize state variables.
     * @param underlyings The underlying ERC20 token addresses to link to `_oracles`.
     * @param _oracles The `PriceOracle` contracts to be assigned to `underlyings`.
     * @param _admin The admin who can assign oracles to underlying tokens.
     * @param _canAdminOverwrite Controls if `admin` can overwrite existing assignments of oracles to underlying tokens.
     */
    constructor(
        address[] memory underlyings,
        PriceOracleInterface[] memory _oracles,
        address _wethAddress,
        address _admin,
        bool _canAdminOverwrite

    Missing: @param _wethAddress

16. File: contracts/vault_and_oracles/MasterPriceOracle.sol (lines 100-105)

    /**
     * @notice Returns the price in ETH of the token underlying `cToken`.
     * @dev Implements the `PriceOracle` interface for Fuse pools (and Compound v2).
     * @return Price in ETH of the token underlying `cToken`, scaled by `10 ** (36 - underlyingDecimals)`.
     */
    function getUnderlyingPrice(CTokenInterface cToken) external view override returns (uint256) {

    Missing: @param cToken

17. File: contracts/compound_rari_fork/CToken.sol (lines 17-33)

    /**
     * @notice Initialize the money market
     * @param comptroller_ The address of the Comptroller
     * @param interestRateModel_ The address of the interest rate model
     * @param initialExchangeRateMantissa_ The initial exchange rate, scaled by 1e18
     * @param name_ EIP-20 name of this token
     * @param symbol_ EIP-20 symbol of this token
     * @param decimals_ EIP-20 decimal precision of this token
     */
    function initialize(
        ComptrollerInterface comptroller_,
        InterestRateModel interestRateModel_,
        uint256 initialExchangeRateMantissa_,
        string memory name_,
        string memory symbol_,
        uint8 decimals_,
        uint256 reserveFactorMantissa_

    Missing: @param reserveFactorMantissa_

18. File: contracts/compound_rari_fork/CToken.sol (lines 627-636)

    /**
     * @notice Sender redeems cTokens on behalf of redeemer in exchange for a specified amount of underlying asset
     * @dev Accrues interest whether or not the operation succeeds, unless reverted
     * @param redeemAmount The amount of underlying to receive from redeeming cTokens
     * @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
     */
    function redeemUnderlyingBehalfInternal(address redeemer, uint256 redeemAmount)
        internal
        nonReentrant(false)
        returns (uint256)

    Missing: @param redeemer

19. File: contracts/compound_rari_fork/CToken.sol (lines 1396-1401)

    /**
     * @notice Sets a new comptroller for the market
     * @dev Internal function to set a new comptroller
     * @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
     */
    function _setComptroller(ComptrollerInterface newComptroller) internal returns (uint256) {

    Missing: @param newComptroller

20. File: contracts/compound_rari_fork/CToken.sol (lines 1415-1420)

    /**
     * @notice accrues interest and sets a new reserve factor for the protocol using _setReserveFactorFresh
     * @dev Admin function to accrue interest and set a new reserve factor
     * @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
     */
    function _setReserveFactor(uint256 newReserveFactorMantissa) external nonReentrant(false) returns (uint256) {

    Missing: @param newReserveFactorMantissa

21. File: contracts/compound_rari_fork/CToken.sol (lines 1430-1435)

    /**
     * @notice Sets a new reserve factor for the protocol (*requires fresh interest accrual)
     * @dev Admin function to set a new reserve factor
     * @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
     */
    function _setReserveFactorFresh(uint256 newReserveFactorMantissa) internal returns (uint256) {

    Missing: @param newReserveFactorMantissa

22. File: contracts/compound_rari_fork/CToken.sol (lines 1702-1717)

    /**
     * @dev Performs a Solidity function call using a low level `call`. A
     * plain `call` is an unsafe replacement for a function call: use this
     * function instead.
     * If `target` reverts with a revert reason, it is bubbled up by this
     * function (like regular Solidity function calls).
     * Returns the raw returned data. To convert to the expected return value,
     * use https://solidity.readthedocs.io/en/latest/units-and-global-variables.html?highlight=abi.decode#abi-encoding-and-decoding-functions[`abi.decode`].
     * @param data The call data (encoded using abi.encode or one of its variants).
     * @param errorMessage The revert string to return on failure.
     */
    function _functionCall(
        address target,
        bytes memory data,
        string memory errorMessage
    ) internal returns (bytes memory) {

    Missing: @param target @return

23. File: contracts/compound_rari_fork/Comptroller.sol (lines 471-479)

     * @param repayAmount The amount of underlying being repaid
     */
    function liquidateBorrowAllowed(
        address cTokenBorrowed,
        address cTokenCollateral,
        address liquidator,
        address borrower,
        uint256 repayAmount
    ) external returns (uint256) {

    Missing: @return

24. File: contracts/compound_rari_fork/Comptroller.sol (lines 521-529)

     * @param repayAmount The amount of underlying being repaid
     */
    function liquidateBorrowUniV3Allowed(
        address cTokenBorrowed,
        uint256 collateralTokenId,
        address liquidator,
        address borrower,
        uint256 repayAmount
    ) external returns (uint256) {

    Missing: @return

25. File: contracts/compound_rari_fork/Comptroller.sol (lines 573-581)

     * @param seizeTokens The number of collateral tokens to seize
     */
    function seizeAllowed(
        address cTokenCollateral,
        address cTokenBorrowed,
        address liquidator,
        address borrower,
        uint256 seizeTokens
    ) external returns (uint256) {

    Missing: @return

26. File: contracts/compound_rari_fork/Comptroller.sol (lines 690-702)

    /**
     * @notice Determine the current account liquidity wrt collateral requirements
     * @return (possible error code (semi-opaque),
                account liquidity in excess of collateral requirements,
     *          account shortfall below collateral requirements)
     */
    function getAccountLiquidity(address account)
        public
        view
        returns (
            uint256,
            uint256,
            uint256

    Missing: @param account

27. File: contracts/compound_rari_fork/Comptroller.sol (lines 715-727)

    /**
     * @notice Determine the current account liquidity wrt collateral requirements
     * @return (possible error code,
                account liquidity in excess of collateral requirements,
     *          account shortfall below collateral requirements)
     */
    function getAccountLiquidityInternal(address account)
        internal
        view
        returns (
            Error,
            uint256,
            uint256

    Missing: @param account

28. File: contracts/compound_rari_fork/Comptroller.sol (lines 1059-1064)

    /**
     * @notice Sets a new price oracle for the comptroller
     * @dev Admin function to set a new price oracle
     * @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
     */
    function _setPriceOracle(PriceOracle newOracle) public returns (uint256) {

    Missing: @param newOracle

29. File: contracts/compound_rari_fork/Comptroller.sol (lines 1100-1105)

    /**
     * @notice Sets a new UniV3LpVault for the comptroller
     * @dev Admin function to set a new UniV3LpVault
     * @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
     */
    function _setUniV3LpVault(IUniV3LpVault newVault) public returns (uint256) {

    Missing: @param newVault

30. File: contracts/compound_rari_fork/Comptroller.sol (lines 1518-1520)

     * @param cToken The market to check if deprecated
     */
    function isDeprecated(CToken cToken) public view returns (bool) {

    Missing: @return

31. File: contracts/compound_rari_fork/CErc20.sol (lines 12-26)

    /**
     * @notice Initialize the new money market
     * @param underlying_ The address of the underlying asset
     * @param comptroller_ The address of the Comptroller
     * @param interestRateModel_ The address of the interest rate model
     * @param name_ ERC-20 name of this token
     * @param symbol_ ERC-20 symbol of this token
     */
    function initialize(
        address underlying_,
        ComptrollerInterface comptroller_,
        InterestRateModel interestRateModel_,
        string memory name_,
        string memory symbol_,
        uint256 reserveFactorMantissa_

    Missing: @param reserveFactorMantissa_

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

1. File: contracts/vault_and_oracles/MasterPriceOracle.sol (line 90)

   event NewAdmin(address oldAdmin, address newAdmin);

2. File: contracts/compound_rari_fork/Comptroller.sol (line 20)

   event MarketListed(CToken cToken);

3. File: contracts/compound_rari_fork/Comptroller.sol (line 23)

   event MarketUnlisted(CToken cToken);

4. File: contracts/compound_rari_fork/Comptroller.sol (line 26)

   event MarketEntered(CToken cToken, address account);

5. File: contracts/compound_rari_fork/Comptroller.sol (line 29)

   event MarketExited(CToken cToken, address account);

6. File: contracts/compound_rari_fork/Comptroller.sol (line 32)

   event NewCloseFactor(uint256 oldCloseFactorMantissa, uint256 newCloseFactorMantissa);

7. File: contracts/compound_rari_fork/Comptroller.sol (line 35)

   event NewCollateralFactor(CToken cToken, uint256 oldCollateralFactorMantissa, uint256 newCollateralFactorMantissa);

8. File: contracts/compound_rari_fork/Comptroller.sol (line 38)

   event NewLiquidationIncentive(uint256 oldLiquidationIncentiveMantissa, uint256 newLiquidationIncentiveMantissa);

9. File: contracts/compound_rari_fork/Comptroller.sol (line 41)

   event NewPriceOracle(PriceOracle oldPriceOracle, PriceOracle newPriceOracle);

10. File: contracts/compound_rari_fork/Comptroller.sol (line 44)

    event NewTickOracle(TickOracle oldTickOracle, TickOracle newTickOracle);

11. File: contracts/compound_rari_fork/Comptroller.sol (line 47)

    event NewUniV3LpVault(IUniV3LpVault oldVault, IUniV3LpVault newVault);

12. File: contracts/compound_rari_fork/Comptroller.sol (line 50)

    event NewPauseGuardian(address oldPauseGuardian, address newPauseGuardian);

13. File: contracts/compound_rari_fork/Comptroller.sol (line 53)

    event ActionPaused(string action, bool pauseState);

Not using the named return variables when a function returns, is confusing

1. File: contracts/libs/UniswapTwapLibrary.sol (line 94)

       return OracleLibrary.consultAtHistoricTime(_pool, requestPeriod, 0);

2. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

3. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

4. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

5. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

6. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

7. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

8. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 155)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, true, nfpManager, factory, address(this));

9. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

       return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

10. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

        return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

11. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

        return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

12. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

        return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

13. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

        return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

14. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

        return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

15. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 184)

        return LpBreakdownLibrary._getTokenBreakdown(tokenId, false, nfpManager, factory, address(this));

Use a more recent version of solidity

Use a solidity version of at least 0.8.0 to get overflow protection without needing SafeMath Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>) Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>)

1. File: contracts/libs/LpBreakdownLibrary.sol (line 1)

   pragma solidity ^0.7.6;

2. File: contracts/libs/TicksLibrary.sol (line 2)

   pragma solidity ^0.7.6;

3. File: contracts/libs/UniswapTwapLibrary.sol (line 3)

   pragma solidity ^0.7.6;

4. File: contracts/libs/LiquidityLibrary.sol (line 2)

   pragma solidity ^0.7.6;

5. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 1)

   pragma solidity ^0.7.6;

Don't compare boolean expressions to boolean literals

if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 838)

       require(msg.sender == comptroller.admin() || state == true, "only admin can unpause");

2. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 854)

       require(msg.sender == comptroller.admin() || state == true, "only admin can unpause");

3. File: contracts/compound_rari_fork/Comptroller.sol (line 142)

       if (marketToJoin.accountMembership[borrower] == true) {

4. File: contracts/compound_rari_fork/Comptroller.sol (line 1410)

       require(msg.sender == admin || state == true, "only admin can unpause");

5. File: contracts/compound_rari_fork/Comptroller.sol (line 1420)

       require(msg.sender == admin || state == true, "only admin can unpause");

6. File: contracts/compound_rari_fork/Comptroller.sol (line 1429)

       require(msg.sender == admin || state == true, "only admin can unpause");

7. File: contracts/compound_rari_fork/Comptroller.sol (line 1438)

       require(msg.sender == admin || state == true, "only admin can unpause");

8. File: contracts/compound_rari_fork/Comptroller.sol (line 1523)

           borrowGuardianPaused[address(cToken)] == true &&

Remove unused variables

1. File: contracts/vault_and_oracles/UniswapTwapOracle.sol (line 26)

   uint128 private constant ONE = 1e18;

Duplicated require()/revert() checks should be refactored to a modifier or function

Copy-pasted code is error-prone when conditions need to be updated

1. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 198)

       require(!peripheryGuardianPaused, "periphery functionality is paused");

2. File: contracts/vault_and_oracles/UniV3LpVault.sol (lines 850-853)

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

3. File: contracts/vault_and_oracles/UniV3LpVault.sol (line 854)

       require(msg.sender == comptroller.admin() || state == true, "only admin can unpause");

4. File: contracts/vault_and_oracles/MasterPriceOracle.sol (lines 62-65)

       require(
           underlyings.length > 0 && underlyings.length == _oracles.length,
           "Lengths of both arrays must be equal and greater than 0."
       );

5. File: contracts/vault_and_oracles/MasterPriceOracle.sol (lines 128-131)

       require(
           address(oracles[underlying]) != address(0),
           "Price oracle not found for this underlying token address."
       );

6. File: contracts/compound_rari_fork/CToken.sol (line 285)

       require(accrueInterest() == uint256(Error.NO_ERROR), "accrue interest failed");

7. File: contracts/compound_rari_fork/CToken.sol (lines 638-641)

       require(
           msg.sender == address(comptroller.uniV3LpVault()) || msg.sender == redeemer,
           "only the LpVault may redeem other's assets"
       );

8. File: contracts/compound_rari_fork/CToken.sol (line 1233)

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

9. File: contracts/compound_rari_fork/Comptroller.sol (line 394)

           require(msg.sender == cToken, "sender must be cToken");

10. File: contracts/compound_rari_fork/Comptroller.sol (line 546)

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

11. File: contracts/compound_rari_fork/Comptroller.sol (line 617)

        require(!seizeGuardianPaused, "seize is paused");

12. File: contracts/compound_rari_fork/Comptroller.sol (line 1418)

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

13. File: contracts/compound_rari_fork/Comptroller.sol (line 1419)

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

14. File: contracts/compound_rari_fork/Comptroller.sol (line 1420)

        require(msg.sender == admin || state == true, "only admin can unpause");

15. File: contracts/compound_rari_fork/Comptroller.sol (line 1485)

        require(pools.length > 0, "must have at least one pool");

Non-exploitable re-entrancies

Reentrancy in UniV3LpVault._beforeNonReentrant(bool) (contracts/vault_and_oracles/UniV3LpVault.sol#934-938):
    External calls:
    - comptroller._beforeNonReentrant() (contracts/vault_and_oracles/UniV3LpVault.sol#936)
    State variables written after the call(s):
    - _notEntered = false (contracts/vault_and_oracles/UniV3LpVault.sol#937)

Reentrancy in CToken.borrowFresh(address,address,uint256) (contracts/compound_rari_fork/CToken.sol#835-916):
    External calls:
    - allowed = comptroller.borrowAllowed(address(this),borrower,borrowAmount) (contracts/compound_rari_fork/CToken.sol#841)
    State variables written after the call(s):
    - accountBorrows[borrower].principal = vars.accountBorrowsNew (contracts/compound_rari_fork/CToken.sol#908)
    - accountBorrows[borrower].interestIndex = borrowIndex (contracts/compound_rari_fork/CToken.sol#909)
    - totalBorrows = vars.totalBorrowsNew (contracts/compound_rari_fork/CToken.sol#910)
Reentrancy in CErc20.initialize(address,ComptrollerInterface,InterestRateModel,string,string,uint256) (contracts/compound_rari_fork/CErc20.sol#20-44):
    External calls:
    - super.initialize(comptroller_,interestRateModel_,initialExchangeRateMantissa_,name_,symbol_,decimals_,reserveFactorMantissa_) (contracts/compound_rari_fork/CErc20.sol#31-39)
        - address(oldInterestRateModel).call(abi.encodeWithSignature(resetInterestCheckpoints())) (contracts/compound_rari_fork/CToken.sol#1624)
        - address(newInterestRateModel).call(abi.encodeWithSignature(checkpointInterest())) (contracts/compound_rari_fork/CToken.sol#1627)
    State variables written after the call(s):
    - underlying = underlying_ (contracts/compound_rari_fork/CErc20.sol#42)

Reentrancy in UniV3LpVault._mint(INonfungiblePositionManager.MintParams) (contracts/vault_and_oracles/UniV3LpVault.sol#696-728):
    External calls:
    - IERC20Detailed(params.token0).approve(address(nonfungiblePositionManager),params.amount0Desired) (contracts/vault_and_oracles/UniV3LpVault.sol#704)
    - IERC20Detailed(params.token1).approve(address(nonfungiblePositionManager),params.amount1Desired) (contracts/vault_and_oracles/UniV3LpVault.sol#705)
    - (tokenId,None,amount0,amount1) = nonfungiblePositionManager.mint(INonfungiblePositionManager.MintParams(params.token0,params.token1,params.fee,params.tickLower,params.tickUpper,params.amount0Desired,params.amount1Desired,params.amount0Min,params.amount1Min,address(this),params.deadline)) (contracts/vault_and_oracles/UniV3LpVault.sol#707-721)
    - IERC20Detailed(params.token0).approve(address(nonfungiblePositionManager),0) (contracts/vault_and_oracles/UniV3LpVault.sol#723)
    - IERC20Detailed(params.token1).approve(address(nonfungiblePositionManager),0) (contracts/vault_and_oracles/UniV3LpVault.sol#724)
    State variables written after the call(s):
    - _processNewToken(tokenId,params.recipient) (contracts/vault_and_oracles/UniV3LpVault.sol#727)
        - ownerOf[tokenId] = account (contracts/vault_and_oracles/UniV3LpVault.sol#757)
    - _processNewToken(tokenId,params.recipient) (contracts/vault_and_oracles/UniV3LpVault.sol#727)
        - userTokens[account].push(tokenId) (contracts/vault_and_oracles/UniV3LpVault.sol#756)

[0xdramaone]

Great report