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

After the transaction is removed, the re-synchronization is still on the transaction list. #1630

Open
nieaowei opened this issue Sep 28, 2024 · 6 comments
Labels
bug Something isn't working discussion There's still a discussion ongoing

Comments

@nieaowei
Copy link

nieaowei commented Sep 28, 2024

Describe the bug
After the transaction is removed, the re-synchronization is still on the transaction list.

To Reproduce

  1. Create tx by other wallet
  2. Sync wallet
  3. Bdk Wallet display the tx at moment
  4. RBF the tx by another wallet
  5. Sync Wallet
  6. The tx still display
    Expected behavior

Build environment

  • BDK tag/commit: v1.0.0-beta4
  • OS+version: macOS 15
  • Rust/Cargo version: 1.80
  • Rust/Cargo target: x86_64-apple-darwin,

Additional context

image image
@nieaowei nieaowei added the bug Something isn't working label Sep 28, 2024
@oleonardolima
Copy link
Contributor

I'm looking to try to reproduce this error. @nieaowei are there any specific code snippets, other than the described steps above, to reproduce it?

@notmandatory notmandatory added this to BDK Nov 4, 2024
@notmandatory notmandatory moved this to Todo in BDK Nov 4, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Nov 4, 2024
@notmandatory
Copy link
Member

notmandatory commented Nov 5, 2024

@nieaowei I created a quick unit test to try and reproduce your scenario but did not see the same issue you're getting. Can you take a look at my steps and see if I'm missing anything? I'm not using multiple wallets, but should be simulating the same scenario where a wallet receives two versions of the same transaction before and after doing an RBF.

#[test]
fn test_fee_bump_replaces_orig_tx() {
    // create the original unconfirmed tx
    let (mut wallet, _) = get_funded_wallet_wpkh();
    let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX")
        .unwrap()
        .assume_checked();
    let mut builder = wallet.build_tx();
    builder.drain_wallet().drain_to(addr.script_pubkey());
    let psbt = builder.finish().unwrap();
    let mut orig_tx = psbt.extract_tx().expect("failed to extract tx");
    let orig_txid = orig_tx.compute_txid();
    for txin in &mut orig_tx.input {
        txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
    }
    // insert original unconfirmed tx in the wallet
    wallet.insert_tx(orig_tx);
    insert_seen_at(&mut wallet, orig_txid, 0);
    // confirm wallet contains original tx
    assert!(wallet.transactions().map(|tx| tx.tx_node.txid).find(|txid| txid == &orig_txid).is_some());

    // create a new fee bump tx with higher fee rate
    let mut builder = wallet.build_fee_bump(orig_txid).unwrap();
    builder
        .fee_rate(FeeRate::from_sat_per_vb_unchecked(15))
        // remove original tx drain_to address and amount
        .set_recipients(Vec::new())
        // set back original drain_to address
        .drain_to(addr.script_pubkey())
        // drain wallet output amount will be re-calculated with new fee rate
        .drain_wallet();
    let psbt = builder.finish().unwrap();
    let mut fee_bump_tx = psbt.extract_tx().expect("failed to extract tx");
    let fee_bump_txid = fee_bump_tx.compute_txid();
    for txin in &mut fee_bump_tx.input {
        txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
    }
    // insert fee bump unconfirmed tx in the wallet
    wallet.insert_tx(fee_bump_tx);
    insert_seen_at(&mut wallet, fee_bump_txid, 1);

    // confirm both orig and fee_bump tx have different ids
    assert_ne!(orig_txid, fee_bump_txid);

    // confirm wallet transactions list contains the fee bump tx but not the original tx
    assert!(wallet.transactions().find(|tx| tx.tx_node.txid == fee_bump_txid).is_some());
    assert!(wallet.transactions().find(|tx| tx.tx_node.txid == orig_txid).is_none());
}

Note that for unconfirmed transactions the wallet's TxGraph uses the latest "seen_at" transaction to determine which of multiple conflicting transactions is the "canonical" or current valid one. See also: https://docs.rs/bdk_chain/latest/bdk_chain/tx_graph/index.html.

@nieaowei
Copy link
Author

nieaowei commented Nov 7, 2024

The important point is that other wallets can use RBF to modify the outputs of the transaction.
Origin tx:

input output
outpoint1 wallet2 addr
wallet1 change addr

RBF tx:

input output
outpoint1 wallet3 addr
wallet1 change addr

First, I used the Sparrow Wallet to initiate the original transaction, and after syncing, Wallet 2 displayed the original transaction. Then, I used RBF on the original transaction to modify the receiving address. However, even after the RBF transaction was confirmed, Wallet 2 still showed the original transaction.

One more thing, if I want to clear this invalid transaction(Origin), I have to delete the SQLite file and resynchronize.

@nieaowei
Copy link
Author

nieaowei commented Nov 7, 2024

@nieaowei I created a quick unit test to try and reproduce your scenario but did not see the same issue you're getting. Can you take a look at my steps and see if I'm missing anything? I'm not using multiple wallets, but should be simulating the same scenario where a wallet receives two versions of the same transaction before and after doing an RBF.

#[test]
fn test_fee_bump_replaces_orig_tx() {
    // create the original unconfirmed tx
    let (mut wallet, _) = get_funded_wallet_wpkh();
    let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX")
        .unwrap()
        .assume_checked();
    let mut builder = wallet.build_tx();
    builder.drain_wallet().drain_to(addr.script_pubkey());
    let psbt = builder.finish().unwrap();
    let mut orig_tx = psbt.extract_tx().expect("failed to extract tx");
    let orig_txid = orig_tx.compute_txid();
    for txin in &mut orig_tx.input {
        txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
    }
    // insert original unconfirmed tx in the wallet
    wallet.insert_tx(orig_tx);
    insert_seen_at(&mut wallet, orig_txid, 0);
    // confirm wallet contains original tx
    assert!(wallet.transactions().map(|tx| tx.tx_node.txid).find(|txid| txid == &orig_txid).is_some());

    // create a new fee bump tx with higher fee rate
    let mut builder = wallet.build_fee_bump(orig_txid).unwrap();
    builder
        .fee_rate(FeeRate::from_sat_per_vb_unchecked(15))
        // remove original tx drain_to address and amount
        .set_recipients(Vec::new())
        // set back original drain_to address
        .drain_to(addr.script_pubkey())
        // drain wallet output amount will be re-calculated with new fee rate
        .drain_wallet();
    let psbt = builder.finish().unwrap();
    let mut fee_bump_tx = psbt.extract_tx().expect("failed to extract tx");
    let fee_bump_txid = fee_bump_tx.compute_txid();
    for txin in &mut fee_bump_tx.input {
        txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
    }
    // insert fee bump unconfirmed tx in the wallet
    wallet.insert_tx(fee_bump_tx);
    insert_seen_at(&mut wallet, fee_bump_txid, 1);

    // confirm both orig and fee_bump tx have different ids
    assert_ne!(orig_txid, fee_bump_txid);

    // confirm wallet transactions list contains the fee bump tx but not the original tx
    assert!(wallet.transactions().find(|tx| tx.tx_node.txid == fee_bump_txid).is_some());
    assert!(wallet.transactions().find(|tx| tx.tx_node.txid == orig_txid).is_none());
}

Note that for unconfirmed transactions the wallet's TxGraph uses the latest "seen_at" transaction to determine which of multiple conflicting transactions is the "canonical" or current valid one. See also: https://docs.rs/bdk_chain/latest/bdk_chain/tx_graph/index.html.

Here’s an additional piece of somewhat messy code.

fn main() {
    // create();
    let mut conn1 = bdk_wallet::rusqlite::Connection::open(format!("./{}.sqlite", "wallet1").as_str()).unwrap();
    let mut conn2 = bdk_wallet::rusqlite::Connection::open(format!("./{}.sqlite", "wallet2").as_str()).unwrap();

    let mut w1 = Wallet::load()
        .descriptor(External, Some("tr()#h305zpuu"))
        .extract_keys()
        .load_wallet(&mut conn1).unwrap().unwrap();
    let mut w2 = Wallet::load()
        .descriptor(External, Some("tr()#dyn6d6zd"))
        .extract_keys()
        .load_wallet(&mut conn2).unwrap().unwrap();


    let client = bdk_esplora::esplora_client::Builder::new("https://mempool.space/testnet4/api").build_blocking();


    // sync wallet 2
    let sync = w2.start_sync_with_revealed_spks().build();
    let up = client.sync(sync, 5).unwrap();
    let _ = w2.apply_update(up).unwrap();

    w2.persist(&mut conn2).unwrap();

    for transaction in w2.transactions() {
        println!("{}", transaction.tx_node.txid);
    }

    // build origin tx
    let w1_addr = w1.peek_address(External, 0).address;
    let w2_addr = w2.peek_address(External, 0).address;
    let mut origin_psbt = w1.build_tx()
        .ordering(TxOrdering::Untouched)
        .add_recipient(w2_addr.script_pubkey(), Amount::from_sat(1000))
        .fee_rate(FeeRate::from_sat_per_vb(2).unwrap())
        .drain_to(w1_addr.script_pubkey())
        .clone()
        .finish().unwrap();

    let ok = w1.sign(&mut origin_psbt, SignOptions::default()).unwrap();
    let origin_tx = origin_psbt.extract_tx().unwrap();
    println!("{}", origin_tx.compute_txid());

    client.broadcast(&origin_tx).unwrap();

    // sync wallet 2
    let sync = w2.start_sync_with_revealed_spks().build();
    let up = client.sync(sync, 5).unwrap();
    let _ = w2.apply_update(up).unwrap();

    w2.persist(&mut conn2).unwrap();

    for transaction in w2.transactions() { // contain origin txid
        println!("{}", transaction.tx_node.txid);
    }

    // rbf
    let w3_addr = Address::from_str("").unwrap().assume_checked();
    let tx = Transaction {
        version: origin_tx.version,
        lock_time: origin_tx.lock_time,
        input: origin_tx.input.iter().map(|input| TxIn {
            previous_output: input.previous_output,
            script_sig: Default::default(),
            sequence: input.sequence,
            witness: Default::default(),
        }).collect(),
        output: vec![
            TxOut { value: origin_tx.output[0].value, script_pubkey: w3_addr.script_pubkey() },
            TxOut { value: origin_tx.output[1].value - Amount::from_sat(300), script_pubkey: w1_addr.script_pubkey() },
        ],
    };
    let mut rbf_psbt = Psbt {
        unsigned_tx: tx,
        version: 0,
        xpub: Default::default(),
        proprietary: Default::default(),
        unknown: Default::default(),
        inputs: vec![Input {
            witness_utxo: Some(TxOut { value: Amount::from_sat(10000), script_pubkey: w1_addr.script_pubkey() }),
            ..Default::default()
        }],
        outputs: vec![Default::default(); 2],
    };

    let ok = w1.sign(&mut rbf_psbt, SignOptions::default()).unwrap();
    let rbf_tx = rbf_psbt.extract_tx().unwrap();
    println!("{}", rbf_tx.compute_txid());

    client.broadcast(&rbf_tx).unwrap();

    // sync wallet 2
    let sync = w2.start_sync_with_revealed_spks().build();
    let up = client.sync(sync, 5).unwrap();
    let _ = w2.apply_update(up).unwrap();

    w2.persist(&mut conn2).unwrap();

    for transaction in w2.transactions() { // Still contain the original tx
        println!("{}", transaction.tx_node.txid);
    }
}

@notmandatory
Copy link
Member

thanks for the example, I'll use this to try to reproduce the issue. I have a suspicion that what's going on is you're still seeing the original tx as "unconfirmed" in w2 since the wallet saw it after the first broadcast and sync. In the above scenario has the rbf_tx already been confirmed?

@notmandatory notmandatory added the discussion There's still a discussion ongoing label Nov 21, 2024
@notmandatory notmandatory removed this from the 1.0.0-beta milestone Nov 21, 2024
@oleonardolima
Copy link
Contributor

I'll try to reproduce it, but AFAICT from the snippets and example code it seems to be the same issue as #1740 (cc @notmandatory).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discussion There's still a discussion ongoing
Projects
Status: Todo
Development

No branches or pull requests

3 participants