Venus Protocol Isolated Pools - lukris02's results

QA Report for Venus Protocol Isolated Pools contest

Overview

During the audit, 1 low and 8 non-critical issues were found.

Low Risk Findings(1)

L-1. Add a timelock to critical functions

Description

Giving users time to react and adjust to critical changes in protocol provides more guarantees and increases the transparency of the protocol.

Instances

• function setCloseFactor(uint256 newCloseFactorMantissa) external {
• function setCollateralFactor(
• function setMarketBorrowCaps(VToken[] calldata vTokens, uint256[] calldata newBorrowCaps) external {
• function setMarketSupplyCaps(VToken[] calldata vTokens, uint256[] calldata newSupplyCaps) external {
• function setMinLiquidatableCollateral(uint256 newMinLiquidatableCollateral) external {
• function setPriceOracle(PriceOracle newOracle) external onlyOwner {
• function setMaxLoopsLimit(uint256 limit) external onlyOwner {
• function setReserveFactor(uint256 newReserveFactorMantissa) external override nonReentrant {
• function setInterestRateModel(InterestRateModel newInterestRateModel) external override {
• function setProtocolShareReserve(address payable protocolShareReserve_) external onlyOwner {
• function setShortfallContract(address shortfall_) external onlyOwner {
• function setMaxLoopsLimit(uint256 limit) external onlyOwner {
• function setProtocolShareReserve(address payable protocolShareReserve_) external onlyOwner {
• function setShortfallContract(Shortfall shortfall_) external onlyOwner {
• function setPoolRegistry(address _poolRegistry) external onlyOwner {
• function setShortfallContractAddress(address shortfallContractAddress_) external onlyOwner {
• function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner {
• function setMinAmountToConvert(uint256 minAmountToConvert_) external {
• function setMaxLoopsLimit(uint256 limit) external onlyOwner {
• function setPoolRegistry(address _poolRegistry) external onlyOwner {

Recommendation

Consider adding a timelock.

Non-Critical Risk Findings(8)

NC-1. Update import usages

Description

For modern and more readable code, consider updating import usages.

Instances

• all contracts

Recommendation

Change to: import {contract1 , contract2} from "filename.sol";

NC-2. Scientific notation may be used

Description

For readability and to avoid misprints, it is better to use scientific notation.

Instances

• uint256 private constant MAX_BPS = 10000;
• incentiveBps = 1000;

Recommendation

Replace 10000 with 10e4.

NC-3. Unused event

Description

The event AmountOutMinUpdated is not emitted.

Instances

• event AmountOutMinUpdated(uint256 oldAmountOutMin, uint256 newAmountOutMin);

Recommendation

Emit event or delete it.

NC-4. If possible, place if/require-statements at the top of the function

Description

Validation of input parameters should be at the beginning of the function.

Instances

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

NC-5. Prevent zero transfers

Description

Check that amount to transfer > 0.

Instances

• token.safeTransferFrom(from, address(this), amount);
• token.safeTransfer(to, amount);
• token.safeTransferFrom(from, address(this), amount);
• IERC20Upgradeable(convertibleBaseAsset).safeTransfer(shortfall, amount);

NC-6. Lack of event emission in initialize() function

Description

Lack of event emission complicates recording the init parameters for off-chain monitoring and reduces transparency.

Instances

• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol#L138
• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L59
• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L131
• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Rewards/RewardsDistributor.sol#L111
• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Pool/PoolRegistry.sol#L164
• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/RiskFund.sol#L73
• https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/ProtocolShareReserve.sol#L39

Recommendation

Consider emitting an event in the initialize() function

NC-7. No same value input control

Instances

• function setCloseFactor(uint256 newCloseFactorMantissa) external {
• function setLiquidationIncentive(uint256 newLiquidationIncentiveMantissa) external {
• function setMinLiquidatableCollateral(uint256 newMinLiquidatableCollateral) external {
• function setPriceOracle(PriceOracle newOracle) external onlyOwner {
• function _setComptroller(ComptrollerInterface newComptroller) internal {
• function _setShortfallContract(address shortfall_) internal {
• function _setProtocolShareReserve(address payable protocolShareReserve_) internal {
• function _setShortfallContract(Shortfall shortfall_) internal {
• function _setProtocolShareReserve(address payable protocolShareReserve_) internal {
• function setPoolRegistry(address _poolRegistry) external onlyOwner {
• function setPancakeSwapRouter(address pancakeSwapRouter_) external onlyOwner {
• function setMinAmountToConvert(uint256 minAmountToConvert_) external {
• function setPoolRegistry(address _poolRegistry) external onlyOwner {
• function _setMaxLoopsLimit(uint256 limit) internal {

Recommendation

Check if (variableA == parameterA) revert SameValue();

NC-8. Missing leading underscores

Description

Internal and private state variables, constants, immutables and functions should have a leading underscore.

Instances

• function updateMarketBorrowIndex(
• function updateMarketSupplyIndex(
• function calculateBorrowerReward(
• function calculateSupplierReward(
• IRiskFund private riskFund;
• uint256 private incentiveBps;
• uint256 private constant MAX_BPS = 10000;
• Comptroller private comptroller;
• address private pancakeSwapRouter;
• uint256 private minAmountToConvert;
• address private convertibleBaseAsset;
• address private shortfall;
• uint256 internal constant borrowRateMaxMantissa = 0.0005e16;
• uint256 internal constant reserveFactorMaxMantissa = 1e18;
• uint256 internal initialExchangeRateMantissa;
• mapping(address => uint256) internal accountTokens;
• mapping(address => mapping(address => uint256)) internal transferAllowances;
• mapping(address => BorrowSnapshot) internal accountBorrows;
• uint256 internal constant expScale = 1e18;
• uint256 internal constant doubleScale = 1e36;
• uint256 internal constant halfExpScale = expScale / 2;
• uint256 internal constant mantissaOne = expScale;
• function truncate(Exp memory exp) internal pure returns (uint256) {
• function mul_ScalarTruncate(Exp memory a, uint256 scalar) internal pure returns (uint256) {
• function mul_ScalarTruncateAddUInt(
• function lessThanExp(Exp memory left, Exp memory right) internal pure returns (bool) {
• function safe224(uint256 n, string memory errorMessage) internal pure returns (uint224) {
• function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) {
• function add_(Exp memory a, Exp memory b) internal pure returns (Exp memory) {
• function add_(Double memory a, Double memory b) internal pure returns (Double memory) {
• function add_(uint256 a, uint256 b) internal pure returns (uint256) {
• function sub_(Exp memory a, Exp memory b) internal pure returns (Exp memory) {
• function sub_(Double memory a, Double memory b) internal pure returns (Double memory) {
• function sub_(uint256 a, uint256 b) internal pure returns (uint256) {
• function mul_(Exp memory a, Exp memory b) internal pure returns (Exp memory) {
• function mul_(Exp memory a, uint256 b) internal pure returns (Exp memory) {
• function mul_(uint256 a, Exp memory b) internal pure returns (uint256) {
• function mul_(Double memory a, Double memory b) internal pure returns (Double memory) {
• function mul_(Double memory a, uint256 b) internal pure returns (Double memory) {
• function mul_(uint256 a, Double memory b) internal pure returns (uint256) {
• function mul_(uint256 a, uint256 b) internal pure returns (uint256) {
• function div_(Exp memory a, Exp memory b) internal pure returns (Exp memory) {
• function div_(Exp memory a, uint256 b) internal pure returns (Exp memory) {
• function div_(uint256 a, Exp memory b) internal pure returns (uint256) {
• function div_(Double memory a, Double memory b) internal pure returns (Double memory) {
• function div_(Double memory a, uint256 b) internal pure returns (Double memory) {
• function div_(uint256 a, Double memory b) internal pure returns (uint256) {
• function div_(uint256 a, uint256 b) internal pure returns (uint256) {
• function fraction(uint256 a, uint256 b) internal pure returns (Double memory) {
• uint256 private constant BASE = 1e18;
• RewardsDistributor[] internal rewardsDistributors;
• mapping(address => bool) internal rewardsDistributorExists;
• uint256 internal constant NO_ERROR = 0;
• uint256 internal constant closeFactorMinMantissa = 0.05e18; // 0.05
• uint256 internal constant closeFactorMaxMantissa = 0.9e18; // 0.9
• uint256 internal constant collateralFactorMaxMantissa = 0.9e18; // 0.9
• address private protocolIncome;
• address private riskFund;
• uint256 private constant protocolSharePercentage = 70;
• uint256 private constant baseUnit = 100;
• uint256 private constant BASE = 1e18;
• mapping(address => uint256) internal assetsReserves;
• mapping(address => mapping(address => uint256)) internal poolsAssetsReserves;
• address internal poolRegistry;

Recommendation

Add leading underscores where needed.