fix(chain): redefine relevant txs in IndexedTxGraph

A transaction's relevancy was originally only determined by the spks
referenced by the tx's inputs and outputs. A new rule is added where if
a tx shares inputs with anything contained in TxGraph, then it should
also be considered relevant. This fixes a potential double spending
problem.

Author: LagginTimes

crates/chain/src/indexed_tx_graph.rs

@@ -178,7 +178,12 @@ where
 
         let mut tx_graph = tx_graph::ChangeSet::default();
         for (tx, anchors) in txs {
-            if self.index.is_tx_relevant(&tx) {
+            if self.index.is_tx_relevant(&tx)
+                || self
+                    .graph
+                    .direct_conflicts(&tx)
+                    .any(|(_, txid)| self.graph.get_tx(txid).is_some())
+            {
                 let txid = tx.compute_txid();
                 tx_graph.merge(self.graph.insert_tx(tx.clone()));
                 for anchor in anchors {
@@ -218,11 +223,18 @@ where
             indexer.merge(self.index.index_tx(tx));
         }
 
-        let graph = self.graph.batch_insert_unconfirmed(
-            txs.into_iter()
-                .filter(|(tx, _)| self.index.is_tx_relevant(tx))
-                .map(|(tx, seen_at)| (tx.clone(), seen_at)),
-        );
+        let mut relevant_txs = Vec::new();
+        for (tx, seen_at) in txs.into_iter() {
+            if self.index.is_tx_relevant(&tx)
+                || self
+                    .graph
+                    .direct_conflicts(&tx)
+                    .any(|(_, txid)| self.graph.get_tx(txid).is_some())
+            {
+                relevant_txs.push((tx.clone(), seen_at));
+            }
+        }
+        let graph = self.graph.batch_insert_unconfirmed(relevant_txs);
 
         ChangeSet {
             tx_graph: graph,
@@ -278,7 +290,13 @@ where
         let mut changeset = ChangeSet::<A, I::ChangeSet>::default();
         for (tx_pos, tx) in block.txdata.iter().enumerate() {
             changeset.indexer.merge(self.index.index_tx(tx));
-            if self.index.is_tx_relevant(tx) {
+
+            if self.index.is_tx_relevant(tx)
+                || self
+                    .graph
+                    .direct_conflicts(tx)
+                    .any(|(_, txid)| self.graph.get_tx(txid).is_some())
+            {
                 let txid = tx.compute_txid();
                 let anchor = TxPosInBlock {
                     block,

crates/wallet/Cargo.toml

@@ -47,6 +47,10 @@ bdk_file_store = { path = "../file_store" }
 anyhow = "1"
 rand = "^0.8"
 
+# temporary dev-dependencies for testing purposes
+bdk_bitcoind_rpc = { path = "../bitcoind_rpc" }
+bdk_testenv = { path = "../testenv" }
+
 [package.metadata.docs.rs]
 all-features = true
 rustdoc-args = ["--cfg", "docsrs"]

crates/wallet/tests/wallet.rs

@@ -4196,8 +4196,125 @@ fn test_transactions_sort_by() {
     assert_eq!([None, Some(2000), Some(1000)], conf_heights.as_slice());
 }
 
+use std::collections::HashMap;
+
+use bdk_bitcoind_rpc::Emitter;
+use bdk_testenv::bitcoincore_rpc::json::CreateRawTransactionInput;
+use bdk_testenv::bitcoincore_rpc::{RawTx, RpcApi};
+use bdk_testenv::TestEnv;
+use bdk_wallet::CreateParams;
+
 #[test]
 fn test_tx_builder_is_send_safe() {
     let (mut wallet, _txid) = get_funded_wallet_wpkh();
     let _box: Box<dyn Send + Sync> = Box::new(wallet.build_tx());
 }
+
+// ErikDeSmedt's double spend test scenario
+#[test]
+fn detect_double_spend() -> anyhow::Result<()> {
+    // Create a test environment and mine initial funds
+    let env = TestEnv::new().expect("Failed to launch bitcoind");
+    let client = env
+        .bitcoind
+        .create_wallet("")
+        .expect("Failed to create wallet");
+    let address = client
+        .get_new_address(None, None)
+        .expect("Failed to create address");
+    client
+        .generate_to_address(106, address.assume_checked_ref())
+        .expect("Failed to mine block");
+
+    // Create the wallet under test
+    let mut alice = {
+        const DESC: &str ="tr(tprv8ZgxMBicQKsPdnxnfzsvrUZ58eNTq85PA6nhJALQiGy9GVhcvXmHX2r9znpyApMVNLdkPBp3WArLgU3UnA6npK9TtGoZDKdAjjkoYm3rY7F/84'/0'/0'/0/*)";
+        Wallet::create_with_params(
+            CreateParams::new_single(DESC).network(Network::Regtest)
+        )
+    }
+    .expect("Wallet can be created");
+
+    // Sync it with the chain
+    let mut emitter = Emitter::new(&env.bitcoind.client, alice.latest_checkpoint(), 0);
+    while let Some(ev) = emitter.next_block().unwrap() {
+        alice
+            .apply_block_connected_to(&ev.block, ev.block_height(), ev.connected_to())
+            .unwrap();
+    }
+
+    // Creates some transactions
+    let unspent = client
+        .list_unspent(None, None, None, Some(false), None)
+        .unwrap();
+
+    let input_amount = unspent[0].amount;
+    let destination_amount = Amount::from_sat(100_000);
+    let fee_amount = Amount::from_sat(2_000);
+    let change_amount = input_amount - destination_amount - fee_amount;
+
+    // Create a transaction that pays Alice
+    let address = alice.reveal_next_address(KeychainKind::External).address;
+    let change_address = client.get_new_address(None, None).unwrap().assume_checked();
+    let inputs = [CreateRawTransactionInput {
+        txid: unspent[0].txid,
+        vout: unspent[0].vout,
+        sequence: Some(0xFFFFFFFE),
+    }];
+    let mut outputs = HashMap::new();
+    outputs.insert(address.to_string(), destination_amount);
+    outputs.insert(change_address.to_string(), change_amount);
+    let tx1a = client
+        .create_raw_transaction(&inputs, &outputs, None, None)
+        .unwrap();
+    let tx1a = client
+        .sign_raw_transaction_with_wallet(tx1a.raw_hex(), None, None)
+        .unwrap()
+        .transaction()
+        .unwrap();
+
+    // Create a double-spent of tx1a
+    let address = client.get_new_address(None, None).unwrap().assume_checked();
+    let mut outputs = HashMap::new();
+    outputs.insert(address.to_string(), destination_amount);
+    outputs.insert(change_address.to_string(), change_amount);
+    let tx1b = client
+        .create_raw_transaction(&inputs, &outputs, None, None)
+        .unwrap();
+    let tx1b: Transaction = client
+        .sign_raw_transaction_with_wallet(tx1b.raw_hex(), None, None)
+        .unwrap()
+        .transaction()
+        .unwrap();
+
+    // Alice observes tx1a in the mempool
+    alice.apply_unconfirmed_txs(vec![(tx1a.clone(), 100)]);
+
+    // A block is create
+    // In this block tx1a is doublespent by tx1b
+    client.send_raw_transaction(tx1b.raw_hex()).unwrap();
+    let address = client
+        .get_new_address(None, None)
+        .expect("Failed to create address");
+    client
+        .generate_to_address(6, address.assume_checked_ref())
+        .expect("Failed to mine block");
+
+    // Apply the block to the w1
+    let mut emitter = Emitter::new(&env.bitcoind.client, alice.latest_checkpoint(), 0);
+    while let Some(ev) = emitter.next_block().unwrap() {
+        alice
+            .apply_block_connected_to(&ev.block, ev.block_height(), ev.connected_to())
+            .unwrap();
+    }
+
+    // We also add txb do the wallet
+    alice.apply_unconfirmed_txs([(tx1b.clone(), 101)]);
+
+    // I expect list_unspent to be empty.
+    // (tx1a) was double-spent
+    assert_eq!(alice.list_unspent().collect::<Vec<_>>(), vec![]);
+    //assert_eq!(alice.transactions().collect::<Vec<_>>(), vec![]);
+
+    Ok(())
+}