Venus Protocol Isolated Pools - K42's results

Gas (or mana(as of new eip)) Optimization Report by K42

Possible Optimizations in Comptroller.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol

Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol#LL449C6-L454C10 -- In the preLiquidateHook function, you are using an nested if statement that checks skipLiquidityCheck and isDeprecated this can be simplified by combining the conditions with a single || operator, like so if (skipLiquidityCheck || isDeprecated(VToken(vTokenBorrowed)) || repayAmount > borrowBalance) { revert TooMuchRepay(); } doing this will save gas.

Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol#LL501C1-L516C10 -- In the preSeizeHook function, you are calling VToken(vTokenCollateral).comptroller() twice in different conditions. Whereas you can store it in a variable to avoid calling it twice. This will save gas costs, you can do this like so address collateralComptroller = address(VToken(vTokenCollateral).comptroller()); if (seizerContract == address(this)) { if (collateralComptroller != address(this)) { revert ComptrollerMismatch(); } } else { if (!markets[seizerContract].isListed) { revert MarketNotListed(seizerContract); } if (collateralComptroller != VToken(seizerContract).comptroller()) { revert ComptrollerMismatch(); } } Possible Optimization 3 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol#LL521C5-L525C10 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol#LL558C7-L562C10 -- In both preSeizeHook and preTransferHook, you are looping over the rewardsDistributors array and performing the same operations multiple times which consumes a lot of gas, so you could create an internal function instead, like this function _updateAndDistributeRewards( address vToken, address src, address dst ) internal { uint256 rewardDistributorsCount = rewardsDistributors.length; for (uint256 i; i < rewardDistributorsCount; ++i) { rewardsDistributors[i].updateRewardTokenSupplyIndex(vToken); rewardsDistributors[i].distributeSupplierRewardToken(vToken, src); rewardsDistributors[i].distributeSupplierRewardToken(vToken, dst); } } Then, you can replace the duplicated code in preSeizeHook and preTransferHook, for preSeizeHook you can do this _updateAndDistributeRewards(vTokenCollateral, borrower, liquidator); and for preTransferHook you can do this _updateAndDistributeRewards(vToken, src, dst); Other than that, this contract is looking awesome.

Possible Optimization in VToken.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L752 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L808 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L882 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L939 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L1035 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L1156 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L1183 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L1205 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L1248 -- Instead of checking freshness with the if (accrualBlockNumber != _getBlockNumber()) statements at the following links^, consider using a modifier instead to reduce code repetition and reduce gas, like so modifier onlyFresh() { require(accrualBlockNumber == _getBlockNumber(), "Not fresh"); _; } Then add it to _mintFresh, _redeemFresh, _borrowFresh, _repayBorrowFresh, _liquidateBorrowFresh, _setReserveFactorFresh, _addReservesFresh, _reduceReservesFresh, _setInterestRateModelFresh function declarations.

Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#LL789C1-L790C55 --emit Transfer(address(0), minter, mintTokens); and emit Mint(minter, actualMintAmount, mintTokens, balanceAfter); both contain minter and mintTokens. So you could merge them into a single event instead of 2 seperate ones, to save gas like so event MintAndTransfer(address indexed minter, uint256 actualMintAmount, uint256 mintTokens, uint256 balanceAfter); Then emit this instead emit MintAndTransfer(minter, actualMintAmount, mintTokens, balanceAfter); Other than that, looking optimized.

Possible Optimization in PoolLens.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Lens/PoolLens.sol Possible Optimization = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Lens/PoolLens.sol#LL290C9-L292C73 -- In the vTokenBalances function, you initialize IERC20 underlying, then use it only for calling balanceOf and allowance with this method IERC20 underlying = IERC20(vToken.underlying()); tokenBalance = underlying.balanceOf(account); tokenAllowance = underlying.allowance(account, address(vToken)); However you can call these methods directly instead, and it will save you gas, you can do this like this tokenBalance = IERC20(vToken.underlying()).balanceOf(account); tokenAllowance = IERC20(vToken.underlying()).allowance(account, address(vToken)); Other than that, looking optimized.

Possible Optimization in Shortfall.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L69 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#LL51C11-L51C11 -- You could use immutable for convertibleBaseAsset to save some gas, like so address public immutable convertibleBaseAsset; Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#LL105C1-L112C105 -- You could combine multiple events with similar data into one event to save gas. Specifically instead of having separate events for MinimumPoolBadDebtUpdated, WaitForFirstBidderUpdated, and NextBidderBlockLimitUpdated, you could create a single event with a name like ParameterUpdated with a string parameter indicating which parameter was updated, this will save you gas.

Possible Optimizations in RewardsDistributor.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Rewards/RewardsDistributor.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Rewards/RewardsDistributor.sol#LL277C1-L292C6 -- In claimRewardToken you can cache the rewardToken address in memory as this will save gas on SLOAD operations, to do this, in claimRewardToken before the for loop, add this IERC20Upgradeable _rewardToken = rewardToken; Then inside the for loop do this rewardTokenAccrued[holder] = _grantRewardToken(holder, _rewardToken, rewardTokenAccrued[holder]); If you make this change then also change _grantRewardToken to function _grantRewardToken(address user, IERC20Upgradeable _rewardToken, uint256 amount) internal returns (uint256) this will reduce SLOAD operations and save gas. Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Rewards/RewardsDistributor.sol#LL126C44-L126C44 -- You could remove all uses of the getBlockNumber() function and just use block.number instead. This will reduce the overhead of the function call therefore saving gas. To do this, replace all occurrences of getBlockNumber() with block.number, however keep it if being use for testing purposes. Possible Optimization 3 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Rewards/RewardsDistributor.sol#L202 -- In setRewardTokenSpeeds, instead of checking for access permissions using _checkAccessAllowed, you could use the onlyRole modifier from the AccessControl contract by OpenZeppelin, doing this will save you gas and optimize the use of roles more so, to do this add bytes32 public constant SET_REWARD_TOKEN_SPEEDS_ROLE = keccak256("SET_REWARD_TOKEN_SPEEDS_ROLE"); to beginning of contract, then add this _setupRole(SET_REWARD_TOKEN_SPEEDS_ROLE, msg.sender); to the initialize function (https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Rewards/RewardsDistributor.sol#L111) then you can add the role to function setRewardTokenSpeeds( VToken[] memory vTokens, uint256[] memory supplySpeeds, uint256[] memory borrowSpeeds ) external onlyRole(SET_REWARD_TOKEN_SPEEDS_ROLE) this will save gas compared to using _checkAccessAllowed in setRewardTokenSpeeds.

Possible Optimization in PoolRegistry.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Pool/PoolRegistry.sol Possible Optimization = -- In _transferIn, you can use unchecked for balanceAfter - balanceBefore; since balanceAfter will always be >= balanceBefore, there is no risk of underflow, so to save gas change to function _transferIn( IERC20Upgradeable token, address from, uint256 amount ) internal returns (uint256) { uint256 balanceBefore = token.balanceOf(address(this)); token.safeTransferFrom(from, address(this), amount); uint256 balanceAfter = token.balanceOf(address(this)); unchecked { return balanceAfter - balanceBefore; } }

Possible Optimizations in RiskFund.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/RiskFund.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/RiskFund.sol#L238 -- In the _swapAsset function you can use staticcall instead of a regular call to save gas, by avoiding state changes, to do this change ComptrollerViewInterface(comptroller).oracle().updatePrice(address(vToken)); on line 238, to this (bool success, bytes memory data) = address(comptroller).staticcall( abi.encodeWithSignature("oracle().updatePrice(address)", address(vToken))); require(success, "RiskFund: oracle updatePrice failed"); Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/RiskFund.sol#LL175C1-L176C55 -- In swapPoolsAssets, you can wrap the following statements with an unchecked block unchecked { poolReserves[comptroller] = poolReserves[comptroller] + swappedTokens; totalAmount = totalAmount + swappedTokens; } to save gas (overflow protection is already there, so you can do this safely and save gas). Possible Optimization 3 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/RiskFund.sol#LL193C4-L193C72 -- You can also use unchecked for poolReserves[comptroller] = poolReserves[comptroller] - amount; in transferReserveForAuction to save gas (underflow protection is already there, so you can do this safely and save gas).

Possible Optimizations in ExponentialNoError.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/ExponentialNoError.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/ExponentialNoError.sol#LL38C4-L41C6 -- mul_ScalarTruncate can be modified to avoid creating a new Exp memory variable, therefore saving gas, instead of this, directly perform the multiplication and division operations like so function mul_ScalarTruncate(Exp memory a, uint256 scalar) internal pure returns (uint256) { return (a.mantissa * scalar) / expScale; } Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/ExponentialNoError.sol#LL47C4-L54C6 -- mul_ScalarTruncateAddUInt can also be optimized in a similar way, like this function mul_ScalarTruncateAddUInt( Exp memory a, uint256 scalar, uint256 addend ) internal pure returns (uint256) { return ((a.mantissa * scalar) / expScale) + addend; }

Possible Optimizations in BaseJumpRateModelV2.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/BaseJumpRateModelV2.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/BaseJumpRateModelV2.sol#LL157C8-L160C22 -- You can refactor the calculation of multiplierPerBlock in _updateJumpRateModel to reduce the number of divisions and therefore reducing gas, do this like so multiplierPerBlock = (multiplierPerYear * BASE) / blocksPerYear; multiplierPerBlock = multiplierPerBlock / kink_; Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/BaseJumpRateModelV2.sol#LL157C1-L159C72 -- You can also refactor the calculation of baseRatePerBlock and jumpMultiplierPerBlock like so uint256 invBlocksPerYear = BASE / blocksPerYear; baseRatePerBlock = (baseRatePerYear * invBlocksPerYear) / BASE; jumpMultiplierPerBlock = (jumpMultiplierPerYear * invBlocksPerYear) / BASE;

So given the last 2 optimizations are implemented _updateJumpRateModel would look like this function _updateJumpRateModel( uint256 baseRatePerYear, uint256 multiplierPerYear, uint256 jumpMultiplierPerYear, uint256 kink_ ) internal { uint256 invBlocksPerYear = BASE / blocksPerYear; baseRatePerBlock = (baseRatePerYear * invBlocksPerYear) / BASE; jumpMultiplierPerBlock = (jumpMultiplierPerYear * invBlocksPerYear) / BASE; multiplierPerBlock = (multiplierPerYear * BASE) / blocksPerYear; multiplierPerBlock = multiplierPerBlock / kink_; kink = kink_; emit NewInterestParams(baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink); } Possible Optimization 3 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/BaseJumpRateModelV2.sol#LL137C1-L141C63 -- You can optimize utilizationRate by eliminating if (borrows == 0) { return 0;}, as the division operation will yield 0 anyway when borrows is 0, change to just return (borrows * BASE) / (cash + borrows - reserves); to save gas. Possible Optimization 4 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/BaseJumpRateModelV2.sol#L172 -- In _getBorrowRate, you can use a ternary expression to calculate the rate without explicitly using an if statement, as conditional branching can be expensive in terms of gas, this modification would look like this function _getBorrowRate( uint256 cash, uint256 borrows, uint256 reserves ) internal view returns (uint256) { uint256 util = utilizationRate(cash, borrows, reserves); uint256 excessUtil = util > kink ? util - kink : 0; uint256 jumpPart = (excessUtil * jumpMultiplierPerBlock) / BASE; uint256 normalRate = ((util * multiplierPerBlock) / BASE) + baseRatePerBlock; return normalRate + jumpPart; }

Possible Optimization in ComptrollerStorage.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/ComptrollerStorage.sol Possible Optimization = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/ComptrollerStorage.sol#L29 -- In the Market``` struct, you could use uint96forcollateralFactorMantissaandliquidationThresholdMantissa`` as they are both less than 1, and are stored as mantissa values. This will optimize storage space and gas.

Possible Optimization in ProtocolShareReserve.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/ProtocolShareReserve.sol Possible Optimization = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/ProtocolShareReserve.sol#LL66C1-L90C6 -- In this contract, there is a one minor gas optimization that can be achieved by caching the value of amount - protocolIncomeAmount instead of recalculating it in the releaseFunds function, like so 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; poolsAssetsReserves[comptroller][asset] -= amount; uint256 protocolIncomeAmount = mul_( Exp({ mantissa: amount }), div_(Exp({ mantissa: protocolSharePercentage * expScale }), baseUnit) ).mantissa; // Add the optimization here. uint256 riskFundAmount = amount - protocolIncomeAmount; IERC20Upgradeable(asset).safeTransfer(protocolIncome, protocolIncomeAmount); // Then here. IERC20Upgradeable(asset).safeTransfer(riskFund, riskFundAmount); // Update the pool asset's state in the risk fund for the above transfer. IRiskFund(riskFund).updateAssetsState(comptroller, asset); emit FundsReleased(comptroller, asset, amount); return amount; }

Possible Optimizations in VTokenProxyFactory.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Factories/VTokenProxyFactory.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Factories/VTokenProxyFactory.sol#L32 -- Instead of VToken.initialize.selector you can use the function signature directly as a constant, to save gas, to do this add bytes4 private constant INITIALIZATION_SIGNATURE = bytes4(keccak256("initialize(address,ComptrollerInterface,InterestRateModel,uint256,string,string,uint8,address,AccessControlManager,(uint256,uint256,uint256,uint256),uint256)")); Then, replace VToken.initialize.selector (line 32) in the abi.encodeWithSelector with INITIALIZATION_SIGNATURE.

Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Factories/VTokenProxyFactory.sol#LL22C7-L22C31 If you're deploying multiple VToken proxies with the same beaconAddress, you can use an immutable state var and then remove it from the VTokenArgs struct, like so address public immutable beaconAddress; , then add constructor(address _beaconAddress) { beaconAddress = _beaconAddress; } to set it. Then change (https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Factories/VTokenProxyFactory.sol#L30) to just beaconAddress, instead of input.beaconAddress when creating a new BeaconProxy. This will save gas.

Possible Optimizations in ReserveHelpers.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/ReserveHelpers.sol Possible Optimization = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/ReserveHelpers.sol#LL50C2-L71C2 -- Currently IERC20Upgradeable(asset) is being called twice in updateAssetsState. However, you can cache this object to a variable, reducing the number of times IERC20Upgradeable(asset) is called, like so function updateAssetsState(address comptroller, address asset) public virtual { require(ComptrollerInterface(comptroller).isComptroller(), "ReserveHelpers: Comptroller address invalid"); require(asset != address(0), "ReserveHelpers: Asset address invalid"); require(poolRegistry != address(0), "ReserveHelpers: Pool Registry address is not set"); require( PoolRegistryInterface(poolRegistry).getVTokenForAsset(comptroller, asset) != address(0), "ReserveHelpers: The pool doesn't support the asset" ); IERC20Upgradeable assetToken = IERC20Upgradeable(asset); uint256 currentBalance = assetToken.balanceOf(address(this)); uint256 assetReserve = assetsReserves[asset]; if (currentBalance > assetReserve) { uint256 balanceDifference; unchecked { balanceDifference = currentBalance - assetReserve; } assetsReserves[asset] += balanceDifference; poolsAssetsReserves[comptroller][asset] += balanceDifference; emit AssetsReservesUpdated(comptroller, asset, balanceDifference); } } This will save you some gas.

Possible Optimization in MaxLoopsLimitHelper.sol

Scope = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/MaxLoopsLimitHelper.sol Possible Optimization 1 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/MaxLoopsLimitHelper.sol#L13 -- You can remove the __gap variable, and instead use the Initializable contract from OpenZeppelin. As the Initializable contract already has storage gap management built-in. This change will make the contract easier to maintain and more consistent with other upgradeable contracts. Import and Inherit from Initializable like so, import import "@openzeppelin/contracts/proxy/utils/Initializable.sol" and abstract contract MaxLoopsLimitHelper is Initializable {} Possible Optimization 2 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/MaxLoopsLimitHelper.sol#L25 -- Change setMaxLoopsLimit function to external rather than internal. As it modifies the contract state and emits an event, it should not be called from another contract function within the same contract. By changing it to external, you will slightly reduce the gas cost when calling the function directly, because external function arguments are read directly from calldata instead of being copied to memory. Possible Optimization 3 = https://github.com/code-423n4/2023-05-venus/blob/main/contracts/MaxLoopsLimitHelper.sol#L39 -- _ensureMaxLoops currently checks if the provided loop length len is > maxLoopsLimit. However, you can pass both the loop length and the maximum allowed limit as parameters. This way, you can use the function to check loop limits in different contexts, not just against a single maxLoopsLimit value, change like so function _ensureMaxLoops(uint256 len, uint256 allowedLimit) internal pure { if (len > allowedLimit) { revert MaxLoopsLimitExceeded(allowedLimit, len); } } @notice = this version uses pure as it doesn't access any storage variables, this will also save gas (as pure doesn't require SLOAD operations or access to storage).