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

NEP: SBT Metadata Standard #504

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

Conversation

robert-zaremba
Copy link
Contributor

@robert-zaremba robert-zaremba commented Sep 6, 2023

A standard interface for SBT Metadata: tokens, classes and issuers.
This allows an SBT registry and issuers to be interrogated for its name and for details an SBT represent.

The proposal addresses potential challenges related to metadata reconstruction, off-chain storage security, owner/operator dependence, and data privacy.

Related update to NEP-393: ad56e5f


NEP Status (Updated by NEP moderators)

SME reviews:

  • sme1
  • sme2

Contract Standards WG voting indications (❔ | 👍 | 👎 ):

Wallet Standards WG voting indications:

Concerns

@render
Copy link

render bot commented Sep 6, 2023

@robert-zaremba robert-zaremba marked this pull request as ready for review September 6, 2023 22:23
@robert-zaremba robert-zaremba requested a review from a team as a code owner September 6, 2023 22:23
neps/nep-504.md Outdated
// Default token icon
"icon": {
"png": "https://genadrop.mypinata.cloud/ipfs/...",
"svg": "https://genadrop.mypinata.cloud/ipfs/..."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we should have one more nesting level, to indicate the picture size? This will allow to set multiple sizes. Example:

"png": {"200x100": "link1", "4000x2000": "link2"}

Similarly we can do with vide and audio: video.mp4.full_hd = "link..."

Copy link
Contributor

Choose a reason for hiding this comment

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

@robert-zaremba ...or it may be just a part of the URL: "https://image.com/?w=1024&h=748"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what if a http server doesn't support that?

@frol frol added WG-contract-standards Contract Standards Work Group should be accountable S-draft/needs-moderator-review A NEP in the DRAFT stage that needs a moderator review. A-NEP A NEAR Enhancement Proposal (NEP). labels Sep 9, 2023
@codingshot
Copy link
Contributor

  • Adding .com to label for social media seems like a waste of space
  • event emitted should match contract_metadata_update https://github.com/near/NEPs/pull/423/commits or it should be clear whether which individual SBT is updated vs the contract metadata as a whole
  • interesting choice for contact to be a list
  • soundcloud is uncessary and general website should be replaced with this as an example

I think there is limitations with just png and svg, in future object files and gif could be including. Was recently talking to a team from Solart about bringing a metadata specific chain to handle the robustness of document types. and information about the underlying file.

@robert-zaremba
Copy link
Contributor Author

Adding .com to label for social media seems like a waste of space

Not every platform url ends with .com, example: near.social

soundcloud is uncessary and general website should be replaced with this as an example

An example provides the full picture of the use case. That's why I use few concrete examples (email, twitter, facebook, soundcloud ....)

event emitted should match contract_metadata_update

The proposal introduces Nep504Event namespace, which is all about metadata updates. We need to know what exactly is updated (class or issuer metadata). #423 doesn't provide that .

neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated
// Default token icon
"icon": {
"png": "https://genadrop.mypinata.cloud/ipfs/...",
"svg": "https://genadrop.mypinata.cloud/ipfs/..."
Copy link
Contributor

Choose a reason for hiding this comment

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

@robert-zaremba ...or it may be just a part of the URL: "https://image.com/?w=1024&h=748"

neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated
NEP-393 `TokenMetadata` is minimalistic. It only requires data which a contract can reason about, and all user facing details (such as title, description) are meant to be part of the reference.
Similarly, `ContractMetadata` only defines issuer basic info.

### Class Metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

The big question is: where the class metadata is actually used? Any API (view/call methods)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should represent information about the whole class. For example see the "SBT" cards https://i-am-human.app/community-sbts - it will use information from the class metadata (currently this is hardcoded in the UI).

Copy link
Contributor

Choose a reason for hiding this comment

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

@robert-zaremba I think I still don't get whether it's returned via API, or is posted as a log string in the L1 block (?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ClassMetadata is stored in the Issuer, and returned via an issuer call. Will add this information

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added in the paragraph below

neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated Show resolved Hide resolved
neps/nep-504.md Outdated
Comment on lines 50 to 54
/// JSON or an URL to a JSON file with more info. If it doesn't start with a scheme
/// (eg: https://) then base_uri should be prepended.
pub reference: Option<String>,
/// Base64-encoded sha256 hash of JSON from reference field. Required if `reference` is included.
pub reference_hash: Option<Base64VecU8>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@robert-zaremba Let me try to deliver my point once again:

NEPs/neps/nep-0177.md

Lines 45 to 46 in 6eee9dc

reference: string|null, // URL to a JSON file with more info
reference_hash: string|null, // Base64-encoded sha256 hash of JSON from reference field. Required if `reference` is included.

reference field is designed to be a link and nothing else.

If you want to have more fields stored as part of ClassMetadata, just add those fields on the same level as name, symbol, ... fields - this is not a breaking change and won't affect consumers of the API as they will just ignore unsupported fields. There is no need in string-encoding JSON data into the reference field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. For link vs having reference directly. For small content, I think it makes sense to store it directly, rather than querying another resource.

Copy link
Contributor Author

@robert-zaremba robert-zaremba Oct 30, 2023

Choose a reason for hiding this comment

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

Issue about adding more records to the ClassMetadata vs requiring them in reference

ClassMetadata is not open ended. If we would like to add it, then we would need to add bunch of optional maps: social_media, contact, icon....

My reasoning is to have a strict minimum amount of keys in ClassMetadata, and add more structure for other common requirements, so tools and websites can display more information about tokens.

So, I'm not against the @frol suggestion (adding all fields from the ReferenceBase to the ClassMetadata and potentially to the IssuerMetadata). Maybe this is a solution? In such case, we would need to extend the ClassMetadata as more requirements will emerge from the ecosystem. So far, for the SBTs we were working with, we need different pictures (size / format), website address, social_media links ...

I would like to hear more opinions from the tool designers and other projects dealing with tokens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My reasoning is to have a strict minimum amount of keys in ClassMetadata, and add more structure for other common requirements, so tools and websites can display more information about tokens.

There is no reason to overcomplicate things. If you want to standardize fields, add them to ClassMetadata optional or not. reference is needed when the metadata is too big to be stored inline. There is no reason to store JSON as a string inside JSON value.

In such case, we would need to extend the ClassMetadata as more requirements will emerge from the ecosystem. So far, for the SBTs we were working with, we need different pictures (size / format), website address, social_media links ...

If you will want to standardize on those, you will still need to extend the NEP and some structure, be it ClassMetadata or some other struct, it does not really change things. Also, you don't always need to standardize every single field as that may limit the usability of the whole spec if those fields will be used sparsely.

I would like to hear more opinions from the tool designers and other projects dealing with tokens.

Sure, make a call in community channels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you will still need to extend the NEP and some structure

This is what an appendix is for.

We need to have a structure to make sure tools will have a common way to handle the data we describe in this PR. Can be in reference (as originally proposed), or by adding more optional attributes to the top level Metadata. Either way we need to have that structure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to have a structure to make sure tools will have a common way to handle the data we describe in this PR. Can be in reference (as originally proposed), or by adding more optional attributes to the top level Metadata. Either way we need to have that structure.

I don't argue against having the defined structure, I just suggest moving it to where it belongs (new optional fields on ClassMetadata), instead of abusing reference field.

Copy link
Contributor Author

@robert-zaremba robert-zaremba Dec 13, 2023

Choose a reason for hiding this comment

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

Sure, I'm OK with that, see the second part of the comment above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ClassMetadata is not open ended.

It is, actually, open ended by JSON spec, and that should be encouraged to be used whenever it makes sense.

Sure, I'm OK with that, see the second part of the #504 (comment).

Alright, so you are waiting for inputs from someone else besides me, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, so you are waiting for inputs from someone else besides me, right?

Yes, with @codingshot we left countless messages to get the input.

@frol frol added S-review/needs-author-revision A NEP in the REVIEW stage that needs an author revision. and removed S-draft/needs-moderator-review A NEP in the DRAFT stage that needs a moderator review. labels Jul 22, 2024
@ori-near ori-near removed the S-review/needs-author-revision A NEP in the REVIEW stage that needs an author revision. label Jul 22, 2024
@frol frol added S-draft/needs-author-revision A NEP in the DRAFT stage that needs an author revision. S-review/needs-author-revision A NEP in the REVIEW stage that needs an author revision. and removed S-draft/needs-author-revision A NEP in the DRAFT stage that needs an author revision. labels Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NEP A NEAR Enhancement Proposal (NEP). S-review/needs-author-revision A NEP in the REVIEW stage that needs an author revision. WG-contract-standards Contract Standards Work Group should be accountable
Projects
Status: REVIEW
Development

Successfully merging this pull request may close these issues.

6 participants