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

Fix ERC4906 compliance of ERC721URIStorage #4055

Merged
merged 2 commits into from
Feb 17, 2023

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Feb 17, 2023

As pointed out by @pcaversaccio in this post

#4012 missed the ERC165 part of ERC4906. Its unclear how this value was produced, and why an ERC that doesn't include any function (only event) would have an ERC165 interfaceId.

Still, this PR makes ERC721URIStorage compliant with the ERC165 part of ERC4906.

PR Checklist

  • Tests

@changeset-bot
Copy link

changeset-bot bot commented Feb 17, 2023

⚠️ No Changeset found

Latest commit: 8c63848

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Amxx Amxx requested a review from frangio February 17, 2023 14:21
@Amxx Amxx added this to the 4.9 milestone Feb 17, 2023
@frangio frangio merged commit 08d085f into OpenZeppelin:master Feb 17, 2023
@Amxx Amxx deleted the fix/erf4906/supportsInterface branch February 17, 2023 17:55
@pcaversaccio
Copy link
Contributor

Its unclear how this value was produced

Well, the 0x49064906 comes from the EIP number lol. Twice 4906 simply.

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

Successfully merging this pull request may close these issues.

None yet

3 participants