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

[WIP] Safe cancel #71

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

[WIP] Safe cancel #71

wants to merge 5 commits into from

Conversation

tromp
Copy link
Contributor

@tromp tromp commented Oct 11, 2020

Cancel safely in face of play attacks.

Link to rendered text

@pkariz
Copy link

pkariz commented Oct 11, 2020

Just want to mention that respend doesn't help if you reinstall the wallet before sending the next transaction because the new wallet has no clue about which utxos were marked for respend. That's also true if you use multiple devices since we don't have a shared wallet data

@tromp
Copy link
Contributor Author

tromp commented Oct 11, 2020

That's a separate issue. A restored wallet doesn't know what transactions are pending, and will count all their inputs towards its balance. This RFC doesn't deal with that general problem.

## Motivation
[motivation]: #motivation

A wallet cannot simply cancel a pending transaction bu forgetting about it and returning its inputs to the wallet balance.
Copy link
Member

Choose a reason for hiding this comment

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

typo -> bu

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link

Choose a reason for hiding this comment

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

Linked text doesn't show the fixed version

@lehnberg lehnberg added the wallet dev Related to wallet dev team label Oct 12, 2020
@tromp tromp marked this pull request as draft October 13, 2020 21:29
## Motivation
[motivation]: #motivation

A wallet cannot simply cancel a pending transaction but forgetting about it and returning its inputs to the wallet balance.
Copy link

Choose a reason for hiding this comment

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

Typo but --> by

@pkariz
Copy link

pkariz commented Mar 16, 2021

So the safe-cancel should occur in these cases:

  1. SRS: sender has signed the transaction and broadcasted it, but now wants to cancel it (reason being it was blocked by the receiver)
  2. RSR: sender has signed the transaction and sent the slatepack to the receiver, but now wants to cancel it (cases described in the rfc)

should self-spend be a 1-1 or a 1-2 transaction?
1-1 consequences:

  • everyone knows it's 99% a self-spend
  • Bob is 99% this is Alice's self-spend so he knows her output
  • less fees than a 1-2 transaction

1-2 consequences:

  • nobody knows it's a self-spend, only Bob can speculate it is (could be a respend so he can't be sure)
  • a bit higher fees than a 1-1 transaction

@tromp
Copy link
Contributor Author

tromp commented Mar 16, 2021

A cancel can be done on any pending transaction.
That is, any transaction that the wallet has signed for,
and that hasn't appeared in the mempool yet of the node queried by the wallet.

In the case of SRS, where sender has signed the transaction and broadcasted it, but it was blocked by the receiver, the status should change from "pending" to a new state "blocked by receiver".
The user can then either inform the receiver to please unblock, or determine that the receiver
is a deceiver, and refusing to further deal with them. In the latter case, they should choose to "unspend".

Unspends are always self spends, while respends can be any spend, or even a payjoin receive.
Self spends would not stand out with the CoinSwap proposal at https://forum.grin.mw/t/mimblewimble-coinswap-proposal

@phyro
Copy link
Member

phyro commented Mar 17, 2021

I think it's best to plan this also for the scenario of full blocks. In this case, we'd need to have an option to define the cancel fee to define the priority of a cancel.
I also wonder whether we should differentiate such a self-spend. Should self-spend outputs be labeled somehow in the wallet to distinguish from an actual receiving of money? I think this question is relevant for self-spends in both safe-cancel and the coinswap proposal.

@tromp
Copy link
Contributor Author

tromp commented Mar 21, 2021

What do you mean by a cancel fee? The fee of an unspend tx?
This gets us into the replace-by-fee issue that Bitcoin faced.
Should a higher (priority) fee replace a conflicting tx in the mempool?
That is quite a difficult topic, which I don't want to spend too much time on until
such a time when blocks filling up starts to feel like a remote possibility

I think a self-spend tx could show up in the wallet as a mark/label on an output,
indicating that it might shift a little (change blinding factor and lose a fee in value)
rather than a separate tx.

@phyro
Copy link
Member

phyro commented Mar 23, 2021

When you say mark/label on an output you mean literally on an output (using a new BP bit) or labeling in the wallet context?

@tromp
Copy link
Contributor Author

tromp commented Mar 23, 2021

I mean in the wallet.

will mark the pending transaction, specified either by ID or TxID UUID,
as requiring one of its inputs to be spent in the very next wallet spend.

grin-wallet unspend [OPTIONS]

Choose a reason for hiding this comment

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

Maybe make respend/unspend one sub-command with options to choose the transaction-style + fees. Think this would be clearer to the user, and could even have an interactive UI component explaining the tradeoffs (more of an implementation detail).

Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking about this as well. We will have 3 ways to cancel a transaction "cancel, unspend, respend" which might be confusing to the user. Would it make sense to join these commands e.g. ./grin-wallet cancel -unspend or ./grin-wallet cancel -respend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and grin-wallet cancel -unsafe to reproduce current behaviour?

[unresolved-questions]: #unresolved-questions

## Future possibilities
[future-possibilities]: #future-possibilities

Choose a reason for hiding this comment

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

One possibility that came to mind: mitigate the receiver's ability to block by creating another transaction with the same output. Not sure exactly how this would work, but maybe including in the output some contribution from the sender that can't be included/unlocked by the receiver until the transaction is fully signed.

My understanding of the problem is that the receiver creates/spends a transaction between step 2-3 in SRS flow (1-2 or later in RSR), which spends their output added in step 2. If the sender creates the receiver's output, and includes a contribution (something like a commitment opened at the end of step 3), then the receiver would be unable to create a tx with the duplicate output.

Again, not sure exactly how that would work, since the commitment would need contribution from both sender and receiver, else sender could just steal back the output from the receiver.

What do you think?

Copy link
Member

@phyro phyro Apr 4, 2021

Choose a reason for hiding this comment

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

There are 2 ways to attack as the receiver:

  1. (SRS/RSR) receiver blocks the tx by creating a tx which reuses output created in step 2 in SRS
  2. (RSR) receiver simply does not perform step 3 and can finish it later

If I understand correctly, you're tackling the 1. problem with your idea. I thought about a similar concept a long time ago which I referred to as "unpredictable outputs" where I wanted to randomize the commitments based on the PoW in the block (or block hash) they were included in (you know in which block an output was added so you could get the original commitment back through a simple computation), but this has some obvious downsides:

  1. it only works if the output ends on the chain and doesn't work if it is still in the mempool
  2. changing commitments based on PoW is a really bad idea if a reorg happens because you can't replay any transaction

I think your direction of having the output have this unpredictability at the transaction level does not suffer from these two issues and might solve the blocking tx problem (however, the withholding would still be an issue I believe). But I'm not sure how this would be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that the receiver in MW should have full control over their outputs, which includes the ability to create duplicates. It doesn't seem too difficult to identify the reason why a published tx fails to confirm.

@GeneFerneau
Copy link

After some work on atomic signatures, and a conversation with @phyro on keybase, came up with the idea of using a concurrent height_lock transaction spending the same input (set to some spender chosen threshold). Could be given as an option to the spender at the time of tx creation: "hey, would you like to create a height_lock tx to ensure you get your money back if the receiver pulls any shenanigans?" Then, when/if the receiver acts honestly, the height_lock tx is a double-spend, never to be accepted to the chain.

Thoughts?

@phyro
Copy link
Member

phyro commented Apr 27, 2021

I think it is an interesting idea that deserves to be written down for anyone else reading this discussion in the future. If the height_lock transaction had a 2x fees compared to the average accepted at that time, it might serve as a "contract expiration" transaction so it could be interpreted as the contract might expire after 60 blocks/~1 hour. I'm not sure how such transaction would behave in the mempool and whether they would accept a height locked transaction with the same input though.

@tromp
Copy link
Contributor Author

tromp commented Apr 27, 2021

AFAIK, neither Grin implementation has yet implemented a replace-by-fee policy, so extra fees on a refund tx (the idea of which dates back many years) don't matter much.

@phyro
Copy link
Member

phyro commented May 8, 2021

One possible issue is the following scenario. Suppose Alice sends Bob 100k Grin in RSR. Bob at step 3 withholds the transaction and says it doesn't work. Alice performs the safe-cancel and sees the tx on the chain. Now they create another transaction and when Bob gets to step 3, he reorgs the chain to remove the self-spend from Alice and include both of the transactions to double-spend Alice. This could be solved by using re-spend option, but users need to understand that unspend comes with the regular confirmation depending on the value.

@phyro
Copy link
Member

phyro commented May 26, 2021

What does transaction cancel mean it is done by Bob (the receiver) due to Alice (the sender) not finishing the steps? A scenario would be SRS flow where step3 is not done. There are two transaction cases I see:

  1. Bob only contributed outputs to the tx
  2. Bob contributed outputs and inputs to the tx (payjoin)

Handling 1.: The only way to really cancel the transaction from Bob side where he contributed only outputs is to create the same output in a self-spend and never spend it, but we really don't want this. So I assume we handle this by dropping the expected outputs that the wallet is tracking or similar. This is probably an implementation question, but what happens if you remove the expected outputs and the tx later appears on the chain?

Handling 2.: This is a more interesting scenario. When Bob cancels a payjoin transaction, does it mean that Bob double-spends the contributed inputs (at least one of them)?

@tromp
Copy link
Contributor Author

tromp commented May 26, 2021

In case 1, I don't see any need for Bob to cancel the tx, since no funds of his are at stake.
As to implementation, it should probably remain to show as unconfirmed output.

In case 2, the funds in his input are still considered part of his balance, and again he is not at risk of losing funds. The only risk is this output getting doublespent when Bob cancels with a respend
and reused the input in another tx, which Bob can avoid by doing an unspend instead. The difference with cancelling a sending tx is that Bob can delay the cancellation of a payjoin receive until he's running out of other inputs to use.

@phyro
Copy link
Member

phyro commented Dec 17, 2021

I think we could make it a 2-step process. The main difference between respend/unspend is the speed at which they execute (fees are much less important imo). Both do the exact same thing. They first mark a transaction for cancellation and at some point in the future, a transaction we create consumes an input of this transaction. We can thus simplify these to:

  1. Have cancel_pool which is a pool of transactions marked for cancellation
  2. The next transaction we create where we are the sender simply iterates over cancel_pool, takes the first input we contributed and adds it to the transaction

This means that safe-cancel can be just ./grin-wallet cancel --mark-respend which would put it in the cancel_pool and then have ./grin-wallet self-spend command which creates a transaction to self and since every transaction where we're the sender consumes the cancel_pool it would cancel them.

Separating marking from the respend itself might also be a better option for services consuming this API because you don't need a separate transaction for every cancelled transaction. You can mark multiple transaction for respending and then create a self-spend to consume all of them at once.

Thus a fast respend is just a mark + self-spend e.g. ./grin-wallet cancel --mark-respend txid && ./grin-wallet self-spend 0 where 0 means that we want to send 0 amount (along with the mandatory inputs picked from the cancel_pool).

Edit: Hmm, while this would allow to batch cancel transactions in a single transaction, it might be a bit more complex handling a failure of this respend e.g. one of the cancel transactions actually lands on the chain before it is cancelled.

@pkariz
Copy link

pkariz commented Dec 17, 2021

Cancel pools are a nice idea, but when you run self-spend you would leak some of your tx graph connections if you self-spend all marked txs in a single self-spend and the implementation is, as you noted, a little complicated when one of the canceled txs lands on the chain. I'm not sure I'm convinced respend is necessary at this point, here are the cons I see:

  • it complicates cancel explanation (unspend is very simple to explain if we go with contract/sign/sign flow explanation since it's just unsign and you need to pay for signature removal, respend requires some tech understanding which 99% of the users won't have)
  • it complicates implementation (you mark tx for respend, maybe you also want to allow unmarking, ux becomes more complicated)
  • when a user does unspend or respend he agrees that this tx should be canceled and respend doesn't guarantee you that since it waits for the user to create another tx (or multiple if you want to leak less data and respend each canceled tx separately). Before the user creates new tx he might buy a new phone and the tx is again in a pending state and not canceled (but the user doesn't see it in the wallet, so it's dangerous and would need to self-spend everything, that's very non-intuitive to non-tech users). With unspend the user needs to have the wallet installed only for the next minute or two until tx is mined.

respend is of course useful in some cases, but imo it only really makes a meaningful difference if a single user does many respends and exchanges are candidates for doing that. However exchanges have an easy way to deal with this, they can just do unspend and reduce user's amount in their system. So maybe it might be best to just implement grin-wallet cancel with optional --unsafe flag to avoid creating a self-spend tx and if it later turns out that the community wants to have an option to respend it can be added later. Note that grin-wallet cancel should work also on step1 txs since they're already in db, even if there are no inputs commited yet (it should basically do the "unsafe" variant which is safe in this case)

@phyro
Copy link
Member

phyro commented Dec 17, 2021

Thanks for the comments and you have some good points, especially that it may complicate the handling when one of the transactions appears on the chain (though I'm not yet sure it does). However, observing this self-spend transaction and handling possible cases is unavoidable, even in the case of unspend because like respend it doesn't guarantee the transaction will not be race-conditioned on the chain. So the wallet would need to observe which of the two double-spend transactions actually landed on the chain to clean up the input/output state (drop the outputs created in the transaction that didn't arrive and unlock the inputs that have not been spent).
What would --unsafe do in this case? mark the transaction for spending the output in the next transaction? and since you want each cancel to be done separately then we'd have a queue/sequence of cancel attempts, correct?

@pkariz
Copy link

pkariz commented Dec 17, 2021

even in the case of unspend because like respend it doesn't guarantee the transaction will not be race-conditioned on the chain

yes, it would show as "cancelling", but in case of unspend you know that this cancelling is short-lived. It can turn into "completed" though yes if the original tx gets mined.

What would --unsafe do in this case?

only set status in db as "canceled" and unlock any locked inputs, same as cancel does today

and since you want each cancel to be done separately then we'd have a queue/sequence of cancel attempts, correct?

for each tx you cancel new "canceling tx" is created. I'm not sure if the user should see these "cancel-txs" or not, probably not.

@Anynomouss
Copy link

Anynomouss commented Jan 28, 2022

for each tx you cancel new "canceling tx" is created. I'm not sure if the user should see these "cancel-txs" or not, probably not.

The best would be to show the original transaction, labelled as 'cancelled safely', or something similar. So basically you combine the safe cancelling transaction with the original, at least in the way that it is presented to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wallet dev Related to wallet dev team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants