From 94ee94edbdcdd917df131ad87893406f5f8597a2 Mon Sep 17 00:00:00 2001 From: edouard Date: Fri, 16 Sep 2022 15:25:51 +0200 Subject: [PATCH 1/8] Add blocktime and spent_at to coins table Spent coins are coins where spent_txid and spent_at are both not null. --- src/bitcoin/d/mod.rs | 10 ++++- src/bitcoin/mod.rs | 32 ++++++++++----- src/bitcoin/poller/looper.rs | 12 +++--- src/commands/mod.rs | 6 +++ src/database/mod.rs | 73 +++++++++++++++++++++-------------- src/database/sqlite/mod.rs | 17 +++++--- src/database/sqlite/schema.rs | 19 ++++++--- src/testutils.rs | 34 +++++++++++----- 8 files changed, 137 insertions(+), 66 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 2127cdb4..60ce4fd2 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -751,6 +751,7 @@ impl From for LSBlockRes { #[derive(Debug, Clone, Copy)] pub struct GetTxRes { pub block_height: Option, + pub block_time: Option, } impl From for GetTxRes { @@ -759,6 +760,13 @@ impl From for GetTxRes { .get("blockheight") .and_then(Json::as_i64) .map(|bh| bh as i32); - GetTxRes { block_height } + let block_time = json + .get("blocktime") + .and_then(Json::as_u64) + .map(|bt| bt as u32); + GetTxRes { + block_height, + block_time, + } } } diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 650d5572..25dcc522 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -34,11 +34,14 @@ pub trait BitcoinInterface: Send { /// Get coins received since the specified tip. fn received_coins(&self, tip: &BlockChainTip) -> Vec; - /// Get all coins that were confirmed, and at what height. - fn confirmed_coins(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, i32)>; + /// Get all coins that were confirmed, and at what height and time. + fn confirmed_coins( + &self, + outpoints: &[bitcoin::OutPoint], + ) -> Vec<(bitcoin::OutPoint, i32, u32)>; - /// Get all coins that were spent, and the spending txid. - fn spent_coins( + /// Get all coins that are being spent, and the spending txid. + fn spending_coins( &self, outpoints: &[bitcoin::OutPoint], ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)>; @@ -91,14 +94,19 @@ impl BitcoinInterface for d::BitcoinD { .collect() } - fn confirmed_coins(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, i32)> { + fn confirmed_coins( + &self, + outpoints: &[bitcoin::OutPoint], + ) -> Vec<(bitcoin::OutPoint, i32, u32)> { let mut confirmed = Vec::with_capacity(outpoints.len()); for op in outpoints { // TODO: batch those calls to gettransaction if let Some(res) = self.get_transaction(&op.txid) { if let Some(h) = res.block_height { - confirmed.push((*op, h)); + if let Some(t) = res.block_time { + confirmed.push((*op, h, t)); + } } } else { log::error!("Transaction not in wallet for coin '{}'.", op); @@ -108,7 +116,7 @@ impl BitcoinInterface for d::BitcoinD { confirmed } - fn spent_coins( + fn spending_coins( &self, outpoints: &[bitcoin::OutPoint], ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { @@ -126,6 +134,7 @@ impl BitcoinInterface for d::BitcoinD { ); bitcoin::Txid::from_slice(&[0; 32][..]).unwrap() }; + spent.push((*op, spending_txid)); } } @@ -156,15 +165,18 @@ impl BitcoinInterface for sync::Arc> self.lock().unwrap().received_coins(tip) } - fn confirmed_coins(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, i32)> { + fn confirmed_coins( + &self, + outpoints: &[bitcoin::OutPoint], + ) -> Vec<(bitcoin::OutPoint, i32, u32)> { self.lock().unwrap().confirmed_coins(outpoints) } - fn spent_coins( + fn spending_coins( &self, outpoints: &[bitcoin::OutPoint], ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { - self.lock().unwrap().spent_coins(outpoints) + self.lock().unwrap().spending_coins(outpoints) } } diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 66e0c2d0..2cffcaa1 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -13,8 +13,8 @@ use miniscript::bitcoin; #[derive(Debug, Clone)] struct UpdatedCoins { pub received: Vec, - pub confirmed: Vec<(bitcoin::OutPoint, i32)>, - pub spent: Vec<(bitcoin::OutPoint, bitcoin::Txid)>, + pub confirmed: Vec<(bitcoin::OutPoint, i32, u32)>, + pub spending: Vec<(bitcoin::OutPoint, bitcoin::Txid)>, } // Update the state of our coins. There may be new unspent, and existing ones may become confirmed @@ -40,7 +40,9 @@ fn update_coins( amount, derivation_index, block_height: None, + block_time: None, spend_txid: None, + spent_at: None, }; received.push(coin); } @@ -83,12 +85,12 @@ fn update_coins( } }) .collect(); - let spent = bit.spent_coins(&to_be_spent); + let spending = bit.spending_coins(&to_be_spent); UpdatedCoins { received, confirmed, - spent, + spending, } } @@ -136,7 +138,7 @@ fn updates(bit: &impl BitcoinInterface, db: &impl DatabaseInterface) { // updates up to this block. But not more. db_conn.new_unspent_coins(&updated_coins.received); db_conn.confirm_coins(&updated_coins.confirmed); - db_conn.spend_coins(&updated_coins.spent); + db_conn.spend_coins(&updated_coins.spending); if let Some(tip) = new_tip { db_conn.update_tip(&tip); } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 32487278..5bdb6278 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -568,9 +568,11 @@ mod tests { db_conn.new_unspent_coins(&[Coin { outpoint: dummy_op, block_height: None, + block_time: None, amount: bitcoin::Amount::from_sat(100_000), derivation_index: bip32::ChildNumber::from(13), spend_txid: None, + spent_at: None, }]); let res = control.create_spend(&[dummy_op], &destinations, 1).unwrap(); let tx = res.psbt.global.unsigned_tx; @@ -660,16 +662,20 @@ mod tests { Coin { outpoint: dummy_op_a, block_height: None, + block_time: None, amount: bitcoin::Amount::from_sat(100_000), derivation_index: bip32::ChildNumber::from(13), spend_txid: None, + spent_at: None, }, Coin { outpoint: dummy_op_b, block_height: None, + block_time: None, amount: bitcoin::Amount::from_sat(115_680), derivation_index: bip32::ChildNumber::from(34), spend_txid: None, + spent_at: None, }, ]); diff --git a/src/database/mod.rs b/src/database/mod.rs index fc6d4c44..ff2fafa1 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -60,12 +60,15 @@ pub trait DatabaseConnection { /// Store new UTxOs. Coins must not already be in database. fn new_unspent_coins<'a>(&mut self, coins: &[Coin]); - /// Mark a set of coins as being confirmed at a specified height. - fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32)]); + /// Mark a set of coins as being confirmed at a specified height and block time. + fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32, u32)]); /// Mark a set of coins as being spent by a specified txid. fn spend_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)]); + /// Mark a set of coins as spent by a specified txid at a specified block time. + fn confirm_spend<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid, u32)]); + /// Get specific coins from the database. fn coins_by_outpoints( &mut self, @@ -84,33 +87,6 @@ pub trait DatabaseConnection { fn delete_spend(&mut self, txid: &bitcoin::Txid); } -// FIXME: if possible, avoid reallocating. -fn db_coins_into_coins(db_coins: Vec) -> HashMap { - db_coins - .into_iter() - .map(|db_coin| { - let DbCoin { - outpoint, - block_height, - amount, - derivation_index, - spend_txid, - .. - } = db_coin; - ( - outpoint, - Coin { - outpoint, - block_height, - amount, - derivation_index, - spend_txid, - }, - ) - }) - .collect() -} - impl DatabaseConnection for SqliteConn { fn chain_tip(&mut self) -> Option { match self.db_tip() { @@ -147,7 +123,7 @@ impl DatabaseConnection for SqliteConn { self.new_unspent_coins(coins) } - fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32)]) { + fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32, u32)]) { self.confirm_coins(outpoints) } @@ -155,6 +131,10 @@ impl DatabaseConnection for SqliteConn { self.spend_coins(outpoints) } + fn confirm_spend<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid, u32)]) { + self.confirm_spend(outpoints) + } + fn derivation_index_by_address( &mut self, address: &bitcoin::Address, @@ -190,13 +170,46 @@ impl DatabaseConnection for SqliteConn { } } +// FIXME: if possible, avoid reallocating. +fn db_coins_into_coins(db_coins: Vec) -> HashMap { + db_coins + .into_iter() + .map(|db_coin| { + let DbCoin { + outpoint, + block_height, + block_time, + amount, + derivation_index, + spend_txid, + spent_at, + .. + } = db_coin; + ( + outpoint, + Coin { + outpoint, + block_height, + block_time, + amount, + derivation_index, + spend_txid, + spent_at, + }, + ) + }) + .collect() +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Coin { pub outpoint: bitcoin::OutPoint, pub block_height: Option, + pub block_time: Option, pub amount: bitcoin::Amount, pub derivation_index: bip32::ChildNumber, pub spend_txid: Option, + pub spent_at: Option, } impl std::hash::Hash for Coin { diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index a076ac7a..a4e672e3 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -290,13 +290,13 @@ impl SqliteConn { /// Mark a set of coins as confirmed. pub fn confirm_coins<'a>( &mut self, - outpoints: impl IntoIterator, + outpoints: impl IntoIterator, ) { db_exec(&mut self.conn, |db_tx| { - for (outpoint, height) in outpoints { + for (outpoint, height, time) in outpoints { db_tx.execute( - "UPDATE coins SET blockheight = ?1 WHERE txid = ?2 AND vout = ?3", - rusqlite::params![height, outpoint.txid.to_vec(), outpoint.vout,], + "UPDATE coins SET blockheight = ?1, blocktime = ?2 WHERE txid = ?3 AND vout = ?4", + rusqlite::params![height, time, outpoint.txid.to_vec(), outpoint.vout,], )?; } @@ -528,9 +528,11 @@ mod tests { ) .unwrap(), block_height: None, + block_time: None, amount: bitcoin::Amount::from_sat(98765), derivation_index: bip32::ChildNumber::from_normal_idx(10).unwrap(), spend_txid: None, + spent_at: None, }; conn.new_unspent_coins(&[coin_a.clone()]); // On 1.48, arrays aren't IntoIterator assert_eq!(conn.unspent_coins()[0].outpoint, coin_a.outpoint); @@ -547,9 +549,11 @@ mod tests { ) .unwrap(), block_height: None, + block_time: None, amount: bitcoin::Amount::from_sat(1111), derivation_index: bip32::ChildNumber::from_normal_idx(103).unwrap(), spend_txid: None, + spent_at: None, }; conn.new_unspent_coins(&[coin_b.clone()]); let outpoints: HashSet = conn @@ -580,10 +584,13 @@ mod tests { // Now if we confirm one, it'll be marked as such. let height = 174500; - conn.confirm_coins(&[(coin_a.outpoint, height)]); + let time = 174500; + conn.confirm_coins(&[(coin_a.outpoint, height, time)]); let coins = conn.unspent_coins(); assert_eq!(coins[0].block_height, Some(height)); + assert_eq!(coins[0].block_time, Some(time)); assert!(coins[1].block_height.is_none()); + assert!(coins[1].block_time.is_none()); // Now if we spend one, we'll only get the other one. conn.spend_coins(&[( diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index fe6b662f..e0478eaa 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -35,11 +35,14 @@ CREATE TABLE coins ( id INTEGER PRIMARY KEY NOT NULL, wallet_id INTEGER NOT NULL, blockheight INTEGER, + blocktime INTEGER, txid BLOB NOT NULL, vout INTEGER NOT NULL, amount_sat INTEGER NOT NULL, derivation_index INTEGER NOT NULL, spend_txid BLOB, + /* Time of the block containing the transaction spending the coin, NULL if not confirmed */ + spent_at INTEGER, UNIQUE (txid, vout), FOREIGN KEY (wallet_id) REFERENCES wallets (id) ON UPDATE RESTRICT @@ -129,9 +132,11 @@ pub struct DbCoin { pub wallet_id: i64, pub outpoint: bitcoin::OutPoint, pub block_height: Option, + pub block_time: Option, pub amount: bitcoin::Amount, pub derivation_index: bip32::ChildNumber, pub spend_txid: Option, + pub spent_at: Option, } impl std::hash::Hash for DbCoin { @@ -148,28 +153,32 @@ impl TryFrom<&rusqlite::Row<'_>> for DbCoin { let wallet_id = row.get(1)?; let block_height = row.get(2)?; - let txid: Vec = row.get(3)?; + let block_time = row.get(3)?; + let txid: Vec = row.get(4)?; let txid: bitcoin::Txid = encode::deserialize(&txid).expect("We only store valid txids"); - let vout = row.get(4)?; + let vout = row.get(5)?; let outpoint = bitcoin::OutPoint { txid, vout }; - let amount = row.get(5)?; + let amount = row.get(6)?; let amount = bitcoin::Amount::from_sat(amount); - let der_idx: u32 = row.get(6)?; + let der_idx: u32 = row.get(7)?; let derivation_index = bip32::ChildNumber::from(der_idx); - let spend_txid: Option> = row.get(7)?; + let spend_txid: Option> = row.get(8)?; let spend_txid = spend_txid.map(|txid| encode::deserialize(&txid).expect("We only store valid txids")); + let spent_at = row.get(9)?; Ok(DbCoin { id, wallet_id, outpoint, block_height, + block_time, amount, derivation_index, spend_txid, + spent_at, }) } } diff --git a/src/testutils.rs b/src/testutils.rs index 2aaa0883..1ef694c7 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -48,11 +48,11 @@ impl BitcoinInterface for DummyBitcoind { Vec::new() } - fn confirmed_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, i32)> { + fn confirmed_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, i32, u32)> { Vec::new() } - fn spent_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { + fn spending_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { Vec::new() } } @@ -121,21 +121,35 @@ impl DatabaseConnection for DummyDbConn { } } - fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32)]) { - for (op, height) in outpoints { + fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32, u32)]) { + for (op, height, time) in outpoints { let mut db = self.db.write().unwrap(); - let h = &mut db.coins.get_mut(op).unwrap().block_height; - assert!(h.is_none()); - *h = Some(*height); + let coin = &mut db.coins.get_mut(op).unwrap(); + assert!(coin.block_height.is_none()); + assert!(coin.block_time.is_none()); + coin.block_height = Some(*height); + coin.block_time = Some(*time); } } fn spend_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)]) { for (op, spend_txid) in outpoints { let mut db = self.db.write().unwrap(); - let spender = &mut db.coins.get_mut(op).unwrap().spend_txid; - assert!(spender.is_none()); - *spender = Some(*spend_txid); + let spent = &mut db.coins.get_mut(op).unwrap(); + assert!(spent.spend_txid.is_none()); + assert!(spent.spent_at.is_none()); + spent.spend_txid = Some(*spend_txid); + } + } + + fn confirm_spend<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid, u32)]) { + for (op, spend_txid, time) in outpoints { + let mut db = self.db.write().unwrap(); + let spent = &mut db.coins.get_mut(op).unwrap(); + assert!(!spent.spend_txid.is_none()); + assert!(spent.spent_at.is_none()); + spent.spend_txid = Some(*spend_txid); + spent.spent_at = Some(*time); } } From 3cf6bcbb98fc0034bc46ed58564e8aa4a1b6ffa6 Mon Sep 17 00:00:00 2001 From: edouard Date: Fri, 16 Sep 2022 17:26:14 +0200 Subject: [PATCH 2/8] add spent_coins to bitcoind poller --- src/bitcoin/mod.rs | 47 +++++++++++++++++++++ src/bitcoin/poller/looper.rs | 15 ++++++- src/commands/mod.rs | 2 +- src/database/mod.rs | 81 ++++++++++++++++++++---------------- src/database/sqlite/mod.rs | 67 ++++++++++++++++++++++++++--- src/testutils.rs | 19 ++++++++- 6 files changed, 186 insertions(+), 45 deletions(-) diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 25dcc522..4141edd9 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -6,6 +6,7 @@ pub mod poller; use d::LSBlockEntry; +use std::collections::HashMap; use std::sync; use miniscript::bitcoin::{self, hashes::Hash}; @@ -45,6 +46,12 @@ pub trait BitcoinInterface: Send { &self, outpoints: &[bitcoin::OutPoint], ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)>; + + /// Get all coins that are spent with the final spend tx txid and blocktime. + fn spent_coins( + &self, + outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)], + ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, u32)>; } impl BitcoinInterface for d::BitcoinD { @@ -141,6 +148,39 @@ impl BitcoinInterface for d::BitcoinD { spent } + + fn spent_coins( + &self, + outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)], + ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, u32)> { + let mut spent = Vec::with_capacity(outpoints.len()); + + let mut cache: HashMap> = HashMap::new(); + + for (op, txid) in outpoints { + let tx: Option<&d::GetTxRes> = match cache.get(txid) { + Some(tx) => tx.as_ref(), + None => { + let tx = self.get_transaction(txid); + cache.insert(*txid, tx); + cache.get(txid).unwrap().as_ref() + } + }; + + if let Some(tx) = tx { + if let Some(block_height) = tx.block_height { + if block_height > 1 { + spent.push((*op, *txid, tx.block_time.expect("Spend is confirmed"))) + } + } else { + // TODO: handle the case where new transaction which txid is not the + // coin spending_txid, spent the coin. + } + } + } + + spent + } } // FIXME: do we need to repeat the entire trait implemenation? Isn't there a nicer way? @@ -178,6 +218,13 @@ impl BitcoinInterface for sync::Arc> ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { self.lock().unwrap().spending_coins(outpoints) } + + fn spent_coins( + &self, + outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)], + ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, u32)> { + self.lock().unwrap().spent_coins(outpoints) + } } // FIXME: We could avoid this type (and all the conversions entailing allocations) if bitcoind diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 2cffcaa1..96bb7fd9 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -15,6 +15,7 @@ struct UpdatedCoins { pub received: Vec, pub confirmed: Vec<(bitcoin::OutPoint, i32, u32)>, pub spending: Vec<(bitcoin::OutPoint, bitcoin::Txid)>, + pub spent: Vec<(bitcoin::OutPoint, bitcoin::Txid, u32)>, } // Update the state of our coins. There may be new unspent, and existing ones may become confirmed @@ -27,7 +28,7 @@ fn update_coins( previous_tip: &BlockChainTip, ) -> UpdatedCoins { // Start by fetching newly received coins. - let curr_coins = db_conn.unspent_coins(); + let curr_coins = db_conn.list_unspent_coins(); let mut received = Vec::new(); for utxo in bit.received_coins(&previous_tip) { if let Some(derivation_index) = db_conn.derivation_index_by_address(&utxo.address) { @@ -87,10 +88,21 @@ fn update_coins( .collect(); let spending = bit.spending_coins(&to_be_spent); + // We need to confirm coins that are currently spending and which transactions are now in a + // block. + let mut spending_coins: Vec<(bitcoin::OutPoint, bitcoin::Txid)> = db_conn + .list_spending_coins() + .values() + .map(|coin| (coin.outpoint, coin.spend_txid.expect("Coin is spending"))) + .collect(); + spending_coins.extend(&spending); + let spent = bit.spent_coins(spending_coins.as_slice()); + UpdatedCoins { received, confirmed, spending, + spent, } } @@ -139,6 +151,7 @@ fn updates(bit: &impl BitcoinInterface, db: &impl DatabaseInterface) { db_conn.new_unspent_coins(&updated_coins.received); db_conn.confirm_coins(&updated_coins.confirmed); db_conn.spend_coins(&updated_coins.spending); + db_conn.confirm_spend(&updated_coins.spent); if let Some(tip) = new_tip { db_conn.update_tip(&tip); } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 5bdb6278..b7fd2782 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -197,7 +197,7 @@ impl DaemonControl { pub fn list_coins(&self) -> ListCoinsResult { let mut db_conn = self.db.connection(); let coins: Vec = db_conn - .unspent_coins() + .list_unspent_coins() // Can't use into_values as of Rust 1.48 .into_iter() .map(|(_, coin)| { diff --git a/src/database/mod.rs b/src/database/mod.rs index ff2fafa1..f0b261bc 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -55,7 +55,10 @@ pub trait DatabaseConnection { ) -> Option; /// Get all UTxOs. - fn unspent_coins(&mut self) -> HashMap; + fn list_unspent_coins(&mut self) -> HashMap; + + /// List coins that are being spent and whose spending transaction is still unconfirmed. + fn list_spending_coins(&mut self) -> HashMap; /// Store new UTxOs. Coins must not already be in database. fn new_unspent_coins<'a>(&mut self, coins: &[Coin]); @@ -63,7 +66,7 @@ pub trait DatabaseConnection { /// Mark a set of coins as being confirmed at a specified height and block time. fn confirm_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, i32, u32)]); - /// Mark a set of coins as being spent by a specified txid. + /// Mark a set of coins as being spent by a specified txid of a pending transaction. fn spend_coins<'a>(&mut self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)]); /// Mark a set of coins as spent by a specified txid at a specified block time. @@ -115,8 +118,18 @@ impl DatabaseConnection for SqliteConn { self.increment_derivation_index(secp) } - fn unspent_coins(&mut self) -> HashMap { - db_coins_into_coins(self.unspent_coins()) + fn list_unspent_coins(&mut self) -> HashMap { + self.list_unspent_coins() + .into_iter() + .map(|db_coin| (db_coin.outpoint, db_coin.into())) + .collect() + } + + fn list_spending_coins(&mut self) -> HashMap { + self.list_spending_coins() + .into_iter() + .map(|db_coin| (db_coin.outpoint, db_coin.into())) + .collect() } fn new_unspent_coins<'a>(&mut self, coins: &[Coin]) { @@ -147,7 +160,10 @@ impl DatabaseConnection for SqliteConn { &mut self, outpoints: &[bitcoin::OutPoint], ) -> HashMap { - db_coins_into_coins(self.db_coins(outpoints)) + self.db_coins(outpoints) + .into_iter() + .map(|db_coin| (db_coin.outpoint, db_coin.into())) + .collect() } fn spend_tx(&mut self, txid: &bitcoin::Txid) -> Option { @@ -170,37 +186,6 @@ impl DatabaseConnection for SqliteConn { } } -// FIXME: if possible, avoid reallocating. -fn db_coins_into_coins(db_coins: Vec) -> HashMap { - db_coins - .into_iter() - .map(|db_coin| { - let DbCoin { - outpoint, - block_height, - block_time, - amount, - derivation_index, - spend_txid, - spent_at, - .. - } = db_coin; - ( - outpoint, - Coin { - outpoint, - block_height, - block_time, - amount, - derivation_index, - spend_txid, - spent_at, - }, - ) - }) - .collect() -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct Coin { pub outpoint: bitcoin::OutPoint, @@ -212,6 +197,30 @@ pub struct Coin { pub spent_at: Option, } +impl std::convert::From for Coin { + fn from(db_coin: DbCoin) -> Coin { + let DbCoin { + outpoint, + block_height, + block_time, + amount, + derivation_index, + spend_txid, + spent_at, + .. + } = db_coin; + Coin { + outpoint, + block_height, + block_time, + amount, + derivation_index, + spend_txid, + spent_at, + } + } +} + impl std::hash::Hash for Coin { fn hash(&self, h: &mut H) { self.outpoint.hash(h) diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index a4e672e3..58404e73 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -253,7 +253,7 @@ impl SqliteConn { } /// Get all UTxOs. - pub fn unspent_coins(&mut self) -> Vec { + pub fn list_unspent_coins(&mut self) -> Vec { db_query( &mut self.conn, "SELECT * FROM coins WHERE spend_txid is NULL", @@ -263,6 +263,17 @@ impl SqliteConn { .expect("Db must not fail") } + /// List coins that are being spent and whose spending transaction is still unconfirmed. + pub fn list_spending_coins(&mut self) -> Vec { + db_query( + &mut self.conn, + "SELECT * FROM coins WHERE spend_txid IS NOT NULL AND spent_at IS NULL", + rusqlite::params![], + |row| row.try_into(), + ) + .expect("Db must not fail") + } + // FIXME: don't take the whole coin, we don't need it. /// Store new, unconfirmed and unspent, coins. /// Will panic if given a coin that is already in DB. @@ -323,6 +334,29 @@ impl SqliteConn { .expect("Database must be available") } + /// Mark a set of coins as spent. + pub fn confirm_spend<'a>( + &mut self, + outpoints: impl IntoIterator, + ) { + db_exec(&mut self.conn, |db_tx| { + for (outpoint, spend_txid, time) in outpoints { + db_tx.execute( + "UPDATE coins SET spend_txid = ?1, spent_at = ?2 WHERE txid = ?3 AND vout = ?4", + rusqlite::params![ + spend_txid.to_vec(), + time, + outpoint.txid.to_vec(), + outpoint.vout, + ], + )?; + } + + Ok(()) + }) + .expect("Database must be available") + } + pub fn db_address(&mut self, address: &bitcoin::Address) -> Option { db_query( &mut self.conn, @@ -519,7 +553,7 @@ mod tests { let mut conn = db.connection().unwrap(); // Necessarily empty at first. - assert!(conn.unspent_coins().is_empty()); + assert!(conn.list_unspent_coins().is_empty()); // Add one, we'll get it. let coin_a = Coin { @@ -535,7 +569,7 @@ mod tests { spent_at: None, }; conn.new_unspent_coins(&[coin_a.clone()]); // On 1.48, arrays aren't IntoIterator - assert_eq!(conn.unspent_coins()[0].outpoint, coin_a.outpoint); + assert_eq!(conn.list_unspent_coins()[0].outpoint, coin_a.outpoint); // We can query it by its outpoint let coins = conn.db_coins(&[coin_a.outpoint]); @@ -557,7 +591,7 @@ mod tests { }; conn.new_unspent_coins(&[coin_b.clone()]); let outpoints: HashSet = conn - .unspent_coins() + .list_unspent_coins() .into_iter() .map(|c| c.outpoint) .collect(); @@ -586,7 +620,7 @@ mod tests { let height = 174500; let time = 174500; conn.confirm_coins(&[(coin_a.outpoint, height, time)]); - let coins = conn.unspent_coins(); + let coins = conn.list_unspent_coins(); assert_eq!(coins[0].block_height, Some(height)); assert_eq!(coins[0].block_time, Some(time)); assert!(coins[1].block_height.is_none()); @@ -598,13 +632,34 @@ mod tests { bitcoin::Txid::from_slice(&[0; 32][..]).unwrap(), )]); let outpoints: HashSet = conn - .unspent_coins() + .list_unspent_coins() .into_iter() .map(|c| c.outpoint) .collect(); assert!(!outpoints.contains(&coin_a.outpoint)); assert!(outpoints.contains(&coin_b.outpoint)); + let outpoints: HashSet = conn + .list_spending_coins() + .into_iter() + .map(|c| c.outpoint) + .collect(); + assert!(outpoints.contains(&coin_a.outpoint)); + + // Now if we confirm the spend. + conn.confirm_spend(&[( + coin_a.outpoint, + bitcoin::Txid::from_slice(&[0; 32][..]).unwrap(), + 3, + )]); + // the coin is not in a spending state. + let outpoints: HashSet = conn + .list_spending_coins() + .into_iter() + .map(|c| c.outpoint) + .collect(); + assert!(outpoints.is_empty()); + // Both are still in DB let coins = conn.db_coins(&[coin_a.outpoint, coin_b.outpoint]); assert_eq!(coins.len(), 2); diff --git a/src/testutils.rs b/src/testutils.rs index 1ef694c7..d3462407 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -55,6 +55,13 @@ impl BitcoinInterface for DummyBitcoind { fn spending_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { Vec::new() } + + fn spent_coins( + &self, + _: &[(bitcoin::OutPoint, bitcoin::Txid)], + ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, u32)> { + Vec::new() + } } pub struct DummyDb { @@ -107,10 +114,20 @@ impl DatabaseConnection for DummyDbConn { self.db.write().unwrap().curr_index = next_index; } - fn unspent_coins(&mut self) -> HashMap { + fn list_unspent_coins(&mut self) -> HashMap { self.db.read().unwrap().coins.clone() } + fn list_spending_coins(&mut self) -> HashMap { + let mut result = HashMap::new(); + for (k, v) in self.db.read().unwrap().coins.iter() { + if !v.spend_txid.is_none() { + result.insert(k.clone(), v.clone()); + } + } + result + } + fn new_unspent_coins<'a>(&mut self, coins: &[Coin]) { for coin in coins { self.db From bb9897bdbb9926f6c3f8d9fb95339e45546372e2 Mon Sep 17 00:00:00 2001 From: edouard Date: Mon, 19 Sep 2022 18:25:35 +0200 Subject: [PATCH 3/8] Update coin txid if conflicting tx was confirmed --- src/bitcoin/d/mod.rs | 17 ++++++++++++++++- src/bitcoin/mod.rs | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 60ce4fd2..217dd957 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -748,8 +748,9 @@ impl From for LSBlockRes { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct GetTxRes { + pub conflicting_txs: Vec, pub block_height: Option, pub block_time: Option, } @@ -764,7 +765,21 @@ impl From for GetTxRes { .get("blocktime") .and_then(Json::as_u64) .map(|bt| bt as u32); + let conflicting_txs = json + .get("walletconflicts") + .map(|v| Json::as_array(&v)) + .flatten() + .map(|array| { + array + .into_iter() + .map(|v| { + bitcoin::Txid::from_str(v.as_str().expect("wrong json format")).unwrap() + }) + .collect() + }); + GetTxRes { + conflicting_txs: conflicting_txs.unwrap_or_default(), block_height, block_time, } diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 4141edd9..0c0f9c1c 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -156,7 +156,6 @@ impl BitcoinInterface for d::BitcoinD { let mut spent = Vec::with_capacity(outpoints.len()); let mut cache: HashMap> = HashMap::new(); - for (op, txid) in outpoints { let tx: Option<&d::GetTxRes> = match cache.get(txid) { Some(tx) => tx.as_ref(), @@ -167,16 +166,43 @@ impl BitcoinInterface for d::BitcoinD { } }; + // There is an immutable borrow on the cache, these txs will be added once it is + // dropped. + let mut txs_to_cache: Vec<(bitcoin::Txid, Option)> = Vec::new(); + if let Some(tx) = tx { if let Some(block_height) = tx.block_height { if block_height > 1 { spent.push((*op, *txid, tx.block_time.expect("Spend is confirmed"))) } - } else { - // TODO: handle the case where new transaction which txid is not the - // coin spending_txid, spent the coin. + } else if !tx.conflicting_txs.is_empty() { + for txid in tx.conflicting_txs.clone() { + let tx: Option<&d::GetTxRes> = match cache.get(&txid) { + Some(tx) => tx.as_ref(), + None => { + let tx = self.get_transaction(&txid); + txs_to_cache.push((txid, tx)); + txs_to_cache.last().unwrap().1.as_ref() + } + }; + if let Some(tx) = tx { + if let Some(block_height) = tx.block_height { + if block_height > 1 { + spent.push(( + *op, + txid, + tx.block_time.expect("Spend is confirmed"), + )) + } + } + } + } } } + + for (txid, res) in txs_to_cache { + cache.insert(txid, res); + } } spent From 3534e35b8721c5026b27d4d3c07b87dc6e4fa3dd Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Oct 2022 17:17:45 +0200 Subject: [PATCH 4/8] bitcoin: remove erroneous block height check --- src/bitcoin/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 0c0f9c1c..d3b101d7 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -171,10 +171,10 @@ impl BitcoinInterface for d::BitcoinD { let mut txs_to_cache: Vec<(bitcoin::Txid, Option)> = Vec::new(); if let Some(tx) = tx { - if let Some(block_height) = tx.block_height { - if block_height > 1 { - spent.push((*op, *txid, tx.block_time.expect("Spend is confirmed"))) - } + if let Some(block_time) = tx.block_time { + // TODO: make both block time and height under the same Option. + assert!(tx.block_height.is_some()); + spent.push((*op, *txid, block_time)) } else if !tx.conflicting_txs.is_empty() { for txid in tx.conflicting_txs.clone() { let tx: Option<&d::GetTxRes> = match cache.get(&txid) { From c9b6c6dedbc1e28b05543191238d0a81a92ba238 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Oct 2022 17:19:27 +0200 Subject: [PATCH 5/8] db: re-rename list_unspent_coins into unspent_coins --- src/bitcoin/poller/looper.rs | 2 +- src/commands/mod.rs | 2 +- src/database/mod.rs | 6 +++--- src/database/sqlite/mod.rs | 12 ++++++------ src/testutils.rs | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 96bb7fd9..26c151e0 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -28,7 +28,7 @@ fn update_coins( previous_tip: &BlockChainTip, ) -> UpdatedCoins { // Start by fetching newly received coins. - let curr_coins = db_conn.list_unspent_coins(); + let curr_coins = db_conn.unspent_coins(); let mut received = Vec::new(); for utxo in bit.received_coins(&previous_tip) { if let Some(derivation_index) = db_conn.derivation_index_by_address(&utxo.address) { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index b7fd2782..5bdb6278 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -197,7 +197,7 @@ impl DaemonControl { pub fn list_coins(&self) -> ListCoinsResult { let mut db_conn = self.db.connection(); let coins: Vec = db_conn - .list_unspent_coins() + .unspent_coins() // Can't use into_values as of Rust 1.48 .into_iter() .map(|(_, coin)| { diff --git a/src/database/mod.rs b/src/database/mod.rs index f0b261bc..5e181bf2 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -55,7 +55,7 @@ pub trait DatabaseConnection { ) -> Option; /// Get all UTxOs. - fn list_unspent_coins(&mut self) -> HashMap; + fn unspent_coins(&mut self) -> HashMap; /// List coins that are being spent and whose spending transaction is still unconfirmed. fn list_spending_coins(&mut self) -> HashMap; @@ -118,8 +118,8 @@ impl DatabaseConnection for SqliteConn { self.increment_derivation_index(secp) } - fn list_unspent_coins(&mut self) -> HashMap { - self.list_unspent_coins() + fn unspent_coins(&mut self) -> HashMap { + self.unspent_coins() .into_iter() .map(|db_coin| (db_coin.outpoint, db_coin.into())) .collect() diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 58404e73..cc114139 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -253,7 +253,7 @@ impl SqliteConn { } /// Get all UTxOs. - pub fn list_unspent_coins(&mut self) -> Vec { + pub fn unspent_coins(&mut self) -> Vec { db_query( &mut self.conn, "SELECT * FROM coins WHERE spend_txid is NULL", @@ -553,7 +553,7 @@ mod tests { let mut conn = db.connection().unwrap(); // Necessarily empty at first. - assert!(conn.list_unspent_coins().is_empty()); + assert!(conn.unspent_coins().is_empty()); // Add one, we'll get it. let coin_a = Coin { @@ -569,7 +569,7 @@ mod tests { spent_at: None, }; conn.new_unspent_coins(&[coin_a.clone()]); // On 1.48, arrays aren't IntoIterator - assert_eq!(conn.list_unspent_coins()[0].outpoint, coin_a.outpoint); + assert_eq!(conn.unspent_coins()[0].outpoint, coin_a.outpoint); // We can query it by its outpoint let coins = conn.db_coins(&[coin_a.outpoint]); @@ -591,7 +591,7 @@ mod tests { }; conn.new_unspent_coins(&[coin_b.clone()]); let outpoints: HashSet = conn - .list_unspent_coins() + .unspent_coins() .into_iter() .map(|c| c.outpoint) .collect(); @@ -620,7 +620,7 @@ mod tests { let height = 174500; let time = 174500; conn.confirm_coins(&[(coin_a.outpoint, height, time)]); - let coins = conn.list_unspent_coins(); + let coins = conn.unspent_coins(); assert_eq!(coins[0].block_height, Some(height)); assert_eq!(coins[0].block_time, Some(time)); assert!(coins[1].block_height.is_none()); @@ -632,7 +632,7 @@ mod tests { bitcoin::Txid::from_slice(&[0; 32][..]).unwrap(), )]); let outpoints: HashSet = conn - .list_unspent_coins() + .unspent_coins() .into_iter() .map(|c| c.outpoint) .collect(); diff --git a/src/testutils.rs b/src/testutils.rs index d3462407..3803f99f 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -114,7 +114,7 @@ impl DatabaseConnection for DummyDbConn { self.db.write().unwrap().curr_index = next_index; } - fn list_unspent_coins(&mut self) -> HashMap { + fn unspent_coins(&mut self) -> HashMap { self.db.read().unwrap().coins.clone() } From 51f11a9e2f51945c7ec1f59a9acfd43d2f3977d3 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Oct 2022 17:24:27 +0200 Subject: [PATCH 6/8] looper: cleanup the check for spending coins' confirmation --- src/bitcoin/poller/looper.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 26c151e0..5afd7e85 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -88,14 +88,16 @@ fn update_coins( .collect(); let spending = bit.spending_coins(&to_be_spent); - // We need to confirm coins that are currently spending and which transactions are now in a - // block. - let mut spending_coins: Vec<(bitcoin::OutPoint, bitcoin::Txid)> = db_conn + // Mark coins in a spending state whose Spend transaction was confirmed as such. Note we + // need to take into account the freshly marked as spending coins as well, as their spend + // may have been confirmed within the previous tip and the current one, and we may not poll + // this chunk of the chain anymore. + let spending_coins: Vec<(bitcoin::OutPoint, bitcoin::Txid)> = db_conn .list_spending_coins() .values() .map(|coin| (coin.outpoint, coin.spend_txid.expect("Coin is spending"))) + .chain(spending.iter().cloned()) .collect(); - spending_coins.extend(&spending); let spent = bit.spent_coins(spending_coins.as_slice()); UpdatedCoins { From 7513bcbf09f13f1625b989d52c52087048d90a73 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Oct 2022 17:25:22 +0200 Subject: [PATCH 7/8] bitcoind: use and_then instead of map().flatten() --- src/bitcoin/d/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 217dd957..a6f8ce48 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -767,8 +767,7 @@ impl From for GetTxRes { .map(|bt| bt as u32); let conflicting_txs = json .get("walletconflicts") - .map(|v| Json::as_array(&v)) - .flatten() + .and_then(Json::as_array) .map(|array| { array .into_iter() From 172cda19a0a72b77e3832f2e29ed7ff6f8062c44 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Oct 2022 17:25:56 +0200 Subject: [PATCH 8/8] bitcoin: avoid an unnecessary large clone() --- src/bitcoin/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index d3b101d7..ff60b615 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -176,12 +176,12 @@ impl BitcoinInterface for d::BitcoinD { assert!(tx.block_height.is_some()); spent.push((*op, *txid, block_time)) } else if !tx.conflicting_txs.is_empty() { - for txid in tx.conflicting_txs.clone() { - let tx: Option<&d::GetTxRes> = match cache.get(&txid) { + for txid in &tx.conflicting_txs { + let tx: Option<&d::GetTxRes> = match cache.get(txid) { Some(tx) => tx.as_ref(), None => { let tx = self.get_transaction(&txid); - txs_to_cache.push((txid, tx)); + txs_to_cache.push((*txid, tx)); txs_to_cache.last().unwrap().1.as_ref() } }; @@ -190,7 +190,7 @@ impl BitcoinInterface for d::BitcoinD { if block_height > 1 { spent.push(( *op, - txid, + *txid, tx.block_time.expect("Spend is confirmed"), )) }