Platform: Code4rena
Start Date: 31/01/2023
Pot Size: $90,500 USDC
Total HM: 47
Participants: 169
Period: 7 days
Judge: LSDan
Total Solo HM: 9
Id: 211
League: ETH
Rank: 44/169
Findings: 2
Award: $375.62
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x3b, 0xAgro, 0xBeirao, 0xMirce, 0xNineDec, 0xRobocop, 0xSmartContract, 0xTraub, 0xWeiss, 2997ms, 41i3xn, Awesome, Aymen0909, Bauer, Bnke0x0, Breeje, Cryptor, DadeKuma, Deathstore, Deekshith99, DevABDee, DevTimSch, Dewaxindo, Diana, Ermaniwe, Guild_3, H0, IceBear, Inspectah, JDeryl, Kaiziron, Kaysoft, Kenshin, Mukund, Praise, RaymondFam, Rickard, Rolezn, Ruhum, Sathish9098, SkyWalkerMan, SleepingBugs, UdarTeam, Udsen, Walter, aashar, adeolu, apvlki, arialblack14, ast3ros, btk, chaduke, chandkommanaboyina, chrisdior4, climber2002, codetilda, cryptonue, cryptostellar5, csanuragjain, ddimitrov22, descharre, dharma09, doublesharp, eccentricexit, ethernomad, fs0c, georgits, halden, hansfriese, hashminer0725, immeas, lukris02, luxartvinsec, matrix_0wl, merlin, mookimgo, mrpathfindr, nadin, olegthegoat, pavankv, rbserver, rebase, savi0ur, sayan, scokaf, seeu, shark, simon135, tnevler, tsvetanovv, ulqiorra, ustas, waldenyan20, y1cunhui, yongskiws, yosuke
305.798 USDC - $305.80
During the audit, 1 low and 8 non-critical issues were found.
| № | Title | Risk Rating | Instance Count |
|---|---|---|---|
| L-1 | No way to remove elements from rewardTokens array | Low | 1 |
| NC-1 | Order of Functions | Non-Critical | 16 |
| NC-2 | Order of Layout | Non-Critical | 5 |
| NC-3 | Visibility is not set | Non-Critical | 2 |
| NC-4 | Typos | Non-Critical | 6 |
| NC-5 | Missing leading underscores | Non-Critical | 5 |
| NC-6 | Unused variable | Non-Critical | 2 |
| NC-7 | Missing NatSpec | Non-Critical | 18 |
| NC-8 | Maximum line length exceeded | Non-Critical | 17 |
rewardTokens arrayThere is no way to remove items from the rewardTokens array. Over time, some rewardTokens may become irrelevant, or, accidentally, an incorrect rewardToken may be added to the array. In the best case, all functions that use the accrueRewards modifier will consume more gas, and in the worst case, due to one small mistake, all these functions may become unavailable.
IERC20[] public rewardTokens;rewardTokens.push(rewardToken); (Elements can only be added, not removed)modifier accrueRewards(address _caller, address _receiver) { (The modifier loops through the rewardTokens array)Functions affected by modifier accrueRewards :
Implement function for deleting elements from the rewardTokens array.
According to Style Guide, ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.
Functions should be grouped according to their visibility and ordered:
Public functions should be placed before internal:
External function should be placed before internal:
Public functions should be placed after external:
Public functions should be placed before internal:
It is better to place initialize() function before other functions:
Reorder functions where possible.
According to Order of Layout, inside each contract, library or interface, use the following order:
Modifiers should be placed before functions:
Place modifiers before functions.
It is better to specify visibility explicitly.
* Is used by `VaultController` to check if a target is a registerd clone. => registered* @param template Contains the implementation address and necessary informations to clone the implementation. => information/// @notice The amount of assets that are free to be withdrawn from the yVault after locked profts. => profits* @dev This function is the one stop solution to create a new vault with all necessary admin functions or auxiliery contracts. => auxiliary* @notice Sets a new `DeploymentController` and saves its auxilary contracts. Caller must be owner. => auxiliary/// @Notice Optional - Metadata CID which can be used by the frontend to add informations to a vault/adapter... => informationInternal constants and state variables should have a leading underscore.
uint256 constant DEGRADATION_COEFFICIENT = 10**18;uint256 internal nonce;uint256 constant SECONDS_PER_YEAR = 365.25 days;bytes4 internal immutable DEPLOY_SIG = bytes4(keccak256("deploy(bytes32,bytes32,bytes)"));uint256 internal underlyingBalance;Add leading underscores where needed.
Some variables declared but not used.
uint256 highWaterMark_ = highWaterMark; (highWaterMark_)function _registerVault(address vault, VaultMetadata memory metadata) internal { (vault)NatSpec is missing for 18 functions in 6 contracts.
Add NatSpec for all functions.
According to Style Guide, maximum suggested line length is 120 characters. Longer lines make the code harder to read.
Make the lines shorter.
#0 - c4-judge
2023-02-28T17:43:19Z
dmvt marked the issue as grade-a
🌟 Selected for report: c3phas
Also found by: 0xSmartContract, 0xackermann, 0xdaydream, Aymen0909, CodingNameKiki, Dewaxindo, Diana, IllIllI, Madalad, NoamYakov, Pheonix, Polaris_tow, ReyAdmirado, Rolezn, arialblack14, atharvasama, cryptostellar5, descharre, eyexploit, lukris02, saneryee
69.8247 USDC - $69.82
During the audit, 4 gas issues were found.
Total savings ~7145.
| № | Title | Instance Count | Saved |
|---|---|---|---|
| G-1 | Use unchecked blocks for incrementing | 22 | 770 |
| G-2 | Use unchecked blocks for subtractions where underflow is impossible | 5 | 175 |
| G-3 | Cached variable is not used | 1 | 200 |
| G-4 | Using storage pointer to struct/array/mapping is cheaper than using memory | 8 | 6000 |
In Solidity 0.8+, there’s a default overflow and underflow check on unsigned integers. In the loops or similar cases, "i" will not overflow because the loop will run out of gas before that or max value never be reached.
Change:
for (uint256 i; i < n; ++i) { // ... }
to:
for (uint256 i; i < n;) { // ... unchecked { ++i; } }
This saves ~30-40 gas per iteration.
So, ~35*22 = 770
In Solidity 0.8+, there’s a default overflow and underflow check on unsigned integers. When an overflow or underflow isn’t possible (after require or if-statement), some gas can be saved by using unchecked blocks.
This saves ~35.
So, ~35*5 = 175
The variable highWaterMark_ was meant to be used, but highWaterMark was used.
This saves 100*2 = 200.
storage pointer to struct/array/mapping is cheaper than using memoryWhen you copy a storage struct/array/mapping to a memory variable, you are copying each member by reading it from the storage, which is expensive, but when you use the storage keyword, you are just storing a pointer to the storage, which is much cheaper.
For example, change:
Escrow memory escrow = escrows[escrowId];
to:
Escrow storage escrow = escrows[escrowId];
This saves ~6000.
#0 - c4-judge
2023-02-28T14:52:19Z
dmvt marked the issue as grade-b