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

⚡️ Optimize ERC721 jumps #184

Merged

Conversation

wilsoncusack
Copy link

Description

Rather than putting to.code.length == 0 in a OR conditional in the require, we can optimize by pulling this out of the require and wrapping the require in if (to.code.length != 0)

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • Ran dapp snapshot?
  • Ran npm run lint?
  • Ran forge test?
  • Ran dapp test?

Pull requests with an incomplete checklist will be thrown out.
lol ok but I don't have Dapp setup on my computer

@wilsoncusack
Copy link
Author

Going to close this because I can't perform the check list, but it's here if you want it!

@transmissions11
Copy link
Owner

haha ill be removing dapp support entirely soon

@transmissions11
Copy link
Owner

lol its a bit upsetting this saves gas

@transmissions11 transmissions11 changed the title Save a lil gas in ERC721 "safe" functions ⚡️ Optimize ERC721 jumps May 13, 2022
@transmissions11 transmissions11 changed the base branch from main to v7 May 13, 2022 04:58
@transmissions11 transmissions11 merged commit 4946f52 into transmissions11:v7 May 13, 2022
@0xpranay 0xpranay mentioned this pull request May 15, 2022
3 tasks
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.

2 participants