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
Rank: 73/115
Findings: 1
Award: $15.69
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: DavidGiladi
Also found by: 0x3b, 0xWaitress, 0xhacksmithh, 0xprinc, hihen, jkoppel, lsaudit, oakcobalt, pavankv, pontifex
15.6862 USDC - $15.69
There are 4 instances.
Using the distributionSpeed
variable for caching is redundant.
233 uint256 distributionSpeed = tokenDistributionSpeeds[token_];
https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/PrimeLiquidityProvider.sol#L233
Using the blockNumber
and initializedBlock
variables for caching is redundant.
288 uint256 blockNumber = getBlockNumber(); 289 uint256 initializedBlock = lastAccruedBlock[token_];
https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/PrimeLiquidityProvider.sol#L288-L298
Using the lastBlockAccrued
variable for caching is redundant.
333 uint256 lastBlockAccrued = lastAccruedBlock[token_];
https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/PrimeLiquidityProvider.sol#L333 PrimeLiquidityProvider.sol
Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time the variable is used, which wastes some gas. There are 3 instances:
34 uint256 public constant MINIMUM_STAKED_XVS = 1000 * EXP_SCALE; 37 uint256 public constant MAXIMUM_XVS_CAP = 100000 * EXP_SCALE; 40 uint256 public constant STAKING_PERIOD = 90 * 24 * 60 * 60;
These functions do not read from storage. They can therefore be changed from view to pure to save on gas. There are __ instances:
854 function _xvsBalanceForScore(uint256 xvs) internal view returns (uint256) { 855 if (xvs > MAXIMUM_XVS_CAP) { 856 return MAXIMUM_XVS_CAP; 857 } else { 858 return xvs; 859 } 860 }
904 function isEligible(uint256 amount) internal view returns (bool) { 905 if (amount >= MINIMUM_STAKED_XVS) { 906 return true; 907 } 908 909 return false; 910 }
The _
check is the same as the _
modifier.
The _ensureTokenInitialized
check is called twice during the setTokensDistributionSpeed
function execution: at the line L#162 and again at the line L#252 (_setTokenDistributionSpeed -> accrueTokens).
Consider removing the check at the line L#162.
162 _ensureTokenInitialized(tokens_[i]); 163 _setTokenDistributionSpeed(tokens_[i], distributionSpeeds_[i]);
There are 2 instances of multiple rechecking:
The if (!tokens[user].exists)
check at the lines L#780 and L#795 duplicates the same check from the updateScores
function which call _executeBoost
and _updateScore
. Consider removing this check from _executeBoost
and _updateScore
function and adding it at the accrueInterestAndUpdateScore
and _accrueInterestAndUpdateScore
functions with corresponding logic.
200 function updateScores(address[] memory users) external { 204 for (uint256 i = 0; i < users.length; ) { 207 if (!tokens[user].exists) revert UserHasNoPrimeToken(); 211 for (uint256 j = 0; j < _allMarkets.length; ) { 213 _executeBoost(user, market); 214 _updateScore(user, market); 779 function _executeBoost(address user, address vToken) internal { 780 if (!markets[vToken].exists || !tokens[user].exists) { 794 function _updateScore(address user, address market) internal { 795 if (!markets[market].exists || !tokens[user].exists) {
<x> += 1
and <x> -= 1
Pre increment/decrement costs less than post increment decrement/decrement or <x> += 1
and <x> -= 1
. It can save more than 5 gas per execution.
There are 19 instances:
189 i++; 217 j++; 221 pendingScoreUpdates--; 225 i++; 250 i++; 345 i++; 355 i++; 614 i++; 636 i++; 711 totalIrrevocable++; 713 totalRevocable++; 740 i++; 745 totalIrrevocable--; 747 totalRevocable--; 766 totalIrrevocable++; 767 totalRevocable--; 819 nextScoreUpdateRoundId++; 828 if (totalScoreUpdatesRequired > 0) totalScoreUpdatesRequired--; 831 pendingScoreUpdates--;
There are redundant divisions by the EXP_SCALE
constant at the lines L#881-882, 885-886 which have no effect on the return value:
881 uint256 borrowCapUSD = (xvsPrice * ((xvs * markets[market].borrowMultiplier) / EXP_SCALE)) / EXP_SCALE; 882 uint256 supplyCapUSD = (xvsPrice * ((xvs * markets[market].supplyMultiplier) / EXP_SCALE)) / EXP_SCALE; 885 uint256 supplyUSD = (tokenPrice * supply) / EXP_SCALE; 886 uint256 borrowUSD = (tokenPrice * borrow) / EXP_SCALE; 888 if (supplyUSD >= supplyCapUSD) { 889 supply = supplyUSD > 0 ? (supply * supplyCapUSD) / supplyUSD : 0; 890 } 892 if (borrowUSD >= borrowCapUSD) { 893 borrow = borrowUSD > 0 ? (borrow * borrowCapUSD) / borrowUSD : 0; 894 }
https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/Prime.sol#L881-L893 The proposed implementation:
881 uint256 borrowCapUSD = xvsPrice * ((xvs * markets[market].borrowMultiplier) / EXP_SCALE); 882 uint256 supplyCapUSD = xvsPrice * ((xvs * markets[market].supplyMultiplier) / EXP_SCALE); 885 uint256 supplyUSD = (tokenPrice * supply); 886 uint256 borrowUSD = (tokenPrice * borrow); 888 if (supplyUSD >= supplyCapUSD) { 889 supply = supplyUSD > 0 ? (supply * supplyCapUSD) / supplyUSD : 0; 890 } 892 if (borrowUSD >= borrowCapUSD) { 893 borrow = borrowUSD > 0 ? (borrow * borrowCapUSD) / borrowUSD : 0; 894 }
Consider the xvsUpdated
function optimization to improve both readability and gas economy.
The current implementation:
https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/Prime.sol#L365-L382
function xvsUpdated(address user) external { uint256 totalStaked = _xvsBalanceOfUser(user); bool isAccountEligible = isEligible(totalStaked); if (tokens[user].exists && !isAccountEligible) { if (tokens[user].isIrrevocable) { _accrueInterestAndUpdateScore(user); } else { _burn(user); } } else if (!isAccountEligible && !tokens[user].exists && stakedAt[user] > 0) { stakedAt[user] = 0; } else if (stakedAt[user] == 0 && isAccountEligible && !tokens[user].exists) { stakedAt[user] = block.timestamp; } else if (tokens[user].exists && isAccountEligible) { _accrueInterestAndUpdateScore(user); } }
The proposed implementation:
function xvsUpdated(address user) external { uint256 totalStaked = _xvsBalanceOfUser(user); bool isAccountEligible = isEligible(totalStaked); if (tokens[user].exists) { if (isAccountEligible || tokens[user].isIrrevocable) { _accrueInterestAndUpdateScore(user); } else { _burn(user); } } else if (stakedAt[user] == 0 && isAccountEligible) { stakedAt[user] = block.timestamp; } else if (!isAccountEligible && stakedAt[user] > 0) { stakedAt[user] = 0; } }
if
/validation statements as up as possible in the function bodyif (distributionSpeed > 0)
can be above balance
variable declaration
258 uint256 distributionSpeed = tokenDistributionSpeeds[token_]; 259 uint256 balance = IERC20Upgradeable(token_).balanceOf(address(this)); 261 uint256 balanceDiff = balance - tokenAmountAccrued[token_]; 262 if (distributionSpeed > 0 && balanceDiff > 0) {
There are 5 instances
175 address[] storage _allMarkets = allMarkets; 210 address[] storage _allMarkets = allMarkets; 608 address[] storage _allMarkets = allMarkets; 624 address[] storage _allMarkets = allMarkets; 728 address[] storage _allMarkets = allMarkets;
#0 - c4-pre-sort
2023-10-07T06:48:58Z
0xRobocop marked the issue as low quality report
#1 - c4-judge
2023-11-03T17:05:46Z
fatherGoose1 marked the issue as grade-b