Skip to content

feat: add ERC7572 interface for contracts that expose contract level metadata #5686

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

coffeexcoin
Copy link

@coffeexcoin coffeexcoin commented May 14, 2025

This PR introduces the interface for draft ERC-7572 to the set of interfaces in the package.

This interface is useful for contracts (particularly tokens) that wish to expose a set of metadata for the contract including icon, banner image, description, external url, etc.

PR Checklist

  • Tests (No tests are necessary as this is purely an interface spec)
  • Documentation (Natspec documentation is applied to the added interface)
  • Changeset entry (run npx changeset add)

Copy link

changeset-bot bot commented May 14, 2025

🦋 Changeset detected

Latest commit: dc731cd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

Hi @coffeexcoin,

If I remember correctly, this interface was discussed in the context of #5394. I think we would need to use the interface in 6909 if we approve this PR

@coffeexcoin
Copy link
Author

Hi @coffeexcoin,

If I remember correctly, this interface was discussed in the context of #5394. I think we would need to use the interface in 6909 if we approve this PR

Happy to make those changes - what would be the preferred method, just make 6909ContentURI implement this interface?

@coffeexcoin
Copy link
Author

Also happy to add erc20/721/1155 extensions that implement this if that would be desirable

@ernestognw
Copy link
Member

I'd keep it only for 6909 for now. Other standards like ERC721 and ERC1155 define their own metadata functions. Probably that's why we decided not to include this interface in the first place

cc @arr00

@arr00
Copy link
Contributor

arr00 commented May 26, 2025

@ernestognw do you think it's ok to add an event to an interface defined in an ERC? ERC6909 doesn't define the ContractURIUpdated event. I think it should (maybe we should open a PR), but we shouldn't add it unilaterally.

Copy link
Collaborator

@Amxx Amxx left a comment

Choose a reason for hiding this comment

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

ERC-6909 specifications does not mention ERC-7572 (that is still draft). IMO that change is not desirable.

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.

4 participants