diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ca6b8d5c..7b714c78f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `verify` flag removed from `TransactionDetails`. - Add `get_internal_address` to allow you to get internal addresses just as you get external addresses. - added `ensure_addresses_cached` to `Wallet` to let offline wallets load and cache addresses in their database +- Add `is_spent` field to `LocalUtxo`; when we notice that a utxo has been spent we set `is_spent` field to true instead of deleting it from the db. ### Sync API change diff --git a/src/blockchain/compact_filters/mod.rs b/src/blockchain/compact_filters/mod.rs index e7e235b5b..eb1720592 100644 --- a/src/blockchain/compact_filters/mod.rs +++ b/src/blockchain/compact_filters/mod.rs @@ -163,11 +163,19 @@ impl CompactFiltersBlockchain { if let Some(previous_output) = database.get_previous_output(&input.previous_output)? { inputs_sum += previous_output.value; - if database.is_mine(&previous_output.script_pubkey)? { + // this output is ours, we have a path to derive it + if let Some((keychain, _)) = + database.get_path_from_script_pubkey(&previous_output.script_pubkey)? + { outgoing += previous_output.value; - debug!("{} input #{} is mine, removing from utxo", tx.txid(), i); - updates.del_utxo(&input.previous_output)?; + debug!("{} input #{} is mine, setting utxo as spent", tx.txid(), i); + updates.set_utxo(&LocalUtxo { + outpoint: input.previous_output, + txout: previous_output.clone(), + keychain, + is_spent: true, + })?; } } } @@ -185,6 +193,7 @@ impl CompactFiltersBlockchain { outpoint: OutPoint::new(tx.txid(), i as u32), txout: output.clone(), keychain, + is_spent: false, })?; incoming += output.value; diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index e4a798460..e8da3451b 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -249,7 +249,7 @@ impl WalletSync for RpcBlockchain { let mut list_txs_ids = HashSet::new(); for tx_result in list_txs.iter().filter(|t| { - // list_txs returns all conflicting tx we want to + // list_txs returns all conflicting txs, we want to // filter out replaced tx => unconfirmed and not in the mempool t.info.confirmations > 0 || self.client.get_mempool_entry(&t.info.txid).is_ok() }) { @@ -332,20 +332,23 @@ impl WalletSync for RpcBlockchain { value: u.amount.as_sat(), script_pubkey: u.script_pub_key, }, + is_spent: false, })), }, ) .collect::, Error>>()?; let spent: HashSet<_> = known_utxos.difference(¤t_utxos).collect(); - for s in spent { - debug!("removing utxo: {:?}", s); - db.del_utxo(&s.outpoint)?; + for utxo in spent { + debug!("setting as spent utxo: {:?}", utxo); + let mut spent_utxo = utxo.clone(); + spent_utxo.is_spent = true; + db.set_utxo(&spent_utxo)?; } let received: HashSet<_> = current_utxos.difference(&known_utxos).collect(); - for s in received { - debug!("adding utxo: {:?}", s); - db.set_utxo(s)?; + for utxo in received { + debug!("adding utxo: {:?}", utxo); + db.set_utxo(utxo)?; } for (keykind, index) in indexes { diff --git a/src/blockchain/script_sync.rs b/src/blockchain/script_sync.rs index 6fd3b9c81..ac78188e0 100644 --- a/src/blockchain/script_sync.rs +++ b/src/blockchain/script_sync.rs @@ -332,7 +332,23 @@ impl<'a, D: BatchDatabase> State<'a, D> { batch.del_tx(txid, true)?; } - // Set every tx we observed + let mut spent_utxos = HashSet::new(); + + // track all the spent utxos + 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 { + spent_utxos.insert(&input.previous_output); + } + } + + // 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 @@ -343,30 +359,22 @@ impl<'a, D: BatchDatabase> State<'a, D> { 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, + }; + batch.set_utxo(&LocalUtxo { - outpoint: OutPoint { - txid: finished_tx.txid, - vout: i as u32, - }, + outpoint, txout: output.clone(), keychain, + // Is this UTXO in the spent_utxos set? + is_spent: spent_utxos.get(&outpoint).is_some(), })?; } } - 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. - 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)?; - } + batch.set_tx(finished_tx)?; } for (keychain, last_active_index) in self.last_active_index { diff --git a/src/database/keyvalue.rs b/src/database/keyvalue.rs index 07499e9f1..5baa6c86a 100644 --- a/src/database/keyvalue.rs +++ b/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, }); 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 = 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, })) } } } @@ -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 = val + .get_mut("s") + .and_then(|s| s.take().as_bool()) + .unwrap_or(false); Ok(LocalUtxo { outpoint, txout, keychain, + is_spent, }) }) .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 = val + .get_mut("s") + .and_then(|s| s.take().as_bool()) + .unwrap_or(false); Ok(LocalUtxo { outpoint: *outpoint, txout, keychain, + is_spent, }) }) .transpose() diff --git a/src/database/memory.rs b/src/database/memory.rs index 0d5b7ef3d..1c8d1ccac 100644 --- a/src/database/memory.rs +++ b/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)), + ); 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) = b.downcast_ref().cloned().unwrap(); Ok(Some(LocalUtxo { outpoint: *outpoint, txout, keychain, + is_spent, })) } } @@ -326,11 +329,12 @@ impl Database for MemoryDatabase { .range::, _>((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) = v.downcast_ref().cloned().unwrap(); Ok(LocalUtxo { outpoint, txout, keychain, + is_spent, }) }) .collect() @@ -389,11 +393,12 @@ impl Database for MemoryDatabase { fn get_utxo(&self, outpoint: &OutPoint) -> Result, 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) = b.downcast_ref().cloned().unwrap(); LocalUtxo { outpoint: *outpoint, txout, keychain, + is_spent, } })) } @@ -526,6 +531,7 @@ macro_rules! populate_test_db { vout: vout as u32, }, keychain: $crate::KeychainKind::External, + is_spent: false, }) .unwrap(); } diff --git a/src/database/mod.rs b/src/database/mod.rs index 2318dcc9d..ab2fa9b24 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -316,6 +316,7 @@ pub mod test { txout, outpoint, keychain: KeychainKind::External, + is_spent: true, }; tree.set_utxo(&utxo).unwrap(); diff --git a/src/database/sqlite.rs b/src/database/sqlite.rs index b9c639bd1..8f57205cb 100644 --- a/src/database/sqlite.rs +++ b/src/database/sqlite.rs @@ -40,6 +40,7 @@ static MIGRATIONS: &[&str] = &[ "CREATE TABLE transaction_details (txid BLOB, timestamp INTEGER, received INTEGER, sent INTEGER, fee INTEGER, height INTEGER);", "INSERT INTO transaction_details SELECT txid, timestamp, received, sent, fee, height FROM transaction_details_old;", "DROP TABLE transaction_details_old;", + "ALTER TABLE utxos ADD COLUMN is_spent;", ]; /// Sqlite database stored on filesystem @@ -83,14 +84,16 @@ impl SqliteDatabase { vout: u32, txid: &[u8], script: &[u8], + is_spent: bool, ) -> Result { - 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) VALUES (:value, :keychain, :vout, :txid, :script, :is_spent)")?; statement.execute(named_params! { ":value": value, ":keychain": keychain, ":vout": vout, ":txid": txid, - ":script": script + ":script": script, + ":is_spent": is_spent, })?; Ok(self.connection.last_insert_rowid()) @@ -291,7 +294,7 @@ impl SqliteDatabase { fn select_utxos(&self) -> Result, Error> { let mut statement = self .connection - .prepare_cached("SELECT value, keychain, vout, txid, script FROM utxos")?; + .prepare_cached("SELECT value, keychain, vout, txid, script, is_spent FROM utxos")?; let mut utxos: Vec = vec![]; let mut rows = statement.query([])?; while let Some(row) = rows.next()? { @@ -300,6 +303,7 @@ impl SqliteDatabase { let vout = row.get(2)?; let txid: Vec = row.get(3)?; let script: Vec = row.get(4)?; + let is_spent: bool = row.get(5)?; let keychain: KeychainKind = serde_json::from_str(&keychain)?; @@ -310,19 +314,16 @@ impl SqliteDatabase { script_pubkey: script.into(), }, keychain, + is_spent, }) } Ok(utxos) } - fn select_utxo_by_outpoint( - &self, - txid: &[u8], - vout: u32, - ) -> Result, Error> { + fn select_utxo_by_outpoint(&self, txid: &[u8], vout: u32) -> Result, 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 FROM utxos WHERE txid=:txid AND vout=:vout", )?; let mut rows = statement.query(named_params! {":txid": txid,":vout": vout})?; match rows.next()? { @@ -331,9 +332,18 @@ impl SqliteDatabase { let keychain: String = row.get(1)?; let keychain: KeychainKind = serde_json::from_str(&keychain)?; let script: Vec = row.get(2)?; - let script: Script = script.into(); + let script_pubkey: Script = script.into(); + let is_spent: 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, + })) } None => Ok(None), } @@ -620,6 +630,7 @@ impl BatchOperations for SqliteDatabase { utxo.outpoint.vout, &utxo.outpoint.txid, utxo.txout.script_pubkey.as_bytes(), + utxo.is_spent, )?; Ok(()) } @@ -694,16 +705,9 @@ impl BatchOperations for SqliteDatabase { fn del_utxo(&mut self, outpoint: &OutPoint) -> Result, 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), } @@ -832,17 +836,7 @@ impl Database for SqliteDatabase { } fn get_utxo(&self, outpoint: &OutPoint) -> Result, 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, Error> { diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index 6ac91e88f..0cb61e8a1 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -1124,6 +1124,47 @@ macro_rules! bdk_blockchain_tests { assert_eq!(tx_2.received, 10_000); assert_eq!(tx_2.sent, 0); } + + #[test] + fn test_double_spend() { + // We create a tx and then we try to double spend it; BDK will always allow + // us to do so, as it never forgets about spent UTXOs + let (wallet, blockchain, 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(&blockchain, SyncOptions::default()).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 = blockchain.broadcast(&initial_tx).unwrap(); + wallet.sync(&blockchain, SyncOptions::default()).unwrap(); + for utxo in wallet.list_unspent().unwrap() { + // Making sure the TXO we just spent is not returned by list_unspent + assert!(utxo.outpoint != initial_tx.input[0].previous_output, "wallet displays spent txo in unspents"); + } + // We can still create a transaction double spending `initial_tx` + let mut builder = wallet.build_tx(); + builder + .add_utxo(initial_tx.input[0].previous_output) + .expect("Can't manually add an UTXO spent"); + test_client.generate(1, Some(node_addr)); + wallet.sync(&blockchain, SyncOptions::default()).unwrap(); + // Even after confirmation, we can still create a tx double spend it + let mut builder = wallet.build_tx(); + builder + .add_utxo(initial_tx.input[0].previous_output) + .expect("Can't manually add an UTXO spent"); + for utxo in wallet.list_unspent().unwrap() { + // Making sure the TXO we just spent is not returned by list_unspent + assert!(utxo.outpoint != initial_tx.input[0].previous_output, "wallet displays spent txo in unspents"); + } + } } }; diff --git a/src/types.rs b/src/types.rs index eadfc57b3..fc81bc278 100644 --- a/src/types.rs +++ b/src/types.rs @@ -131,6 +131,8 @@ pub struct LocalUtxo { pub txout: TxOut, /// Type of keychain pub keychain: KeychainKind, + /// Whether this UTXO is spent or not + pub is_spent: bool, } /// A [`Utxo`] with its `satisfaction_weight`. diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 19345bcdf..2b549b45d 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -569,6 +569,7 @@ mod test { script_pubkey: Script::new(), }, keychain: KeychainKind::External, + is_spent: false, }), } } @@ -596,6 +597,7 @@ mod test { script_pubkey: Script::new(), }, keychain: KeychainKind::External, + is_spent: false, }), }); } @@ -615,6 +617,7 @@ mod test { script_pubkey: Script::new(), }, keychain: KeychainKind::External, + is_spent: false, }), }; vec![utxo; utxos_number] diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index fd1054780..6986cf01a 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -387,7 +387,13 @@ where /// Note that this method only operates on the internal database, which first needs to be /// [`Wallet::sync`] manually. pub fn list_unspent(&self) -> Result, Error> { - self.database.borrow().iter_utxos() + Ok(self + .database + .borrow() + .iter_utxos()? + .into_iter() + .filter(|l| !l.is_spent) + .collect()) } /// Returns the `UTXO` owned by this wallet corresponding to `outpoint` if it exists in the @@ -879,6 +885,7 @@ where outpoint: txin.previous_output, txout, keychain, + is_spent: true, }; Ok(WeightedUtxo { diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index e03a93565..c45e619c1 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -838,6 +838,7 @@ mod test { }, txout: Default::default(), keychain: KeychainKind::External, + is_spent: false, }, LocalUtxo { outpoint: OutPoint { @@ -846,6 +847,7 @@ mod test { }, txout: Default::default(), keychain: KeychainKind::Internal, + is_spent: false, }, ] }