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

Coin metadata #1980

Closed
sunnya97 opened this issue Aug 10, 2018 · 16 comments · Fixed by #6253
Closed

Coin metadata #1980

sunnya97 opened this issue Aug 10, 2018 · 16 comments · Fixed by #6253
Labels
C:x/bank T: ADR An issue or PR relating to an architectural decision record

Comments

@sunnya97
Copy link
Member

sunnya97 commented Aug 10, 2018

Bank module should have a store that keeps metadata about different denoms.
Examples:

  • decimals per denom
  • is allowed to transfer through IBC
  • max total supply (?)
@sunnya97 sunnya97 self-assigned this Aug 10, 2018
@fedekunze
Copy link
Collaborator

Interesting proposal @sunnya97. Should we also add the origin zone where the coins get minted ?

@sunnya97
Copy link
Member Author

sunnya97 commented Aug 14, 2018

Yeah, maybe we can put the the IBC transfer path prefixing (#842) as a metadata property, so the token name doesn't have to grow super large.

@rigelrozanski
Copy link
Contributor

Based on a previous discussion, we decided this was postlaunch, as we already keep track of the total supply for atoms in staking - which is the only total supply we'll need to know at launch

@fedekunze
Copy link
Collaborator

blocked by #3972

@fedekunze fedekunze added the S:blocked Status: Blocked label Apr 4, 2019
@fedekunze fedekunze mentioned this issue May 27, 2019
8 tasks
@fedekunze fedekunze assigned fedekunze and unassigned sunnya97 May 27, 2019
@jackzampolin
Copy link
Member

Good pick up @fedekunze

@fedekunze
Copy link
Collaborator

As @sunnya97 explained to me the other day, #4255 doesn't address this issue. This will have to be spec'ed out prior to implementation

@fedekunze fedekunze removed the S:blocked Status: Blocked label Jun 28, 2019
@fedekunze fedekunze changed the title Add Denom Metadata to bank module Coin metadata Jun 28, 2019
@fedekunze
Copy link
Collaborator

I think this metadata also could apply to NFTs (cc: @okwme @marbar3778). Maybe it makes sense to have an Asset interface which NFT and coins also comply with

@okwme
Copy link
Contributor

okwme commented Jun 28, 2019

I'd be really into a separation of asset and metadata. I think the fungible and non-fungible token should do only what they're meant to do: represent an asset with denomination (+origin chain) and amount (ID in the case of NFTs). The metadata should be linked to the generalized asset type of fungible or non-fungible token and should be able to have as much or as little depth as needed.

I'm kind of into the schema.org way of nested categorization:
https://schema.org/docs/full.html

Assets could use this format as well as some way of describing which aspects of this format they satisfy. There could also be a protocol for expanding the schema with versions that various projects could keep in sync with.

For games:
https://schema.org/VideoGame
General assets NFT or otherwise:
https://schema.org/TypeAndQuantityNode
Currency:
https://schema.org/currency (maybe needs to be extended)

As a standard used outside of our specific context with an organization dedicated to it it might be good as a common denominator between all sorts of assets which require metadata

https://github.com/schemaorg/schemaorg

@jackzampolin
Copy link
Member

This needs an ADR. IBC also needs some thinking about white/black listing denoms.

@jackzampolin jackzampolin added this to the v0.38.0 milestone Sep 11, 2019
@jackzampolin jackzampolin added the T: ADR An issue or PR relating to an architectural decision record label Sep 11, 2019
@tnachen tnachen modified the milestones: v0.38.0, Backlog Oct 1, 2019
@fedekunze
Copy link
Collaborator

We need to reprioritize this because it can provide a better UX for IBC users. @sunnya97 can you write an ADR by the end of this week?

@cwgoes
Copy link
Contributor

cwgoes commented Feb 3, 2020

What was the idea behind "is allowed to transfer over IBC"? Is that a sort of whitelist?

Ref cosmos/gaia#269.

@fedekunze
Copy link
Collaborator

What was the idea behind "is allowed to transfer over IBC"? Is that a sort of whitelist?

I think it means that tokens cannot be transferred out from their source chain. Maybe some private chains would like to keep their tokens within their domain.

@cwgoes
Copy link
Contributor

cwgoes commented Feb 3, 2020

What was the idea behind "is allowed to transfer over IBC"? Is that a sort of whitelist?

I think it means that tokens cannot be transferred out from their source chain. Maybe some private chains would like to keep their tokens within their domain.

As in, prevent users from transferring their tokens? Seems a bit authoritarian to me.

@fedekunze
Copy link
Collaborator

Seems a bit authoritarian to me.

That's exactly the use case lol. Think of it as a SendEnabled param from bank

@cwgoes
Copy link
Contributor

cwgoes commented Feb 3, 2020

Seems a bit authoritarian to me.

That's exactly the use case lol. Think of it as a SendEnabled param from bank

Hmm, OK. I would personally vote against such an authoritarian option but it is technically possible!

@alexanderbez
Copy link
Contributor

We certainly need this. Moving this alongside the ADR issue to v0.39 release.

@clevinson clevinson added this to the v0.39 milestone Apr 30, 2020
@mergify mergify bot closed this as completed in #6253 Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/bank T: ADR An issue or PR relating to an architectural decision record
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants