Gas Optimizations

[code423n4]

Cache storage values

Storage variables that are re-read in the same function can be cached to avoid expensive SLOAD and save gas

Proof of concept

versionNFTCollection in

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L217

    emit ImplementationNFTCollectionUpdated(_implementation, versionNFTCollection);

latestTokenId in

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L176-L179

    unchecked {
      // If +1 overflows then +count would also overflow, unless count==0 in which case the loop would exceed gas limits
      firstTokenId = latestTokenId + 1;
    }

    latestTokenId = latestTokenId + count;  
    require(latestTokenId <= maxTokenId, "NFTDropCollection: Exceeds max tokenId"); 

Cache function calls

Functions that are called multiple times in the same function can be cached and re-used to save gas

Proof of concept

versionNFTCollection.toString() in

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L213

      string.concat("NFT Collection Implementation v", versionNFTCollection.toString()),
      string.concat("NFTv", versionNFTCollection.toString())

versionNFTDropCollection.toString() in

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L240

      string.concat("NFT Drop Collection Implementation v", versionNFTDropCollection.toString()), 
      string.concat("NFTDropV", versionNFTDropCollection.toString()),

Use function arguments instead of storage variables in events

In functions use arguments and local variables instead of storage variables in emitting events to avoid expensive storage read operations

Proof of concept

baseURI,postRevealBaseURIHash can be replaced by function arguments

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L242

    emit URIUpdated(baseURI, postRevealBaseURIHash);

nftContract, saleConfig.seller, saleConfig.price, saleConfig.limitPerAccount in

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L156

    emit CreateFixedPriceSale(nftContract, saleConfig.seller, saleConfig.price, saleConfig.limitPerAccount);

Use custom errors instead of revert strings

Custom errors from solidity 0.8.4 are more efficient than revert strings with cheaper deployment and runtime time costs when revert condition is met

Refer: https://blog.soliditylang.org/2021/04/21/custom-errors/](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).

Variables less than 32 bytes

As EVM operates on 256 bits at a time, using a variable less than 256 bits costs more as additional operation has to be performed to reduce variable to the desired size

Refer: https://docs.soliditylang.org/en/v0.8.0/internals/layout_in_storage.html#layout-of-state-variables-in-storage

Proof of concept

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L80

  uint32 public versionNFTCollection; 

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L95

  uint32 public versionNFTDropCollection; 

Use != 0 instead of != 0 for unsigned integers

Unsigned integers will never have value less than 0, so checking != 0 than > 0 costs less gas

Proof of concept

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L88

    require(bytes(_baseURI).length > 0, "NFTDropCollection: `_baseURI` must be set"); 

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L130

    require(_maxTokenId > 0, "NFTDropCollection: `_maxTokenId` must be set");

Cache array length outside loop

Array length can be cached and used in the loop condition instead of reading it in every iteration and save gas

Proof of concept

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L126

      for (uint256 i = 0; i < creatorRecipients.length; ++i)

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L198

    for (uint256 i = 0; i < creatorShares.length; ++i) {  

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L484

    for (uint256 i = 0; i < creatorRecipients.length; ++i)

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L503

    for (uint256 i = 1; i < creatorRecipients.length; )

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L181

    for (uint256 i = firstTokenId; i <= latestTokenId; ) 

Constant variables can be internal

Constant variables can be declared internal to save gas as it can avoid creating a non-payable getter function and not adding another entry to the method ID table

Proof of concept

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L70

  bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/roles/MinterRole.sol#L19

  bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");   

Reduce size of revert strings

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

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L173

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L182

    require(rolesContract.isAdmin(msg.sender), "NFTCollectionFactory: Caller does not have the Admin role");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L203

    require(_implementation.isContract(), "NFTCollectionFactory: Implementation is not a contract");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L227

    require(_implementation.isContract(), "NFTCollectionFactory: Implementation is not a contract");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L262

    require(bytes(symbol).length != 0, "NFTCollectionFactory: Symbol is required");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L158

    require(tokenCreatorPaymentAddress != address(0), "NFTCollection: tokenCreatorPaymentAddress is required");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L263

    require(bytes(tokenCID).length != 0, "NFTCollection: tokenCID is required"); 
    require(!cidToMinted[tokenCID], "NFTCollection: NFT was already minted");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L268

require(maxTokenId == 0 || tokenId <= maxTokenId, "NFTCollection: Max token count has already been minted");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L327

    require(_exists(tokenId), "NFTCollection: URI query for nonexistent token");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L88

    require(bytes(_baseURI).length > 0, "NFTDropCollection: `_baseURI` must be set");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L93

    require(postRevealBaseURIHash != bytes32(0), "NFTDropCollection: Already revealed");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L130

    require(bytes(_symbol).length > 0, "NFTDropCollection: `_symbol` must be set"); 
    require(_maxTokenId > 0, "NFTDropCollection: `_maxTokenId` must be set");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L172

    require(count != 0, "NFTDropCollection: `count` must be greater than 0");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L179

    require(latestTokenId <= maxTokenId, "NFTDropCollection: Exceeds max tokenId");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L238

    require(_postRevealBaseURIHash != bytes32(0), "NFTDropCollection: use `reveal` instead");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/collections/SequentialMintCollection.sol#L58

    require(msg.sender == owner, "SequentialMintCollection: Caller is not creator");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/collections/SequentialMintCollection.sol#L74

    require(_creator != address(0), "SequentialMintCollection: Creator cannot be the zero address");  

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/collections/SequentialMintCollection.sol#L87

    require(_maxTokenId != 0, "SequentialMintCollection: Max token ID may not be cleared");
    require(maxTokenId == 0 || _maxTokenId < maxTokenId, "SequentialMintCollection: Max token ID may not increase");
    require(latestTokenId <= _maxTokenId, "SequentialMintCollection: Max token ID must be >= last mint");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/ContractFactory.sol#L22

    require(msg.sender == contractFactory, "ContractFactory: Caller is not the factory"); 

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/ContractFactory.sol#L31

    require(_contractFactory.isContract(), "ContractFactory: Factory is not a contract"); 

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/treasury/AdminRole.sol#L20

    require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "AdminRole: caller does not have the Admin role");

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/roles/AdminRole.sol#L19

    require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "AdminRole: caller does not have the Admin role"); 

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/roles/MinterRole.sol#L22

    require(isMinter(msg.sender) || isAdmin(msg.sender), "MinterRole: Must have the minter or admin role"); 

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/libraries/AddressLibrary.sol#L31

    require(contractAddress.isContract(), "InternalProxyCall: did not return a contract"); 

[HardlyDifficult]

Cache storage values

Agree, we'll look at making changes here.

Cache function calls

Agree that could help, but we are not concerned with optimizing admin-only functions.

Use function arguments instead of storage variables in events

Agree, fixed

Custom errors

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.

uints/ints smaller than 32 bytes (256 bits) incurs overhead.

Potentially valid but won't fix. This is uint32 to allow packing storage for a significant gas savings. Accepting the input as uint256 would incur additional overhead in checking for overflow before saving these values. Other inputs are logically limited to the size exposed, huge values cannot be used. Accepting the input as uint256 would incur additional overhead in checking for overflow before using these values.

Use != 0 instead of > 0

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

Cache Array Length Outside of Loop

May be theoretically valid, but won't fix. I tested this: gas-reporter and our gas-stories suite is reporting a small regression using this technique. It also hurts readability a bit so we wouldn't want to include it unless it was a clear win.

constants expressions are expressions, not constants

While it seems this should help, changing to immutable shows a regression in our test suite. Gas costs go up by ~50 gas.

Use short error messages

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.