extension icon indicating copy to clipboard operation
extension copied to clipboard

Add option to cancel pending transaction

Open 314159265359879 opened this issue 1 year ago • 22 comments

image

Brice: The problem is that its not possible to cancel a transaction. Once it's reached the mempool, a miner could include it in a block. There's no way to prove whether or not the miner received your attempt to cancel it. The better option is to create a new transaction with the same nonce and a higher fee. Then it is expected that the miner will pick up the higher fee transaction instead.

We can make replacing a transaction super easy. For the user it could be much like "delete transaction" By creating a STX transfer of 0.000001 to the burn address with a fee that is default set to 0.000001 STX more than the transaction being replaced. We could call it a cancel request rather than a delete transaction, as we can never be 100% certain the old isn't picked up anyway, the new transaction would have to be propagated through the network before a miner picks it up.

I think it would make it a lot easier for users to deal with troublesome transactions. Because it wouldn't require a user to understand nonces or RBF... it is handled for them.

Lets combine this with always showing the button to increase the fee for a transaction. They should be always visible. Users do not realize the option is there if it is only shown on hover.

This has been theorized and requested many times. It is a great help when an account has questionable transactions if something went wrong. For example a transaction to send 500 STX was send twice instead of once.

Please note that if we make sending STX very strictly #3389 possible based on available balance. I would want a "cancel request" option in case a user still succeeds in creating transactions that will remain pending until dropped. Because the regular flow may then nolonger allow sending an RBF transaction like this to unclog the wallet: This transfer of 1 uSTX and a fee of 0.003001 STX replaces the 0.027 transfer with a fee of 0.003 STX: image

314159265359879 avatar Mar 10 '23 13:03 314159265359879

Dupe of https://github.com/hirosystems/stacks-wallet-web/issues/1834?

markmhendrickson avatar Mar 10 '23 14:03 markmhendrickson

Hi @314159265359879,

I would like to work on adding a "Cancel request" button to delete pending transactions, as mentioned in issue #3406. I have over 2 years of experience in JavaScript, Python, and building scalable systems. I am eager to begin my open source journey by contributing to this project.

Please consider assigning this issue to me, and let me know if there are any additional details or specifications I should be aware of before I start working on it.

Thank you!

Best regards, Muditya Raghav

ghost avatar Apr 11 '23 16:04 ghost

Hi @mudityadev thanks for wanting to pick this up, that is awesome. @markmhx anything to keep in mind and do you know if Michelly had time to look at design for this already. or is the current mockup okay for now?

314159265359879 avatar Apr 13 '23 08:04 314159265359879

Thanks for offering to help here, @mudityadev.

@314159265359879 I presume these are the specific design needs:

  1. Option to select transaction as listed under activity for cancellation (e.g. "Cancel transaction")
  2. Screen for confirming cancellation upon its selection (e.g. "Are you sure you want to cancel this transaction? It will cost N STX in fees")
  3. (Optional) Confirmation screen for cancellation request with link to more information about how cancellations work and the lack of guarantee that it works
  4. (Optional) Special treatment of the cancellation transaction in activity (e.g. "Cancellation request" with indication of just which other transaction is intended for cancellation by it)

Given these pieces, I think it'd be best if @mica000 design this properly before we head into development. I'll get it on the board.

markmhendrickson avatar Apr 17 '23 09:04 markmhendrickson

"I think it would make it a lot easier for users to deal with troublesome transactions.

@314159265359879 We discussed this briefly during the product kick-off meeting yesterday and @kyranjamie was hoping to get some more input here on the particular use cases that this feature would help solve.

Can you provide a short list of the highest frequency issues with transactions that cancelling would resolve?

markmhendrickson avatar Jul 04 '23 11:07 markmhendrickson

I think this is the top 3

  1. User making a mistake and prefer not to have it become part of the immutable history of the Stacks blockchain.
  2. A user wants to send STX/Standard Token they already have a pending transaction for, the effective balance is too low to make the adjustment until they delete the prior transaction.
  3. To delete forever pending transactions (nonce issues/insufficient STX to afford fees, etc).

314159265359879 avatar Jul 04 '23 20:07 314159265359879

Upon discussion it appears the most pressing use case here is for canceling transactions with incorrect nonces so they don't linger for days in the mempool and cause downstream problems with subsequent transactions.

markmhendrickson avatar Jul 17 '23 13:07 markmhendrickson

Another pressing use case is removing transactions that influence available balance such as outgoing STX transactions. Sometimes users can't replace a transaction because the wallet doesn't allow it because the available balance is 0 STX.

Cancelling such transactions should take into account balance available after cancelling in order to judge if the fee increase can be covered and if cancelling is possible.

Cancel transactions should create 0.000001 STX transfers to the burn address (Stacks network) with a relatively high fee, to make sure there is a high chance the transaction gets picked up after cancelling. Using the high fee for STX transfers with a cap 1-2 STX seems like a reasonable solution.

For cancelling bitcoin transactions, we may need optionality a higher fee than any other transaction pending to spend the same inputs would have to be the minimum, but typically I think transactions like this should be sent with the high priority fee to give users the best chance of actually cancelling the transaction.

314159265359879 avatar Feb 27 '24 12:02 314159265359879

Rather than some deeply integrated solution, this would be much easier to build if we trigger an extension pop up. This way, we craft the tx, fire, and forget, rather than building an in-app UX flow.

kyranjamie avatar Feb 27 '24 12:02 kyranjamie

@314159265359879 can i work on this?

  • I plan to focus first on working out the solution for stacks transactions and then move on to bitcoin transactions
  • The UX flow will be similar to increase fee
  • To cancel a transaction i plan to create and replace a new transaction that sends 0.000001 STX to the burn address with a 10% increase in fee and a cap of 1-2 STX
  • In mainnet i am planing to use SP000000000000000000002Q6VF78 as burn address, and ST000000000000000000002AMW42H for testnet
  • To address the edge case of available balance i am planning to calculate the effective balance by considering the pending transaction outgoing STX and judge if a cancellation is possible, if not inform the user that they don't have sufficient balance to cover the new increased fee.
  • unit tests

Is there any other details or important edge cases that i need to keep in mind?

Aman-zishan avatar Jun 04 '24 06:06 Aman-zishan

Hi again @Aman-zishan, yes, absolutely, you are welcome to work on this, thanks!

I plan to focus first on working out the solution for stacks transactions and then move on to bitcoin transactions

Excellent

The UX flow will be similar to increase fee

Please consider this comment from Kyran: https://github.com/leather-wallet/extension/issues/3406#issuecomment-1966470854

To cancel a transaction i plan to create and replace a new transaction that sends 0.000001 STX to the burn address with a 10% increase in fee and a cap of 1-2 STX

✅ Ideally the user pick their own fee, low, medium, high or custom as with other transactions.

In mainnet i am planing to use SP000000000000000000002Q6VF78 as burn address, and ST000000000000000000002AMW42H for testnet

Use SP00000000000003SCNSJTCSE62ZF4MSE for mainnet instead, we'll spare the genesis address a little bit from spam ;-)

To address the edge case of available balance i am planning to calculate the effective balance by considering the pending transaction outgoing STX and judge if a cancellation is possible, if not inform the user that they don't have sufficient balance to cover the new increased fee.

We already have some logic to determine available balance based on pending outgoing STX transfers and pending fees. I think you should also consider the transaction being replaced if a user. Consider this case: if a user has 1000 STX in their wallet, and they have a pending transaction to send 999.9 STX at a 0.1 STX fee, their available balance is 0 STX. They wouldn't be able to cover the minimal fee increase 0.000001 STX but their transaction can still be replaced because when the transaction is cancelled the transfer becomes 0.000001 STX instead of 999.9 STX which leaves more then enough to increase the fee.

unit tests

314159265359879 avatar Jun 04 '24 07:06 314159265359879

@314159265359879 Thanks! a few clarifications:

✅ Ideally the user pick their own fee, low, medium, high or custom as with other transactions.

In that case i can replicate the current fee selection UI with default set to high?.

Please consider this comment from Kyran: https://github.com/leather-wallet/extension/issues/3406#issuecomment-1966470854

So when a user click on cancel transaction, the extension opens up a new dialog ( similar to increase fee dialog ) , user chooses a fee , on choosing i have to calculate if user can afford replacement and this cancel request wont be shown as a pending transaction is that what is implied?

Use SP00000000000003SCNSJTCSE62ZF4MSE for mainnet instead, we'll spare the genesis address a little bit from spam ;-)

We already have some logic to determine available balance based on pending outgoing STX transfers and pending fees. I think you should also consider the transaction being replaced if a user. Consider this case: if a user has 1000 STX in their wallet, and they have a pending transaction to send 999.9 STX at a 0.1 STX fee, their available balance is 0 STX. They wouldn't be able to cover the minimal fee increase 0.000001 STX but their transaction can still be replaced because when the transaction is cancelled the transfer becomes 0.000001 STX instead of 999.9 STX which leaves more then enough to increase the fee.

Aman-zishan avatar Jun 04 '24 08:06 Aman-zishan

✅ Ideally the user pick their own fee, low, medium, high or custom as with other transactions.

In that case i can replicate the current fee selection UI with default set to high?.

Setting the default to high is a good idea. That way we increase the likelihood that the transaction is actually replaced.

Please consider this comment from Kyran: #3406 (comment)

So when a user click on cancel transaction, the extension opens up a new dialog ( similar to increase fee dialog ) , user chooses a fee , on choosing i have to calculate if user can afford replacement and this cancel request wont be shown as a pending transaction is that what is implied?

That sounds good one caveat:

We will have to show the transaction in the pending tab; it would be confusing not to. But I understand that "cancel" implies it vanishes, so the user may be confused.

These two additions could help mitigate that confusion: explain and add memo/label.

  1. Show a notice to users that Canceling a transaction isn't guaranteed to work. A higher fee can help replace the old transaction, but sometimes miners pick up a transaction before the new one replaces the old one in all network nodes.
  2. Add a memo to the transfer we make for example "_cancel transaction" That way we can add appropriate labeling in the UI later and/or as a quick fix just add this feature and display the memo for now literally: https://github.com/leather-wallet/extension/issues/2695.

314159265359879 avatar Jun 04 '24 09:06 314159265359879

@314159265359879 i have finished working on the stacks transactions part, right now working on the integrations tests for this feature, but i noticed that few integration tests are failing (found an issue related to tests unreliability -> #3919) should i add tests anyway even though they are being unreliable? (i verified the working via manual testing with the earlier discussed edge case and can produce a test report in the new PR). I could move on to bitcoin transaction solution if that is the case. Let me know your thoughts

Screenshot 2024-06-06 at 9 48 50 AM

Aman-zishan avatar Jun 06 '24 04:06 Aman-zishan

should i add tests anyway even though they are being unreliable?

cc @kyranjamie since he created the related Playwright tests unreliability issue

markmhendrickson avatar Jun 06 '24 05:06 markmhendrickson

How can i create a test environment for BTC transactions? even though i switch to testnet the BTC address remains to be mainnet one. leather gitbook seems to be down too

Aman-zishan avatar Jun 06 '24 07:06 Aman-zishan

@Aman-zishan you should be able to access bitcoin testnet(3) when you select this option in the wallet:

image

You should see testnet addresses on the receive button. image

When adding custom (stacks) testnets there is currently an issue where the mainnet details are used instead. https://github.com/leather-wallet/extension/issues/5228 perhaps that is what you are running into.

Bitcoin testnet 3 is unreliable, constant blockstorm, but you may still be able to utilise it. image

Work is being done to replace it with a new one public bitcoin testnet (testnet4), I just added an issue here to accommodate that later when it is ready. Right now https://github.com/leather-wallet/extension/issues/5491

314159265359879 avatar Jun 06 '24 10:06 314159265359879

@314159265359879

Work is being done to replace it with a new one public bitcoin testnet (testnet4), I just added an issue here to accommodate that later when it is ready. Right now https://github.com/leather-wallet/extension/issues/5491

In that case, once I get confirmation from @kyranjamie regarding the status of integration tests, I would like to include the Stacks transactions cancel feature in a single PR and focus on BTC transactions in a later PR once I am able to reproduce a good testing environment, if that is okay.

Aman-zishan avatar Jun 06 '24 11:06 Aman-zishan

Ideally we should not be adding more tests that are unreliable, as this might exacerbate the issues we're having.

Do we know why the test in being unreliable?

kyranjamie avatar Jun 06 '24 11:06 kyranjamie

@kyranjamie

Ideally we should not be adding more tests that are unreliable, as this might exacerbate the issues we're having. Do we know why the test in being unreliable?

Not all but a few tests are failing when test selectors are not found Screenshot 2024-06-07 at 10 46 49 AM

Aman-zishan avatar Jun 07 '24 05:06 Aman-zishan

@Aman-zishan have you set WALLET_ENVIRONMENT=testing in .env?

alter-eggo avatar Jun 07 '24 07:06 alter-eggo

@alter-eggo

@Aman-zishan have you set WALLET_ENVIRONMENT=testing in .env?

No, i re ran after setting WALLET_ENVIRONMENT=testing and a TEST_ACCOUNT_SECRET_KEY i still get 21 failed tests Screenshot 2024-06-07 at 1 35 39 PM

Screenshot 2024-06-07 at 1 37 08 PM

Aman-zishan avatar Jun 07 '24 08:06 Aman-zishan