Venus Protocol Isolated Pools - matrix_0wl's results

Non Critical Issues

[NC-1] ADD A TIMELOCK TO CRITICAL FUNCTIONS

Description:

It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user).

Proof Of Concept

File: Comptroller.sol

726:     function setCollateralFactor(

779:     function setLiquidationIncentive(uint256 newLiquidationIncentiveMantissa) external {

836:     function setMarketBorrowCaps(VToken[] calldata vTokens, uint256[] calldata newBorrowCaps) external {

862:     function setMarketSupplyCaps(VToken[] calldata vTokens, uint256[] calldata newSupplyCaps) external {

885:     function setActionsPaused(

912:     function setMinLiquidatableCollateral(uint256 newMinLiquidatableCollateral) external {

961:     function setPriceOracle(PriceOracle newOracle) external onlyOwner {

973:     function setMaxLoopsLimit(uint256 limit) external onlyOwner {

1224:     function _setActionPaused(

1381:     function _getCollateralFactor(VToken asset) internal view returns (Exp memory) {

1390:     function _getLiquidationThreshold(VToken asset) internal view returns (Exp memory) {

File: MaxLoopsLimitHelper.sol

25:     function _setMaxLoopsLimit(uint256 limit) internal {

File: Pool/PoolRegistry.sol

188:     function setProtocolShareReserve(address payable protocolShareReserve_) external onlyOwner {

198:     function setShortfallContract(Shortfall shortfall_) external onlyOwner {

332:     function setPoolName(address comptroller, string calldata name) external {

File: Rewards/RewardsDistributor.sol

197:     function setRewardTokenSpeeds(

219:     function setContributorRewardTokenSpeed(address contributor, uint256 rewardTokenSpeed) external onlyOwner {

249:     function setMaxLoopsLimit(uint256 limit) external onlyOwner {

304:     function _setRewardTokenSpeed(

File: RiskFund/ProtocolShareReserve.sol

53:     function setPoolRegistry(address _poolRegistry) external onlyOwner {

File: RiskFund/RiskFund.sol

99:     function setPoolRegistry(address _poolRegistry) external onlyOwner {

110:     function setShortfallContractAddress(address shortfallContractAddress_) external onlyOwner {

126:     function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner {

137:     function setMinAmountToConvert(uint256 minAmountToConvert_) external {

205:     function setMaxLoopsLimit(uint256 limit) external onlyOwner {

File: VToken.sol

310:     function setProtocolSeizeShare(uint256 newProtocolSeizeShareMantissa_) external {

330:     function setReserveFactor(uint256 newReserveFactorMantissa) external override nonReentrant {

369:     function setInterestRateModel(InterestRateModel newInterestRateModel) external override {

505:     function setProtocolShareReserve(address payable protocolShareReserve_) external onlyOwner {

515:     function setShortfallContract(address shortfall_) external onlyOwner {

1138:     function _setComptroller(ComptrollerInterface newComptroller) internal {

1154:     function _setReserveFactorFresh(uint256 newReserveFactorMantissa) internal {

1243:     function _setInterestRateModelFresh(InterestRateModel newInterestRateModel) internal {

1398:     function _setShortfallContract(address shortfall_) internal {

1407:     function _setProtocolShareReserve(address payable protocolShareReserve_) internal {

File: VTokenInterfaces.sol

325:     function setReserveFactor(uint256 newReserveFactorMantissa) external virtual;

333:     function setInterestRateModel(InterestRateModel newInterestRateModel) external virtual;

[NC-2] GENERATE PERFECT CODE HEADERS EVERY TIME

Description:

I recommend using header for Solidity code layout and readability: https://github.com/transmissions11/headers

/*//////////////////////////////////////////////////////////////
                           TESTING 123
//////////////////////////////////////////////////////////////*/

[NC-3] USE A SINGLE FILE FOR ALL SYSTEM-WIDE CONSTANTS

Description:

There are many addresses and constants used in the system. It is recommended to put the most used ones in one file (for example constants.sol, use inheritance to access these values).

Proof Of Concept

File: BaseJumpRateModelV2.sol

23:     uint256 public constant blocksPerYear = 10512000;

File: ErrorReporter.sol

5:     uint256 public constant NO_ERROR = 0; // support legacy return codes

File: InterestRateModel.sol

10:     bool public constant isInterestRateModel = true;

File: Rewards/RewardsDistributor.sol

29:     uint224 public constant rewardTokenInitialIndex = 1e36;

File: VTokenInterfaces.sol

142:     bool public constant isVToken = true;

File: WhitePaperInterestRateModel.sol

17:     uint256 public constant blocksPerYear = 2102400;

[NC-4] FUNCTIONS, PARAMETERS AND VARIABLES IN SNAKE CASE

Description:

Use camel case for all functions, parameters and variables.

Proof Of Concept

File: ExponentialNoError.sol

38:     function mul_ScalarTruncate(Exp memory a, uint256 scalar) internal pure returns (uint256) {

47:     function mul_ScalarTruncateAddUInt(

[NC-5] FOR MODERN AND MORE READABLE CODE; UPDATE IMPORT USAGES

Description:

Solidity code is also cleaner in another way that might not be noticeable: the struct Point. We were importing it previously with global import but not using it. The Point struct polluted the source code with an unnecessary object we were not using because we did not need it.

Proof Of Concept

File: BaseJumpRateModelV2.sol

4: import "@venusprotocol/governance-contracts/contracts/Governance/IAccessControlManagerV8.sol";

5: import "./InterestRateModel.sol";

File: Comptroller.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

6: import "./VToken.sol";

7: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

8: import "./ComptrollerInterface.sol";

9: import "./ComptrollerStorage.sol";

10: import "./Rewards/RewardsDistributor.sol";

11: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlManager.sol";

12: import "./MaxLoopsLimitHelper.sol";

File: ComptrollerInterface.sol

4: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

5: import "./VToken.sol";

6: import "./Rewards/RewardsDistributor.sol";

File: ComptrollerStorage.sol

4: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

5: import "./VToken.sol";

File: Factories/JumpRateModelFactory.sol

4: import "../JumpRateModelV2.sol";

File: Factories/VTokenProxyFactory.sol

4: import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";

6: import "../VToken.sol";

7: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlManager.sol";

8: import "../VTokenInterfaces.sol";

File: Factories/WhitePaperInterestRateModelFactory.sol

4: import "../WhitePaperInterestRateModel.sol";

File: JumpRateModelV2.sol

4: import "./BaseJumpRateModelV2.sol";

File: Lens/PoolLens.sol

4: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";

5: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

7: import "../VToken.sol";

8: import "../ComptrollerInterface.sol";

9: import "../Pool/PoolRegistryInterface.sol";

10: import "../Pool/PoolRegistry.sol";

File: Pool/PoolRegistry.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

5: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

6: import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";

7: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";

9: import "../Comptroller.sol";

10: import "../Factories/VTokenProxyFactory.sol";

11: import "../Factories/JumpRateModelFactory.sol";

12: import "../Factories/WhitePaperInterestRateModelFactory.sol";

13: import "../WhitePaperInterestRateModel.sol";

14: import "../VToken.sol";

15: import "../InterestRateModel.sol";

16: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlManager.sol";

17: import "../Shortfall/Shortfall.sol";

18: import "../VTokenInterfaces.sol";

19: import "./PoolRegistryInterface.sol";

File: Proxy/UpgradeableBeacon.sol

4: import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";

File: Rewards/RewardsDistributor.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "../ExponentialNoError.sol";

7: import "../VToken.sol";

8: import "../Comptroller.sol";

9: import "../MaxLoopsLimitHelper.sol";

10: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlledV8.sol";

File: RiskFund/ProtocolShareReserve.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

7: import "../ExponentialNoError.sol";

8: import "./IRiskFund.sol";

9: import "./ReserveHelpers.sol";

10: import "./IProtocolShareReserve.sol";

File: RiskFund/ReserveHelpers.sol

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "../ComptrollerInterface.sol";

7: import "../Pool/PoolRegistryInterface.sol";

File: RiskFund/RiskFund.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

7: import "../VToken.sol";

8: import "../Pool/PoolRegistry.sol";

9: import "../IPancakeswapV2Router.sol";

10: import "./ReserveHelpers.sol";

11: import "./IRiskFund.sol";

12: import "../Shortfall/IShortfall.sol";

13: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlledV8.sol";

14: import "../MaxLoopsLimitHelper.sol";

File: Shortfall/Shortfall.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

7: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

9: import "../VToken.sol";

10: import "../ComptrollerInterface.sol";

11: import "../RiskFund/IRiskFund.sol";

12: import "./IShortfall.sol";

13: import "../Pool/PoolRegistry.sol";

14: import "../Pool/PoolRegistryInterface.sol";

15: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlledV8.sol";

File: VToken.sol

4: import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

7: import "./ComptrollerInterface.sol";

8: import "./VTokenInterfaces.sol";

9: import "./ErrorReporter.sol";

10: import "./InterestRateModel.sol";

11: import "./ExponentialNoError.sol";

12: import "@venusprotocol/governance-contracts/contracts/Governance/AccessControlledV8.sol";

13: import "./RiskFund/IProtocolShareReserve.sol";

File: VTokenInterfaces.sol

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";

5: import "@venusprotocol/oracle/contracts/PriceOracle.sol";

6: import "./ComptrollerInterface.sol";

7: import "./InterestRateModel.sol";

8: import "./ErrorReporter.sol";

File: WhitePaperInterestRateModel.sol

4: import "./InterestRateModel.sol";

Recommended Mitigation Steps:

import {contract1 , contract2} from "filename.sol"; OR Use specific imports syntax per solidity docs recommendation.

[NC-6] LACK OF EVENT EMISSION AFTER CRITICAL INITIALIZE() FUNCTIONS

Description:

To record the init parameters for off-chain monitoring and transparency reasons, please consider emitting an event after the initialize() functions:

Proof Of Concept

File: Comptroller.sol

138:     function initialize(uint256 loopLimit, address accessControlManager) external initializer {

File: Pool/PoolRegistry.sol

164:     function initialize(

File: Rewards/RewardsDistributor.sol

111:     function initialize(

File: RiskFund/ProtocolShareReserve.sol

39:     function initialize(address _protocolIncome, address _riskFund) external initializer {

File: RiskFund/RiskFund.sol

73:     function initialize(

File: Shortfall/Shortfall.sol

131:     function initialize(

File: VToken.sol

59:     function initialize(

[NC-7] LARGE MULTIPLES OF TEN SHOULD USE SCIENTIFIC NOTATION

Description:

OUse (e.g. 1e6) rather than decimal literals (e.g. 100000), for better code readability.

Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits.

Proof Of Concept

File: Shortfall/Shortfall.sol

60:     uint256 private constant MAX_BPS = 10000;

[NC-8] NO SAME VALUE INPUT CONTROL

Description:

INPUT like address oracle should be inputAdd code like this; if (oracle == _oracle revert ADDRESS_SAME();

Proof Of Concept

File: RiskFund/ProtocolShareReserve.sol

45:         protocolIncome = _protocolIncome;

46:         riskFund = _riskFund;

56:         poolRegistry = _poolRegistry;

File: RiskFund/RiskFund.sol

102:         poolRegistry = _poolRegistry;

File: Shortfall/Shortfall.sol

297:         nextBidderBlockLimit = _nextBidderBlockLimit;

311:         incentiveBps = _incentiveBps;

324:         minimumPoolBadDebt = _minimumPoolBadDebt;

337:         waitForFirstBidder = _waitForFirstBidder;

351:         poolRegistry = _poolRegistry;

[NC-9] Omissions in events

Description:

Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters

The events should include the new value and old value where possible:.

Proof Of Concept

File: Comptroller.sol

823:         emit MarketSupported(vToken);

File: Factories/VTokenProxyFactory.sol

47:         emit VTokenProxyDeployed(input);

File: Rewards/RewardsDistributor.sol

149:         emit MarketInitialized(vToken);

450:         emit RewardTokenSupplyIndexUpdated(vToken);

File: VToken.sol

530:         emit SweepToken(address(token));

[NC-10] SOLIDITY COMPILER OPTIMIZATIONS CAN BE PROBLEMATIC

Description:

Protocol has enabled optional compiler optimizations in Solidity. There have been several optimization bugs with security implications. Moreover, optimizations are actively being developed. Solidity compiler optimizations are disabled by default, and it is unclear how many contracts in the wild actually use them.

Therefore, it is unclear how well they are being tested and exercised. High-severity security issues due to optimization bugs have occurred in the past. A high-severity bug in the emscripten-generated solc-js compiler used by Truffle and Remix persisted until late 2018. The fix for this bug was not reported in the Solidity CHANGELOG.

Another high-severity optimization bug resulting in incorrect bit shift results was patched in Solidity 0.5.6. More recently, another bug due to the incorrect caching of keccak256 was reported. A compiler audit of Solidity from November 2018 concluded that the optional optimizations may not be safe. It is likely that there are latent bugs related to optimization and that new bugs will be introduced due to future optimizations.

Exploit Scenario:

A latent or future bug in Solidity compiler optimizations—or in the Emscripten transpilation to solc-js—causes a security vulnerability in the contracts.

Proof Of Concept

File: hardhat.config.ts

  solidity: {
    compilers: [
      {
        version: "0.8.13",
        settings: {
          optimizer: {
            enabled: true,
            details: {
              yul: !process.env.CI,
            },
          },
          outputSelection: {
            "*": {
              "*": ["storageLayout"],
            },
          },
        },
      },
      {
        version: "0.6.6",
        settings: {
          optimizer: {
            enabled: true,
            details: {
              yul: !process.env.CI,
            },
          },
          outputSelection: {
            "*": {
              "*": ["storageLayout"],
            },
          },
        },
      },
    ],
  },

Recommended Mitigation Steps

Short term, measure the gas savings from optimizations and carefully weigh them against the possibility of an optimization-related bug.

Long term, monitor the development and adoption of Solidity compiler optimizations to assess their maturity.

[NC-11] FUNCTION WRITING THAT DOES NOT COMPLY WITH THE SOLIDITY STYLE GUIDE

Context:

All Contracts

Description:

Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.

https://docs.soliditylang.org/en/v0.8.17/style-guide.html

Functions should be grouped according to their visibility and ordered: constructor, receive function (if exists), fallback function (if exists), external, public, internal, private, within a grouping, place the view and pure functions last

[NC-12] USE A MORE RECENT VERSION OF SOLIDITY

Description:

For security, it is best practice to use the latest Solidity version. For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md

Proof Of Concept

File: BaseJumpRateModelV2.sol

2: pragma solidity 0.8.13;

File: Comptroller.sol

2: pragma solidity 0.8.13;

File: ComptrollerInterface.sol

2: pragma solidity 0.8.13;

File: ComptrollerStorage.sol

2: pragma solidity 0.8.13;

File: ErrorReporter.sol

2: pragma solidity 0.8.13;

File: ExponentialNoError.sol

2: pragma solidity 0.8.13;

File: Factories/JumpRateModelFactory.sol

2: pragma solidity 0.8.13;

File: Factories/VTokenProxyFactory.sol

2: pragma solidity 0.8.13;

File: Factories/WhitePaperInterestRateModelFactory.sol

2: pragma solidity 0.8.13;

File: IPancakeswapV2Router.sol

2: pragma solidity 0.8.13;

File: InterestRateModel.sol

2: pragma solidity 0.8.13;

File: JumpRateModelV2.sol

2: pragma solidity 0.8.13;

File: Lens/PoolLens.sol

2: pragma solidity 0.8.13;

File: MaxLoopsLimitHelper.sol

2: pragma solidity 0.8.13;

File: Pool/PoolRegistry.sol

2: pragma solidity 0.8.13;

File: Pool/PoolRegistryInterface.sol

2: pragma solidity 0.8.13;

File: Proxy/UpgradeableBeacon.sol

2: pragma solidity 0.8.13;

File: Rewards/RewardsDistributor.sol

2: pragma solidity 0.8.13;

File: RiskFund/IProtocolShareReserve.sol

2: pragma solidity 0.8.13;

File: RiskFund/IRiskFund.sol

2: pragma solidity 0.8.13;

File: RiskFund/ProtocolShareReserve.sol

2: pragma solidity 0.8.13;

File: RiskFund/ReserveHelpers.sol

2: pragma solidity 0.8.13;

File: RiskFund/RiskFund.sol

2: pragma solidity 0.8.13;

File: Shortfall/IShortfall.sol

2: pragma solidity 0.8.13;

File: Shortfall/Shortfall.sol

2: pragma solidity 0.8.13;

File: VToken.sol

2: pragma solidity 0.8.13;

File: VTokenInterfaces.sol

2: pragma solidity 0.8.13;

File: WhitePaperInterestRateModel.sol

2: pragma solidity 0.8.13;

[NC-13] USE UNDERSCORES FOR NUMBER LITERALS

Proof Of Concept

File: BaseJumpRateModelV2.sol

23:     uint256 public constant blocksPerYear = 10512000;

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

File: Shortfall/Shortfall.sol

60:     uint256 private constant MAX_BPS = 10000;

149:         incentiveBps = 1000;

File: WhitePaperInterestRateModel.sol

17:     uint256 public constant blocksPerYear = 2102400;

[NC-14] Event is missing indexed fields

Description:

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.Parameters of certain events are expected to be indexed (e.g. ERC20 Transfer/Approval events) so that they’re included in the block’s bloom filter for faster access. Failure to do so might confuse off-chain tooling looking for such indexed events.

https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-oarameters

Proof Of Concept

File: BaseJumpRateModelV2.sol

45:     event NewInterestParams(

File: Comptroller.sol

30:     event MarketEntered(VToken vToken, address account);

33:     event MarketExited(VToken vToken, address account);

36:     event NewCloseFactor(uint256 oldCloseFactorMantissa, uint256 newCloseFactorMantissa);

39:     event NewCollateralFactor(VToken vToken, uint256 oldCollateralFactorMantissa, uint256 newCollateralFactorMantissa);

42:     event NewLiquidationThreshold(

49:     event NewLiquidationIncentive(uint256 oldLiquidationIncentiveMantissa, uint256 newLiquidationIncentiveMantissa);

52:     event NewPriceOracle(PriceOracle oldPriceOracle, PriceOracle newPriceOracle);

55:     event ActionPausedMarket(VToken vToken, Action action, bool pauseState);

58:     event NewBorrowCap(VToken indexed vToken, uint256 newBorrowCap);

61:     event NewMinLiquidatableCollateral(uint256 oldMinLiquidatableCollateral, uint256 newMinLiquidatableCollateral);

64:     event NewSupplyCap(VToken indexed vToken, uint256 newSupplyCap);

70:     event MarketSupported(VToken vToken);

File: Factories/VTokenProxyFactory.sol

26:     event VTokenProxyDeployed(VTokenArgs args);

File: MaxLoopsLimitHelper.sol

16:     event MaxLoopsLimitUpdated(uint256 oldMaxLoopsLimit, uint256 newmaxLoopsLimit);

File: Pool/PoolRegistry.sol

113:     event PoolRegistered(address indexed comptroller, VenusPool pool);

118:     event PoolNameSet(address indexed comptroller, string oldName, string newName);

123:     event PoolMetadataUpdated(

132:     event MarketAdded(address indexed comptroller, address vTokenAddress);

File: Rewards/RewardsDistributor.sol

57:     event DistributedSupplierRewardToken(

66:     event DistributedBorrowerRewardToken(

75:     event RewardTokenSupplySpeedUpdated(VToken indexed vToken, uint256 newSpeed);

78:     event RewardTokenBorrowSpeedUpdated(VToken indexed vToken, uint256 newSpeed);

81:     event RewardTokenGranted(address recipient, uint256 amount);

84:     event ContributorRewardTokenSpeedUpdated(address indexed contributor, uint256 newSpeed);

87:     event MarketInitialized(address vToken);

90:     event RewardTokenSupplyIndexUpdated(address vToken);

93:     event RewardTokenBorrowIndexUpdated(address vToken, Exp marketBorrowIndex);

96:     event ContributorRewardsUpdated(address contributor, uint256 rewardAccrued);

File: RiskFund/ProtocolShareReserve.sol

22:     event FundsReleased(address comptroller, address asset, uint256 amount);

File: RiskFund/ReserveHelpers.sol

30:     event AssetsReservesUpdated(address indexed comptroller, address indexed asset, uint256 amount);

File: RiskFund/RiskFund.sol

47:     event AmountOutMinUpdated(uint256 oldAmountOutMin, uint256 newAmountOutMin);

50:     event MinAmountToConvertUpdated(uint256 oldMinAmountToConvert, uint256 newMinAmountToConvert);

53:     event SwappedPoolsAssets(address[] markets, uint256[] amountsOutMin, uint256 totalAmount);

56:     event TransferredReserveForAuction(address comptroller, uint256 amount);

File: Shortfall/Shortfall.sol

75:     event AuctionStarted(

86:     event BidPlaced(address indexed comptroller, uint256 auctionStartBlock, uint256 bidBps, address indexed bidder);

89:     event AuctionClosed(

100:     event AuctionRestarted(address indexed comptroller, uint256 auctionStartBlock);

106:     event MinimumPoolBadDebtUpdated(uint256 oldMinimumPoolBadDebt, uint256 newMinimumPoolBadDebt);

109:     event WaitForFirstBidderUpdated(uint256 oldWaitForFirstBidder, uint256 newWaitForFirstBidder);

112:     event NextBidderBlockLimitUpdated(uint256 oldNextBidderBlockLimit, uint256 newNextBidderBlockLimit);

115:     event IncentiveBpsUpdated(uint256 oldIncentiveBps, uint256 newIncentiveBps);

File: VTokenInterfaces.sol

149:     event AccrueInterest(uint256 cashPrior, uint256 interestAccumulated, uint256 borrowIndex, uint256 totalBorrows);

154:     event Mint(address indexed minter, uint256 mintAmount, uint256 mintTokens, uint256 accountBalance);

159:     event Redeem(address indexed redeemer, uint256 redeemAmount, uint256 redeemTokens, uint256 accountBalance);

164:     event Borrow(address indexed borrower, uint256 borrowAmount, uint256 accountBorrows, uint256 totalBorrows);

169:     event RepayBorrow(

184:     event BadDebtIncreased(address indexed borrower, uint256 badDebtDelta, uint256 badDebtOld, uint256 badDebtNew);

191:     event BadDebtRecovered(uint256 badDebtOld, uint256 badDebtNew);

232:     event NewProtocolSeizeShare(uint256 oldProtocolSeizeShareMantissa, uint256 newProtocolSeizeShareMantissa);

237:     event NewReserveFactor(uint256 oldReserveFactorMantissa, uint256 newReserveFactorMantissa);

242:     event ReservesAdded(address indexed benefactor, uint256 addAmount, uint256 newTotalReserves);

247:     event ReservesReduced(address indexed admin, uint256 reduceAmount, uint256 newTotalReserves);

252:     event Transfer(address indexed from, address indexed to, uint256 amount);

257:     event Approval(address indexed owner, address indexed spender, uint256 amount);

262:     event HealBorrow(address payer, address borrower, uint256 repayAmount);

267:     event SweepToken(address token);

File: WhitePaperInterestRateModel.sol

29:     event NewInterestParams(uint256 baseRatePerBlock, uint256 multiplierPerBlock);

[NC-15] Functions not used internally could be marked external

Proof Of Concept

File: Comptroller.sol

1144:     function getRewardDistributors() public view returns (RewardsDistributor[] memory) {

File: RiskFund/RiskFund.sol

214:     function updateAssetsState(address comptroller, address asset) public override(IRiskFund, ReserveHelpers) {

File: VToken.sol

625:     function increaseAllowance(address spender, uint256 addedValue) public returns (bool) {

File: WhitePaperInterestRateModel.sol

67:     function getSupplyRate(

Low Issues

[L-1] Avoid transfer()/send() as reentrancy mitigations

Description:

Although transfer() and send() have been recommended as a security best-practice to prevent reentrancy attacks because they only forward 2300 gas, the gas repricing of opcodes may break deployed contracts. Use call() instead, without hardcoded gas limits along with checks-effects-interactions pattern or reentrancy guards for reentrancy protection.

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

https://swcregistry.io/docs/SWC-134

Proof Of Concept

File: VToken.sol

99:     function transfer(address dst, uint256 amount) external override nonReentrant returns (bool) {

File: VTokenInterfaces.sol

311:     function transfer(address dst, uint256 amount) external virtual returns (bool);

[L-2] CRITICAL CHANGES SHOULD USE TWO-STEP PROCEDURE

Description:

The critical procedures should be two step process.

Proof Of Concept

File: Pool/PoolRegistry.sol

188:     function setProtocolShareReserve(address payable protocolShareReserve_) external onlyOwner {

430:     function _setProtocolShareReserve(address payable protocolShareReserve_) internal {

File: Rewards/RewardsDistributor.sol

197:     function setRewardTokenSpeeds(

219:     function setContributorRewardTokenSpeed(address contributor, uint256 rewardTokenSpeed) external onlyOwner {

File: RiskFund/ProtocolShareReserve.sol

53:     function setPoolRegistry(address _poolRegistry) external onlyOwner {

File: RiskFund/RiskFund.sol

99:     function setPoolRegistry(address _poolRegistry) external onlyOwner {

126:     function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner {

File: VToken.sol

505:     function setProtocolShareReserve(address payable protocolShareReserve_) external onlyOwner {

515:     function setShortfallContract(address shortfall_) external onlyOwner {

1398:     function _setShortfallContract(address shortfall_) internal {

1407:     function _setProtocolShareReserve(address payable protocolShareReserve_) internal {

Recommended Mitigation Steps:

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

[L-3] DO NOT USE DEPRECATED LIBRARY FUNCTIONS

Description:

You use safeApprove of openZeppelin although it’s deprecated. (see here.

You should change it to increase/decrease Allowance as OpenZeppilin says.

Proof Of Concept

File: Pool/PoolRegistry.sol

322:         token.safeApprove(address(vToken), 0);

323:         token.safeApprove(address(vToken), amountToSupply);

File: RiskFund/RiskFund.sol

258:                     IERC20Upgradeable(underlyingAsset).safeApprove(pancakeSwapRouter, 0);

259:                     IERC20Upgradeable(underlyingAsset).safeApprove(pancakeSwapRouter, balanceOfUnderlyingAsset);

[L-4] DON’T USE OWNER AND TIMELOCK

Description:

Using a timelock contract gives confidence to the user, but when check onlyByOwnGov allow the owner and the timelock The owner manipulates the contract without a lock time period.

Proof Of Concept

File: VToken.sol

525:         require(msg.sender == owner(), "VToken::sweepToken: only admin can sweep tokens");

[L-5] Initializers could be front-run

Description:

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

Proof Of Concept

File: Comptroller.sol

138:     function initialize(uint256 loopLimit, address accessControlManager) external initializer {

138:     function initialize(uint256 loopLimit, address accessControlManager) external initializer {

139:         __Ownable2Step_init();

File: Pool/PoolRegistry.sol

164:     function initialize(

171:     ) external initializer {

172:         __Ownable2Step_init();

File: Rewards/RewardsDistributor.sol

111:     function initialize(

116:     ) external initializer {

119:         __Ownable2Step_init();

File: RiskFund/ProtocolShareReserve.sol

39:     function initialize(address _protocolIncome, address _riskFund) external initializer {

39:     function initialize(address _protocolIncome, address _riskFund) external initializer {

43:         __Ownable2Step_init();

File: RiskFund/RiskFund.sol

73:     function initialize(

79:     ) external initializer {

85:         __Ownable2Step_init();

File: Shortfall/Shortfall.sol

131:     function initialize(

136:     ) external initializer {

141:         __Ownable2Step_init();

143:         __ReentrancyGuard_init();

File: VToken.sol

59:     function initialize(

71:     ) external initializer {

75:         _initialize(

1350:     function _initialize(

1363:         __Ownable2Step_init();

[L-6] INITIALIZE() FUNCTION CAN BE CALLED BY ANYBODY

Description:

initialize() function can be called anybody when the contract is not initialized.

More importantly, if someone else runs this function, they will have full authority because of the __Ownable_init() function. Also, there is no 0 address check in the address arguments of the initialize() function, which must be defined.

Proof Of Concept

File: Comptroller.sol

138:     function initialize(uint256 loopLimit, address accessControlManager) external initializer {

File: Pool/PoolRegistry.sol

171:     ) external initializer {

File: Rewards/RewardsDistributor.sol

116:     ) external initializer {

File: RiskFund/ProtocolShareReserve.sol

39:     function initialize(address _protocolIncome, address _riskFund) external initializer {

File: RiskFund/RiskFund.sol

79:     ) external initializer {

File: Shortfall/Shortfall.sol

136:     ) external initializer {

File: VToken.sol

71:     ) external initializer {

[L-7] MISSING CALLS TO __REENTRANCYGUARD_INIT FUNCTIONS OF INHERITED CONTRACTS

Description:

Most contracts use the delegateCall proxy pattern and hence their implementations require the use of initialize() functions instead of constructors. This requires derived contracts to call the corresponding init functions of their inherited base contracts. This is done in most places except a few.

Impact: The inherited base classes do not get initialized which may lead to undefined behavior.

Missing call to __ReentrancyGuard_init

Proof Of Concept

File: Comptroller.sol

138:     function initialize(uint256 loopLimit, address accessControlManager) external initializer {

File: Pool/PoolRegistry.sol

164:     function initialize(

File: Rewards/RewardsDistributor.sol

111:     function initialize(

File: RiskFund/ProtocolShareReserve.sol

39:     function initialize(address _protocolIncome, address _riskFund) external initializer {

File: RiskFund/RiskFund.sol

73:     function initialize(

File: Shortfall/Shortfall.sol

131:     function initialize(

File: VToken.sol

59:     function initialize(

75:         _initialize(

1350:     function _initialize(

Recommended Mitigation Steps:

Add missing calls to init functions of inherited contracts.

[L-8] UNIFY RETURN CRITERIA

Description:

In files sometimes the name of the return variable is not defined and sometimes is, unifying the way of writing the code makes the code more uniform and readable.

Proof Of Concept

File: Comptroller.sol

154:     function enterMarkets(address[] memory vTokens) external override returns (uint256[] memory) {

187:     function exitMarket(address vTokenAddress) external override returns (uint256) {

988:         returns (

1017:         returns (

1038:     function getAllMarkets() external view override returns (VToken[] memory) {

1047:     function isMarketListed(VToken vToken) external view returns (bool) {

1058:     function getAssetsIn(address account) external view returns (VToken[] memory) {

1070:     function checkMembership(address account, VToken vToken) external view returns (bool) {

1088:     ) external view override returns (uint256 error, uint256 tokensToSeize) {

1119:     function getRewardsByMarket(address vToken) external view returns (RewardSpeeds[] memory rewardSpeeds) {

1136:     function isComptroller() external pure override returns (bool) {

1144:     function getRewardDistributors() public view returns (RewardsDistributor[] memory) {

1154:     function actionPaused(address market, Action action) public view returns (bool) {

1164:     function isDeprecated(VToken vToken) public view returns (bool) {

1276:     function _getCurrentLiquiditySnapshot(address account, function(VToken) internal view returns (Exp memory) weight)

1279:         returns (AccountLiquiditySnapshot memory snapshot)

1291:          liquidation threshold. Accepts the address of the VToken and returns the weight

1301:         function(VToken) internal view returns (Exp memory) weight

1302:     ) internal view returns (AccountLiquiditySnapshot memory snapshot) {

1368:     function _safeGetUnderlyingPrice(VToken asset) internal view returns (uint256) {

1381:     function _getCollateralFactor(VToken asset) internal view returns (Exp memory) {

1390:     function _getLiquidationThreshold(VToken asset) internal view returns (Exp memory) {

1405:         returns (

File: IPancakeswapV2Router.sol

11:     ) external returns (uint256[] memory amounts);

[L-9] POTENTIAL LOSS OF FUNDS ON TOKENS WITH BIG SUPPLIES

Description:

swap() and mint() both reverts if either 2^112 or 2^128 tokens are sent to the pair. This would result in the funds being stuck and nobody being able to mint or swap. Submitting as low because the cost of attack is extremely high, but it’s good to be aware of it.

Proof Of Concept

File: VToken.sol

180:     function mint(uint256 mintAmount) external override nonReentrant returns (uint256) {

File: VTokenInterfaces.sol

271:     function mint(uint256 mintAmount) external virtual returns (uint256);

[L-10] THE SAFETRANSFER FUNCTION DOES NOT CHECK FOR POTENTIALLY SELF-DESTROYED TOKENS

Description:

If a pair gets created and after a while one of the tokens gets self-destroyed (maybe because of a bug) then safeTransfer would still succeed. It’s probably a good idea to check if the contract still exists by checking the bytecode length.

Proof Of Concept

File: Rewards/RewardsDistributor.sol

419:             rewardToken.safeTransfer(user, amount);

File: RiskFund/ProtocolShareReserve.sol

81:         IERC20Upgradeable(asset).safeTransfer(protocolIncome, protocolIncomeAmount);

82:         IERC20Upgradeable(asset).safeTransfer(riskFund, amount - protocolIncomeAmount);

File: RiskFund/RiskFund.sol

194:         IERC20Upgradeable(convertibleBaseAsset).safeTransfer(shortfall, amount);

File: Shortfall/Shortfall.sol

183:                     erc20.safeTransfer(auction.highestBidder, previousBidAmount);

190:                     erc20.safeTransfer(auction.highestBidder, auction.marketDebt[auction.markets[i]]);

229:                 erc20.safeTransfer(address(auction.markets[i]), bidAmount);

232:                 erc20.safeTransfer(address(auction.markets[i]), auction.marketDebt[auction.markets[i]]);

248:         IERC20Upgradeable(convertibleBaseAsset).safeTransfer(auction.highestBidder, riskFundBidAmount);

File: VToken.sol

528:         token.safeTransfer(owner(), balance);

1286:         token.safeTransfer(to, amount);

[L-11] UNSAFE CAST

Description:

Keep in mind that the version of solidity used, despite being greater than 0.8, does not prevent integer overflows during casting, it only does so in mathematical operations.

It is necessary to safely convert between the different numeric types.

Proof Of Concept

File: ExponentialNoError.sol

70:         return uint32(n);

Recommended Mitigation Steps:

Use a safeCast from Open Zeppelin or increase the type length.

[L-12] USE SAFETRANSFER INSTEAD OF TRANSFER

Description:

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

For example, Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)‘s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert.

Proof Of Concept

Recommended Mitigation Steps:

Consider using safeTransfer/safeTransferFrom or require() consistently.

File: VToken.sol

99:     function transfer(address dst, uint256 amount) external override nonReentrant returns (bool) {

File: VTokenInterfaces.sol

311:     function transfer(address dst, uint256 amount) external virtual returns (bool);

[L-13] USE SAFETRANSFEROWNERSHIP INSTEAD OF TRANSFEROWNERSHIP FUNCTION

Description:

transferOwnership function is used to change Ownership from Owned.sol.

Use a 2 structure transferOwnership which is safer.

safeTransferOwnership, use it is more secure due to 2-stage ownership transfer.

Proof Of Concept

File: Pool/PoolRegistry.sol

245:         comptrollerProxy.transferOwnership(msg.sender);

File: VToken.sol

1395:         _transferOwnership(admin_);

Recommended Mitigation Steps:

Use Ownable2Step.sol

Gas Optimizations

[GAS-1] <x> += <y>/<x> -= <y> costs more gas than <x> = <x> + <y>/<x> = <x> - <y> for state variables

Description:

Using the addition/substraction operator instead of plus-equals/minus-equals saves gas

Proof Of Concept

File: RiskFund/ProtocolShareReserve.sol

74:         assetsReserves[asset] -= amount;

75:         poolsAssetsReserves[comptroller][asset] -= amount;

File: RiskFund/ReserveHelpers.sol

66:             assetsReserves[asset] += balanceDifference;

67:             poolsAssetsReserves[comptroller][asset] += balanceDifference;

File: RiskFund/RiskFund.sol

249:                 assetsReserves[underlyingAsset] -= balanceOfUnderlyingAsset;

250:                 poolsAssetsReserves[comptroller][underlyingAsset] -= balanceOfUnderlyingAsset;

File: VToken.sol

630:         newAllowance += addedValue;

652:             currentAllowance -= subtractedValue;

[GAS-2] Use assembly to check for address(0)

Description:

Saves 6 gas per instance

Proof Of Concept

File: BaseJumpRateModelV2.sol

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

File: Comptroller.sol

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

962:         require(address(newOracle) != address(0), "invalid price oracle address");

File: Pool/PoolRegistry.sol

225:         require(beaconAddress != address(0), "PoolRegistry: Invalid Comptroller beacon address.");

226:         require(priceOracle != address(0), "PoolRegistry: Invalid PriceOracle address.");

257:         require(input.comptroller != address(0), "PoolRegistry: Invalid comptroller address");

258:         require(input.asset != address(0), "PoolRegistry: Invalid asset address");

259:         require(input.beaconAddress != address(0), "PoolRegistry: Invalid beacon address");

260:         require(input.vTokenReceiver != address(0), "PoolRegistry: Invalid vTokenReceiver address");

264:             _vTokens[input.comptroller][input.asset] == address(0),

396:         require(venusPool.creator == address(0), "PoolRegistry: Pool already exists in the directory.");

422:         if (address(shortfall_) == address(0)) {

431:         if (protocolShareReserve_ == address(0)) {

File: Proxy/UpgradeableBeacon.sol

8:         require(implementation_ != address(0), "Invalid implementation address");

File: RiskFund/ProtocolShareReserve.sol

40:         require(_protocolIncome != address(0), "ProtocolShareReserve: Protocol Income address invalid");

41:         require(_riskFund != address(0), "ProtocolShareReserve: Risk Fund address invalid");

54:         require(_poolRegistry != address(0), "ProtocolShareReserve: Pool registry address invalid");

71:         require(asset != address(0), "ProtocolShareReserve: Asset address invalid");

File: RiskFund/ReserveHelpers.sol

40:         require(asset != address(0), "ReserveHelpers: Asset address invalid");

52:         require(asset != address(0), "ReserveHelpers: Asset address invalid");

53:         require(poolRegistry != address(0), "ReserveHelpers: Pool Registry address is not set");

55:             PoolRegistryInterface(poolRegistry).getVTokenForAsset(comptroller, asset) != address(0),

File: RiskFund/RiskFund.sol

80:         require(pancakeSwapRouter_ != address(0), "Risk Fund: Pancake swap address invalid");

81:         require(convertibleBaseAsset_ != address(0), "Risk Fund: Base asset address invalid");

100:         require(_poolRegistry != address(0), "Risk Fund: Pool registry address invalid");

111:         require(shortfallContractAddress_ != address(0), "Risk Fund: Shortfall contract address invalid");

127:         require(pancakeSwapRouter_ != address(0), "Risk Fund: PancakeSwap address invalid");

157:         require(poolRegistry != address(0), "Risk fund: Invalid pool registry.");

File: Shortfall/Shortfall.sol

137:         require(convertibleBaseAsset_ != address(0), "invalid base asset address");

138:         require(address(riskFund_) != address(0), "invalid risk fund address");

166:                 ((auction.highestBidder != address(0) && bidBps > auction.highestBidBps) ||

167:                     (auction.highestBidder == address(0) && bidBps >= auction.startBidBps))) ||

169:                     ((auction.highestBidder != address(0) && bidBps < auction.highestBidBps) ||

170:                         (auction.highestBidder == address(0) && bidBps <= auction.startBidBps))),

180:                 if (auction.highestBidder != address(0)) {

189:                 if (auction.highestBidder != address(0)) {

214:             block.number > auction.highestBidBlock + nextBidderBlockLimit && auction.highestBidder != address(0),

349:         require(_poolRegistry != address(0), "invalid address");

468:         bool noBidder = auction.highestBidder == address(0);

File: VToken.sol

72:         require(admin_ != address(0), "invalid admin address");

134:         require(spender != address(0), "invalid spender address");

196:         require(minter != address(0), "invalid minter address");

626:         require(spender != address(0), "invalid spender address");

646:         require(spender != address(0), "invalid spender address");

1399:         if (shortfall_ == address(0)) {

1408:         if (protocolShareReserve_ == address(0)) {

[GAS-3] array[index] += amount is cheaper than array[index] = array[index] + amount (or related variants)

Description:

When updating a value in an array with arithmetic, using array[index] += amount is cheaper than array[index] = array[index] + amount. This is because you avoid an additonal mload when the array is stored in memory, and an sload when the array is stored in storage. This can be applied for any arithmetic operation including +=, -=,/=,*=,^=,&=, %=, <<=,>>=, and >>>=. This optimization can be particularly significant if the pattern occurs during a loop.

Saves 28 gas for a storage array, 38 for a memory array Source

Proof Of Concept

File: RiskFund/RiskFund.sol

175:             poolReserves[comptroller] = poolReserves[comptroller] + swappedTokens;

193:         poolReserves[comptroller] = poolReserves[comptroller] - amount;

File: VToken.sol

1129:         accountTokens[borrower] = accountTokens[borrower] - seizeTokens;

1130:         accountTokens[liquidator] = accountTokens[liquidator] + liquidatorSeizeTokens;

[GAS-4] Use calldata instead of memory for function arguments that do not get mutated

Description:

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage. Source Source 2

Proof Of Concept

File: Comptroller.sol

154:     function enterMarkets(address[] memory vTokens) external override returns (uint256[] memory) {

File: Factories/VTokenProxyFactory.sol

28:     function deployVTokenProxy(VTokenArgs memory input) external returns (VToken) {

File: Lens/PoolLens.sol

309:     function getPoolDataFromVenusPool(address poolRegistryAddress, PoolRegistry.VenusPool memory venusPool)

388:     function vTokenMetadataAll(VToken[] memory vTokens) public view returns (VTokenMetadata[] memory) {

File: Pool/PoolRegistry.sol

255:     function addMarket(AddMarketInput memory input) external {

343:     function updatePoolMetadata(address comptroller, VenusPoolMetaData memory _metadata) external {

File: Rewards/RewardsDistributor.sol

166:         Exp memory marketBorrowIndex

187:     function updateRewardTokenBorrowIndex(address vToken, Exp memory marketBorrowIndex) external onlyComptroller {

198:         VToken[] memory vTokens,

199:         uint256[] memory supplySpeeds,

200:         uint256[] memory borrowSpeeds

277:     function claimRewardToken(address holder, VToken[] memory vTokens) public {

File: VToken.sol

64:         string memory name_,

65:         string memory symbol_,

69:         RiskManagementInit memory riskManagement,

[GAS-5] Setting the constructor to payable

Description:

Saves ~13 gas per instance

Proof Of Concept

File: BaseJumpRateModelV2.sol

65:     constructor(

File: Comptroller.sol

127:     constructor(address poolRegistry_) {

File: JumpRateModelV2.sol

12:     constructor(

File: Pool/PoolRegistry.sol

150:     constructor() {

File: Proxy/UpgradeableBeacon.sol

7:     constructor(address implementation_) UpgradeableBeacon(implementation_) {

File: Rewards/RewardsDistributor.sol

104:     constructor() {

File: RiskFund/ProtocolShareReserve.sol

28:     constructor() {

File: RiskFund/RiskFund.sol

61:     constructor() {

File: Shortfall/Shortfall.sol

118:     constructor() {

File: VToken.sol

41:     constructor() {

File: WhitePaperInterestRateModel.sol

36:     constructor(uint256 baseRatePerYear, uint256 multiplierPerYear) {

[GAS-6] USE FUNCTION INSTEAD OF MODIFIERS

Proof Of Concept

File: Rewards/RewardsDistributor.sol

98:     modifier onlyComptroller() {

File: VToken.sol

33:     modifier nonReentrant() {

[GAS-7] Don't initialize variables with default value

Description:

If a variable is not set/initialized, the default value is assumed (0, false, 0x0 … depending on the data type). You are simply wasting gas if you directly initialize it with its default value.

Proof Of Concept

File: ComptrollerStorage.sol

103:     uint256 internal constant NO_ERROR = 0;

File: ErrorReporter.sol

5:     uint256 public constant NO_ERROR = 0; // support legacy return codes

[GAS-8] The increment in for loop postcondition can be made unchecked

Description:

This is only relevant if you are using the default solidity checked arithmetic. the for loop postcondition, i.e., i++ involves checked arithmetic, which is not required. This is because the value of i is always strictly less than length <= 2**256 - 1. Therefore, the theoretical maximum value of i to enter the for-loop body is 2**256 - 2. This means that the i++ in the for loop can never overflow. Regardless, the overflow checks are performed by the compiler.

Unfortunately, the Solidity optimizer is not smart enough to detect this and remove the checks.One can manually do this.

Source

Proof Of Concept

File: Pool/PoolRegistry.sol

399:         _numberOfPools++;

[GAS-9] USING SOLIDITY VERSION 0.8.19 WILL PROVIDE AN OVERALL GAS OPTIMIZATION

Proof Of Concept

File: BaseJumpRateModelV2.sol

2: pragma solidity 0.8.13;

File: Comptroller.sol

2: pragma solidity 0.8.13;

File: ComptrollerInterface.sol

2: pragma solidity 0.8.13;

File: ComptrollerStorage.sol

2: pragma solidity 0.8.13;

File: ErrorReporter.sol

2: pragma solidity 0.8.13;

File: ExponentialNoError.sol

2: pragma solidity 0.8.13;

File: Factories/JumpRateModelFactory.sol

2: pragma solidity 0.8.13;

File: Factories/VTokenProxyFactory.sol

2: pragma solidity 0.8.13;

File: Factories/WhitePaperInterestRateModelFactory.sol

2: pragma solidity 0.8.13;

File: IPancakeswapV2Router.sol

2: pragma solidity 0.8.13;

File: InterestRateModel.sol

2: pragma solidity 0.8.13;

File: JumpRateModelV2.sol

2: pragma solidity 0.8.13;

File: Lens/PoolLens.sol

2: pragma solidity 0.8.13;

File: MaxLoopsLimitHelper.sol

2: pragma solidity 0.8.13;

File: Pool/PoolRegistry.sol

2: pragma solidity 0.8.13;

File: Pool/PoolRegistryInterface.sol

2: pragma solidity 0.8.13;

File: Proxy/UpgradeableBeacon.sol

2: pragma solidity 0.8.13;

File: Rewards/RewardsDistributor.sol

2: pragma solidity 0.8.13;

File: RiskFund/IProtocolShareReserve.sol

2: pragma solidity 0.8.13;

File: RiskFund/IRiskFund.sol

2: pragma solidity 0.8.13;

File: RiskFund/ProtocolShareReserve.sol

2: pragma solidity 0.8.13;

File: RiskFund/ReserveHelpers.sol

2: pragma solidity 0.8.13;

File: RiskFund/RiskFund.sol

2: pragma solidity 0.8.13;

File: Shortfall/IShortfall.sol

2: pragma solidity 0.8.13;

File: Shortfall/Shortfall.sol

2: pragma solidity 0.8.13;

File: VToken.sol

2: pragma solidity 0.8.13;

File: VTokenInterfaces.sol

2: pragma solidity 0.8.13;

File: WhitePaperInterestRateModel.sol

2: pragma solidity 0.8.13;

[GAS-10] Use shift Right/Left instead of division/multiplication if possible

Proof Of Concept

File: ExponentialNoError.sol

22:     uint256 internal constant halfExpScale = expScale / 2;

[GAS-11] State variables only set in the constructor should be declared immutable

Description:

While strings are not value types, and therefore cannot be immutable/constant if not hard-coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for the string accessors, and having a child contract override the functions with the hard-coded implementation-specific values.

Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD. Variables only set in the constructor and never edited afterwards should be marked as immutable, as it would avoid the expensive storage-writing operation in the constructor (around 20 000 gas per variable) and replace the expensive storage-reading operations (around 2100 gas per reading) to a less expensive value reading (3 gas)

Proof Of Concept

File: BaseJumpRateModelV2.sol

65:     constructor(

[GAS-12] TERNARY OPERATION IS CHEAPER THAN IF-ELSE STATEMENT

Description:

There are instances where a ternary operation can be used instead of if-else statement. In these cases, using ternary operation will save modest amounts of gas. Note that this optimization seems to be dependent on usage of a more recent Solidity version. The following gas savings are on version 0.8.17.

Proof Of Concept

File: VToken.sol

1313         if (spender == src) {
1314             startingAllowance = type(uint256).max;
1315         } else {
1316             startingAllowance = transferAllowances[src][spender];
1317:        }

[GAS-13] Usage of uint/int smaller than 32 bytes (256 bits) incurs overhead

Description:

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html

Use a larger size then downcast where needed.

Proof Of Concept

File: Factories/VTokenProxyFactory.sol

18:         uint8 decimals_;

File: Lens/PoolLens.sol

99:         uint32 block;

File: Pool/PoolRegistry.sol

36:         uint8 decimals;

File: Rewards/RewardsDistributor.sol

19:         uint32 block;

126:         uint32 blockNumber = safe32(getBlockNumber(), "block number exceeds 32 bits");

433:         uint32 blockNumber = safe32(getBlockNumber(), "block number exceeds 32 bits");

461:         uint32 blockNumber = safe32(getBlockNumber(), "block number exceeds 32 bits");

File: VToken.sol

66:         uint8 decimals_,

1357:         uint8 decimals_,

File: VTokenInterfaces.sol

45:     uint8 public decimals;

[GAS-14] Use != 0 instead of > 0 for unsigned integer comparison

Description:

To update this with using at least 0.8.6 there is no difference in gas usage with!= 0 or > 0. Source

Proof Of Concept

File: Comptroller.sol

367:         if (snapshot.shortfall > 0) {

1262:         if (snapshot.shortfall > 0) {

File: Lens/PoolLens.sol

462:         if (deltaBlocks > 0 && borrowSpeed > 0) {

462:         if (deltaBlocks > 0 && borrowSpeed > 0) {

466:             Double memory ratio = borrowAmount > 0 ? fraction(tokensAccrued, borrowAmount) : Double({ mantissa: 0 });

470:         } else if (deltaBlocks > 0) {

483:         if (deltaBlocks > 0 && supplySpeed > 0) {

483:         if (deltaBlocks > 0 && supplySpeed > 0) {

486:             Double memory ratio = supplyTokens > 0 ? fraction(tokensAccrued, supplyTokens) : Double({ mantissa: 0 });

490:         } else if (deltaBlocks > 0) {

506:         if (borrowerIndex.mantissa == 0 && borrowIndex.mantissa > 0) {

526:         if (supplierIndex.mantissa == 0 && supplyIndex.mantissa > 0) {

File: Rewards/RewardsDistributor.sol

261:         if (deltaBlocks > 0 && rewardTokenSpeed > 0) {

261:         if (deltaBlocks > 0 && rewardTokenSpeed > 0) {

418:         if (amount > 0 && amount <= rewardTokenRemaining) {

435:         if (deltaBlocks > 0 && supplySpeed > 0) {

435:         if (deltaBlocks > 0 && supplySpeed > 0) {

438:             Double memory ratio = supplyTokens > 0

446:         } else if (deltaBlocks > 0) {

463:         if (deltaBlocks > 0 && borrowSpeed > 0) {

463:         if (deltaBlocks > 0 && borrowSpeed > 0) {

466:             Double memory ratio = borrowAmount > 0

474:         } else if (deltaBlocks > 0) {

File: RiskFund/RiskFund.sol

82:         require(minAmountToConvert_ > 0, "Risk Fund: Invalid min amount to convert");

83:         require(loopsLimit_ > 0, "Risk Fund: Loops limit can not be zero");

139:         require(minAmountToConvert_ > 0, "Risk Fund: Invalid min amount to convert");

244:         if (balanceOfUnderlyingAsset > 0) {

File: VToken.sol

818:         if (redeemTokensIn > 0) {

837:         if (redeemTokens == 0 && redeemAmount > 0) {

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

[GAS-15] USE BYTES32 INSTEAD OF STRING

Description:

Use bytes32 instead of string to save gas whenever possible. String is a dynamic data structure and therefore is more gas consuming then bytes32.

Proof Of Concept

File: BaseJumpRateModelV2.sol

55:     error Unauthorized(address sender, address calledContract, string methodSignature);

94:         string memory signature = "updateJumpRateModel(uint256,uint256,uint256,uint256)";

File: ExponentialNoError.sol

63:     function safe224(uint256 n, string memory errorMessage) internal pure returns (uint224) {

68:     function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) {

File: Factories/VTokenProxyFactory.sol

16:         string name_;

17:         string symbol_;

File: Lens/PoolLens.sol

17:         string name;

22:         string category;

23:         string logoURL;

24:         string description;

File: Pool/PoolRegistry.sol

37:         string name;

38:         string symbol;

118:     event PoolNameSet(address indexed comptroller, string oldName, string newName);

118:     event PoolNameSet(address indexed comptroller, string oldName, string newName);

214:         string calldata name,

223:         _checkAccessAllowed("createRegistryPool(string,address,uint256,uint256,uint256,address,uint256,address)");

332:     function setPoolName(address comptroller, string calldata name) external {

333:         _checkAccessAllowed("setPoolName(address,string)");

335:         string memory oldName = _poolByComptroller[comptroller].name;

393:     function _registerPool(string calldata name, address comptroller) internal returns (uint256) {

439:     function _ensureValidName(string calldata name) internal pure {

File: Pool/PoolRegistryInterface.sol

9:         string name;

20:         string category;

21:         string logoURL;

22:         string description;

File: VToken.sol

64:         string memory name_,

65:         string memory symbol_,

1355:         string memory name_,

1356:         string memory symbol_,

File: VTokenInterfaces.sol

35:     string public name;

40:     string public symbol;

[GAS-16] USING > 0 COSTS MORE GAS THAN != 0 WHEN USED ON A UINT IN A REQUIRE() STATEMENT

Description:

When dealing with unsigned integer types, comparisons with != 0 are cheaper then with > 0. This change saves 6 gas per instance

Proof Of Concept

File: RiskFund/RiskFund.sol

82:         require(minAmountToConvert_ > 0, "Risk Fund: Invalid min amount to convert");

83:         require(loopsLimit_ > 0, "Risk Fund: Loops limit can not be zero");

139:         require(minAmountToConvert_ > 0, "Risk Fund: Invalid min amount to convert");

File: VToken.sol

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