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

draft sealed nft metadata #3569

Merged
merged 9 commits into from
May 18, 2021
Merged

draft sealed nft metadata #3569

merged 9 commits into from
May 18, 2021

Conversation

pizzarob
Copy link
Contributor

No description provided.

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

I left a couple comments. On a higher level, I'm not sure how the sealedURI is much different than tokenURI. Additionally, we already have a decentralized storage mechanism. It's called "Ethereum" :) It's just really expensive. You could just encode the NFT data in a data-only URI.

EIPS/eip-draft_sealed_nft_metadata.md Outdated Show resolved Hide resolved
EIPS/eip-draft_sealed_nft_metadata.md Outdated Show resolved Hide resolved
EIPS/eip-draft_sealed_nft_metadata.md Outdated Show resolved Hide resolved
@pizzarob
Copy link
Contributor Author

Thanks @lightclient .

As NFTs continue to evolve so will the media files associated with them. Your suggestion is completely unfeasible and not scalable. It may work for the absolute smallest amount of data, but what about for 10,000 NFTs with a 500kb or 1MB image associated, or what about a video file?

Ethereum is not built to store these types of files and even if it could the cost would make it unaffordable. You should be storing the smallest amount of data possible on Ethereum. This is why a decentralized URI is preferable. I prefer IPFS given that it is content hashed.

The difference between the tokenURI and the sealedURI is this - the sealedURI 1) cannot be changed after it is set, 2) should point to a decentralized file storage system, 3) points to a JSON file which contains metadata for many NFTs rather than just one. Number 3 is what makes sealedURI incompatible with tokenURI. The tokenURI function returns a URI which is the metadata for a single NFT, sealedURI can be many.

If you are familiar with the growing NFT space then you will know that metadata longevity and verification is an issue. Given that ethereum will never be able to communicate with a storage layer (which is actually built for storing files) then we need a solution which allows us to verify the metadata will persist.

@lightclient
Copy link
Member

That makes sense. My comment was mainly tongue-in-cheek :) Thanks for the info though, it helps my understanding.

@alita-moore
Copy link
Contributor

alita-moore commented May 16, 2021

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

 - File with name EIPS/eip-3569.md is new and new files must be reviewed

@pizzarob pizzarob requested a review from lightclient May 16, 2021 17:04
@lightclient lightclient merged commit d4756e8 into ethereum:master May 18, 2021
@MaxflowO2
Copy link

Hey I work with NFT's 24/7...

You want to override _tokenURI with one JSON and let the wallets break down the solo ID's from it? Thus saving a mapping(uint256 => string)? Just trying to figure out the long game on this... and if you need help.

-Max

@pizzarob
Copy link
Contributor Author

@lightclient how can I remove this draft from the EIP repo?

I no longer wish to champion/do not think this is needed for the following reasons:

  • A dynamic base token URI could, as efficiently, handle many different token IDs and so a JSON map of token ID to token URI is not needed.
  • There are different ways an NFT author could effectively "seal" the metadata. A couple being: 1) render the metadata entirely "on-chain" 2) change the owner of the smart contract to address(0) after updating the base token URI to an IPFS hash.

@MicahZoltu
Copy link
Contributor

@pizzarob Submit a PR that changes status: Draft to status: Withdrawn.

eip: 3569
title: Sealed NFT Metadata Standard
author: Sean Papanikolas (@pizzarob)
discussions-to: https://github.com/ethereum/EIPs/pull/3569
Copy link
Member

Choose a reason for hiding this comment

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

How was this merged with a PR as discussion url? 😅

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this is my mistake. I've created a FEM thread and a PR to update: #4221

@axic
Copy link
Member

axic commented Sep 16, 2021

Oh just seen the recent discussion above, in that light my comment doesn't matter that much.

@axic
Copy link
Member

axic commented Sep 16, 2021

@pizzarob I was wondering however what do you think about OpenSeas recommendation to emit PermanentURI(uri, tokenId)?

@pizzarob
Copy link
Contributor Author

pizzarob commented Sep 16, 2021

@axic Yeah it's interesting for sure. Do you know if they have drafted an official standard for that? It's simple which is nice. I'd expect that if the event was emitted for a token ID it should not be allowed to be emitted again for the given token ID.

Although I just realized this would be very inefficient if you have many tokens. This wouldn't be feasible for any of the 10k collections that have been coming out recently. Maybe a permanent base URI would make more sense, and the token ID is dynamically appended within the smart contract. PermanentURIBase(baseUri)

@axic
Copy link
Member

axic commented Sep 16, 2021

@pizzarob haven't found anything else, that's how I ended up at your ERC.

I'd say a range option as yours may be also a good solution: PermanentURI(uri, firstTokenId, lastTokenId).

@pizzarob
Copy link
Contributor Author

Oh yeah! A range would work well too. It's more flexible and I could see a project like art blocks using it - where the contract houses many different projects.

PhABC pushed a commit to PhABC/EIPs that referenced this pull request Jan 25, 2022
* draft sealed nft metadata

* adding rationale and other sections

* remove sentence

* adding bullet point regarding reading and caching

* Update EIPS/eip-draft_sealed_nft_metadata.md

Co-authored-by: lightclient <[email protected]>

* Update EIPS/eip-draft_sealed_nft_metadata.md

Co-authored-by: lightclient <[email protected]>

* add discussion link

* update filename

* Update description

Co-authored-by: lightclient <[email protected]>
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.

6 participants