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

EIP-2477: Non-Fungible Token Metadata Integrity.md #2477

Merged
merged 12 commits into from Feb 12, 2020

Conversation

fulldecent
Copy link
Contributor

@fulldecent fulldecent commented Jan 19, 2020

No description provided.

@fulldecent fulldecent changed the title Create eip-xxxx-Non-Fungible Token Metadata Integrity.md EIP-2477: Non-Fungible Token Metadata Integrity.md Jan 19, 2020
EIPS/eip-2477.md Outdated

## Abstract

An interface `ERC2477` with two functions `tokenURIIntegrity` and ``tokenURISchemaIntegrity` are specified for smart contracts and a narrative is provided to explain how this improves the integrity of the token metadata documents.
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Syntax: ``tokenURISchemaIntegrity->tokenURISchemaIntegrity`

EIPS/eip-2477.md Outdated
* ERC-1155 metadata extension (`ERC1155Metadata_URI`)
* ERC-1046 (DRAFT) ERC-20 Metadata Extension

Although all these standards allow storing the metadata entirely on-chain (using the "data" URI, RFC 2397), or using a content-addressible 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.
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Spelling: addressible => addressable

EIPS/eip-2477.md Outdated
/// @dev The ERC-165 identifier for this interface is 0x#######. //TODO: FIX THIS
interface ERC2477 /* is ERC165 */ {
/**
* @notice Get a cryptographic hash for the specificed token metadata
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Spelling: specificed -> specified
Suggest rewriting to:
* @notice Get the cryptographic hash of the specified tokenID's metadata

EIPS/eip-2477.md Outdated
function tokenURIIntegrity(uint256 tokenId) external view returns(bytes digest, string hashAlgorithm);

/**
* @notice Get a cryptographic hash for the specificed token metadata's schema
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Spelling: specificed -> specified
Suggest rewriting to:
* @notice Get the cryptographic hash of the specified tokenID's metadata schema

EIPS/eip-2477.md Outdated

**Function return tuple**

The SRI integrity attribute encodes elements of the tuple $$(cryptographic\ hash\ function, digest, options)$$. This ERC-2477 standard returns a digest and hash function name and omits forward-compatability options.
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Spelling: forward-compatability -> forward-compatibility

EIPS/eip-2477.md Outdated

Clients must support the SHA-256 algorithm may support others. This is a departure from the SRI specification here SHA-256, SHA-384 and SHA-512 are all required. The rationale for this less-secure requirement is because we expect some clients to be on-chain. Currently SHA-256 is simple and cheap to do on Ethereum whereas SHA-384 and SHA-512 are more expensive and cumbersome.

The most popular hash function size below 256 bits in current use is SHA-1 at 160 bits. Multiple collissions (the "Shattered" PDF file, the 320 byte file, the chosen prefix) have been published and a recipe is given to generate infinitely more collisions. SHA-1 is broken. The United States National Institute of Standards and Technology (NIST) has first deprecated SHA-1 for certain use cases in November 2015 and has later further expanded this deprecation.
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Spelling: collissions -> collisions

EIPS/eip-2477.md Outdated

## Backwards Compatibility

Both ERC-721 and ERC-1155 provide compatible token metadata specifications built on top of URIs and using JSON schemas. This standard is compatible with both and all specifications are addititive. Therefore, there are no backwards compatability regressions.
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Suggest rewrite to:
Both ERC-721 and ERC-1155 provide compatible token metadata specifications that use URIs and JSON schemas. The ERC-2477 standard is compatible with both, and all specifications are additive. Therefore, there are no backward compatibility regressions.

EIPS/eip-2477.md Outdated

Both ERC-721 and ERC-1155 provide compatible token metadata specifications built on top of URIs and using JSON schemas. This standard is compatible with both and all specifications are addititive. Therefore, there are no backwards compatability regressions.

ERC-1523 Standard for Insurance Policies as ERC-721 Non Fungible Tokens (DRAFT) proposes an extension to ERC-721 which also tightens the requirements on metadata. Because this is wholly an extension of ERC-721 and this standard ERC-2477 already supports ERC-721 then therefore ERC-1523 is automatically supported.
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Suggest rewrite to:
ERC-1523 Standard for Insurance Policies as ERC-721 Non Fungible Tokens (DRAFT) proposes an extension to ERC-721 which also tightens the requirements on metadata. Because it is wholly an extension of ERC-721, ERC-1523 is automatically supported by ERC-2477 (since this standard already supports ERC-721).


```json
{
"$schema": "https://URL_TO_SCHEMA_DOCUMENT",
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

$schema is not mentioned anywhere else in the standard, could this be expanded on or clarified to avoid confusion?

EIPS/eip-2477.md Outdated
}
```

Assume that the metadata and schema above apply to a token with identifier 1234. (In ERC-721 this would be a specific token, in ERC-1155 this would be a token type.) Then the these two function calls MAY have the following output:
Copy link
Contributor

@coinfork coinfork Jan 20, 2020

Choose a reason for hiding this comment

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

Suggest rewrite to:
Assume that the metadata and schema above apply to a token with identifier 1234. (In ERC-721 this would be a specific token, in ERC-1155 this would be a token type). Then these two function calls MAY have the following output:

@MoMannn MoMannn mentioned this pull request Jan 22, 2020
@fulldecent
Copy link
Contributor Author

@fulldecent fulldecent commented Jan 24, 2020

@coinfork Added all your updates at 2db9d46, thank you!

@coinfork
Copy link
Contributor

@coinfork coinfork commented Jan 24, 2020

@coinfork Added all your updates at 2db9d46, thank you!

Thanks a lot! Glad to be part of this.

EIPS/eip-2477.md Outdated
* @return digest Bytes returned from the hash algorithm
* @return hashAlgorithm The name of the cryptographic hash algorithm
*/
function tokenURIIntegrity(uint256 tokenId) external view returns(bytes digest, string hashAlgorithm);
Copy link

@MoMannn MoMannn Jan 24, 2020

Choose a reason for hiding this comment

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

In returns add memory data location indicator.

Copy link
Contributor Author

@fulldecent fulldecent Jan 28, 2020

Choose a reason for hiding this comment

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

Thank you, fixed in 8cdf3c7

/// @title ERC-2477 Non-Fungible Token Standard
/// @dev See https://eips.ethereum.org/EIPS/eip-2477
/// @dev The ERC-165 identifier for this interface is 0x#######. //TODO: FIX THIS
interface ERC2477 /* is ERC165 */ {
Copy link

@MoMannn MoMannn Jan 27, 2020

Choose a reason for hiding this comment

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

Since the methods are per token should the methods throw if the tokenId does not exist or return a specific value. This should probably be defined.

Copy link
Contributor Author

@fulldecent fulldecent Jan 28, 2020

Choose a reason for hiding this comment

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

721 says this:

/// @dev NFTs assigned to zero address are considered invalid, and queries
///  about them do throw.

but 1155 does not have such a prescription.

I do not see an obvious way to specify this here yet. Please open this as a discussion item in the discussions-to.

@xpepermint
Copy link

@xpepermint xpepermint commented Feb 9, 2020

Are we good to go? Is there anything that's blocking this issue to be accepted as final?

@xpepermint
Copy link

@xpepermint xpepermint commented Feb 11, 2020

@coinfork @MoMannn please approve this PR if there's nothing else to change. Thanks.

@Arachnid Arachnid merged commit 66e1f04 into ethereum:master Feb 12, 2020
1 check passed
pizzarob pushed a commit to pizzarob/EIPs that referenced this issue Jun 12, 2020
* Create eip-xxxx-Non-Fungible Token Metadata Integrity.md

* Update EIP number

* Update eip-2477.md

* Add coauthor, with permission

* Add wording updates from @coinfork

* Fix spelling

* Specify parameter storage

* Qualify: nearly every implementation we have found is using Uniform Resource Locators

* Add rationale for separate data field

* Justify need for schema integrity

* Remove "non-fungible" limiting word, make more general

* Add reference to The Sandbox
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this issue Nov 7, 2020
* Create eip-xxxx-Non-Fungible Token Metadata Integrity.md

* Update EIP number

* Update eip-2477.md

* Add coauthor, with permission

* Add wording updates from @coinfork

* Fix spelling

* Specify parameter storage

* Qualify: nearly every implementation we have found is using Uniform Resource Locators

* Add rationale for separate data field

* Justify need for schema integrity

* Remove "non-fungible" limiting word, make more general

* Add reference to The Sandbox
Arachnid pushed a commit to Arachnid/EIPs that referenced this issue Mar 6, 2021
* Create eip-xxxx-Non-Fungible Token Metadata Integrity.md

* Update EIP number

* Update eip-2477.md

* Add coauthor, with permission

* Add wording updates from @coinfork

* Fix spelling

* Specify parameter storage

* Qualify: nearly every implementation we have found is using Uniform Resource Locators

* Add rationale for separate data field

* Justify need for schema integrity

* Remove "non-fungible" limiting word, make more general

* Add reference to The Sandbox
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.

None yet

5 participants