Venus Protocol Isolated Pools - 0xAce's results

[L-01] Check if baseRatePerYear > blocksPerYear to avoid division-error

Here we need to check if baseRatePerYear > blocksPerYear to avoid division-error.

BaseJumpRateModelV2.sol

 function _updateJumpRateModel(
        uint256 baseRatePerYear,
        uint256 multiplierPerYear,
        uint256 jumpMultiplierPerYear,
        uint256 kink_
    ) internal {
        baseRatePerBlock = baseRatePerYear / blocksPerYear; // @audit check if baseRatePerYear > blocksPerYear to avoid division-error
        multiplierPerBlock = (multiplierPerYear * BASE) / (blocksPerYear * kink_);
        jumpMultiplierPerBlock = jumpMultiplierPerYear / blocksPerYear;
        kink = kink_;

        emit NewInterestParams(baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink);
    }

[L-02] Use require() statement to prevent irregular flow of marketCount and actionCount

what if marketIdx > actionIdx or any combination that cause irregular flow @discuss marketCount and actionsCount using require.

    function setActionsPaused(VToken[] calldata marketsList, Action[] calldata actionsList, bool paused) external {
        _checkAccessAllowed("setActionsPaused(address[],uint256[],bool)");

        uint256 marketsCount = marketsList.length;
        uint256 actionsCount = actionsList.length;

        _ensureMaxLoops(marketsCount);

        for (uint256 marketIdx; marketIdx < marketsCount; ++marketIdx) {
            // @audit what if marketIdx > actionIdx or any combination that cause irregular flow @discuss or check marketCount and actionsCount using `require`
            for (uint256 actionIdx; actionIdx < actionsCount; ++actionIdx) {
                _setActionPaused(address(marketsList[marketIdx]), actionsList[actionIdx], paused);
            }
        }
    }

Also here require() statement is necessary to avoid underflow.

function releaseFunds(address comptroller, address asset, uint256 amount) external returns (uint256) {
        require(asset != address(0), "ProtocolShareReserve: Asset address invalid");
        require(amount <= poolsAssetsReserves[comptroller][asset], "ProtocolShareReserve: Insufficient pool balance");

        assetsReserves[asset] -= amount; // @note
        poolsAssetsReserves[comptroller][asset] -= amount; // @note
        uint256 protocolIncomeAmount = mul_(
            Exp({ mantissa: amount }),
            div_(Exp({ mantissa: protocolSharePercentage * expScale }), baseUnit)
        ).mantissa;

        IERC20Upgradeable(asset).safeTransfer(protocolIncome, protocolIncomeAmount);
        IERC20Upgradeable(asset).safeTransfer(riskFund, amount - protocolIncomeAmount); // @audit this will underflow if the amount is less than protocolIncomeAmount -> this will drain the funds to riskFund may be

        // Update the pool asset's state in the risk fund for the above transfer.
        IRiskFund(riskFund).updateAssetsState(comptroller, asset); // @audit before updation of state the attacker may call a withdraw function or will call a function that contrain asset for his own benefit

        emit FundsReleased(comptroller, asset, amount);

        return amount;
    }

[L-03] Unecessary use of extra variables results in slow process of code

Comptroller.sol

   uint256 rewardsDistributorsLength = rewardsDistributors.length;// 1st time

        for (uint256 i; i < rewardsDistributorsLength; ++i) {
            address rewardToken = address(rewardsDistributors[i].rewardToken());
            require(
                rewardToken != address(_rewardsDistributor.rewardToken()),
                "distributor already exists with this reward"
            );
        }

        uint256 rewardsDistributorsLen = rewardsDistributors.length;// again here

[L-04] Comment for protocolShareReserve variable should be changed

Here comment must be //protocolShareReserve address

 /**
     * @notice Shortfall contract address
     */
    Shortfall public shortfall;

    /**
     * @notice Shortfall contract address // @audit this comment should be changed
     */
    address payable public protocolShareReserve;

[G-01] Use Assembly for address(0) check

We can save 6 gas by writing the address(0) check of the require statement in assembly. For Eg:

Instead of this...

function ownerNotZero(address _addr) public pure {
    require(_addr != address(0), "zero address)");
}

We can use this...

function assemblyOwnerNotZero(address _addr) public pure {
    assembly {
        if iszero(_addr) {
            mstore(0x00, "zero address")
            revert(0x00, 0x20)
        }
    }
}

BaseJumpRateModelV2.sol

require(address(accessControlManager_) != address(0), "invalid ACM address");

Comptroller.sol

require(poolRegistry_ != address(0), "invalid pool registry address");

[G-02] Storing variable.length in local variables in for loops

Use an local variable to store the length uint256 length = rewardsDistributors.lengthso that for loop doesn't the fetch the length all time.

PoolLens.sol

for (uint256 i; i < rewardsDistributors.length; ++i) {...}

 for (uint256 i; i < markets.length; ++i) {...}

[G-03] Write the result rather than exponent

Use For Example: uint256 var = 1000 rather than exponentiation uint256 var = 10**3

[G-04] Use Assembly in the constructor while assigning address

Use assembly to assign address in the constructor() to save Gas.

    // @audit check all the contructor assigning address to use assembly -> it will optimize it
    constructor(address poolRegistry_) {
        require(poolRegistry_ != address(0), "invalid pool registry address");
        // @audit
        poolRegistry = poolRegistry_; //
        @note use assembly to store address in constructor
        // @note eg->  assembly {sstore(vault. revenueAddress, _revenueAddress)}
        _disableInitializers();
    }

[G-05] Use a=a+b or a=a-b instead of a+=b or a+=b

Use a=a+b or a=a-b instead of a+=b or a+=b which saves Gas.

ProtocolShareReserve.sol

 assetsReserves[asset] -= amount; // @note

poolsAssetsReserves[comptroller][asset]
-= amount; // @note

[G-06] Use bytes32 instead of shortstring datatype variable

Use bytes32 instead of string for short strings to save Gas.

VTokenInterfaces.sol

 string public name;

 string public symbol;

[G-07] Use != instead of >0 for uint datatypes

As here deletaBlocks has datatype uint256 and as it cannot be negative value, it is better to use !=0 rather than >0to save gas.

RewardsDistributor.sol
       if (deltaBlocks > 0 && supplySpeed > 0) {...}