Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tenthirtyone
Copy link

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 tenthirtyone changed the title Increase Batch Mint gas usage 7%, reduce contract deployment gas by 21% Increase Batch Transfer gas usage 7%, reduce contract deployment gas by 21% Jul 30, 2024
@channing-magiceden
Copy link
Contributor

@tenthirtyone tenthirtyone changed the title Increase Batch Transfer gas usage 7%, reduce contract deployment gas by 21% Decrease Batch Transfer gas usage 7%, reduce contract deployment gas by 21% Jul 31, 2024
@nothing0012
Copy link
Contributor

@tenthirtyone can you rebase?

channing-magiceden and others added 4 commits August 27, 2024 14:58
* Clear _totalMintFee in withdraw

* Fix more
* Add authorizedMint and whitelist Reservoir Relay EOA

* Add ability to change authorizedMinter list

* pump version
@tenthirtyone
Copy link
Author

@nothing0012 good to go 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants