From 34069083a9573d11d2318718ede675b7e4fdab29 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Wed, 25 May 2022 18:56:50 +0100 Subject: [PATCH] Avoid using immature coinbase inputs Fixes #413 --- src/wallet/mod.rs | 123 ++++++++++++++++++++++++++++++++++----- src/wallet/tx_builder.rs | 11 +++- 2 files changed, 115 insertions(+), 19 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 5caf162483..d428438055 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -73,6 +73,7 @@ use crate::testutils; use crate::types::*; const CACHE_ADDR_BATCH_SIZE: u32 = 100; +const COINBASE_MATURITY: u32 = 100; /// A Bitcoin wallet /// @@ -765,6 +766,7 @@ where params.drain_wallet, params.manually_selected_only, params.bumping_fee.is_some(), // we mandate confirmed transactions if we're bumping the fee + current_height, )?; let coin_selection = coin_selection.coin_select( @@ -1335,6 +1337,7 @@ where /// Given the options returns the list of utxos that must be used to form the /// transaction and any further that may be used if needed. #[allow(clippy::type_complexity)] + #[allow(clippy::too_many_arguments)] fn preselect_utxos( &self, change_policy: tx_builder::ChangeSpendPolicy, @@ -1343,6 +1346,7 @@ where must_use_all_available: bool, manual_only: bool, must_only_use_confirmed_tx: bool, + current_height: Option, ) -> Result<(Vec, Vec), Error> { // must_spend <- manually selected utxos // may_spend <- all other available utxos @@ -1361,23 +1365,44 @@ where return Ok((must_spend, vec![])); } - let satisfies_confirmed = match must_only_use_confirmed_tx { - true => { - let database = self.database.borrow(); - may_spend - .iter() - .map(|u| { - database - .get_tx(&u.0.outpoint.txid, true) - .map(|tx| match tx { - None => false, - Some(tx) => tx.confirmation_time.is_some(), - }) + let database = self.database.borrow(); + let satisfies_confirmed = may_spend + .iter() + .map(|u| { + database + .get_tx(&u.0.outpoint.txid, true) + .map(|tx| match tx { + // We don't have the tx in the db for some reason, + // so we can't know for sure if it's mature or not. + // We prefer not to spend it. + None => false, + Some(tx) => { + // Whether the UTXO is mature and, if needed, confirmed + let mut spendable = true; + if tx + .transaction + .expect("We specifically ask for the transaction above") + .is_coin_base() + { + if let Some(current_height) = current_height { + match &tx.confirmation_time { + Some(t) => { + // https://github.com/bitcoin/bitcoin/blob/c5e67be03bb06a5d7885c55db1f016fbf2333fe3/src/validation.cpp#L373-L375 + spendable &= (current_height.saturating_sub(t.height)) + >= COINBASE_MATURITY; + } + None => spendable = false, + } + } + } + if must_only_use_confirmed_tx { + spendable &= tx.confirmation_time.is_some(); + } + spendable + } }) - .collect::, _>>()? - } - false => vec![true; may_spend.len()], - }; + }) + .collect::, _>>()?; let mut i = 0; may_spend.retain(|u| { @@ -4643,4 +4668,70 @@ pub(crate) mod test { "The signature should have been made with the right sighash" ); } + + #[test] + fn test_spend_coinbase() { + let descriptors = testutils!(@descriptors (get_test_wpkh())); + let wallet = Wallet::new( + &descriptors.0, + None, + Network::Regtest, + AnyDatabase::Memory(MemoryDatabase::new()), + ) + .unwrap(); + + let confirmation_time = 5; + + crate::populate_test_db!( + wallet.database.borrow_mut(), + testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 0)), + Some(confirmation_time), + (@coinbase true) + ); + + let not_yet_mature_time = confirmation_time + COINBASE_MATURITY - 1; + let maturity_time = confirmation_time + COINBASE_MATURITY; + + // The balance is nonzero, even if we can't spend anything + // FIXME: we should differentiate the balance between immature, + // trusted, untrusted_pending + // See https://github.com/bitcoindevkit/bdk/issues/238 + let balance = wallet.get_balance().unwrap(); + assert!(balance != 0); + + // We try to create a transaction, only to notice that all + // our funds are unspendable + let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); + let mut builder = wallet.build_tx(); + builder + .add_recipient(addr.script_pubkey(), balance / 2) + .set_current_height(confirmation_time); + assert!(matches!( + builder.finish().unwrap_err(), + Error::InsufficientFunds { + needed: _, + available: 0 + } + )); + + // Still unspendable... + let mut builder = wallet.build_tx(); + builder + .add_recipient(addr.script_pubkey(), balance / 2) + .set_current_height(not_yet_mature_time); + assert!(matches!( + builder.finish().unwrap_err(), + Error::InsufficientFunds { + needed: _, + available: 0 + } + )); + + // ...Now the coinbase is mature :) + let mut builder = wallet.build_tx(); + builder + .add_recipient(addr.script_pubkey(), balance / 2) + .set_current_height(maturity_time); + builder.finish().unwrap(); + } } diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index 5f16b8538a..09fe733553 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -547,10 +547,15 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> /// Set the current blockchain height. /// - /// This will be used to set the nLockTime for preventing fee sniping. If the current height is - /// not provided, the last sync height will be used instead. - /// + /// This will be used to: + /// 1. Set the nLockTime for preventing fee sniping. /// **Note**: This will be ignored if you manually specify a nlocktime using [`TxBuilder::nlocktime`]. + /// 2. Decide whether coinbase outputs are mature or not. If the coinbase outputs are not + /// mature at `current_height`, we ignore them in the coin selection. + /// If you want to create a transaction that spends immature coinbase inputs, manually + /// add them using [`TxBuilder::add_utxos`]. + /// + /// In both cases, if you don't provide a current height, we use the last sync height. pub fn set_current_height(&mut self, height: u32) -> &mut Self { self.params.current_height = Some(height); self