proof of concept: implement cancel_tx

[ValuedMammal]

Description

This is not a merge candidate. The goal of this PR is implement cancel_tx, as doing so has the potential to fix a long-standing TODO in the library saying "make this free up reserved utxos when that's implemented". This was initiated in response to user feedback claiming that the method has no effect.

issue #1743

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

[ErikDeSmedt]

Thanks for looking into this.

I'm not very familiar with the BDK-codebase but I am happy to see where this is going

[ErikDeSmedt]

I don't really understand the design of this enum.

What is the difference between Record::Canceled and Record::Details { canceled: true}.
Why are both structs required?

[ValuedMammal]

This made it easy to match the kind of change to the specific SQL statement, with the assumption that the real TxDetails would have more than a single field and hence be subject to different kinds of changes. Granted it's a bit contrived at the moment. Anyway, appreciate your comments!

[ErikDeSmedt]

Nice, this test behaves exactly as I would expect

[evanlinjin]

@ValuedMammal could you further elaborate on the goals of this PR? Thanks!

[buffrr]

Even after PR #1765, I think cancel_tx would still be needed for different reasons. If a transaction gets dropped from the mempool but persisted in the wallet as unconfirmed, we need a way to handle it - otherwise it would stay as unconfirmed in the wallet db forever, right? Users would have no choice but to rescan their wallet (I guess they could try to rebroadcast or bump the fee)

[evanlinjin]

@buffrr The only way we can confidently say "a transaction is dropped" is if a conflicting transaction gets confirmed. Therefore, having cancel_tx is not the correct fix here. Instead, we should somehow introduce the conflicting tx into our TxGraph.

[buffrr]

It's possible for a transaction to be dropped from the mempool without a conflicting tx - such as when it has a low fee and stays in the mempool too long. While we can have some level of confidence after seeing a conflicting tx, we also can't be certain that it will be mined over the original. Waiting for a confirmation of a foreign/irrelevant tx might not be desirable as it locks the users funds during that period which could be hours or days.

It would be helpful to give developers the option to explicitly mark a tx as 'ok to double spend' and I guess that's what would cancel_tx enable. I'm grateful for #1765 so this is more of a nice to have!

[evanlinjin]

Imagine this from a UX perspective. The user creates a tx because they have intent to spend their Bitcoin. Then because feerate too low, it gets dropped from mempool and the tx just disappears from the wallet. I would say this is unexpected behavior. I expect the user would want to RBF the tx?

What if the feerate is acceptable after some time, and someone, all this time, has a mempool that didn't drop txs and this tx gets rebroadcasted? So now do we need a method to uncancel the tx?

There are other ways to explicitly mark a tx as "okay to double spend".

[buffrr]

Then because feerate too low, it gets dropped from mempool and the tx just disappears from the wallet. I would say this is unexpected behavior. I expect the user would want to RBF the tx?

While I agree in that specific case for typical transactions, it really depends on the use case and how developers would want to tailor the UX for end users.

What if the feerate is acceptable after some time, and someone, all this time, has a mempool that didn't drop txs and this tx gets rebroadcasted? So now do we need a method to uncancel the tx?

There's no need for such a method as that wouldn't result in locking up funds. The worst that could happen is they end up replacing it, or it gets confirmed, which, depending on the use case, might be ok.

There are other ways to explicitly mark a tx as "okay to double spend".

I’m not sure that’s easily achievable. Perhaps only by attempting to manually find and double-spend the locked-up funds?

[ValuedMammal]

Sorry for the late reply, I do have several thoughts

I'm still at a light concept ACK for this feature, because it allows you to abandon an untrusted, unconfirmed tx that has no viability on chain but would nonetheless be stuck pending in the wallet and risk poisoning the local utxo pool with bad outputs. That said, I agree that simply introducing the spending tx should be enough to resolve the conflict internally.

To reiterate the rules of cancel_tx:

1. Txouts of a canceled tx are ignored, provided the canceled tx is unconfirmed
2. Unspents are kept if spent by a canceled tx, provided the canceled tx is unconfirmed

What if someone rebroadcasts?

• In the event the tx confirms there is no issue because confirmation takes precedence over cancellation, no need to un-cancel
• If we cancel tx1 and then reuse the coins in tx2 and tx2 confirms, then a rebroadcast of tx1 will be rejected by the network.

The Bitcoin Core wallet has an abandontransaction RPC and apparently does this on its own for txs that have been stuck for two weeks (-mempoolexpiry), so in terms of wallet behavior this isn't particularly groundbreaking.

There are a few caveats though:

• With this approach we can't actually mark the change address unused if the tx is indexed internally, so in that sense we could be trading off one weakness for another.
• In terms of implementation I think a more permanent solution could be done in bdk_chain where an abandoned tx would be represented by transaction state 4 (Don't double spend created transaction just because they haven't been broadcasted #1748). But I'm not opposed to a solution similar to this PR if there's enough interest in further developing TxDetails.

I agree that in the absence of this feature, we will likely need better solutions for RBF and CPFP.

[evanlinjin]

@ValuedMammal Thank you for clarifying. This now makes sense for me.