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

Golfing ERC721A #270

Closed
0xPhaze opened this issue May 8, 2022 · 2 comments · Fixed by #272
Closed

Golfing ERC721A #270

0xPhaze opened this issue May 8, 2022 · 2 comments · Fixed by #272

Comments

@0xPhaze
Copy link

0xPhaze commented May 8, 2022

Hey,

I tried bringing the costs down for ERC721A. Here's a gas report with a small readme:
https://github.com/0xPhaze/ERC721A-Improved

  • Internally, tokenData is passed as storage instead of memory.
  • Keeps track of whether the next token data has already been set before, so that we don't have to check it (1 cold sload) on every transfer for all eternity.
  • Saves (warm) sload on variables such as collectionSize and maxPerWallet.
Function Gas
mint 1
test_mint1_ERC721A() 59713
test_mint1_ERC721AX() 59333
mint 5
test_mint5_ERC721A() 67486
test_mint5_ERC721AX() 67131
transfer 1
test_transferFrom1_ERC721A() 73837
test_transferFrom1_ERC721AX() 49435
transfer 5
test_transferFrom5_ERC721A() 154195
test_transferFrom5_ERC721AX() 127657

Wanted to ask first, if ERC721A is interested having some of these optimizations implemented.

Vectorized added a commit to Vectorized/ERC721A that referenced this issue May 9, 2022
@Vectorized Vectorized linked a pull request May 9, 2022 that will close this issue
@Vectorized
Copy link
Collaborator

Vectorized commented May 9, 2022

@0xPhaze I've added in #272. Will credit you in the next release. Big thanks!

I've opened a PR to your repo explaining the issue with using storage modifer in your case.

@0xPhaze
Copy link
Author

0xPhaze commented May 9, 2022

Awesome, thanks!
Although, I think that doesn't include the 'nextTokenDataSet' boolean which is a big part of the optimization. Instead of checking whether the next packed ownership has been set already by loading its storage, this would keep track of whether it has already previously been set and save that sload for all future transfers. And it can directly be assumed to be set (meaning we never have to check) this if the user only minted one.

Vectorized added a commit to Vectorized/ERC721A that referenced this issue May 9, 2022
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 a pull request may close this issue.

2 participants