From 6dd92052af0afc37bddb67493a64af396e7964c9 Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Thu, 28 Nov 2024 12:12:24 +0000 Subject: [PATCH] database: track whether coin is from self This populates the new columns from the previous migration for existing rows and then maintains them in the poller moving forward. --- lianad/src/bitcoin/poller/looper.rs | 3 + lianad/src/database/mod.rs | 9 + lianad/src/database/sqlite/mod.rs | 459 ++++++++++++++++++++++++++-- lianad/src/database/sqlite/utils.rs | 147 +++++++++ lianad/src/testutils.rs | 4 + 5 files changed, 605 insertions(+), 17 deletions(-) diff --git a/lianad/src/bitcoin/poller/looper.rs b/lianad/src/bitcoin/poller/looper.rs index baa95c2c..181d3c9d 100644 --- a/lianad/src/bitcoin/poller/looper.rs +++ b/lianad/src/bitcoin/poller/looper.rs @@ -315,6 +315,9 @@ fn updates( db_conn.unspend_coins(&updated_coins.expired_spending); db_conn.spend_coins(&updated_coins.spending); db_conn.confirm_spend(&updated_coins.spent); + // Update info about which coins are from self only after + // coins have been inserted & updated above. + db_conn.update_coins_from_self(current_tip.height); if latest_tip != current_tip { db_conn.update_tip(&latest_tip); log::debug!("New tip: '{}'", latest_tip); diff --git a/lianad/src/database/mod.rs b/lianad/src/database/mod.rs index 6ffce1e3..614d4c82 100644 --- a/lianad/src/database/mod.rs +++ b/lianad/src/database/mod.rs @@ -181,6 +181,10 @@ pub trait DatabaseConnection { /// Store transactions in database, ignoring any that already exist. fn new_txs(&mut self, txs: &[bitcoin::Transaction]); + /// For all unconfirmed coins and those confirmed after `prev_tip_height`, + /// update whether the coin is from self or not. + fn update_coins_from_self(&mut self, prev_tip_height: i32); + /// Retrieve a list of transactions and their corresponding block heights and times. fn list_wallet_transactions( &mut self, @@ -379,6 +383,11 @@ impl DatabaseConnection for SqliteConn { self.new_txs(txs) } + fn update_coins_from_self(&mut self, prev_tip_height: i32) { + self.update_coins_from_self(prev_tip_height) + .expect("must not fail") + } + fn list_wallet_transactions( &mut self, txids: &[bitcoin::Txid], diff --git a/lianad/src/database/sqlite/mod.rs b/lianad/src/database/sqlite/mod.rs index 59ba5c9a..2084c86c 100644 --- a/lianad/src/database/sqlite/mod.rs +++ b/lianad/src/database/sqlite/mod.rs @@ -43,7 +43,7 @@ use miniscript::bitcoin::{ secp256k1, }; -const DB_VERSION: i64 = 7; +const DB_VERSION: i64 = 8; /// Last database version for which Bitcoin transactions were not stored in database. In practice /// this meant we relied on the bitcoind watchonly wallet to store them for us. @@ -750,6 +750,91 @@ impl SqliteConn { .expect("Database must be available") } + /// Update `is_from_self` in coins table for all unconfirmed coins + /// and those confirmed after `prev_tip_height`. + /// + /// This only sets the value to true as we do not expect the value + /// to change from true to false. In case of a reorg, the value + /// for all unconfirmed coins should be set to false before this + /// method is called. + pub fn update_coins_from_self(&mut self, prev_tip_height: i32) -> Result<(), rusqlite::Error> { + db_exec(&mut self.conn, |db_tx| { + // Given the requirement for unconfirmed coins that all ancestors + // be from self, we perform the update in a loop until no further + // rows are updated in order to iterate over the unconfirmed coins. + // Although we don't expect any unconfirmed transaction to have + // more than 25 in-mempool descendants including itself, there + // could be more descendants in the DB following a reorg and a + // rollback of the tip. The max number of iterations would be + // one per unconfirmed coin not from self plus one for all + // confirmed coins. + // In any case, the query only sets `is_from_self` to 1 for + // those coins with value 0 and so the number of rows affected + // by each iteration must become 0. + let max_iterations = { + let num_unconfirmed: u64 = db_tx.query_row( + "SELECT COUNT(*) FROM coins + WHERE blockheight IS NULL AND is_from_self = 0", + [], + |row| row.get(0), + )?; + // Add 1 for the confirmed coins, which will all + // be updated in the first iteration, and another 1 + // as a final check there's nothing left to update. + num_unconfirmed.checked_add(2).expect("must fit") + }; + log::debug!( + "Updating is_from_self in up to {} iterations..", + max_iterations + ); + let mut updated = 0; + for i in 0..max_iterations { + updated = db_tx.execute( + " + UPDATE coins + SET is_from_self = 1 + FROM transactions t + INNER JOIN ( + SELECT + spend_txid, + SUM( + CASE + WHEN blockheight IS NOT NULL THEN 1 + -- If the spending coin is unconfirmed, only count + -- it as an input coin if it is from self. + WHEN blockheight IS NULL AND is_from_self = 1 THEN 1 + ELSE 0 + END + ) AS cnt + FROM coins + WHERE spend_txid IS NOT NULL + -- We only need to consider spend transactions that are + -- unconfirmed or confirmed after `prev_tip_height + -- as only these transactions will affect the coins that + -- we are updating. + AND (spend_block_height IS NULL OR spend_block_height > ?1) + GROUP BY spend_txid + ) spends + ON t.txid = spends.spend_txid AND t.num_inputs = spends.cnt + WHERE coins.txid = t.txid + AND (coins.blockheight IS NULL OR coins.blockheight > ?1) + AND coins.is_from_self = 0 + ", + [prev_tip_height], + )?; + if updated == 0 { + log::debug!("Finished updating is_from_self in {} iterations.", i + 1); + break; + } + } + assert_eq!( + updated, 0, + "no rows expected to be updated on final iteration while updating is_from_self", + ); + Ok(()) + }) + } + pub fn list_wallet_transactions( &mut self, txids: &[bitcoin::Txid], @@ -814,6 +899,10 @@ impl SqliteConn { /// - Spending transactions confirmation /// - Tip /// + /// The `is_from_self` value for all unconfirmed coins following the rollback is + /// set to false. This is because this value depends on the confirmation status + /// of ancestor coins and so will need to be re-evaluated. + /// /// This will have to be updated if we are to add new fields based on block data /// in the database eventually. pub fn rollback_tip(&mut self, new_tip: &BlockChainTip) { @@ -826,6 +915,12 @@ impl SqliteConn { "UPDATE coins SET spend_block_height = NULL, spend_block_time = NULL WHERE spend_block_height > ?1", rusqlite::params![new_tip.height], )?; + // This statement must be run after updating `blockheight` above so that it includes coins + // that become unconfirmed following the rollback. + db_tx.execute( + "UPDATE coins SET is_from_self = 0 WHERE blockheight IS NULL", + rusqlite::params![], + )?; db_tx.execute( "UPDATE tip SET blockheight = (?1), blockhash = (?2)", rusqlite::params![new_tip.height, new_tip.hash[..].to_vec()], @@ -847,7 +942,7 @@ mod tests { str::FromStr, }; - use bitcoin::{bip32, ScriptBuf}; + use bitcoin::{bip32, BlockHash, ScriptBuf, TxIn}; // The database schema used by the first versions of Liana (database version 0). Used to test // migrations starting from the first version. @@ -2469,7 +2564,332 @@ CREATE TABLE labels ( } #[test] - fn v0_to_v7_migration() { + fn sqlite_update_coins_from_self() { + let (tmp_dir, _, _, db) = dummy_db(); + + // Helper to create a dummy transaction. + // Varying `lock_time_height` allows to obtain a unique txid for the given `num_inputs`. + fn dummy_tx(num_inputs: u32, lock_time_height: u32) -> bitcoin::Transaction { + bitcoin::Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::from_height(lock_time_height).unwrap(), + input: (0..num_inputs).map(|_| TxIn::default()).collect(), + output: vec![bitcoin::TxOut::minimal_non_dust(ScriptBuf::default())], // a single output, + } + } + + { + let mut conn = db.connection().unwrap(); + + // Deposit two coins from two different external transactions. + let tx_a = dummy_tx(1, 0); + let tx_b = dummy_tx(1, 1); + let coin_tx_a: Coin = Coin { + outpoint: bitcoin::OutPoint::new(tx_a.txid(), 0), + is_immature: false, + amount: bitcoin::Amount::from_sat(1_000_000), + derivation_index: bip32::ChildNumber::from_normal_idx(0).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_tx_b: Coin = Coin { + outpoint: bitcoin::OutPoint::new(tx_b.txid(), 0), + is_immature: false, + amount: bitcoin::Amount::from_sat(1_000_000), + derivation_index: bip32::ChildNumber::from_normal_idx(1).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + conn.new_txs(&[tx_a, tx_b]); + conn.new_unspent_coins(&[coin_tx_a, coin_tx_b]); + + // The coins are not from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + // Update from self info. + conn.update_coins_from_self(0).unwrap(); + // As expected, the coins are still not marked as from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + + // Spend `coin_tx_a` in `tx_c` with change `coin_tx_c`. + let tx_c = dummy_tx(1, 2); + let coin_tx_c: Coin = Coin { + outpoint: bitcoin::OutPoint::new(tx_c.txid(), 0), + is_immature: false, + amount: bitcoin::Amount::from_sat(1_000_000), + derivation_index: bip32::ChildNumber::from_normal_idx(2).unwrap(), + is_change: true, + block_info: None, + spend_txid: None, + spend_block: None, + }; + conn.new_txs(&[tx_c.clone()]); + conn.spend_coins(&[(coin_tx_a.outpoint, tx_c.txid())]); + conn.new_unspent_coins(&[coin_tx_c]); + + // Although `coin_tx_c` has only one parent, `coin_tx_a` is + // unconfirmed and not from self. So all our coins are still + // not marked as from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + conn.update_coins_from_self(0).unwrap(); + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + + // Now refresh `coin_tx_c` in `tx_d`, creating `coin_tx_d`. + let tx_d = dummy_tx(1, 3); + let coin_tx_d: Coin = Coin { + outpoint: bitcoin::OutPoint::new(tx_d.txid(), 0), + is_immature: false, + amount: bitcoin::Amount::from_sat(1_000_000), + derivation_index: bip32::ChildNumber::from_normal_idx(3).unwrap(), + is_change: true, + block_info: None, + spend_txid: None, + spend_block: None, + }; + conn.new_txs(&[tx_d.clone()]); + conn.spend_coins(&[(coin_tx_c.outpoint, tx_d.txid())]); + conn.new_unspent_coins(&[coin_tx_d]); + + // All coins are unconfirmed and none are from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + conn.update_coins_from_self(0).unwrap(); + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + + // Spend the deposited coin `coin_tx_b` and the refreshed coin `coin_tx_d` + // together in `tx_e`, creating `coin_tx_e`. + let tx_e = dummy_tx(2, 4); // 2 inputs + let coin_tx_e: Coin = Coin { + outpoint: bitcoin::OutPoint::new(tx_e.txid(), 0), + is_immature: false, + amount: bitcoin::Amount::from_sat(1_000_000), + derivation_index: bip32::ChildNumber::from_normal_idx(4).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + conn.new_txs(&[tx_e.clone()]); + conn.spend_coins(&[ + (coin_tx_b.outpoint, tx_e.txid()), + (coin_tx_d.outpoint, tx_e.txid()), + ]); + conn.new_unspent_coins(&[coin_tx_e]); + + // Still there are no confirmed coins, so everything remains as not from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + conn.update_coins_from_self(0).unwrap(); + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + + // Finally, refresh `coin_tx_e` in transaction `tx_f`, creating `coin_tx_f`. + let tx_f = dummy_tx(1, 5); + let coin_tx_f: Coin = Coin { + outpoint: bitcoin::OutPoint::new(tx_f.txid(), 0), + is_immature: false, + amount: bitcoin::Amount::from_sat(1_000_000), + derivation_index: bip32::ChildNumber::from_normal_idx(5).unwrap(), + is_change: true, + block_info: None, + spend_txid: None, + spend_block: None, + }; + conn.new_txs(&[tx_f.clone()]); + conn.spend_coins(&[(coin_tx_e.outpoint, tx_f.txid())]); + conn.new_unspent_coins(&[coin_tx_f]); + + // Still no coins are from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + conn.update_coins_from_self(0).unwrap(); + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + + // Now confirm `tx_a` and `tx_c` in successive blocks. + conn.confirm_coins(&[ + (coin_tx_a.outpoint, 100, 1_000), + (coin_tx_c.outpoint, 101, 1_001), + ]); + conn.confirm_spend(&[(coin_tx_a.outpoint, tx_c.txid(), 101, 1_001)]); + // Coins are still not marked as from self. + assert!(conn.coins(&[], &[]).iter().all(|c| !c.is_from_self)); + // Now update from self for coins confirmed after 101, which excludes the two coins above. + // Only `coin_tx_d` is from self, because it's unconfirmed and its parent is a confirmed coin. + // `coin_tx_e` still depends on `coin_tx_b` which is an unconfirmed deposit. + conn.update_coins_from_self(101).unwrap(); + assert!(conn + .coins(&[], &[coin_tx_d.outpoint]) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins( + &[], + &[ + coin_tx_a.outpoint, + coin_tx_b.outpoint, + coin_tx_c.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint + ] + ) + .iter() + .all(|c| !c.is_from_self)); + + // Now run the update for coins confirmed after 100. + conn.update_coins_from_self(100).unwrap(); + // `coin_tx_c` is now marked as from self as it has a single parent + // that is confirmed (even though its parent is an external deposit). + assert!(conn + .coins(&[], &[coin_tx_c.outpoint, coin_tx_d.outpoint]) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins( + &[], + &[ + coin_tx_a.outpoint, + coin_tx_b.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint + ] + ) + .iter() + .all(|c| !c.is_from_self)); + + // Even if we run the update for coins confirmed after height 99, + // `coin_tx_a` will not be marked as from self as it's an external deposit. + conn.update_coins_from_self(99).unwrap(); + assert!(conn + .coins(&[], &[coin_tx_c.outpoint, coin_tx_d.outpoint]) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins( + &[], + &[ + coin_tx_a.outpoint, + coin_tx_b.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint + ] + ) + .iter() + .all(|c| !c.is_from_self)); + + // Now confirm the other external deposit coin. + conn.confirm_coins(&[(coin_tx_b.outpoint, 102, 1_002)]); + // If we run the update, it doesn't matter if we use a later height + // as there are only unconfirmed coins that need to be updated. + conn.update_coins_from_self(110).unwrap(); + // `coin_tx_e` and `coin_tx_f` are also now from self. + assert!(conn + .coins( + &[], + &[ + coin_tx_c.outpoint, + coin_tx_d.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint + ] + ) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins(&[], &[coin_tx_a.outpoint, coin_tx_b.outpoint,]) + .iter() + .all(|c| !c.is_from_self)); + + // Even if we now run the update with an earlier height, + // `coin_tx_b` will not be marked as from self. + conn.update_coins_from_self(101).unwrap(); + // No changes from above. + assert!(conn + .coins( + &[], + &[ + coin_tx_c.outpoint, + coin_tx_d.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint + ] + ) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins(&[], &[coin_tx_a.outpoint, coin_tx_b.outpoint,]) + .iter() + .all(|c| !c.is_from_self)); + + // Now we will roll the tip back earlier than some of our confirmed coins. + let new_tip = { + // It doesn't matter what this hash value is as we only care about the height. + let hash = BlockHash::from_str( + "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f", + ) + .unwrap(); + &BlockChainTip { height: 101, hash } + }; + conn.rollback_tip(new_tip); + + // Only `coin_tx_a` and `coin_tx_c` are still confirmed. + assert_eq!( + conn.coins(&[], &[]) + .iter() + .filter_map(|c| if c.block_info.is_some() { + Some(c.outpoint) + } else { + None + }) + .collect::>(), + vec![coin_tx_a.outpoint, coin_tx_c.outpoint] + ); + // Rolling back sets all unconfirmed coins as not from self so only + // `coin_tx_c` is still marked as from self. + assert!(conn + .coins(&[], &[coin_tx_c.outpoint]) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins( + &[], + &[ + coin_tx_a.outpoint, + coin_tx_b.outpoint, + coin_tx_d.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint + ] + ) + .iter() + .all(|c| !c.is_from_self)); + + // Now run the update from the current tip height of 101. + conn.update_coins_from_self(101).unwrap(); + // `coin_tx_d` is now marked as from self as its parent `coin_tx_c` + // is confirmed. Coins `coin_tx_e` and `coin_tx_f` depend on the + // unconfirmed `tx_coin_b` and so remain as not from self. + assert!(conn + .coins(&[], &[coin_tx_c.outpoint, coin_tx_d.outpoint]) + .iter() + .all(|c| c.is_from_self)); + assert!(conn + .coins( + &[], + &[ + coin_tx_a.outpoint, + coin_tx_b.outpoint, + coin_tx_e.outpoint, + coin_tx_f.outpoint, + ] + ) + .iter() + .all(|c| !c.is_from_self)); + } + + fs::remove_dir_all(tmp_dir).unwrap(); + } + + #[test] + fn v0_to_v8_migration() { let secp = secp256k1::Secp256k1::verification_only(); // Create a database with version 0, using the old schema. @@ -2492,8 +2912,8 @@ CREATE TABLE labels ( .map(|i| bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, lock_time: bitcoin::absolute::LockTime::from_height(i).unwrap(), - input: Vec::new(), - output: Vec::new(), + input: vec![bitcoin::TxIn::default()], // a single input + output: vec![bitcoin::TxOut::minimal_non_dust(ScriptBuf::default())], // a single output, }) .collect(); // The helper that was used to store Spend transaction in previous versions of the software @@ -2575,7 +2995,7 @@ CREATE TABLE labels ( { let mut conn = db.connection().unwrap(); let version = conn.db_version(); - assert_eq!(version, 7); + assert_eq!(version, 8); } // We should now be able to insert another PSBT, to query both, and the first PSBT must // have no associated timestamp. @@ -2649,7 +3069,7 @@ CREATE TABLE labels ( } #[test] - fn v3_to_v7_migration() { + fn v3_to_v8_migration() { let secp = secp256k1::Secp256k1::verification_only(); // Create a database with version 3, using the old schema. @@ -2672,8 +3092,8 @@ CREATE TABLE labels ( .map(|i| bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, lock_time: bitcoin::absolute::LockTime::from_height(i).unwrap(), - input: Vec::new(), - output: Vec::new(), + input: vec![bitcoin::TxIn::default()], // a single input + output: vec![bitcoin::TxOut::minimal_non_dust(ScriptBuf::default())], // a single output, }) .collect(); @@ -2799,10 +3219,10 @@ CREATE TABLE labels ( // Migrate the DB. maybe_apply_migration(&db_path, &bitcoin_txs).unwrap(); - assert_eq!(conn.db_version(), 7); + assert_eq!(conn.db_version(), 8); // Migrating twice will be a no-op. No need to pass `bitcoin_txs` second time. maybe_apply_migration(&db_path, &[]).unwrap(); - assert!(conn.db_version() == 7); + assert!(conn.db_version() == 8); // Compare the `DbCoin`s with the expected values. let coins_post = conn.coins(&[], &[]); @@ -2821,6 +3241,11 @@ CREATE TABLE labels ( assert_eq!(c_post.is_change, c_pre.is_change); assert_eq!(c_post.spend_txid, c_pre.spend_txid); assert_eq!(c_post.spend_block, c_pre.spend_block); + // only coins D and E are from self. + assert_eq!( + c_post.is_from_self, + [coin_d_outpoint, coin_e_outpoint].contains(&c_pre.outpoint) + ); } } @@ -2847,8 +3272,8 @@ CREATE TABLE labels ( .map(|i| bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, lock_time: bitcoin::absolute::LockTime::from_height(i).unwrap(), - input: Vec::new(), - output: Vec::new(), + input: vec![bitcoin::TxIn::default()], // a single input + output: vec![bitcoin::TxOut::minimal_non_dust(ScriptBuf::default())], // a single output, }) .collect(); let spend_txs: Vec<_> = (0..10) @@ -2857,12 +3282,12 @@ CREATE TABLE labels ( bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, lock_time: bitcoin::absolute::LockTime::from_height(1_234 + i).unwrap(), - input: Vec::new(), - output: Vec::new(), + input: vec![bitcoin::TxIn::default()], // a single input + output: vec![bitcoin::TxOut::minimal_non_dust(ScriptBuf::default())], // a single output, }, if i % 2 == 0 { Some(DbBlockInfo { - height: (i % 5) as i32 * 2_000, + height: 1 + (i % 5) as i32 * 2_000, time: 1722488619 + (i % 5) * 84_999, }) } else { @@ -2890,7 +3315,7 @@ CREATE TABLE labels ( is_change: (i % 4) == 0, block_info: if i & 2 == 0 { Some(DbBlockInfo { - height: (i % 100) as i32 * 1_000, + height: 1 + (i % 100) as i32 * 1_000, time: 1722408619 + (i % 100) as u32 * 42_000, }) } else { diff --git a/lianad/src/database/sqlite/utils.rs b/lianad/src/database/sqlite/utils.rs index f26f4f0e..ad7aab6b 100644 --- a/lianad/src/database/sqlite/utils.rs +++ b/lianad/src/database/sqlite/utils.rs @@ -340,6 +340,148 @@ fn migrate_v6_to_v7(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError Ok(()) } +fn migrate_v7_to_v8(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError> { + // This migration is done as several database transactions in order not to + // have a very large database transaction containing all rows from the + // transactions table. + const TXIDS_BATCH_SIZE: u32 = 100; + loop { + let txids = db_query( + conn, + "SELECT txid FROM transactions WHERE num_inputs IS NULL LIMIT ?1", + rusqlite::params![TXIDS_BATCH_SIZE], + |row| { + let txid: Vec = row.get(0)?; + let txid: bitcoin::Txid = bitcoin::consensus::encode::deserialize(&txid) + .expect("We only store valid txids"); + Ok(txid) + }, + )?; + if txids.is_empty() { + break; + } + for txid in &txids { + let tx = db_query_row( + conn, + "SELECT tx FROM transactions WHERE txid = ?1", + rusqlite::params![txid[..].to_vec()], + |row| { + let tx: Vec = row.get(0)?; + let tx: bitcoin::Transaction = bitcoin::consensus::encode::deserialize(&tx) + .expect("We only store valid transactions"); + Ok(tx) + }, + )?; + db_exec(conn, |db_tx| { + let updated = db_tx.execute( + "UPDATE transactions SET num_inputs = ?1, num_outputs = ?2, is_coinbase = ?3 WHERE txid = ?4", + rusqlite::params![tx.input.len(), tx.output.len(), tx.is_coinbase(), txid[..].to_vec()], + )?; + assert_eq!(updated, 1); + Ok(()) + })?; + } + } + + // Update the `is_from_self` column for all unconfirmed coins and those + // confirmed after height 0, i.e. this will act on all coins. + let prev_tip_height = 0; + // As part of the same db_tx, first make sure that all rows of the + // transactions table have been updated. + db_exec(conn, |db_tx| { + let num_txs_to_update: u32 = db_tx.query_row( + "SELECT count(txid) FROM transactions WHERE num_inputs IS NULL OR num_outputs IS NULL", + [], + |row| row.get(0), + )?; + assert_eq!(num_txs_to_update, 0); + + // This is a copy of `SqliteConn::update_coins_from_self` as of + // the time of writing this migration. We don't use that method + // directly in case the schema changes in future and it no longer + // works on the V7 schema. + + // Given the requirement for unconfirmed coins that all ancestors + // be from self, we perform the update in a loop until no further + // rows are updated in order to iterate over the unconfirmed coins. + // Although we don't expect any unconfirmed transaction to have + // more than 25 in-mempool descendants including itself, there + // could be more descendants in the DB following a reorg and a + // rollback of the tip. The max number of iterations would be + // one per unconfirmed coin not from self plus one for all + // confirmed coins. + // In any case, the query only sets `is_from_self` to 1 for + // those coins with value 0 and so the number of rows affected + // by each iteration must become 0. + let max_iterations = { + let num_unconfirmed: u64 = db_tx.query_row( + "SELECT COUNT(*) FROM coins + WHERE blockheight IS NULL AND is_from_self = 0", + [], + |row| row.get(0), + )?; + // Add 1 for the confirmed coins, which will all + // be updated in the first iteration, and another 1 + // as a final check there's nothing left to update. + num_unconfirmed.checked_add(2).expect("must fit") + }; + log::debug!( + "Updating is_from_self in up to {} iterations..", + max_iterations + ); + let mut updated = 0; + for i in 0..max_iterations { + updated = db_tx.execute( + " + UPDATE coins + SET is_from_self = 1 + FROM transactions t + INNER JOIN ( + SELECT + spend_txid, + SUM( + CASE + WHEN blockheight IS NOT NULL THEN 1 + -- If the spending coin is unconfirmed, only count + -- it as an input coin if it is from self. + WHEN blockheight IS NULL AND is_from_self = 1 THEN 1 + ELSE 0 + END + ) AS cnt + FROM coins + WHERE spend_txid IS NOT NULL + -- We only need to consider spend transactions that are + -- unconfirmed or confirmed after `prev_tip_height + -- as only these transactions will affect the coins that + -- we are updating. + AND (spend_block_height IS NULL OR spend_block_height > ?1) + GROUP BY spend_txid + ) spends + ON t.txid = spends.spend_txid AND t.num_inputs = spends.cnt + WHERE coins.txid = t.txid + AND (coins.blockheight IS NULL OR coins.blockheight > ?1) + AND coins.is_from_self = 0 + ", + [prev_tip_height], + )?; + if updated == 0 { + log::debug!("Finished updating is_from_self in {} iterations.", i + 1); + break; + } + } + assert_eq!( + updated, 0, + "no rows expected to be updated on final iteration while updating is_from_self", + ); + + // Finally update the DB version. + db_tx.execute("UPDATE version SET version = 8", [])?; + Ok(()) + })?; + + Ok(()) +} + /// Check the database version and if necessary apply the migrations to upgrade it to the current /// one. The `bitcoin_txs` parameter is here for the migration from versions 4 and earlier, which /// did not store the Bitcoin transactions in database, to versions 5 and later, which do. For a @@ -398,6 +540,11 @@ pub fn maybe_apply_migration( migrate_v6_to_v7(&mut conn)?; log::warn!("Migration from database version 6 to version 7 successful."); } + 7 => { + log::warn!("Upgrading database from version 7 to version 8."); + migrate_v7_to_v8(&mut conn)?; + log::warn!("Migration from database version 7 to version 8 successful."); + } _ => return Err(SqliteDbError::UnsupportedVersion(version)), } } diff --git a/lianad/src/testutils.rs b/lianad/src/testutils.rs index 04ae6820..75aa5ef1 100644 --- a/lianad/src/testutils.rs +++ b/lianad/src/testutils.rs @@ -484,6 +484,10 @@ impl DatabaseConnection for DummyDatabase { } } + fn update_coins_from_self(&mut self, _prev_tip_height: i32) { + // noop + } + fn list_wallet_transactions( &mut self, txids: &[bitcoin::Txid],