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

ERC-2477 the discussion #2483

Closed
fulldecent opened this issue Jan 20, 2020 · 23 comments
Closed

ERC-2477 the discussion #2483

fulldecent opened this issue Jan 20, 2020 · 23 comments
Labels

Comments

@fulldecent
Copy link
Contributor

fulldecent commented Jan 20, 2020

To discuss ERC-2477...

Simple Summary

This specification defines a mechanism by which clients may verify that a fetched token metadata document has been delivered without unexpected manipulation.

This is the Web3 counterpart of the W3C Subresource Integrity (SRI) specification.

@mudgen
Copy link
Contributor

mudgen commented Jan 21, 2020

Hi there. Let's discuss ERC-2477. What is it?

@MoMannn
Copy link
Contributor

MoMannn commented Jan 22, 2020

@mudgen #2477

@wighawag
Copy link
Contributor

Hey the following is not true :

Although all these standards allow storing the metadata entirely on-chain (using the "data" URI, RFC 2397), or using a content-addressable system (e.g. IPFS's Content IDentifiers [sic]), every implementation we have found is using Uniform Resource Locators. These URLs provide no guarantees of content correctness or immutability. This standard adds such guarantees.

At Sandbox we use IPFS content-addressable url and as such these url guarantees correctness and immutability (as our contract does not allow change of metatdata uri)

On that note, why not simply reuse the tokenURI standatrds and have the uri contain the hash of the data (even if it lives on a DNS domain) like https://centralised.com/asset/1?hash=bafybeigub4nnpk7rg2lni2nm6tlpdeo2k2remfeuoobbv3272jdgjqmyty
As for the schema it could be referenced with the same kind of uri inside the metadata itself

@fulldecent
Copy link
Contributor Author

I am glad to see it is getting used that way! Can you please provide some references for Sandbox, marketing page and implementation details?

You are correct, we could have created a convention to shove hashes into a URL. There are a few reasons we have not chosen this. The strongest reason is that the World Wide Web has the same problem and they chose to use the Sub-Resource Integrity approach, which is a separate data field.

And perhaps this discussion can help to strengthen the rationale in the text. Other supplementary reasons are:

  1. For on-chain consumers of data, it is easier to parse a direct hash field than to perform string operations
  2. Maybe there are some URIs which are not amenable to being modified in that way, therefore limiting the generalizability of that approach

@wighawag
Copy link
Contributor

Regarding our use of IPFS for metadata, you can find it here : https://github.com/pixowl/sandbox-smart-contracts/blob/master/src/Asset/ERC1155ERC721.sol
We wrote a blog post mentioning it here : https://medium.com/sandbox-game/erc-1155-a-new-standard-for-the-sandbox-c95ee1e45072

@wighawag
Copy link
Contributor

As for having a separate method just for integrity, I guess it make sense, but why not instead use the multicodec format a la https://eips.ethereum.org/EIPS/eip-1577 ?

And if you still need to use tokenURI / uri for backward compatibility (or because you still want to handle the serving of the data through your domain) the multicodec can serve as the integrity of the data served at tokenURI

As for tokenURISchemaIntegrity I think it is unecessary as this can be dealt like you deal with schema uri in your example. There could be integrity checksum in the metadata format itself.

@wighawag
Copy link
Contributor

wighawag commented Jan 29, 2020

This standard is usable with any non-fungible token standard or any token standard that has some non-fungible properties. More precisely: anything that we can index using an id parameter. This EIP metadata lists ERC-721 and ERC-1155 as "required" for implementation, due to a technical limitation of EIP metadata. In actuality, *either* of these standards is required, or any other applicable token standard could be used.

I don't think it is useful to limit to what you call "Non fungible token". I think it should apply to any standard that have a tokenURI / uri method or even future standard that can reference EIP-2477.

@fulldecent
Copy link
Contributor Author

fulldecent commented Feb 4, 2020

Thank you, all great points.

Updates made:

  • a5fb2a4 Qualify: NEARLY every implementation we have found is using Uniform Resource Locators
  • cdc046b Add rationale for separate data field
  • 25cdb07 Justify need for schema integrity
  • 3518f30 Remove "non-fungible" limiting word, make more general
  • e29147b Add reference to The Sandbox

@wighawag
Copy link
Contributor

Hi @fulldecent

I have still few comments that merit consideration in my opinion :`

Rather than changing how JSON-LD works, or changing JSON Schemas, we have the tokenURISchemaIntegrity property to just provide the integrity.

This is not true, specifying the integrity of the schema in the document would not change how JSON-LD works. the @Schema would still be valid. You would just need to add an integrity field. This is backward compatible with json-ld.
The advantage though is that you do not need to store it on-chain.
This would encourage application to use schema since they would not need an extra cost for it nor an extra function in their smart contract

I would also like to see discussion on the use of a contentHash a la https://eips.ethereum.org/EIPS/eip-1577

The idea would be replace tokenURI and uri function with a more strict mechanism so integrity is builtin.

New standard could use it instead of tokenURI and for old standard, they woudl still provide tokenURI as fallback but the integrity would be provided by 1577

The 2477 interface would then simply be

interface ERC2477 /* is ERC165 */ {
  function contenthash(uint256 id) public view returns (bytes);
}

@fulldecent
Copy link
Contributor Author

@wighawag, thank you, you have good points to bring up.

tokenURISchemaIntegrity is duplicative since in can be referenced from token metadata

Yes, the token metadata can reference the schema and can use some approach to provide for referential integrity. Example:

{
    "$schema": "./schema.json",
    "$schemaIntegrity": "sha256-sha256-CSXorXvZcTkaix6Yvo6HppcZGetbYMGWSFlBw8HfCJo="
}

This approach provides integrity guarantees for the metadata schema. And it does not require any additional storage on the blockchain.

A benefit of the duplicity is that it is possible to upgrade the schema document without requiring to update every token metadata integrity. What I'm saying sounds stupid if you think implementations would need to update storage for each token individually.

Actual storage implementations

I expect that many applications will not use O(N) storage for tokenURISchemaIntegrity.

  • Non-fungible tokens applications, which use one class of token, can use a single schema document (this is O(1) storage).
  • Multi token applications, which use batches of tokens (see https://eips.ethereum.org/EIPS/eip-1155#non-fungible-tokens at "Split ID Bits"), can use a single schema document per class (also O(#CLASSES).

The benefit having a separate tokenURISchemaIntegrity is that it is possible to upgrade a schema document (O(1)) without requiring to update every token metadata integrity (O(N)).

TODO

This design decision should be understood and documented in the text.

Change design of tokenURISchemaIntegrity to optional?

Our current design is favoring (and anticipating) applications that have many tokens with the same schema. In order to better accommodate other applications we could update:

     * @return digest Bytes returned from the hash algorithm or "" if there is no schema
     * @return hashAlgorithm The name of the cryptographic hash algorithm or "" if there is no schema

to

     * @return digest Bytes returned from the hash algorithm or "" if not available
     * @return hashAlgorithm The name of the cryptographic hash algorithm or "" if not available

The latter opens the possible for a schema to be interpreted using $schemaIntegrity or similar.

TODO: Discuss if the above change is appropriate.

Related work

I have reached out to the JSON-LD and JSON Schema working groups to discuss if $schemaIntegrity approach above can be standardized at the JSON Schema level. This discussion is in progress.

Regarding contentHash

The content hash approach at #1577 is interesting. There is work with IPFS and IETF to standardize this. At current, none of these specifications have left draft status nor have any date to consider for final status. Because these specifications are not finalized I do not recommend we adopt them as dependencies for this project. To do so would delay our work here.

References:

Regarding removing tokenURI

Functionality relating to URIs is implemented on billions of devices on Earth and beyond. I do not recommend we drop support for URIs.


🏆 Achievement unlocked: This post is the most literally metan I've written or seen.

@wighawag
Copy link
Contributor

How do you see the scenario where the metatdata do not change but the schema does ?

Were you thinking of the case where the new schema is still backward compatible with the old metadata ?
That feel unnecessary if the schema is linked to the metadata itself since you can simply link to the new schema in the new metadata without affecting existing metadata.
Also allowing the schema to change for existing metadata is dangerous. The point of integrity check is that they do not change.

Note I was not proposing to remove tokenURI from existing standard, just that new standard that need to link to content off-chain can use a contentHash approach instead, making things like tokenURI obsolete for them

But if you want 2477 for existing standard, the content hash fucntion would be on top of the tokenURI acting as both an integrity check (for application that use tokenURI to fetch the data) and content uri (for those that prefer to directly use contentHash

@fulldecent
Copy link
Contributor Author

Here is a real world scenario for schema changes without metadata changes:

KnightStory is "an innovative mobile RPG powered by blockchain". And according to Etherscan, its tokens are the top ERC-721 asset which has associated JSON metadata (measured by transfers, past 7 days).

One of the tokens IDs is 765908694

And the tokenURI for this ID is: https://qtsaj4j9bc.execute-api.ap-southeast-1.amazonaws.com/prod/v4/game/dozer/doll/metadata/91

Here is the token metadata:

{
    "name":"Lamlam",
    "language":"en-US",
    "image":"https://cryptodozer.io/static/images/dozer/meta/dolls/91.png",
    "attributes":{"id":"91","type":"Fancy01","grade":"rainbow","star":"1"}
}

There is no schema.

So if this application has deployed using ERC-2477 they would have a tokenURIIntegrity of hash(that JSON).

I don't have anything bad to say about the developers at KnightStory, I don't know them. But if my experience is any indication, people will implement ERC-2477 by copying the reference implementation and (maybe) adjusting the code indent level to match the rest of the code that they just pasted it into. This means, no, I'm not expecting them to think about implementing a proper well-thought-out, internationalized, forward-thinking data schema when adding the integrity feature to that token.

So I expect it will be common that people deploy ERC-2477 applications hastily, and I would like to support this development methodology by allowing them to upgrade their schema (using tokenURISchemaIntegrity) at a later time.

This includes adding a schema when previously there was none, and also as you mentioned, upgrading to a new backwards compatible schema (e.g. adding documentation to existing properties).


That feel unnecessary if the schema is linked to the metadata itself since you can simply link to the new schema in the new metadata without affecting existing metadata.

I don't follow here. The hash for metadata A:

{
    "$schema": "https://example.com/schemas/v1.json",
    "$schemaIntegrity": "sha256-CSXorXvZcTkaix6Yvo6HppcZGetbYMGWSFlBw8HfCJo="
    "data": "moar"
}

does NOT equal the hash of metadata B:

{
    "$schema": "https://example.com/schemas/v2.json",
    "$schemaIntegrity": "sha256-someotherintegrity======================"
    "data": "moar"
}

So changing the schema URL in a metadata would require updating tokenURIIntegrity.

However, a looser coupling is possible:

{
    "$schema": "https://example.com/schemas/schema.json",
    "data": "moar"
}

And that allows the changing the schema only by updating the web server and updating tokenURISchemaIntegrity which can be O(1).


Regarding contenthash. This specification (is it a specification?) is too new to consider building on top of. For example they have not made a 1.0 release, they are not committed to a policy of making backwards-compatible releases and they have minimal adoption outside of the IPFS Project.

W3C is a much more reliable and stable specification to build on top of and so the ERC-2477 specification builds on top of SRI.

That said, it could be helpful to mention the multicodec's table database (https://github.com/multiformats/multicodec/blob/master/table.csv). But I would keep this as a non-normative reference until the IPFS Project gets that file referenced in from the W3C specification (which they should totally do).

@xpepermint
Copy link

Posting 0xcert/framework#663 here as an implementation example.

@wighawag
Copy link
Contributor

So the only use case for having schema integrity on-chain is that the schema can change ?
I am not sure I understand the appeal of the integrity check then.

For those that care of the schema not being able to change, the extra function is pointless
Maybe the schema integrity thing should be another standard ?

What I meant by

That feel unnecessary if the schema is linked to the metadata itself since you can simply link to the new schema in the new metadata without affecting existing metadata.

Was that having the schema in the metadata itself, still allow you to use different schema for later token, since the schema is per metadata.

@fulldecent
Copy link
Contributor Author

The latest update includes an in-depth design review, including pictures and a explanation of which designs require which on-chain data changes for which metadata changes.

Also a specification is added for $schemaIntegrity inside JSON files.

#2529

@fulldecent
Copy link
Contributor Author

I am considering this above comment as spam. It is unrelated to this EIP, and it is proposes an incompatible alternative to NFT (i.e. not strictly compatible).

I am saying nothing here about whether that proposal is good or bad.

@xpepermint
Copy link

Let's move forward with this. We've been using it for more than a year, the implementation is good and the benefit is there. @fulldecent

@MoMannn
Copy link
Contributor

MoMannn commented Sep 15, 2021

Any reason why this isn't moving forward? I believe it is ready to move to review status.

@fulldecent
Copy link
Contributor Author

@xpepermint @MoMannn Thank you. I addressed all remaining points in the draft and sent the PR which should be automatically merged.

@coinfork do you have any implementation to share for this standard and are you good to publish?

@xpepermint
Copy link

@fulldecent awesome, thank you!

@fulldecent
Copy link
Contributor Author

We can include more implementations here and at least one consumer

@github-actions
Copy link

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Mar 16, 2022
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

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

No branches or pull requests

5 participants