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

cancel_tx has no effect #1743

Open
ErikDeSmedt opened this issue Nov 22, 2024 · 2 comments
Open

cancel_tx has no effect #1743

ErikDeSmedt opened this issue Nov 22, 2024 · 2 comments
Labels
api A breaking API change bug Something isn't working documentation Improvements or additions to documentation

Comments

@ErikDeSmedt
Copy link

Describe the bug

The cancel_tx method does not free up the change.

To Reproduce
See https://github.com/ErikDeSmedt/bdk-gists/blob/master/tests/cancel_takes_effect.rs

Expected behavior
I expect that the change is freed

Build environment

  • BDK tag/commit: bdk_wallet 1.0.5
  • OS+version: debian
  • Rust/Cargo version: 1.79.0

Additional context

@ErikDeSmedt ErikDeSmedt added the bug Something isn't working label Nov 22, 2024
@notmandatory
Copy link
Member

I think the issue you're running into is a misunderstanding of what cancel_tx does. That function only frees up the script index of the change address if they haven't been seen as used/spent by the wallet via broadcast/sync OR by manually inserting them into the wallet tx graph as you've done with apply_unconfirmed_txs.

    /// Informs the wallet that you no longer intend to broadcast a tx that was built from it.
    ///
    /// This frees up the change address used when creating the tx for use in future transactions.
    // TODO: Make this free up reserved utxos when that's implemented
    pub fn cancel_tx(&mut self, tx: &Transaction) {
        let txout_index = &mut self.indexed_graph.index;
        for txout in &tx.output {
            if let Some((keychain, index)) = txout_index.index_of_spk(txout.script_pubkey.clone()) {
                // NOTE: unmark_used will **not** make something unused if it has actually been used
                // by a tx in the tracker. It only removes the superficial marking.
                txout_index.unmark_used(*keychain, *index);
            }
        }
    }

@notmandatory notmandatory added this to BDK Nov 22, 2024
@notmandatory notmandatory moved this to Discussion in BDK Nov 22, 2024
@evanlinjin
Copy link
Member

I think we should probably rename cancel_tx to unreserve_change_address.

@notmandatory notmandatory added documentation Improvements or additions to documentation api A breaking API change labels Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api A breaking API change bug Something isn't working documentation Improvements or additions to documentation
Projects
Status: Discussion
Development

No branches or pull requests

3 participants