Allow using spent unconfirmed UTXOs in add_utxo

UTXOs will be deleted from the database only when the
transaction spending them is confirmed, this way you can
build a tx double-spending one in mempool using `add_utxo`.
listunspent won't return spent in mempool utxos, effectively excluding them from the
coin selection and balance calculation.
Closes #414

Author: danielabrozzoni

src/blockchain/script_sync.rs

@@ -319,41 +319,62 @@ impl<'a, D: BatchDatabase> State<'a, D> {
             batch.del_tx(txid, true)?;
         }
 
-        // Set every tx we observed
+        let mut spent_utxos = HashSet::new();
+        let mut spent_unconfirmed_utxos = HashSet::new();
+
+        // track all the spent and spent_unconfirmed utxos
         for finished_tx in &finished_txs {
             let tx = finished_tx
                 .transaction
                 .as_ref()
                 .expect("transaction will always be present here");
-            for (i, output) in tx.output.iter().enumerate() {
-                if let Some((keychain, _)) =
-                    self.db.get_path_from_script_pubkey(&output.script_pubkey)?
-                {
-                    // add utxos we own from the new transactions we've seen.
-                    batch.set_utxo(&LocalUtxo {
-                        outpoint: OutPoint {
-                            txid: finished_tx.txid,
-                            vout: i as u32,
-                        },
-                        txout: output.clone(),
-                        keychain,
-                    })?;
+            for input in &tx.input {
+                if finished_tx.confirmation_time.is_some() {
+                    spent_utxos.insert(&input.previous_output);
+                } else {
+                    spent_unconfirmed_utxos.insert(&input.previous_output);
                 }
             }
-            batch.set_tx(finished_tx)?;
         }
 
-        // we don't do this in the loop above since we may want to delete some of the utxos we
-        // just added in case there are new tranasactions that spend form each other.
+        // set every utxo we observed, unless it's already spent
+        // we don't do this in the loop above as we want to know all the spent outputs before
+        // adding the non-spent to the batch in case there are new tranasactions
+        // that spend form each other.
         for finished_tx in &finished_txs {
             let tx = finished_tx
                 .transaction
                 .as_ref()
                 .expect("transaction will always be present here");
-            for input in &tx.input {
-                // Delete any spent utxos
-                batch.del_utxo(&input.previous_output)?;
+            for (i, output) in tx.output.iter().enumerate() {
+                if let Some((keychain, _)) =
+                    self.db.get_path_from_script_pubkey(&output.script_pubkey)?
+                {
+                    // add utxos we own from the new transactions we've seen.
+                    let outpoint = OutPoint {
+                        txid: finished_tx.txid,
+                        vout: i as u32,
+                    };
+
+                    // If it's not spent, we add it to the batch
+                    if spent_utxos.get(&outpoint).is_none() {
+                        batch.set_utxo(&LocalUtxo {
+                            outpoint,
+                            txout: output.clone(),
+                            keychain,
+                            // Is this UTXO spent in a tx still in mempool?
+                            is_spent_unconfirmed: spent_unconfirmed_utxos.contains(&outpoint),
+                        })?;
+                    }
+                }
             }
+
+            batch.set_tx(finished_tx)?;
+        }
+
+        // delete all the spent utxos
+        for spent_utxo in spent_utxos {
+            batch.del_utxo(spent_utxo)?;
         }
 
         for (keychain, last_active_index) in self.last_active_index {

src/database/keyvalue.rs

@@ -43,6 +43,7 @@ macro_rules! impl_batch_operations {
             let value = json!({
                 "t": utxo.txout,
                 "i": utxo.keychain,
+                "s": utxo.is_spent_unconfirmed,
             });
             self.insert(key, serde_json::to_vec(&value)?)$($after_insert)*;
 
@@ -125,8 +126,9 @@ macro_rules! impl_batch_operations {
                     let mut val: serde_json::Value = serde_json::from_slice(&b)?;
                     let txout = serde_json::from_value(val["t"].take())?;
                     let keychain = serde_json::from_value(val["i"].take())?;
+                    let is_spent_unconfirmed = val.get_mut("s").and_then(|s| s.take().as_bool()).unwrap_or(false);
 
-                    Ok(Some(LocalUtxo { outpoint: outpoint.clone(), txout, keychain }))
+                    Ok(Some(LocalUtxo { outpoint: outpoint.clone(), txout, keychain, is_spent_unconfirmed, }))
                 }
             }
         }
@@ -246,11 +248,16 @@ impl Database for Tree {
                 let mut val: serde_json::Value = serde_json::from_slice(&v)?;
                 let txout = serde_json::from_value(val["t"].take())?;
                 let keychain = serde_json::from_value(val["i"].take())?;
+                let is_spent_unconfirmed = val
+                    .get_mut("s")
+                    .and_then(|s| s.take().as_bool())
+                    .unwrap_or(false);
 
                 Ok(LocalUtxo {
                     outpoint,
                     txout,
                     keychain,
+                    is_spent_unconfirmed,
                 })
             })
             .collect()
@@ -314,11 +321,16 @@ impl Database for Tree {
                 let mut val: serde_json::Value = serde_json::from_slice(&b)?;
                 let txout = serde_json::from_value(val["t"].take())?;
                 let keychain = serde_json::from_value(val["i"].take())?;
+                let is_spent_unconfirmed = val
+                    .get_mut("s")
+                    .and_then(|s| s.take().as_bool())
+                    .unwrap_or(false);
 
                 Ok(LocalUtxo {
                     outpoint: *outpoint,
                     txout,
                     keychain,
+                    is_spent_unconfirmed,
                 })
             })
             .transpose()

src/database/memory.rs

@@ -150,8 +150,10 @@ impl BatchOperations for MemoryDatabase {
 
     fn set_utxo(&mut self, utxo: &LocalUtxo) -> Result<(), Error> {
         let key = MapKey::Utxo(Some(&utxo.outpoint)).as_map_key();
-        self.map
-            .insert(key, Box::new((utxo.txout.clone(), utxo.keychain)));
+        self.map.insert(
+            key,
+            Box::new((utxo.txout.clone(), utxo.keychain, utxo.is_spent_unconfirmed)),
+        );
 
         Ok(())
     }
@@ -228,11 +230,12 @@ impl BatchOperations for MemoryDatabase {
         match res {
             None => Ok(None),
             Some(b) => {
-                let (txout, keychain) = b.downcast_ref().cloned().unwrap();
+                let (txout, keychain, is_spent_unconfirmed) = b.downcast_ref().cloned().unwrap();
                 Ok(Some(LocalUtxo {
                     outpoint: *outpoint,
                     txout,
                     keychain,
+                    is_spent_unconfirmed,
                 }))
             }
         }
@@ -326,11 +329,12 @@ impl Database for MemoryDatabase {
             .range::<Vec<u8>, _>((Included(&key), Excluded(&after(&key))))
             .map(|(k, v)| {
                 let outpoint = deserialize(&k[1..]).unwrap();
-                let (txout, keychain) = v.downcast_ref().cloned().unwrap();
+                let (txout, keychain, is_spent_unconfirmed) = v.downcast_ref().cloned().unwrap();
                 Ok(LocalUtxo {
                     outpoint,
                     txout,
                     keychain,
+                    is_spent_unconfirmed,
                 })
             })
             .collect()
@@ -389,11 +393,12 @@ impl Database for MemoryDatabase {
     fn get_utxo(&self, outpoint: &OutPoint) -> Result<Option<LocalUtxo>, Error> {
         let key = MapKey::Utxo(Some(outpoint)).as_map_key();
         Ok(self.map.get(&key).map(|b| {
-            let (txout, keychain) = b.downcast_ref().cloned().unwrap();
+            let (txout, keychain, is_spent_unconfirmed) = b.downcast_ref().cloned().unwrap();
             LocalUtxo {
                 outpoint: *outpoint,
                 txout,
                 keychain,
+                is_spent_unconfirmed,
             }
         }))
     }
@@ -527,6 +532,7 @@ macro_rules! populate_test_db {
                     vout: vout as u32,
                 },
                 keychain: $crate::KeychainKind::External,
+                is_spent_unconfirmed: false,
             })
             .unwrap();
         }

src/database/mod.rs

@@ -316,6 +316,7 @@ pub mod test {
             txout,
             outpoint,
             keychain: KeychainKind::External,
+            is_spent_unconfirmed: true,
         };
 
         tree.set_utxo(&utxo).unwrap();

src/database/sqlite.rs

@@ -35,7 +35,8 @@ static MIGRATIONS: &[&str] = &[
     "CREATE UNIQUE INDEX idx_indices_keychain ON last_derivation_indices(keychain);",
     "CREATE TABLE checksums (keychain TEXT, checksum BLOB);",
     "CREATE INDEX idx_checksums_keychain ON checksums(keychain);",
-    "CREATE TABLE sync_time (id INTEGER PRIMARY KEY, height INTEGER, timestamp INTEGER);"
+    "CREATE TABLE sync_time (id INTEGER PRIMARY KEY, height INTEGER, timestamp INTEGER);",
+    "ALTER TABLE utxos ADD COLUMN is_spent_unconfirmed;",
 ];
 
 /// Sqlite database stored on filesystem
@@ -79,14 +80,16 @@ impl SqliteDatabase {
         vout: u32,
         txid: &[u8],
         script: &[u8],
+        is_spent_unconfirmed: bool,
     ) -> Result<i64, Error> {
-        let mut statement = self.connection.prepare_cached("INSERT INTO utxos (value, keychain, vout, txid, script) VALUES (:value, :keychain, :vout, :txid, :script)")?;
+        let mut statement = self.connection.prepare_cached("INSERT INTO utxos (value, keychain, vout, txid, script, is_spent_unconfirmed) VALUES (:value, :keychain, :vout, :txid, :script, :is_spent_unconfirmed)")?;
         statement.execute(named_params! {
             ":value": value,
             ":keychain": keychain,
             ":vout": vout,
             ":txid": txid,
-            ":script": script
+            ":script": script,
+            ":is_spent_unconfirmed": is_spent_unconfirmed,
         })?;
 
         Ok(self.connection.last_insert_rowid())
@@ -287,9 +290,9 @@ impl SqliteDatabase {
     }
 
     fn select_utxos(&self) -> Result<Vec<LocalUtxo>, Error> {
-        let mut statement = self
-            .connection
-            .prepare_cached("SELECT value, keychain, vout, txid, script FROM utxos")?;
+        let mut statement = self.connection.prepare_cached(
+            "SELECT value, keychain, vout, txid, script, is_spent_unconfirmed FROM utxos",
+        )?;
         let mut utxos: Vec<LocalUtxo> = vec![];
         let mut rows = statement.query([])?;
         while let Some(row) = rows.next()? {
@@ -298,6 +301,7 @@ impl SqliteDatabase {
             let vout = row.get(2)?;
             let txid: Vec<u8> = row.get(3)?;
             let script: Vec<u8> = row.get(4)?;
+            let is_spent_unconfirmed: bool = row.get(5)?;
 
             let keychain: KeychainKind = serde_json::from_str(&keychain)?;
 
@@ -308,19 +312,16 @@ impl SqliteDatabase {
                     script_pubkey: script.into(),
                 },
                 keychain,
+                is_spent_unconfirmed,
             })
         }
 
         Ok(utxos)
     }
 
-    fn select_utxo_by_outpoint(
-        &self,
-        txid: &[u8],
-        vout: u32,
-    ) -> Result<Option<(u64, KeychainKind, Script)>, Error> {
+    fn select_utxo_by_outpoint(&self, txid: &[u8], vout: u32) -> Result<Option<LocalUtxo>, Error> {
         let mut statement = self.connection.prepare_cached(
-            "SELECT value, keychain, script FROM utxos WHERE txid=:txid AND vout=:vout",
+            "SELECT value, keychain, script, is_spent_unconfirmed FROM utxos WHERE txid=:txid AND vout=:vout",
         )?;
         let mut rows = statement.query(named_params! {":txid": txid,":vout": vout})?;
         match rows.next()? {
@@ -329,9 +330,18 @@ impl SqliteDatabase {
                 let keychain: String = row.get(1)?;
                 let keychain: KeychainKind = serde_json::from_str(&keychain)?;
                 let script: Vec<u8> = row.get(2)?;
-                let script: Script = script.into();
+                let script_pubkey: Script = script.into();
+                let is_spent_unconfirmed: bool = row.get(3)?;
 
-                Ok(Some((value, keychain, script)))
+                Ok(Some(LocalUtxo {
+                    outpoint: OutPoint::new(deserialize(&txid)?, vout),
+                    txout: TxOut {
+                        value,
+                        script_pubkey,
+                    },
+                    keychain,
+                    is_spent_unconfirmed,
+                }))
             }
             None => Ok(None),
         }
@@ -624,6 +634,7 @@ impl BatchOperations for SqliteDatabase {
             utxo.outpoint.vout,
             &utxo.outpoint.txid,
             utxo.txout.script_pubkey.as_bytes(),
+            utxo.is_spent_unconfirmed,
         )?;
         Ok(())
     }
@@ -698,16 +709,9 @@ impl BatchOperations for SqliteDatabase {
 
     fn del_utxo(&mut self, outpoint: &OutPoint) -> Result<Option<LocalUtxo>, Error> {
         match self.select_utxo_by_outpoint(&outpoint.txid, outpoint.vout)? {
-            Some((value, keychain, script_pubkey)) => {
+            Some(local_utxo) => {
                 self.delete_utxo_by_outpoint(&outpoint.txid, outpoint.vout)?;
-                Ok(Some(LocalUtxo {
-                    outpoint: *outpoint,
-                    txout: TxOut {
-                        value,
-                        script_pubkey,
-                    },
-                    keychain,
-                }))
+                Ok(Some(local_utxo))
             }
             None => Ok(None),
         }
@@ -836,17 +840,7 @@ impl Database for SqliteDatabase {
     }
 
     fn get_utxo(&self, outpoint: &OutPoint) -> Result<Option<LocalUtxo>, Error> {
-        match self.select_utxo_by_outpoint(&outpoint.txid, outpoint.vout)? {
-            Some((value, keychain, script_pubkey)) => Ok(Some(LocalUtxo {
-                outpoint: *outpoint,
-                txout: TxOut {
-                    value,
-                    script_pubkey,
-                },
-                keychain,
-            })),
-            None => Ok(None),
-        }
+        self.select_utxo_by_outpoint(&outpoint.txid, outpoint.vout)
     }
 
     fn get_raw_tx(&self, txid: &Txid) -> Result<Option<Transaction>, Error> {

src/testutils/blockchain_tests.rs

@@ -1073,6 +1073,50 @@ macro_rules! bdk_blockchain_tests {
                 let taproot_balance = taproot_wallet_client.get_balance(None, None).unwrap();
                 assert_eq!(taproot_balance.as_sat(), 25_000, "node has incorrect taproot wallet balance");
             }
+
+            #[test]
+            fn test_double_spend() {
+                // We create a tx and then we try to double spend it; bdk will allow
+                // us to do so while the initial tx is still in mempool, even if it's not
+                // RBF enabled
+                let (wallet, descriptors, mut test_client) = init_single_sig();
+                let node_addr = test_client.get_node_address(None);
+                let _ = test_client.receive(testutils! {
+                    @tx ( (@external descriptors, 0) => 50_000 )
+                });
+
+                wallet.sync(noop_progress(), None).unwrap();
+                let mut builder = wallet.build_tx();
+                builder.add_recipient(node_addr.script_pubkey(), 25_000);
+                let (mut psbt, _details) = builder.finish().unwrap();
+                let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
+                assert!(finalized, "Cannot finalize transaction");
+                let initial_tx = psbt.extract_tx();
+                let _sent_txid = wallet.broadcast(&initial_tx).unwrap();
+                wallet.sync(noop_progress(), None).unwrap();
+                // We can still create a transaction double spending `initial_tx`, as we keep
+                // the utxos spent in mempool in our database until confirmation
+                let mut builder = wallet.build_tx();
+                builder
+                    .add_utxo(initial_tx.input[0].previous_output)
+                    .expect("Can't manually add an UTXO spent in an unconfirmed tx");
+                test_client.generate(1, Some(node_addr));
+                wallet.sync(noop_progress(), None).unwrap();
+                // Now that the transaction is confirmed, we can't create one double spending it
+                let mut builder = wallet.build_tx();
+                assert!(builder
+                    .add_utxo(initial_tx.input[0].previous_output)
+                    .is_err(), "The UTXO should be unknown");
+                // Now we invalidate the last block, making `initial_tx` unconfirmed
+                test_client.invalidate(1);
+                // `initial_tx` is in mempool, we have the info for creating a tx
+                // double-spending it
+                wallet.sync(noop_progress(), None).unwrap();
+                let mut builder = wallet.build_tx();
+                builder
+                    .add_utxo(initial_tx.input[0].previous_output)
+                    .expect("Can't manually add an UTXO spent in an unconfirmed tx");
+            }
         }
     };