erc721m icon indicating copy to clipboard operation
erc721m copied to clipboard

Decrease Batch Transfer gas usage 7%, reduce contract deployment gas by 21%

Open tenthirtyone opened this issue 6 months ago • 4 comments

Description

This change removes an ownership check and relies on the 721 standard's internal ownership check to enforce token ownership and reduces variable declarations. It increases gas usage effectiveness by ~7%. The previous max number of batchTransferToSingleWallet transfers was 777 under ideal conditions. With these changes 834 batch transfers are possible in the same amount of gas.

The code removals also reduce the amount of gas needed to deploy from 499,259 to 390,851 ~ 21%. You can test the deployment gas changes by changing this line to be:

    const ERC721BatchTransferFactory = await ethers.getContractFactory('ERC721BatchTransfer');
    const batchTransfer = await ERC721BatchTransferFactory.deploy();
    const txReceipt = await batchTransfer.deployed();
    console.log(`Gas used for deploying ERC721BatchTransfer: ${txReceipt.deployTransaction.gasLimit.toString()}`);

To Test

There is a new, skipped, unit test that pushes the batchTransferToSingleWallet function to the HH gas limit. Change .skip to .only or just remove .skip but the test will take ~10s to use up the entire block gas.

Notes

This also removes a false positive high risk from slither output. Slither doesn't like arbitrary from addresses and prefers to see msg.sender as the from address. The contracts were correctly using msg.sender but were instantiating a variable to do so.

tenthirtyone avatar Jul 30 '24 00:07 tenthirtyone