Add EIP: ERC-721 Multi-Metadata Extension #7160
Conversation
|
✅ All reviewers have approved. |
github-actions
bot
added
c-new
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
eth-bot
changed the title
github-actions
bot
added
w-ci
0xGh
force-pushed
the
erc-multi-metadata
branch
2 times, most recently
from
141799a
to
c3b3186
Compare
eth-bot
changed the title
|
The commit 8722f0e (as a parent of 4a17802) contains errors. |
EIPS/eip-7160.md
Outdated
|
|
||
| ## Motivation | ||
|
|
||
| The current [ERC-721](./eip-721.md) standard allows for a single metadata URI per token. However, there are use cases where multiple metadata URIs are desirable, such as when a token represents a collection of (cycling) assets with individual metadata, historic token metadata, collaborative and multi-artist tokens, evolving tokens. This extension enables such use cases by introducing the concept of multi-metadata support. |
There was a problem hiding this comment.
I think it would be important to include answers to:
- Why does this need to be more than just
tokenURIand some token-specific functions to change the active URI? - Who would be interested in the non-pinned URIs?
- How would this information be displayed?
There was a problem hiding this comment.
Added a paragraph right below. Let me know what you think. Also @mpeyfuss can chime in and add further reasons for why having a multi-metadata standard might be a good idea.
There was a problem hiding this comment.
Yeah there are quite a few examples we've found working with creators and collectors on why multi-metadata is interesting. Can add example use cases. One of the most mentioned reasons is that of having the option to provide versions of the token that match different aspect ratios and could be utilized for specific display purposes while keeping provenance/ownership intact.
EIPS/eip-7160.md
Outdated
| The `IERC721MultiMetadata` interface extends the existing `IERC721` interface and introduces additional methods and events: | ||
|
|
||
| ```solidity | ||
| interface IERC721MultiMetadata is IERC721 { |
There was a problem hiding this comment.
I'm no Solidity expert, but I've been told to avoid making interfaces depend on one another:
| interface IERC721MultiMetadata is IERC721 { | |
| interface IERC721MultiMetadata /* is IERC721 */ { |
There was a problem hiding this comment.
Commented out as suggested by you. By the way should the interface name reflect the EIP name (IERC7160) instead of being called IERC721MultiMetadata or is it fine to give it a name?
There was a problem hiding this comment.
You can name it however you like!
| event TokenUriPinned(uint256 indexed tokenId, uint256 indexed index, address indexed sender); | ||
| event TokenUriUnpinned(uint256 indexed tokenId, address indexed sender); | ||
|
|
||
| function tokenURIs(uint256 tokenId) external view returns (uint256 index, string[] memory uris); |
There was a problem hiding this comment.
@SamWilsn initially my idea was to propose only the addition of the tokenURIs method as it would allow dapps to retrieve all the uris and the current index (whether it is pinned or not).
However in this proposal we provide a standard interface to dapps/marketplaces to implement pinning, unpinning and related events. Should they decide to adopt this EIP I assume that it would make sense for them to have a standard way to offer pinning/unpinning via UI.
Thoughts on these additional methods and events for pinning/unpinning? Should we keep or drop them?
By the way we'll try to get their (marketplaces) opinion too.
There was a problem hiding this comment.
@0xGh don't the functions on lines 34 and 35 accomplish this?
There was a problem hiding this comment.
Yes they do. Just sharing some context and thoughts based on what we discussed in private.
|
@SamWilsn thank you for taking the time to review this pr! |
- added natspec to the interface and reference implementation - made use of keywords from RFC 2119 - listed out potential use cases rather than in one sentence - general updates to wording - removed reference that the default method of handling unpinned uris is defined as the latest uri as that is highly dependent on the application - added interface id as is - improved rationale section - removed token ownership check in the reference implementation from `hasPinnedTokenUri` as that is a view function and doesn't need that check - added imports from openzeppelin in the reference implementation - change linearzation of parent contracts in reference implementation - added Ownable constructor explicitly in the reference implementation - updated security considerations as there are some to be aware of but nothing out of the ordinary
marco/read-through-updates
This EIP proposes an extension to the ERC721 standards to support multiple metadata URIs per token via a new tokenURIs method that returns the pinned metadata index and a list of metadata URIs.
Please discuss here: https://ethereum-magicians.org/t/erc721-multi-metadata-extension/14629