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: 87/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
Note: Most explanations here are taken from this report by @hickuphh3 as they are written well, so credits to him where it's due
Uninitialized variables are assigned with a default value depending on its type (0
for uint
). Thus, explicitly initializing a variable with its default value costs unnecesary gas.
I suggest changing ForgottenRunesWarriorsGuild:24
from:
uint256 public numMinted = 0;
to
uint256 public numMinted;
> 0
is less efficient than != 0
for unsigned integersIn require
statements, comparisons with != 0
are cheaper than > 0
for unsigned integers.
Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in a require statement, this will save gas. You can see this tweet for more proof:
I suggest changing from > 0
to != 0
in these lines:
// ForgottenRunesWarriorsMinter.sol:142 require( numWarriors > 0 && numWarriors <= 20, 'You can summon no more than 20 Warriors at a time' ); // ForgottenRunesWarriorsMinter.sol:212 require( numWarriors > 0 && numWarriors <= 20, 'You can summon no more than 20 Warriors at a time' );
++i
costs less gas compared to i++
or i += 1
++i
costs less gas compared to i++
or i += 1
for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration).
i++
increments i
and returns the initial value of i
. Which means:
uint i = 1; i++; // == 1 but i == 2
But ++i
returns the actual incremented value:
uint i = 1; ++i; // == 2 and i == 2 too, so no need for a temporary variable
In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.
I suggest changing from i += 1
to ++i
in these lines:
// ForgottenRunesWarriorsGuild:104 numMinted += 1; // ForgottenRunesWarriorsMinter.sol:193 numSold += 1; // ForgottenRunesWarriorsMinter.sol:248 numClaimed += 1;
for
loopIn these instances, there is no risk that the loop counter can overflow. Thus, Solidity's unchecked
block can be used to remove the overflow checks and save gas.
With ForgottenRunesWarriorsMinter.sol:162
as an example:
// Original code: for (uint256 i = 0; i < numWarriors; i++) { _mint(msg.sender); } // Can be changed to: for (uint256 i = 0; i < numWarriors; ) { _mint(msg.sender); unchecked { ++i; } }
Other instances include:
// ForgottenRunesWarriorsMinter.sol:220 for (uint256 i = 0; i < numWarriors; i++) { _mint(msg.sender); } // ForgottenRunesWarriorsMinter.sol:259 for (uint256 i = 0; i < count; i++) { _mint(recipient); } // ForgottenRunesWarriorsMinter.sol:355 for (uint256 i = startIdx; i < endIdx + 1; i++) { _refundAddress(daMinters[i]); }
Since Solidity 0.8.0, all arithmetic operations come with implicit overflow and underflow checks. In instances where an overflow/underflow is impossible (For example, a comparison occurs before the arithmetic operation), gas can be saved by using an unchecked block.
For example, in ForgottenRunesWarriorsMinter.sol:154
, the arithemtic operation can be wrapped in an unchecked
block as such:
// At line 138, there is a check that ensures numSold + numWarriors will not overflow require(numSold + numWarriors <= maxDaSupply, 'Not enough remaining'); // Original code at line 154: numSold += numWarriors; // Change to: unchecked { numSold += numWarriors; }
Other instances include:
// ForgottenRunesWarriorsGuild.sol:104 numMinted += 1; // ForgottenRunesWarriorsMinter.sol:193 numSold += 1; // ForgottenRunesWarriorsMinter.sol:219 numSold += numWarriors; // ForgottenRunesWarriorsMinter.sol:248 numClaimed += 1;
If a constant is not used outside of its contract, declaring it as private
or internal
instead of public
can save gas.
I suggest changing the visibility of the following from public
to internal
or private
:
// ForgottenRunesWarriorsGuild.sol:21 uint256 public constant MAX_WARRIORS = 16000; // ForgottenRunesWarriorsGuild.sol:32 string public constant R = "Old men forget: yet all shall be forgot, But he'll remember with advantages What feats he did that day: then shall our names Familiar in his mouth as household words. This story shall the good man teach his son From this day to the ending of the world";