flow-ft icon indicating copy to clipboard operation
flow-ft copied to clipboard

WIP: V2 FungibleToken Standard

Open joshuahannan opened this issue 3 years ago • 20 comments

Adds the v2 fungible token standard, as described in this forum post: https://forum.onflow.org/t/streamlined-token-standards-proposal/3075/37

A few highlights:

  • Turns FungibleToken into a contract instead of a contract interface
  • Introduces a new contract interface with a totalSupply field and function to get the vault types that the contract implements. This will help external parties learn about which vaults a contract implements since contracts can implement multiple vault types now
  • Adds a Transferable interface to include a transfer function
  • Adds getAcceptedTypes() to the Receiver interface so a receiver interface can indicate which vault types it accepts.
  • Moves standard paths to inside the vault object definition because they are specific to the actual vault instead of the contract now.
  • Defines the VaultInfo struct to return type and path information for a specific vault type
  • Moves createEmptyVault into the vault resource
  • Adds type parameters to the all the events since contracts can have multiple vault types defined

There are probably more that I am missing, but most of the details are in the forum post, so that is another good resource.

joshuahannan avatar Jul 18 '22 16:07 joshuahannan

The only test I have written so far is a deployment test, which is currently failing because it says the ExampleToken.Vault does not implement FungibleToken.Vault correctly, which I am confused about. maybe it is a bug.

Once we can align on which features to include, I'll work on adding more tests

joshuahannan avatar Jul 18 '22 16:07 joshuahannan

The only test I have written so far is a deployment test, which is currently failing because it says the ExampleToken.Vault does not implement FungibleToken.Vault correctly, which I am confused about. maybe it is a bug

Transfer?

bluesign avatar Jul 18 '22 17:07 bluesign

oh yeah, good catch! The test still fails with the same error though. 😅

joshuahannan avatar Jul 18 '22 17:07 joshuahannan

oh yeah, good catch! The test still fails with the same error though. 😅

createEmptyVault inside vault interface, it should be on contract interface I guess. tried to do a PR but couldn't manage

bluesign avatar Jul 18 '22 19:07 bluesign

I'm actually going to hold off on including the scoped provider for now. It is something we will definitely include in this upgrade the future, but I don't want that discussion to distract from the discussion of the core proposal yet

joshuahannan avatar Jul 18 '22 20:07 joshuahannan

I'm not sure if I'm not getting something here but does this proposal allow creating new vault types at run time? ie. using LP Tokens as an example.... how would you create a new token/vault type to match the new swap pool? https://github.com/onflow/flow-ft/issues/59

justjoolz avatar Jul 19 '22 02:07 justjoolz

Can we consider overflow or flow-js-testing for tests here? Both a very much more readable then these tests that contains alot of boilerplate.

bjartek avatar Jul 19 '22 11:07 bjartek

@justjoolz I don't understand the use case very well, so I'm not totally sure, I don't think it could create new token types at run time that are equivalent to every other fungible token that gets implemented with this standard. You could potentially give your Vaults IDs so that they would be the same type but differentiated by IDs, but I don't know if that solves your use case, because to Cadence, they still would look like the same type. Does that help? Do you have any ideas?

joshuahannan avatar Jul 19 '22 20:07 joshuahannan

@bjartek Yes, we're also going to refactor the tests here to use a better testing framework, but just haven't gotten to it yet

joshuahannan avatar Jul 19 '22 20:07 joshuahannan

@joshuahannan use case is just that, creating tokens at run time. For example currently BloctoSwap and Incremental.fi have to deploy a new copy of the LP token contract to a new address for every swap pair that they list, which means a lot of identical code deployed on the network... extra storage space used etc.

A protocol that wants to issue 'social tokens' like Roll would have to do the same. Or an NFT fractionalizing protocol like Fractional.art .. any platform that wanted to programmatically issue unique tokens to creators on their platform.

With the current proposal here you'd need to be able to create a new resource type definitions at run time, I can't see how that could happen easily....

One possible solution following same pattern as https://github.com/onflow/flow-ft/issues/59 would be to use a more generic string identifier (which can still come from getType().identifier and be appended to if there are multiple tokens of the same type defined... something like: A.00000000.ExampleToken.Vault.TypeA A.00000000.ExampleSwap.Vault.FLOW_FUSD

The identifier could be passed in as an optional parameter to the createNewVault, withdraw and transfer functions.... though it would be ugly passing in nil all the time when you didn't want to use this feature..... it would work great if Optional parameters where totally optional... as in you could choose to not pass in a value at all (instead of passing in nil it would simply default to nil if omitted) That would improve the semantics for interacting with normal single token per contract

What do you think? Is it too much to try and force this into the same standard? Would it be better as a separate complementary standard for these kind of use cases? Or better to just have everyone who wants this functionality to roll their own?

justjoolz avatar Jul 20 '22 06:07 justjoolz

I still don't really see the need to deploy a whole new contract to do this, even with the existing fungible token standard. They could just give their vaults an identifier field and add a pre-condition to the deposit function that the deposited vault must have the same identifier as the vault being deposited into. Then they could also define a separate createEmptyVault with another parameter, like you say, to create a vault for a specific token type. Same with minting.

With this standard, if there is enough support for it, we could even make a getIdentifier(): AnyStruct? that optionally returns an identifier and also make the createEmptyVault() method include that second optional identifier by default.

Does all that make sense?

joshuahannan avatar Jul 20 '22 15:07 joshuahannan

@joshuahannan With the existing standard how would the vault correctly track it's balances ? maybe I'm not seeing what you can see. From my limited understanding I'm pretty sure what you suggest is not possible.

Can you show an example of what you suggest?

I asked previously a few months ago when you said it's possible to do this but I still haven't seen a single example of how this can be achieved in a compliant way anywhere in the Flow ecosystem.

justjoolz avatar Jul 21 '22 00:07 justjoolz

@justjoolz maybe I am misunderstanding what you mean by "compliant," but it seems pretty straightforward. You just add an identifier field to the vault and change the minting and vault creation to require an identifier. Each vault tracks its balance the same way it usually does. You would need to figure out something else to do with totalSupply and probably define some new events to include identifiers, but I think that might be it:

pub resource Vault: FungibleToken.Provider, FungibleToken.Receiver, FungibleToken.Balance {

        /// The total balance of this vault
        pub var balance: UFix64

        /// Identifies which specific swap token this is
        pub let identifier: String

        // initialize the balance at resource creation time
        init(balance: UFix64, identifier: String) {
            self.balance = balance
            self.identifier = identifier
        }

        pub fun withdraw(amount: UFix64): @FungibleToken.Vault {
            self.balance = self.balance - amount
            emit TokensWithdrawn(amount: amount, from: self.owner?.address)
            return <-create Vault(balance: amount)
        }

        pub fun deposit(from: @FungibleToken.Vault) {
            pre {
                from.identifier == self.identifier: "Cannot deposit a different token identifier"
            }
            let vault <- from as! @ExampleToken.Vault
            self.balance = self.balance + vault.balance
            emit TokensDeposited(amount: vault.balance, to: self.owner?.address)
            emit SwapTokenDeposited(amount: vault.balance, to: self.owner?.address, identifier: self.identifier)
            vault.balance = 0.0
            destroy vault
        }
    }
    
    pub fun createEmptyVault(identifier: String): @Vault {
        return <-create Vault(balance: 0.0, identifier: identifier)
    }

    /// Minter
    ///
    /// Resource object that token admin accounts can hold to mint new tokens.
    ///
    pub resource Minter {

        pub fun mintTokens(amount: UFix64, identifier: String): @ExampleToken.Vault {
            emit TokensMinted(amount: amount, identifier: identifier)
            return <-create Vault(balance: amount, identifier: identifier)
        }
    }

Does that work for your use case? Let me know if I am missing something

joshuahannan avatar Jul 21 '22 14:07 joshuahannan

......and change the minting and vault creation to require an identifier.

right, this is the part that doesn't work smoothly with the current standard, as the vault creation init can't be changed to take an additional parameter and you can't access the identifier in other contracts through any of the FungibleToken interfaces

Does that work for your use case? Let me know if I am missing something

The new vault in the withdraw needs to have it's identifier set but yes this approach works well, main drawback to adding it to the standard I can think of is having to pass a blank/nil identifier when you work with a simple token.... would something like this work?

pub resource Vault: FungibleToken.Provider, FungibleToken.Receiver, FungibleToken.Balance {

        /// The total balance of this vault
        pub var balance: UFix64

        /// Identifies which specific token type this is
        pub let identifier: String?

        // initialize the balance at resource creation time
        init(balance: UFix64) {
            self.balance = balance
            self.identifier = nil
        }

        pub fun setIdentifier(_ identifier: String) { // doesn't need to be part of Vault interface just shown in examples 
            pre {
                self.identifier == nil && self.balance == 0.0
            }
            self.identifier = identifier
        }

        pub fun withdraw(amount: UFix64): @FungibleToken.Vault {
            self.balance = self.balance - amount
            emit TokensWithdrawn(amount: amount, from: self.owner?.address)
            let v <- create Vault(balance: amount)
            v.setIdentifier(self.identifier)
            return <- v
        }
}

let v <- createEmptyVault(balance: 0.0)
v.setIdentifier("FLOW/USDC")

perhaps also add a standard for a collection to store them, so they can share a single set of Storage and Capability Paths something like this: https://github.com/onflow/flow-ft/pull/60#discussion_r801753092

You would need to figure out something else to do with totalSupply

Actually thinking about it totalSupply should probably be a derived field in any case, if the individual balances are going to be derived. Maybe something like this would work, but it's a bit much for the single token use case:

pub fun getTotalSupply(type: Type, identifier: String?): {Type: {String: UFix64}}

Anyone have any better ideas? Perhaps make it part of the VaultInfo struct rather than the contract? (and maybe call it TokenInfo instead as it's generic information about a Token not a specific Vault)

justjoolz avatar Jul 22 '22 03:07 justjoolz

Can we remove the "post" checks on deposit/withdraw? What's the use of having those checks inside the standard? We can't override them and having them inside the standard makes it impossible to implement fees on withdraw/deposit transactions.

...
post {
        self.getBalance() == before(self.getBalance()) + before(from.getBalance()):
            "New Vault balance must be the sum of the previous balance and the deposited Vault"
  }
...

highskore avatar Sep 13 '22 12:09 highskore

That is an interesting suggestion, @lukaracki. They're in the standard as a security measure to make sure the core vault resource has a strictly defined behavior. If you want to take fees for something, you could create a separate resource that implements the Provider and/or Receiver that takes fees from the vault separately from interacting with the stored vault. Is there a particular reason you want to do it directly in the Vault instead?

joshuahannan avatar Sep 13 '22 15:09 joshuahannan

@joshuahannan I want to implement an obligatory fee for every fungible token transaction. Having an alternate receiver would allow users to circumvent the fee right? They can just use the standard deposit method instead of the "alternate receiver deposit".

highskore avatar Sep 13 '22 15:09 highskore

What would be people thoughts on including all the resource interfaces inside the FungibleToken contract interface? Since we still have to keep that for defining the standard events I thought it could be simpler for developers to have it that way. I made a PR to this branch so people can have a look into how it would look like: https://github.com/onflow/flow-ft/pull/94

alilloig avatar Sep 27 '22 19:09 alilloig

@alilloig I already pitched this idea in the NFT standard PR, so lets keep discussion over there for now since it is the same as it is here

joshuahannan avatar Sep 27 '22 19:09 joshuahannan

yeah that makes sense, sorry about that!!

alilloig avatar Sep 27 '22 19:09 alilloig

Closing this in favor of https://github.com/onflow/flow-ft/pull/131 because I screwed up Git and I'm not sure how to fix it

joshuahannan avatar Mar 22 '23 18:03 joshuahannan