frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoindevkit/bdk#614: Avoid using immature coinbase inputs

Browse Source 
e85aa247cb85601e96f4d82b7a996f709559223f Avoid using immature coinbase inputs (Daniela Brozzoni)
0e0d5a0e957fbf602023011d9114d8bcb8effb67 populate_test_db accepts a `coinbase` param (Daniela Brozzoni)

Pull request description:

  ### Description

  With this PR we start considering how many confirmations a coinbase has. If it's not mature yet, we don't use it for building transactions.
  Fixes #413

  ### Notes to the reviewers

  This PR is based on #611, review that one before reviewing this 😄

  007c5a78335a3e9f6c9c28a077793c2ba34bbb4e adds a coinbase parameter to `populate_test_db`, to specify if you want the db to be populated with immature coins. This is useful for `test_spend_coinbase`, but that's probably going to be the only use case.
  I don't think it's a big deal to have a test function take an almost_always_useless parameter - it's not an exposed API, anyways. But, if you can come up with a different way of implementing `test_spend_coinbase` that doesn't require 007c5a78335a3e9f6c9c28a077793c2ba34bbb4e, even better! I looked for it for a while, but other than duplicating the whole `populate_test_db` code, which made the test way harder to comprehend, I didn't find any other way.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  afilini:
    ACK e85aa24

Tree-SHA512: 30f470c33f9ffe928500a58f821f8ce445c653766459465eb005031ac523c6f143856fc9ca68a8e1f23a485c6543a9565bd889f9557c92bf5322e81291212a5f
This commit is contained in:
Alekos Filini 2022-07-05 22:25:28 +02:00
commit ec22fa2ad0
No known key found for this signature in database
GPG Key ID: 431401E4A4530061
3 changed files with 124 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/database/memory.rs
View File

@ -482,15 +482,23 @@ impl ConfigurableDatabase for MemoryDatabase {
/// don't have `test` set.
macro_rules! populate_test_db {
($db:expr, $tx_meta:expr, $current_height:expr$(,)?) => {{
$crate::populate_test_db!($db, $tx_meta, $current_height, (@coinbase false))
}};
($db:expr, $tx_meta:expr, $current_height:expr, (@coinbase $is_coinbase:expr)$(,)?) => {{
use std::str::FromStr;
use $crate::database::BatchOperations;
let mut db = $db;
let tx_meta = $tx_meta;
let current_height: Option<u32> = $current_height;
let input = if $is_coinbase {
vec![$crate::bitcoin::TxIn::default()]
} else {
vec![]
};
let tx = $crate::bitcoin::Transaction {
version: 1,
lock_time: 0,
input: vec![],
input,
output: tx_meta
.output
.iter()

123
src/wallet/mod.rs
View File

@ -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(
@ -1342,6 +1344,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,
@ -1350,6 +1353,7 @@ where
must_use_all_available: bool,
manual_only: bool,
must_only_use_confirmed_tx: bool,
current_height: Option<u32>,
) -> Result<(Vec<WeightedUtxo>, Vec<WeightedUtxo>), Error> {
// must_spend <- manually selected utxos
// may_spend <- all other available utxos
@ -1368,23 +1372,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 must_only_use_confirmed_tx && tx.confirmation_time.is_none() {
return false;
}
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,
}
}
}
spendable
}
})
.collect::<Result<Vec<_>, _>>()?
}
false => vec![true; may_spend.len()],
};
})
.collect::<Result<Vec<_>, _>>()?;
let mut i = 0;
may_spend.retain(|u| {
@ -4684,4 +4709,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();
}
}

11
src/wallet/tx_builder.rs
View File

@ -547,10 +547,15 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>, 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