diff --git a/src/commands/mod.rs b/src/commands/mod.rs index a77fb8d2..f28cdbb4 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -472,7 +472,7 @@ impl DaemonControl { .filter_map(|(op, c)| { if c.block_info.is_some() { Some((c, None)) // confirmed coins have no ancestor info - } else if c.is_change { + } else if c.is_change && !c.is_immature { // In case the mempool_entry is None, the coin will be included without // any ancestor info. Some(( @@ -1704,7 +1704,7 @@ mod tests { txid: dummy_op.txid, vout: dummy_op.vout + 100, }; - db_conn.new_unspent_coins(&[Coin { + let unconfirmed_coin = Coin { outpoint: confirmed_op_1, is_immature: false, block_info: None, @@ -1713,7 +1713,8 @@ mod tests { is_change: true, spend_txid: None, spend_block: None, - }]); + }; + db_conn.new_unspent_coins(&[unconfirmed_coin]); // Coin selection error due to insufficient funds. assert!(matches!( control.create_spend(&destinations, &[], 1, None), @@ -1795,6 +1796,41 @@ mod tests { manual_input.sort(); assert_eq!(auto_input, manual_input); + // Now check that the spend created above using auto-selection only works when the unconfirmed coin + // is change and not immature. + // 1. not change and not immature + db_conn.remove_coins(&[unconfirmed_coin.outpoint]); + let mut unconfirmed_coin_2 = unconfirmed_coin; + unconfirmed_coin_2.is_change = false; + unconfirmed_coin_2.is_immature = false; // (this is already the case) + db_conn.new_unspent_coins(&[unconfirmed_coin_2]); + assert!(matches!( + control.create_spend(&destinations, &[], 1, None), + Ok(CreateSpendResult::InsufficientFunds { .. }), + )); + // 2. change and immature + db_conn.remove_coins(&[unconfirmed_coin_2.outpoint]); + unconfirmed_coin_2.is_change = true; + unconfirmed_coin_2.is_immature = true; + db_conn.new_unspent_coins(&[unconfirmed_coin_2]); + assert!(matches!( + control.create_spend(&destinations, &[], 1, None), + Ok(CreateSpendResult::InsufficientFunds { .. }), + )); + // 3. not change and immature + db_conn.remove_coins(&[unconfirmed_coin_2.outpoint]); + unconfirmed_coin_2.is_change = false; + unconfirmed_coin_2.is_immature = true; + db_conn.new_unspent_coins(&[unconfirmed_coin_2]); + assert!(matches!( + control.create_spend(&destinations, &[], 1, None), + Ok(CreateSpendResult::InsufficientFunds { .. }), + )); + + // Re-insert the original unconfirmed coin again. + db_conn.remove_coins(&[unconfirmed_coin_2.outpoint]); + db_conn.new_unspent_coins(&[unconfirmed_coin]); + // Now do the same again, but this time specifying the change address to be the same // as for the auto spend. let change_address = bitcoin::Address::from_script( diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 0433c42e..ad99ac55 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -43,7 +43,7 @@ use miniscript::bitcoin::{ secp256k1, }; -const DB_VERSION: i64 = 3; +const DB_VERSION: i64 = 4; #[derive(Debug)] pub enum SqliteDbError { @@ -807,6 +807,92 @@ CREATE TABLE spend_transactions ( psbt BLOB UNIQUE NOT NULL, txid BLOB UNIQUE NOT NULL ); +"; + + const V3_SCHEMA: &str = "\ +CREATE TABLE version ( + version INTEGER NOT NULL +); + +/* About the Bitcoin network. */ +CREATE TABLE tip ( + network TEXT NOT NULL, + blockheight INTEGER, + blockhash BLOB +); + +/* This stores metadata about our wallet. We only support single wallet for + * now (and the foreseeable future). + * + * The 'timestamp' field is the creation date of the wallet. We guarantee to have seen all + * information related to our descriptor(s) that occured after this date. + * The optional 'rescan_timestamp' field is a the timestamp we need to rescan the chain + * for events related to our descriptor(s) from. + */ +CREATE TABLE wallets ( + id INTEGER PRIMARY KEY NOT NULL, + timestamp INTEGER NOT NULL, + main_descriptor TEXT NOT NULL, + deposit_derivation_index INTEGER NOT NULL, + change_derivation_index INTEGER NOT NULL, + rescan_timestamp INTEGER +); + +/* Our (U)TxOs. + * + * The 'spend_block_height' and 'spend_block.time' are only present if the spending + * transaction for this coin exists and was confirmed. + * + * The 'is_immature' field is for coinbase deposits that are not yet buried under 100 + * blocks. Note coinbase deposits can't be change. They also technically can't be + * unconfirmed but we keep them as such until they become mature. + */ +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, + is_change BOOLEAN NOT NULL CHECK (is_change IN (0,1)), + spend_txid BLOB, + spend_block_height INTEGER, + spend_block_time INTEGER, + is_immature BOOLEAN NOT NULL CHECK (is_immature IN (0,1)), + CHECK (is_change IS 0 OR is_immature IS 0), + UNIQUE (txid, vout), + FOREIGN KEY (wallet_id) REFERENCES wallets (id) + ON UPDATE RESTRICT + ON DELETE RESTRICT +); + +/* A mapping from descriptor address to derivation index. Necessary until + * we can get the derivation index from the parent descriptor from bitcoind. + */ +CREATE TABLE addresses ( + receive_address TEXT NOT NULL UNIQUE, + change_address TEXT NOT NULL UNIQUE, + derivation_index INTEGER NOT NULL UNIQUE +); + +/* Transactions we created that spend some of our coins. */ +CREATE TABLE spend_transactions ( + id INTEGER PRIMARY KEY NOT NULL, + psbt BLOB UNIQUE NOT NULL, + txid BLOB UNIQUE NOT NULL, + updated_at INTEGER +); + +/* Labels applied on addresses (0), outpoints (1), txids (2) */ +CREATE TABLE labels ( + id INTEGER PRIMARY KEY NOT NULL, + wallet_id INTEGER NOT NULL, + item_kind INTEGER NOT NULL CHECK (item_kind IN (0,1,2)), + item TEXT UNIQUE NOT NULL, + value TEXT NOT NULL +); "; fn psbt_from_str(psbt_str: &str) -> Psbt { @@ -1457,7 +1543,7 @@ CREATE TABLE spend_transactions ( block_info: None, amount: bitcoin::Amount::from_sat(424242), derivation_index: bip32::ChildNumber::from_normal_idx(4103).unwrap(), - is_change: false, // Cannot be both a coinbase deposit and change. + is_change: false, spend_txid: None, spend_block: None, }; @@ -2130,7 +2216,198 @@ CREATE TABLE spend_transactions ( } #[test] - fn v0_to_v3_migration() { + fn v3_to_v4_migration() { + let secp = secp256k1::Secp256k1::verification_only(); + + // Create a database with version 3, using the old schema. + let tmp_dir = tmp_dir(); + fs::create_dir_all(&tmp_dir).unwrap(); + let db_path: path::PathBuf = [tmp_dir.as_path(), path::Path::new("lianad_v3.sqlite3")] + .iter() + .collect(); + let mut options = dummy_options(); + options.schema = V3_SCHEMA; + options.version = 3; + create_fresh_db(&db_path, options, &secp).unwrap(); + + { + // Don't use SqliteDb::new() in order not to apply migration. + let db = SqliteDb { + db_path: db_path.clone(), + }; + let mut conn = db.connection().unwrap(); + assert!(conn.db_version() == 3); + + // The following coins will be inserted into the DB as unconfirmed and then + // some of them will be subsequently confirmed and spent. + // Note that `block_info`, `spend_txid` and `spend_block` will all be set to + // NULL in the DB by the `new_unspent_coins` method, but are set to `None` + // here anyway. + let coin_a = Coin { + outpoint: bitcoin::OutPoint::from_str( + "6f0dc85a369b44458eba3a1f0ea5b5935d563afb6994f70f5b0094e05be1676c:1", + ) + .unwrap(), + is_immature: false, + amount: bitcoin::Amount::from_sat(1231001), + derivation_index: bip32::ChildNumber::from_normal_idx(101).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_b = Coin { + outpoint: bitcoin::OutPoint::from_str( + "81b2f327d4c1fd67afd039374f8798fd9ff37932c6f5c221c1c569350eac5ac8:19234", + ) + .unwrap(), + is_immature: false, + amount: bitcoin::Amount::from_sat(23145), + derivation_index: bip32::ChildNumber::from_normal_idx(10).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_c = Coin { + outpoint: bitcoin::OutPoint::from_str( + "7477017f992cdc7ba08acafb77cb3b5bc0f42ac340d3e1e1da0785bdda20d5f6:932", + ) + .unwrap(), + is_immature: false, + amount: bitcoin::Amount::from_sat(354764), + derivation_index: bip32::ChildNumber::from_normal_idx(3401).unwrap(), + is_change: true, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_d = Coin { + outpoint: bitcoin::OutPoint::from_str( + "ed6c8f1af9325f84de521e785e7ddfd33dc28c9ada4d687dcd3850100bde54e9:1456", + ) + .unwrap(), + is_immature: false, + amount: bitcoin::Amount::from_sat(23200), + derivation_index: bip32::ChildNumber::from_normal_idx(4793235).unwrap(), + is_change: true, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_e = Coin { + outpoint: bitcoin::OutPoint::from_str( + "4753a1d74c0af8dd0a0f3b763c14faf3bd9ed03cbdf33337a074fb0e9f6c7810:4633", + ) + .unwrap(), + is_immature: false, + amount: bitcoin::Amount::from_sat(675000), + derivation_index: bip32::ChildNumber::from_normal_idx(3).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_imma_a = Coin { + outpoint: bitcoin::OutPoint::from_str( + "c449539458c60bee6c0d8905ba1dadb20b9187b82045d306a408b894cea492b0:5", + ) + .unwrap(), + is_immature: true, + amount: bitcoin::Amount::from_sat(4564347), + derivation_index: bip32::ChildNumber::from_normal_idx(453).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + let coin_imma_b = Coin { + outpoint: bitcoin::OutPoint::from_str( + "f0801fd9ca8bca0624c230ab422b2e2c4c8dc995e4e1dbc6412510959cce1e4f:19234", + ) + .unwrap(), + is_immature: true, + amount: bitcoin::Amount::from_sat(731453), + derivation_index: bip32::ChildNumber::from_normal_idx(98).unwrap(), + is_change: false, + block_info: None, + spend_txid: None, + spend_block: None, + }; + // After the following operations, the state of the coins will be: + // - coin_a is spent. + // - coin_b is confirmed and spending. + // - coin_c is confirmed. + // - coin_d is the unconfirmed output of coin_b's spend and is spending. + // - coin_e is the unconfirmed output of coin_d's spend. + // - coin_imma_a is confirmed. + // - coin_imma_b is still immature. + conn.new_unspent_coins(&[ + coin_a, + coin_b, + coin_c, + coin_d, + coin_e, + coin_imma_a, + coin_imma_b, + ]); + conn.confirm_coins(&[ + (coin_a.outpoint, 175500, 1755001001), + (coin_b.outpoint, 175502, 1755001032), + (coin_c.outpoint, 175504, 1755005032), + (coin_imma_a.outpoint, 176001, 1755001004), + ]); + conn.spend_coins(&[ + ( + coin_a.outpoint, + bitcoin::Txid::from_slice(&[1; 32][..]).unwrap(), + ), + (coin_b.outpoint, coin_d.outpoint.txid), + (coin_d.outpoint, coin_e.outpoint.txid), + ]); + conn.confirm_spend(&[( + coin_a.outpoint, + bitcoin::Txid::from_slice(&[1; 32][..]).unwrap(), + 245500, + 1755003000, + )]); + assert_eq!(conn.coins(&[CoinStatus::Unconfirmed], &[]).len(), 2); + assert_eq!(conn.coins(&[CoinStatus::Confirmed], &[]).len(), 2); + assert_eq!(conn.coins(&[CoinStatus::Spending], &[]).len(), 2); + assert_eq!(conn.coins(&[CoinStatus::Spent], &[]).len(), 1); + let coins_pre = conn.coins(&[], &[]); + assert_eq!(coins_pre.len(), 7); + assert_eq!( + coins_pre + .iter() + .filter(|c| c.is_immature) + .collect::>() + .len(), + 1 + ); + assert_eq!( + coins_pre + .iter() + .filter(|c| c.is_change) + .collect::>() + .len(), + 2 + ); + + // Migrate the DB. + maybe_apply_migration(&db_path).unwrap(); + assert!(conn.db_version() == 4); + maybe_apply_migration(&db_path).unwrap(); // Migrating twice will be a no-op. + assert!(conn.db_version() == 4); + let coins_post = conn.coins(&[], &[]); + assert_eq!(coins_pre, coins_post); + } + + fs::remove_dir_all(tmp_dir).unwrap(); + } + + #[test] + fn v0_to_v4_migration() { let secp = secp256k1::Secp256k1::verification_only(); // Create a database with version 0, using the old schema. @@ -2150,7 +2427,7 @@ CREATE TABLE spend_transactions ( { let mut conn = db.connection().unwrap(); let version = conn.db_version(); - assert_eq!(version, 3); + assert_eq!(version, 4); let txid_str = "0c62a990d20d54429e70859292e82374ba6b1b951a3ab60f26bb65fee5724ff7"; let txid = LabelItem::from_str(txid_str, bitcoin::Network::Bitcoin).unwrap(); diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 6add975a..ac101bcd 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -39,8 +39,8 @@ CREATE TABLE wallets ( * transaction for this coin exists and was confirmed. * * The 'is_immature' field is for coinbase deposits that are not yet buried under 100 - * blocks. Note coinbase deposits can't be change. They also technically can't be - * unconfirmed but we keep them as such until they become mature. + * blocks. Note coinbase deposits can't technically be unconfirmed but we keep them + * as such until they become mature. */ CREATE TABLE coins ( id INTEGER PRIMARY KEY NOT NULL, @@ -56,7 +56,6 @@ CREATE TABLE coins ( spend_block_height INTEGER, spend_block_time INTEGER, is_immature BOOLEAN NOT NULL CHECK (is_immature IN (0,1)), - CHECK (is_change IS 0 OR is_immature IS 0), UNIQUE (txid, vout), FOREIGN KEY (wallet_id) REFERENCES wallets (id) ON UPDATE RESTRICT @@ -217,10 +216,6 @@ impl TryFrom<&rusqlite::Row<'_>> for DbCoin { }); let is_immature: bool = row.get(12)?; - assert!( - !is_immature || !is_change, - "A coin cannot be both created in a coinbase and be change" - ); Ok(DbCoin { id, diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index efe92644..e1981ee4 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -194,6 +194,41 @@ fn migrate_v2_to_v3(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError Ok(()) } +fn migrate_v3_to_v4(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError> { + db_exec(conn, |tx| { + tx.execute_batch( + "CREATE TABLE coins_new ( + 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, + is_change BOOLEAN NOT NULL CHECK (is_change IN (0,1)), + spend_txid BLOB, + spend_block_height INTEGER, + spend_block_time INTEGER, + is_immature BOOLEAN NOT NULL CHECK (is_immature IN (0,1)), + UNIQUE (txid, vout), + FOREIGN KEY (wallet_id) REFERENCES wallets (id) + ON UPDATE RESTRICT + ON DELETE RESTRICT + ); + + INSERT INTO coins_new SELECT * FROM coins; + + DROP TABLE coins; + + ALTER TABLE coins_new RENAME TO coins; + + UPDATE version SET version = 4;", + ) + })?; + Ok(()) +} + /// Check the database version and if necessary apply the migrations to upgrade it to the current /// one. pub fn maybe_apply_migration(db_path: &path::Path) -> Result<(), SqliteDbError> { @@ -222,6 +257,11 @@ pub fn maybe_apply_migration(db_path: &path::Path) -> Result<(), SqliteDbError> migrate_v2_to_v3(&mut conn)?; log::warn!("Migration from database version 2 to version 3 successful."); } + 3 => { + log::warn!("Upgrading database from version 3 to version 4."); + migrate_v3_to_v4(&mut conn)?; + log::warn!("Migration from database version 3 to version 4 successful."); + } _ => return Err(SqliteDbError::UnsupportedVersion(version)), } } diff --git a/tests/test_misc.py b/tests/test_misc.py index f4a81c5f..c7f29a6d 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -239,6 +239,20 @@ def test_coinbase_deposit(lianad, bitcoind): and coin["spend_info"] is not None ) + # We must also properly detect coinbase deposits to a change address. We used to have + # an assertion that a coin cannot both be change and a coinbase deposit. Since change + # is determined by the address... Technically we can. + change_desc = lianad.multi_desc.singlepath_descriptors()[1] + change_addr = bitcoind.rpc.deriveaddresses(str(change_desc), [0, 0])[0] + bitcoind.rpc.generatetoaddress(1, change_addr) + wait_for(lambda: any(c["is_immature"] for c in lianad.rpc.listcoins()["coins"])) + coin = next(c for c in lianad.rpc.listcoins()["coins"] if c["is_immature"]) + assert coin["is_change"] + bitcoind.generate_block(100) + wait_for_sync() + coin = next(c for c in lianad.rpc.listcoins()["coins"] if c["outpoint"] == coin["outpoint"]) + assert not coin["is_immature"] and coin["block_height"] is not None + @pytest.mark.skipif( OLD_LIANAD_PATH is None or USE_TAPROOT, reason="Need the old lianad binary to create the datadir."