diff --git a/doc/API.md b/doc/API.md index 943556cd..68c7a1f6 100644 --- a/doc/API.md +++ b/doc/API.md @@ -96,6 +96,7 @@ This command does not take any parameter for now. | `outpoint` | string | Transaction id and output index of this coin. | | `block_height` | int or null | Block height the transaction was confirmed at, or `null`. | | `spend_info` | object | Information about the transaction spending this coin. See [Spending transaction info](#spending_transaction_info). | +| `is_immature` | bool | Whether this coin was created by a coinbase transaction that is still immature. | ##### Spending transaction info diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 2af4690c..73e959dd 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -1111,6 +1111,7 @@ pub struct LSBlockEntry { pub block_height: Option, pub address: bitcoin::Address, pub parent_descs: Vec>, + pub is_immature: bool, } impl From<&Json> for LSBlockEntry { @@ -1156,12 +1157,19 @@ impl From<&Json> for LSBlockEntry { }) .expect("bitcoind can't give invalid descriptors"); + let is_immature = json + .get("category") + .and_then(Json::as_str) + .expect("must be present") + == "immature"; + LSBlockEntry { outpoint, amount, block_height, address, parent_descs, + is_immature, } } } @@ -1189,7 +1197,7 @@ impl From for LSBlockRes { .get("category") .and_then(Json::as_str) .expect("must be present"); - if category == "receive" || category == "generate" { + if ["receive", "generate", "immature"].contains(&category) { let lsb_entry: LSBlockEntry = j.into(); Some(lsb_entry) } else { @@ -1207,6 +1215,8 @@ pub struct GetTxRes { pub conflicting_txs: Vec, pub block: Option, pub tx: bitcoin::Transaction, + pub is_coinbase: bool, + pub confirmations: i32, } impl From for GetTxRes { @@ -1244,10 +1254,21 @@ impl From for GetTxRes { let bytes = Vec::from_hex(hex).expect("bitcoind returned a wrong transaction format"); let tx: bitcoin::Transaction = bitcoin::consensus::encode::deserialize(&bytes) .expect("bitcoind returned a wrong transaction format"); + let is_coinbase = json + .get("generated") + .and_then(Json::as_bool) + .unwrap_or(false); + let confirmations = json + .get("confirmations") + .and_then(Json::as_i64) + .expect("Must be present in the response") as i32; + GetTxRes { conflicting_txs: conflicting_txs.unwrap_or_default(), block, tx, + is_coinbase, + confirmations, } } } diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index a3b9e01a..b1d32fae 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -14,6 +14,8 @@ use std::{fmt, sync}; use miniscript::bitcoin::{self, address}; +const COINBASE_MATURITY: i32 = 100; + /// Information about a block #[derive(Debug, Clone, Eq, PartialEq, Copy)] pub struct Block { @@ -146,6 +148,7 @@ impl BitcoinInterface for d::BitcoinD { block_height, address, parent_descs, + is_immature, } = entry; if parent_descs .iter() @@ -156,6 +159,7 @@ impl BitcoinInterface for d::BitcoinD { amount, block_height, address, + is_immature, }) } else { None @@ -184,6 +188,11 @@ impl BitcoinInterface for d::BitcoinD { // If the transaction was confirmed, mark the coin as such. if let Some(block) = res.block { + // Do not mark immature coinbase deposits as confirmed until they become mature. + if res.is_coinbase && res.confirmations < COINBASE_MATURITY { + log::debug!("Coin at '{}' comes from an immature coinbase transaction with {} confirmations. Not marking it as confirmed for now.", op, res.confirmations); + continue; + } confirmed.push((*op, block.height, block.time)); continue; } @@ -409,4 +418,5 @@ pub struct UTxO { pub amount: bitcoin::Amount, pub block_height: Option, pub address: bitcoin::Address, + pub is_immature: bool, } diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index c3f246d8..a1726780 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -24,6 +24,8 @@ struct UpdatedCoins { // or spent. // NOTE: A coin may be updated multiple times at once. That is, a coin may be received, confirmed, // and spent in a single poll. +// NOTE: Coinbase transaction deposits are very much an afterthought here. We treat them as +// unconfirmed until the CB tx matures. fn update_coins( bit: &impl BitcoinInterface, db_conn: &mut Box, @@ -42,6 +44,7 @@ fn update_coins( outpoint, amount, address, + is_immature, .. } = utxo; // We can only really treat them if we know the derivation index that was used. @@ -66,6 +69,7 @@ fn update_coins( if !curr_coins.contains_key(&utxo.outpoint) { let coin = Coin { outpoint, + is_immature, amount, derivation_index, is_change, diff --git a/src/commands/mod.rs b/src/commands/mod.rs index e5ca6322..168b1f1f 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -50,6 +50,7 @@ pub enum CommandError { InvalidFeerate(/* sats/vb */ u64), UnknownOutpoint(bitcoin::OutPoint), AlreadySpent(bitcoin::OutPoint), + ImmatureCoinbase(bitcoin::OutPoint), Address(bitcoin::address::Error), InvalidOutputValue(bitcoin::Amount), InsufficientFunds( @@ -77,6 +78,7 @@ impl fmt::Display for CommandError { Self::NoOutpoint => write!(f, "No provided outpoint. Need at least one."), Self::InvalidFeerate(sats_vb) => write!(f, "Invalid feerate: {} sats/vb.", sats_vb), Self::AlreadySpent(op) => write!(f, "Coin at '{}' is already spent.", op), + Self::ImmatureCoinbase(op) => write!(f, "Coin at '{}' is from an immature coinbase transaction.", op), Self::UnknownOutpoint(op) => write!(f, "Unknown outpoint '{}'.", op), Self::Address(e) => write!( f, @@ -298,6 +300,7 @@ impl DaemonControl { block_info, spend_txid, spend_block, + is_immature, .. } = coin; let spend_info = spend_txid.map(|txid| LCSpendInfo { @@ -310,6 +313,7 @@ impl DaemonControl { outpoint, block_height, spend_info, + is_immature, } }) .collect(); @@ -349,6 +353,10 @@ impl DaemonControl { if coin.is_spent() { return Err(CommandError::AlreadySpent(*op)); } + if coin.is_immature { + return Err(CommandError::ImmatureCoinbase(*op)); + } + // Fetch the transaction that created it if necessary if !spent_txs.contains_key(op) { let tx = self @@ -840,6 +848,8 @@ pub struct ListCoinsEntry { pub block_height: Option, /// Information about the transaction spending this coin. pub spend_info: Option, + /// Whether this coin was created by a coinbase transaction that is still immature. + pub is_immature: bool, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -976,6 +986,7 @@ mod tests { let mut db_conn = control.db().lock().unwrap().connection(); db_conn.new_unspent_coins(&[Coin { outpoint: dummy_op, + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(100_000), derivation_index: bip32::ChildNumber::from(13), @@ -1081,6 +1092,7 @@ mod tests { }; db_conn.new_unspent_coins(&[Coin { outpoint: dummy_op_dup, + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(400_000), derivation_index: bip32::ChildNumber::from(42), @@ -1095,6 +1107,26 @@ mod tests { ))) ); + // Can't create a transaction that spends an immature coinbase deposit. + let imma_op = bitcoin::OutPoint::from_str( + "4753a1d74c0af8dd0a0f3b763c14faf3bd9ed03cbdf33337a074fb0e9f6c7810:0", + ) + .unwrap(); + db_conn.new_unspent_coins(&[Coin { + outpoint: imma_op, + is_immature: true, + block_info: None, + amount: bitcoin::Amount::from_sat(100_000), + derivation_index: bip32::ChildNumber::from(13), + is_change: false, + spend_txid: None, + spend_block: None, + }]); + assert_eq!( + control.create_spend(&destinations, &[imma_op], 1_001), + Err(CommandError::ImmatureCoinbase(imma_op)) + ); + ms.shutdown(); } @@ -1127,6 +1159,7 @@ mod tests { db_conn.new_unspent_coins(&[ Coin { outpoint: dummy_op_a, + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(100_000), derivation_index: bip32::ChildNumber::from(13), @@ -1136,6 +1169,7 @@ mod tests { }, Coin { outpoint: dummy_op_b, + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(115_680), derivation_index: bip32::ChildNumber::from(34), @@ -1303,6 +1337,7 @@ mod tests { // Deposit 1 Coin { is_change: false, + is_immature: false, outpoint: OutPoint { txid: deposit1.txid(), vout: 0, @@ -1316,6 +1351,7 @@ mod tests { // Deposit 2 Coin { is_change: false, + is_immature: false, outpoint: OutPoint { txid: deposit2.txid(), vout: 0, @@ -1329,6 +1365,7 @@ mod tests { // This coin is a change output. Coin { is_change: true, + is_immature: false, outpoint: OutPoint::new(spend_tx.txid(), 1), block_info: Some(BlockInfo { height: 3, time: 3 }), spend_block: None, @@ -1339,6 +1376,7 @@ mod tests { // Deposit 3 Coin { is_change: false, + is_immature: false, outpoint: OutPoint { txid: deposit3.txid(), vout: 0, diff --git a/src/database/mod.rs b/src/database/mod.rs index 23daf8c2..21b5a32d 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -91,6 +91,9 @@ pub trait DatabaseConnection { fn remove_coins(&mut self, coins: &[bitcoin::OutPoint]); /// Mark a set of coins as being confirmed at a specified height and block time. + /// NOTE: if the coin comes from an immature coinbase transaction, this will mark it as mature. + /// Immature coinbase deposits must not be confirmed before they are 100 blocks deep in the + /// chain. fn confirm_coins(&mut self, outpoints: &[(bitcoin::OutPoint, i32, u32)]); /// Mark a set of coins as being spent by a specified txid of a pending transaction. @@ -281,6 +284,7 @@ impl From for BlockInfo { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Coin { pub outpoint: bitcoin::OutPoint, + pub is_immature: bool, pub block_info: Option, pub amount: bitcoin::Amount, pub derivation_index: bip32::ChildNumber, @@ -293,6 +297,7 @@ impl std::convert::From for Coin { fn from(db_coin: DbCoin) -> Coin { let DbCoin { outpoint, + is_immature, block_info, amount, derivation_index, @@ -303,6 +308,7 @@ impl std::convert::From for Coin { } = db_coin; Coin { outpoint, + is_immature, block_info: block_info.map(BlockInfo::from), amount, derivation_index, diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 2e63ec12..d05e4663 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -35,7 +35,7 @@ use miniscript::bitcoin::{ secp256k1, }; -const DB_VERSION: i64 = 1; +const DB_VERSION: i64 = 2; #[derive(Debug)] pub enum SqliteDbError { @@ -383,8 +383,8 @@ impl SqliteConn { for coin in coins { let deriv_index: u32 = coin.derivation_index.into(); db_tx.execute( - "INSERT INTO coins (wallet_id, txid, vout, amount_sat, derivation_index, is_change) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6)", + "INSERT INTO coins (wallet_id, txid, vout, amount_sat, derivation_index, is_change, is_immature) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", rusqlite::params![ WALLET_ID, coin.outpoint.txid[..].to_vec(), @@ -392,6 +392,7 @@ impl SqliteConn { coin.amount.to_sat(), deriv_index, coin.is_change, + coin.is_immature, ], )?; } @@ -416,6 +417,9 @@ impl SqliteConn { } /// Mark a set of coins as confirmed. + /// + /// NOTE: this will also mark the coin as mature if it originates from an immature coinbase + /// deposit. pub fn confirm_coins<'a>( &mut self, outpoints: impl IntoIterator, @@ -423,7 +427,7 @@ impl SqliteConn { db_exec(&mut self.conn, |db_tx| { for (outpoint, height, time) in outpoints { db_tx.execute( - "UPDATE coins SET blockheight = ?1, blocktime = ?2 WHERE txid = ?3 AND vout = ?4", + "UPDATE coins SET blockheight = ?1, blocktime = ?2, is_immature = 0 WHERE txid = ?3 AND vout = ?4", rusqlite::params![height, time, outpoint.txid[..].to_vec(), outpoint.vout,], )?; } @@ -593,11 +597,12 @@ impl SqliteConn { .expect("Db must not fail"); } + // TODO: mark coinbase deposits that were mature and became immature as such. /// Unconfirm all data that was marked as being confirmed *after* the given chain /// tip, and set it as our new best block seen. /// /// This includes: - /// - Coins + /// - Coins (coinbase deposits that became immature isn't currently implemented) /// - Spending transactions confirmation /// - Tip /// @@ -823,6 +828,7 @@ CREATE TABLE spend_transactions ( "6f0dc85a369b44458eba3a1f0ea5b5935d563afb6994f70f5b0094e05be1676c:1", ) .unwrap(), + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(98765), derivation_index: bip32::ChildNumber::from_normal_idx(10).unwrap(), @@ -855,6 +861,7 @@ CREATE TABLE spend_transactions ( "61db3e276b095e5b05f1849dd6bfffb4e7e5ec1c4a4210099b98fce01571936f:12", ) .unwrap(), + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(1111), derivation_index: bip32::ChildNumber::from_normal_idx(103).unwrap(), @@ -949,6 +956,37 @@ CREATE TABLE spend_transactions ( assert!(coin.spend_block.is_some()); assert_eq!(coin.spend_block.as_ref().unwrap().time, time); assert_eq!(coin.spend_block.unwrap().height, height); + + // Add an immature coin. As all coins it's first registered as unconfirmed (even though + // it's not). + let coin_imma = Coin { + outpoint: bitcoin::OutPoint::from_str( + "61db3e276b095e5b05f1849dd6bfffb4e7e5ec1c4a4210099b98fce01571937a:42", + ) + .unwrap(), + is_immature: true, + 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. + spend_txid: None, + spend_block: None, + }; + conn.new_unspent_coins(&[coin_imma]); + let outpoints: HashSet = conn + .coins(CoinType::All) + .into_iter() + .map(|c| c.outpoint) + .collect(); + assert!(outpoints.contains(&coin_imma.outpoint)); + let coin = conn.db_coins(&[coin_imma.outpoint]).pop().unwrap(); + assert!(coin.is_immature && !coin.is_change); + + // Confirming an immature coin marks it as mature. + let (height, time) = (424242, 424241); + conn.confirm_coins(&[(coin_imma.outpoint, height, time)]); + let coin = conn.db_coins(&[coin_imma.outpoint]).pop().unwrap(); + assert!(!coin.is_immature); } fs::remove_dir_all(tmp_dir).unwrap(); @@ -1084,12 +1122,14 @@ CREATE TABLE spend_transactions ( // - One confirmed before the rollback height but spent after // - One confirmed after the rollback height // - One spent after the rollback height + // TODO: immature deposits let coins = [ Coin { outpoint: bitcoin::OutPoint::from_str( "6f0dc85a369b44458eba3a1f0ea5b5935d563afb6994f70f5b0094e05be1676c:1", ) .unwrap(), + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(98765), derivation_index: bip32::ChildNumber::from_normal_idx(10).unwrap(), @@ -1102,6 +1142,7 @@ CREATE TABLE spend_transactions ( "c449539458c60bee6c0d8905ba1dadb20b9187b82045d306a408b894cea492b0:2", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_095, time: 1_111_899, @@ -1117,6 +1158,7 @@ CREATE TABLE spend_transactions ( "f0801fd9ca8bca0624c230ab422b2e2c4c8dc995e4e1dbc6412510959cce1e4f:3", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_099, time: 1_121_899, @@ -1140,6 +1182,7 @@ CREATE TABLE spend_transactions ( "19f56e65069f0a7a3bfb00c6a7085cc0669e03e91befeca1ee9891c9e737b2fb:4", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_100, time: 1_131_899, @@ -1155,6 +1198,7 @@ CREATE TABLE spend_transactions ( "ed6c8f1af9325f84de521e785e7ddfd33dc28c9ada4d687dcd3850100bde54e9:5", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_102, time: 1_134_899, @@ -1303,6 +1347,7 @@ CREATE TABLE spend_transactions ( "6f0dc85a369b44458eba3a1f0ea5b5935d563afb6994f70f5b0094e05be1676c:1", ) .unwrap(), + is_immature: false, block_info: None, amount: bitcoin::Amount::from_sat(98765), derivation_index: bip32::ChildNumber::from_normal_idx(10).unwrap(), @@ -1315,6 +1360,7 @@ CREATE TABLE spend_transactions ( "c449539458c60bee6c0d8905ba1dadb20b9187b82045d306a408b894cea492b0:2", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_095, time: 1_121_000, @@ -1330,6 +1376,7 @@ CREATE TABLE spend_transactions ( "f0801fd9ca8bca0624c230ab422b2e2c4c8dc995e4e1dbc6412510959cce1e4f:3", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_099, time: 1_122_000, @@ -1353,6 +1400,7 @@ CREATE TABLE spend_transactions ( "19f56e65069f0a7a3bfb00c6a7085cc0669e03e91befeca1ee9891c9e737b2fb:4", ) .unwrap(), + is_immature: true, block_info: Some(BlockInfo { height: 101_100, time: 1_124_000, @@ -1368,6 +1416,7 @@ CREATE TABLE spend_transactions ( "ed6c8f1af9325f84de521e785e7ddfd33dc28c9ada4d687dcd3850100bde54e9:5", ) .unwrap(), + is_immature: false, block_info: Some(BlockInfo { height: 101_102, time: 1_125_000, @@ -1448,7 +1497,7 @@ CREATE TABLE spend_transactions ( } #[test] - fn v0_to_v1_migration() { + fn v0_to_v2_migration() { let secp = secp256k1::Secp256k1::verification_only(); // Create a database with version 0, using the old schema. @@ -1490,11 +1539,66 @@ CREATE TABLE spend_transactions ( store_spend_old(&mut conn, &first_psbt); } - // Migrate the DB. We should be able to insert another PSBT, to query both, and the first - // PSBT must have no associated timestamp. + // The helper that was used to store coins in previous versions of the software, stripped + // down to a single coin. + fn store_coin_old( + conn: &mut rusqlite::Connection, + outpoint: &bitcoin::OutPoint, + amount: bitcoin::Amount, + derivation_index: bip32::ChildNumber, + is_change: bool, + ) { + db_exec(conn, |db_tx| { + let deriv_index: u32 = derivation_index.into(); + db_tx.execute( + "INSERT INTO coins (wallet_id, txid, vout, amount_sat, derivation_index, is_change) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6)", + rusqlite::params![ + WALLET_ID, + outpoint.txid[..].to_vec(), + outpoint.vout, + amount.to_sat(), + deriv_index, + is_change, + ], + )?; + Ok(()) + }) + .expect("Database must be available") + } + + // Store a couple coins before the migration. + { + let mut conn = rusqlite::Connection::open(&db_path).unwrap(); + store_coin_old( + &mut conn, + &bitcoin::OutPoint::from_str( + "ed6c8f1af9325f84de521e785e7ddfd33dc28c9ada4d687dcd3850100bde54e9:5", + ) + .unwrap(), + bitcoin::Amount::from_sat(14_000), + 24.into(), + true, + ); + store_coin_old( + &mut conn, + &bitcoin::OutPoint::from_str( + "81b2f327d4c1fd67afd039374f8798fd9ff37932c6f5c221c1c569350eac5ac8:2", + ) + .unwrap(), + bitcoin::Amount::from_sat(392_093_123), + 24_567.into(), + false, + ); + } + + // Migrate the DB. maybe_apply_migration(&db_path).unwrap(); maybe_apply_migration(&db_path).unwrap(); // Migrating twice will be a no-op. let db = SqliteDb::new(db_path, None, &secp).unwrap(); + + // We should now be able to insert another PSBT, to query both, and the first PSBT must + // have no associated timestamp. { let mut conn = db.connection().unwrap(); conn.store_spend(&second_psbt); @@ -1511,6 +1615,28 @@ CREATE TABLE spend_transactions ( assert!(second_spend.updated_at.is_some()); } + // We should now be able to store an immature coin, query all of them, and the first two + // should not be immature. + { + let mut conn = db.connection().unwrap(); + conn.new_unspent_coins(&[Coin { + outpoint: bitcoin::OutPoint::from_str( + "6f0dc85a369b44458eba3a1f0ea5b5935d563afb6994f70f5b0094e05be1676c:1", + ) + .unwrap(), + is_immature: true, + block_info: None, + amount: bitcoin::Amount::from_sat(98765), + derivation_index: bip32::ChildNumber::from_normal_idx(10).unwrap(), + is_change: false, + spend_txid: None, + spend_block: None, + }]); + let coins = conn.coins(CoinType::All); + assert_eq!(coins.len(), 3); + assert_eq!(coins.iter().filter(|c| !c.is_immature).count(), 2); + } + fs::remove_dir_all(tmp_dir).unwrap(); } } diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 93e101e7..b4b4b7b7 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -39,6 +39,10 @@ CREATE TABLE wallets ( * * 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, @@ -53,6 +57,8 @@ CREATE TABLE coins ( 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 @@ -156,6 +162,8 @@ pub struct DbBlockInfo { pub struct DbCoin { pub id: i64, pub wallet_id: i64, + /// Whether this coin was created by a yet-to-be-mature coinbase transaction. + pub is_immature: bool, pub outpoint: bitcoin::OutPoint, pub block_info: Option, pub amount: bitcoin::Amount, @@ -201,9 +209,16 @@ impl TryFrom<&rusqlite::Row<'_>> for DbCoin { time: spend_time.expect("Must be there if height is"), }); + 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, wallet_id, + is_immature, outpoint, block_info, amount, diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index 073fbe7f..a782036f 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -165,6 +165,25 @@ fn migrate_v0_to_v1(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError Ok(()) } +// After Liana 1.0 we upgraded the schema to record whether a coin originated from an immature +// coinbase transaction. +fn migrate_v1_to_v2(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError> { + db_exec(conn, |tx| { + tx.execute( + "ALTER TABLE coins ADD COLUMN is_immature", + rusqlite::params![], + )?; + tx.execute( + "UPDATE coins SET is_immature = 0 WHERE is_immature IS NULL", + rusqlite::params![], + )?; + tx.execute("UPDATE version SET version = 2", rusqlite::params![])?; + Ok(()) + })?; + + 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> { @@ -183,6 +202,11 @@ pub fn maybe_apply_migration(db_path: &path::Path) -> Result<(), SqliteDbError> migrate_v0_to_v1(&mut conn)?; log::warn!("Migration from database version 0 to version 1 successful."); } + 1 => { + log::warn!("Upgrading database from version 1 to version 2."); + migrate_v1_to_v2(&mut conn)?; + log::warn!("Migration from database version 1 to version 2 successful."); + } _ => return Err(SqliteDbError::UnsupportedVersion(version)), } } diff --git a/src/jsonrpc/mod.rs b/src/jsonrpc/mod.rs index 440e776b..6af7b9e8 100644 --- a/src/jsonrpc/mod.rs +++ b/src/jsonrpc/mod.rs @@ -155,6 +155,7 @@ impl From for Error { | commands::CommandError::UnknownOutpoint(..) | commands::CommandError::InvalidFeerate(..) | commands::CommandError::AlreadySpent(..) + | commands::CommandError::ImmatureCoinbase(..) | commands::CommandError::Address(..) | commands::CommandError::InvalidOutputValue(..) | commands::CommandError::InsufficientFunds(..) diff --git a/tests/test_misc.py b/tests/test_misc.py index 92394c1a..189b85ca 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -2,7 +2,7 @@ import pytest from fixtures import * from test_framework.serializations import PSBT -from test_framework.utils import wait_for, RpcError, OLD_LIANAD_PATH, LIANAD_PATH +from test_framework.utils import wait_for, RpcError, OLD_LIANAD_PATH, LIANAD_PATH, COIN from threading import Thread @@ -182,16 +182,44 @@ def test_multipath(lianad_multipath, bitcoind): def test_coinbase_deposit(lianad, bitcoind): """Check we detect deposits from (mature) coinbase transactions.""" - # Create a new deposit in a coinbase transaction. + wait_for_sync = lambda: wait_for( + lambda: lianad.rpc.getinfo()["block_height"] == bitcoind.rpc.getblockcount() + ) + wait_for_sync() + + # Create a new deposit in a coinbase transaction. We must detect it and treat it as immature. addr = lianad.rpc.getnewaddress()["address"] bitcoind.rpc.generatetoaddress(1, addr) - assert len(lianad.rpc.listcoins()["coins"]) == 0 + wait_for_sync() + coins = lianad.rpc.listcoins()["coins"] + assert ( + len(coins) == 1 and coins[0]["is_immature"] and coins[0]["spend_info"] is None + ) - # Generate 100 blocks to make the coinbase mature. + # Generate 100 blocks to make the coinbase mature. We should detect it as such. bitcoind.generate_block(100) + wait_for_sync() + coin = lianad.rpc.listcoins()["coins"][0] + assert not coin["is_immature"] and coin["block_height"] is not None - # We must have detected a new deposit. - wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 1) + # We must be able to spend the mature coin. + destinations = {bitcoind.rpc.getnewaddress(): int(0.999999 * COIN)} + res = lianad.rpc.createspend(destinations, [coin["outpoint"]], 42) + psbt = PSBT.from_base64(res["psbt"]) + txid = psbt.tx.txid().hex() + signed_psbt = lianad.signer.sign_psbt(psbt) + lianad.rpc.updatespend(signed_psbt.to_base64()) + lianad.rpc.broadcastspend(txid) + bitcoind.generate_block(1, wait_for_mempool=txid) + 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 + and coin["spend_info"] is not None + ) @pytest.mark.skipif( @@ -204,13 +232,16 @@ def test_migration(lianad_multisig, bitcoind): # Set the old binary and re-create the datadir. lianad.cmd_line[0] = OLD_LIANAD_PATH lianad.restart_fresh(bitcoind) - assert lianad.rpc.getinfo()["version"] == "0.3.0" + old_lianad_ver = lianad.rpc.getinfo()["version"] + assert old_lianad_ver in ["0.3.0", "1.0.0"] # Perform some transactions. On Liana v0.3 there was no "updated_at" for Spend # transaction drafts. receive_and_send(lianad, bitcoind) spend_txs = lianad.rpc.listspendtxs()["spend_txs"] - assert len(spend_txs) == 2 and all("updated_at" not in s for s in spend_txs) + assert len(spend_txs) == 2 + if old_lianad_ver == "0.3.0": + assert all("updated_at" not in s for s in spend_txs) # Set back the new binary. We should be able to read and, if necessary, upgrade # the old database and generally all files from the datadir. diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 4d1cf06b..119b04c9 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -132,6 +132,16 @@ def test_create_spend(lianad, bitcoind): # We can sign it and broadcast it. sign_and_broadcast(lianad, bitcoind, PSBT.from_base64(res["psbt"])) + # Try creating a transaction that spends an immature coinbase deposit. + addr = lianad.rpc.getnewaddress()["address"] + bitcoind.rpc.generatetoaddress(1, addr) + wait_for( + lambda: lianad.rpc.getinfo()["block_height"] == bitcoind.rpc.getblockcount() + ) + imma_coin = next(c for c in lianad.rpc.listcoins()["coins"] if c["is_immature"]) + with pytest.raises(RpcError, match=".*is from an immature coinbase transaction."): + lianad.rpc.createspend(destinations, [imma_coin["outpoint"]], 1) + def test_list_spend(lianad, bitcoind): # Start by creating two conflicting Spend PSBTs. The first one will have a change