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

NEP-366: Meta transactions #366

Merged
merged 9 commits into from
Nov 24, 2022
Merged

NEP-366: Meta transactions #366

merged 9 commits into from
Nov 24, 2022

Conversation

ilblackdragon
Copy link
Member

Summary

In-protocol meta transactions allowing for third-party account to initiate and pay transaction fees on behalf of the account.

Motivation

NEAR has been designed with simplify of onboarding in mind. One of the large hurdles right now is that after creating an implicit or even named account user doesn't have NEAR to pay gas fees to interact with apps.

For example, apps that pay user for doing work (like NEARCrowd or Sweatcoin) or free-to-play games.

Aurora Plus has shown viability of the relayers that can offer some number of free transactions and a subscription model. Shifting the complexity of dealing with fees to the infrastructure from the user space.

Rationale and alternatives

Proposed here design provides the easiest for users and developers way to onboard and pay for user transactions.
There is no requirement on the user account, including user account may not even exist on chain and implicit account can be used.

An alterantive is a proxy contracts deployed on the user account.
This design has severe limitations as it requires user to deploy such contract and additional costs for storage.

Specification

Next changes to protocol are required:

  • New Action kind: DelegateAction(receiver_id, Action, signer_pk, signature)
  • On conversion to Receipt, such action is sent as DelegateAction to the receiver_id.
  • On processing Receipt, it verifies the signature over the given account and signer_pk. Unwraps into receiver_id: AccountId, action: Action inside into new Receipt with given receiver_id and action. This means that predeccessor_id of such action has been overriden.

See the DelegateAction specification for details.

Security Implications

Delegate actions do not override signer_pk, leaving that to the original signer that initiated transaction (eg relayer in meta transaction case). Although it is possible to override signer_pk in the context with one from DelegateAction, there is no clear value to do that.

See Validation section in DelegateAction specification for security considerations around what user signs and validation of actions with different permissions.

Drawbacks

Increases complexity of the NEAR's transactional model.

Meta transactions take an extra block to execute, as they first need to be included by the originating account, then routed to the delegate account and only after that to the real destination.

Delegate actions are not programmable as they require having signatures. Original proposal contained a new AccessKey kind that would support programmable delegated actions. On the other hand, that would be limiting without progamability of smart contracts, hence that idea evolved into Account Extensions.

Future possibilities

Supporting ZK proofs instead of just signatures can allow for anonymous transactions, which pay fees to relayers in anonymous way.

Copyright

Copyright and related rights waived via CC0.

@ilblackdragon ilblackdragon requested a review from a team as a code owner June 23, 2022 20:00
@render
Copy link

render bot commented Jun 23, 2022

Copy link

@ebramanti ebramanti left a comment

Choose a reason for hiding this comment

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

Love this proposal! Wanted to suggest some small spelling/grammar changes, but overall looks like a huge UX improvement for new users

neps/nep-0366.md Outdated Show resolved Hide resolved
neps/nep-0366.md Outdated Show resolved Hide resolved
neps/nep-0366.md Outdated Show resolved Hide resolved
neps/nep-0366.md Outdated Show resolved Hide resolved
neps/nep-0366.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Meta comment (no pun intended): we should separate proposal from modification to the spec, i.e, the proposal strictly follows https://github.com/near/NEPs/blob/master/nep-0000-template.md and the spec is updated in a separate PR if the proposal is accepted and implemented. This way it makes the proposal more self-contained and easier to review.

neps/nep-0366.md Outdated Show resolved Hide resolved
neps/nep-0366.md Outdated Show resolved Hide resolved
@ilblackdragon ilblackdragon changed the title Meta transactions NEP-366: Meta transactions Jun 24, 2022
@MaksymZavershynskyi
Copy link
Contributor

The proposal leaves some things out and is a bit ambiguous, especially given that DelegateAction specification is a broken link. The following two things would significantly improve my understanding:

  • A sequence diagram that includes the construction of meta transaction off-chain;
  • Examples of Transaction and Receipt data structures involved;

The following things contribute to my confusion:

On conversion to Receipt, such action is sent as DelegateAction to the receiver_id. On processing Receipt, it verifies the signature over the given account and signer_pk

So DelegateAction is sent to receiver_id and it is "the given account" that has signer_pk public key. If yes, then I suggest we call it DelegateAction::signer_id, because in Transaction data structure receiver_id denotes the account that the receipt will be sent to after the signature is verified, not before.

Delegate actions do not override signer_pk, leaving that to the original signer that initiated transaction (eg relayer in meta transaction case).

So the signer is not the author of the transaction but the relayer? I suggest we find a different naming here, since signer_pk can refer to the pk of the relayer (Transaction::public_key and DelegateAction::signer_pk), it will be unclear what signer_account_pk host function returns in the contract that is hosted on the account that receives the original transaction from the relayer.

neps/nep-0366.md Outdated
The main flow of the meta transaction will be as follows:
- User will sign `DelegateActionMessage` specifying set of actions that they need to be executed. It also includes specific relayer to ensure secure execution.
- User sends signed `DelegateAction` data to the relayer
- Relayer forms a `Transaction` with `receiver_id` equal to the user's account and `actions: [DelegateAction { ... }]`, and signs it with it's key. Note, that such transaction can contain other actions toward user's account (for example calling a function).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to clarify, do you mean here that the protocol specifies that for meta transactions, the actions field is a singleton that consists of one DelegateAction? In other words, if a transaction contains more than one action and one of them is DelegateAction, then this transaction is invalid

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if that was the goal, but I don't think having this as a singleton is mandatory.

neps/nep-0366.md Outdated
Comment on lines 114 to 115
/// Nonce must be account[signer_pk].nonce + 1
DelegateActionInvalidNonce
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we enforce +1 here? That is not how access key works in other cases

Copy link
Member

@mfornet mfornet Sep 28, 2022

Choose a reason for hiding this comment

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

@bowenwang1996 Can you share a reference about how nonces work today? I can't find it in nomicon.

@waldmatias
Copy link
Contributor

As this was discussed today during the Protocol meeting, I wonder if this is still relevant or has it been moved/migrated to a different place? (in accordance with the new NEP process?)

e-uleyskiy pushed a commit to binary-star-near/nearcore that referenced this pull request Aug 29, 2022
This is a draft implementation of NEP-366 (near/NEPs#366)

Not done yet:
* Need to implement DelegateAction for implicit accounts (nonce problem)
* Need new error codes for DelegateAction
* Implement Fees for DelegateAction
* Add tests
e-uleyskiy pushed a commit to binary-star-near/nearcore that referenced this pull request Aug 29, 2022
This is a draft implementation of NEP-366 (near/NEPs#366)

Not done yet:
* Need to implement DelegateAction for implicit accounts (nonce problem)
* Need new error codes for DelegateAction
* Implement Fees for DelegateAction
* Add tests
@fadeevab
Copy link
Contributor

We've created a draft reference implementation of meta transactions here: near/nearcore#7497 (WIP)

@frol frol added T-runtime About NEAR Protocol Runtime (actions, Wasm, fees accounting) WG-protocol Protocol Standards Work Group should be accountable labels Sep 5, 2022
neps/nep-0366.md Outdated
- This transaction is processed normally, by creating a receipt with copy of the all the actions into the `Receipt`.
- When processing `DelegateAction` a number of checks are done (see below), mainly `signature` matching user account's key.
- When such `Receipt` with valid `DelegateAction` in actions arrives to the user's account it gets executed. The executed means creation of a new Receipt with `receiver_id: AccountId`, `actions: Action` matching `receiver_id` and `actions` inside `DelegateAction`.
- The new `Receipt` looks like normal receipt that would have been originating from user's account, with `predeccessor_id` equal to user's account.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unclear what signer_id should be equal to. According to the discussion in near/nearcore#7497 it should be equal to Relayer's account.

Copy link
Member

Choose a reason for hiding this comment

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

Please note that using signer_id should be considered unsafe most of the time. See this comment for the equivalent in Ethereum: ethereum/solidity#683 (comment).


Having said that I think signer_id should be the account that is paying for the attached balance (not gas), and if I understand correctly it is not the relayer, but the user signing the transaction.

neps/nep-0366.md Outdated

The main flow of the meta transaction will be as follows:
- User will sign `DelegateActionMessage` specifying set of actions that they need to be executed. It also includes specific relayer to ensure secure execution.
- User sends signed `DelegateAction` data to the relayer
Copy link
Contributor

Choose a reason for hiding this comment

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

Does a user send DelegateAction data to the relayer out off-chain?

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the advantage of having two structures: DelegateActionMessage and DelegateAction? A user can send DelegateActionMessage to the relayer and the realyer can add DelegateActionMessage to the transaction as an action. I think, this is simpler.

Copy link

Choose a reason for hiding this comment

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

+1 for explaining more clearly the role of the two messages.

Copy link

Choose a reason for hiding this comment

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

also - does user send a DelegateAction or DelegateActionMessage (based on context, I think it is the latter - as this is the one that was created in the previous point)

@e-uleyskiy
Copy link
Contributor

e-uleyskiy commented Sep 7, 2022

This NEP doesn't cover an implicit account. It would be good if an implicit account could use a delegate action.
There is the following issue: an implicit account doesn't have an access key in storage (because doesn't have storage) therefore there is not a way to verify nonce. To solve this issue, a relayer can transfer some NEAR to the implicit account.
How to make sure that a user will pay the rewards to the relayer? My idea was: a relayer could add a "transfer" action to the transaction right before the delegate action. Creation an account by "transfer" can't be used in conjunction with other actions therefore this requires changing "transfer" action code in nearcore.
Do you have other ideas?

The discussion was started in near/nearcore#7497.

/// Receiver of the delegated actions.
receiver_id: AccountId,
/// List of actions to be executed.
actions: Vec<Action>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Vec<Action> here leads to a type recursion:

pub struct Transaction {
   ....
   pub actions: Vec<Action>, // <--- Recursion
}

pub enum Action {
   ...
   Delegate(DelegateAction),
}

pub struct DelegateAction {
   ...
   pub actions: Vec<Action>, /// <--- Recursion
}

Copy link
Member

Choose a reason for hiding this comment

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

It is acceptable in this context because an Action can't refer to itself or an ancestor, so recursion is finite.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant, rust compiler would not compile such code because of the recursion.
Example code in playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a6c54d6ed7588eb03d2ad19da1c0a9c6

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, a common way to prevent this recursion is to use a Box. As an example, a singly-linked list in Rust can be represented as:

struct Node<T> {
    element: T,
    next: Option<Box<Node<T>>>,
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=14f24298eee755a2a08c7b5b66d8b4fa suggests a fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right. When I wrote my comment, I didn't know that Borsh supported Box. But there is another issue with this structure: Borsh doesn't support a recursive types near/borsh-rs#96

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed in the video call, for the purposes of the NEP, the recursive data structure is fine. For nearcore, we will have to come up with a workaround. Best to discuss on Zulip.

@ori-near ori-near added the S-voting/needs-wg-voting-indication A NEP in the VOTING stage that needs the working group voting indication. label Sep 22, 2022
@ori-near
Copy link
Contributor

ori-near commented Sep 22, 2022

@bowenwang1996 @mm-near @mfornet – We moved this NEP to the Review stage. Could you please assign at least 2 SME Reviewers to review the NEP within one week? Per our NEP process, the reviewers will review the technical details of the proposal and assess its merits. They may ask clarification questions, request changes, or reject the proposal.

@near near deleted a comment from marmichalski Sep 22, 2022
@fadeevab
Copy link
Contributor

@ori-near @mm-near @mfornet @bowenwang1996 Hi! Please, consider, that the practical PoC of the NEP was introduced here: near/nearcore#7497. The comments of @e-uleyskiy in here are based on the issues uncovered during the practical implementation. There are few improvements yet to get implemented which we are working on.

Please let us know in case of kicking off any parallel development activities and in case of any updates. Thank you!

@fadeevab
Copy link
Contributor

I'd like to add more context to the discussion.

Fungible tokens (FT) are smart contracts, it's essentially an opaque application layer (opaque from the blockchain layer perspective), and it means that there is no way the blockchain can guarantee that the Relayer gets its reward with FT.

What blockchain can guarantee:

  1. User account (Alice) is a real account on the blockchain.
  2. That Relayer has an ability to pay for gas for an Alice's delegated action.
  3. Rolling back of all (function call) actions inside of the delegated action.

Everything else should be done on the "application" level, and some security verification pieces lay on the Relayer.

Here are 2 practical use cases which are feasible considering this NEP implementation:

UC 1: Transferring FT

META TX  Relayer Use Case #1

UC 2: Calling a DApp (smart contract)

META TX  Relayer Use Case #2


Bottom line

Relayer should have trust in the target smart contract using out-of-bound (off-chain) mechanism of establishing such trust to some extent.

  1. Whitelisting the target smart contract.
  2. Double checking the contents of the delegated action (e.g. that there is a reward function call).

At the same time, with Meta Transaction, Relayer doesn't need to have a trust to Alice (the user):

  1. No need to fund Alice's account with NEAR.
  2. No need to verify existence of Alice account.
  3. The failed transaction is being rolled back.

@jakmeier
Copy link
Contributor

@fadeevab thanks for the clear presentation of two possible use cases! That's exactly what we need to bring everyone on the same page about what is possible and what is not possible with the current proposal.

What blockchain can guarantee:
...
...
Rolling back of all (function call) actions inside of the delegated action.

Can you elaborate more on that? What would be the protocol's role here? What would happen on the application layer?

@frol
Copy link
Collaborator

frol commented Nov 22, 2022

But I am a bit afraid of the can of worms we open when we "just make it a list of delegate actions". If the expectation is that the relayer has some guarantees of getting paid, then we would have to add dependencies between the actions. I think we could implement that using data receipts but that's where discussions become more complex.

Agree. @fadeevab and I discussed this a lot some time ago. Yes, we can implement this with "data dependencies" but it becomes much complex. Moreover, sometimes the nearcore code relies on the assumption that such receipts are used only by contracts. Therefore such change might affect contracts.

The another issue is "how to guarantee the relayer got paid?". If ft_trasnfer doesn't fail, it doesn't guarantee that the relayer will get payment because this depends on ft_transfer implementation in the contract. I think, the only way to guarantee this for FT, is the relayer's contract should submit the DelegateAction from ft_on_transfer callback. But this approach requires the contracts to be able to submit DelegateAction (need to export this feature to contract API). In this NEP we assume, that the relayer takes the risk.

Ideally, all such considerations should be included in the "Drawback" section, so working group members don't suddenly rediscover them and start going in the loops. I think the main concern that blocked united voting for approval was the fact that it needs some time to think through the pros and cons of approving the NEP as-is. We are not living in an ideal world, so having NEPs with drawbacks is totally normal as long as it does not introduce a security flaw.

Personally, I believe that if Decentral Bank and Aurora will benefit from having this NEP as-is, it is already a huge win and the only question is "is there a quick fix that would unlock even more use cases?" It sounds like you guys have already thought about it and there is no quick fix, and if that is the case, it sounded to me that the protocol working group is ready to approve this NEP as-is (@mfornet @bowenwang1996 @mm-near correct me if I am wrong 🙏). @e-uleyskiy @fadeevab Please, include all the known drawbacks in the NEP itself.

At the same time, with Meta Transaction, Relayer doesn't need to have a trust to Alice (the user)

I believe that the main concern was around the use case when the relayer expects on-chain payment per transaction and the trust issue is on both sides: Alice doesn't want to pay ahead of time, Relayer does not want to be abused. Thinking about it now (again, there was no time to properly weigh this concern during the call), I don't think it is the main use case that Meta Transactions feature is trying to address, as it is clearly stated in the motivation section of the NEP:

NEAR has been designed with simplicity of onboarding in mind. One of the large hurdles right now is that after creating an implicit or even named account the user does not have NEAR to pay gas fees to interact with apps.

For example, apps that pay user for doing work (like NEARCrowd or Sweatcoin) or free-to-play games.

Aurora Plus has shown viability of the relayers that can offer some number of free transactions and a subscription model. Shifting the complexity of dealing with fees to the infrastructure from the user space.

@e-uleyskiy
Copy link
Contributor

@jakmeier

Can you elaborate more on that? What would be the protocol's role here? What would happen on the application layer?

I don't know nearcore code deeply. Below is my understanding.

Near protocol guarantee that the only actions are executed in the current block are rolled back. So if the called contract function doesn't contain cross-contract calls and it fails, then the protocol guarantee this FunctionCall and all previous executed actions will be rolled back. But if the contract triggers a cross-contract call, this cross-contract call is scheduled to another block therefore such FunctionCall can't be rolled back by the protocol.

In other words, if DelegateAction calls ft_transfer and ft_transfer has failed (caused by panic or other execution errors) and hasn't triggered cross-contract calls, the protocol will roll-back it. But if ft_transfer has called withdraw method (for example) from another contract and withdraw method has failed, the protocol can't roll-back ft_transfer.

@jakmeier
Copy link
Contributor

Yes, you got that exactly right @e-uleyskiy . Near Protocol can only rollback actions within the same receipt, we cannot rollback on a transaction level. (I find the term transaction particularly confusing in this context but what I mean is the transitive set of receipts caused by a Transaction accepted by the chain.)

In the following part it wasn't 100% clear to me you were talking about single receipt rollback only.

At the same time, with Meta Transaction, Relayer doesn't need to have a trust to Alice (the user):

No need to fund Alice's account with NEAR.
No need to verify existence of Alice account.
The failed transaction is being rolled back.

Some people will read "Meta Transaction" and think of "A pays for a transaction on B sent by C" regardless of what you just wrote aboe that quote. The important part to get across here is that the rollback you refer to only works with a single delegate action and would be broken if we make it a list. Otheriwse it will be two different receipt, hence not atomic.

And to be clear, @fadeevab you have already written this correctly and well articulated. I just double down on this to make sure we don't have another misunderstanding of what a "Meta Transaction" is exactly. The final NEP to vote on after the delay should also be as unambiguous about this as possible.

@mfornet
Copy link
Member

mfornet commented Nov 22, 2022

The main reason we didn't approve this NEP during the call was due to the difficulty of the following use case:

The relayer will execute a transaction on behalf of the user, covering the NEAR gas, but it needs to make sure it will be paid in the same transaction (potentially in the form of an FT). Guaranteeing that the payment will go through during the same transaction at the protocol level is hard due to the async nature of NEAR + FT being part of an opaque application layer.

After thinking more about this, I have the impression we can have this NEP as is and try to solve the previous problem at the application level.

There will be several kinds of relayers with different tradeoffs. The first few relayers I foresee are centralized-like:

  • Cover the whole gas of the tx without expecting any payback, allowing only transactions of a certain form.
  • Similar to the previous one, the payment is done in advance in a separate channel. Either on-chain or off-chain. This is the model used by Aurora+.

Building decentralized relayers should be possible using two transactions (this was proposed in the call). One is where the user sends tokens to the relayer, and the other is with the actual transaction. To make this work in a trustless setup, the transactions where the relayer claims the tokens (execute the transfer) should have as input proof that the main transaction was executed (this can be done naively using Merkle proofs, but potentially it can be done in a simpler way if the protocol is modified appropriately). Btw, I think there are even more solutions to this problem, for example, using HTLC.

@e-uleyskiy
Copy link
Contributor

If we allow contracts to submit SignedDelegateAction, then we will be able to solve mentioned issue (payment issue) on application level. The User can pay Relayer with ft_transfer_call (using DelegateActtion) and attach another DelegateAction as msg argument. After that Relayer's contract will receive ft_on_transfer callback with msg argument and can submit the DelegateAction.

Something like the following:
Untitled Diagram
Note, in this approach Relayer executes User's actions (submits AnotherDelegateAction) after getting payment.
Moreover, this approach covers the use case when another user can pay rewards for Alice.

@frol
I will add some Drawbacks to the NEP. And we will post here the list of security concerns we had when modified this NEP.

@fadeevab
Copy link
Contributor

fadeevab commented Nov 23, 2022

@jakmeier @mfornet @frol and others

Security Concern Exploitation Scenario Countermeasure
Untrusted Relayer If the User uses the same key for different accounts, the Relayer can submit DelegateAction for both accounts A signed DelegateAction contains sender_id: delegate_action.sender_id == tx.receiver_id, which converts into predecessor_id on the contract level.
Untrusted Relayer: Replay Attack Relayer can submit DelegateAction twice. Added nonce: delegate_action.nonce > user_key.nonce. If it’s ok, user_key.nonce = delegate_action.nonce
Untrusted Relayer Relayer can delay submitting DelegateAction Added max_block_height. current_height <= max_block_height
Untrusted Relayer Relayer might not submit the DelegateAction but send an error response to User, If User sends DelegateAction again, Relayer can submit both of DelegateAction (old and new)
  1. User must not trust Relayer’s response and should check execution errors in Blockchain.
  2. If it’s impossible (because DelegateAction had incorrect format and NEAR Protocol hasn’t recorded it to the Blockchain) then User must not submit another DelegateAction until the previous one has not expired (see max_block_height).
Untrusted User: No Reward User can send an incorrect DelegateAction (for example incorrect nonce) and Relayer will burn its money. The Relayer must verify the most of parameters before submitting DelegateAction, making sure that one of the function calls is the reward action. Either way, this is a risk for Relayer in general.
Untrusted User: Burning Relayer's Money User can cancel DelegateAction after sending it to the Relayer. For the cancellation, User should submit another DelegateAction or Transaction with the greater nonce. The user key should be the same.

Relayer’s security concern is: \ User can make DelegateAction incorrect even after sending it to Relayer. Relayer will burn its money.

Relayer should take this risk in general. But it can verify the most of parameters before submitting DelegateAction, making sure that one of the function calls is the reward action.
Untrusted FT contract An FT contract can return ok for ft_transfer but doesn’t transfer tokens. This might happen because the contract was replaced by an attacker.

Then Relayer doesn’t receive FT but pays for executing DelegateAction

Relayer should take this risk in general. This can’t be solved on the protocol level.

Relayer should whitelist trusted FT contracts.

These features are not implemented but there are the following security concerns
Untrusted Relayer The list of DelegateAction, a nested DelegateAction.

Because all DelegateActions are signed, the Relayer can submit them separately or change the order.

There are some limits like the maximum number of actions in the Transaction. Such limits should be shared between all DelegateAction in the transaction. Otherwise, Relayer can create a transaction with numerous actions.

Untrusted User. Implicit account The problem:

DelegateAction requires nonce which is stored in the user's access key. The implicit account doesn’t have stored access keys. So, before execution DelegateAction, it needs to create the account, transfer some NEAR to this account and add the access key. And Relayer should receive rewards for this. This will require two receipts because there are two receivers: User (account creation, transferring NEAR) and Relayer (transferring reward). And the account creation should be the first because otherwise, the Relayer can implement the reply attack.

User can create such reward action which always fails. Account creation will not be rolled-back because it was done in another receipt. After that, it can delete the created account and steal NEAR tokens.

There are two possible solutions:

One,

Two

@mm-near
Copy link

mm-near commented Nov 23, 2022

@fadeevab - thanks a lot for your explanations above.
Especially the "UC 2: Calling a DApp (smart contract)" was very informative (as in all the previous examples, I've seen only the UC1 case - and that's what originally got me worried).

With this knowledge, I'm ready to approve this NEP.

@e-uleyskiy
Copy link
Contributor

I've updated drawbacks: #437

Co-authored-by: Egor Uleyskiy <eu@nearua.com>
@bowenwang1996
Copy link
Collaborator

@fadeevab @e-uleyskiy thanks for the detailed explanations and diagrams! I am ready to approve the NEP as is.

@ilblackdragon
Copy link
Member Author

ilblackdragon commented Nov 23, 2022

Just to complete the discussion from the call: Am I correctly assuming that we don't want to turn DelegateAction to support (receiver_id, actions) due to increase in complexity that will not address the problem at hand? And indeed 2 DelegateAction can be submitted in a single transaction by relayer that would work in similar way as having a list of (receiver_id, actions) in DelegateAction as there is no atomiticty there.

I also would like to address the business concern of "relayer doesn't trust user and user doesn't trust relayer" -- here I think need in user trust is not business critical as they are selecting relayer, paying small amounts and can stop using given relayer if it doesn't really perform the required job. So getting user to pay in a separate action that is non atomic with the action they want to execute would be a normal business practice. Similarly how @mfornet mentioned that other paths of payment can be establish like pre-payment or subscription.

If we allow contracts to submit SignedDelegateAction, then we will be able to solve mentioned issue (payment issue) on application level. The User can pay Relayer with ft_transfer_call (using DelegateActtion) and attach another DelegateAction as msg argument. After that Relayer's contract will receive ft_on_transfer callback with msg argument and can submit the DelegateAction.

@e-uleyskiy This was part of original intention to allow contracts to initiate DelegateActions as this opens up for other use cases like cron jobs: user pre-signs transaction to be executed by specific contract and records it there; when some condition is hit and contract has been activated this DelegateAction gets executed.

Indeed this can be used in case of pay -> call back to relayer to than execute the DelegateAction with main action.
One potential issue here is gas management, but given its linear path it should be manageable.

If we allow contracts to submit SignedDelegateAction

This NEP doesn't include a precompile to initiate DelegateAction out of smart contract? Should be done in a separate NEP?

PS. I would add that if we had FT standard that stored amounts on the user account (e.g. account extensions/global contract storage/user storage line of thinking) it would allow to do payment and further execution in atomic action. But that is the story for another day.

@bowenwang1996
Copy link
Collaborator

Just to complete the discussion from the call: Am I correctly assuming that we don't want to turn DelegateAction to support (receiver_id, actions) due to increase in complexity that will not address the problem at hand? And indeed 2 DelegateAction can be submitted in a single transaction by relayer that would work in similar way as having a list of (receiver_id, actions) in DelegateAction as there is no atomiticty there.

Yes

@ori-near
Copy link
Contributor

ori-near commented Nov 24, 2022

As the NEP moderator, I would like to give a huge thank you to the original author @ilblackdragon for submitting this NEP and to the champions @e-uleyskiy @fadeevab who worked so hard to move it forward and address the concerns. Thank you also to the Subject Matter Experts (@jakmeier @akhi3030) and Protocol Working Group members (@bowenwang1996, @mfornet, @mm-near, @k3y0k3y) for all your reviews.

Based on the voting above, the Protocol Working Group members reached following consensus:

Status: Approved

Next Steps:

@ori-near ori-near merged commit 343ac92 into master Nov 24, 2022
@ori-near ori-near deleted the meta-tx branch November 24, 2022 00:23
@ori-near ori-near added S-approved A NEP that was approved by a working group. and removed S-voting/final-comment-period A NEP in the final REVIEW stage. Last chance to bring up concerns before the voting call. labels Nov 24, 2022
@e-uleyskiy
Copy link
Contributor

@ilblackdragon

This NEP doesn't include a precompile to initiate DelegateAction out of smart contract? Should be done in a separate NEP?

Yes, should be done in a separate NEP. I think it would be a good feature because then:

  1. It would simplify UC#2
  2. Cover the use case where Alice doesn't have FT for paying Relayer, But someone else have FT and wants to pay Relayer for Alice.

- If the list of Transaction actions contains several `DelegateAction`
```rust
/// There should be the only one DelegateAction
DelegateActionMustBeOnlyOne
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why this is the case? I don't see the reasoning in this doc; sorry if I skimmed over it. This seems like a benefit to have to be able to schedule multiple meta transactions within a single transaction, to minimize costs for the relayer.

If you actually want this to be the only thing executed within the transaction, should regular actions not be allowed with a delegate action? Currently, the draft implementation seems to allow other actions with a delegate action, and I just want to make sure this is intended

Copy link
Contributor

@e-uleyskiy e-uleyskiy Jan 12, 2023

Choose a reason for hiding this comment

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

@austinabell
I added this case for two reasons:

  1. At this moment all actions in the transaction are executed in one by one. But in case of DelegateAction, they will be executed in parallel (inner actions will be executed in parallel). I think such behavior might be confusing for developer/users.
  2. For code simplification. If we allow several DelegateAction in a transaction, we should decide how limits should be handled. For example there is the limit for total number of actions in the transactions. Should it be shared between DelegateAction and the transaction or we need to define a new limit for number of actions in DelegateAction?

@robert-zaremba
Copy link
Contributor

I'm reading about the Meta Transactions. One thing is not clear to me, and is not documented well in the NEP:
Who pays for the deposit (attached NEAR)? I assume relayer should pay for that... but then, why in the Limitations section it is mentioned that the deposit will be burned? Imagine a situation of a new account with 0 NEAR (Alice). She makes a transaction which require 1 NEAR deposit. I don't think it should be burned.

@jakmeier
Copy link
Contributor

jakmeier commented Apr 3, 2023

@robert-zaremba Yeah, the NEP description isn't 100% clear about all details. But it is indeed the relayer who pays the balance fees on top of the gas fees.

The limitation section speaks specifically about the corner case where the user account doesn't exist. That's not an expected workflow, it's an error. It would only happen if you use the wrong account id for the meta transaction, or if you somehow delete your account before the meta transaction has a chance to execute. If that case happens, yes, the attached tokens are burned because implementing it in another way would require larger changes to the protocol (e.g. adding a "relayer_id" field to all receipts).

Imagine a situation of a new account with 0 NEAR (Alice).

If by "new" account mean one that has not been created, yet, then it's specifically forbidden with meta transactions. You or the relayer must create the account first (to initialize the Nonce counter) before you can create a meta transaction.

If it's an account which was created but it just has no balance, nothing will be burned.

And btw, as of right now, you will find the most extensive meta-tx documentation in the nearcore developer guide, you might want to take a look there while other documentation is still written: https://near.github.io/nearcore/architecture/how/meta-tx.html

@Sunilkherde
Copy link

Nice

Copy link

@Wayde31 Wayde31 left a comment

Choose a reason for hiding this comment

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

😎😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NEP A NEAR Enhancement Proposal (NEP). S-approved A NEP that was approved by a working group. T-runtime About NEAR Protocol Runtime (actions, Wasm, fees accounting) WG-protocol Protocol Standards Work Group should be accountable
Projects
Status: APPROVED NEPs
Development

Successfully merging this pull request may close these issues.