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

[Tokens RFC] - Research desired functionality to expose #233

Closed
4 tasks done
MartinMinkov opened this issue Jun 6, 2022 · 11 comments
Closed
4 tasks done

[Tokens RFC] - Research desired functionality to expose #233

MartinMinkov opened this issue Jun 6, 2022 · 11 comments
Assignees
Labels
rfc Issues that contain some important documentation

Comments

@MartinMinkov
Copy link
Contributor

MartinMinkov commented Jun 6, 2022

Description

Before implementing token functionality in SnarkyJS, it is important that we align on what we intended to expose and how it works. Some pre-homework to this task is to do a quick survey of existing token implementations in other chains. Following that, this issue has been broken into 3 steps.

  1. Examine protocol code to see what functionalities are exposed with tokens
  2. Interview a select number of protocol and product engineers to agree on interfaces that should be exposed in SnarkyJS
  • - Brandon
  • - Gregor
  • - Matthew
  • - Izaak
  1. Finalize RFC
@MartinMinkov MartinMinkov self-assigned this Jun 6, 2022
@jasongitmail jasongitmail changed the title [Tokens] - Research desired functionality to expose [Tokens RFC] - Research desired functionality to expose Jun 13, 2022
@MartinMinkov
Copy link
Contributor Author

MartinMinkov commented Jun 28, 2022

Tokens RFC

Overview

Custom tokens are supported at a protocol level, and we wish to bring that functionality to SnarkyJS. Custom tokens are denoted by a token identifier which is used to distinguish between different tokens and a token account. A token account is similar to a regular account, but its balance is stored in the chosen custom token. One may use the same public key for multiple token accounts for different custom tokens.

In addition to token accounts, there is a notion of a token owner account. The token owner account is the owner of a custom token and supervises the behavior of accounts whose balance is stored in that custom token. They are the only account that can provide certain invariants on a custom token before the supervised accounts are updated. The token owner must be included in the Party's transaction layout of a custom token transaction for the update to be authorized.

Token owner accounts are created by specifying a public key and a token identifier. For the purposes of most token owner accounts, using the default, MINA token identifier can be used to derive a new custom token identifier.

In this RFC, we propose a bare-bones API for SnarkyJS to support creating token accounts and transacting in custom tokens.

Note: Tokens cannot be used to pay for fees or be used in stake delegations.

Creating Token Accounts

Given an existing account, a new token account is created by simply receiving a payment denoted in a custom token. When such a transaction occurs, the protocol will create a token account for the public key receiving the new custom token.

When one wants to create a new custom token, it must be derived from an existing account and an existing token identifier. If an existing public key is specified in deriving a new custom token, that public key will be the token owner. The native MINA token identifier can be used when setting an existing token identifier. In this model, developers will want their zkApp to be the token owner of their custom token to automate any authorization for other users to interact.

To derive a new custom token identifier from a public key and an existing token identifier, we can expose a function that will accept a public key and token identifier and spit out a new custom token identifier. This new custom token identifier can be used by the specified public key to mint new tokens.

import { createNewTokenId } from "snarkyjs";

// Create a new token identifier
const newToken = createNewTokenId({
    owner: PrivateKey.random().toPublicKey(),
    tokenId: Token.defaultTokenId, // This can be optional
});

// New token will be encoded as a base58 field element
console.log(newToken);

One thing to note about createNewTokenId is that it's derived from the owner and the token identifier. This means that no transaction is needed to broadcast to create a new token and can be recomputed from the public key and token identifier each time since it is a deterministic process.

Transferring tokens

To support an easy way for zkApps to send transactions to other accounts, we should expose a function that abstracts the details of creating a Parties transaction manually for balance changes. Such a function could look like this:

class TransferExample extends SmartContract {
    ...
    @method sendTokens(recieverAddress: PublicKey) {
        // Create a Party structure for the receiever
        let receiverParty = Party.createUnsigned(recieverAddress);
        // Inside a zkApp smart contract method, send to the 'receiverAddress'
        this.transfer({
            to: receiverParty,
            amount: 100_000,
        })
    }
}

...
let zkapp = new TransferExample(zkappAddress);
// Create a transaction specifying a transfer
txn = await Local.transaction(feePayer, () => {
    zkapp.sendTokens(privilegedKey);
    zkapp.sign(zkappKey);
});
await tx.send();

This transfer function would create a Parties transaction that specifies the zkApp as the sender and the receiver address for an amount of 100_000. The Party's transaction output would contain the fee payer, the parties involved in transferring, and an optional memo.

Minting/Burning Tokens

The token owner account has permission to mint/burn from any account that holds a balance for its custom token. Given that the token owner must be the one to authorize such transactions, the token owner must be specified in the Party's structure when it's sent to the protocol layer. To enable minting/burning, we can simply reuse the this.transfer function in the following way:

// ... code as before
let customToken = createNewTokenId({
    address: this.address,
    tokenId: defaultMinaTokenId(),
});

// This will mint 100,000 tokens to the receiverParty address
this.transfer({
    to: receiverParty,
    amount: 100_000, // To burn, we simply negate this value
    tokenId: customToken, // Will default to the default MINA ID if not used
});

This transfer call instead mints 100_000 new custom tokens for the receieverParty. To burn tokens for the receiverParty, we simply have to negate the amount that will be deducted from the custom token.

Note: If the token owner zkApp attempts to burn more tokens than what the balance is, the transaction will fail.

Let's assume that account A has received a custom token and wants to send that custom token to another account B. The zkApp token owner account could expose a method that handles the transfer between the two accounts to handle the authorization needed. The following code is an attempt to give an illustration to how this looks:

@method sendCustomTokens(senderAddress: PublicKey, recieverAddress: PublicKey) {
    let senderParty = Party.createUnsigned(senderAddress);
    let receiverParty = Party.createUnsigned(recieverAddress);

	// Derive a custom token ID that is owned by this operating zkApp
    let customToken = createNewTokenId({
        address: this.address,
        tokenId: defaultMinaTokenId()
    })
    this.transfer({
        to: receiverParty,
        from: senderParty,
        amount: 100_000,
        tokenId: customToken
    })
}

Asserting

One can ensure certain preconditions on their custom token transfers by adding assert methods inside their smart contract method like they could regularly. It will be up to the zkApp developer to define what kind of limitations or restrictions their custom token should behave under. One such example could look like:

@method mintIfNonceIsTen(recieverAddress: PublicKey) {
    let receiverParty = Party.createUnsigned(recieverAddress);
    receiverParty.account.nonce.assertEquals(new UInt32(10));
    receiverParty.body.incrementNonce = Bool(true);
    let customToken = createNewTokenId({
        address: this.address,
        tokenID: defaultMinaTokenId()
    })
    this.transfer({
        to: receiverParty,
        amount: 100_000,
        tokenId: customToken
    })
}

This sort of approach is easily extensible to whatever type of functionality you want to enable for your custom token transfers.

Open Questions

  • Currently, proving a transaction does not work within SnarkyJS. Does this pose a problem in any way?
  • This RFC proposes some "must-haves" for supporting tokens in SnarkyJS. Is there anything missing that is a "must-have" that has been missed?

TODO List

[] - Implement this.transfer API for MINA and custom tokens
[] - Implement a function to derive a custom token identifier
[] - Implement API that provides helpers for custom tokens
- Get balance of a specified token
- Get the token owner of an existing token identifier
- Get/set token symbol for a custom token
- Other such helpers that are not captured here
[] - Create unit tests which confirm that
- Balances are updated correctly after using this.transfer with MINA tokens and custom tokens
- Minting/Burning behaves correctly
- Using preconditions on a zkApp token owner successfully applies preconditions when interacting with a custom token

@bkase
Copy link
Member

bkase commented Jun 29, 2022

Nit: free functions are too hard to discover via intellisense we should export something more discoverable

this is great! We should include a discussion around which parts of erc20 and erc777 we are explicitly supporting or choosing not to support and why. From a skim, i think we should build an abstraction that fires relevant erc777 events automatically for you and maybe have this on by default to maximize interoperability for clients

@MartinMinkov
Copy link
Contributor Author

Updated API

After some internal discussions, we have decided we want to compare/contrast different styles of APIs to understand what would be most ergonomic for users of SnarkyJS. Therefore, the core feature set is unchanged, but instead, we are proposing a different "wrapper" to interact with tokens.

Tokens namespace in SnarkyJS

There was a discussion about creating a namespace for tokens inside SnarkyJS to abstract out some of these functions so that they are not "free functions" by themselves. In addition, putting tokens inside a namespace inside SnarkyJS will allow for a better IntelliSense/discoverability experience for zkApp developers, as "free functions" will be somewhat hard to find with IntelliSense. The following code shows the potential differences:

// Old proposal
import { createNewTokenId, getOwner, getAccounts, getBalance } from "snarkyjs";
const newToken = createNewTokenId({...});
const owner = getOwner(newToken); // Gets the account ID that owns the specified token
const accounts = getAccounts(newToken); // Find all accounts for a specified token
const balance = getBalance({
  address,
  tokenId,
}); // Get balance of a specified account denoted in the custom token ID

// New Proposal
import { Token } from "snarkyjs";
const newToken = Token.createNewTokenId({});

// Extra methods to be added in the namespace, maybe this makes sense to live in the Ledger namespace instead?
const owner = Token.getOwner(newToken); // Gets the account ID that owns the specified token
const accounts = Token.getAccounts(newToken); // Find all accounts for a specified token
const balance = Token.getBalance({
  address,
  tokenId,
}); // Get balance of a specified account denoted in the custom token ID

With the new proposal, users can type Tokens. in their code editor, and IntelliSense will show potential functions to use. This will be clearer for developers checking out the API instead of referencing "free functions" that are exported.

Another option is instead of using functions, we adopt a class-based approach for using tokens. The following code shows potential differences:

import { Token } from "snarkyjs";

const newToken = new Token({...});
const id = newToken.id;
const ownerAddress = newToken.owner;

// The following methods should be static because different token ids and addresses should be allowed to query
const owner = Token.getOwner(newToken);
const accounts = Token.getAccounts(newToken);
const balance = Token.getBalance({
  address,
  tokenId,
});

Splitting up this.transfer

After reviewing some POC code interacting with tokens, it was brought up that this.transfer for a zkApp is overloaded in terms of responsibilities. Currently, this.transfer will handle transferring between a zkApp and a specified party, transferring between two separate parties, minting, and burning of tokens. Instead of having this.transfer accomplish all this, we can separate out these functions for ease of use.

Firstly, we can attach a this.token property to zkApps. When we call this.token, it will return an object containing a set of functions to handle interacting with tokens plugin to the zkApp nicely. These functions will be a split-up version of this.transfer for better decoupling of responsibilities.

The set of functions could look something like this:

// Return an object with a set of functions for minting, burning, and sending tokens
this.token();

// Get the token id for this given zkApp (assuming the MINA default)
this.token().id();

// Get the token id for this given zkApp specifiying another token to derive from
this.token("some-other-token-id").id();

// Mint tokens to a specified Party address
this.token().mint({
  address: somePublicAddress,
  amount: 100_000,
});

// Burn tokens of a specified Party address
this.token().burn({
  address: somePublicAddress,
  amount: 100_000,
});

// Send tokens to a specified Party address with the zkApp being the sender
this.token().transfer({
  to: somePublicAddress,
  amount: 100_000,
  tokenId: "custom-token-id", // Will default to the MINA token id if nothing is specified
});

// Send tokens to a specified Party address with the `someOtherAddress` being the sender
this.token().transfer({
  to: somePublicAddress,
  from: someOtherAddress, // This defaults to `this.address` if not specified
  amount: 100_000,
  tokenId: "custom-token-id", // Will default to the MINA token id if nothing is specified
});

Renaming

One issue that was brought up was the naming of this function. Perhaps we can come up with a better name than createNewTokenId as this is a pretty loaded name for a function. Ideas such as deriveTokenId and tokenId were suggested, but nothing was concretely decided. This is still a point of discussion but can easily be changed, so implementation is not halted by this issue.

Additionally, the property of tokenId in createNewTokenId was not as clear as we wanted. Because tokens are derived from another existing token, perhaps it would be helpful to rename tokenId to something like parentTokenId. The code could look like:

// Old way
let customToken = createNewTokenId({
  address: this.address,
  tokenId: defaultMinaTokenId(),
});

// Using `deriveTokenId` name
let customToken = deriveTokenId({
  address: this.address,
  tokenId: defaultMinaTokenId(),
});

// Using `tokenId` name
let customToken = tokenId({
  address: this.address,
  tokenId: defaultMinaTokenId(),
});

// Change with `parentTokenId`
let customToken = createNewTokenId({
  address: this.address,
  parentTokenId: defaultMinaTokenId(),
});

Adding Useful Preconditions for Developers

Another issue that was brought up was adding an API that zkApp developers could use that adds common precondition patterns for tokens. The thought was that writing preconditions for every zkApp could become tedious, so abstracting out some common preconditions would be helpful for developers. This is a great idea but needs to be thought out more as there have been no strong recommendations on what to add. This could be added later once we understand how zkApp developers use preconditions since this is hard to predict given the current ecosystem. Some initial ideas are:

  1. Ensure there is a max amount of a current token being minted
  2. Ensure that given some on-chain state, do something in tokens as a response
  3. ...

One area where we can draw motivation is how Solidity developers use require in their smart contracts. If we can find high-level patterns useful across a different set of smart contracts, we could also apply that to SnarkyJS.

Adding Events

One feature that would be nice for users is the option for Events with tokens in SnarkyJS. For example, Ethereum ERC 777 supports several different events, most notably being:

  1. Sent(...)
  2. Minted(...)
  3. Burned(...)

Events are still yet to be added in SnarkyJS, so this feature will have to wait until they are added. However, once added, we can scope out adding Events for the 3 events listed above.

Open Items

[] - Decide on the particular naming we want to adopt
[] - Decide on the type of API we want to support (e.g., class-based, functional based?)
[] - Explore potential options for preconditions API for token use cases
[] - Decide what sorts of events we want to support for tokens. However, this might be better handled by the zkApp developer.

@mitschabaude
Copy link
Collaborator

mitschabaude commented Jun 29, 2022

Very nice! Some notes:

this.token().id();

I think id doesn't have to be a function here, could be this.token().id

this.token().transfer({
to: somePublicAddress,
amount: 100_000,
tokenId: "custom-token-id", // Will default to the MINA token id if nothing is specified
});

I don't think we should be able to pass in a token id here, and it also shouldn't default to the MINA token. That's confusing if token() is an object that already refers to some specific token id. Instead, this.token().transfer should always use the token id derived from this.address.

Also, I think this method should require the from field

To send MINA, I think we should have the following API, separate from the token namespace:

this.send({to: someAdress, amount: 10e9 }); // send MINA from this SmartContract

let party = Party.createSigned(privateKey); // create party from a private key
party.send({ to: someAdress, amount: 10e9 }); // send MINA from the party's account

the send method could take an additional argument, tokenId, to send a custom token. however, I imagine that a more convenient / typical pattern to send a custom token would be to do it directly inside a method on the token owner account with this.token().transfer(..), not in a SmartContract that is called by that token owner contract.

with the send API, a smart contract would do this to receive MINA:

Party.createSigned(privateKey).send({ to: this.address, amount: 10e9 })

a shortcut for that could be:

this.receive({ from: privateKey, amount: 10e9 });

@MartinMinkov
Copy link
Contributor Author

I don't think we should be able to pass in a token id here, and it also shouldn't default to the MINA token. That's confusing if token() is an object that already refers to some specific token id. Instead, this.token().transfer should always use the token id derived from this.address.
Also, I think this method should require the from field

To send MINA, I think we should have the following API, separate from the token namespace
The send method could take an additional argument, tokenId, to send a custom token. however, I imagine that a more convenient / typical pattern to send a custom token would be to do it directly inside a method on the token owner account with this.token().transfer(..), not in a SmartContract that is called by that token owner contract.

Got it! So we will have two API calls here.

// Sends MINA
this.send({to: someAdress, amount: 10e9 }); 

// Sends custom token derived from `this.address` and the MINA token identifier
this.transfer({
  to: someAddress,
  from: this.address,
  amount: 10e9
})

Did I get this correct? In addition to this.send() we will also have party.send()

@mitschabaude
Copy link
Collaborator

mitschabaude commented Jun 30, 2022

yes, except that it should be this.token().transfer(..) instead of this.transfer(..). so I propose to not have this.transfer at all (but if we have it, it should have similar semantics to this.send / this.receive, i.e. defaulting to the MINA token, while anything on this.token(..) has the token that's derived from this.address and the optional parent token)

I don't have strong feelings about what to call send and what to call transfer. if anything, transfer feels like a "generalization" of send & receive and so makes sense on an API that takes both a to and a from field

@jasongitmail
Copy link
Contributor

I like where this has come / is going!

this.token().id();

I think id doesn't have to be a function here, could be this.token().id

I agree this.token().id is more intuitive and readable, but should we be concerned with the potential for a dev to misuse this and attempt to set the value via this.token().id = 'abc';? @mitschabaude

I don't have a strong opinion either way, just want to be sure we've recognize this as a potential failure case and are ok with it.

To send MINA, I think we should have the following API, separate from the token namespace:

this.send({to: someAdress, amount: 10e9 }); // send MINA from this SmartContract

let party = Party.createSigned(privateKey); // create party from a private key
party.send({ to: someAdress, amount: 10e9 }); // send MINA from the party's account

+1

// send MINA
this.send({to: someAdress, amount: 10e9 }); 

// Sends custom token derived from `this.address` and the MINA token identifier
this.token().send({
  // from: this.address, // make default, if not provided?
  to: someAddress,
  amount: 10e9
});
  • It seems more intuitive to use the same word (send or transfer) for when sending either Mina or custom tokens.
  • Is the from property necessary for the latter? We could default to this.address and it'd feel intuitive and mirror this.send()

@bkase
Copy link
Member

bkase commented Jul 1, 2022

I think it's too unintuitive to default from to this.address for either tokens or normal Mina, I think this really hurts understandability -- also devs can write their own helper function if they're annoyed at having to write out the from explicitly.

I agree with Jason, shouldn't we stick to the same name? (transfer is best I think)

@jasongitmail
Copy link
Contributor

I think it's too unintuitive to default from to this.address for either tokens or normal Mina, I think this really hurts understandability

I'm fine to start with it being required too. It's easy to make something optional later, harder to go from optional to required.

@MartinMinkov MartinMinkov mentioned this issue Jul 5, 2022
@MartinMinkov
Copy link
Contributor Author

Testing

To ensure that the SnarkyJS implementation of tokens is sound, there should some sort of testing story that exists. Currently, there is no integration infrastructure in the CI workflow, so we will ignore that part of the testing story for now. This leaves us with unit tests to ensure that the tokens implementation is working correctly.

We can add a new test file in the SnarkyJS repo which holds all the unit tests needed for tokens. The unit tests should confirm the following behaviors:

  1. A zkApp can derive a custom token successfully
  2. A zkApp can mint tokens for an existing account in the ledger
  3. A zkApp can burn tokens for an existing account, given the appropriate authorization
  4. A zkApp can send tokens from its own balance to another existing account
  5. A zkApp can facilitate an account sending tokens to another account, given the appropriate authorization
  6. Some basic preconditions are set around token minting/burning/sending and the transaction logic follows those preconditions
  7. A zkApp can facilitate minting, burning, and sending custom tokens in a single zkApp method
  8. ...

These unit tests can be run when the main branch is updated in the SnarkyJS repo. This will also ensure that any new changes to the SnarkyJS repo will behave up to spec.

@jasongitmail
Copy link
Contributor

Implemented

@mitschabaude mitschabaude added the rfc Issues that contain some important documentation label Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Issues that contain some important documentation
Projects
None yet
Development

No branches or pull requests

4 participants