Gas Optimizations

[code423n4]

Title: Consider make constant as private to save gas

Proof of Concept:
NFTDropMarketFixedPriceSale.sol#L70
MinterRole.sol#L19

Recommended Mitigation Steps:
I suggest changing the visibility from public to internal or private

---

Title: Expression for constant values such as a call to keccak256(), should use immutable rather than constant

Proof of Concept:
NFTDropMarketFixedPriceSale.sol#L70
MinterRole.sol#L19

Recommended Mitigation Steps:
Change from constant to immutable
reference: here

---

Title: Comparison operators

Proof of Concept:
NFTDropMarketFixedPriceSale.sol#L240

Recommended Mitigation Steps:
Replace <= with <, and >= with > for gas optimization

---

Title: Use unchecked can save gas

Proof of Concept:
NFTDropMarketFixedPriceSale.sol#L245 (because of if() L#240)

Recommended Mitigation Steps:
Use unchecked

---

Title: Reduce the size of error messages (Long revert Strings)

Impact:
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.
Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept:
AddressLibrary.sol#L31
NFTCollectionFactory.sol#L227
NFTCollectionFactory.sol#L262

Recommended Mitigation Steps:
Consider shortening the revert strings to fit in 32 bytes

---

Title: Custom errors from Solidity 0.8.4 are cheaper than 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) while providing the same amount of information

Custom errors are defined using the error statement
reference: https://blog.soliditylang.org/2021/04/21/custom-errors/

Proof of Concept:
AddressLibrary.sol#L31
NFTCollectionFactory.sol#L227
NFTCollectionFactory.sol#L262

Recommended Mitigation Steps:
Replace require statements with custom errors.

---

Title: Using != in require statement is more gas efficient

Proof of Concept:
NFTDropCollection.sol#L130-L131

Recommended Mitigation Steps:
Change > 0 to != 0

---

[batu-inal]

Consider make constant as private to save gas

Agree but won't fix. For ease of use and consistency we will continue to expose some constants publicly.

Expression for constant values such as a call to keccak256(), should use immutable rather than constant

Valid but won't fix. While there may be gas savings here we like the readability and maintainability of this pattern. Adding hard-coded hashes makes the code fragile and harder to read.

Comparison operators

Invalid. This is semantically different and violates the boundary case.

Use unchecked can save gas

Valid. We will make this change.

Reduce the size of error messages (Long revert Strings)

Agree but won't fix. We use up to 64 bytes, aiming to respect the incremental cost but 32 bytes is a bit too short to provide descriptive error messages for our users.

Custom errors from Solidity 0.8.4 are cheaper than revert strings

Agree but won't fix at this time. We use these in the market but not in collections. Unfortunately custom errors are still not as good of an experience for users (e.g. on etherscan). We used them in the market originally because we were nearing the max contract size limit and this was a good way to reduce the bytecode. We'll consider this in the future as tooling continues to improve.

Using != in require statement is more gas efficient

Invalid. We tested the recommendation and got the following results:

createNFTDropCollection gas reporter results:

  using > 0 (current):
    - 319246  ·     319578  ·      319361
  using != 0 (recommendation):
    -  319252  ·     319584  ·      319367
  impact: +6 gas