ERC721FloorBidMatcher.sol gas optimizations#
1. Packing ERC721FloorBidOrder Struct#
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:
2. NftTransferProxy and ERC20TransferProxy contracts increase gas costs#
The transfer proxy contracts add gas overhead to transferring tokens.
createBuyOrder an ERC20 transfer proxy adds the following overhead:
- The gas cost to call the
approveexternal function which involves state variable reads and writes.
- The gas cost to call the
erc20safeTransferFromexternal function on the ERC20 proxy.
erc20safeTransferFromon 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.
erc20safeTransferFromfunction 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.
NftTransferProxy contract is called in a loop in the
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
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
amount variable from the
ERC721FloorBidOrder struct saves 20,000 gas in the
createBuyOrder functions and 5,000 gas in the
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
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.
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#
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