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

FLIP 55: Fungible Token Standard Version 2 #55

Merged
merged 5 commits into from
Mar 11, 2024
Merged

FLIP 55: Fungible Token Standard Version 2 #55

merged 5 commits into from
Mar 11, 2024

Conversation

joshuahannan
Copy link
Member

@joshuahannan joshuahannan commented Dec 19, 2022

Proposes a fundamental upgrade to the Flow Fungible Token standard.

Original Proposal here: https://forum.onflow.org/t/streamlined-token-standards-proposal/3075/1
Code PR: onflow/flow-ft#131

@joshuahannan
Copy link
Member Author

We're having another working group meeting on Nov 7th to discuss the token standard improvements! We're hoping to finalize most of the changes so we can more confidently say what will be part of the release. You can see the event on the Flow public event calendar.

@j1010001 j1010001 modified the milestones: OKR-23-Q4, C1.0-Launch-M2 Nov 14, 2023
@joshuahannan
Copy link
Member Author

In the most recent smart contract design calls, we made some modifications to the standard proposal:

  • Removed transfer interface, function, and events

We also have some things we still need to discuss about the standards, mostly concerning the NFT standard.

If you have thoughts about wanting to keep some of the things that were removed or want to discuss the proposed changes or anything else about the standard, please join the next token standards discussion on Tuesday Dec 5th to share your feedback! The event is on the flow public events calendar

@joshuahannan
Copy link
Member Author

We had another discussion today about the token standards. We made a few decisions:

  • We will not be including a default destruction event for Vault. Instead we are adding a global function on FungibleToken that can be called to burn a Vault. This will only emit a destruction event if the balance of the vault is greater than zero
  • We decided to include methods in the default events to say the UUID of all the vaults involved. For example, withdraw will look like this now:
emit Withdraw(amount: amount, type: self.getType().identifier, from: self.owner?.address, fromUUID: self.uuid, withdrawnUUID: result.uuid)
  • We will add the Balance interface back to make migration simpler.
  • We had a discussion about how we should manage the balance field for Vault and the various interfaces. We mostly thought that including balance in the Vault interface was a good idea, but we disagreed on what additional balance-related methods should be included in the standard, particularly on the Provider and Balance interfaces. Options:
    * getWithdrawableBalance(): UFix64: it would return the amount of tokens that are actually available
    to be withdrawn from the container. This would keep the standard relatively simple,
    but might not help all use-cases because the balance might be too complicated to manage with this method
    * isAmountAbleToBeWithdrawn(amount: UFix64): Bool: Would return a boolean to indicate if the requested amount
    can be withdrawn from the container. This could support more complicated use cases

I am of the opinion that just getWIthdrawableBalance is enough, but it has not been decided yet. We'll discuss again at the next meeting at 9am Pacific on Thurs Jan 11.

Proposed Agenda:

  • Should we include balance and id as fields in {NFT} and {Vault}, respectively?
  • Should we have methods to getWithdrawableBalance() or isAvailableToWithdraw(): Bool in the vault, provider, and balance interfaces?
  • Should the token standards be contract interfaces to enforce ViewResolver and createEmptyVault()?
  • How to name entitlements
  • should ViewResolver.resolveView be renamed to resolveContractView?

@joshuahannan
Copy link
Member Author

We had another discussion about the token standards today. here is a summary of what we talked about:

  • removing getter functions for standard paths and only including them in metadata views

    • They’re accessible as metadata views so it might make sense to have that be the only way to get them in a standardized way, simplifies the standard
      *** DECISION:
      • Remove getter functions for standard paths.
      • remove provider linked path and provider type from VaultData view**
  • Should the token standards be contract interfaces

    • If they are interfaces, we can enforce conformance to ViewResolver and createEmptyVault
    • BUT, we would lose the ability to have standard events and utility functions like burn
    • DECISION: Keep as a contract and find other ways to enforce that people are conforming to ViewResolver
  • Incompatible upgrades that are part of the new standards

    • Let people know that we are going to have a conversation with the cadence team soon to figure out how to migrate all these weird changes
    • Might be able to include new interfaces that are for legacy tokens that implement the new ones to make migration easier
    • DECISION: Have a conversation with the Cadence team next week during the next one of these meetings about what the migration difficulties are that need to be reverted or addressed

Topics for next week:

  • Should we have balance as a field in Vault?
    • Should we have methods to getWithdrawableBalance() or isAvailableToWithdraw(): Bool in the vault, provider, and balance interfaces?
  • How to name entitlements
    • Adjectives, Verbs?
    • Introduce three entitlement with Owner and have a mapping that maps to both withdraw and update?

@joshuahannan
Copy link
Member Author

We had another discussion about the token standards today. We focused on migration difficulties with the Cadence team. You can find notes here

Next week, we'll be discussing entitlement naming and default implementations

@joshuahannan
Copy link
Member Author

We had another discussion today that, pending final approval of the code, finalizes the design of the standards!
Here is a summary of the decisions we made:

  • Contract vs interface

    • We will make both standards interfaces now
    • Events that are emitted from implementations do not have the same type, so they are safe
    • Implementations will have to implement a getTypes method and a createEmpty method that takes a type as an argument
    • We will also propose new views for contracts that return information about token types in contracts
  • How to name entitlements

    • nouns are used for "coarse-grained" entitlements (e.g. Storage to access all of the account storage functionality), and verbs are used for "fine-grained" entitlements, like specific functions (e.g. SaveValue to be able to store a value into account storage)
    • Rename entitlements to Withdraw, rename events to Withdrawn and Deposited
    • Add Owner entitlement to NFT standard to map to withdraw and update
  • Default implementations

    • Remove default implementations for all view resolver methods
    • Remove them from every other method that makes sense

    We'll have another chat next Thursday to finish everything up

@joshuahannan
Copy link
Member Author

joshuahannan commented Jan 26, 2024

We had our last design meeting for the new token standards yesterday and settled on a design! Here are notes from the meeting that are relevant to the FT standard:

  • Decided to not try to have a conditional that emits events only if to or from is nil. It wasn't possible to include this cleanly in a contract interface and the benefit isn't enough to warrant putting them in another contract
  • Added the Burner contract, which allows resources to specify a method that is called when it is destroyed, but ONLY if you destroy it through the Burner.burn method. FungibleToken.Vault now inherits from Burner.Burnable because it needs the callback to emit the standard Burned event. Token implementations can also implement the bunCallback() to update their tokens total supply when tokens are burned, but MUST set the balance of the vault to zero before the end of the function execution. This is to prevent spam.
  • Removed getBalance() and added isAvailableToWithdraw(). It can be more performant because it doesn’t necessarily need to iterate through all the balances to know the answer. getBalance() was also redundant because we had re-added the balance field back to the standard.

The code in the PR is now the finalized code for the new standard and will be released in the upcoming CLI version and likely will be the version that is deployed to all the networks unless a critical issue is surfaced in the meantime.

Thank you to everyone who contributed, especially @bjartek and @austinkline! The standards are looking quite good. 😎

@joshuahannan
Copy link
Member Author

Looking for some approvals on this FLIP so we can get it merged since it all has been resolved and implemented for a while now!

@joshuahannan joshuahannan changed the title Fungible Token Standard Version 2 FLIP 55: Fungible Token Standard Version 2 Mar 7, 2024
Copy link
Contributor

@sisyphusSmiling sisyphusSmiling left a comment

Choose a reason for hiding this comment

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

Left some minor comments otherwise LGTM! 🚀

@joshuahannan
Copy link
Member Author

Thanks for the reviews and approvals everybody! I've addressed all your comments and I think we have enough approvals now that I'm okay to merge. 🥳 ❤️

@joshuahannan joshuahannan merged commit a78e324 into main Mar 11, 2024
@joshuahannan joshuahannan deleted the ft-v2 branch March 11, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants