Gas Optimizations - Githubissues

PercentSplitETH.sol (Out of Scope)
BytesLibrary.sol (Out of Scope)
NFTCollectionFactory.sol
NFTDropCollection.sol
MarketFees.sol
NFTDropMarketFixedPriceSale.sol
SequentialMintCollection.sol
Use custom errors instead of require
CollectionRoyalties.sol
= 0 assignment can be avoided for default values

1. contracts/PercentSplitETH.sol (Out of Scope)

Can be improved a lot. Since out of scope, did not allocate time to write a report here.

2. contracts/libraries/BytesLibrary.sol (Out of Scope)

2.1 `replaceAtIf` function

We can avoid abi.encodePacked and also the loop in replaceAtIf function, by implementing it in an assembly block. Also note that the assertion and replacement each can be done at once. So the lines 21-32 can be changed to:

assembly {
    // 0x14 = 0x20 - 0x0c = 12 index to the right of `startLocation`
    // so that `dataChunk` has the right most 20 bytes as the potential
    // `expectedAddress`.
    let dataPtr := add(data, add(0x14, startLocation))
    let dataChunk := mload(dataPtr)
    if (eq(expectedAddress, and(dataChunk, expectedAddress))) {
        // the 12 left most bytes of `maskedDataChunk` will be the same as
        // `dataChunk` and the rest would be 0.
        let maskedDataChunk = and(
            dataChunk,
            0xffffffffffffffffffffffff0000000000000000000000000000000000000000
        )
        mstore(dataPtr, or(
            maskedDataChunk,
            newAddress
        ))
    } else {
        // 0x370d3a6a00000000000000000000000000000000000000000000000000000000
        // is the bytes32 right padded signature of BytesLibrary_Expected_Address_Not_Found()
        mstore(0, 0x370d3a6a00000000000000000000000000000000000000000000000000000000)
        revert(0, 0x20)
    }
}

2.2 `startsWith` function

Again, we can avoid the loop, and also multiple memory reads. Here is the implementation using assembly blocks:


function startsWith(bytes memory callData, bytes4 functionSig) internal pure returns (bool doesStartsWith) {
  // A signature is 4 bytes long
  if (callData.length < 4) {
    return false;
  }

  assembly {
    // peek 4 bytes after the memory pointer
    let callDataChunk := mload(add(callData, 4))
    doesStartsWith := eq(
      and(callDataChunk, functionSig),
      functionSig
    )
  }
}

startsWith is only used in contracts/PercentSplitETH.sol for the external function proxyCall. So the gas savings would be noticed there.

3. contracts/NFTCollectionFactory.sol

3.1 `adminUpdateNFTCollectionImplementation` function

The unchecked block in adminUpdateNFTCollectionImplementation can be rewritten as

unchecked {
  // Version cannot overflow 256 bits.
-  versionNFTCollection++;
+  ++versionNFTCollection;
}

Also the versionNFTCollection.toString() which is passed to INFTCollectionInitializer(_implementation).initialize can be factored out into a variable to avoid the toString() conversion twice.

string memory version = versionNFTCollection.toString();

// The implementation is initialized when assigned so that others may not claim it as their own.
  INFTCollectionInitializer(_implementation).initialize(
    payable(address(rolesContract)),
    string.concat("NFT Collection Implementation v", version),
    string.concat("NFTv", version)
  );

4. contracts/NFTDropCollection.sol

The loop (L181-L186) in mintCountTo of NFTDropCollection can be gas optimized by moving the loop condition inside the loop block:

for (uint256 i = firstTokenId; ; ) { 
  _mint(to, i);
  unchecked {
    ++i;
    if (i > latestTokenId) {
      break;
    }
  }
}

By moving the condition inside and modifying it to i > latestTokenId instead of i <= latestTokenId, the compiler would translate it into only one comparison (gt) versus two (lt or eq). Therefore, it would say gas.

Note: with this change, the loop would run at least once, but this would also be the case in the original code, since

$$ \text{firstTokenId} = \text{latestTokenId} + 1 \le \text{latestTokenId} + \text{count} = \text{latestTokenId'} $$

And this is because since we are passed line 172, we know that count is greater than 0.

5. contracts/mixins/shared/MarketFees.sol

The unchecked loop block in the _distributeFunds (line 126-135) can be optimized by caching creatorRecipients.length, it will avoid multiple mloads per iteration. So here is how it would look like after modification (also note the @audit comments):

unchecked {
  uint256 i; //@audit we don't need to assign 0, since 0 is the default value.
  uint256 creatorRecipientsLength = creatorRecipients.length;
  for (; i < creatorRecipientsLength; ) {
    _sendValueWithFallbackWithdraw(
      creatorRecipients[i],
      creatorShares[i],
      SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS
    );
    // Sum the total creator rev from shares
    // creatorShares is in ETH so creatorRev will not overflow here.
    creatorRev += creatorShares[i];
    ++i;
  }
}

The same type of optimization can be applied to the for loop block in getFeesAndRecipients function on line 198-200

uint256 i; //@audit we don't need to assign 0, since 0 is the default value.
uint256 creatorSharesLength = creatorShares.length;
for (; i < creatorSharesLength; ) {
  creatorRev += creatorShares[i];
  ++i;
}

`internalGetImmutableRoyalties` function

address payable[] memory recipients, uint256[] memory splitPerRecipientInBasisPoints can be combined into 1 dynamic array, it would save memory and therefore gas. IGetRoyalties would also need to be modified.

6. contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol

In mintFromFixedPriceSale we can cache saleConfig.limitPerAccount (line 180) on the stack which has been used 3 times. This would remove 2 extra mloads.

FixedPriceSaleConfig memory saleConfig = nftContractToFixedPriceSaleConfig[nftContract];
uint256 limitPerAccount = uint256(saleConfig.limitPerAccount); //@audit use this variable from this point on instead of `saleConfig.limitPerAccount`

7. contract/mixins/collections/SequentialMintCollection.sol

In _updateMaxTokenId we can change the condition in the 3rd require from

Before:

require(latestTokenId <= _maxTokenId, "SequentialMintCollection: Max token ID must be >= last mint");

To:

require(!(latestTokenId > _maxTokenId), "SequentialMintCollection: Max token ID must be >= last mint");

aka:

- latestTokenId <= _maxTokenId
+ !(latestTokenId > _maxTokenId)

Since EVM doesn't have a less than or equal opcode the compiler has to translate that into 2 comparisons LT and EQ which both require 2 stack items and both results have to be true to pass. We can save gas here by using GT and negate the result by NOT opcode which would only require the result on the stack from GT.

8. Use custom errors instead of require

The following require statements can be turned into custom errors to save gas. The list of locations require statement can be changed to custom errors:

9. contracts/mixins/collections/CollectionRoyalties.sol

In CollectionRoyalties, the functions getFeeRecipients, getFeeBps, getRoyalties only return arrays of length 1. It would be best to redesign some contracts so that these return values can be instead just values and not arrays. This would prevent using memory and the gas costs associated with using memory (MLOAD, MSTORE, and memory expansion gas costs).

10. `= 0` assignment can be avoided for default values

Since value types have by default the value of 0, explicit 0 assignment can be avoided which actually would require more gas.

Example:

- uint256 i = 0;
+ uint256 i;

List of locations where this change can be applied:

code-423n4 / 2022-08-foundation-findings

Gas Optimizations #187

Table of Contents

1. contracts/PercentSplitETH.sol (Out of Scope)

2. contracts/libraries/BytesLibrary.sol (Out of Scope)

2.1 `replaceAtIf` function

2.2 `startsWith` function

3. contracts/NFTCollectionFactory.sol

3.1 `adminUpdateNFTCollectionImplementation` function

4. contracts/NFTDropCollection.sol

5. contracts/mixins/shared/MarketFees.sol

`internalGetImmutableRoyalties` function

6. contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol

7. contract/mixins/collections/SequentialMintCollection.sol

8. Use custom errors instead of require

9. contracts/mixins/collections/CollectionRoyalties.sol

10. `= 0` assignment can be avoided for default values

code-423n4 / 2022-08-foundation-findings

Gas Optimizations #187

Table of Contents

1. contracts/PercentSplitETH.sol (Out of Scope)

2. contracts/libraries/BytesLibrary.sol (Out of Scope)

2.1 replaceAtIf function

2.2 startsWith function

3. contracts/NFTCollectionFactory.sol

3.1 adminUpdateNFTCollectionImplementation function

4. contracts/NFTDropCollection.sol

5. contracts/mixins/shared/MarketFees.sol

internalGetImmutableRoyalties function

6. contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol

7. contract/mixins/collections/SequentialMintCollection.sol

8. Use custom errors instead of require

9. contracts/mixins/collections/CollectionRoyalties.sol

10. = 0 assignment can be avoided for default values

2.1 `replaceAtIf` function

2.2 `startsWith` function

3.1 `adminUpdateNFTCollectionImplementation` function

`internalGetImmutableRoyalties` function

10. `= 0` assignment can be avoided for default values