bitcoindevkit / bdk

A modern, lightweight, descriptor-based wallet library written in Rust!
Other
871 stars 312 forks source link

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

Open nieaowei opened 2 months ago

nieaowei commented 2 months ago

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

Additional context

image image
oleonardolima commented 3 weeks ago

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 commented 3 weeks ago

@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 commented 2 weeks ago
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 commented 2 weeks ago

@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 commented 2 weeks ago

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?