Gas Optimizations

[code423n4]

Table of Contents:

• Caching storage values in memory
• Unchecking arithmetics operations that can't underflow/overflow
• Unnecessary initialize() function
• ForgottenRunesWarriorsGuild.forwardERC20s() and ForgottenRunesWarriorsMinter.forwardERC20s(): Unnecessary require statements
• ForgottenRunesWarriorsMinter: bidSummon()and publicSummon(): Unnecessary require statement
• Boolean comparisons
• > 0 is less efficient than != 0 for unsigned integers (with proof)
• ForgottenRunesWarriorsMinter.currentDaPrice(): > should be >=
• Splitting require() statements that use && saves gas
• ++i costs less gas compared to i++ or i += 1
• Increments can be unchecked
• Public functions to external
• No need to explicitly initialize variables with default values
• Upgrade pragma to at least 0.8.4
• Use msg.sender instead of OpenZeppelin's _msgSender() when meta-transactions capabilities aren't used
• Use Custom Errors instead of Revert Strings to save Gas

Caching storage values in memory

The code can be optimized by minimising the number of SLOADs. SLOADs are expensive (100 gas) compared to MLOADs/MSTOREs (3 gas). Here, storage values should get cached in memory (see the @audit tags for further details):

contracts/ForgottenRunesWarriorsGuild.sol:
  100:         require(numMinted < MAX_WARRIORS, 'All warriors have been summoned');  //@audit gas: numMinted SLOAD 1 (should declare tokenId earlier and use it instead)
  102:         uint256 tokenId = numMinted; //@audit gas: numMinted SLOAD 2 (should be declared earlier)
  104:         numMinted += 1;  //@audit gas: numMinted SLOAD 3 (should be numMinted = tokenId + 1)

contracts/ForgottenRunesWarriorsMinter.sol:
  136:         require(numSold < maxDaSupply, 'Auction sold out'); //@audit gas: numSold SLOAD 1, maxDaSupply SLOAD 1
  137:         require(numSold + numWarriors <= maxDaSupply, 'Not enough remaining'); //@audit gas: numSold SLOAD 2, maxDaSupply SLOAD 2
  154:         numSold += numWarriors; //@audit gas: numSold SLOAD 3 (equivalent to numSold = numSold + numWarriors)
  156:         if (numSold == maxDaSupply) { //@audit gas: numSold SLOAD 4, maxDaSupply SLOAD 3
  177:         require(numSold < maxForSale, 'Sold out'); //@audit gas: numSold SLOAD 1
  193:         numSold += 1; //@audit gas: numSold SLOAD 2 (equivalent to numSold = numSold + 1)
  207:         require(numSold < maxForSale, 'Sold out'); //@audit gas: numSold SLOAD 1, maxDaSupply SLOAD 1
  208:         require(numSold + numWarriors <= maxForSale, 'Not enough remaining'); //@audit gas: numSold SLOAD 2, maxDaSupply SLOAD 2
  219:         numSold += numWarriors; //@audit gas: numSold SLOAD 3 (equivalent to numSold = numSold + numWarriors)
  234:         require(numClaimed < maxForClaim, 'No more claims'); //@audit gas: numSold SLOAD 1
  248:         numClaimed += 1; //@audit gas: numSold SLOAD 2 (equivalent to numSold = numSold + 1)
  279:         if (block.timestamp >= daStartTime + daPriceCurveLength) {//@audit gas: daStartTime SLOAD 1, daPriceCurveLength SLOAD 1
  284:         uint256 dropPerStep = (startPrice - lowestPrice) / //@audit gas: startPrice SLOAD 1, lowestPrice SLOAD 1
  285:             (daPriceCurveLength / daDropInterval); //@audit gas: daPriceCurveLength SLOAD 2, daDropInterval SLOAD 1
  287:         uint256 elapsed = block.timestamp - daStartTime;//@audit gas: daStartTime SLOAD 2
  288:         uint256 steps = elapsed / daDropInterval; //@audit gas: daDropInterval SLOAD 2
  292:         if (stepDeduction > startPrice) { //@audit gas: startPrice SLOAD 2
  293:             return lowestPrice; //@audit gas: lowestPrice SLOAD 2
  295:         uint256 currentPrice = startPrice - stepDeduction; //@audit gas: startPrice SLOAD 3
  296:         return currentPrice > lowestPrice ? currentPrice : lowestPrice;  //@audit gas: lowestPrice SLOAD 2 & 3
  401:             IWETH(weth).deposit{value: amount}(); //@audit gas: weth SLOAD 1
  402:             IERC20(weth).transfer(to, amount); //@audit gas: weth SLOAD 2
  609:         require(address(vault) != address(0), 'no vault'); //@audit gas: vault SLOAD 1
  610:         require(payable(vault).send(_amount)); //@audit gas: vault SLOAD 2
  617:         require(address(vault) != address(0), 'no vault'); //@audit gas: vault SLOAD 1
  618:         require(payable(vault).send(address(this).balance)); //@audit gas: vault SLOAD 2

Unchecking arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

I suggest wrapping L295 with an unchecked block (see @audit):

File: ForgottenRunesWarriorsMinter.sol
291:         // don't go negative in the next step
292:         if (stepDeduction > startPrice) {
293:             return lowestPrice;
294:         }
295:         uint256 currentPrice = startPrice - stepDeduction; //@audit gas: should be unchecked due to L292-L293

Unnecessary initialize() function

The initialize() function isn't an initializer. It just calls setMinter(), which has the same visibility and authorization level as initialize():

File: ForgottenRunesWarriorsGuild.sol
52:     function initialize(address newMinter) public onlyOwner { 
53:         setMinter(newMinter);
54:     }
...
137:     function setMinter(address newMinter) public onlyOwner {
138:         minter = newMinter;
139:     }

It could even be called repeatedly.

As the initialize() function is not needed, I suggest deleting it and directly calling setMinter() to "Conveniently initialize the contract".

ForgottenRunesWarriorsGuild.forwardERC20s() and ForgottenRunesWarriorsMinter.forwardERC20s(): Unnecessary require statements

Here, as the onlyOwner modifier is applied, the address(0) checks are not needed here:

contracts/ForgottenRunesWarriorsGuild.sol:
  173      function forwardERC20s(IERC20 token, uint256 amount) public onlyOwner {
  174:         require(address(msg.sender) != address(0)); //@audit gas: there's the onlyOwner modifier: msg.sender can't be address(0)
  175          token.transfer(msg.sender, amount);
  176      }

contracts/ForgottenRunesWarriorsMinter.sol:
  627      function forwardERC20s(IERC20 token, uint256 amount) public onlyOwner {
  628:         require(address(msg.sender) != address(0)); //@audit gas: there's the onlyOwner modifier: msg.sender can't be address(0)
  629          token.transfer(msg.sender, amount);
  630      }

I suggest removing these checks.

ForgottenRunesWarriorsMinter: bidSummon()and publicSummon(): Unnecessary require statement

The code is as such:

File: ForgottenRunesWarriorsMinter.sol
130:     function bidSummon(uint256 numWarriors)
131:         external
132:         payable
133:         nonReentrant
134:         whenNotPaused
135:     {
136:         require(numSold < maxDaSupply, 'Auction sold out');
137:         require(numSold + numWarriors <= maxDaSupply, 'Not enough remaining');
138:         require(daStarted(), 'Auction not started');
139:         require(!mintlistStarted(), 'Auction phase over');
140:         require(
141:             numWarriors > 0 && numWarriors <= 20,
142:             'You can summon no more than 20 Warriors at a time'
143:         );
...
201:     function publicSummon(uint256 numWarriors)
202:         external
203:         payable
204:         nonReentrant
205:         whenNotPaused
206:     {
207:         require(numSold < maxForSale, 'Sold out');
208:         require(numSold + numWarriors <= maxForSale, 'Not enough remaining');
209:         require(publicStarted(), 'Public sale not started');
210:         require(
211:             numWarriors > 0 && numWarriors <= 20,
212:             'You can summon no more than 20 Warriors at a time'
213:         );

Logically speaking, numSold + numWarriors <= maxForSale could only reach the edge-case if numWarriors == 0, but that's prevented with the condition that follows in both functions: numWarriors > 0 && numWarriors <= 20. Meaning that, with numSold + numWarriors <= maxForSale and numWarriors > 0, we don't need to check if numSold < maxForSale as it just can't happen.

I suggest removing the 2 require(numSold < maxDaSupply) checks L136 and L207.

Furthermore, notice that 'Not enough remaining' and 'Sold out' kinda mean the same thing, so the additionnal require statement might not be justified.

Boolean comparisons

Comparing to a constant (true or false) is a bit more expensive than directly checking the returned boolean value. I suggest using if(!directValue) instead of if(directValue == false) here:

ForgottenRunesWarriorsMinter.sol:182:        require(mintlistMinted[msg.sender] == false, 'Already minted');
ForgottenRunesWarriorsMinter.sol:238:        require(claimlistMinted[msg.sender] == false, 'Already claimed');

> 0 is less efficient than != 0 for unsigned integers (with proof)

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

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 proofs: https://twitter.com/gzeon/status/1485428085885640706

I suggest changing > 0 with != 0 here:

ForgottenRunesWarriorsMinter.sol:141:            numWarriors > 0 && numWarriors <= 20,
ForgottenRunesWarriorsMinter.sol:211:            numWarriors > 0 && numWarriors <= 20,

Also, please enable the Optimizer.

ForgottenRunesWarriorsMinter.currentDaPrice(): > should be >=

The return statement is as follows:

ForgottenRunesWarriorsMinter.sol:296:        return currentPrice > lowestPrice ? currentPrice : lowestPrice;

Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas)
Furthermore, lowestPrice is read from storage while currentPrice is read from memory.

Therefore, it's possible to always save 3 gas and sometimes further save 1 SLOAD (when currentPrice == lowestPrice) by replacing the code to:

ForgottenRunesWarriorsMinter.sol:296:        return currentPrice >= lowestPrice ? currentPrice : lowestPrice;

Splitting require() statements that use && saves gas

If you're using the Optimizer at 200, instead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement:

contracts/ForgottenRunesWarriorsMinter.sol:
  140          require(
  141:             numWarriors > 0 && numWarriors <= 20,
  142              'You can summon no more than 20 Warriors at a time'
  143          );

  210          require(
  211:             numWarriors > 0 && numWarriors <= 20,
  212              'You can summon no more than 20 Warriors at a time'
  213          );

++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

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

Instances include:

ForgottenRunesWarriorsGuild.sol:104:        numMinted += 1;
ForgottenRunesWarriorsMinter.sol:162:        for (uint256 i = 0; i < numWarriors; i++) {
ForgottenRunesWarriorsMinter.sol:193:        numSold += 1;
ForgottenRunesWarriorsMinter.sol:220:        for (uint256 i = 0; i < numWarriors; i++) {
ForgottenRunesWarriorsMinter.sol:248:        numClaimed += 1;
ForgottenRunesWarriorsMinter.sol:259:        for (uint256 i = 0; i < count; i++) {
ForgottenRunesWarriorsMinter.sol:355:        for (uint256 i = startIdx; i < endIdx + 1; i++) {

I suggest using ++i instead of i++ to increment the value of an uint variable.

Increments can be unchecked

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Instances include:

ForgottenRunesWarriorsMinter.sol:162:        for (uint256 i = 0; i < numWarriors; i++) {
ForgottenRunesWarriorsMinter.sol:220:        for (uint256 i = 0; i < numWarriors; i++) {
ForgottenRunesWarriorsMinter.sol:259:        for (uint256 i = 0; i < count; i++) {
ForgottenRunesWarriorsMinter.sol:355:        for (uint256 i = startIdx; i < endIdx + 1; i++) {

The code would go from:

for (uint256 i; i < numIterations; i++) {  
 // ...  
}  

to:

for (uint256 i; i < numIterations;) {  
 // ...  
 unchecked { ++i; }  
}  

The risk of overflow is inexistant for a uint256 here.

Public functions to external

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

initialize(address) should be declared external:
 - ForgottenRunesWarriorsGuild.initialize(address) (contracts/ForgottenRunesWarriorsGuild.sol#52-54)
exists(uint256) should be declared external:
 - ForgottenRunesWarriorsGuild.exists(uint256) (contracts/ForgottenRunesWarriorsGuild.sol#85-87)
setProvenanceHash(string) should be declared external:
 - ForgottenRunesWarriorsGuild.setProvenanceHash(string) (contracts/ForgottenRunesWarriorsGuild.sol#145-147)
withdrawAll() should be declared external:
 - ForgottenRunesWarriorsGuild.withdrawAll() (contracts/ForgottenRunesWarriorsGuild.sol#163-165)
forwardERC20s(IERC20,uint256) should be declared external:
 - ForgottenRunesWarriorsGuild.forwardERC20s(IERC20,uint256) (contracts/ForgottenRunesWarriorsGuild.sol#173-176)
numDaMinters() should be declared external:
 - ForgottenRunesWarriorsMinter.numDaMinters() (contracts/ForgottenRunesWarriorsMinter.sol#337-339)
issueRefunds(uint256,uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.issueRefunds(uint256,uint256) (contracts/ForgottenRunesWarriorsMinter.sol#350-358)
refundAddress(address) should be declared external:
 - ForgottenRunesWarriorsMinter.refundAddress(address) (contracts/ForgottenRunesWarriorsMinter.sol#364-366)
selfRefund() should be declared external:
 - ForgottenRunesWarriorsMinter.selfRefund() (contracts/ForgottenRunesWarriorsMinter.sol#371-374)
pause() should be declared external:
 - ForgottenRunesWarriorsMinter.pause() (contracts/ForgottenRunesWarriorsMinter.sol#427-429)
unpause() should be declared external:
 - ForgottenRunesWarriorsMinter.unpause() (contracts/ForgottenRunesWarriorsMinter.sol#434-436)
setSelfRefundsStartTime(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setSelfRefundsStartTime(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#469-471)
setPhaseTimes(uint256,uint256,uint256,uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setPhaseTimes(uint256,uint256,uint256,uint256) (contracts/ForgottenRunesWarriorsMinter.sol#480-500)
setMintlist1MerkleRoot(bytes32) should be declared external:
 - ForgottenRunesWarriorsMinter.setMintlist1MerkleRoot(bytes32) (contracts/ForgottenRunesWarriorsMinter.sol#505-507)
setMintlist2MerkleRoot(bytes32) should be declared external:
 - ForgottenRunesWarriorsMinter.setMintlist2MerkleRoot(bytes32) (contracts/ForgottenRunesWarriorsMinter.sol#513-515)
setClaimlistMerkleRoot(bytes32) should be declared external:
 - ForgottenRunesWarriorsMinter.setClaimlistMerkleRoot(bytes32) (contracts/ForgottenRunesWarriorsMinter.sol#520-522)
setStartPrice(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setStartPrice(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#550-552)
setLowestPrice(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setLowestPrice(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#557-559)
setDaPriceCurveLength(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setDaPriceCurveLength(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#564-566)
setDaDropInterval(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setDaDropInterval(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#571-573)
setFinalPrice(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setFinalPrice(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#579-581)
setMaxDaSupply(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setMaxDaSupply(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#586-588)
setMaxForSale(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setMaxForSale(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#593-595)
setMaxForClaim(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.setMaxForClaim(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#600-602)
withdraw(uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.withdraw(uint256) (contracts/ForgottenRunesWarriorsMinter.sol#608-611)
withdrawAll() should be declared external:
 - ForgottenRunesWarriorsMinter.withdrawAll() (contracts/ForgottenRunesWarriorsMinter.sol#616-619)
forwardERC20s(IERC20,uint256) should be declared external:
 - ForgottenRunesWarriorsMinter.forwardERC20s(IERC20,uint256) (contracts/ForgottenRunesWarriorsMinter.sol#627-630)

No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {

Instances include:

ForgottenRunesWarriorsGuild.sol:24:    uint256 public numMinted = 0;
ForgottenRunesWarriorsMinter.sol:162:        for (uint256 i = 0; i < numWarriors; i++) {
ForgottenRunesWarriorsMinter.sol:220:        for (uint256 i = 0; i < numWarriors; i++) {
ForgottenRunesWarriorsMinter.sol:259:        for (uint256 i = 0; i < count; i++) {

I suggest removing explicit initializations for default values.

Upgrade pragma to at least 0.8.4

Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.

The advantages here are:

• Low level inliner (>= 0.8.2): Cheaper runtime gas (especially relevant when the contract has small functions).
• Optimizer improvements in packed structs (>= 0.8.3)
• Custom errors (>= 0.8.4): cheaper deployment cost and runtime cost. Note: the runtime cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

Consider upgrading pragma to at least 0.8.4:

ForgottenRunesWarriorsGuild.sol:1:pragma solidity ^0.8.0;
ForgottenRunesWarriorsMinter.sol:1:pragma solidity ^0.8.0;

Use msg.sender instead of OpenZeppelin's _msgSender() when meta-transactions capabilities aren't used

msg.sender costs 2 gas (CALLER opcode). _msgSender() represents the following:

function _msgSender() internal view virtual returns (address payable) {
  return msg.sender;
}

When no meta-transactions capabilities are used: msg.sender is enough.

See https://docs.openzeppelin.com/contracts/2.x/gsn for more information about GSN capabilities.

Consider replacing _msgSender() with msg.sender here:

ForgottenRunesWarriorsGuild.sol:101:        require(_msgSender() == minter, 'Not a minter');
ForgottenRunesWarriorsGuild.sol:115:            _isApprovedOrOwner(_msgSender(), tokenId),

In the solution, msg.sender is used everywhere else:

ForgottenRunesWarriorsGuild.sol:164:        require(payable(msg.sender).send(address(this).balance));
ForgottenRunesWarriorsGuild.sol:174:        require(address(msg.sender) != address(0));
ForgottenRunesWarriorsGuild.sol:175:        token.transfer(msg.sender, amount);
ForgottenRunesWarriorsMinter.sol:113:        setVaultAddress(msg.sender);
ForgottenRunesWarriorsMinter.sol:151:        daMinters.push(msg.sender);
ForgottenRunesWarriorsMinter.sol:152:        daAmountPaid[msg.sender] += msg.value;
ForgottenRunesWarriorsMinter.sol:153:        daNumMinted[msg.sender] += numWarriors;
ForgottenRunesWarriorsMinter.sol:163:            _mint(msg.sender);
ForgottenRunesWarriorsMinter.sol:182:        require(mintlistMinted[msg.sender] == false, 'Already minted');
ForgottenRunesWarriorsMinter.sol:183:        mintlistMinted[msg.sender] = true;
ForgottenRunesWarriorsMinter.sol:186:        bytes32 node = keccak256(abi.encodePacked(msg.sender));
ForgottenRunesWarriorsMinter.sol:194:        _mint(msg.sender);
ForgottenRunesWarriorsMinter.sol:221:            _mint(msg.sender);
ForgottenRunesWarriorsMinter.sol:238:        require(claimlistMinted[msg.sender] == false, 'Already claimed');
ForgottenRunesWarriorsMinter.sol:239:        claimlistMinted[msg.sender] = true;
ForgottenRunesWarriorsMinter.sol:242:        bytes32 node = keccak256(abi.encodePacked(msg.sender));
ForgottenRunesWarriorsMinter.sol:249:        _mint(msg.sender);
ForgottenRunesWarriorsMinter.sol:373:        _refundAddress(msg.sender);
ForgottenRunesWarriorsMinter.sol:628:        require(address(msg.sender) != address(0));
ForgottenRunesWarriorsMinter.sol:629:        token.transfer(msg.sender, amount);

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances include:

ForgottenRunesWarriorsGuild.sol:68:        require(
ForgottenRunesWarriorsGuild.sol:100:        require(numMinted < MAX_WARRIORS, 'All warriors have been summoned');
ForgottenRunesWarriorsGuild.sol:101:        require(_msgSender() == minter, 'Not a minter');
ForgottenRunesWarriorsGuild.sol:114:        require(
ForgottenRunesWarriorsGuild.sol:164:        require(payable(msg.sender).send(address(this).balance));
ForgottenRunesWarriorsGuild.sol:174:        require(address(msg.sender) != address(0));
ForgottenRunesWarriorsMinter.sol:136:        require(numSold < maxDaSupply, 'Auction sold out');
ForgottenRunesWarriorsMinter.sol:137:        require(numSold + numWarriors <= maxDaSupply, 'Not enough remaining');
ForgottenRunesWarriorsMinter.sol:138:        require(daStarted(), 'Auction not started');
ForgottenRunesWarriorsMinter.sol:139:        require(!mintlistStarted(), 'Auction phase over');
ForgottenRunesWarriorsMinter.sol:140:        require(
ForgottenRunesWarriorsMinter.sol:146:        require(
ForgottenRunesWarriorsMinter.sol:177:        require(numSold < maxForSale, 'Sold out');
ForgottenRunesWarriorsMinter.sol:178:        require(mintlistStarted(), 'Mintlist phase not started');
ForgottenRunesWarriorsMinter.sol:179:        require(msg.value == finalPrice, 'Ether value incorrect');
ForgottenRunesWarriorsMinter.sol:182:        require(mintlistMinted[msg.sender] == false, 'Already minted');
ForgottenRunesWarriorsMinter.sol:187:        require(
ForgottenRunesWarriorsMinter.sol:207:        require(numSold < maxForSale, 'Sold out');
ForgottenRunesWarriorsMinter.sol:208:        require(numSold + numWarriors <= maxForSale, 'Not enough remaining');
ForgottenRunesWarriorsMinter.sol:209:        require(publicStarted(), 'Public sale not started');
ForgottenRunesWarriorsMinter.sol:210:        require(
ForgottenRunesWarriorsMinter.sol:214:        require(
ForgottenRunesWarriorsMinter.sol:234:        require(numClaimed < maxForClaim, 'No more claims');
ForgottenRunesWarriorsMinter.sol:235:        require(claimsStarted(), 'Claim phase not started');
ForgottenRunesWarriorsMinter.sol:238:        require(claimlistMinted[msg.sender] == false, 'Already claimed');
ForgottenRunesWarriorsMinter.sol:243:        require(
ForgottenRunesWarriorsMinter.sol:258:        require(address(recipient) != address(0), 'address req');
ForgottenRunesWarriorsMinter.sol:372:        require(selfRefundsStarted(), 'Self refund period not started');
ForgottenRunesWarriorsMinter.sol:488:        require(
ForgottenRunesWarriorsMinter.sol:492:        require(
ForgottenRunesWarriorsMinter.sol:609:        require(address(vault) != address(0), 'no vault');
ForgottenRunesWarriorsMinter.sol:610:        require(payable(vault).send(_amount));
ForgottenRunesWarriorsMinter.sol:617:        require(address(vault) != address(0), 'no vault');
ForgottenRunesWarriorsMinter.sol:618:        require(payable(vault).send(address(this).balance));
ForgottenRunesWarriorsMinter.sol:628:        require(address(msg.sender) != address(0));

I suggest replacing revert strings with custom errors.

[gzeoneth]

Most are valid, except

ForgottenRunesWarriorsMinter.currentDaPrice(): > should be >=

strict is cheaper since there is no opcode for non-strict comparison in evm

No need to explicitly initialize variables with default values

yes but I don't think it save gas in for loop with optimizer enabled