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

Add flip for fungible tokens metadata #1087

Merged
merged 9 commits into from
Aug 31, 2022
Merged

Conversation

alilloig
Copy link
Member

@alilloig alilloig commented Aug 11, 2022

Closes: onflow/flow-ft#81

Description

Flip for creating a Metadata standard for fungible tokens.


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@alilloig alilloig added FLIP Flow Improvement Proposal SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board labels Aug 11, 2022
@vercel
Copy link

vercel bot commented Aug 11, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
flow-docs ✅ Ready (Inspect) Visit Preview Aug 16, 2022 at 8:51PM (UTC)

@alilloig
Copy link
Member Author

I could really use some help with the Motivation and User Benefit writing!

@alilloig
Copy link
Member Author

I'm still missing any related issues, and will much appreciate a look at Objective, Motivation & User Benefit, haven't feel specially litterateur with those

Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

Good start so far! I just have a few comments


## Objective

This proposal, in a similar fashion as the NFT Metadata, will make possible to create generic solutions that will interoperate through views rather than through hard-coded types. This will allow better interoperability between fungible tokens and external applications.
Copy link
Member

Choose a reason for hiding this comment

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

Probably include an embedded link to the NFT metadata flip and standard here

Copy link
Member

Choose a reason for hiding this comment

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

make sure to address this

Copy link
Member Author

Choose a reason for hiding this comment

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

A bazillion thanks for remembering this, there was a bunch of great suggestions made by @turbolent, including this point, that I thought I had merged but actually lost track of them at some commit.


## User Benefit

The creation of a Metadata standard for fungible tokens will bring two major user benefits, discoverability and interoperability. It will make way easier to any FT to be presented on any app that would want to feature several FT and also will ease how those FT can be used by an user programmatically.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The creation of a Metadata standard for fungible tokens will bring two major user benefits, discoverability and interoperability. It will make way easier to any FT to be presented on any app that would want to feature several FT and also will ease how those FT can be used by an user programmatically.
The creation of a Metadata standard for fungible tokens will bring two major user benefits, discoverability and interoperability. It will make it way easier for any FT to be presented on any app that would want to feature several FT and also will ease how those FT can be used by a user programmatically.

flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
}
```

In order to be able to use the `FTDisplay` we will need to re-use or re-implement (depending on if this ends up being part of the original `MetadataViews.cdc` contract or having its own contract) the following views: `ExternalURL`, `File`, `HTTPFile`, `IPFSFile`, `Media` and `Medias`.
Copy link
Member

Choose a reason for hiding this comment

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

I think we can import those views from the original contract. no need to re-implement them

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah totally agree


## Questions and Discussion Topics

Main friction point will be if we want this standard to be in a new different contract of if we want it to be part of the existent `MetadataViews.cdc` contract.
Copy link
Member

Choose a reason for hiding this comment

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

I think the new views should be a new contract, but like I said above, reuse some of the primitive views.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we discuss this on the forum or should we start the discussion assuming that what makes more sense is to put the new views in a different contract rather than updating the NFT one?

Copy link
Member

Choose a reason for hiding this comment

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

We should definitely put the FT specific views in a new contract

Copy link
Member Author

Choose a reason for hiding this comment

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

so maybe we should remove that from the FLIP to avoid any confusion?

Copy link
Member

Choose a reason for hiding this comment

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

No, they should definitely stay in this FLIP because this is the FLIP that proposes them. I just mean that when we're writing the code, they should be in their own contract in the flow-ft repo instead of in the MetadataViews contract in the flow-nft repo

Copy link
Member Author

@alilloig alilloig Aug 31, 2022

Choose a reason for hiding this comment

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

Sorry, I expressed myself quite poorly there. I was saying if we should remove this sentence Main friction point will be if we want this standard to be in a new different contract of if we want it to be part of the existent MetadataViews.cdc contract. from the FLIP since it seems that we all agree on creating a new contract for the new views.

Copy link
Member

Choose a reason for hiding this comment

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

oh yeah. sorry about the misunderstanding. We can remove that line


Are we missing any core views that should be mandatory from the beginning?

Could there be some view, like serial, that differentiates tokens from the same `Vault` depending for instance of when they where minted? (think of different years coins with the same value) Would that make them NFT rather than FT? Would ever any developer want to do that? How could we keep track of a single balance which contains slightly different tokens? Would that introduce an unnecessary complication when transferring tokens? (of course it would). This could make sense along with the [streamlined tokens proposal new `FungibleToken.Balance` interface that exposes a `getBalance()` function rather than a field just in case the FT owner wants to compute the balance](https://forum.onflow.org/t/streamlined-token-standards-proposal/3075)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to worry about this now. I think if people want that kind of view, they can submit a proposal after these initial views have been accepted and deployed.

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense, getting rid of it!

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Good idea to add metadata support to fungible tokens, and leveraging the experience gained from adding metadata to NFTs. 👍

flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Show resolved Hide resolved

### Best Practices

Fungible Token issuers should implement the `FTView` to achieve the as much ecosystem compatibility as possible.
Copy link
Member

Choose a reason for hiding this comment

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

See comment above and clarify if the two sub-views should be also provided.

flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
flips/20220811-fungible-tokens-metadata.md Outdated Show resolved Hide resolved
Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

I left a couple more comments. Once those are addressed, I think this can be merged and you can start working on the new contracts in the flow-ft repo!

@alilloig
Copy link
Member Author

@pgebheim should I merge the FLIP here or we should only take it to the FLIPs repo???

@pgebheim
Copy link
Contributor

Let's merge here and I'll port to the FLIPs repo

@pgebheim pgebheim merged commit 924883a into master Aug 31, 2022
@peterargue peterargue deleted the alilloig/flip-ft-metadata branch January 17, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FLIP Flow Improvement Proposal SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create design proposal for the FT Metadata Views
5 participants