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

fix(contract-standards): allow ft_transfer_call deposits #344

Closed
wants to merge 1 commit into from

Conversation

chadoh
Copy link
Contributor

@chadoh chadoh commented Mar 17, 2021

Related to discussion at https://gov.near.org/t/discuss-improving-contract-development-chain-improvements/940/6

If a receiver contract wants to, say, register a user for a different FT (in the case of AMM swaps), it will need some attached deposit.

TODO

  • Update spec to match

If the receiver contract wants to, say, register a user for a
different FT (in the case of AMM swaps), it will need some attached
deposit.
@chadoh
Copy link
Contributor Author

chadoh commented Mar 17, 2021

Marking as draft until we 1. get satisfactory discussion around this and 2. update the Nomicon spec.

@evgenykuzyakov
Copy link
Contributor

As I've described before, the issue with this approach is the attached deposit to ft_transfer_call might be lost or consumed by the FT contract. It's unclear from the standard where the deposit should go and how it can be used.

@chadoh
Copy link
Contributor Author

chadoh commented Mar 17, 2021

I think the standard should state something along the lines of:

 // Transfer tokens and call a method on a receiver contract. A successful
 // workflow will end in a success execution outcome to the callback on the same
 // contract at the method `ft_resolve_transfer`.
 //
 // You can think of this as being similar to attaching native NEAR tokens to a
 // function call. It allows you to attach any Fungible Token in a call to a
 // receiver contract.
 //
 // Requirements:
-// * Caller of the method must attach a deposit of 1 yoctoⓃ for security
-//   purposes
+// * Caller of the method must attach a deposit of at least 1 yoctoⓃ for security
+//   purposes. If more than 1 yoctoⓃ is attached, the full deposit must be
+//   passed along to `ft_on_transfer` on the receiving contract.
 // * Caller must have greater than or equal to the `amount` being requested
 // * The receiving contract must implement `ft_on_transfer` according to the
 //   standard. If it does not, FT contract's `ft_resolve_transfer` MUST deal
 //   with the resulting failed cross-contract call and roll back the transfer.
 // * Contract MUST implement the behavior described in `ft_resolve_transfer`
 //
 // Arguments:
 // * `receiver_id`: the valid NEAR account receiving the fungible tokens.
 // * `amount`: the number of tokens to transfer, wrapped in quotes and treated
 //   like a string, although the number will be stored as an unsigned integer
 //   with 128 bits.
 // * `memo` (optional): for use cases that may benefit from indexing or
 //    providing information for a transfer.
 // * `msg`: specifies information needed by the receiving contract in
 //    order to properly handle the transfer. Can indicate both a function to
 //    call and the parameters to pass to that function.
 function ft_transfer_call(
    receiver_id: string,
    amount: string,
    memo: string|null,
    msg: string
 ): Promise {}

@chadoh
Copy link
Contributor Author

chadoh commented Mar 17, 2021

And

 // This function is implemented on the receving contract.
 // As mentioned, the `msg` argument contains information necessary for the receiving contract to know how to process the request. This may include method names and/or arguments. 
 // Returns a value, or a promise which resolves with a value. The value is the
 // number of unused tokens in string form. For instance, if `amount` is 10 but only 9 are
 // needed, it will return "1".
+//
+// If a deposit is attached, contract must use only what it needs and refund
+// excess deposit to `sender_id`.
 function ft_on_transfer(
     sender_id: string,
     amount: string,
     msg: string
 ): string {}

@evgenykuzyakov
Copy link
Contributor

What happens in ft_on_transfer panics? The deposit will be lost on FT contract. If we decided to refund it, then the receiver may trick us to think it panicked, while taking tokens and redirecting to a promise that would panic.

@chadoh
Copy link
Contributor Author

chadoh commented Mar 18, 2021

As an alternative workflow, could the AMM app create a batch request to register the user for Token 2, then call ft_transfer_call on Token 1?

@evgenykuzyakov
Copy link
Contributor

As an alternative workflow, could the AMM app create a batch request to register the user for Token 2, then call ft_transfer_call on Token 1?

The execution order is not guaranteed

@chadoh chadoh closed this Mar 31, 2021
@chadoh chadoh deleted the fix/ft/transfer-call-deposit branch March 31, 2021 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants