Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: master
Are you sure you want to change the base?
NEP: SBT Metadata Standard #504
Changes from 4 commits
634ee11
3394cd0
ca6b0a7
2540920
1705709
ab36573
37aaa74
35d179c
1308eba
c19b2d9
b5eebfe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 (?).
There was a problem hiding this comment.
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 informationThere was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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 asname
,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 thereference
field.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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 inreference
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 theClassMetadata
and potentially to theIssuerMetadata
). Maybe this is a solution? In such case, we would need to extend theClassMetadata
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.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.Sure, make a call in community channels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 abusingreference
field.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, actually, open ended by JSON spec, and that should be encouraged to be used whenever it makes sense.
Alright, so you are waiting for inputs from someone else besides me, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, with @codingshot we left countless messages to get the input.
There was a problem hiding this comment.
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:
Similarly we can do with vide and audio:
video.mp4.full_hd = "link..."
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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?