Venus Protocol Isolated Pools - tnevler's results

Non-Critical Issues

[N-1]: Variables names are inconsistent

Context:

1. emit BadDebtRecovered(badDebtOld, badDebtNew); L496

Description:

in all contracts more often "old" and "new" placed at the beginning of variable names. Examples:

1. emit NewLiquidationIncentive(oldLiquidationIncentiveMantissa, newLiquidationIncentiveMantissa); L791
2. emit NewMinLiquidatableCollateral(oldMinLiquidatableCollateral, newMinLiquidatableCollateral); L917
3. emit NewProtocolSeizeShare(oldProtocolSeizeShareMantissa, newProtocolSeizeShareMantissa_); L319

Recommendation:

Change variable name of "badDebtOld" and "badDebtNew" to "oldBadDebt" and "newBadDebt".

[N-2]: Move validation statements to the top of the function when validating input parameters

Context:

1. require(newComptroller.isComptroller(), "marker method returned false"); L1141

[N-3]: Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

Context:

1. uint256 private constant MAX_BPS = 10000; L60
2. incentiveBps = 1000; L149

Description:

Scientific notation should be used for better code readability.

[N-4]: No same value input check

Context:

1. function setCloseFactor(uint256 newCloseFactorMantissa) external { L702
2. function setLiquidationIncentive(uint256 newLiquidationIncentiveMantissa) external { L779
3. function setMinLiquidatableCollateral(uint256 newMinLiquidatableCollateral) external { L912
4. function setPriceOracle(PriceOracle newOracle) external onlyOwner { L961
5. function _setComptroller(ComptrollerInterface newComptroller) internal { L1138
6. function _setShortfallContract(address shortfall_) internal { L1398
7. function _setProtocolShareReserve(address payable protocolShareReserve_) internal { L1407
8. function _setShortfallContract(Shortfall shortfall_) internal { L421
9. function _setProtocolShareReserve(address payable protocolShareReserve_) internal { L430
10. function setPoolRegistry(address _poolRegistry) external onlyOwner { L99
11. function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner { L126
12. function setMinAmountToConvert(uint256 minAmountToConvert_) external { L137
13. function setPoolRegistry(address _poolRegistry) external onlyOwner { L53
14. function _setMaxLoopsLimit(uint256 limit) internal { L25

Recommendation:

Example how to fix require(_newOwner != owner, " Same address");

[N-5]: Add a timelock to critical functions

Context:

1. function setCloseFactor(uint256 newCloseFactorMantissa) external { L702
2. function setLiquidationIncentive(uint256 newLiquidationIncentiveMantissa) external { L779
3. function setMinLiquidatableCollateral(uint256 newMinLiquidatableCollateral) external { L912
4. function setPriceOracle(PriceOracle newOracle) external onlyOwner { L961
5. function _setComptroller(ComptrollerInterface newComptroller) internal { L1138
6. function _setShortfallContract(address shortfall_) internal { L1398
7. function _setProtocolShareReserve(address payable protocolShareReserve_) internal { L1407
8. function _setShortfallContract(Shortfall shortfall_) internal { L421
9. function _setProtocolShareReserve(address payable protocolShareReserve_) internal { L430
10. function setPoolRegistry(address _poolRegistry) external onlyOwner { L99
11. function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner { L126
12. function setMinAmountToConvert(uint256 minAmountToConvert_) external { L137
13. function setPoolRegistry(address _poolRegistry) external onlyOwner { L53
14. function _setMaxLoopsLimit(uint256 limit) internal { L25

Description:

It is a good practice to give time for users to react and adjust to critical changes. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user).

[N-6]: Change imports

Context:

All contracts.

Recommendation:

Example: Instead of import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

You can do your imports like this: import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

[N-7]: Missing leading underscores

Context:

1. function updateMarketBorrowIndex( L453
2. function updateMarketSupplyIndex( L475
3. function calculateBorrowerReward( L495
4. function calculateSupplierReward( L516
5. IRiskFund private riskFund; L51
6. uint256 private incentiveBps; L57
7. uint256 private constant MAX_BPS = 10000; L60
8. Comptroller private comptroller; L52
9. address private pancakeSwapRouter; L29
10. uint256 private minAmountToConvert; L30
11. address private convertibleBaseAsset; L31
12. address private shortfall; L32
13. uint256 internal constant borrowRateMaxMantissa = 0.0005e16; L53
14. uint256 internal constant reserveFactorMaxMantissa = 1e18; L56
15. uint256 internal initialExchangeRateMantissa; L69
16. mapping(address => uint256) internal accountTokens; L107
17. mapping(address => mapping(address => uint256)) internal transferAllowances; L110
18. mapping(address => BorrowSnapshot) internal accountBorrows; L113
19. uint256 internal constant expScale = 1e18; L20
20. uint256 internal constant doubleScale = 1e36; L21
21. uint256 internal constant halfExpScale = expScale / 2; L22
22. uint256 internal constant mantissaOne = expScale; L23
23. function truncate(Exp memory exp) internal pure returns (uint256) { L29
24. function mul_ScalarTruncate(Exp memory a, uint256 scalar) internal pure returns (uint256) { L38
25. function mul_ScalarTruncateAddUInt( L47
26. function lessThanExp(Exp memory left, Exp memory right) internal pure returns (bool) { L59
27. function safe224(uint256 n, string memory errorMessage) internal pure returns (uint224) { L63
28. function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) { L68
29. function add_(Exp memory a, Exp memory b) internal pure returns (Exp memory) { L73
30. function add_(Double memory a, Double memory b) internal pure returns (Double memory) { L77
31. function add_(uint256 a, uint256 b) internal pure returns (uint256) { L81
32. function sub_(Exp memory a, Exp memory b) internal pure returns (Exp memory) { L85
33. function sub_(Double memory a, Double memory b) internal pure returns (Double memory) { L89
34. function sub_(uint256 a, uint256 b) internal pure returns (uint256) { L93
35. function mul_(Exp memory a, Exp memory b) internal pure returns (Exp memory) { L97
36. function mul_(Exp memory a, uint256 b) internal pure returns (Exp memory) { L101
37. function mul_(uint256 a, Exp memory b) internal pure returns (uint256) { L105
38. function mul_(Double memory a, Double memory b) internal pure returns (Double memory) { L109
39. function mul_(Double memory a, uint256 b) internal pure returns (Double memory) { L113
40. function mul_(uint256 a, Double memory b) internal pure returns (uint256) { L117
41. function mul_(uint256 a, uint256 b) internal pure returns (uint256) { L121
42. function div_(Exp memory a, Exp memory b) internal pure returns (Exp memory) { L125
43. function div_(Exp memory a, uint256 b) internal pure returns (Exp memory) { L129
44. function div_(uint256 a, Exp memory b) internal pure returns (uint256) { L133
45. function div_(Double memory a, Double memory b) internal pure returns (Double memory) { L137
46. function div_(Double memory a, uint256 b) internal pure returns (Double memory) { L141
47. function div_(uint256 a, Double memory b) internal pure returns (uint256) { L145
48. function div_(uint256 a, uint256 b) internal pure returns (uint256) { L149
49. function fraction(uint256 a, uint256 b) internal pure returns (Double memory) { L153
50. uint256 private constant BASE = 1e18; L13
51. RewardsDistributor[] internal rewardsDistributors; L98
52. mapping(address => bool) internal rewardsDistributorExists; L101
53. uint256 internal constant NO_ERROR = 0; L103
54. uint256 internal constant closeFactorMinMantissa = 0.05e18; // 0.05 L106
55. uint256 internal constant closeFactorMaxMantissa = 0.9e18; // 0.9 L109
56. uint256 internal constant collateralFactorMaxMantissa = 0.9e18; // 0.9 L112
57. address private protocolIncome; L15
58. address private riskFund; L16
59. uint256 private constant protocolSharePercentage = 70; L18
60. uint256 private constant baseUnit = 100; L19
61. uint256 private constant BASE = 1e18; L12
62. mapping(address => uint256) internal assetsReserves; L13
63. mapping(address => mapping(address => uint256)) internal poolsAssetsReserves; L17
64. address internal poolRegistry; L20

Description:

Internal and private functions, state variables, constants, and immutables should starting with an underscore.

[N-8]: Emit events in initialize() functions

Context:

1. function initialize(uint256 loopLimit, address accessControlManager) external initializer { L138
2. function initialize( L59
3. function initialize( L131
4. function initialize( L111
5. function initialize( L164
6. function initialize( L73
7. function initialize(address _protocolIncome, address _riskFund) external initializer { L39

Recommendation:

Define an initialize event and emit that at the end of initialize() function.

[N-9]: Event is never emitted

Context:

1. event AmountOutMinUpdated(uint256 oldAmountOutMin, uint256 newAmountOutMin); L47

Description:

There is the event defined that is never emitted and it can be removed.