Forgotten Runes Warrior Guild contest - MiloTruck's results

16,000 Warrior NFTs sold in a phased Dutch Auction.

General Information

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

Forgotten Runes

Findings Distribution

Researcher Performance

Rank: 87/93

Findings: 1

Award: $15.49

🌟 Selected for report: 0

🚀 Solo Findings: 0

Gas Optimizations Report

Note: Most explanations here are taken from this report by @hickuphh3 as they are written well, so credits to him where it's due

Unecessary initialization of variables with default values

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 integers

In 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;

Unnecessary checked arithmetic in for loop

In 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]);
}

Unchecking arithmetics operations that cannot underflow/overflow

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;

Consider declaring some constants as non-public to save gas

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";
AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter