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: 86/93
Findings: 1
Award: $15.49
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 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.491 USDC - $15.49
###[G003]: Use != 0 instead of > 0 for Unsigned Integer Comparison###
I recommand use != 0 that take less gas compare to > 0 you use >0 in those lines : 141, 211, 378.
I alse thinks in be more effective to change the comparison from: numWarriors > 0 && numWarriors <= 20 to: numWarriors <= 20 && numWarriors != 0 Beacuse most of the time there is more chance that the cunsumers will try buy more than 20 Warriors than 0, so in that case in save from doing to comparison, but it 'whould have Tiny impact.
###[G002]: Cache Array Length Outside of Loop### In line 341 there is this line for (uint256 i = startIdx; i < endIdx + 1; i++) that will do the opeartion endIdx + 1 multiple times. Instead you can do: * ++ endIdx; for (uint256 i = startIdx; i < endIdx ; i++) * or * for (uint256 i = startIdx; i <= endIdx ; i++) *
*** Use ++i instead of i++*** ++i is one less opcode therefore take less gas. I recommend to replace all of them especially those in the loop. In addition using the += 1 is the same as i++ so I recommend changing it to ++ prefix. You used in lines 193 and 248 and ForgottenRunesWarriorsGuild:104.
###Cancle the atuo checking###
from version 8+ the compilers auto-tests to check over / underflow every time a mathematical operation is performed.
Becuase the numbers of Warriors are limited and you use the uint256 therefore there is no need for those checking.
I would therefore recommend changing the following function
from:
*
function refundOwed(address minter) public view returns (uint256) {
uint256 totalCostOfMints = finalPrice * daNumMinted[minter];
uint256 refundsPaidAlready = daAmountRefunded[minter];
return daAmountPaid[minter] - totalCostOfMints - refundsPaidAlready;
}
*
to:
*
function refundOwed(address minter) public view returns (uint256) {
uint256 refundsPaidAlready = daAmountRefunded[minter];
unchecked {
uint256 totalCostOfMints = finalPrice * daNumMinted[minter];
return daAmountPaid[minter] - totalCostOfMints - refundsPaidAlready;
}
}
*
You could also change the for loops like that:
for (uint256 i;i < len;)
{
unchecked{++i;}
}
but it less readable and in my view not necessarily worth it in this case.