search

near / NEPs

The Near Enhancement Proposals repository
https://nomicon.io
216 stars 138 forks source link

Proposal: Allowance-free vault-based token standard #122

Closed evgenykuzyakov closed 3 years ago

evgenykuzyakov commented 4 years ago

Rational:

Background

There are a few reasons for allowance:

Allowance is also often abused by dApps setting unlimited allowance all the time, so it defeats the purpose.

Safe-based transfers

Instead of having permanent allowance, we can introduce a one-time temporary allowance that only lives for the duration of the transaction. We call this a safe. This idea is very similar to Auto-unlock with Safes idea, but it doesn't require protocol changes in the nearcore Runtime.

It works the following way:

We call this temporary lock a safe.

Implementation

Token interface

/// Simple transfers
/// Gas requirement: 5 TGas or 5000000000000 Gas
/// Should be called by the balance owner.
///
/// Actions:
/// - Transfers `amount` of tokens from `predecessor_id` to `receiver_id`.
pub fn transfer_unsafe(&mut self, receiver_id: ValidAccountId, amount: U128);

/// Transfer to a contract with payload
/// Gas requirement: 40+ TGas or 40000000000000 Gas.
/// Consumes: 30 TGas and the remaining gas is passed to the `receiver_id` (at least 10 TGas)
/// Should be called by the balance owner.
/// Returns a promise, that will result in the unspent balance from the transfer `amount`.
///
/// Actions:
/// - Withdraws `amount` from the `predecessor_id` account.
/// - Creates a new local safe with a new unique `safe_id` with the following content:
///     `{sender_id: predecessor_id, amount: amount, receiver_id: receiver_id}`
/// - Saves this safe to the storage.
/// - Calls on `receiver_id` method `on_token_receive(sender_id: predecessor_id, amount, safe_id, payload)`/
/// - Attaches a self callback to this promise `resolve_safe(safe_id, sender_id)`
pub fn transfer_with_safe(
    &mut self,
    receiver_id: ValidAccountId,
    amount: U128,
    payload: String,
) -> Promise;

/// Withdraws from a given safe
/// Gas requirement: 5 TGas or 5000000000000 Gas
/// Should be called by the contract that owns a given safe.
///
/// Actions:
/// - checks that the safe with `safe_id` exists and `predecessor_id == safe.receiver_id`
/// - withdraws `amount` from the safe or panics if `safe.amount < amount`
/// - deposits `amount` on the `receiver_id`
pub fn withdraw_from_safe(
    &mut self,
    safe_id: SafeId,
    receiver_id: ValidAccountId,
    amount: U128,
);

/// Resolves a given safe
/// Gas requirement: 5 TGas or 5000000000000 Gas
/// A callback. Should be called by this fungible token contract (`current_account_id`)
/// Returns the remaining balance.
///
/// Actions:
/// - Reads safe with `safe_id`
/// - Deposits remaining `safe.amount` to `sender_id`
/// - Deletes the safe
/// - Returns the total withdrawn amount from the safe `original_amount - safe.amount`.
/// #[private]
pub fn resolve_safe(&mut self, safe_id: SafeId, sender_id: AccountId) -> U128;

Receiving side interface

/// Called when a given amount of tokens is locked in a safe by a given sender with payload.
/// Gas requirements: 2+ BASE
/// Should be called by the fungible token contract
///
/// This methods should withdraw tokens from the safe and act on them. When this method returns a value, the
/// safe will be released and the unused tokens from the safe will be returned to the sender.
/// There are bunch of options what the contract can do. E.g.
/// - Option 1: withdraw and account internally
///     - Increase inner balance by `amount` for the `sender_id` of a token contract ID `predecessor_id`.
///     - Promise call `withdraw_from_safe(safe_id, receiver_id: env::current_account_id(), amount)` to withdraw the amount to this contract
///     - Return the promise
/// - Option 2: Simple redirect to another account
///     - Promise call `withdraw_from_safe(safe_id, receiver_id: ANOTHER_ACCOUNT_ID, amount)` to withdraw to `ANOTHER_ACCOUNT_ID`
///     - Return the promise
/// - Option 3: Partial redirect to another account (e.g. with commission)
///     - Promise call `withdraw_from_safe(safe_id, receiver_id: ANOTHER_ACCOUNT_ID, amount: ANOTHER_AMOUNT)` to withdraw to `ANOTHER_ACCOUNT_ID`
///     - Chain with (using .then) promise call `withdraw_from_safe(safe_id, receiver_id: env::current_account_id(), amount: amount - ANOTHER_AMOUNT)` to withdraw to self
///     - Return the 2nd promise
/// - Option 4: redirect some of the payments and call another contract `NEW_RECEIVER_ID`
///     - Promise call `withdraw_from_safe(safe_id, receiver_id: current_account_id, amount)` to withdraw the amount to this contract
///     - Chain with promise call `transfer_with_safe(receiver_id: NEW_RECEIVER_ID, amount: SOME_AMOUNT, payload: NEW_PAYLOAD)`
///     - Chain with the promise call to this contract to handle callback (in case we want to refund).
///     - Return the callback promise.
on_receive_with_safe(sender_id: ValidAccountId, amount: U128, safe_id: SafeId, payload: String);
miohtama commented 4 years ago

Here I will drop some comments in random order.

on_receive_with_safe(sender_id: ValidAccountId, amount: U128, safe_id: SafeId, payload: String);

I suggest having payload asVec<u8> as it is more efficient. Most use cases with message involved determining the message type (first byte) and then parsing its content and taking a different action based on the message type. If someone wants to transfer human-readable data then they can just decode payload as UTF-8.

The following is only maybe good idea: A lot of smart contract use cases need to know the current balance (balance_of) and the incoming transfer amount. I suggest adding two amounts: amount_transferred and amount_total, because this way the receiving contract avoids the round-trip to ask "what is my current balance now". Alternatively the receving contract can also keep the total balance on itself as a counter, but it is just simpler to pass it as an argument.

miohtama commented 4 years ago 
pub fn transfer(receiver_id: ValidAccountId, amount: U128);

I suggest trying to simplify so that there is only one entry-point for transfers:

pub fn transfer(receiver_id: ValidAccountId, amount: U128, payload: Vec<u8>);

Do not make two similar functions, as the end-users still may end up to use the wrong one. Here is an example of an end-user executing ERC-20 transfer() against a contract account:

https://etherscan.io/tx/0x18ff0886581735dc189bdd96a77f01b43a03fe769055fe25f78f3c7d2c1e5502

In this case, the contract or the user cannot recover from this situation.

Here are some different use cases we need to separate out

Ideally they all would use the same call signature, and behind the scenes the token smart contract would construct the promise in such a way that transfer_with_safe is used internally when needed. However, a new feature for NEAR VM might be needed: either by executing a promise (on_receive_with_safe) only if the target is a code account or let the smart contract query if the target is a code account.

Alternatively if the above is not possible offer a standardized function to recover from the situation when the wrong transfer was used.

miohtama commented 4 years ago

A good token standard needs some additional features outside the core transfer semantics

1) A good metadata so wallets can display tokens correctly

2) A way for a wallet to easily query all the tokens user have received, to populate the list of assets of the user

3) Standardised mint and burn functionality

For 1) it is just coming up with some core metadata fields besides name

For 2) I have no idea how it is supposed to work on NEAR: Is there are a centralised (yuc) indexer server that all clients rely on to get this kind of data?

For 3) is now needed more in DeFi scenarios than back in the day when tokens started. Ethereum mints and burns are somwhat fragmented, so it is hard for blockchain explorers like Etherscan to parse when new tokens popped in and out of existence. This leads to the fact that often "the total available supply" number is not correctly tracked.

evgenykuzyakov commented 4 years ago

Regarding payload: String. It's just a generic JSON entry. The receiving side can interpret it how they want. If they want to have Vec<u8>, then there is a helper class Base64VecU8 that takes input as a String and decodes Vec<u8> from base64. The reason for not using raw Vec<u8> is the JSON serialization is not efficient. It'll produce something like: [123, 123, 32, 0]. That's why we prefer to use base64 for encoding raw bytes.

miohtama commented 4 years ago

Are promises going to be serialised as JSON forever? Isn't that quite inefficient.

evgenykuzyakov commented 4 years ago

Yep. It seems we're not going back to Borsh anytime soon. So I can assume it'll be json forever :)

luciotato commented 4 years ago

@evgenykuzyakov would you consider also adding a secondary option transfer_to_contract(receiver, amount, payload) , so contract developers can have one more option and use the most convenient according to what's required for the particular interaction?

transfer_to_contract sequence:

/// Direct transfer to contract
/// Gas requirement: 5 TGas or 5000000000000 Gas
/// Should be called by the balance owner.
///

/// Transfer to contract with payload
/// Gas requirement: 40+ TGas or 40000000000000 Gas.
/// Consumes: 30 TGas and the remaining gas is passed to the `receiver_id` (at least 10 TGas)
/// Should be called by the balance owner.
/// chains a then-promise, that will undo the transfer if `receiver.on_transfer_sent(payload)` promise fails
///
/// Actions:
/// - Transfer `amount` from the `predecessor_id` account to `receiver_id` contract account.
/// - Calls on `receiver_id` method `on_transfer_sent(sender_id: predecessor_id, amount, payload)`/
/// - Attaches a then-callback `on_callback_transfer_to_contract(receiver_id, amount)`
pub fn transfer_to_contract(
    &mut self,
    receiver_id: ValidAccountId,
    amount: U128,
    payload: String,
);

/// Callback to check if the receiving contract processed ok
/// Gas requirement: 5 TGas or 5000000000000 Gas
/// A callback. Should be called by this fungible token contract (`current_account_id`)
/// undo the transfer if the previous promise failed
///
/// Actions:
/// - check if is_promise_success()
/// - if the previous promise failed, undoes the transfer
/// #[private]
pub fn on_callback_transfer_to_contract(&mut self, receiver_id: AccountId, amount: U128) -> U128;

Receiving side interface

/// Called when a given amount of tokens were transferred to this contract
/// Gas requirements: 2+ BASE
/// Should be scheduled by the fungible token contract during transfer_to_contract()
///
/// This methods should act on the recent transfer. 
/// The receiving contract already has the funds accredited. 
/// This mechanism is simpler but does not have automatic partial refunds.
/// If this method aborts or panics, the callback will undo the transfer (full refund)
/// If the contract needs to do a partial refund it has to schedule a transfer to return the partial funds on the NEP122 contract
/// There are bunch of options what the receiving contract can do. E.g.
/// - Option 1: register the transfer internally:
///     - Increase inner balance by `amount` for the `sender_id` of a token contract ID `predecessor_id`.
///     ...
on_transfer_sent(sender_id: ValidAccountId, amount: U128, payload: String);
evgenykuzyakov commented 4 years ago

transfer_to_contract sequence:

  • First apply the transfer moving amount tokens from predecessor_id to receiver_id account
  • Schedule a call to receiver.on_transfer_sent(sender,amount, payload)
  • If the call fails, undo the transfer, returning amount tokens from receiver_id to predecessor_id account

I think the issue with this solution is to be able to spend transferred tokens while they are in-flight. E.g. a contract may be able to transfer them all out and then fail (in multiple promises). The token contract will not be able to cancel the transfer.

luciotato commented 4 years ago

The counter-argument is that normally receiving contracts have a limit to spend tokens based on its internal accounting. So it can't spend the funds in-flight because they're not registered in its internal accounting.

evgenykuzyakov commented 4 years ago

If feel like safe approach is more explicit. It requires a contract to withdraw the tokens from the safe and specify the amount of tokens to withdraw. It doesn't require the contract not to fail after withdrawal, so it can pass the process further without having a 100% success callback.

Also you may need to do a refund. Consider uniswap with slippage limit. Let's say you want to swap 1000 DAI for 1000 USDT. You send 1001 DAI to account for potential slippage of price and uniswap contract has to refund you for the unused DAI amount.

miohtama commented 4 years ago

The counter-argument is that if you trust the receiving contract/is audited, it should have a limit to spend tokens based on its internal accounting. So it can't spend the funds in-flight because they're not registered in its internal accounting.

This comment makes no sense. You are transferring tokens to a contract, so you are already trusting it.

luciotato commented 4 years ago

Maybe I didn't express that correctly, I'll edit for clarity

evgenykuzyakov commented 4 years ago

FYI. I'm going to update this standard method names and topics to use vault instead of safe to avoid confusion with safe/unsafe

robert-zaremba commented 4 years ago

transfer_to_contract sequence:

* First apply the transfer moving amount tokens from predecessor_id to receiver_id account

* Schedule a call to **receiver.on_transfer_sent(sender,amount, payload)**

* If the call fails, undo the transfer, returning amount tokens from receiver_id to predecessor_id account

I think the issue with this solution is to be able to spend transferred tokens while they are in-flight. E.g. a contract may be able to transfer them all out and then fail (in multiple promises). The token contract will not be able to cancel the transfer. The

In fact, the transfer_to_contract is what we do in NEARswap (we call it transfer_sc). For safety, we can firstly call the callback, and the transfer. I need to write that spec andpropose it for NEP.

jasperdg commented 3 years ago

I'm trying wrap my head around gas sensitivity when there are multiple cross contract calls.

What if the on_receive_with_safe external tx runs out of gas? Wouldn't the funds be stuck? The receiver contract could have some sort of claim_safe_and_callback. A cleaner solution might be if there could be a timelock on the vault that would allow the sender to withdraw the funds after T.

oysterpack commented 3 years ago

I'm trying wrap my head around gas sensitivity when there are multiple cross contract calls.

What if the on_receive_with_safe external tx runs out of gas? Wouldn't the funds be stuck? The receiver contract could have some sort of claim_safe_and_callback. A cleaner solution might be if there could be a timelock on the vault that would allow the sender to withdraw the funds after T.

The funds will not get stuck because the resolve_vault callback always gets called when on_receive_with_safe completes.

oysterpack commented 3 years ago

The proposed spec has evolved based on @evgenykuzyakov latest and greatest "reference implementation" :

My feedback on the latest and greatest is:

Registered Accounts

Thoughts on "How do we finalize the standard ?"

... this is a great discussion but we need to bring this to closure soon in order to drive adoption .... To move forward, I propose having live online discussions (via Zoom or other venue) and design sessions to hammer this out. Can someone from the NEAR DEV team please drive this and set this up? I vote for @evgenykuzyakov :)

... closing thoughts ... we need to work on standardizing the process to standardize

robert-zaremba commented 3 years ago

@oysterpack - I agree - we need to finalize standards to drive the adoption. And there is a need for "reactive" fungible tokens. At the same time we need to make sure that we find a right and robust interface. Please have a look at the NEP-136 Interactive Fungible token - an alternative approach for reactive FT. I like your idea with account registration. I'm happy to accommodate it there.

oysterpack commented 3 years ago

Here's my two cents ... all of the proposed standards have different token "transfer" use cases. We need to decouple the transfer protocol from the Fungible Token. I whipped together some quick and dirty interfaces to illustrate the concept.

Account registration is required, especially on NEAR because of storage usage fees. The benefits for registered accounts are:

The key to decoupling the transfer protocol interface is to let the registered account choose the transfer protocol from the receiver side. Sender's simply want to transfer tokens to the receiver, unaware of the underlying transfer protocol.

The tricky part is gas, because different protocols will have different gas profiles and requirements. Thus, the gas requirements need to be specified as part of the spec on the transfer protocol.

Thoughts?

https://github.com/oysterpack/oysterpack-near-stake-token/blob/main/contract/tests/fungible_token.rs

use near_sdk::json_types::{ValidAccountId, U128};
use near_sdk::{
    ext_contract,
    serde::{Deserialize, Serialize},
    AccountId, Promise, PromiseOrValue,
};

/// The design intent is to decouple the token asset from the token transfer protocol.
///
/// - Fungible token supports 1 or more [TransferProtocol]s as specified per [MetaData]
/// - Accounts must register with the token contract and pay for account storage fees.
///   - account storage fees are escrowed and refunded when the account unregisters
///   - account chooses the transfer protocol to use as transfer recipient
/// - FT has generic [transfer] function interface
/// - sender account does not choose the transfer protocol - the receiver account chooses how they
///   want to receive the tokens
///
/// The key advantage of this design is that it decouples the protocol interface from the implementation.
/// The problem with all of the "standard" interfaces is that they are too tightly coupled with implementation.
/// We need decoupled interface that will allow transfer protocols to evolve.
pub trait FungibleToken: AccountRegistry {
    fn metadata() -> Metadata;

    /// Returns total supply.
    /// MUST equal to total_amount_of_token_minted - total_amount_of_token_burned
    fn total_supply(&self) -> U128;

    /// Returns the token balance for `holder` account
    fn balance_of(&self, account_id: ValidAccountId) -> U128;

    /// ## Panics
    /// - if accounts are not registered
    /// - insufficient funds
    fn transfer(
        &mut self,
        receiver_id: ValidAccountId,
        amount: U128,
        msg: Option<String>,
        memo: Option<String>,
    ) -> PromiseOrValue<TransferProtocol>;
}

/// Suggested protocol names:
/// - NEP_122
/// - NEP_136
/// - NEP_21
///
/// - each protocol defines min amount of gas required excluding gas required to cover `msg` `memo`
pub struct TransferProtocol(String, Gas);

pub struct Gas(pub u64);

pub trait AccountRegistry {
    /// Registers the predecessor account ID with the contract.
    /// The account is required to pay for its storage. Storage fees will be escrowed and refunded
    /// when the account is unregistered.
    ///
    /// #[payable]
    /// - storage escrow fee is required
    ///   - use [account_storage_escrow_fee] to lookup the required storage fee amount
    /// - any amount above the storage fee will be refunded
    ///
    /// ## Panics
    /// - if deposit is not enough to cover storage fees
    /// - is account is already registered
    /// - if transfer protocol is not supported
    ///
    /// #[payable]
    fn register_account(&mut self, transfer_protocol: TransferProtocol);

    /// Unregisters the account and refunds the escrowed storage fees.
    ///
    /// ## Panics
    /// - if account is not registered
    /// - if registered account has funds
    fn unregister_account(&mut self);

    /// changes the account's transfer type
    ///
    /// ## Panics
    /// - if account is not registered
    /// - if transfer protocol is not supported
    fn set_transfer_type(&mut self, transfer_protocol: TransferProtocol);

    /// Burns owned token funds unregisters the account. Escrowed storage fees are refunded.
    ///
    /// ## Panics
    /// - if account is not registered
    fn burn_account(&mut self);

    ////////////////////////////
    ///     VIEW METHODS    ///
    //////////////////////////

    /// Returns the required deposit amount that is required for account registration.
    fn account_storage_fee(&self) -> U128;

    fn account_registered(&self, account_id: ValidAccountId) -> bool;

    /// returns None if the account is not registered
    fn account_transfer_protocol(&self, account_id: ValidAccountId) -> Option<String>;

    fn total_registered_accounts(&self) -> U128;
}

/// Each token must have 18 digits precision (decimals)
pub const DECIMALS: u8 = 18;

pub struct Metadata {
    pub name: String,
    pub symbol: String,

    /// URL to additional resources about the token.
    pub reference: String,

    /// the smallest part of the token that’s (denominated in e18) not divisible
    /// In other words, the granularity is the smallest amount of tokens (in the internal denomination)
    /// which MAY be minted, sent or burned at any time.
    /// - The following rules MUST be applied regarding the granularity:
    /// - The granularity value MUST be set at creation time.
    /// - The granularity value MUST NOT be changed, ever.
    /// - The granularity value MUST be greater than or equal to 1.
    /// - All balances MUST be a multiple of the granularity.
    /// - Any amount of tokens (in the internal denomination) minted, sent or burned MUST be a
    ///   multiple of the granularity value.
    /// - Any operation that would result in a balance that’s not a multiple of the granularity value
    ///   MUST be considered invalid, and the transaction MUST revert.
    ///
    /// NOTE: Most tokens SHOULD be fully partition-able. I.e., this function SHOULD return 1 unless
    ///       there is a good reason for not allowing any fraction of the token.
    pub granularity: U128,

    /// Transfer protocols that are supported by the token contract
    pub supported_transfer_protocols: Vec<TransferProtocol>,
}
oysterpack commented 3 years ago

To help drive this, we should pull in the NEAR wallet team into the discussion ... the NEAR wallet should/must have built in support for FT and would be an excellent POC to drive the FT interface standardization. If we can make this work for the NEAR wallet, then it would pave the way for community adoption and be a boon for the NEAR ecosystem.

evgenykuzyakov commented 3 years ago

I agree that the account registration should be a separate standard. Because this pattern is likely will be used for other contracts and standards.

As for the fungible token standard, I tried to design the bare minimum that is required for the transfers to work.

Metadata is not part of the standard, because it's not needed for the main functionality to work. It's required for wallets to display the token, but it doesn't have to belong to this particular standard.

As for the memo, it's an interesting question. Since we use JSON, we can include it as an optional field, that will be automatically ignored by the contract that do not want to work with memo.

Also, note that this standard lacked balance_of view method, which probably should be included into this standard.

robert-zaremba commented 3 years ago

Here's my two cents ... all of the proposed standards have different token "transfer" use cases. [...]

It this a comment to this proposal, or to https://github.com/near/NEPs/issues/136 ?

robert-zaremba commented 3 years ago

I was writing about a need for memo in last summer, and included it the first version of NEARswap (for the multi-tokens). I don't think we should use Option<> in public API though. It complicates. Most of the types should be simple. Option could be used for an optional numeric value.

robert-zaremba commented 3 years ago

@oysterpack , your motivation for AccountRegistry is to separate the accounting from storage, right? I don't think it will work here, because token implementations may have different storage layout (that being said, I believe most of them will be simple and have the same layout). Moreover separating AccountRegistry from Token into 2 different contracts will complicate implementation - each transfer will require additional async call.

evgenykuzyakov commented 3 years ago

I was writing about a need for memo in last summer, and included it the first version of NEARswap (for the multi-tokens). I don't think we should use Option<> in public API though. It complicates. Most of the types should be simple. Option could be used for an optional numeric value.

memo: Option<String> in Rust will be handled fairly easy. It's either there in json like {"memo": "ABC"} or it's just not there {} or {"memo": null}. Having it as required for something the contract doesn't use is waste of gas, similar how having a full metadata in the contract is not required. Just a verifiable link to get it.

evgenykuzyakov commented 3 years ago

But if we standardize events as part of this NEP and one of the event requires to put a memo, then we can include it as required. But even an event may have optional fields.

robert-zaremba commented 3 years ago

@evgenykuzyakov is an empty string ("") more expensive than null? If so, is it "noticeable" to the contract call?

oysterpack commented 3 years ago

@robert-zaremba I am not saying to have a separate contract for AccountRegistry - I am saying the token contract should implement the AccountRegistry interface in addition to the FungibleToken interface:

pub trait FungibleToken: AccountRegistry {
...
}

and regarding Option ... it is a best practice because the intent is explicit, i.e., something that is optional should be wrapped in Option

robert-zaremba commented 3 years ago

Ah, I didn't notice the inheritance. Re Option: how do you distinguish None from Some("") (in the business logic)? I see a reason for it for numeric arguments, or structural arguments.

oysterpack commented 3 years ago

@robert-zaremba

Re Option: how do you distinguish None from Some("") (in the business logic)? I see a reason for it for numeric arguments, or structural arguments.

Required numeric arguments should never be passed in as empty string, and it is better to fail fast with a validation error. The intent should be clear ... let the type system do its job - it makes the code cleaner, more robust, prevents bugs, helps protect against human error, etc

That being said, let's say you had reason for a type such as Option<String>, then you would simply use a match clause to distinguish None from Some(""):

   let value: Option<String> = Some("".to_string());
    match value {
        None => println!("value is None"),
        Some(value) if value.is_empty() => println!("value is empty string"),
        Some(value) => println!("value is {}", value),
    }
oysterpack commented 3 years ago

I thought it would be good to share some working code ... I have implemented NEP-122 for the STAKE token project I am working on using @evgenykuzyakov's implementation within Berry Farm as a reference with the following modifications:

  1. all token owners must be registered with the contract, which implies that token transfers can only be between registered accounts
    • this removes the need to require an attached deposit on each transfer because the accounts are pre-registered
    • the STAKE token contract requires pre-registration because it collects account storage usage fees upfront ... there is no free storage (the storage fees are escrowed and refunded to the account when it unregisters)
    • eliminates transfers to non-existent accounts
  2. renamed transfer_raw to transfer

Code

VaultFungibleToken

The contract is deployed on testnet at stake.oysterpack.testnet

NEAR CLI Examples

# VaultFungibleToken
near view stake.oysterpack.testnet get_total_supply
near view stake.oysterpack.testnet get_balance --args '{"account_id":"alfio-zappala-oysterpack.testnet"}'
near call stake.oysterpack.testnet transfer --accountId alfio-zappala-oysterpack.testnet --args '{"receiver_id":"oysterpack.testnet", "amount":"1000000000000"}'

# AccountManagement
# returns the account storage fee amount that is required when registering an account
near view stake.oysterpack.testnet account_storage_fee
# payment is required for account storage usage fee (currently 0.0681 NEAR) - change is refunded
near call stake.oysterpack.testnet register_account --accountId oysterpack.testnet --amount 1
near call stake.oysterpack.testnet unregister_account --accountId alfio-zappala-oysterpack.testnet

NOTE: I haven't tested the transfer_with_vault on testnet yet ...

Feedback is welcomed and appreciated.

robert-zaremba commented 3 years ago

Required numeric arguments should never be passed in as empty string, and it is better to fail fast with a validation error.

@oysterpack, yes, this is what I meant.

robert-zaremba commented 3 years ago

@oysterpack - in your use-case, what is the advantage of using vaults vs #136 ?

oysterpack commented 3 years ago

@robert-zaremba I would say it depends on the specific use case which is determined by the specific receiver.

I chose to implement the vault based implementation simply because I needed to choose something for my STAKE token project (and even my implementation is a variant because of the requirement for registered accounts) ...

the use cases for both transfer_with_call and transfer_with_vault is similar but different ... Both are designed to transact with another contract through a predefined protocol, i.e., the receiver must implement a specific function interface.

Both require a "finalize" callback on the token contract, in NEP-122 this is defined as

fn resolve_vault(&mut self, vault_id: VaultId, sender_id: AccountId) -> U128;

and in NEP-136 the proposed "finalize" callback is finalize_token_call()

NEP-122 locks up a "temporary one-time allowance" within the "vault", and it is up to the receiver to withdraw the tokens from the vault. NEP-136 transfer_call is a specialization of NEP-122 transfer_with_vault where the receiver would always withdraw all tokens from the vault. Thus, NEP-122 vault transfers covers the transfer_call use case, but with more overhead, i.e., NEP-136 is more efficient for the use case where no allowance is required and all tokens are transferred before notifying the receiver about the token transfer.

... this brings me back to my previous comment in the discussion ...

There is a use case for both ... and there might be more use cases we think of in the future ... the problem with the current proposed fungible token standards is that the interfaces are too tightly coupled with the underlying transfer protocol. We need a generic FT transfer interface that is decoupled from the multitude of token transfer use cases. We have been discussing the following token transfer use cases:

  1. NEP-21 simple transfer
  2. NEP-21 long term allowance support
    • NOTE: to prevent abuse, it must require storage usage fee to setup the allowance
  3. NEP-122 transfer with vault (one-time temporary allowance)
  4. NEP-136 reactive transfer

When the receiver is a contract, it should decide how to receive the tokens. If the token contract supports account registration then the receiver contract can specify as part of its "preferences" how it wants to receive tokens.

Go back and take a look at: https://github.com/oysterpack/oysterpack-near-stake-token/blob/main/contract/tests/fungible_token.rs

This discussion has been focused on the token transfer protocol, but standardizing metadata and having a token registry are also fundamental ... we need to have that discussion, but let's first continue to focus on the token transfer interface.

In closing, I leave with you with some food for thought ...

How can we bring this FT standard home and to a conclusion ?

"the proof is in the pudding" ... we need to drive and prove the standard with live code. I offer my STAKE token project to be used as a POC, but that is only side of the equation. We need to prove is end-to-end. We also need POC contracts that represent receiver accounts. And most importantly, to prove the usability and complete experience we need an FT wallet POC application. The logical best choice that comes to mind is the NEAR wallet. Including standard FT support in the NEAR wallet would be huge and pave the way for an illustrious FT market. This is key to deliver on NEAR's vision:

To help drive this standard and bring it home, it needs leadership and some form of dedicated "task-force". We need to avoid the analysis-paralysis trap ... this is where we need to get folks from the NEAR team more engaged

robert-zaremba commented 3 years ago

@oysterpack I like that. However I don't expect that something as NEAR Wallet will support many standards. For the moment only NEP-21 is finalized.

I can use your STAKE code for NEP-136 PoC unless you will like to do it.

oysterpack commented 3 years ago

@robert-zaremba since I am actively working on it, I can whip that together pretty fast. I'll let you know once it's deployed on testnet ...

regarding NEAR wallet ... FT should be a core NEAR standard ... in order to harness the Internet of value, then the value must be easy to trade and transfer ... the path to fastest adoption is wallet adoption ... thus, regardless, we need to ensure the FT standard makes it simple for wallet integration and the end consumer. Having standard FT support within the NEAR wallet would be a game changer and provide a turbo boost for building dApps ... IMHO

oysterpack commented 3 years ago

@robert-zaremba the latest and greatest version of the STAKE token has been deployed to testnet at stake.oysterpack.testnet

It implements 3 types of FT transfer protocols:

  1. simple transfer (NEP-21)
  2. vault based transfer (NEP-122)
  3. transfer call (NEP-136)- I named the interface

see the code and it's also included below

NOTES

use crate::domain::{self, Gas};
use near_sdk::json_types::{ValidAccountId, U128};
#[allow(unused_imports)]
use near_sdk::AccountId;
use near_sdk::{
    borsh::{self, BorshDeserialize, BorshSerialize},
    ext_contract,
    serde::{Deserialize, Serialize},
    Promise,
};

/// - Fungible token supports 1 or more [TransferProtocol]s as specified per [MetaData]
/// - Accounts must register with the token contract and pay for account storage fees.
///   - account storage fees are escrowed and refunded when the account unregisters
///   - account chooses the transfer protocol to use as transfer recipient
pub trait FungibleToken {
    fn metadata(&self) -> Metadata;

    /// Returns total supply.
    /// MUST equal to total_amount_of_token_minted - total_amount_of_token_burned
    fn total_supply(&self) -> U128;

    /// Returns the token balance for `holder` account
    fn balance(&self, account_id: ValidAccountId) -> U128;
}

#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(crate = "near_sdk::serde")]
pub struct Metadata {
    pub name: String,
    pub symbol: String,

    /// URL to additional resources about the token.
    pub reference: Option<String>,

    /// the smallest part of the token that’s (denominated in e18) not divisible
    /// In other words, the granularity is the smallest amount of tokens (in the internal denomination)
    /// which MAY be minted, sent or burned at any time.
    /// - The following rules MUST be applied regarding the granularity:
    /// - The granularity value MUST be set at creation time.
    /// - The granularity value MUST NOT be changed, ever.
    /// - The granularity value MUST be greater than or equal to 1.
    /// - All balances MUST be a multiple of the granularity.
    /// - Any amount of tokens (in the internal denomination) minted, sent or burned MUST be a
    ///   multiple of the granularity value.
    /// - Any operation that would result in a balance that’s not a multiple of the granularity value
    ///   MUST be considered invalid, and the transaction MUST revert.
    ///
    /// NOTE: Most tokens SHOULD be fully partition-able. I.e., this function SHOULD return 1 unless
    ///       there is a good reason for not allowing any fraction of the token.
    pub granularity: u8,

    /// Transfer protocols that are supported by the token contract
    pub supported_transfer_protocols: Vec<TransferProtocol>,
}

impl Metadata {
    /// Each token must have 18 digits precision (decimals)
    pub const DECIMALS: u8 = 18;
}

#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(crate = "near_sdk::serde")]
pub struct TransferProtocol {
    /// Suggested protocol names:
    /// - simple - NEP-21
    /// - allowance - NEP 21
    /// - vault_transfer - NEP-122
    /// - transfer_and_notify - NEP-136
    pub name: String,
    /// - each protocol defines min amount of gas required, excluding gas required to cover `msg` `memo`
    pub gas: Gas,
}

impl TransferProtocol {
    pub fn simple(gas: Gas) -> Self {
        Self {
            name: "simple".to_string(),
            gas,
        }
    }

    pub fn allowance(gas: Gas) -> Self {
        Self {
            name: "allowance".to_string(),
            gas,
        }
    }

    pub fn vault_transfer(gas: Gas) -> Self {
        Self {
            name: "vault_transfer".to_string(),
            gas,
        }
    }

    pub fn transfer_and_notify(gas: Gas) -> Self {
        Self {
            name: "transfer_and_notify".to_string(),
            gas,
        }
    }
}

pub trait SimpleTransfer {
    /// Simple direct transfers between registered accounts.
    ///
    /// Gas requirement: 5 TGas
    /// Should be called by the balance owner.
    /// Requires that the sender and the receiver accounts be registered.
    ///
    /// Actions:
    /// - Transfers `amount` of tokens from `predecessor_id` to `recipient`.
    ///
    /// ## Panics
    /// - if predecessor account is not registered - sender account
    /// - if [recipient] account is not registered
    /// - if sender account is same as receiver account
    /// - if account balance has insufficient funds for transfer
    fn transfer(
        &mut self,
        recipient: ValidAccountId,
        amount: U128,
        msg: Option<String>,
        memo: Option<String>,
    );
}

pub trait TransferCall {
    /// Transfer `amount` of tokens from the predecessor account to a `recipient` contract.
    /// The recipient contract MUST implement [TransferCallRecipient] interface. The tokens are
    /// deposited but locked in the recipient account until the transfer has been confirmed by the
    /// recipient contract and then finalized. The transfer workflow steps are:
    /// 1. sender initiates the transfer via `transder_call`
    /// 2. token transfers the funds from the sender's account to the recipient's account but locks
    ///    the transfer amount on the recipient account. The locked tokens cannot be used until
    ///    the recipient contract confirms the transfer.
    /// 3. The recipient contract is then notified of the transfer via [`TransferCallRecipient::on_ft_receive`].
    /// 4. Once the transfer notification call completes, then the [`TransferCallRecipient::on_ft_receive`]
    ///    callback on the token contract is invoked to finalize the transfer. If the recipient contract
    ///    successfully completed the transfer notification call, then the funds are unlocked
    ///    via the [`FinalizeTransferCallback::finalize_ft_transfer`] callback. If the [`TransferCallRecipient::on_ft_receive`]
    ///    call fails for any reason, then the fund transfer is rolled back in the finalize callback.
    ///
    /// `msg`: is a message sent to the recipient. It might be used to send additional call
    //      instructions.
    /// `memo`: arbitrary data with no specified format used to link the transaction with an
    ///     external event. If referencing a binary data, it should use base64 serialization.
    ///
    /// ## Panics
    /// - if accounts are not registered
    /// - insufficient funds
    fn transfer_call(
        &mut self,
        recipient: ValidAccountId,
        amount: U128,
        msg: Option<String>,
        memo: Option<String>,
    ) -> Promise;
}

pub trait FinalizeTransferCallback {
    /// Finalizes the token transfer
    ///
    /// Actions:
    /// - if the call `TransferCallRecipient::on_ft_receive` succeeds, then commit the transfer,
    ///   i.e., unlock the balance on the recipient account
    /// - else rollback the transfer by returning the locked balance to the sender
    ///
    /// #[private]
    fn finalize_ft_transfer(&mut self, sender: AccountId, recipient: AccountId, amount: U128);
}

/// Interface for recipient call on fungible-token transfers.
/// `token` is an account address of the token  - a smart-contract defining the token
///     being transferred.
/// `from` is an address of a previous holder of the tokens being sent
#[ext_contract(ext_transfer_call_recipient)]
pub trait TransferCallRecipient {
    fn on_ft_receive(
        &mut self,
        from: ValidAccountId,
        amount: U128,
        msg: Option<String>,
        memo: Option<String>,
    );
}

#[ext_contract(ext_self_finalize_transfer_callback)]
pub trait ExtFinalizeTransferCallback {
    /// Finalizes the token transfer
    ///
    /// Actions:
    /// - if the call `TransferCallRecipient::on_ft_receive` succeeds, then commit the transfer,
    ///   i.e., unlock the balance on the recipient account
    /// - else rollback the transfer by returning the locked balance to the sender
    ///
    /// #[private]
    fn finalize_ft_transfer(&mut self, sender: AccountId, recipient: AccountId, amount: U128);
}

/// Implements [NEP-122 vault based fungible token standard](https://github.com/near/NEPs/issues/122)
/// with the following modifications:
/// - all token owners must be registered with the contract, which implies that token transfers can
///   only be between registered accounts
///   - this removes the need to require an attached deposit on each transfer because the accounts
///     are pre-registered
///   - eliminates transfers to non-existent accounts
/// - `transfer_raw` has been moved to [`SimpleTransfer::transfer`]
/// - `payload` has been replaced with `msg` and `memo` optional args
pub trait VaultBasedTransfer {
    /// Transfer to a contract with payload
    /// Gas requirement: 40+ TGas or 40000000000000 Gas.
    /// Consumes: 30 TGas and the remaining gas is passed to the `recipient` (at least 10 TGas)
    /// Should be called by the balance owner.
    /// Returns a promise, that will result in the unspent balance from the transfer `amount`.
    ///
    /// Actions:
    /// - Withdraws `amount` from the `predecessor_id` account.
    /// - Creates a new local safe with a new unique `safe_id` with the following content:
    ///     `{sender_id: predecessor_id, amount: amount, recipient: recipient}`
    /// - Saves this safe to the storage.
    /// - Calls on `recipient` method `on_token_receive(sender_id: predecessor_id, amount, safe_id, payload)`/
    /// - Attaches a self callback to this promise `resolve_safe(safe_id, sender_id)`
    ///
    /// ## Panics
    /// - if predecessor account is not registered
    /// - if [recipient] account is not registered
    /// - if sender account is same as receiver account
    /// - if account balance has insufficient funds for transfer
    fn transfer_with_vault(
        &mut self,
        recipient: ValidAccountId,
        amount: U128,
        msg: Option<String>,
        memo: Option<String>,
    ) -> Promise;

    /// Withdraws from a given vault and transfers the funds to the specified receiver account ID.
    ///
    /// Gas requirement: 5 TGas
    /// Should be called by the contract that owns a given safe.
    ///
    /// Actions:
    /// - checks that the safe with `vault_id` exists and `predecessor_id == vault.recipient`
    /// - withdraws `amount` from the vault or panics if `vault.amount < amount`
    /// - deposits `amount` on the `recipient`
    ///
    /// ## panics
    /// - if predecessor account is not registered
    /// - if predecessor account does not own the vault
    /// - if [recipient] account is not registered
    /// - if vault balance has insufficient funds for transfer
    fn withdraw_from_vault(&mut self, vault_id: VaultId, recipient: ValidAccountId, amount: U128);
}

/// implements required callbacks defined in [ExtResolveVaultCallback]
pub trait ResolveVaultCallback {
    /// Resolves a given vault, i.e., transfers any remaining vault balance to the sender account
    /// and then deletes the vault. Returns the vault remaining balance.
    ///
    /// Gas requirement: 5 TGas
    ///
    /// Actions:
    /// - Reads safe with `safe_id`
    /// - Deposits remaining `safe.amount` to `sender_id`
    /// - Deletes the safe
    /// - Returns the total withdrawn amount from the safe `original_amount - safe.amount`.
    /// #\[private\]
    ///
    /// ## Panics
    /// - if not called by self as callback
    /// - following panics should never happen (if they do, then there is a bug in the code)
    ///   - if the sender account is not registered
    ///   - if the vault does not exist
    fn resolve_vault(&mut self, vault_id: VaultId, sender_id: AccountId) -> U128;
}

/// Must be implemented by contracts that support [VaultBasedTransfer] token transfers
#[ext_contract(ext_token_receiver)]
pub trait ExtTokenVaultReceiver {
    /// Called when a given amount of tokens is locked in a safe by a given sender with payload.
    /// Gas requirements: 2+ BASE
    /// Should be called by the fungible token contract
    ///
    /// This methods should withdraw tokens from the safe and act on them. When this method returns a value, the
    /// safe will be released and the unused tokens from the safe will be returned to the sender.
    /// There are bunch of options what the contract can do. E.g.
    /// - Option 1: withdraw and account internally
    ///     - Increase inner balance by `amount` for the `sender_id` of a token contract ID `predecessor_id`.
    ///     - Promise call `withdraw_from_safe(safe_id, recipient: env::current_account_id(), amount)` to withdraw the amount to this contract
    ///     - Return the promise
    /// - Option 2: Simple redirect to another account
    ///     - Promise call `withdraw_from_safe(safe_id, recipient: ANOTHER_ACCOUNT_ID, amount)` to withdraw to `ANOTHER_ACCOUNT_ID`
    ///     - Return the promise
    /// - Option 3: Partial redirect to another account (e.g. with commission)
    ///     - Promise call `withdraw_from_safe(safe_id, recipient: ANOTHER_ACCOUNT_ID, amount: ANOTHER_AMOUNT)` to withdraw to `ANOTHER_ACCOUNT_ID`
    ///     - Chain with (using .then) promise call `withdraw_from_safe(safe_id, recipient: env::current_account_id(), amount: amount - ANOTHER_AMOUNT)` to withdraw to self
    ///     - Return the 2nd promise
    /// - Option 4: redirect some of the payments and call another contract `NEW_RECEIVER_ID`
    ///     - Promise call `withdraw_from_safe(safe_id, recipient: current_account_id, amount)` to withdraw the amount to this contract
    ///     - Chain with promise call `transfer_with_safe(recipient: recipient, amount: SOME_AMOUNT, payload: NEW_PAYLOAD)`
    ///     - Chain with the promise call to this contract to handle callback (in case we want to refund).
    ///     - Return the callback promise.
    fn on_receive_with_vault(
        &mut self,
        sender_id: AccountId,
        amount: U128,
        vault_id: VaultId,
        msg: Option<String>,
        memo: Option<String>,
    );
}

#[ext_contract(ext_self_resolve_vault_callback)]
pub trait ExtResolveVaultCallback {
    /// Resolves a given vault - transfers vault remoining balance back to sender account and deletes
    /// the vault.
    ///
    /// Gas requirement: 5 TGas or 5000000000000 Gas
    /// A callback. Should be called by this fungible token contract (`current_account_id`)
    /// Returns the remaining balance.
    ///
    /// Actions:
    /// - Reads safe with `safe_id`
    /// - Deposits remaining `safe.amount` to `sender_id`
    /// - Deletes the safe
    /// - Returns the total withdrawn amount from the safe `original_amount - safe.amount`.
    /// #[private]
    fn resolve_vault(&mut self, vault_id: VaultId, sender_id: AccountId) -> U128;
}

#[derive(BorshDeserialize, BorshSerialize, Serialize, Deserialize, Clone, PartialEq)]
#[serde(crate = "near_sdk::serde")]
pub struct VaultId(pub U128);

impl From<u128> for VaultId {
    fn from(value: u128) -> Self {
        Self(value.into())
    }
}

impl From<domain::VaultId> for VaultId {
    fn from(id: domain::VaultId) -> Self {
        Self(id.0.into())
    }
}
evgenykuzyakov commented 3 years ago

While vault-based token provides clean rollbacks, e.g. the receiver can't overspend the received amount, I'd propose to adapt a slightly modified NEP-136 like standard for transfer_call because of its simplicity. See my comment there: https://github.com/near/NEPs/issues/136#issuecomment-754939099.

Idea is to have the following interface:

#[payable]
fn transfer_call(
    &mut self,
    receiver_id: AccountId,
    amount: U128,
    payload: String,
    memo: Option<String>,
) -> Promise;

It will do the following:

  1. Assert at least 1 yoctoNEAR is attached to a call, to prevent function-call permission access-key security flaw. See attached comment above.
  2. assert the receiver_id account is registered (storage is allocated).
  3. take amount from the predecessor_account_id.
  4. add amount to the receiver_id account balance.
  5. call the receiver_id contract with the on_ft_receive method and pass the payload
  6. attach a callback resolve_transfer to the on_ft_receive call
  7. return the callback
pub trait TransferCallReciever {
    fn on_ft_receive(&mut self, sender_id: AccountId, amount: U128, payload: String) -> U128;
}

The receiver contract should return used_amount - the amount of tokens the receiving contract keeps from the sender's transfer amount. If the promise fails or the returned amount is not valid, it's the same as used_amount == 0.

Now the resolve_transfer method should refund to sender_id the amount amount - used_amount from the receiver_id account. If the receiver_id account doesn't have the full balance (malicious or a contract with a bug), it returns the remaining available balance.

It has the following pros and cons comparing to vault transfer: PROS:

CONS:

Token metadata, account registration, and balance view calls can be discussed separately.

robert-zaremba commented 3 years ago

@evgenykuzyakov are you planning to add transfer_call to NEP-122? If not then let's continue the discussion in #136 .

Re CONS:

oysterpack commented 3 years ago

I would propose consolidating all of the open FT related NEPs into a single NEP the FT discussion - and close out the rest.

Let's create a new github repo that defines the contract interface. The goal of the new git repo would be that anybody that wants to implement the FT token standard interface can simply pull it into their project. Once we come to agreement on the contract interface then we can even officially publish the crate to https://crates.io

evgenykuzyakov commented 3 years ago

@evgenykuzyakov are you planning to add transfer_call to NEP-122? If not then let's continue the discussion in #136 .

No I don't think we should continue work on this NEP. At the same time #136 contains token metadata that I don't agree with. I would make sense to either remove some items from the metadata or remove metadata completely from the standard and move it to a different NEP.

Also account registration should be a separate NEP. @robert-zaremba if you agree, I suggest split NEP#136 into 3 NEPs to separately discuss the following:

I'll start working on 3 new NEPs to cover each part.

robert-zaremba commented 3 years ago

@evgenykuzyakov - yes, this makes sens. The final FT token will be a merge of the 3 other interfaces.

oysterpack commented 3 years ago

perfect - we need to also close #136, #110, #102, #121