-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Optimized ERC721 transfers. #1539
Conversation
For reference, a call to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Save for my Transfer
comment above, the logic LGTM.
Coverage dropped since a condition of I'll check coveralls and see if we're missing a setting, since this should've been a test failure. |
Definitely. Created a v2.2 milestone. |
…merable behavior.
@frangio please review the new |
Created #1541 to track future work that could be done here (don't want to increase the scope of this PR). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to _transferFrom
.
I have huge doubts about the new _removeTokenFromOwnerEnumeration
function. See comments below.
There seems to be missing coverage in ERC721. https://coveralls.io/jobs/42933704/source_files/440498540 |
We should be good now - due to the original refactor @frangio should be ready to merge now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Credit goes to @abandeali1 for coming up with the original idea in #1031.
Because Solidity doesn't inline
internal
calls, it misses the opportunity to optimize consecutiveSSTORE
s to the same location, leading to extra gas costs in the currentERC721
implementation (wheretransfer
is implemented asremoveFrom
followed byaddTo
). This PR adds a new function that encapsulates this behavior, implementing the optimization directly on the source code.