Platform: Code4rena
Start Date: 20/09/2022
Pot Size: $30,000 USDC
Total HM: 12
Participants: 198
Period: 3 days
Judge: 0xean
Total Solo HM: 2
Id: 164
League: ETH
Rank: 111/198
Findings: 2
Award: $27.95
๐ Selected for report: 0
๐ Solo Findings: 0
๐ Selected for report: AkshaySrivastav
Also found by: 0v3rf10w, 0x040, 0x1f8b, 0x4non, 0x5rings, 0x85102, 0xA5DF, 0xDecorativePineapple, 0xNazgul, 0xSky, 0xSmartContract, 0xbepresent, 0xf15ers, 0xmatt, 2997ms, Aeros, Aymen0909, B2, Bahurum, Bnke0x0, CertoraInc, Chom, ChristianKuri, CodingNameKiki, Deivitto, Diana, Diraco, Dravee, ElKu, Funen, IllIllI, JC, JLevick, JohnSmith, JohnnyTime, KIntern_NA, Lambda, Margaret, MasterCookie, OptimismSec, RaymondFam, Respx, ReyAdmirado, RockingMiles, Rohan16, Rolezn, Ruhum, RustyRabbit, Sm4rty, SooYa, StevenL, TomJ, Tomo, V_B, Waze, Yiko, __141345__, a12jmx, ajtra, ak1, async, ayeslick, aysha, berndartmueller, bin2chen, bobirichman, brgltd, bulej93, c3phas, carrotsmuggler, cccz, ch13fd357r0y3r, chatch, cryptostellar5, cryptphi, csanuragjain, d3e4, datapunk, delfin454000, dic0de, djxploit, durianSausage, eighty, erictee, exd0tpy, fatherOfBlocks, gogo, got_targ, hansfriese, ignacio, ikbkln, indijanc, innertia, joestakey, karanctf, ladboy233, leosathya, lukris02, martin, medikko, millersplanet, nalus, natzuu, neko_nyaa, neumo, obront, oyc_109, pcarranzav, peanuts, pedr02b2, pedroais, peiw, peritoflores, prasantgupta52, rajatbeladiya, rbserver, reassor, ret2basic, rokinot, romand, rotcivegaf, rvierdiiev, sach1r0, seyni, sikorico, slowmoses, sorrynotsorry, supernova, tibthecat, tnevler, ubermensch, yongskiws, zzykxx, zzzitron
18.8574 USDC - $18.86
I would say the contracts are well written, with good comments and the functionality is protected with a good test suite. As mentioned in the contest description the admins / founders will have certain capabilities of managing the contract which will ultimately also need to be acompanied by certain trust. The contest description also states that the projects makes an effort to make any changes to claims transparent. Hence I would advise the following:
VTVLVesting is open to have many admins, so it would make sense to include the admin address on all relevant events to increase transparency
VTVLVesting.sol L59 Consider including the address of the admin that created the claim for better transparency.
VTVLVesting.sol L69 Consider including the address of the admin that revoked the claim for better transparency.
VTVLVesting.sol L74 Consider including the address of the admin that withdrew. This is probably less important than the other two events emitting claim changes, but still may be valuable as there may be several admins associated with a vesting contract and would want to have this included in the event.
๐ Selected for report: IllIllI
Also found by: 0v3rf10w, 0x040, 0x1f8b, 0x4non, 0x85102, 0xA5DF, 0xDanielC, 0xNazgul, 0xSmartContract, 0xbepresent, 0xc0ffEE, 0xsam, 2997ms, AkshaySrivastav, Amithuddar, Atarpara, Aymen0909, B2, Bnke0x0, CertoraInc, Chom, ChristianKuri, CodingNameKiki, Deivitto, Diana, DimitarDimitrov, Diraco, Funen, JC, JLevick, JohnSmith, Junnon, KIntern_NA, Lambda, MasterCookie, Matin, Noah3o6, Ocean_Sky, OptimismSec, RaymondFam, Respx, ReyAdmirado, RockingMiles, Rohan16, Rolezn, Ruhum, Saintcode_, Satyam_Sharma, Sm4rty, SnowMan, SooYa, Sta1400, StevenL, Tadashi, Tagir2003, TomJ, Tomio, Tomo, V_B, Waze, WilliamAmbrozic, Yiko, __141345__, a12jmx, adriro, ajtra, ak1, async, aysha, beardofginger, bobirichman, brgltd, bulej93, c3phas, carrotsmuggler, caventa, ch0bu, cryptostellar5, cryptphi, csanuragjain, d3e4, delfin454000, dharma09, djxploit, durianSausage, eighty, emrekocak, erictee, exd0tpy, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, got_targ, hxzy, ignacio, ikbkln, imare, indijanc, jag, jpserrat, karanctf, ladboy233, leosathya, lucacez, lukris02, m9800, malinariy, martin, medikko, mics, millersplanet, mrpathfindr, nalus, natzuu, neko_nyaa, oyc_109, pauliax, peanuts, pedroais, peiw, pfapostol, prasantgupta52, rbserver, ret2basic, rokinot, rotcivegaf, rvierdiiev, sach1r0, samruna, seyni, slowmoses, subtle77, supernova, tgolding55, tibthecat, tnevler, w0Lfrum, yaemsobak, zishansami
9.086 USDC - $9.09
The contracts are already quite well optimized, but there's still some room for improvements, especially when it comes to createClaimsBatch() as it includes a for loop with number of loops based on the input. Here's a few I see.
true in hasActiveClaim() as isActive is already a bool variableisActive can be used for the หrequire() inside hasActiveClaim() and the additional comparison to true is unnecessary and just wastes gas.
i++ to ++i in for loops++i will consume less gas because it doesn't need a temporary variable to return current value. Would suggest to use ++i if possible in any loop.
require() with stringsThe project is compiled with Solidity v0.8.14 which allows for custom errors (introduced in v0.8.4). Consider using custom errors across all contracts as it will reduce gas cost compared to using require() with strings.