Skip to content

ERC721FloorBidMatcher.sol gas optimizations#

Informational

1. Packing ERC721FloorBidOrder Struct#

The numberOfTokens and endTime variables in the ERC721FloorBidOrder struct can use smaller data types. Doing this will cause less gas cost to read and write to the ERC721FloorBidOrder struct as fewer storage slots will be used.

I suggest the struct to be changed to the following: 

    struct ERC721FloorBidOrder {
        address erc721TokenAddress;
        uint16 numberOfTokens;
        uint256[] erc721TokenIdsSold;
        uint256 tokenPrice;
        address paymentTokenAddress;
        uint256 amount;
        uint32 endTime;
        address creator;
        ERC721FloorBidMatcher.OrderStatus orderStatus;
    }

2. NftTransferProxy and ERC20TransferProxy contracts increase gas costs#

The transfer proxy contracts add gas overhead to transferring tokens.

In the createBuyOrder an ERC20 transfer proxy adds the following overhead:

  1. The gas cost to call the approve external function which involves state variable reads and writes.
  2. The gas cost to call the erc20safeTransferFrom external function on the ERC20 proxy.
  3. Calling erc20safeTransferFrom on the ERC20 proxy causes the proxy to read a state variable and make a second external function call using delegatecall -- this is mechanics and gas overhead of the proxy implementation.
  4. The erc20safeTransferFrom function itself does a state read for operator authorization.

Direct ERC20 and ERC721 transfers are gas expensive. Using ERC20 transfer proxies increases the cost further. Since the Berlin hardfork last year the gas cost for an external function call increased from 700 gas to 2600 gas, and storage slot reads increased from 800 to 2100. But this is for the first access to an external contract or storage reads. Subsequent external calls to the same contracts and reads to the same storage slots cost 100 gas. So it makes sense to optimize away from external function calls and storage reads.

The NftTransferProxy contract is called in a loop in the matchBuyOrder function.

The createBuyOrder, matchBuyOrder, cancelOrder, withdrawFundsFromExpiredOrder functions are using transfer proxies.

The transfer proxy function calls can be replaced with direct transfer calls which will cost less gas for users.

Using a transfer proxy for NFTs has a benefit that it makes it easy to add support for non-standard ERC721 contracts and new NFT standards in the future. However ERC721FloorBidMatcher is an upgradeable contract and could be upgraded in the future to support other NFTs transfers directly.

One idea is to do a gas cost analysis of ERC721FloorBidMatcher.sol. Could compare the gas costs to using transfer proxies versus not using them, and find out or determine what is acceptable. Keep in mind that a project generally has no or very little control of gas prices on Ethereum. Gas prices may increase or decrease in the future.

Related issue here: Gas Optimize Transfers in TransferExecutor.sol #5

3. Not Necessary to Store amount variable in ERC721FloorBidOrder struct#

The numberOfTokens variable times the tokenPrice variable in the ERC721FloorBidOrder struct provide the same value as the amount variable, so the amount variable does not need to exist in the ERC721FloorBidOrder struct.

Removing the amount variable from the ERC721FloorBidOrder struct saves 20,000 gas in the createBuyOrder functions and 5,000 gas in the matchBuyOrder function.

I also suggest using the uint256 tokenPrice parameter in the createBuyOrder function instead of the uint256 amount parameter. Because the uint256 amount parameter can have a small rounding error when calculating uint256 tokenPrice.

4. State Variables Read Multiple Times#

The first time a state variable is read it costs 2100 gas. Subsequent reads of the same state variable in the same transaction cost 100 gas. Reading a local variable costs no or very little gas.

If a state variable will be used multiple times in the same function it will cost less gas if the state variable value is assigned to a local variable and the local variable is used.

In the matchBuyOrder function the order.erc721TokenAddress state variable is read twice within a loop.

I recommend assigning state variable values to local variables when they are used more than once in the same function, or when the same values are used in loops. This change can apply to the matchBuyOrder function and other functions that are reading the same state variables more than once in the same function or using the same state variables in loops.

5. Consider removing erc721TokenIdsSold from the ERC721FloorBidOrder struct#

The matchBuyOrder function adds each tokenId of an NFT that is sold to the order.erc721TokenIdsSold array. This adds a gas cost of 20,000 gas for each NFT that is sold. So if matchBuyOrder sold 20 NFTs this adds a gas cost of 20,000 * 20 which is 400,000 gas. At today's low price of gas of about 85 gwei that is $88.66 USD.

If possible I suggest removing the erc721TokenIdsSold variable from the ERC721FloorBidOrder struct to save gas. Other ways can be used to show what NFTs are sold. Block explorers show ERC721 transfers for both parties. The LogMatchBuyOrder event that is emitted shows what tokenIds were sold.

I suggest removal of erc721TokenIdsSold because it adds significant gas and is not necessary for the operation of the exchange smart contract logic. It is only records data that can be retrieved with the getSoldTokensFromOrder(uint256 orderId) function.

6. nonReentrant Modifier#

If the Checks-Effects-Interactions Pattern is used then some of the functions currently using the nonReentrant modifier don't need to use it. The nonReentrant modifier adds about 2244 gas to a function according to my testing.

Examples of functions that could use the Checks-Effects-Interactions Pattern pattern instead of the nonReentrant modifier: createBuyOrder, createBuyOrderETH, cancelOrder, withdrawFundsFromExpiredOrder.