Gas Optimizations

[code423n4]

Gas

1. Contracts in scope are using unlocked pragma.

Impact

All the contracts in scope use pragma solidity ^0.8.0, allowing wide enough range of versions.

Proof of Concept

 ^0.8.0 (contracts/ForgottenRunesWarriorsGuild.sol#1)
 ^0.8.0 (contracts/ForgottenRunesWarriorsMinter.sol#1)

Recommendation

Consider locking compiler version, for example pragma solidity 0.8.6.
This can have additional benefits, for example using custom errors to save gas and so forth.

2. Use != 0 instead of > 0 for Unsigned Integer Comparison.

Impact

!= 0 is cheapear than > 0 when comparing unsigned integers in require statements.

Proof of Concept

  /ForgottenRunesWarriorsMinter.sol::141 => numWarriors > 0 && numWarriors <= 20,
  /ForgottenRunesWarriorsMinter.sol::211 => numWarriors > 0 && numWarriors <= 20,
  /ForgottenRunesWarriorsMinter.sol::378 => if (owed > 0) {

Recommendation

Use != 0 instead of > 0.

3. No need to initialize variables with default values.

Impact

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Proof of Concept

  /ForgottenWarriorsGuild.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++) {

Recommendation

Remove explicit zero initialization.

4. publicFunctions can beexternal`.

Impact

public functions that are never called by the contract should be declared external to save gas.

Proof of Concept

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)
mint(address) should be declared external:
        - ForgottenRunesWarriorsGuild.mint(address) (contracts/ForgottenRunesWarriorsGuild.sol#94-106)
burn(uint256) should be declared external:
        - ForgottenRunesWarriorsGuild.burn(uint256) (contracts/ForgottenRunesWarriorsGuild.sol#113-119)
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)

Recommendation

Use the external attribute for functions never called from the contract.

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

Impact

++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.

Proof of Concept

  /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++) {

Recommendation

Use ++i instead of i++ to increment the value of an uint variable.
Same thing for --i and i--.

6. Use Custom Errors instead of Revert Strings.

Impact

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

Recommendation

Use custom errors instead of revert strings.

Tools used

c4udit, manual, slither.