Skip to content

Instantly share code, notes, and snippets.

@CloudEllie
Created May 9, 2023 14:22
Show Gist options
  • Save CloudEllie/6639dbfd7dc1809a3baa28bb2895e1d9 to your computer and use it in GitHub Desktop.
Save CloudEllie/6639dbfd7dc1809a3baa28bb2895e1d9 to your computer and use it in GitHub Desktop.

Winning bot race submission

This is the top-ranked automated findings report, from warden IllIllI's IllIllI-bot. All findings in this report will be considered known issues for the purposes of your C4 audit.

Summary

Medium Risk Issues

Issue Instances
[M‑01] The owner is a single point of failure and a centralization risk 16

Total: 16 instances over 1 issues

Low Risk Issues

Issue Instances
[L‑01] Division by zero not prevented 1
[L‑02] Loss of precision 14
[L‑03] require() should be used instead of assert() 1
[L‑04] safeApprove() is deprecated 4
[L‑05] Missing checks for address(0x0) when assigning values to address state variables 1
[L‑06] Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions 8
[L‑07] External calls in an un-bounded for-loop may result in a DOS 4

Total: 33 instances over 7 issues

Non-critical Issues

Issue Instances
[N‑01] Consider disabling renounceOwnership() 7
[N‑02] Events are missing sender information 12
[N‑03] Variables need not be initialized to zero 2
[N‑04] Imports could be organized more systematically 4
[N‑05] Large numeric literals should use underscores for readability 2
[N‑06] Constants in comparisons should appear on the left side 45
[N‑07] Variable names don't follow the Solidity style guide 17
[N‑08] else-block not required 2
[N‑09] Events may be emitted out of order due to reentrancy 5
[N‑10] if-statement can be converted to a ternary 1
[N‑11] Consider using named mappings 33
[N‑12] Import declarations should import specific identifiers, rather than the whole file 96
[N‑13] Adding a return statement when the function defines a named return variable, is redundant 2
[N‑14] public functions not called by the contract should be declared external instead 2
[N‑15] 2**<n> - 1 should be re-written as type(uint<n>).max 2
[N‑16] constants should be defined rather than using magic numbers 11
[N‑17] Events that mark critical parameter changes should contain both the old and the new value 5
[N‑18] Constant redefined elsewhere 1
[N‑19] Inconsistent spacing in comments 3
[N‑20] Lines are too long 22
[N‑21] Typos 10
[N‑22] File is missing NatSpec 10
[N‑23] NatSpec @param is missing 47
[N‑24] NatSpec @return argument is missing 27
[N‑25] Event is not properly indexed 12
[N‑26] Not using the named return variables anywhere in the function is confusing 14
[N‑27] Duplicated require()/revert() checks should be refactored to a modifier or function 5
[N‑28] Interfaces should be indicated with an I prefix in the contract name 3
[N‑29] Numeric values having to do with time should use time units for readability 2
[N‑30] Consider using delete rather than assigning zero/false to clear values 8
[N‑31] Contracts should have full test coverage 1
[N‑32] Large or complicated code bases should implement invariant tests 1
[N‑33] internal functions not called by the contract should be removed 2

Total: 416 instances over 33 issues

Gas Optimizations

Issue Instances Total Gas Saved
[G‑01] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate 3 -
[G‑02] Structs can be packed into fewer storage slots 3 -
[G‑03] Using storage instead of memory for structs/arrays saves gas 6 25200
[G‑04] State variables should be cached in stack variables rather than re-reading them from storage 22 2134
[G‑05] Multiple accesses of a mapping/array should use a local variable cache 13 546
[G‑06] The result of function calls should be cached rather than re-calling the function 1 -
[G‑07] internal functions only called once can be inlined to save gas 19 380
[G‑08] Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement 3 255
[G‑09] <array>.length should not be looked up in every loop of a for-loop 3 9
[G‑10] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops 37 2220
[G‑11] require()/revert() strings longer than 32 bytes cost extra gas 61 -
[G‑12] Optimize names to save gas 16 352
[G‑13] Using bools for storage incurs overhead 3 51300
[G‑14] >= costs less gas than > 1 3
[G‑15] internal functions not called by the contract should be removed to save deployment gas 2 -
[G‑16] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 1 5
[G‑17] Splitting require() statements that use && saves gas 4 12
[G‑18] Using private rather than public for constants, saves gas 8 -
[G‑19] Division by two should use bit shifting 1 20
[G‑20] Stack variable used as a cheaper cache for a state variable is only used once 10 30
[G‑21] require() or revert() statements that check input arguments should be at the top of the function 12 -
[G‑22] Use custom errors rather than revert()/require() strings to save gas 99 -
[G‑23] Functions guaranteed to revert when called by normal users can be marked payable 22 462
[G‑24] Constructors can be marked payable 11 231

Total: 361 instances over 24 issues with 83159 gas saved

Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.

Medium Risk Issues

[M‑01] The owner is a single point of failure and a centralization risk

Having a single EOA as the only owner of contracts is a large centralization risk and a single point of failure. A single private key may be taken in a hack, or the sole holder of the key may become unable to retrieve the key when necessary. Consider changing to a multi-signature setup, or having a role-based authorization model.

There are 16 instances of this issue:

File: contracts/Comptroller.sol

927:     function addRewardsDistributor(RewardsDistributor _rewardsDistributor) external onlyOwner {

961:     function setPriceOracle(PriceOracle newOracle) external onlyOwner {

973:     function setMaxLoopsLimit(uint256 limit) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L927-L927

File: contracts/Pool/PoolRegistry.sol

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

198:     function setShortfallContract(Shortfall shortfall_) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L188-L188

File: contracts/Rewards/RewardsDistributor.sol

181:     function grantRewardToken(address recipient, uint256 amount) external onlyOwner {

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

249:     function setMaxLoopsLimit(uint256 limit) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L181-L181

File: contracts/RiskFund/ProtocolShareReserve.sol

53:      function setPoolRegistry(address _poolRegistry) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L53-L53

File: contracts/RiskFund/RiskFund.sol

99:      function setPoolRegistry(address _poolRegistry) external onlyOwner {

110:     function setShortfallContractAddress(address shortfallContractAddress_) external onlyOwner {

126:     function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner {

205:     function setMaxLoopsLimit(uint256 limit) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L99-L99

File: contracts/Shortfall/Shortfall.sol

348:     function updatePoolRegistry(address _poolRegistry) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L348-L348

File: contracts/VToken.sol

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

515:     function setShortfallContract(address shortfall_) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L505-L505

Low Risk Issues

[L‑01] Division by zero not prevented

The divisions below take an input parameter which does not have any zero-value checks, which may lead to the functions reverting when zero is passed.

There is one instance of this issue:

File: contracts/ExponentialNoError.sol

150:         return a / b;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L150-L150

[L‑02] Loss of precision

Division by large numbers may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator

There are 14 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

141:          return (borrows * BASE) / (cash + borrows - reserves);

157:          baseRatePerBlock = baseRatePerYear / blocksPerYear;

158:          multiplierPerBlock = (multiplierPerYear * BASE) / (blocksPerYear * kink_);

159:          jumpMultiplierPerBlock = jumpMultiplierPerYear / blocksPerYear;

180:              return ((util * multiplierPerBlock) / BASE) + baseRatePerBlock;

182:              uint256 normalRate = ((kink * multiplierPerBlock) / BASE) + baseRatePerBlock;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L141

File: contracts/Shortfall/Shortfall.sol

408                   (MAX_BPS * MAX_BPS * remainingRiskFundBalance) /
409:                  (poolBadDebt * (MAX_BPS + incentiveBps));

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L408-L409

File: contracts/VToken.sol

1478:             uint256 exchangeRate = (cashPlusBorrowsMinusReserves * expScale) / _totalSupply;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L1478

File: contracts/WhitePaperInterestRateModel.sol

37:           baseRatePerBlock = baseRatePerYear / blocksPerYear;

38:           multiplierPerBlock = multiplierPerYear / blocksPerYear;

56:           return ((ur * multiplierPerBlock) / BASE) + baseRatePerBlock;

75:           uint256 rateToPool = (borrowRate * oneMinusReserveFactor) / BASE;

76:           return (utilizationRate(cash, borrows, reserves) * rateToPool) / BASE;

96:           return (borrows * BASE) / (cash + borrows - reserves);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L37

[L‑03] require() should be used instead of assert()

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

There is one instance of this issue:

File: contracts/Comptroller.sol

225:          assert(assetIndex < len);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L225

[L‑04] safeApprove() is deprecated

Deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance(). If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance() can be used instead. The function may currently work, but if a bug is found in this version of OpenZeppelin, and the version that you're forced to upgrade to no longer has this function, you'll encounter unnecessary delays in porting and testing replacement contracts.

There are 4 instances of this issue:

File: contracts/Pool/PoolRegistry.sol

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L322

File: contracts/RiskFund/RiskFund.sol

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L258

[L‑05] Missing checks for address(0x0) when assigning values to address state variables

There is one instance of this issue:

File: contracts/VToken.sol

1390:         underlying = underlying_;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L1390

[L‑06] Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

See this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

There are 8 instances of this issue:

File: contracts/Comptroller.sol

17    contract Comptroller is
18        Ownable2StepUpgradeable,
19        AccessControlledV8,
20        ComptrollerStorage,
21        ComptrollerInterface,
22        ExponentialNoError,
23:       MaxLoopsLimitHelper

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L17-L23

File: contracts/Pool/PoolRegistry.sol

25:   contract PoolRegistry is Ownable2StepUpgradeable, AccessControlledV8, PoolRegistryInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L25

File: contracts/Proxy/UpgradeableBeacon.sol

6:    contract Beacon is UpgradeableBeacon {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Proxy/UpgradeableBeacon.sol#L6

File: contracts/Rewards/RewardsDistributor.sol

12:   contract RewardsDistributor is ExponentialNoError, Ownable2StepUpgradeable, AccessControlledV8, MaxLoopsLimitHelper {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L12

File: contracts/RiskFund/ProtocolShareReserve.sol

12:   contract ProtocolShareReserve is Ownable2StepUpgradeable, ExponentialNoError, ReserveHelpers, IProtocolShareReserve {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L12

File: contracts/RiskFund/RiskFund.sol

19    contract RiskFund is
20        Ownable2StepUpgradeable,
21        AccessControlledV8,
22        ExponentialNoError,
23        ReserveHelpers,
24        MaxLoopsLimitHelper,
25:       IRiskFund

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L19-L25

File: contracts/Shortfall/Shortfall.sol

17:   contract Shortfall is Ownable2StepUpgradeable, AccessControlledV8, ReentrancyGuardUpgradeable, IShortfall {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L17

File: contracts/VToken.sol

19    contract VToken is
20        Ownable2StepUpgradeable,
21        AccessControlledV8,
22        VTokenInterface,
23        ExponentialNoError,
24:       TokenErrorReporter

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L19-L24

[L‑07] External calls in an un-bounded for-loop may result in a DOS

Consider limiting the number of iterations in for-loops that make external calls

There are 4 instances of this issue:

File: contracts/Comptroller.sol

/// @audit borrowIndex()
402:          for (uint256 i; i < rewardDistributorsCount; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L402

File: contracts/Lens/PoolLens.sol

/// @audit badDebt()
263:          for (uint256 i; i < markets.length; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L263

File: contracts/RiskFund/RiskFund.sol

/// @audit getPoolByComptroller()
/// @audit isMarketListed()
166:          for (uint256 i; i < marketsCount; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L166

Non-critical Issues

[N‑01] Consider disabling renounceOwnership()

If the plan for your project does not include eventually giving up all ownership control, consider overwriting OpenZeppelin's Ownable's renounceOwnership() function in order to disable it.

There are 7 instances of this issue:

File: contracts/Comptroller.sol

18:      Ownable2StepUpgradeable,

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L18-L18

File: contracts/Pool/PoolRegistry.sol

25:  contract PoolRegistry is Ownable2StepUpgradeable, AccessControlledV8, PoolRegistryInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L25-L25

File: contracts/Rewards/RewardsDistributor.sol

12:  contract RewardsDistributor is ExponentialNoError, Ownable2StepUpgradeable, AccessControlledV8, MaxLoopsLimitHelper {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L12-L12

File: contracts/RiskFund/ProtocolShareReserve.sol

12:  contract ProtocolShareReserve is Ownable2StepUpgradeable, ExponentialNoError, ReserveHelpers, IProtocolShareReserve {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L12-L12

File: contracts/RiskFund/RiskFund.sol

20:      Ownable2StepUpgradeable,

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L20-L20

File: contracts/Shortfall/Shortfall.sol

17:  contract Shortfall is Ownable2StepUpgradeable, AccessControlledV8, ReentrancyGuardUpgradeable, IShortfall {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L17-L17

File: contracts/VToken.sol

20:      Ownable2StepUpgradeable,

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L20-L20

[N‑02] Events are missing sender information

When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users, especially when msg.sender is not tx.origin.

There are 12 instances of this issue:

File: contracts/Pool/PoolRegistry.sol

326:         emit MarketAdded(address(comptroller), address(vToken));

406:         emit PoolRegistered(comptroller, pool);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L326-L326

File: contracts/RiskFund/RiskFund.sol

196:         emit TransferredReserveForAuction(comptroller, amount);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L196-L196

File: contracts/VToken.sol

138:         emit Approval(src, spender, amount);

408:             emit RepayBorrow(payer, borrower, actualRepayAmount, 0, totalBorrowsNew);

420:             emit RepayBorrow(address(this), borrower, badDebtDelta, accountBorrowsPrev - badDebtDelta, totalBorrowsNew);

421:             emit BadDebtIncreased(borrower, badDebtDelta, badDebtOld, badDebtNew);

428:         emit HealBorrow(payer, borrower, repayAmount);

496:         emit BadDebtRecovered(badDebtOld, badDebtNew);

530:         emit SweepToken(address(token));

633:         emit Approval(src, spender, newAllowance);

657:         emit Approval(src, spender, currentAllowance);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L138-L138

[N‑03] Variables need not be initialized to zero

The default value for variables is zero, so initializing them to zero is superfluous.

There are 2 instances of this issue:

File: contracts/ComptrollerStorage.sol

103:     uint256 internal constant NO_ERROR = 0;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L103-L103

File: contracts/ErrorReporter.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ErrorReporter.sol#L5-L5

[N‑04] Imports could be organized more systematically

The contract's interface should be imported first, followed by each of the interfaces it uses, followed by all other files. The examples below do not follow this layout.

There are 4 instances of this issue:

File: contracts/RiskFund/ProtocolShareReserve.sol

9:   import "./ReserveHelpers.sol";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L9-L9

File: contracts/RiskFund/RiskFund.sol

10:  import "./ReserveHelpers.sol";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L10-L10

File: contracts/Shortfall/Shortfall.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L10-L10

File: contracts/VToken.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L12-L12

[N‑05] Large numeric literals should use underscores for readability

There are 2 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

23:      uint256 public constant blocksPerYear = 10512000;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L23-L23

File: contracts/WhitePaperInterestRateModel.sol

17:      uint256 public constant blocksPerYear = 2102400;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L17-L17

[N‑06] Constants in comparisons should appear on the left side

Doing so will prevent typo bugs

There are 45 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

137:         if (borrows == 0) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L137-L137

File: contracts/Comptroller.sol

194:         if (amountOwed != 0) {

345:         if (oracle.getUnderlyingPrice(vToken) == 0) {

464:         if (snapshot.shortfall == 0) {

595:         if (snapshot.shortfall == 0) {

618:             if (tokens != 0) {

622:             if (borrowBalance != 0) {

661:         if (snapshot.shortfall == 0) {

692:             require(borrowBalance == 0, "Nonzero borrow balance after liquidation");

755:         if (newCollateralFactorMantissa != 0 && oracle.getUnderlyingPrice(address(vToken)) == 0) {

780:         require(newLiquidationIncentiveMantissa >= 1e18, "liquidation incentive should be greater than 1e18");

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

866:         require(vTokensCount != 0, "invalid number of markets");

1166:            markets[address(vToken)].collateralFactorMantissa == 0 &&

1168:            vToken.reserveFactorMantissa() == 1e18;

1370:        if (oraclePriceMantissa == 0) {

1413:        if (err != 0) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L194-L194

File: contracts/Lens/PoolLens.sol

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L506-L506

File: contracts/Pool/PoolRegistry.sol

440:         require(bytes(name).length <= 100, "Pool's name is too large");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L440-L440

File: contracts/Rewards/RewardsDistributor.sol

134:         if (supplyState.index == 0) {

139:         if (borrowState.index == 0) {

183:         require(amountLeft == 0, "insufficient rewardToken for grant");

222:         if (rewardTokenSpeed == 0) {

348:         if (supplierIndex == 0 && supplyIndex >= rewardTokenInitialIndex) {

386:         if (borrowerIndex == 0 && borrowIndex >= rewardTokenInitialIndex) {

399:         if (borrowerAmount != 0) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L134-L134

File: contracts/RiskFund/RiskFund.sol

231:         require(amountOutMin != 0, "RiskFund: amountOutMin must be greater than 0 to swap vToken");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L231-L231

File: contracts/Shortfall/Shortfall.sol

139:         require(minimumPoolBadDebt_ != 0, "invalid minimum pool bad debt");

295:         require(_nextBidderBlockLimit != 0, "_nextBidderBlockLimit must not be 0");

309:         require(_incentiveBps != 0, "incentiveBps must not be 0");

365:             (auction.startBlock == 0 && auction.status == AuctionStatus.NOT_STARTED) ||

459:         return auction.startBlock != 0 && auction.status == AuctionStatus.STARTED;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L139-L139

File: contracts/VToken.sol

403:         if (repayAmount != 0) {

413:         if (badDebtDelta != 0) {

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

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

1050:        if (repayAmount == 0) {

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

1446:        if (borrowSnapshot.principal == 0) {

1465:        if (_totalSupply == 0) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L403-L403

File: contracts/WhitePaperInterestRateModel.sol

92:          if (borrows == 0) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L92-L92

[N‑07] Variable names don't follow the Solidity style guide

For constant variable names, each word should use all capital letters, with underscores separating each word (CONSTANT_CASE)

There are 17 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

23:      uint256 public constant blocksPerYear = 10512000;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L23-L23

File: contracts/ComptrollerStorage.sol

106:     uint256 internal constant closeFactorMinMantissa = 0.05e18; // 0.05

109:     uint256 internal constant closeFactorMaxMantissa = 0.9e18; // 0.9

112:     uint256 internal constant collateralFactorMaxMantissa = 0.9e18; // 0.9

115:     bool internal constant _isComptroller = true;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L106-L106

File: contracts/ExponentialNoError.sol

20:      uint256 internal constant expScale = 1e18;

21:      uint256 internal constant doubleScale = 1e36;

22:      uint256 internal constant halfExpScale = expScale / 2;

23:      uint256 internal constant mantissaOne = expScale;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L20-L20

File: contracts/InterestRateModel.sol

10:      bool public constant isInterestRateModel = true;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/InterestRateModel.sol#L10-L10

File: contracts/Rewards/RewardsDistributor.sol

29:      uint224 public constant rewardTokenInitialIndex = 1e36;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L29-L29

File: contracts/RiskFund/ProtocolShareReserve.sol

18:      uint256 private constant protocolSharePercentage = 70;

19:      uint256 private constant baseUnit = 100;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L18-L18

File: contracts/VTokenInterfaces.sol

53:      uint256 internal constant borrowRateMaxMantissa = 0.0005e16;

56:      uint256 internal constant reserveFactorMaxMantissa = 1e18;

142:     bool public constant isVToken = true;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L53-L53

File: contracts/WhitePaperInterestRateModel.sol

17:      uint256 public constant blocksPerYear = 2102400;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L17-L17

[N‑08] else-block not required

One level of nesting can be removed by not having an else block when the if-block returns:

if (foo) {
  return 1;
} else {
  return 2;
}

becomes

if (foo) return 1
return 2

There are 2 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

179          if (util <= kink) {
180              return ((util * multiplierPerBlock) / BASE) + baseRatePerBlock;
181          } else {
182              uint256 normalRate = ((kink * multiplierPerBlock) / BASE) + baseRatePerBlock;
183              uint256 excessUtil;
184              unchecked {
185                  excessUtil = util - kink;
186              }
187              return ((excessUtil * jumpMultiplierPerBlock) / BASE) + normalRate;
188:         }

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L179-L188

File: contracts/VToken.sol

1465         if (_totalSupply == 0) {
1466             /*
1467              * If there are no tokens minted:
1468              *  exchangeRate = initialExchangeRate
1469              */
1470             return initialExchangeRateMantissa;
1471         } else {
1472             /*
1473              * Otherwise:
1474              *  exchangeRate = (totalCash + totalBorrows + badDebt - totalReserves) / totalSupply
1475              */
1476             uint256 totalCash = _getCashPrior();
1477             uint256 cashPlusBorrowsMinusReserves = totalCash + totalBorrows + badDebt - totalReserves;
1478             uint256 exchangeRate = (cashPlusBorrowsMinusReserves * expScale) / _totalSupply;
1479 
1480             return exchangeRate;
1481:        }

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L1465-L1481

[N‑09] Events may be emitted out of order due to reentrancy

Ensure that events follow the best practice of check-effects-interaction, and are emitted before external calls

There are 5 instances of this issue:

File: contracts/RiskFund/ProtocolShareReserve.sol

/// @audit safeTransfer() prior to emission of FundsReleased()
87:          emit FundsReleased(comptroller, asset, amount);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L87-L87

File: contracts/RiskFund/RiskFund.sol

/// @audit safeTransfer() prior to emission of TransferredReserveForAuction()
196:         emit TransferredReserveForAuction(comptroller, amount);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L196-L196

File: contracts/Shortfall/Shortfall.sol

/// @audit safeTransfer() prior to emission of BidPlaced()
201:         emit BidPlaced(comptroller, auction.startBlock, bidBps, msg.sender);

/// @audit safeTransfer() prior to emission of AuctionClosed()
250          emit AuctionClosed(
251              comptroller,
252              auction.startBlock,
253              auction.highestBidder,
254              auction.highestBidBps,
255              transferredAmount,
256              auction.markets,
257              marketsDebt
258:         );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L201-L201

File: contracts/VToken.sol

/// @audit safeTransfer() prior to emission of SweepToken()
530:         emit SweepToken(address(token));

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L530-L530

[N‑10] if-statement can be converted to a ternary

The code can be made more compact while also increasing readability by converting the following if-statements to ternaries (e.g. foo += (x > y) ? a : b)

There is one instance of this issue:

File: contracts/VToken.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L1313-L1317

[N‑11] Consider using named mappings

Consider moving to solidity version 0.8.18 or later, and using named mappings to make it easier to understand the purpose of each mapping

There are 33 instances of this issue:

File: contracts/ComptrollerStorage.sol

41:          mapping(address => bool) accountMembership;

74:      mapping(address => VToken[]) public accountAssets;

80:      mapping(address => Market) public markets;

86:      mapping(address => uint256) public borrowCaps;

92:      mapping(address => uint256) public supplyCaps;

95:      mapping(address => mapping(Action => bool)) internal _actionPaused;

101:     mapping(address => bool) internal rewardsDistributorExists;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L41-L41

File: contracts/Pool/PoolRegistry.sol

83:      mapping(address => VenusPoolMetaData) public metadata;

88:      mapping(uint256 => address) private _poolsByID;

98:      mapping(address => VenusPool) private _poolByComptroller;

103:     mapping(address => mapping(address => address)) private _vTokens;

108:     mapping(address => address[]) private _supportedPools;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L83-L83

File: contracts/Rewards/RewardsDistributor.sol

23:      mapping(address => RewardToken) public rewardTokenSupplyState;

26:      mapping(address => mapping(address => uint256)) public rewardTokenSupplierIndex;

32:      mapping(address => uint256) public rewardTokenAccrued;

35:      mapping(address => uint256) public rewardTokenBorrowSpeeds;

38:      mapping(address => uint256) public rewardTokenSupplySpeeds;

41:      mapping(address => RewardToken) public rewardTokenBorrowState;

44:      mapping(address => uint256) public rewardTokenContributorSpeeds;

47:      mapping(address => uint256) public lastContributorBlock;

50:      mapping(address => mapping(address => uint256)) public rewardTokenBorrowerIndex;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L23-L23

File: contracts/RiskFund/ReserveHelpers.sol

13:      mapping(address => uint256) internal assetsReserves;

17:      mapping(address => mapping(address => uint256)) internal poolsAssetsReserves;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L13-L13

File: contracts/RiskFund/RiskFund.sol

35:      mapping(address => uint256) public poolReserves;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L35-L35

File: contracts/Shortfall/Shortfall.sol

72:      mapping(address => Auction) public auctions;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L72-L72

File: contracts/VTokenInterfaces.sol

107:     mapping(address => uint256) internal accountTokens;

110:     mapping(address => mapping(address => uint256)) internal transferAllowances;

113:     mapping(address => BorrowSnapshot) internal accountBorrows;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L107-L107

[N‑12] Import declarations should import specific identifiers, rather than the whole file

Using import declarations of the form import {<identifier_name>} from "some/file.sol" avoids polluting the symbol namespace making flattened files smaller, and speeds up compilation

There are 96 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

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

5:    import "./InterestRateModel.sol";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L4

File: contracts/ComptrollerInterface.sol

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

5:    import "./VToken.sol";

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerInterface.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L4

File: contracts/ComptrollerStorage.sol

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

5:    import "./VToken.sol";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L4

File: contracts/Factories/JumpRateModelFactory.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Factories/JumpRateModelFactory.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Factories/VTokenProxyFactory.sol#L4

File: contracts/Factories/WhitePaperInterestRateModelFactory.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Factories/WhitePaperInterestRateModelFactory.sol#L4

File: contracts/JumpRateModelV2.sol

4:    import "./BaseJumpRateModelV2.sol";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/JumpRateModelV2.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L4

File: contracts/Proxy/UpgradeableBeacon.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Proxy/UpgradeableBeacon.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L4

File: contracts/RiskFund/ReserveHelpers.sol

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

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L4

File: contracts/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";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L4

File: contracts/WhitePaperInterestRateModel.sol

4:    import "./InterestRateModel.sol";

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L4

[N‑13] Adding a return statement when the function defines a named return variable, is redundant

There are 2 instances of this issue:

File: contracts/Comptroller.sol

1130:         return rewardSpeeds;

1360:         return snapshot;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1130

[N‑14] 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.

There are 2 instances of this issue:

File: contracts/Comptroller.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1144

File: contracts/VToken.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L625

[N‑15] 2**<n> - 1 should be re-written as type(uint<n>).max

Earlier versions of solidity can use uint<n>(-1) instead. Expressions not including the - 1 can often be re-written to accomodate the change (e.g. by using a > rather than a >=, which will also save some gas)

There are 2 instances of this issue:

File: contracts/ExponentialNoError.sol

64:           require(n < 2**224, errorMessage);

69:           require(n < 2**32, errorMessage);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L64

[N‑16] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 11 instances of this issue:

File: contracts/Comptroller.sol

/// @audit 1e18
780:          require(newLiquidationIncentiveMantissa >= 1e18, "liquidation incentive should be greater than 1e18");

/// @audit 1e18
1168:             vToken.reserveFactorMantissa() == 1e18;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L780

File: contracts/ExponentialNoError.sol

/// @audit 224
64:           require(n < 2**224, errorMessage);

/// @audit 32
69:           require(n < 2**32, errorMessage);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L64

File: contracts/Pool/PoolRegistry.sol

/// @audit 18
290:              10**(underlyingDecimals + 18 - input.decimals),

/// @audit 100
440:          require(bytes(name).length <= 100, "Pool's name is too large");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L290

File: contracts/Shortfall/Shortfall.sol

/// @audit 100
147:          waitForFirstBidder = 100;

/// @audit 1000
149:          incentiveBps = 1000;

/// @audit 1e18
393:              uint256 usdValue = (priceOracle.getUnderlyingPrice(address(vTokens[i])) * marketBadDebt) / 1e18;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L147

File: contracts/VToken.sol

/// @audit 1e18
313:          if (newProtocolSeizeShareMantissa_ + 1e18 > liquidationIncentive) {

/// @audit 5e16
1387:         protocolSeizeShareMantissa = 5e16; // 5%

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L313

[N‑17] Events that mark critical parameter changes should contain both the old and the new value

This should especially be done if the new value is not required to be different from the old value

There are 5 instances of this issue:

File: contracts/Comptroller.sol

/// @audit setMarketBorrowCaps()
848:              emit NewBorrowCap(vTokens[i], newBorrowCaps[i]);

/// @audit setMarketSupplyCaps()
873:              emit NewSupplyCap(vTokens[i], newSupplyCaps[i]);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L848

File: contracts/Rewards/RewardsDistributor.sol

/// @audit setContributorRewardTokenSpeed()
230:          emit ContributorRewardTokenSpeedUpdated(contributor, rewardTokenSpeed);

/// @audit updateContributorRewards()
268:              emit ContributorRewardsUpdated(contributor, rewardTokenAccrued[contributor]);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L230

File: contracts/RiskFund/ReserveHelpers.sol

/// @audit updateAssetsState()
68:               emit AssetsReservesUpdated(comptroller, asset, balanceDifference);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L68

[N‑18] Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

There is one instance of this issue:

File: contracts/WhitePaperInterestRateModel.sol

/// @audit seen in contracts/BaseJumpRateModelV2.sol 
17:       uint256 public constant blocksPerYear = 2102400;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L17

[N‑19] Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There are 3 instances of this issue:

File: contracts/Lens/PoolLens.sol

314:          //get tokens in the Pool

426:              //Update market supply and borrow index in-memory

430:              //Calculate pending rewards

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L314

[N‑20] Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. The solidity style guide recommends a maximumum line length of 120 characters, so the lines below should be split when they reach that length.

There are 22 instances of this issue:

File: contracts/Comptroller.sol

419:       * @custom:error MinimalCollateralViolated is thrown if the users' total collateral is lower than the threshold for non-batch liquidations

722:       * @custom:error InvalidLiquidationThreshold error is thrown when liquidation threshold is lower than collateral factor

827:       * @notice Set the given borrow caps for the given vToken markets. Borrowing that brings total borrows to or above borrow cap will revert.

833:       * @param newBorrowCaps The new borrow cap values in underlying to be set. A value of -1 corresponds to unlimited borrowing.

853:       * @notice Set the given supply caps for the given vToken markets. Supply that brings total Supply to or above supply cap will revert.

859:       * @param newSupplyCaps The new supply cap values in underlying to be set. A value of -1 corresponds to unlimited supply.

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L419

File: contracts/ComptrollerStorage.sol

85:       /// @notice Borrow caps enforced by borrowAllowed for each vToken address. Defaults to zero which restricts borrowing.

91:       /// @notice Supply caps enforced by mintAllowed for each vToken address. Defaults to zero which corresponds to minting notAllowed

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L85

File: contracts/Rewards/RewardsDistributor.sol

25:       /// @notice The REWARD TOKEN borrow index for each market for each supplier as of the last time they accrued REWARD TOKEN

49:       /// @notice The REWARD TOKEN borrow index for each market for each borrower as of the last time they accrued REWARD TOKEN

159:       *      We avoid an external call to check if they are in the market to save gas because this function is called in many places.

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L25

File: contracts/RiskFund/ProtocolShareReserve.sol

17:       // Percentage of funds not sent to the RiskFund contract when the funds are released, following the project Tokenomics

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L17

File: contracts/Shortfall/Shortfall.sol

329:       * @notice Update wait for first bidder block count. If the first bid is not made within this limit, the auction is closed and needs to be restarted

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L329

File: contracts/VToken.sol

163:       * @notice Accrue interest to updated borrowIndex and then calculate account's borrow balance using the updated borrowIndex

285:       * @custom:error LiquidateAccrueCollateralInterestFailed is thrown when it is not possible to accrue interest on the collateral vToken

286:       * @custom:error LiquidateCollateralFreshnessCheck is thrown when interest has not been accrued on the collateral vToken

442:       * @custom:error LiquidateAccrueCollateralInterestFailed is thrown when it is not possible to accrue interest on the collateral vToken

443:       * @custom:error LiquidateCollateralFreshnessCheck is thrown when interest has not been accrued on the collateral vToken

520:       * @notice A public function to sweep accidental ERC-20 transfers to this contract. Tokens are sent to admin (timelock)

797:       * @param redeemTokensIn The number of vTokens to redeem into underlying (only one of redeemTokensIn or redeemAmountIn may be non-zero)

798:       * @param redeemAmountIn The number of underlying tokens to receive from redeeming vTokens (only one of redeemTokensIn or redeemAmountIn may be non-zero)

1000:             // accrueInterest emits logs on errors, but we still want to log the fact that an attempted liquidation failed

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L163

[N‑21] Typos

There are 10 instances of this issue:

File: contracts/Comptroller.sol

/// @audit Throwed
94:        * @notice Throwed during the liquidation if user's total collateral amount is lower than

/// @audit Liquidaton
1388:      * @return Liquidaton threshold as exponential

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L94

File: contracts/MaxLoopsLimitHelper.sol

/// @audit Comapre
35:        * @notice Comapre the maxLoopsLimit with number of the times loop iterate

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/MaxLoopsLimitHelper.sol#L35

File: contracts/RiskFund/ReserveHelpers.sol

/// @audit updation
28:       // Event emitted after the updation of the assets reserves.

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L28

File: contracts/RiskFund/RiskFund.sol

/// @audit mininum
146:       * @notice Swap array of pool assets into base asset's tokens of at least a mininum amount.

/// @audit recieve
148:       * @param amountsOutMin Minimum amount to recieve for swap

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L146

File: contracts/Shortfall/Shortfall.sol

/// @audit Initalize
125:       * @notice Initalize the shortfall contract

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L125

File: contracts/VToken.sol

/// @audit suceeded
94:        * @return success True if the transfer suceeded, reverts otherwise

/// @audit suceeded
109:       * @return success True if the transfer suceeded, reverts otherwise

/// @audit fo
352:       * @param addAmount The amount fo underlying token to add as reserves

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L94

[N‑22] File is missing NatSpec

There are 10 instances of this issue:

File: contracts/ComptrollerInterface.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerInterface.sol

File: contracts/ErrorReporter.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ErrorReporter.sol

File: contracts/Factories/JumpRateModelFactory.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Factories/JumpRateModelFactory.sol

File: contracts/Factories/VTokenProxyFactory.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Factories/VTokenProxyFactory.sol

File: contracts/Factories/WhitePaperInterestRateModelFactory.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Factories/WhitePaperInterestRateModelFactory.sol

File: contracts/IPancakeswapV2Router.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/IPancakeswapV2Router.sol

File: contracts/Proxy/UpgradeableBeacon.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Proxy/UpgradeableBeacon.sol

File: contracts/RiskFund/IProtocolShareReserve.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/IProtocolShareReserve.sol

File: contracts/RiskFund/IRiskFund.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/IRiskFund.sol

File: contracts/Shortfall/IShortfall.sol

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/IShortfall.sol

[N‑23] NatSpec @param is missing

There are 47 instances of this issue:

File: contracts/Comptroller.sol

/// @audit Missing: '@param poolRegistry_'
126       /// @custom:oz-upgrades-unsafe-allow constructor
127:      constructor(address poolRegistry_) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L126-L127

File: contracts/ExponentialNoError.sol

/// @audit Missing: '@param exp'
25        /**
26         * @dev Truncates the given exp to a whole number value.
27         *      For example, truncate(Exp{mantissa: 15 * expScale}) = 15
28         */
29:       function truncate(Exp memory exp) internal pure returns (uint256) {

/// @audit Missing: '@param a'
/// @audit Missing: '@param scalar'
34        /**
35         * @dev Multiply an Exp by a scalar, then truncate to return an unsigned integer.
36         */
37        // solhint-disable-next-line func-name-mixedcase
38:       function mul_ScalarTruncate(Exp memory a, uint256 scalar) internal pure returns (uint256) {

/// @audit Missing: '@param a'
/// @audit Missing: '@param scalar'
/// @audit Missing: '@param addend'
43        /**
44         * @dev Multiply an Exp by a scalar, truncate, then add an to an unsigned integer, returning an unsigned integer.
45         */
46        // solhint-disable-next-line func-name-mixedcase
47        function mul_ScalarTruncateAddUInt(
48            Exp memory a,
49            uint256 scalar,
50            uint256 addend
51:       ) internal pure returns (uint256) {

/// @audit Missing: '@param left'
/// @audit Missing: '@param right'
56        /**
57         * @dev Checks if first Exp is less than second Exp.
58         */
59:       function lessThanExp(Exp memory left, Exp memory right) internal pure returns (bool) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L25-L29

File: contracts/Lens/PoolLens.sol

/// @audit Missing: '@param poolRegistryAddress'
305       /**
306        * @param venusPool The VenusPool Object from PoolRegistry.
307        * @notice Returns enriched PoolData.
308        */
309       function getPoolDataFromVenusPool(address poolRegistryAddress, PoolRegistry.VenusPool memory venusPool)
310           public
311           view
312:          returns (PoolData memory)

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L305-L312

File: contracts/Pool/PoolRegistryInterface.sol

/// @audit Missing: '@param comptroller'
28        /*** get a Pool by comptrollerAddress ***/
29:       function getPoolByComptroller(address comptroller) external view returns (VenusPool memory);

/// @audit Missing: '@param comptroller'
/// @audit Missing: '@param asset'
31        /*** get VToken in the Pool for an Asset ***/
32:       function getVTokenForAsset(address comptroller, address asset) external view returns (address);

/// @audit Missing: '@param asset'
34        /*** get Pools supported by Asset ***/
35:       function getPoolsSupportedByAsset(address asset) external view returns (address[] memory);

/// @audit Missing: '@param comptroller'
37        /*** get metadata of a Pool by comptroller ***/
38:       function getVenusPoolMetadata(address comptroller) external view returns (VenusPoolMetaData memory);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistryInterface.sol#L28-L29

File: contracts/Pool/PoolRegistry.sol

/// @audit Missing: '@param shortfall_'
156       /**
157        * @dev Initializes the deployer to owner.
158        * @param vTokenFactory_ vToken factory address.
159        * @param jumpRateFactory_ jump rate factory address.
160        * @param whitePaperFactory_ white paper factory address.
161        * @param protocolShareReserve_ protocol's shares reserve address.
162        * @param accessControlManager_ AccessControlManager contract address.
163        */
164       function initialize(
165           VTokenProxyFactory vTokenFactory_,
166           JumpRateModelFactory jumpRateFactory_,
167           WhitePaperInterestRateModelFactory whitePaperFactory_,
168           Shortfall shortfall_,
169           address payable protocolShareReserve_,
170           address accessControlManager_
171:      ) external initializer {

/// @audit Missing: '@param minLiquidatableCollateral'
/// @audit Missing: '@param accessControlManager'
202       /**
203        * @dev Deploys a new Venus pool and adds to the directory.
204        * @param name The name of the pool
205        * @param beaconAddress The upgradeable beacon contract address for Comptroller implementation
206        * @param closeFactor The pool's close factor (scaled by 1e18)
207        * @param liquidationIncentive The pool's liquidation incentive (scaled by 1e18)
208        * @param priceOracle The pool's PriceOracle address
209        * @param maxLoopsLimit The maximum limit for the loops can iterate.
210        * @return index The index of the registered Venus pool
211        * @return proxyAddress The the Comptroller proxy address
212        */
213       function createRegistryPool(
214           string calldata name,
215           address beaconAddress,
216           uint256 closeFactor,
217           uint256 liquidationIncentive,
218           uint256 minLiquidatableCollateral,
219           address priceOracle,
220           uint256 maxLoopsLimit,
221           address accessControlManager
222:      ) external virtual returns (uint256 index, address proxyAddress) {

/// @audit Missing: '@param comptroller'
/// @audit Missing: '@param name'
329       /**
330        * @notice Modify existing Venus pool name.
331        */
332:      function setPoolName(address comptroller, string calldata name) external {

/// @audit Missing: '@param comptroller'
/// @audit Missing: '@param _metadata'
340       /**
341        * @notice Update metadata of an existing pool.
342        */
343:      function updatePoolMetadata(address comptroller, VenusPoolMetaData memory _metadata) external {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L156-L171

File: contracts/Rewards/RewardsDistributor.sol

/// @audit Missing: '@param comptroller_'
/// @audit Missing: '@param rewardToken_'
/// @audit Missing: '@param loopsLimit_'
/// @audit Missing: '@param accessControlManager_'
108       /**
109        * @dev Initializes the deployer to owner.
110        */
111       function initialize(
112           Comptroller comptroller_,
113           IERC20Upgradeable rewardToken_,
114           uint256 loopsLimit_,
115           address accessControlManager_
116:      ) external initializer {

/// @audit Missing: '@param marketBorrowIndex'
154       /**
155        * @notice Calculate reward token accrued by a borrower and possibly transfer it to them
156        *         Borrowers will begin to accrue after the first interaction with the protocol.
157        * @dev This function should only be called when the user has a borrow position in the market
158        *      (e.g. Comptroller.preBorrowHook, and Comptroller.preRepayHook)
159        *      We avoid an external call to check if they are in the market to save gas because this function is called in many places.
160        * @param vToken The market in which the borrower is interacting
161        * @param borrower The address of the borrower to distribute REWARD TOKEN to
162        */
163       function distributeBorrowerRewardToken(
164           address vToken,
165           address borrower,
166           Exp memory marketBorrowIndex
167:      ) external onlyComptroller {

/// @audit Missing: '@param marketBorrowIndex'
369       /**
370        * @notice Calculate reward token accrued by a borrower and possibly transfer it to them.
371        * @param vToken The market in which the borrower is interacting
372        * @param borrower The address of the borrower to distribute REWARD TOKEN to
373        */
374       function _distributeBorrowerRewardToken(
375           address vToken,
376           address borrower,
377:          Exp memory marketBorrowIndex

/// @audit Missing: '@param marketBorrowIndex'
453       /**
454        * @notice Accrue REWARD TOKEN to the market by updating the borrow index.
455        * @param vToken The market whose borrow index to update
456        * @dev Index is a cumulative sum of the REWARD TOKEN per vToken accrued.
457        */
458:      function _updateRewardTokenBorrowIndex(address vToken, Exp memory marketBorrowIndex) internal {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L108-L116

File: contracts/RiskFund/ProtocolShareReserve.sol

/// @audit Missing: '@param comptroller'
60        /**
61         * @dev Release funds
62         * @param asset  Asset to be released
63         * @param amount Amount to release
64         * @return Number of total released tokens
65         */
66        function releaseFunds(
67            address comptroller,
68            address asset,
69            uint256 amount
70:       ) external returns (uint256) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L60-L70

File: contracts/RiskFund/RiskFund.sol

/// @audit Missing: '@param paths'
145       /**
146        * @notice Swap array of pool assets into base asset's tokens of at least a mininum amount.
147        * @param markets Array of vTokens whose assets to swap for base asset
148        * @param amountsOutMin Minimum amount to recieve for swap
149        * @return Number of swapped tokens.
150        */
151       function swapPoolsAssets(
152           address[] calldata markets,
153           uint256[] calldata amountsOutMin,
154           address[][] calldata paths
155:      ) external override returns (uint256) {

/// @audit Missing: '@param path'
218       /**
219        * @dev Swap single asset to base asset.
220        * @param vToken VToken
221        * @param comptroller Comptroller address
222        * @param amountOutMin Minimum amount to receive for swap
223        * @return Number of swapped tokens.
224        */
225       function _swapAsset(
226           VToken vToken,
227           address comptroller,
228           uint256 amountOutMin,
229           address[] calldata path
230:      ) internal returns (uint256) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L145-L155

File: contracts/VTokenInterfaces.sol

/// @audit Missing: '@param mintAmount'
269       /*** User Interface ***/
270   
271:      function mint(uint256 mintAmount) external virtual returns (uint256);

/// @audit Missing: '@param newReserveFactorMantissa'
323       /*** Admin Functions ***/
324   
325:      function setReserveFactor(uint256 newReserveFactorMantissa) external virtual;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L269-L271

File: contracts/VToken.sol

/// @audit Missing: '@param accessControlManager_'
/// @audit Missing: '@param reserveFactorMantissa_'
47        /**
48         * @notice Construct a new money market
49         * @param underlying_ The address of the underlying asset
50         * @param comptroller_ The address of the Comptroller
51         * @param interestRateModel_ The address of the interest rate model
52         * @param initialExchangeRateMantissa_ The initial exchange rate, scaled by 1e18
53         * @param name_ ERC-20 name of this token
54         * @param symbol_ ERC-20 symbol of this token
55         * @param decimals_ ERC-20 decimal precision of this token
56         * @param admin_ Address of the administrator of this token
57         * @param riskManagement Addresses of risk fund contracts
58         */
59        function initialize(
60            address underlying_,
61            ComptrollerInterface comptroller_,
62            InterestRateModel interestRateModel_,
63            uint256 initialExchangeRateMantissa_,
64            string memory name_,
65            string memory symbol_,
66            uint8 decimals_,
67            address admin_,
68            address accessControlManager_,
69            RiskManagementInit memory riskManagement,
70            uint256 reserveFactorMantissa_
71:       ) external initializer {

/// @audit Missing: '@param minter'
187       /**
188        * @notice Sender calls on-behalf of minter. minter supplies assets into the market and receives vTokens in exchange
189        * @dev Accrues interest whether or not the operation succeeds, unless reverted
190        * @param mintAmount The amount of the underlying asset to supply
191        * @return error Always NO_ERROR for compatibility with Venus core tooling
192        * @custom:event Emits Mint and Transfer events; may emit AccrueInterest
193        * @custom:access Not restricted
194        */
195:      function mintBehalf(address minter, uint256 mintAmount) external override nonReentrant returns (uint256) {

/// @audit Missing: '@param newReserveFactorMantissa'
322       /**
323        * @notice accrues interest and sets a new reserve factor for the protocol using _setReserveFactorFresh
324        * @dev Admin function to accrue interest and set a new reserve factor
325        * @custom:event Emits NewReserveFactor event; may emit AccrueInterest
326        * @custom:error Unauthorized error is thrown when the call is not authorized by AccessControlManager
327        * @custom:error SetReserveFactorBoundsCheck is thrown when the new reserve factor is too high
328        * @custom:access Controlled by AccessControlManager
329        */
330:      function setReserveFactor(uint256 newReserveFactorMantissa) external override nonReentrant {

/// @audit Missing: '@param borrower'
873       /**
874        * @notice Users borrow assets from the protocol to their own address
875        * @param borrowAmount The amount of the underlying asset to borrow
876        */
877:      function _borrowFresh(address borrower, uint256 borrowAmount) internal {

/// @audit Missing: '@param newReserveFactorMantissa'
1150      /**
1151       * @notice Sets a new reserve factor for the protocol (*requires fresh interest accrual)
1152       * @dev Admin function to set a new reserve factor
1153       */
1154:     function _setReserveFactorFresh(uint256 newReserveFactorMantissa) internal {

/// @audit Missing: '@param from'
/// @audit Missing: '@param amount'
1267      /**
1268       * @dev Similar to ERC-20 transfer, but handles tokens that have transfer fees.
1269       *      This function returns the actual amount received,
1270       *      which may be less than `amount` if there is a fee attached to the transfer.
1271       */
1272:     function _doTransferIn(address from, uint256 amount) internal virtual returns (uint256) {

/// @audit Missing: '@param to'
/// @audit Missing: '@param amount'
1281      /**
1282       * @dev Just a regular ERC-20 transfer, reverts on failure
1283       */
1284:     function _doTransferOut(address to, uint256 amount) internal virtual {

/// @audit Missing: '@param admin_'
/// @audit Missing: '@param accessControlManager_'
/// @audit Missing: '@param riskManagement'
1339      /**
1340       * @notice Initialize the money market
1341       * @param underlying_ The address of the underlying asset
1342       * @param comptroller_ The address of the Comptroller
1343       * @param interestRateModel_ The address of the interest rate model
1344       * @param initialExchangeRateMantissa_ The initial exchange rate, scaled by 1e18
1345       * @param name_ EIP-20 name of this token
1346       * @param symbol_ EIP-20 symbol of this token
1347       * @param decimals_ EIP-20 decimal precision of this token
1348       * @param reserveFactorMantissa_ Reserve factor mantissa (between 0 and 1e18)
1349       */
1350      function _initialize(
1351          address underlying_,
1352          ComptrollerInterface comptroller_,
1353          InterestRateModel interestRateModel_,
1354          uint256 initialExchangeRateMantissa_,
1355          string memory name_,
1356          string memory symbol_,
1357          uint8 decimals_,
1358          address admin_,
1359          address accessControlManager_,
1360          RiskManagementInit memory riskManagement,
1361          uint256 reserveFactorMantissa_
1362:     ) internal onlyInitializing {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L47-L71

[N‑24] NatSpec @return argument is missing

There are 27 instances of this issue:

File: contracts/Comptroller.sol

/// @audit Missing: '@return'
1134       * @notice A marker method that returns true for a valid Comptroller contract
1135       */
1136:     function isComptroller() external pure override returns (bool) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1134-L1136

File: contracts/ExponentialNoError.sol

/// @audit Missing: '@return'
25        /**
26         * @dev Truncates the given exp to a whole number value.
27         *      For example, truncate(Exp{mantissa: 15 * expScale}) = 15
28         */
29:       function truncate(Exp memory exp) internal pure returns (uint256) {

/// @audit Missing: '@return'
34        /**
35         * @dev Multiply an Exp by a scalar, then truncate to return an unsigned integer.
36         */
37        // solhint-disable-next-line func-name-mixedcase
38:       function mul_ScalarTruncate(Exp memory a, uint256 scalar) internal pure returns (uint256) {

/// @audit Missing: '@return'
43        /**
44         * @dev Multiply an Exp by a scalar, truncate, then add an to an unsigned integer, returning an unsigned integer.
45         */
46        // solhint-disable-next-line func-name-mixedcase
47        function mul_ScalarTruncateAddUInt(
48            Exp memory a,
49            uint256 scalar,
50            uint256 addend
51:       ) internal pure returns (uint256) {

/// @audit Missing: '@return'
56        /**
57         * @dev Checks if first Exp is less than second Exp.
58         */
59:       function lessThanExp(Exp memory left, Exp memory right) internal pure returns (bool) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L25-L29

File: contracts/Lens/PoolLens.sol

/// @audit Missing: '@return'
122        * @notice Returns the BalanceInfo of all VTokens.
123        */
124:      function vTokenBalancesAll(VToken[] calldata vTokens, address account) external returns (VTokenBalances[] memory) {

/// @audit Missing: '@return'
136        * @dev This function is not designed to be called in a transaction: it is too gas-intensive.
137        */
138:      function getAllPools(address poolRegistryAddress) external view returns (PoolData[] memory) {

/// @audit Missing: '@return'
157        * @notice Returns Venus pool Unitroller (Comptroller proxy) contract addresses.
158        */
159       function getPoolByComptroller(address poolRegistryAddress, address comptroller)
160           external
161           view
162:          returns (PoolData memory)

/// @audit Missing: '@return'
172        * @notice Returns VToken in a Pool for an Asset.
173        */
174       function getVTokenForAsset(
175           address poolRegistryAddress,
176           address comptroller,
177           address asset
178:      ) external view returns (address) {

/// @audit Missing: '@return'
186        * @notice Returns all Pools supported by an Asset.
187        */
188       function getPoolsSupportedByAsset(address poolRegistryAddress, address asset)
189           external
190           view
191:          returns (address[] memory)

/// @audit Missing: '@return'
199        * @notice Returns the underlyingPrice Info of all VTokens.
200        */
201       function vTokenUnderlyingPriceAll(VToken[] calldata vTokens)
202           external
203           view
204:          returns (VTokenUnderlyingPrice[] memory)

/// @audit Missing: '@return'
281        * @notice Returns the BalanceInfo of VToken.
282        */
283:      function vTokenBalances(VToken vToken, address account) public returns (VTokenBalances memory) {

/// @audit Missing: '@return'
305       /**
306        * @param venusPool The VenusPool Object from PoolRegistry.
307        * @notice Returns enriched PoolData.
308        */
309       function getPoolDataFromVenusPool(address poolRegistryAddress, PoolRegistry.VenusPool memory venusPool)
310           public
311           view
312:          returns (PoolData memory)

/// @audit Missing: '@return'
350        * @notice Returns the metadata of VToken.
351        */
352:      function vTokenMetadata(VToken vToken) public view returns (VTokenMetadata memory) {

/// @audit Missing: '@return'
386        * @notice Returns the metadata of all VTokens.
387        */
388:      function vTokenMetadataAll(VToken[] memory vTokens) public view returns (VTokenMetadata[] memory) {

/// @audit Missing: '@return'
399        * @notice Returns the underlyingPrice of VToken.
400        */
401:      function vTokenUnderlyingPrice(VToken vToken) public view returns (VTokenUnderlyingPrice memory) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L122-L124

File: contracts/Pool/PoolRegistryInterface.sol

/// @audit Missing: '@return'
25        /*** get All Pools in PoolRegistry ***/
26:       function getAllPools() external view returns (VenusPool[] memory);

/// @audit Missing: '@return'
28        /*** get a Pool by comptrollerAddress ***/
29:       function getPoolByComptroller(address comptroller) external view returns (VenusPool memory);

/// @audit Missing: '@return'
31        /*** get VToken in the Pool for an Asset ***/
32:       function getVTokenForAsset(address comptroller, address asset) external view returns (address);

/// @audit Missing: '@return'
34        /*** get Pools supported by Asset ***/
35:       function getPoolsSupportedByAsset(address asset) external view returns (address[] memory);

/// @audit Missing: '@return'
37        /*** get metadata of a Pool by comptroller ***/
38:       function getVenusPoolMetadata(address comptroller) external view returns (VenusPoolMetaData memory);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistryInterface.sol#L25-L26

File: contracts/Pool/PoolRegistry.sol

/// @audit Missing: '@return'
352        * @dev This function is not designed to be called in a transaction: it is too gas-intensive.
353        */
354:      function getAllPools() external view override returns (VenusPool[] memory) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L352-L354

File: contracts/Shortfall/Shortfall.sol

/// @audit Missing: '@return'
456        * @param auction The auction to query the status for
457        */
458:      function _isStarted(Auction storage auction) internal view returns (bool) {

/// @audit Missing: '@return'
465        * @param auction The auction to query the status for
466        */
467:      function _isStale(Auction storage auction) internal view returns (bool) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L456-L458

File: contracts/VTokenInterfaces.sol

/// @audit Missing: '@return'
269       /*** User Interface ***/
270   
271:      function mint(uint256 mintAmount) external virtual returns (uint256);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L269-L271

File: contracts/VToken.sol

/// @audit Missing: '@return'
1267      /**
1268       * @dev Similar to ERC-20 transfer, but handles tokens that have transfer fees.
1269       *      This function returns the actual amount received,
1270       *      which may be less than `amount` if there is a fee attached to the transfer.
1271       */
1272:     function _doTransferIn(address from, uint256 amount) internal virtual returns (uint256) {

/// @audit Missing: '@return'
1428       *  This exists mainly for inheriting test contracts to stub this result.
1429       */
1430:     function _getBlockNumber() internal view virtual returns (uint256) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L1267-L1272

[N‑25] Event is not properly indexed

Index event fields make the field more quickly accessible to off-chain tools that parse events. This is especially useful when it comes to filtering based on an address. 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). Where applicable, 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 applicable fields, all of the applicable fields should be indexed.

There are 12 instances of this issue:

File: contracts/Comptroller.sol

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L30

File: contracts/Pool/PoolRegistry.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L132

File: contracts/Rewards/RewardsDistributor.sol

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

87:       event MarketInitialized(address vToken);

90:       event RewardTokenSupplyIndexUpdated(address vToken);

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L81

File: contracts/RiskFund/ProtocolShareReserve.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L22

File: contracts/RiskFund/RiskFund.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L56

File: contracts/VTokenInterfaces.sol

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

267:      event SweepToken(address token);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L262

[N‑26] Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 14 instances of this issue:

File: contracts/Comptroller.sol

/// @audit error
/// @audit liquidity
/// @audit shortfall
985       function getAccountLiquidity(address account)
986           external
987           view
988           returns (
989               uint256 error,
990               uint256 liquidity,
991:              uint256 shortfall

/// @audit error
/// @audit liquidity
/// @audit shortfall
1009      function getHypotheticalAccountLiquidity(
1010          address account,
1011          address vTokenModify,
1012          uint256 redeemTokens,
1013          uint256 borrowAmount
1014      )
1015          external
1016          view
1017          returns (
1018              uint256 error,
1019              uint256 liquidity,
1020:             uint256 shortfall

/// @audit error
/// @audit tokensToSeize
1084      function liquidateCalculateSeizeTokens(
1085          address vTokenBorrowed,
1086          address vTokenCollateral,
1087          uint256 actualRepayAmount
1088:     ) external view override returns (uint256 error, uint256 tokensToSeize) {

/// @audit snapshot
1276      function _getCurrentLiquiditySnapshot(address account, function(VToken) internal view returns (Exp memory) weight)
1277          internal
1278          view
1279:         returns (AccountLiquiditySnapshot memory snapshot)

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L985-L991

File: contracts/Pool/PoolRegistry.sol

/// @audit index
213       function createRegistryPool(
214           string calldata name,
215           address beaconAddress,
216           uint256 closeFactor,
217           uint256 liquidationIncentive,
218           uint256 minLiquidatableCollateral,
219           address priceOracle,
220           uint256 maxLoopsLimit,
221           address accessControlManager
222:      ) external virtual returns (uint256 index, address proxyAddress) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L213-L222

File: contracts/VToken.sol

/// @audit error
/// @audit vTokenBalance
/// @audit borrowBalance
/// @audit exchangeRate
561       function getAccountSnapshot(address account)
562           external
563           view
564           override
565           returns (
566               uint256 error,
567               uint256 vTokenBalance,
568               uint256 borrowBalance,
569:              uint256 exchangeRate

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L561-L569

[N‑27] Duplicated require()/revert() checks should be refactored to a modifier or function

The compiler will inline the function, which will avoid JUMP instructions usually associated with functions

There are 5 instances of this issue:

File: contracts/RiskFund/ReserveHelpers.sol

51:           require(ComptrollerInterface(comptroller).isComptroller(), "ReserveHelpers: Comptroller address invalid");

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L51

File: contracts/RiskFund/RiskFund.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L139

File: contracts/Shortfall/Shortfall.sol

212:          require(_isStarted(auction), "no on-going auction");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L212

File: contracts/VToken.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L626

[N‑28] Interfaces should be indicated with an I prefix in the contract name

There are 3 instances of this issue:

File: contracts/ComptrollerInterface.sol

8:    interface ComptrollerInterface {

72:   interface ComptrollerViewInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerInterface.sol#L8

File: contracts/Pool/PoolRegistryInterface.sol

4:    interface PoolRegistryInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistryInterface.sol#L4

[N‑29] Numeric values having to do with time should use time units for readability

There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used

There are 2 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

/// @audit 10512000
20        /**
21         * @notice The approximate number of blocks per year that is assumed by the interest rate model
22         */
23:       uint256 public constant blocksPerYear = 10512000;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L20-L23

File: contracts/WhitePaperInterestRateModel.sol

/// @audit 2102400
14        /**
15         * @notice The approximate number of blocks per year that is assumed by the interest rate model
16         */
17:       uint256 public constant blocksPerYear = 2102400;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L14-L17

[N‑30] Consider using delete rather than assigning zero/false to clear values

The delete keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic

There are 8 instances of this issue:

File: contracts/Comptroller.sol

812:          newMarket.collateralFactorMantissa = 0;

813:          newMarket.liquidationThresholdMantissa = 0;

1353:                 snapshot.shortfall = 0;

1355:                 snapshot.liquidity = 0;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L812

File: contracts/Shortfall/Shortfall.sol

370:          auction.highestBidBps = 0;

371:          auction.highestBidBlock = 0;

376:              auction.marketDebt[vToken] = 0;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L370

File: contracts/VToken.sol

424:          accountBorrows[borrower].principal = 0;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L424

[N‑31] Contracts should have full test coverage

While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.

There is one instance of this issue:

File: Various Files

[N‑32] Large or complicated code bases should implement invariant tests

Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement invariant fuzzing tests. Invariant fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and invariant fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.

There is one instance of this issue:

File: Various Files

[N‑33] internal functions not called by the contract should be removed

All unused code should be removed

There are 2 instances of this issue:

File: contracts/Comptroller.sol

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1381

Gas Optimizations

[G‑01] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

There are 3 instances of this issue:

File: contracts/ComptrollerStorage.sol

92        mapping(address => uint256) public supplyCaps;
93    
94        /// @notice True if a certain action is paused on a certain market
95:       mapping(address => mapping(Action => bool)) internal _actionPaused;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L92-L95

File: contracts/Rewards/RewardsDistributor.sol

23        mapping(address => RewardToken) public rewardTokenSupplyState;
24    
25        /// @notice The REWARD TOKEN borrow index for each market for each supplier as of the last time they accrued REWARD TOKEN
26:       mapping(address => mapping(address => uint256)) public rewardTokenSupplierIndex;

32        mapping(address => uint256) public rewardTokenAccrued;
33    
34        /// @notice The rate at which rewardToken is distributed to the corresponding borrow market (per block)
35        mapping(address => uint256) public rewardTokenBorrowSpeeds;
36    
37        /// @notice The rate at which rewardToken is distributed to the corresponding supply market (per block)
38        mapping(address => uint256) public rewardTokenSupplySpeeds;
39    
40        /// @notice The REWARD TOKEN market borrow state for each market
41        mapping(address => RewardToken) public rewardTokenBorrowState;
42    
43        /// @notice The portion of REWARD TOKEN that each contributor receives per block
44        mapping(address => uint256) public rewardTokenContributorSpeeds;
45    
46        /// @notice Last block at which a contributor's REWARD TOKEN rewards have been allocated
47        mapping(address => uint256) public lastContributorBlock;
48    
49        /// @notice The REWARD TOKEN borrow index for each market for each borrower as of the last time they accrued REWARD TOKEN
50:       mapping(address => mapping(address => uint256)) public rewardTokenBorrowerIndex;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L23-L26

[G‑02] Structs can be packed into fewer storage slots

Each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the struct. Subsequent reads as well as writes have smaller gas savings

There are 3 instances of this issue:

File: contracts/Lens/PoolLens.sol

/// @audit Variable ordering with 15 slots instead of the current 16:
///           uint256(32):exchangeRateCurrent, uint256(32):supplyRatePerBlock, uint256(32):borrowRatePerBlock, uint256(32):reserveFactorMantissa, uint256(32):supplyCaps, uint256(32):borrowCaps, uint256(32):totalBorrows, uint256(32):totalReserves, uint256(32):totalSupply, uint256(32):totalCash, uint256(32):collateralFactorMantissa, uint256(32):vTokenDecimals, uint256(32):underlyingDecimals, address(20):vToken, bool(1):isListed, address(20):underlyingAssetAddress
35        struct VTokenMetadata {
36            address vToken;
37            uint256 exchangeRateCurrent;
38            uint256 supplyRatePerBlock;
39            uint256 borrowRatePerBlock;
40            uint256 reserveFactorMantissa;
41            uint256 supplyCaps;
42            uint256 borrowCaps;
43            uint256 totalBorrows;
44            uint256 totalReserves;
45            uint256 totalSupply;
46            uint256 totalCash;
47            bool isListed;
48            uint256 collateralFactorMantissa;
49            address underlyingAssetAddress;
50            uint256 vTokenDecimals;
51            uint256 underlyingDecimals;
52:       }

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L35-L52

File: contracts/Pool/PoolRegistry.sol

/// @audit Variable ordering with 16 slots instead of the current 17:
///           string(32):name, string(32):symbol, uint256(32):baseRatePerYear, uint256(32):multiplierPerYear, uint256(32):jumpMultiplierPerYear, uint256(32):kink_, uint256(32):collateralFactor, uint256(32):liquidationThreshold, uint256(32):reserveFactor, uint256(32):initialSupply, uint256(32):supplyCap, uint256(32):borrowCap, address(20):comptroller, uint8(1):decimals, uint8(1):rateModel, user-defined(null):accessControlManager, address(20):asset, address(20):beaconAddress, address(20):vTokenReceiver
33        struct AddMarketInput {
34            address comptroller;
35            address asset;
36            uint8 decimals;
37            string name;
38            string symbol;
39            InterestRateModels rateModel;
40            uint256 baseRatePerYear;
41            uint256 multiplierPerYear;
42            uint256 jumpMultiplierPerYear;
43            uint256 kink_;
44            uint256 collateralFactor;
45            uint256 liquidationThreshold;
46            uint256 reserveFactor;
47            AccessControlManager accessControlManager;
48            address beaconAddress;
49            uint256 initialSupply;
50            address vTokenReceiver;
51            uint256 supplyCap;
52            uint256 borrowCap;
53:       }

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L33-L53

File: contracts/Shortfall/Shortfall.sol

/// @audit Variable ordering with 8 slots instead of the current 9:
///           uint256(32):startBlock, user-defined[](32):markets, uint256(32):seizedRiskFund, uint256(32):highestBidBps, uint256(32):highestBidBlock, uint256(32):startBidBps, mapping(32):marketDebt, address(20):highestBidder, uint8(1):auctionType, uint8(1):status
34        struct Auction {
35            uint256 startBlock;
36            AuctionType auctionType;
37            AuctionStatus status;
38            VToken[] markets;
39            uint256 seizedRiskFund;
40            address highestBidder;
41            uint256 highestBidBps;
42            uint256 highestBidBlock;
43            uint256 startBidBps;
44            mapping(VToken => uint256) marketDebt;
45:       }

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L34-L45

[G‑03] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

There are 6 instances of this issue:

File: contracts/Comptroller.sol

213:          VToken[] memory userAssetList = accountAssets[msg.sender];

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L213

File: contracts/Lens/PoolLens.sol

146:              PoolRegistry.VenusPool memory venusPool = venusPools[i];

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L146

File: contracts/Rewards/RewardsDistributor.sol

438               Double memory ratio = supplyTokens > 0
439                   ? fraction(accruedSinceUpdate, supplyTokens)
440:                  : Double({ mantissa: 0 });

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L438-L440

[G‑04] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

There are 22 instances of this issue:

File: contracts/Comptroller.sol

/// @audit oracle on line 331
345:          if (oracle.getUnderlyingPrice(vToken) == 0) {

/// @audit oracle on line 436
437:          oracle.updatePrice(vTokenCollateral);

/// @audit closeFactorMantissa on line 708
709:          emit NewCloseFactor(oldCloseFactorMantissa, closeFactorMantissa);

/// @audit minLiquidatableCollateral on line 459
461:              revert MinimalCollateralViolated(minLiquidatableCollateral, snapshot.totalCollateral);

/// @audit minLiquidatableCollateral on line 591
592:              revert CollateralExceedsThreshold(minLiquidatableCollateral, snapshot.totalCollateral);

/// @audit minLiquidatableCollateral on line 646
648:              revert CollateralExceedsThreshold(minLiquidatableCollateral, snapshot.totalCollateral);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L345

File: contracts/MaxLoopsLimitHelper.sol

/// @audit maxLoopsLimit on line 26
28:           uint256 oldMaxLoopsLimit = maxLoopsLimit;

/// @audit maxLoopsLimit on line 29
31:           emit MaxLoopsLimitUpdated(oldMaxLoopsLimit, maxLoopsLimit);

/// @audit maxLoopsLimit on line 40
41:               revert MaxLoopsLimitExceeded(maxLoopsLimit, len);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/MaxLoopsLimitHelper.sol#L28

File: contracts/Pool/PoolRegistry.sol

/// @audit _numberOfPools on line 355
356:          for (uint256 i = 1; i <= _numberOfPools; ++i) {

/// @audit _numberOfPools on line 399
403:          _poolsByID[_numberOfPools] = comptroller;

/// @audit _numberOfPools on line 403
407:          return _numberOfPools;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L356

File: contracts/Rewards/RewardsDistributor.sol

/// @audit rewardToken on line 417
419:              rewardToken.safeTransfer(user, amount);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L419

File: contracts/RiskFund/ProtocolShareReserve.sol

/// @audit riskFund on line 82
85:           IRiskFund(riskFund).updateAssetsState(comptroller, asset);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L85

File: contracts/RiskFund/ReserveHelpers.sol

/// @audit poolRegistry on line 53
55:               PoolRegistryInterface(poolRegistry).getVTokenForAsset(comptroller, asset) != address(0),

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L55

File: contracts/RiskFund/RiskFund.sol

/// @audit pancakeSwapRouter on line 258
259:                      IERC20Upgradeable(underlyingAsset).safeApprove(pancakeSwapRouter, balanceOfUnderlyingAsset);

/// @audit pancakeSwapRouter on line 259
260:                      uint256[] memory amounts = IPancakeswapV2Router(pancakeSwapRouter).swapExactTokensForTokens(

/// @audit minAmountToConvert on line 232
248:              if (amountInUsd >= minAmountToConvert) {

/// @audit convertibleBaseAsset on line 252
255:                          path[path.length - 1] == convertibleBaseAsset,

/// @audit shortfall on line 191
194:          IERC20Upgradeable(convertibleBaseAsset).safeTransfer(shortfall, amount);

/// @audit poolRegistry on line 157
170:              PoolRegistry.VenusPool memory pool = PoolRegistry(poolRegistry).getPoolByComptroller(comptroller);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L259

File: contracts/Shortfall/Shortfall.sol

/// @audit incentiveBps on line 405
409:                  (poolBadDebt * (MAX_BPS + incentiveBps));

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L409

[G‑05] Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata

There are 13 instances of this issue:

File: contracts/Comptroller.sol

/// @audit rewardsDistributors[i] on line 275
276:              rewardsDistributors[i].distributeSupplierRewardToken(vToken, minter);

/// @audit rewardsDistributors[i] on line 305
306:              rewardsDistributors[i].distributeSupplierRewardToken(vToken, redeemer);

/// @audit markets[vToken] on line 333
337:          if (!markets[vToken].accountMembership[borrower]) {

/// @audit rewardsDistributors[i] on line 377
378:              rewardsDistributors[i].distributeBorrowerRewardToken(vToken, borrower, borrowIndex);

/// @audit rewardsDistributors[i] on line 404
405:              rewardsDistributors[i].distributeBorrowerRewardToken(vToken, borrower, borrowIndex);

/// @audit rewardsDistributors[i] on line 522
523:              rewardsDistributors[i].distributeSupplierRewardToken(vTokenCollateral, borrower);

/// @audit rewardsDistributors[i] on line 523
524:              rewardsDistributors[i].distributeSupplierRewardToken(vTokenCollateral, liquidator);

/// @audit rewardsDistributors[i] on line 559
560:              rewardsDistributors[i].distributeSupplierRewardToken(vToken, src);

/// @audit rewardsDistributors[i] on line 560
561:              rewardsDistributors[i].distributeSupplierRewardToken(vToken, dst);

/// @audit rewardsDistributors[i] on line 1123
1126:                 supplySpeed: rewardsDistributors[i].rewardTokenSupplySpeeds(vToken),

/// @audit rewardsDistributors[i] on line 1126
1127:                 borrowSpeed: rewardsDistributors[i].rewardTokenBorrowSpeeds(vToken)

/// @audit markets[vToken] on line 1245
1250:         if (!markets[vToken].accountMembership[redeemer]) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L276

File: contracts/Pool/PoolRegistry.sol

/// @audit _poolByComptroller[comptroller] on line 335
336:          _poolByComptroller[comptroller].name = name;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L336

[G‑06] The result of function calls should be cached rather than re-calling the function

The instances below point to the second+ call of the function within a single function

There is one instance of this issue:

File: contracts/Lens/PoolLens.sol

/// @audit vToken.underlying() on line 360
361:          underlyingDecimals = IERC20Metadata(vToken.underlying()).decimals();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L361

[G‑07] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

There are 19 instances of this issue:

File: contracts/Comptroller.sol

1205:     function _addMarket(address vToken) internal {

1224      function _setActionPaused(
1225          address market,
1226          Action action,
1227:         bool paused

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1205

File: contracts/Lens/PoolLens.sol

412       function _calculateNotDistributedAwards(
413           address account,
414           VToken[] memory markets,
415           RewardsDistributor rewardsDistributor
416:      ) internal view returns (PendingReward[] memory) {

453       function updateMarketBorrowIndex(
454           address vToken,
455           RewardsDistributor rewardsDistributor,
456           RewardTokenState memory borrowState,
457:          Exp memory marketBorrowIndex

475       function updateMarketSupplyIndex(
476           address vToken,
477           RewardsDistributor rewardsDistributor,
478:          RewardTokenState memory supplyState

495       function calculateBorrowerReward(
496           address vToken,
497           RewardsDistributor rewardsDistributor,
498           address borrower,
499           RewardTokenState memory borrowState,
500           Exp memory marketBorrowIndex
501:      ) internal view returns (uint256) {

516       function calculateSupplierReward(
517           address vToken,
518           RewardsDistributor rewardsDistributor,
519           address supplier,
520           RewardTokenState memory supplyState
521:      ) internal view returns (uint256) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L412-L416

File: contracts/Pool/PoolRegistry.sol

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

410       function _transferIn(
411           IERC20Upgradeable token,
412           address from,
413           uint256 amount
414:      ) internal returns (uint256) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L393

File: contracts/Rewards/RewardsDistributor.sol

304       function _setRewardTokenSpeed(
305           VToken vToken,
306           uint256 supplySpeed,
307:          uint256 borrowSpeed

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L304-L307

File: contracts/RiskFund/RiskFund.sol

225       function _swapAsset(
226           VToken vToken,
227           address comptroller,
228           uint256 amountOutMin,
229           address[] calldata path
230:      ) internal returns (uint256) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L225-L230

File: contracts/Shortfall/Shortfall.sol

441:      function _getPriceOracle(address comptroller) internal view returns (PriceOracle) {

450:      function _getAllMarkets(address comptroller) internal view returns (VToken[] memory) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L441

File: contracts/VToken.sol

877:      function _borrowFresh(address borrower, uint256 borrowAmount) internal {

1018      function _liquidateBorrowFresh(
1019          address liquidator,
1020          address borrower,
1021          uint256 repayAmount,
1022          VTokenInterface vTokenCollateral,
1023:         bool skipLiquidityCheck

1138:     function _setComptroller(ComptrollerInterface newComptroller) internal {

1177:     function _addReservesFresh(uint256 addAmount) internal returns (uint256) {

1200:     function _reduceReservesFresh(uint256 reduceAmount) internal {

1350      function _initialize(
1351          address underlying_,
1352          ComptrollerInterface comptroller_,
1353          InterestRateModel interestRateModel_,
1354          uint256 initialExchangeRateMantissa_,
1355          string memory name_,
1356          string memory symbol_,
1357          uint8 decimals_,
1358          address admin_,
1359          address accessControlManager_,
1360          RiskManagementInit memory riskManagement,
1361          uint256 reserveFactorMantissa_
1362:     ) internal onlyInitializing {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L877

[G‑08] Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement

require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }

There are 3 instances of this issue:

File: contracts/RiskFund/RiskFund.sol

/// @audit require() on line 192
193:          poolReserves[comptroller] = poolReserves[comptroller] - amount;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L193

File: contracts/Comptroller.sol

/// @audit if-condition on line 1351
1352:                 snapshot.liquidity = snapshot.weightedCollateral - borrowPlusEffects;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1352

File: contracts/RiskFund/ReserveHelpers.sol

/// @audit if-condition on line 61
64:                   balanceDifference = currentBalance - assetReserve;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L64

[G‑09] <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

  • storage arrays incur a Gwarmaccess (100 gas)
  • memory arrays use MLOAD (3 gas)
  • calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 3 instances of this issue:

File: contracts/Lens/PoolLens.sol

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

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L229

[G‑10] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

There are 37 instances of this issue:

File: contracts/Comptroller.sol

162:          for (uint256 i; i < len; ++i) {

217:          for (uint256 i; i < len; ++i) {

274:          for (uint256 i; i < rewardDistributorsCount; ++i) {

304:          for (uint256 i; i < rewardDistributorsCount; ++i) {

376:          for (uint256 i; i < rewardDistributorsCount; ++i) {

402:          for (uint256 i; i < rewardDistributorsCount; ++i) {

521:          for (uint256 i; i < rewardDistributorsCount; ++i) {

558:          for (uint256 i; i < rewardDistributorsCount; ++i) {

584:          for (uint256 i; i < userAssetsCount; ++i) {

611:          for (uint256 i; i < userAssetsCount; ++i) {

669:          for (uint256 i; i < ordersCount; ++i) {

690:          for (uint256 i; i < marketsCount; ++i) {

819:          for (uint256 i; i < rewardDistributorsCount; ++i) {

846:          for (uint256 i; i < numMarkets; ++i) {

871:          for (uint256 i; i < vTokensCount; ++i) {

897:          for (uint256 marketIdx; marketIdx < marketsCount; ++marketIdx) {

898:              for (uint256 actionIdx; actionIdx < actionsCount; ++actionIdx) {

932:          for (uint256 i; i < rewardsDistributorsLength; ++i) {

948:          for (uint256 i; i < marketsCount; ++i) {

1122:         for (uint256 i; i < rewardsDistributorsLength; ++i) {

1208:         for (uint256 i; i < marketsCount; ++i) {

1307:         for (uint256 i; i < assetsCount; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L162

File: contracts/Lens/PoolLens.sol

127:          for (uint256 i; i < vTokenCount; ++i) {

145:          for (uint256 i; i < poolLength; ++i) {

208:          for (uint256 i; i < vTokenCount; ++i) {

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

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

391:          for (uint256 i; i < vTokenCount; ++i) {

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L127

File: contracts/Pool/PoolRegistry.sol

356:          for (uint256 i = 1; i <= _numberOfPools; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L356

File: contracts/Rewards/RewardsDistributor.sol

209:          for (uint256 i; i < numTokens; ++i) {

282:          for (uint256 i; i < vTokensCount; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L209

File: contracts/RiskFund/RiskFund.sol

166:          for (uint256 i; i < marketsCount; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L166

File: contracts/Shortfall/Shortfall.sol

175:          for (uint256 i; i < marketsCount; ++i) {

223:          for (uint256 i; i < marketsCount; ++i) {

374:          for (uint256 i; i < marketsCount; ++i) {

389:          for (uint256 i; i < marketsCount; ++i) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L175

[G‑11] require()/revert() strings longer than 32 bytes cost extra gas

Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas

There are 61 instances of this issue:

File: contracts/Comptroller.sol

692:              require(borrowBalance == 0, "Nonzero borrow balance after liquidation");

704:          require(closeFactorMaxMantissa >= newCloseFactorMantissa, "Close factor greater than maximum close factor");

705:          require(closeFactorMinMantissa <= newCloseFactorMantissa, "Close factor smaller than minimum close factor");

780:          require(newLiquidationIncentiveMantissa >= 1e18, "liquidation incentive should be greater than 1e18");

934               require(
935                   rewardToken != address(_rewardsDistributor.rewardToken()),
936                   "distributor already exists with this reward"
937:              );

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L692

File: contracts/MaxLoopsLimitHelper.sol

26:           require(limit > maxLoopsLimit, "Comptroller: Invalid maxLoopsLimit");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/MaxLoopsLimitHelper.sol#L26

File: contracts/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");

263           require(
264               _vTokens[input.comptroller][input.asset] == address(0),
265               "PoolRegistry: Market already added for asset comptroller combination"
266:          );

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L225

File: contracts/Rewards/RewardsDistributor.sol

99:           require(address(comptroller) == msg.sender, "Only comptroller can call this function");

183:          require(amountLeft == 0, "insufficient rewardToken for grant");

204           require(
205               numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length,
206               "RewardsDistributor::setRewardTokenSpeeds invalid input"
207:          );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L99

File: contracts/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");

72:           require(amount <= poolsAssetsReserves[comptroller][asset], "ProtocolShareReserve: Insufficient pool balance");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L40

File: contracts/RiskFund/ReserveHelpers.sol

39:           require(ComptrollerInterface(comptroller).isComptroller(), "ReserveHelpers: Comptroller address invalid");

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

51:           require(ComptrollerInterface(comptroller).isComptroller(), "ReserveHelpers: Comptroller address invalid");

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

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

54            require(
55                PoolRegistryInterface(poolRegistry).getVTokenForAsset(comptroller, asset) != address(0),
56                "ReserveHelpers: The pool doesn't support the asset"
57:           );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L39

File: contracts/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");

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

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

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

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

112           require(
113               IShortfall(shortfallContractAddress_).convertibleBaseAsset() == convertibleBaseAsset,
114               "Risk Fund: Base asset doesn't match"
115:          );

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

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

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

158:          require(markets.length == amountsOutMin.length, "Risk fund: markets and amountsOutMin are unequal lengths");

159:          require(markets.length == paths.length, "Risk fund: markets and paths are unequal lengths");

171:              require(pool.comptroller == comptroller, "comptroller doesn't exist pool registry");

191:          require(msg.sender == shortfall, "Risk fund: Only callable by Shortfall contract");

192:          require(amount <= poolReserves[comptroller], "Risk Fund: Insufficient pool reserve.");

231:          require(amountOutMin != 0, "RiskFund: amountOutMin must be greater than 0 to swap vToken");

232:          require(amountOutMin >= minAmountToConvert, "RiskFund: amountOutMin should be greater than minAmountToConvert");

253:                      require(path[0] == underlyingAsset, "RiskFund: swap path must start with the underlying asset");

254                       require(
255                           path[path.length - 1] == convertibleBaseAsset,
256                           "RiskFund: finally path must be convertible base asset"
257:                      );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L80

File: contracts/Shortfall/Shortfall.sol

163:          require(bidBps <= MAX_BPS, "basis points cannot be more than 10000");

213           require(
214               block.number > auction.highestBidBlock + nextBidderBlockLimit && auction.highestBidder != address(0),
215               "waiting for next bidder. cannot close auction"
216:          );

279:          require(_isStale(auction), "you need to wait for more time for first bidder");

295:          require(_nextBidderBlockLimit != 0, "_nextBidderBlockLimit must not be 0");

361:          require(pool.comptroller == comptroller, "comptroller doesn't exist pool registry");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L163

File: contracts/VToken.sol

489:          require(msg.sender == shortfall, "only shortfall contract can update bad debt");

490:          require(recoveredAmount_ <= badDebt, "more than bad debt recovered from auction");

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

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

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

1072:         require(amountSeizeError == NO_ERROR, "LIQUIDATE_COMPTROLLER_CALCULATE_AMOUNT_SEIZE_FAILED");

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L489

[G‑12] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 16 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

/// @audit updateJumpRateModel(), utilizationRate()
12:   abstract contract BaseJumpRateModelV2 is InterestRateModel {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L12

File: contracts/ComptrollerInterface.sol

/// @audit enterMarkets(), exitMarket(), preMintHook(), preRedeemHook(), preBorrowHook(), preRepayHook(), preLiquidateHook(), preSeizeHook(), preTransferHook(), isComptroller(), liquidateCalculateSeizeTokens(), getAllMarkets()
8:    interface ComptrollerInterface {

/// @audit markets(), oracle(), getAssetsIn(), closeFactorMantissa(), liquidationIncentiveMantissa(), minLiquidatableCollateral(), getRewardDistributors(), getAllMarkets(), borrowCaps(), supplyCaps()
72:   interface ComptrollerViewInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerInterface.sol#L8

File: contracts/Comptroller.sol

/// @audit initialize(), healAccount(), liquidateAccount(), setCloseFactor(), setCollateralFactor(), setLiquidationIncentive(), supportMarket(), setMarketBorrowCaps(), setMarketSupplyCaps(), setActionsPaused(), setMinLiquidatableCollateral(), addRewardsDistributor(), setPriceOracle(), setMaxLoopsLimit(), getAccountLiquidity(), getHypotheticalAccountLiquidity(), isMarketListed(), getAssetsIn(), checkMembership(), getRewardsByMarket(), getRewardDistributors(), actionPaused(), isDeprecated()
17:   contract Comptroller is

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L17

File: contracts/InterestRateModel.sol

/// @audit getBorrowRate(), getSupplyRate()
8:    abstract contract InterestRateModel {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/InterestRateModel.sol#L8

File: contracts/Lens/PoolLens.sol

/// @audit vTokenBalancesAll(), getAllPools(), getPoolByComptroller(), getVTokenForAsset(), getPoolsSupportedByAsset(), vTokenUnderlyingPriceAll(), getPendingRewards(), getPoolBadDebt(), vTokenBalances(), getPoolDataFromVenusPool(), vTokenMetadata(), vTokenMetadataAll(), vTokenUnderlyingPrice()
12:   contract PoolLens is ExponentialNoError {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Lens/PoolLens.sol#L12

File: contracts/Pool/PoolRegistryInterface.sol

/// @audit getAllPools(), getPoolByComptroller(), getVTokenForAsset(), getPoolsSupportedByAsset(), getVenusPoolMetadata()
4:    interface PoolRegistryInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistryInterface.sol#L4

File: contracts/Pool/PoolRegistry.sol

/// @audit initialize(), setProtocolShareReserve(), setShortfallContract(), createRegistryPool(), addMarket(), setPoolName(), updatePoolMetadata()
25:   contract PoolRegistry is Ownable2StepUpgradeable, AccessControlledV8, PoolRegistryInterface {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L25

File: contracts/Rewards/RewardsDistributor.sol

/// @audit initialize(), initializeMarket(), distributeBorrowerRewardToken(), updateRewardTokenSupplyIndex(), grantRewardToken(), updateRewardTokenBorrowIndex(), setRewardTokenSpeeds(), setContributorRewardTokenSpeed(), distributeSupplierRewardToken(), claimRewardToken(), setMaxLoopsLimit(), updateContributorRewards(), claimRewardToken(), getBlockNumber()
12:   contract RewardsDistributor is ExponentialNoError, Ownable2StepUpgradeable, AccessControlledV8, MaxLoopsLimitHelper {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L12

File: contracts/RiskFund/IRiskFund.sol

/// @audit swapPoolsAssets(), transferReserveForAuction(), updateAssetsState(), poolReserves()
4:    interface IRiskFund {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/IRiskFund.sol#L4

File: contracts/RiskFund/ProtocolShareReserve.sol

/// @audit initialize(), setPoolRegistry(), releaseFunds()
12:   contract ProtocolShareReserve is Ownable2StepUpgradeable, ExponentialNoError, ReserveHelpers, IProtocolShareReserve {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L12

File: contracts/RiskFund/ReserveHelpers.sol

/// @audit getPoolAssetReserve(), updateAssetsState()
9:    contract ReserveHelpers {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L9

File: contracts/RiskFund/RiskFund.sol

/// @audit initialize(), setPoolRegistry(), setShortfallContractAddress(), setPancakeSwapRouter(), setMinAmountToConvert(), setMaxLoopsLimit()
19:   contract RiskFund is

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L19

File: contracts/Shortfall/Shortfall.sol

/// @audit initialize(), placeBid(), closeAuction(), startAuction(), restartAuction(), updateNextBidderBlockLimit(), updateIncentiveBps(), updateMinimumPoolBadDebt(), updateWaitForFirstBidder(), updatePoolRegistry()
17:   contract Shortfall is Ownable2StepUpgradeable, AccessControlledV8, ReentrancyGuardUpgradeable, IShortfall {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L17

File: contracts/VTokenInterfaces.sol

/// @audit mintBehalf(), healBorrow(), forceLiquidateBorrow(), sweepToken(), setReserveFactor(), reduceReserves(), setInterestRateModel(), addReserves()
133:  abstract contract VTokenInterface is VTokenStorage {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L133

File: contracts/VToken.sol

/// @audit initialize(), setProtocolSeizeShare(), badDebtRecovered(), setProtocolShareReserve(), setShortfallContract()
19:   contract VToken is

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L19

[G‑13] Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false to true, after having been true in the past

There are 3 instances of this issue:

File: contracts/ComptrollerStorage.sol

95:       mapping(address => mapping(Action => bool)) internal _actionPaused;

101:      mapping(address => bool) internal rewardsDistributorExists;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ComptrollerStorage.sol#L95

File: contracts/VTokenInterfaces.sol

25:       bool internal _notEntered;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L25

[G‑14] >= costs less gas than >

The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas

There is one instance of this issue:

File: contracts/VToken.sol

946:          uint256 repayAmountFinal = repayAmount > accountBorrowsPrev ? accountBorrowsPrev : repayAmount;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L946

[G‑15] internal functions not called by the contract should be removed to save deployment gas

If the functions are required by an interface, the contract should inherit from that interface and use the override keyword

There are 2 instances of this issue:

File: contracts/Comptroller.sol

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L1381

[G‑16] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

There is one instance of this issue:

File: contracts/Pool/PoolRegistry.sol

399:          _numberOfPools++;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L399

[G‑17] Splitting require() statements that use && saves gas

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper by 3 gas

There are 4 instances of this issue:

File: contracts/Comptroller.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L842

File: contracts/Rewards/RewardsDistributor.sol

204           require(
205               numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length,
206               "RewardsDistributor::setRewardTokenSpeeds invalid input"
207:          );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L204-L207

File: contracts/Shortfall/Shortfall.sol

213           require(
214               block.number > auction.highestBidBlock + nextBidderBlockLimit && auction.highestBidder != address(0),
215               "waiting for next bidder. cannot close auction"
216:          );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L213-L216

File: contracts/VToken.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L1365

[G‑18] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

There are 8 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

23:       uint256 public constant blocksPerYear = 10512000;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L23

File: contracts/ErrorReporter.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ErrorReporter.sol#L5

File: contracts/InterestRateModel.sol

10:       bool public constant isInterestRateModel = true;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/InterestRateModel.sol#L10

File: contracts/Rewards/RewardsDistributor.sol

29:       uint224 public constant rewardTokenInitialIndex = 1e36;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L29

File: contracts/VTokenInterfaces.sol

142:      bool public constant isVToken = true;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VTokenInterfaces.sol#L142

File: contracts/WhitePaperInterestRateModel.sol

17:       uint256 public constant blocksPerYear = 2102400;

22:       uint256 public immutable multiplierPerBlock;

27:       uint256 public immutable baseRatePerBlock;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L17

[G‑19] Division by two should use bit shifting

<x> / 2 is the same as <x> >> 1. While the compiler uses the SHR opcode to accomplish both, the version that uses division incurs an overhead of 20 gas due to JUMPs to and from a compiler utility function that introduces checks which can be avoided by using unchecked {} around the division by two

There is one instance of this issue:

File: contracts/ExponentialNoError.sol

22:       uint256 internal constant halfExpScale = expScale / 2;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L22

[G‑20] Stack variable used as a cheaper cache for a state variable is only used once

If the variable is only accessed once, it's cheaper to use the state variable directly that one time, and save the 3 gas the extra stack assignment would spend

There are 10 instances of this issue:

File: contracts/MaxLoopsLimitHelper.sol

28:           uint256 oldMaxLoopsLimit = maxLoopsLimit;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/MaxLoopsLimitHelper.sol#L28

File: contracts/Pool/PoolRegistry.sol

434:          address oldProtocolShareReserve = protocolShareReserve;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L434

File: contracts/RiskFund/RiskFund.sol

117:          address oldShortfallContractAddress = shortfall;

128:          address oldPancakeSwapRouter = pancakeSwapRouter;

140:          uint256 oldMinAmountToConvert = minAmountToConvert;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L117

File: contracts/Shortfall/Shortfall.sol

296:          uint256 oldNextBidderBlockLimit = nextBidderBlockLimit;

310:          uint256 oldIncentiveBps = incentiveBps;

323:          uint256 oldMinimumPoolBadDebt = minimumPoolBadDebt;

336:          uint256 oldWaitForFirstBidder = waitForFirstBidder;

350:          address oldPoolRegistry = poolRegistry;

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L296

[G‑21] require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas*) in a function that may ultimately revert in the unhappy case.

There are 12 instances of this issue:

File: contracts/Comptroller.sol

/// @audit expensive op on line 703
704:          require(closeFactorMaxMantissa >= newCloseFactorMantissa, "Close factor greater than maximum close factor");

/// @audit expensive op on line 703
705:          require(closeFactorMinMantissa <= newCloseFactorMantissa, "Close factor smaller than minimum close factor");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L704

File: contracts/Pool/PoolRegistry.sol

/// @audit expensive op on line 223
225:          require(beaconAddress != address(0), "PoolRegistry: Invalid Comptroller beacon address.");

/// @audit expensive op on line 223
226:          require(priceOracle != address(0), "PoolRegistry: Invalid PriceOracle address.");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L225

File: contracts/RiskFund/ReserveHelpers.sol

/// @audit expensive op on line 39
40:           require(asset != address(0), "ReserveHelpers: Asset address invalid");

/// @audit expensive op on line 51
52:           require(asset != address(0), "ReserveHelpers: Asset address invalid");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L40

File: contracts/RiskFund/RiskFund.sol

/// @audit expensive op on line 138
139:          require(minAmountToConvert_ > 0, "Risk Fund: Invalid min amount to convert");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L139

File: contracts/Shortfall/Shortfall.sol

/// @audit expensive op on line 138
139:          require(minimumPoolBadDebt_ != 0, "invalid minimum pool bad debt");

/// @audit expensive op on line 162
163:          require(bidBps <= MAX_BPS, "basis points cannot be more than 10000");

/// @audit expensive op on line 294
295:          require(_nextBidderBlockLimit != 0, "_nextBidderBlockLimit must not be 0");

/// @audit expensive op on line 308
309:          require(_incentiveBps != 0, "incentiveBps must not be 0");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L139

File: contracts/VToken.sol

/// @audit expensive op on line 489
490:          require(recoveredAmount_ <= badDebt, "more than bad debt recovered from auction");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L490

[G‑22] Use custom errors rather than revert()/require() strings to save gas

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

There are 99 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L72

File: contracts/Comptroller.sol

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

692:              require(borrowBalance == 0, "Nonzero borrow balance after liquidation");

704:          require(closeFactorMaxMantissa >= newCloseFactorMantissa, "Close factor greater than maximum close factor");

705:          require(closeFactorMinMantissa <= newCloseFactorMantissa, "Close factor smaller than minimum close factor");

780:          require(newLiquidationIncentiveMantissa >= 1e18, "liquidation incentive should be greater than 1e18");

808:          require(vToken.isVToken(), "Comptroller: Invalid vToken"); // Sanity check to make sure its really a VToken

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

866:          require(vTokensCount != 0, "invalid number of markets");

867:          require(vTokensCount == newSupplyCaps.length, "invalid number of markets");

928:          require(!rewardsDistributorExists[address(_rewardsDistributor)], "already exists");

934               require(
935                   rewardToken != address(_rewardsDistributor.rewardToken()),
936                   "distributor already exists with this reward"
937:              );

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L128

File: contracts/ExponentialNoError.sol

64:           require(n < 2**224, errorMessage);

69:           require(n < 2**32, errorMessage);

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/ExponentialNoError.sol#L64

File: contracts/MaxLoopsLimitHelper.sol

26:           require(limit > maxLoopsLimit, "Comptroller: Invalid maxLoopsLimit");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/MaxLoopsLimitHelper.sol#L26

File: contracts/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");

263           require(
264               _vTokens[input.comptroller][input.asset] == address(0),
265               "PoolRegistry: Market already added for asset comptroller combination"
266:          );

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

440:          require(bytes(name).length <= 100, "Pool's name is too large");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L225

File: contracts/Proxy/UpgradeableBeacon.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Proxy/UpgradeableBeacon.sol#L8

File: contracts/Rewards/RewardsDistributor.sol

99:           require(address(comptroller) == msg.sender, "Only comptroller can call this function");

183:          require(amountLeft == 0, "insufficient rewardToken for grant");

204           require(
205               numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length,
206               "RewardsDistributor::setRewardTokenSpeeds invalid input"
207:          );

284:              require(comptroller.isMarketListed(vToken), "market must be listed");

309:          require(comptroller.isMarketListed(vToken), "rewardToken market is not listed");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L99

File: contracts/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");

72:           require(amount <= poolsAssetsReserves[comptroller][asset], "ProtocolShareReserve: Insufficient pool balance");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L40

File: contracts/RiskFund/ReserveHelpers.sol

39:           require(ComptrollerInterface(comptroller).isComptroller(), "ReserveHelpers: Comptroller address invalid");

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

51:           require(ComptrollerInterface(comptroller).isComptroller(), "ReserveHelpers: Comptroller address invalid");

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

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

54            require(
55                PoolRegistryInterface(poolRegistry).getVTokenForAsset(comptroller, asset) != address(0),
56                "ReserveHelpers: The pool doesn't support the asset"
57:           );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ReserveHelpers.sol#L39

File: contracts/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");

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

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

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

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

112           require(
113               IShortfall(shortfallContractAddress_).convertibleBaseAsset() == convertibleBaseAsset,
114               "Risk Fund: Base asset doesn't match"
115:          );

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

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

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

158:          require(markets.length == amountsOutMin.length, "Risk fund: markets and amountsOutMin are unequal lengths");

159:          require(markets.length == paths.length, "Risk fund: markets and paths are unequal lengths");

171:              require(pool.comptroller == comptroller, "comptroller doesn't exist pool registry");

172:              require(Comptroller(comptroller).isMarketListed(vToken), "market is not listed");

191:          require(msg.sender == shortfall, "Risk fund: Only callable by Shortfall contract");

192:          require(amount <= poolReserves[comptroller], "Risk Fund: Insufficient pool reserve.");

231:          require(amountOutMin != 0, "RiskFund: amountOutMin must be greater than 0 to swap vToken");

232:          require(amountOutMin >= minAmountToConvert, "RiskFund: amountOutMin should be greater than minAmountToConvert");

253:                      require(path[0] == underlyingAsset, "RiskFund: swap path must start with the underlying asset");

254                       require(
255                           path[path.length - 1] == convertibleBaseAsset,
256                           "RiskFund: finally path must be convertible base asset"
257:                      );

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L80

File: contracts/Shortfall/Shortfall.sol

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

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

139:          require(minimumPoolBadDebt_ != 0, "invalid minimum pool bad debt");

161:          require(_isStarted(auction), "no on-going auction");

162:          require(!_isStale(auction), "auction is stale, restart it");

163:          require(bidBps <= MAX_BPS, "basis points cannot be more than 10000");

164           require(
165               (auction.auctionType == AuctionType.LARGE_POOL_DEBT &&
166                   ((auction.highestBidder != address(0) && bidBps > auction.highestBidBps) ||
167                       (auction.highestBidder == address(0) && bidBps >= auction.startBidBps))) ||
168                   (auction.auctionType == AuctionType.LARGE_RISK_FUND &&
169                       ((auction.highestBidder != address(0) && bidBps < auction.highestBidBps) ||
170                           (auction.highestBidder == address(0) && bidBps <= auction.startBidBps))),
171               "your bid is not the highest"
172:          );

212:          require(_isStarted(auction), "no on-going auction");

213           require(
214               block.number > auction.highestBidBlock + nextBidderBlockLimit && auction.highestBidder != address(0),
215               "waiting for next bidder. cannot close auction"
216:          );

278:          require(_isStarted(auction), "no on-going auction");

279:          require(_isStale(auction), "you need to wait for more time for first bidder");

295:          require(_nextBidderBlockLimit != 0, "_nextBidderBlockLimit must not be 0");

309:          require(_incentiveBps != 0, "incentiveBps must not be 0");

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

361:          require(pool.comptroller == comptroller, "comptroller doesn't exist pool registry");

364           require(
365               (auction.startBlock == 0 && auction.status == AuctionStatus.NOT_STARTED) ||
366                   auction.status == AuctionStatus.ENDED,
367               "auction is on-going"
368:          );

401:          require(poolBadDebt >= minimumPoolBadDebt, "pool bad debt is too low");

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L137

File: contracts/VToken.sol

34:           require(_notEntered, "re-entered");

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

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

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

489:          require(msg.sender == shortfall, "only shortfall contract can update bad debt");

490:          require(recoveredAmount_ <= badDebt, "more than bad debt recovered from auction");

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

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

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

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

650:          require(currentAllowance >= subtractedValue, "decreased allowance below zero");

696:          require(borrowRateMantissa <= borrowRateMaxMantissa, "borrow rate is absurdly high");

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

1072:         require(amountSeizeError == NO_ERROR, "LIQUIDATE_COMPTROLLER_CALCULATE_AMOUNT_SEIZE_FAILED");

1075:         require(vTokenCollateral.balanceOf(borrower) >= seizeTokens, "LIQUIDATE_SEIZE_TOO_MUCH");

1141:         require(newComptroller.isComptroller(), "marker method returned false");

1256:         require(newInterestRateModel.isInterestRateModel(), "marker method returned false");

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

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L34

[G‑23] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

There are 22 instances of this issue:

File: contracts/Comptroller.sol

927:      function addRewardsDistributor(RewardsDistributor _rewardsDistributor) external onlyOwner {

961:      function setPriceOracle(PriceOracle newOracle) external onlyOwner {

973:      function setMaxLoopsLimit(uint256 limit) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L927

File: contracts/Pool/PoolRegistry.sol

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

198:      function setShortfallContract(Shortfall shortfall_) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L188

File: contracts/Rewards/RewardsDistributor.sol

125:      function initializeMarket(address vToken) external onlyComptroller {

163       function distributeBorrowerRewardToken(
164           address vToken,
165           address borrower,
166           Exp memory marketBorrowIndex
167:      ) external onlyComptroller {

171:      function updateRewardTokenSupplyIndex(address vToken) external onlyComptroller {

181:      function grantRewardToken(address recipient, uint256 amount) external onlyOwner {

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

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

233:      function distributeSupplierRewardToken(address vToken, address supplier) external onlyComptroller {

249:      function setMaxLoopsLimit(uint256 limit) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L125

File: contracts/RiskFund/ProtocolShareReserve.sol

53:       function setPoolRegistry(address _poolRegistry) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L53

File: contracts/RiskFund/RiskFund.sol

99:       function setPoolRegistry(address _poolRegistry) external onlyOwner {

110:      function setShortfallContractAddress(address shortfallContractAddress_) external onlyOwner {

126:      function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner {

205:      function setMaxLoopsLimit(uint256 limit) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L99

File: contracts/Shortfall/Shortfall.sol

348:      function updatePoolRegistry(address _poolRegistry) external onlyOwner {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L348

File: contracts/VToken.sol

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

515:      function setShortfallContract(address shortfall_) external onlyOwner {

1350      function _initialize(
1351          address underlying_,
1352          ComptrollerInterface comptroller_,
1353          InterestRateModel interestRateModel_,
1354          uint256 initialExchangeRateMantissa_,
1355          string memory name_,
1356          string memory symbol_,
1357          uint8 decimals_,
1358          address admin_,
1359          address accessControlManager_,
1360          RiskManagementInit memory riskManagement,
1361          uint256 reserveFactorMantissa_
1362:     ) internal onlyInitializing {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L505

[G‑24] Constructors can be marked payable

Payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided. A constructor can safely be marked as payable, since only the deployer would be able to pass funds, and the project itself would not pass any funds.

There are 11 instances of this issue:

File: contracts/BaseJumpRateModelV2.sol

65        constructor(
66            uint256 baseRatePerYear,
67            uint256 multiplierPerYear,
68            uint256 jumpMultiplierPerYear,
69            uint256 kink_,
70:           IAccessControlManagerV8 accessControlManager_

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/BaseJumpRateModelV2.sol#L65-L70

File: contracts/Comptroller.sol

127:      constructor(address poolRegistry_) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Comptroller.sol#L127

File: contracts/JumpRateModelV2.sol

12        constructor(
13            uint256 baseRatePerYear,
14            uint256 multiplierPerYear,
15            uint256 jumpMultiplierPerYear,
16            uint256 kink_,
17            IAccessControlManagerV8 accessControlManager_
18        )
19:           BaseJumpRateModelV2(baseRatePerYear, multiplierPerYear, jumpMultiplierPerYear, kink_, accessControlManager_)

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/JumpRateModelV2.sol#L12-L19

File: contracts/Pool/PoolRegistry.sol

150       constructor() {
151           // Note that the contract is upgradeable. Use initialize() or reinitializers
152           // to set the state variables.
153:          _disableInitializers();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Pool/PoolRegistry.sol#L150-L153

File: contracts/Proxy/UpgradeableBeacon.sol

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

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Proxy/UpgradeableBeacon.sol#L7

File: contracts/Rewards/RewardsDistributor.sol

104       constructor() {
105:          _disableInitializers();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Rewards/RewardsDistributor.sol#L104-L105

File: contracts/RiskFund/ProtocolShareReserve.sol

28        constructor() {
29            // Note that the contract is upgradeable. Use initialize() or reinitializers
30            // to set the state variables.
31:           _disableInitializers();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/ProtocolShareReserve.sol#L28-L31

File: contracts/RiskFund/RiskFund.sol

61        constructor() {
62:           _disableInitializers();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/RiskFund/RiskFund.sol#L61-L62

File: contracts/Shortfall/Shortfall.sol

118       constructor() {
119           // Note that the contract is upgradeable. Use initialize() or reinitializers
120           // to set the state variables.
121:          _disableInitializers();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/Shortfall/Shortfall.sol#L118-L121

File: contracts/VToken.sol

41        constructor() {
42            // Note that the contract is upgradeable. Use initialize() or reinitializers
43            // to set the state variables.
44:           _disableInitializers();

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/VToken.sol#L41-L44

File: contracts/WhitePaperInterestRateModel.sol

36:       constructor(uint256 baseRatePerYear, uint256 multiplierPerYear) {

https://github.com/code-423n4/2023-05-venus/blob/9853f6f4fe906b635e214b22de9f627c6a17ba5b/contracts/WhitePaperInterestRateModel.sol#L36

@CloudEllie
Copy link
Author

Comments from judge:

Judge's notes:

Issue Instances Risk level
[M‑01] The owner is a single point of failure and a centralization risk 16 M
[L‑01] Division by zero not prevented 1 L
[L‑02] Loss of precision 14 L
[L‑03] require() should be used instead of assert() 1 R
[L‑04] safeApprove() is deprecated 4
[L‑05] Missing checks for address(0x0) when assigning values to address state variables 1 L
[L‑06] Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions 8 L
[L‑07] External calls in an un-bounded for-loop may result in a DOS 4 L
[N‑01] Consider disabling renounceOwnership() 7 NC
[N‑02] Events are missing sender information 12
[N‑03] Variables need not be initialized to zero 2 R
[N‑04] Imports could be organized more systematically 4 NC
[N‑05] Large numeric literals should use underscores for readability 2 R
[N‑06] Constants in comparisons should appear on the left side 45 NC
[N‑07] Variable names don't follow the Solidity style guide 17 NC
[N‑08] else-block not required 2
[N‑09] Events may be emitted out of order due to reentrancy 5 NC
[N‑10] if-statement can be converted to a ternary 1
[N‑11] Consider using named mappings 33 R
[N‑12] Import declarations should import specific identifiers, rather than the whole file 96 NC
[N‑13] Adding a return statement when the function defines a named return variable, is redundant 2 R
[N‑14] public functions not called by the contract should be declared external instead 2 R
[N‑15] 2**<n> - 1 should be re-written as type(uint<n>).max 2 R
[N‑16] constants should be defined rather than using magic numbers 11 R
[N‑17] Events that mark critical parameter changes should contain both the old and the new value 5 NC
[N‑18] Constant redefined elsewhere 1 NC
[N‑19] Inconsistent spacing in comments 3 NC
[N‑20] Lines are too long 22 NC
[N‑21] Typos 10 NC
[N‑22] File is missing NatSpec 10 NC
[N‑23] NatSpec @param is missing 47 NC
[N‑24] NatSpec @return argument is missing 27 NC
[N‑25] Event is not properly indexed 12 R
[N‑26] Not using the named return variables anywhere in the function is confusing 14 N-13
[N‑27] Duplicated require()/revert() checks should be refactored to a modifier or function 5 R
[N‑28] Interfaces should be indicated with an I prefix in the contract name 3 NC
[N‑29] Numeric values having to do with time should use time units for readability 2 NC
[N‑30] Consider using delete rather than assigning zero/false to clear values 8 NC
[N‑31] Contracts should have full test coverage 1
[N‑32] Large or complicated code bases should implement invariant tests 1
[N‑33] internal functions not called by the contract should be removed 2 NC

Gas Optimizations

Issue Instances Total Gas Saved
[G‑01] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate 3 -
[G‑02] Structs can be packed into fewer storage slots 3 -
[G‑03] Using storage instead of memory for structs/arrays saves gas 6 25200
[G‑04] State variables should be cached in stack variables rather than re-reading them from storage 22 2134
[G‑05] Multiple accesses of a mapping/array should use a local variable cache 13 546
[G‑06] The result of function calls should be cached rather than re-calling the function 1 -
[G‑07] internal functions only called once can be inlined to save gas 19 380
[G‑08] Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement 3 255
[G‑09] <array>.length should not be looked up in every loop of a for-loop 3 9
[G‑10] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops 37 2220
[G‑11] require()/revert() strings longer than 32 bytes cost extra gas 61 -
[G‑12] Optimize names to save gas 16 352
[G‑13] Using bools for storage incurs overhead 3 51300
[G‑14] >= costs less gas than > 1 3
[G‑15] internal functions not called by the contract should be removed to save deployment gas 2 -
[G‑16] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 1 5
[G‑17] Splitting require() statements that use && saves gas 4 12
[G‑18] Using private rather than public for constants, saves gas 8 -
[G‑19] Division by two should use bit shifting 1 20
[G‑20] Stack variable used as a cheaper cache for a state variable is only used once 10 30
[G‑21] require() or revert() statements that check input arguments should be at the top of the function 12 -
[G‑22] Use custom errors rather than revert()/require() strings to save gas 99 -
[G‑23] Functions guaranteed to revert when called by normal users can be marked payable 22 462
[G‑24] Constructors can be marked payable 11 231

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment