Platform: Code4rena
Start Date: 21/06/2022
Pot Size: $50,000 USDC
Total HM: 31
Participants: 99
Period: 5 days
Judges: moose-code, JasoonS, denhampreen
Total Solo HM: 17
Id: 139
League: ETH
Rank: 54/99
Findings: 2
Award: $80.04
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x1337, 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xNineDec, 0xc0ffEE, 0xf15ers, 0xmint, Bnke0x0, BowTiedWardens, Chom, ElKu, FudgyDRS, Funen, GalloDaSballo, GimelSec, JC, Kaiziron, Lambda, Limbooo, Metatron, MiloTruck, Noah3o6, Picodes, PumpkingWok, PwnedNoMore, Sm4rty, StErMi, TomJ, TrungOre, UnusualTurtle, Waze, _Adam, aga7hokakological, ak1, antonttc, berndartmueller, cccz, cryptphi, csanuragjain, defsec, delfin454000, dipp, elprofesor, exd0tpy, fatherOfBlocks, hake, hansfriese, hubble, joestakey, kenta, ladboy233, mics, oyc_109, pashov, pedr02b2, reassor, robee, samruna, scaraven, shung, sikorico, simon135, sseefried, tchkvsky, unforgiven, zzzitron
53.3348 USDC - $53.33
#1 Unused code Impact inefficiency code make gas cost higher.
Proof of Concept https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L84-L90
Tool Used Manual review
Recommend Mitigation Steps remove one of these
IERC20Upgradeable(YIELDY_TOKEN).approve( LIQUIDITY_RESERVE, type(uint256).max ); IERC20Upgradeable(YIELDY_TOKEN).approve( LIQUIDITY_RESERVE, type(uint256).max
#2 Use call instead transfer Impact usage of send() or transfer() would cause an out of gas error.
Proof of Concept https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L471
Tool Used remix
Recommend Mitigation Steps use .call() because .transfer() fowards 2300 gas whereas .call() forwards all / set gas.
#3 Missing natspec param orderUid Impact preSign function have natspec comment which is missing the orderUid function parameter. Issues with comments are low risk based on Code4rena risk categories.
Proof of Concept https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L769
Tool Used Remix
Recommend Mitigation Steps Add natspec comments include orderUid parameter in preSign function.
#4 TransferFrom must be external Impact Visibility of transferFrom must external
Proof of Concept https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Yieldy.sol#L205
Tool Used Manual review
Recommend Mitigation Steps check OZ docs for implementation transferFrom.
#5 Missing natspec parm index Impact canBatchContractByIndex function have natspec comment which is missing the index function parameter. Issues with comments are low risk based on Code4rena risk categories.
Proof of Concept https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/BatchRequests.sol#L47
Tool Used Remix
Recommend Mitigation Steps Add natspec comments include index parameter in canBatchContractByIndex function.
#6 Must be indexed Impact event is missing indexed fields.
Proof of Concept https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L32
Tool Used Manual review
Recommend Mitigation Steps Add indexed at affilateAddress.
🌟 Selected for report: BowTiedWardens
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, 8olidity, ACai, Bnke0x0, Chom, ElKu, Fabble, Fitraldys, FudgyDRS, Funen, GalloDaSballo, GimelSec, IllIllI, JC, Kaiziron, Lambda, Limbooo, MiloTruck, Noah3o6, Nyamcil, Picodes, PwnedNoMore, Randyyy, RedOneN, Sm4rty, StErMi, TomJ, Tomio, TrungOre, UnusualTurtle, Waze, _Adam, aga7hokakological, ajtra, antonttc, asutorufos, bardamu, c3phas, defsec, delfin454000, exd0tpy, fatherOfBlocks, hansfriese, ignacio, joestakey, kenta, ladboy233, m_Rassska, mics, minhquanym, oyc_109, pashov, reassor, robee, s3cunda, sach1r0, saian, sashik_eth, scaraven, sikorico, simon135, slywaters
26.7051 USDC - $26.71
#1 Use require jnstead &&
use require instead of && for efficient gas cost. change it from
require( _stakingToken != address(0) && _yieldyToken != address(0) && _tokeToken != address(0) && _tokePool != address(0) && _tokeManager != address(0) && _tokeReward != address(0) && _liquidityReserve != address(0), "Invalid address" );
to
require(_stakingToken != address(0), "Invalid address"); require(_yieldyToken != address(0), "Invalid address"); require(_tokeToken != address(0), "Invalid address"); require(_tokePool != address(0),"Invalid address"); require(_tokeManager != address(0),"Invalid address"); require(_tokeReward != address(0),"Invalid address"); require(_liquidityReserve != address(0),"Invalid address");
#2 Use != 0 instead of > 0.
for unsigned integer, >0 is less efficient then !=0, so use !=0 instead of >0. apply to others.
#3 Use storage instead memory
Use storage instead of memory to reduce the gas fee. i suggest to change
Claim memory userWarmInfo = warmUpInfo[_user];
to
Claim storage userWarmInfo = warmUpInfo[_user];
#4 use require Instead && #2
use require instead of && for efficient gas cost. change it from
require( !isUnstakingPaused && !isInstantUnstakingPaused, "Unstaking is paused");
to
require(!isUnstakingPaused,"Unstaking is paused"); require(!isInstantUnstakingPaused,"Unstaking is paused");
#5 Instead && curvepool
require( CURVE_POOL != address(0) && (curvePoolFrom == 1 || curvePoolTo == 1), "Invalid Curve Pool");
to
require(CURVE_POOL != address(0), "Invalid Curve Pool"); require(curvePoolFrom == 1 || curvePoolTo == 1), "Invalid Curve Pool");
#6 Pre increment
use pre increment e.g ++i because pre-increment more cheaper gas than post increment e.g i++. i suggest to use pre increment.
#7 Calldata
In the external functions where the function argument is read-only, the function() has an inputed parameter that using memory, if this function didnt change the parameter, its cheaper to use calldata then memory. so we suggest to change it.
#8 unnecessary code
fusion the code in if statement
if (updatedTotalSupply > MAX_SUPPLY) { updatedTotalSupply = MAX_SUPPLY;
to
if (updatedTotalSupply >= MAX_SUPPLY) {}
#9 Use require instead if
i suggest to change from
if (_amount == 0) { return;
to
require(amount != 0, "amount can't be zero);
#10 Cache requestedWithdrawals.amount
cache the requestedWithdrawals.amount because it use multiple time. cache it can reduce gas.
#11 Cache iyield as token
cache the IYieldy(YIELDY_TOKEN) because use mutiple times e.g
if (warmUpPeriod == 0) { IYieldy(YIELDY_TOKEN).mint(_recipient, _amount); } else { // create a claim and mint tokens so a user can claim them once warm up has passed warmUpInfo[_recipient] = Claim({ amount: info.amount + _amount, credits: info.credits + IYieldy(YIELDY_TOKEN).creditsForTokenBalance(_amount), expiry: epoch.number + warmUpPeriod }); IYieldy(YIELDY_TOKEN).mint(address(this), _amount); }
change to
IYieldy token = IYieldy(YIELDY_TOKEN); if (warmUpPeriod == 0) { token.mint(_recipient, _amount); } else { // create a claim and mint tokens so a user can claim them once warm up has passed warmUpInfo[_recipient] = Claim({ amount: info.amount + _amount, credits: info.credits + token.creditsForTokenBalance(_amount), expiry: epoch.number + warmUpPeriod }); token.mint(address(this), _amount); }