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

Remove ERC1155 hooks #3876

Merged

Conversation

JulissaDantes
Copy link
Contributor

@JulissaDantes JulissaDantes commented Dec 13, 2022

Fixes #3535

The current way to customize token transfers(mint, burn, transfer) is by overriding the hooks _beforeTokenTransfer and _afterTokenTransfer. This refactor removes both hooks from the ERC1155 contract, and implements the logic inside the new _update internal function, so future customizations only rely on overriding a single entry point.

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@TrejGun
Copy link

TrejGun commented Dec 19, 2022

what about erc721? will it be different from 20 and 1155?

@JulissaDantes
Copy link
Contributor Author

what about erc721? will it be different from 20 and 1155?

Thanks for showing interest. It will be worked on after this to use the same _update pattern instead of multiple hooks.

@frangio frangio changed the title Remove ERC1155 TokenTransfer hooks Remove ERC1155 hooks Dec 19, 2022
@frangio
Copy link
Contributor

frangio commented Dec 19, 2022

@TrejGun Keep in mind that this is planned for 5.0 release which is still months away.

Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged master into next-v5.0 which included some significant changes due to #3898. Please merge next-v5.0 into this PR as there may be conflicts.

contracts/token/ERC1155/ERC1155.sol Outdated Show resolved Hide resolved
contracts/token/ERC1155/ERC1155.sol Show resolved Hide resolved
@frangio frangio requested a review from Amxx January 5, 2023 22:20
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
This was referenced Sep 10, 2024
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.

5 participants