Venus Prime - 0xhacksmithh's results

Earn, borrow & lend on the #1 Decentralized Money Market on the BNB chain.

General Information

Platform: Code4rena

Start Date: 28/09/2023

Pot Size: $36,500 USDC

Total HM: 5

Participants: 115

Period: 6 days

Judge: 0xDjango

Total Solo HM: 1

Id: 290

League: ETH

Venus Protocol

Findings Distribution

Researcher Performance

Rank: 50/115

Findings: 1

Award: $123.75

Gas:
grade-a

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: DavidGiladi

Also found by: 0x3b, 0xWaitress, 0xhacksmithh, 0xprinc, hihen, jkoppel, lsaudit, oakcobalt, pavankv, pontifex

Labels

bug
G (Gas Optimization)
grade-a
sufficient quality report
edited-by-warden
G-01

Awards

123.7515 USDC - $123.75

External Links

Codebase Optimization Report

Table Of Contents

[G-01]Using immutable on variables that are only set in the constructor and never after

Gas per instance: 2.1K Total Instances: 7 Total Gas Saved: 2.1 * 7 = 14700 Gas

function initialize(
        address _xvsVault,
        address _xvsVaultRewardToken,
        uint256 _xvsVaultPoolId,
        uint128 _alphaNumerator,
        uint128 _alphaDenominator,
        address _accessControlManager,
        address _protocolShareReserve,
        address _primeLiquidityProvider,
        address _comptroller,
        address _oracle,
        uint256 _loopsLimit
    ) external virtual initializer {
        .......
        .......

        alphaNumerator = _alphaNumerator;
        alphaDenominator = _alphaDenominator;
        xvsVaultRewardToken = _xvsVaultRewardToken; // @audit immutable
        xvsVaultPoolId = _xvsVaultPoolId;  // @audit immu
        xvsVault = _xvsVault;  // @audit immu
        nextScoreUpdateRoundId = 0;  // @audit default
        protocolShareReserve = _protocolShareReserve;  // @audit immu
        primeLiquidityProvider = _primeLiquidityProvider;  // @audit imm
        comptroller = _comptroller;  // @audit imm
        oracle = ResilientOracleInterface(_oracle);  // @audit imm

Those state variable present in PrimeStorage.sol contract which will made immutables.

[G-02] Variable should created outside of loops

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L179-L181

File: /contracts/perp-vault/PerpetualAtlanticVaultLP.sol
        for (uint256 i = 0; i < _allMarkets.length; ) {
            address market = _allMarkets[i];
            uint256 interestAccrued = getInterestAccrued(market, user);
            uint256 accrued = interests[market][user].accrued;
+       address market;
+       uint256 interestAccrued;
+       uint256 accrued;

        for (uint256 i = 0; i < _allMarkets.length; ) {
+           market = _allMarkets[i];
+           interestAccrued = getInterestAccrued(market, user);
+           accrued = interests[market][user].accrued;

[Gas-03] address[] storage _allMarkets could made as calldata instead storage in multiple instances

If the data called to function does not need to be changed (like updating values in an array), it can be passed in as calldata.

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L175

File: contracts/Tokens/Prime/Prime.sol
            function getPendingInterests(address user) external returns (PendingInterest[] memory pendingInterests) {
        address[] storage _allMarkets = allMarkets;
    function getPendingInterests(address user) external returns (PendingInterest[] memory pendingInterests) {
-       address[] storage _allMarkets = allMarkets;
+       address[] calldata _allMarkets = allMarkets;

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L608

    function _accrueInterestAndUpdateScore(address user) internal {
-       address[] storage _allMarkets = allMarkets;
+       address[] calldata _allMarkets = allMarkets;

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L728

function _burn(address user) internal {
        if (!tokens[user].exists) revert UserHasNoPrimeToken();
-       address[] storage _allMarkets = allMarkets;
+       address[] calldata _allMarkets = allMarkets;

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L608

    function _accrueInterestAndUpdateScore(address user) internal {
-       address[] storage _allMarkets = allMarkets;
+       address[] calldata _allMarkets = allMarkets;

[Gas-04] Some State Variable could cached in memory with out calling them repeatedly (Those are missed by automated finding reports)

[Gas-04-01] nextScoreUpdateRoundId could cached in memory as it called 3 times via updateScores() function

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L200-L230

  function updateScores(address[] memory users) external {
+       uint256 _nextScoreUpdateRoundId = nextScoreUpdateRoundId;
        if (pendingScoreUpdates == 0) revert NoScoreUpdatesRequired();
-       if (nextScoreUpdateRoundId == 0) revert NoScoreUpdatesRequired();
+       if (_nextScoreUpdateRoundId == 0) revert NoScoreUpdatesRequired();

        for (uint256 i = 0; i < users.length; ) {
            address user = users[i];

            if (!tokens[user].exists) revert UserHasNoPrimeToken();
-           if (isScoreUpdated[nextScoreUpdateRoundId][user]) continue;
+           if (isScoreUpdated[_nextScoreUpdateRoundId][user]) continue;

            address[] storage _allMarkets = allMarkets;
            for (uint256 j = 0; j < _allMarkets.length; ) {
                address market = _allMarkets[j];
                _executeBoost(user, market);
                _updateScore(user, market);

                unchecked {
                    j++;
                }
            }

            pendingScoreUpdates--;
-           isScoreUpdated[nextScoreUpdateRoundId][user] = true;
+           isScoreUpdated[_nextScoreUpdateRoundId][user] = true;

            unchecked {
                i++;
            }

            emit UserScoreUpdated(user);
        }
    }

[Gas-05] If()/require() checks which make state variable or important variable checks should be at top of function.

[Gas-05-01] _ensureMaxLoops(allMarkets.length) which checks all created markets are in limit range or not should at top of addMarket()

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L288-L309

function addMarket(address vToken, uint256 supplyMultiplier, uint256 borrowMultiplier) external {
        _checkAccessAllowed("addMarket(address,uint256,uint256)");
        if (markets[vToken].exists) revert MarketAlreadyExists();
+       _ensureMaxLoops(allMarkets.length + 1);

        bool isMarketExist = InterfaceComptroller(comptroller).markets(vToken);
        if (!isMarketExist) revert InvalidVToken();

        markets[vToken].rewardIndex = 0;
        markets[vToken].supplyMultiplier = supplyMultiplier;
        markets[vToken].borrowMultiplier = borrowMultiplier;
        markets[vToken].sumOfMembersScore = 0;
        markets[vToken].exists = true;

        vTokenForAsset[_getUnderlying(vToken)] = vToken;

        allMarkets.push(vToken);
        _startScoreUpdateRound();

-       _ensureMaxLoops(allMarkets.length);

        emit MarketAdded(vToken, supplyMultiplier, borrowMultiplier);
    }

[Gas-06] To update one limit (irrevocableLimit or revocableLimit) other automatically get override

Each time one limit change other will have to chage(or re-right with same value again), this will increase one extra wright to storage even if this behaviour was not intended. https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L316-324

    function setLimit(uint256 _irrevocableLimit, uint256 _revocableLimit) external {
        _checkAccessAllowed("setLimit(uint256,uint256)");
        if (_irrevocableLimit < totalIrrevocable || _revocableLimit < totalRevocable) revert InvalidLimit();

        emit MintLimitsUpdated(irrevocableLimit, revocableLimit, _irrevocableLimit, _revocableLimit);

        revocableLimit = _revocableLimit;
        irrevocableLimit = _irrevocableLimit;
    }

[Gas-07] Use delete key word instead of setting mapping variables to default

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L375-L376

        } else if (!isAccountEligible && !tokens[user].exists && stakedAt[user] > 0) {
-           stakedAt[user] = 0;
+           delete stakedAt[user];

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L460

        IVToken market = IVToken(vToken);
-       unreleasedPSRIncome[_getUnderlying(address(market))] = 0;
+       delete unreleasedPSRIncome[_getUnderlying(address(market))];

[Gas-08] Use assembly for some more check work can save some extra gas

[Gas-08-01] Use to check msg.sender

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L453

    function updateAssetsState(address _comptroller, address asset) external {
        if (msg.sender != protocolShareReserve) revert InvalidCaller();
        if (comptroller != _comptroller) revert InvalidComptroller();

[Gas-08-02] Use to check against some state variables

inputed _comptroller parameter check against comptroller stored in state variable, this could preform using assembly

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L454

    function updateAssetsState(address _comptroller, address asset) external {
        if (msg.sender != protocolShareReserve) revert InvalidCaller();
        if (comptroller != _comptroller) revert InvalidComptroller();

[Gas-9] Ternary Operator could used when ever possible

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L481-L485

        uint256 totalTimeStaked = block.timestamp - stakedAt[user];
+       (totalTimeStaked < STAKING_PERIOD) ? STAKING_PERIOD - totalTimeStaked : 0;

-       if (totalTimeStaked < STAKING_PERIOD) {
-           return STAKING_PERIOD - totalTimeStaked;
-       } else {
-           return 0;
-       }

[Gas-10] Some Mapping value could be cached into storage

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L629-L633

    function _initializeMarkets(address account) internal {
        address[] storage _allMarkets = allMarkets;
        for (uint256 i = 0; i < _allMarkets.length; ) {
            address market = _allMarkets[i];
            accrueInterest(market);

+           Interset storage _interset = interests[market][account];
-           interests[market][account].rewardIndex = markets[market].rewardIndex;
+           _interset.rewardIndex = markets[market].rewardIndex;

            uint256 score = _calculateScore(market, account);
-           interests[market][account].score = score;
+           _interset.score = score;
            markets[market].sumOfMembersScore = markets[market].sumOfMembersScore + score;

            unchecked {
                i++;
            }
        }
    }

[Gas-11] A extra storage call will be save by caching mathematical operation in memory

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L818-L822

    function _startScoreUpdateRound() internal {
        nextScoreUpdateRoundId++;
        totalScoreUpdatesRequired = totalIrrevocable + totalRevocable;
        pendingScoreUpdates = totalScoreUpdatesRequired;
    }
    function _startScoreUpdateRound() internal {
        nextScoreUpdateRoundId++;
+       uint256 _totalScoreUpdatesRequired = totalIrrevocable + totalRevocable;
-       totalScoreUpdatesRequired = totalIrrevocable + totalRevocable;
+       totalScoreUpdatesRequired = _totalScoreUpdatesRequired;
+       pendingScoreUpdates = _totalScoreUpdatesRequired;
-       pendingScoreUpdates = totalScoreUpdatesRequired;
    }

[Gas-12] Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement (These are missing in automated finding reports)

[Gas-12-01] totalScoreUpdatesRequired-- could be uncheked because of previous if (totalScoreUpdatesRequired > 0) check

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L828

    function _updateRoundAfterTokenBurned(address user) internal {
-       if (totalScoreUpdatesRequired > 0) totalScoreUpdatesRequired--;
+       if (totalScoreUpdatesRequired > 0) unchecked{ totalScoreUpdatesRequired--;}

[Gas-12-02] Due to pendingScoreUpdates > 0 check pendingScoreUpdates-- could be unchecked

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L831

        if (pendingScoreUpdates > 0 && !isScoreUpdated[nextScoreUpdateRoundId][user]) { 
-           pendingScoreUpdates--;
+           unchecked{ pendingScoreUpdates--; }

[Gas-13] address(this) could cached in a immutable state variable

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L564

uint256 totalIncomeUnreleased = IProtocolShareReserve(protocolShareReserve).getUnreleasedFunds(
            comptroller,
            IProtocolShareReserve.Schema.SPREAD_PRIME_CORE,
            address(this),

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L682 https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L686

        if (amount > asset.balanceOf(address(this))) {
            address[] memory assets = new address[](1);
            assets[0] = address(asset);
            IProtocolShareReserve(protocolShareReserve).releaseFunds(comptroller, assets);
            if (amount > asset.balanceOf(address(this))) {
                IPrimeLiquidityProvider(primeLiquidityProvider).releaseFunds(address(asset));
                unreleasedPLPIncome[underlying] = 0;
            }

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L960

          return
            IProtocolShareReserve(protocolShareReserve).getPercentageDistribution(
                address(this),

[Gas-14] struct can be packed more precisely

[Gas-14-01] Market struct could packed more strictly

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L

    struct Market {
        uint256 supplyMultiplier;
        uint256 borrowMultiplier;
        uint256 rewardIndex;
        uint256 sumOfMembersScore;
        bool exists;
    }
    struct Market {
-       uint256 supplyMultiplier;
-       uint256 borrowMultiplier;
-       uint256 rewardIndex;
-       uint256 sumOfMembersScore;
-       bool exists;
 
+       uint128 supplyMultiplier;
+       uint128 borrowMultiplier;
+       uint120 rewardIndex;
+       uint128 sumOfMembersScore;
+       bool exists;
    }

[Gas-15] storage can be packed more precisely

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L49-L58

-   uint256 public totalIrrevocable;

-   uint256 public totalRevocable;

-   uint256 public revocableLimit;

-   uint256 public irrevocableLimit;

+   uint128 public totalIrrevocable;

+   uint128 public totalRevocable;

+   uint128 public revocableLimit;

+   uint128 public irrevocableLimit;

[Gas-16] caonstant should be value not expressions

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L

    /// @notice minimum amount of XVS user needs to stake to become a prime member
-   uint256 public constant MINIMUM_STAKED_XVS = 1000 * EXP_SCALE;
+   uint256 public constant MINIMUM_STAKED_XVS = [Resultant Value here];     // 1000 * EXP_SCALE => comment signifies expression

    /// @notice maximum XVS taken in account when calculating user score
    uint256 public constant MAXIMUM_XVS_CAP = 100000 * EXP_SCALE;

    /// @notice number of days user need to stake to claim prime token
    uint256 public constant STAKING_PERIOD = 90 * 24 * 60 * 60;

#0 - c4-pre-sort

2023-10-07T06:34:39Z

0xRobocop marked the issue as sufficient quality report

#1 - c4-judge

2023-11-03T17:06:15Z

fatherGoose1 marked the issue as grade-a

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter