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

Support "amend a pending transaction" #332

Open
Keith-CY opened this issue Dec 8, 2023 · 21 comments
Open

Support "amend a pending transaction" #332

Keith-CY opened this issue Dec 8, 2023 · 21 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@Keith-CY
Copy link
Member

Keith-CY commented Dec 8, 2023

Sometimes there will be transactions that keep pending and cannot be retried until they are removed from Neurno by clearing the cache. It can be done by the following steps:

  1. Go to Settings => Data => Clear Cache;
  2. Wait seconds to rebuild data;
  3. Submit a transaction with the same outputs;

Because the strategy of picking cells is determined, the new transaction should include the same inputs as the pending one. But it depends, and may use different cells when the fee rate changes and the last input is not enough for the transaction fee, then a larger cell will be used. So when there's only 1 input, it would be undetermined.

  1. pending transaction using cell_a to send 100 CKB;
  2. pending transaction is removed from Neuron, but actually it is broadcasted;
  3. a new transaction is required to send 100 CKB, but due to fee change, cell_b is used instead of cell_a.

Now there are two transactions sending 100 CKB to the same receiver.


Another case is that a transaction is sent with a low fee rate, so it won't be packaged by miners. It's not a cache problem, the transaction is already in the transaction pool. The only action a user can take is to raise a transaction with a higher fee rate to invalidate the previous one. In this case, the cell in the second transaction is deliberately set.


For these two cases, a new feature to invalidate a transaction in the pool should be introduced.

The great news is that Replace by fee was just supported in [email protected] with nervosnetwork/ckb#4079

We should go through this function and adopt it in Neuron

@Keith-CY Keith-CY added the documentation Improvements or additions to documentation label Dec 8, 2023
@Keith-CY Keith-CY added this to Neuron Dec 8, 2023
@Keith-CY Keith-CY moved this to 📋 Backlog in Neuron Dec 8, 2023
@Danie0918 Danie0918 moved this from 📋 Backlog to 🆕 New in Neuron Dec 8, 2023
@Keith-CY
Copy link
Member Author

As mentioned above, there are two scenarios for handling pending transactions:

  1. A transaction is rejected by a node, but neuron doesn't realize it and keeps it in the cache ;
  2. A transaction is pending in the transaction pool, but it's not prioritized by miners.

For 1, a status-checking-loop can be introduced to update their status periodically, e.g., every 10 mins Neuron will fetch the statuses of all pending transactions and mark those rejected ones, including those not found in the transaction pool;

For 2, the new feature Replace by fee could be used to retry pending transactions, the workflow could be roughly described as follows:

  1. The wallet submits a transaction.
  2. For some reason, the wallet wants to replace this transaction.
  3. The wallet fetches this transaction from the node.
  4. If the node returns a transaction with a min_replace_fee, the wallet can utilize the Replace-By-Fee (RBF) feature to effect the replacement, proceeding to the following steps.
  5. The wallet constructs a new transaction with the same inputs, setting the fee of the new transaction to min_replace_fee and submits it.
  6. As the replacement transaction may not be smoothly replaced, both transactions need to be displayed on the waiting list simultaneously.
  7. If either of the two transactions is confirmed as "committed," the other one is deleted. If either of the two transactions is confirmed as "rejected," that specific transaction is deleted (rather than marked as "rejected").

@Danie0918 Danie0918 moved this from 🆕 New to 📋 Backlog in Neuron Dec 11, 2023
@Keith-CY
Copy link
Member Author

Feel free to leave questions about the new feature and how to support it in Neuron @Magickbase/neuron

@yanguoyu
Copy link

  1. From Support Replace-by-fee in memory pool nervosnetwork/ckb#3734, it describes that ckb will create a new RPC send_transaction_replaceable to set the tx's replaceable as true. But I can not find any information about replaceable.
    Should we add a field when sending a replacement tx?
  2. Is it easy to test on the testnet? The transaction is always broadcast fast even if the fee is minimum
  3. If the input capacity can not pay a sufficient fee, what should we do?
  4. Tips: After the Neuron supports cell management, it supports generating a tx with the appointed cell.

@Keith-CY
Copy link
Member Author

  1. From Support Replace-by-fee in memory pool nervosnetwork/ckb#3734, it describes that ckb will create a new RPC send_transaction_replaceable to set the tx's replaceable as true. But I can not find any information about replaceable.
    Should we add a field when sending a replacement tx?

When RBF is enabled by the ckb node, the rpc send_transaction would be a wrapper of send_transaction_replaceable, so we don't have to add a new RPC method. If the feature is supported can be confirmed by checking if there's a field named min_replace_fee returned by get_transaction.

Ref: Adds send_transaction_replaceable, make send_transaction as a wrapper for it, defaults replaceable is false: nervosnetwork/ckb#3734

  1. Is it easy to test on the testnet? The transaction is always broadcast fast even if the fee is minimum

It can be verified on devnet with a longer block interval.

  1. If the input capacity can not pay a sufficient fee, what should we do?

Disable the button to submit a replacement.

  1. Tips: After the Neuron supports cell management, it supports generating a tx with the appointed cell.

RBF requires the same set of inputs, so adding actions on a pending transaction would be intuitive.

@Keith-CY Keith-CY changed the title Support "replace a pending transaction" Support "amend a pending transaction" Dec 13, 2023
@Keith-CY
Copy link
Member Author

Keith-CY commented Dec 21, 2023

RBF requires the same set of inputs, so adding actions on a pending transaction would be intuitive.

It should be corrected, the description used in the RFC(nervosnetwork/ckb#3734 (comment)) is with any same input. It means that fixing a specific cell is feasible.

@Danie0918 Danie0918 moved this from 📋 Backlog to 📌Planning in Neuron Dec 25, 2023
@Keith-CY
Copy link
Member Author

Is there a draft of PRD for preview?

@Danie0918
Copy link
Contributor

Is there a draft of PRD for preview?

It's still a work in progress and is expected to be completed this week.

@Danie0918 Danie0918 moved this from 📌Planning to 🎨 Designing in Neuron Jan 2, 2024
@Danie0918
Copy link
Contributor

Danie0918 commented Jan 2, 2024

【Support "amend a pending transaction"】

  • Requirement Description: Users may encounter a pending transaction after submitting a transaction, the introduction of "amend a pending transaction" can help users modify the submitted transaction.

Image

  • Requirement Contents
  1. Provides the option to amend the transaction when its status is 'Pending' or 'Rejected'.
  2. When amending a transaction, only the transaction fee can be modified.
  3. No balance/stealth address is displayed.
  4. During this process, a pop-up window prompts the user when the original transaction is confirmed and returns to the history page.
  5. When a transaction is amemded, the previous transaction should be removed from the history.

@Keith-CY
Copy link
Member Author

Keith-CY commented Jan 2, 2024

  1. Provides the option to amend the transaction when its status is 'Pending'.

I think the amend could be supported when the transaction is pending or rejected

  1. No balance/stealth address is displayed.

What does no stealth address is displayed mean

@Danie0918
Copy link
Contributor

  1. Provides the option to amend the transaction when its status is 'Pending'.

I think the amend could be supported when the transaction is pending or rejected

OK

  1. No balance/stealth address is displayed.

What does no stealth address is displayed mean

#100 An stealth address option will be added to the send page.

@yanguoyu
Copy link

yanguoyu commented Jan 3, 2024

During this process, a pop-up window prompts the user when the original transaction is confirmed and returns to the history page.

Does it mean we should get the old transaction's status with polling? @Danie0918


I think the amend could be supported when the transaction is pending or rejected

For the current Neuron https://github.com/nervosnetwork/neuron/blob/develop/packages/neuron-wallet/src/block-sync-renderer/tx-status-listener.ts#L18-L41, the transaction's status Pending includes others' status except committed @Keith-CY cc @Danie0918


The amended transaction can not be lower than the original

The amended transaction's fee should be bigger than min_replace_fee


Should we support changing the output when sending a replacement transaction? @Keith-CY @Danie0918

@Danie0918
Copy link
Contributor

During this process, a pop-up window prompts the user when the original transaction is confirmed and returns to the history page.

Does it mean we should get the old transaction's status with polling? @Danie0918

Polling is only required if the state is pending.

BTW, can we distinguish between pending and rejected?

@yanguoyu
Copy link

yanguoyu commented Jan 3, 2024

During this process, a pop-up window prompts the user when the original transaction is confirmed and returns to the history page.

Does it mean we should get the old transaction's status with polling? @Danie0918

Polling is only required if the state is pending.

BTW, can we distinguish between pending and rejected?

Of course we can, but currently Neuron did not save these status

@Keith-CY
Copy link
Member Author

Keith-CY commented Jan 3, 2024

For the current Neuron nervosnetwork/neuron@develop/packages/neuron-wallet/src/block-sync-renderer/tx-status-listener.ts#L18-L41, the transaction's status Pending includes others' status except committed

It's the first case mentioned at #332 (comment), status of pending transactions should be examined and updated periodically, so a rejected transaction can be marked as rejected, but amend still works with those transactions because the receiver may not receive the asset as expected

@Keith-CY
Copy link
Member Author

Keith-CY commented Jan 3, 2024

Should we support changing the output when sending a replacement transaction? @Keith-CY @Danie0918

I think it should be disallowed to update outputs. Personally speaking, the outputs represent the goal of the transaction, if I have another goal, I can make another transaction.

In fact, rejected/pending transaction can also be retried with submitting a new transaction, but amend ensures the goal will not be tweaked unexpectedly

@Danie0918
Copy link
Contributor

PR:nervosnetwork/neuron#3045

@Danie0918 Danie0918 moved this from 🏗 In Progress to 🔎 Code Review in Neuron Feb 28, 2024
@Danie0918 Danie0918 moved this from 🔎 Code Review to 👀 Testing in Neuron Mar 11, 2024
@Danie0918 Danie0918 moved this from 👀 Testing to 🚩Pre Release in Neuron Mar 15, 2024
@Danie0918 Danie0918 moved this from 🚩Pre Release to 🏗 In Progress in Neuron Apr 10, 2024
@Danie0918
Copy link
Contributor

support__amend_a_pending_transaction_

  • Requirement Contents
  1. Supports amend all types of transactions.
  2. New popup window added to bring in previous transaction fees by default.
  3. Automatically clear transactions that have failed for more than 24 hours to avoid input hogging.

@devchenyan
Copy link

@Danie0918 Danie0918 moved this from 🏗 In Progress to 🔎 Code Review in Neuron Apr 26, 2024
@Danie0918 Danie0918 moved this from 🔎 Code Review to 🚩Pre Release in Neuron May 7, 2024
@Danie0918 Danie0918 moved this from 🚩Pre Release to 🔎 Code Review in Neuron May 7, 2024
@Danie0918 Danie0918 moved this from 🔎 Code Review to 👀 Testing in Neuron May 8, 2024
@Danie0918 Danie0918 moved this from 👀 Testing to ✅ Done in Neuron May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: ✅ Done
Development

No branches or pull requests

7 participants