Forgotten Runes Warrior Guild contest - FSchmoede's results

Gas optimization report

The total gas saved on average on all methods by applying all of the below suggestions will be: ~1700 gas.

Every single of the below optimizations have been tested with the yarn test:gas command before and after to ensure, that they do in fact save gas. It will be stated how much gas each optimization saves, so it can be used for prioritization.

GO1: Use unchecked arithmetic in issueRefunds

In the contract ForgottenRunesWarriorsMinter.sol the method issueRefunds has a loop where the iterator/counter can use unchecked arithmetic. Since it is a uint256 and the highest index of the array is 8000 (due to the max supply), then there is no chance that the iterator will overflow.

The following loop:

for (uint256 i = startIdx; i < endIdx + 1; i++) {
    _refundAddress(daMinters[i]);
}

Can be optimized into:

for (uint256 i = startIdx; i < endIdx + 1;) {
    _refundAddress(daMinters[i]);
    unchecked { ++i; }
}

This optimization will save on average 104 gas for the method issueRefunds().

GO2: Cache daStartTime in the method currentDaPrice

The method currentDaPrice() in ForgottenRunesWarriorsMinter.sol reads the variable daStartTime from memory 2 times. Reading from memory uses the SLOAD operation which is costly. Gas can be saved by caching the variable in memory. So instead of using 2 SLOAD operations, the contract should instead use 1 SLOAD to load the variable from storage, 1 MSTORE operation to save the variable on memory and 2 MLOAD to read the variable from memory.

The code (line 279-287):

if (block.timestamp >= daStartTime + daPriceCurveLength) { // SLOAD
    // end of the curve
    return lowestPrice;
}

uint256 dropPerStep = (startPrice - lowestPrice) /
    (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - daStartTime; // SLOAD

Should be changes into:

uint256 cachedStartTime= daStartTime; // SLOAD + MSTORE
if (block.timestamp >= cachedStartTime+ daPriceCurveLength) { // MLOAD
    // end of the curve
    return lowestPrice;
}

uint256 dropPerStep = (startPrice - lowestPrice) /
    (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - cachedStartTime; // MLOAD

This optimization will save on average 104 gas for the method bidSummon().

GO3: Cache vault address in the method withdraw

The method withdraw() in ForgottenRunesWarriorsMinter.sol reads the address vault from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 609-610):

require(address(vault) != address(0), 'no vault');
require(payable(vault).send(_amount));

Should be changes into:

address cachedVault= vault;
require(address(cachedVault) != address(0), 'no vault');
require(payable(cachedVault).send(_amount));

This optimization will save on average 101 gas for the method withdraw().

GO4: Cache vault address in the method withdrawAll

The method withdrawAll() in ForgottenRunesWarriorsMinter.sol reads the address vault from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 617-618):

require(address(vault) != address(0), 'no vault');
require(payable(vault).send(address(this).balance));

Should be changes into:

address cachedVault= vault;
require(address(cachedVault) != address(0), 'no vault');
require(payable(cachedVault).send(address(this).balance));

This optimization will save on average 101 gas for the method withdrawAll().

GO5: Cache startPrice in the method currentDaPrice

The method currentDaPrice() in ForgottenRunesWarriorsMinter.sol reads the variable startPrice from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 284-295):

uint256 dropPerStep = (startPrice - lowestPrice) /
            (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / daDropInterval;
uint256 stepDeduction = steps * dropPerStep;

if (stepDeduction > startPrice) {
    return lowestPrice;
}
uint256 currentPrice = startPrice - stepDeduction;

Should be changes into:

uint256 cachedStartPrice = startPrice;
uint256 dropPerStep = (cachedStartPrice - lowestPrice) /
            (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / daDropInterval;
uint256 stepDeduction = steps * dropPerStep;

if (stepDeduction > cachedStartPrice ) {
    return lowestPrice;
}
uint256 currentPrice = cachedStartPrice - stepDeduction;

This optimization will save on average 204 gas for the method bidSummon().

GO6: Cache lowestPrice in the method currentDaPrice

The method currentDaPrice() in ForgottenRunesWarriorsMinter.sol reads the variable lowestPrice from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 284-296):

uint256 dropPerStep = (startPrice - lowestPrice) /
            (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / daDropInterval;
uint256 stepDeduction = steps * dropPerStep;

// don't go negative in the next step
if (stepDeduction > startPrice) {
    return lowestPrice;
}
uint256 currentPrice = startPrice - stepDeduction;
return currentPrice > lowestPrice ? currentPrice : lowestPrice;

Should be changes into:

uint256 cachedLowestPrice = lowestPrice;
uint256 dropPerStep = (startPrice - cachedLowestPrice) /
            (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / daDropInterval;
uint256 stepDeduction = steps * dropPerStep;

// don't go negative in the next step
if (stepDeduction > startPrice) {
    return cachedLowestPrice;
}
uint256 currentPrice = startPrice - stepDeduction;
return currentPrice > cachedLowestPrice ? currentPrice : cachedLowestPrice;

This optimization will save on average 98 gas for the method bidSummon().

GO7: Cache daPriceCurveLength in the method currentDaPrice

The method currentDaPrice() in ForgottenRunesWarriorsMinter.sol reads the variable daPriceCurveLength from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 279-285):

if (block.timestamp >= daStartTime + daPriceCurveLength) {
    // end of the curve
    return lowestPrice;
}

uint256 dropPerStep = (startPrice - lowestPrice) /
    (daPriceCurveLength / daDropInterval);

Should be changes into:

uint256 cachedCurveLength = daPriceCurveLength;
if (block.timestamp >= daStartTime + cachedCurveLength ) {
    // end of the curve
    return lowestPrice;
}

uint256 dropPerStep = (startPrice - lowestPrice) /
    (cachedCurveLength / daDropInterval);

This optimization will save on average 98 gas for the method bidSummon().

GO8: Cache daDropInterval in the method currentDaPrice

The method currentDaPrice() in ForgottenRunesWarriorsMinter.sol reads the variable daDropInterval from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 284-289):

uint256 dropPerStep = (startPrice - lowestPrice) /
            (daPriceCurveLength / daDropInterval);

uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / daDropInterval;
uint256 stepDeduction = steps * dropPerStep;

Should be changes into:

uint cachedDropInterval = daDropInterval;
uint256 dropPerStep = (startPrice - lowestPrice) /
            (daPriceCurveLength / cachedDropInterval);

uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / cachedDropInterval;
uint256 stepDeduction = steps * dropPerStep;

This optimization will save on average 107 gas for the method bidSummon().

GO9: Cache maxDaSupply in the method bidSummon

The method bidSummon() in ForgottenRunesWarriorsMinter.sol reads the variable maxDaSupply from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 136-160):

require(numSold < maxDaSupply, 'Auction sold out');
require(numSold + numWarriors <= maxDaSupply, 'Not enough remaining');
require(daStarted(), 'Auction not started');
require(!mintlistStarted(), 'Auction phase over');
require(
    numWarriors > 0 && numWarriors <= 20,
    'You can summon no more than 20 Warriors at a time'
);

uint256 currentPrice = currentDaPrice();
require(
    msg.value >= (currentPrice * numWarriors),
    'Ether value sent is not sufficient'
);

daMinters.push(msg.sender);
daAmountPaid[msg.sender] += msg.value;
daNumMinted[msg.sender] += numWarriors;
numSold += numWarriors;

if (numSold == maxDaSupply) {
    // optimistic: save gas by not setting on every mint, but will
    // require manual `setFinalPrice` before refunds if da falls short
    finalPrice = currentPrice;
}

Should be changes into:

uint256 cachedMaxSupply = maxDaSupply;
require(numSold < cachedMaxSupply, 'Auction sold out');
require(numSold + numWarriors <= cachedMaxSupply, 'Not enough remaining');
require(daStarted(), 'Auction not started');
require(!mintlistStarted(), 'Auction phase over');
require(
    numWarriors > 0 && numWarriors <= 20,
    'You can summon no more than 20 Warriors at a time'
);

uint256 currentPrice = currentDaPrice();
require(
    msg.value >= (currentPrice * numWarriors),
    'Ether value sent is not sufficient'
);

daMinters.push(msg.sender);
daAmountPaid[msg.sender] += msg.value;
daNumMinted[msg.sender] += numWarriors;
numSold += numWarriors;

if (numSold == cachedMaxSupply) {
    // optimistic: save gas by not setting on every mint, but will
    // require manual `setFinalPrice` before refunds if da falls short
    finalPrice = currentPrice;
}

This optimization will save on average 195 gas for the method bidSummon().

G10: Cache numSold in the method bidSummon

The method bidSummon() in ForgottenRunesWarriorsMinter.sol reads the variable numSold from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 136-160):

require(numSold < maxDaSupply, 'Auction sold out');
require(numSold + numWarriors <= maxDaSupply, 'Not enough remaining');
require(daStarted(), 'Auction not started');
require(!mintlistStarted(), 'Auction phase over');
require(
    numWarriors > 0 && numWarriors <= 20,
    'You can summon no more than 20 Warriors at a time'
);

uint256 currentPrice = currentDaPrice();
require(
    msg.value >= (currentPrice * numWarriors),
    'Ether value sent is not sufficient'
);

daMinters.push(msg.sender);
daAmountPaid[msg.sender] += msg.value;
daNumMinted[msg.sender] += numWarriors;
numSold += numWarriors;

if (numSold == maxDaSupply) {
    // optimistic: save gas by not setting on every mint, but will
    // require manual `setFinalPrice` before refunds if da falls short
    finalPrice = currentPrice;
}

Should be changes into:

uint256 cachedNumSold = numSold;
require(cachedNumSold < maxDaSupply, 'Auction sold out');
require(cachedNumSold + numWarriors <= maxDaSupply, 'Not enough remaining');
require(daStarted(), 'Auction not started');
require(!mintlistStarted(), 'Auction phase over');
require(
    numWarriors > 0 && numWarriors <= 20,
    'You can summon no more than 20 Warriors at a time'
);

uint256 currentPrice = currentDaPrice();
require(
    msg.value >= (currentPrice * numWarriors),
    'Ether value sent is not sufficient'
);

daMinters.push(msg.sender);
daAmountPaid[msg.sender] += msg.value;
daNumMinted[msg.sender] += numWarriors;
cachedNumSold += numWarriors; // Do arithmetic on cached variable instead
numSold = cachedNumSold ;

if (cachedNumSold == maxDaSupply) { // Use the updated cached variable here as well
    // optimistic: save gas by not setting on every mint, but will
    // require manual `setFinalPrice` before refunds if da falls short
    finalPrice = currentPrice;
}

This optimization will save on average 314 gas for the method bidSummon().

GO11: Cache maxForSale in the method publicSummon

The method publicSummon() in ForgottenRunesWarriorsMinter.sol reads the variable maxForSale from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 207-208):

require(numSold < maxForSale, 'Sold out');
require(numSold + numWarriors <= maxForSale, 'Not enough remaining');

Should be changes into:

uint256 cachedMaxForSale = maxForSale;
require(numSold < cachedMaxForSale , 'Sold out');
require(numSold + numWarriors <= cachedMaxForSale , 'Not enough remaining');

This optimization will save on average 95 gas for the method publicSummon().

GO12: Cache numSold in the method publicSummon

The method publicSummon() in ForgottenRunesWarriorsMinter.sol reads the variable numSold from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 207-208):

require(numSold < maxForSale, 'Sold out');
require(numSold + numWarriors <= maxForSale, 'Not enough remaining');

Should be changes into:

uint256 cachedNumSold = numSold;
require(cachedNumSold < maxForSale, 'Sold out');
require(cachedNumSold + numWarriors <= maxForSale, 'Not enough remaining');

This optimization will save on average 101 gas for the method publicSummon().

GO13: Cache numMinted in the method mint

The method publicSummon() in ForgottenRunesWarriorsGuild.sol reads the variable numMinted from memory multiple times. It should be cached in memory instead. To get a more detailed explanation, see GO2.

The code (line 100-102):

require(numMinted < MAX_WARRIORS, 'All warriors have been summoned');
require(_msgSender() == minter, 'Not a minter');
uint256 tokenId = numMinted;

Should be changes into:

uint256 cachedNumMinted = numMinted;
require(cachedNumMinted < MAX_WARRIORS, 'All warriors have been summoned');
require(_msgSender() == minter, 'Not a minter');

This optimization will save on average 95 gas for the method mint().

GO14: Keep revert strings below 32 bytes

Strings are stored in slots of 32 bytes, and hence the length of the revert string should be at max 32 bytes to fit inside 1 slot. If the string is just 33 bytes it will occupy 2 slots (64 bytes). Keeping the string size at 32 bytes or below will save gas on both deployment and when the revert condition is met.

Examples revert strings longer than 32 bytes:

ForgottenRunesWarriorsGuild.sol line 70:         "ERC721Metadata: URI query for nonexistent token"
ForgottenRunesWarriorsGuild.sol line 117:        "ERC721Burnable: caller is not owner nor approved"

ForgottenRunesWarriorsMinter.sol line 144:       "You can summon no more than 20 Warriors at a time"
ForgottenRunesWarriorsMinter.sol line 150:       "Ether value sent is not sufficient"
ForgottenRunesWarriorsMinter.sol line 217:       "You can summon no more than 20 Warriors at a time"

GO15: Unused constant R

In the contract ForgottenRunesWarriorsGuild.sol there is defined a constant R which seems to be unused. If this is the in fact the case it can be removed to save gas upon deployment. Also considering the length of the constant (251 bytes).