Platform: Code4rena
Start Date: 03/05/2022
Pot Size: $30,000 USDC
Total HM: 6
Participants: 93
Period: 3 days
Judge: gzeon
Id: 118
League: ETH
Rank: 74/93
Findings: 2
Award: $45.73
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: defsec
Also found by: 0v3rf10w, 0x1f8b, 0x4non, 0x52, 0xDjango, 0xf15ers, 0xkatana, 0xliumin, AuditsAreUS, BowTiedWardens, CertoraInc, Cr4ckM3, Funen, GimelSec, Hawkeye, IllIllI, Kulk0, M0ndoHEHE, MaratCerby, Picodes, Ruhum, TerrierLover, TrungOre, VAD37, WatchPug, berndartmueller, broccolirob, catchup, cccz, cryptphi, csanuragjain, delfin454000, dirk_y, eccentricexit, ellahi, fatherOfBlocks, gzeon, hake, hansfriese, hickuphh3, horsefacts, hubble, hyh, ilan, joestakey, kebabsec, kenta, kenzo, leastwood, m9800, marximimus, minhquanym, oyc_109, p4st13r4, pauliax, pedroais, peritoflores, plotchy, rajatbeladiya, reassor, rfa, robee, rotcivegaf, samruna, shenwilly, shung, simon135, sorrynotsorry, sseefried, teddav, throttle, tintin, unforgiven, z3s
30.2807 USDC - $30.28
I. Redundant address (0) check. The call to _mint() will guarantee that the recipient is not set to that.
II. For better readability, use ++ instead of +=1
III.Use enumerable set from OZ contracts to deal with the issue of duplicates when adding to daMinters[ ] in BidSummon(), this will prevent checking for the same address when calculating the refund in issueRefund() for the specific address (in the event, there are duplicates) and saving needed gas.
PS. This is a cross-over between a recommendation and also gas optimisation.
IV. Variables are already the default, no need to assign them :
V. Owner can deny users a refund in the worst case
If the auction is not sold out, it is the owner who sets the final price and it can be too high for the calculation of totalCostOfMints in refundOwed(), thereby reverting. Since each user’s daAmountPaid would be different, it's highly unlikely that the finalPrice will be favourably set to initiate a refund.
PS. It was only after continuously asking for clarification(relating to a solution to prevent users from not getting a refund if it is set at an unreasonable price) that the dev(#dotta) mentioned :
“If the auction doesn't sell out, i'll update the price to reflect 0.6”
The comments in SetFinalPrice() does not state this:
Add : require(new Price ≤lowestPrice, “too high”) within setFinalPrice().
🌟 Selected for report: BowTiedWardens
Also found by: 0v3rf10w, 0x1f8b, 0x4non, 0xDjango, 0xNazgul, 0xProf, 0xc0ffEE, 0xf15ers, 0xkatana, 0xliumin, ACai, AlleyCat, CertoraInc, Cityscape, Cr4ckM3, DavidGialdi, Dinddle, FSchmoede, Funen, GimelSec, Hawkeye, IllIllI, Kulk0, M0ndoHEHE, MaratCerby, MiloTruck, Picodes, RoiEvenHaim, Tadashi, TerrierLover, TrungOre, VAD37, WatchPug, antonttc, catchup, defsec, delfin454000, dirk_y, eccentricexit, ellahi, fatherOfBlocks, gzeon, hake, hansfriese, hickuphh3, horsefacts, ilan, joestakey, kebabsec, kenta, kenzo, marximimus, minhquanym, noobie, oyc_109, p4st13r4, pauliax, rajatbeladiya, reassor, rfa, robee, rotcivegaf, saian, samruna, shenwilly, shung, simon135, slywaters, sorrynotsorry, throttle, unforgiven, z3s
15.4498 USDC - $15.45
I. Since the state variable is already cached, no need to write to it again.
Use uint256 tokenId=numMinted++ and remove line 104.
II. daStarted() ensures that block.timestamp >daStartTime. Therefore, use unchecked block to save gas.
III. Since the stepDeduction is being checked:
On line 295, the currentPrice will only be returned when the stepDeduction < startPrice. Therefore, use unchecked block for the calculation of currentPrice.