diff --git a/doc/API.md b/doc/API.md index e85b4f0c7..29abc45a0 100644 --- a/doc/API.md +++ b/doc/API.md @@ -149,7 +149,8 @@ A coin may have one of the following four statuses: Create a transaction spending one or more of our coins. All coins must exist and not be spent. If no coins are specified in `outpoints`, they will be selected automatically from the set of -confirmed coins (see [`listcoins`](#listcoins) for coin status definitions). +confirmed coins together with any unconfirmed coins that are change outputs +(see [`listcoins`](#listcoins) for coin status definitions). Will error if the given coins are not sufficient to cover the transaction cost at 90% (or more) of the given feerate. If on the contrary the transaction is more than sufficiently funded, it will diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index a1b1317c4..faaaf3718 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -1473,6 +1473,7 @@ impl<'a> CachedTxGetter<'a> { #[derive(Debug, Clone)] pub struct MempoolEntry { pub vsize: u64, + pub ancestor_vsize: u64, pub fees: MempoolEntryFees, } @@ -1482,19 +1483,28 @@ impl From for MempoolEntry { .get("vsize") .and_then(Json::as_u64) .expect("Must be present in bitcoind response"); + let ancestor_vsize = json + .get("ancestorsize") + .and_then(Json::as_u64) + .expect("Must be present in bitcoind response"); let fees = json .get("fees") .as_ref() .expect("Must be present in bitcoind response") .into(); - MempoolEntry { vsize, fees } + MempoolEntry { + vsize, + ancestor_vsize, + fees, + } } } #[derive(Debug, Clone)] pub struct MempoolEntryFees { pub base: bitcoin::Amount, + pub ancestor: bitcoin::Amount, pub descendant: bitcoin::Amount, } @@ -1506,11 +1516,20 @@ impl From<&&Json> for MempoolEntryFees { .and_then(Json::as_f64) .and_then(|a| bitcoin::Amount::from_btc(a).ok()) .expect("Must be present and a valid amount"); + let ancestor = json + .get("ancestor") + .and_then(Json::as_f64) + .and_then(|a| bitcoin::Amount::from_btc(a).ok()) + .expect("Must be present and a valid amount"); let descendant = json .get("descendant") .and_then(Json::as_f64) .and_then(|a| bitcoin::Amount::from_btc(a).ok()) .expect("Must be present and a valid amount"); - MempoolEntryFees { base, descendant } + MempoolEntryFees { + base, + ancestor, + descendant, + } } } diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 60388c1e0..6c41c40c1 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -116,6 +116,11 @@ pub trait BitcoinInterface: Send { /// Get the details of unconfirmed transactions spending these outpoints, if any. fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec; + + /// Get mempool data for the given transaction. + /// + /// Returns `None` if the transaction is not in the mempool. + fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option; } impl BitcoinInterface for d::BitcoinD { @@ -366,6 +371,10 @@ impl BitcoinInterface for d::BitcoinD { .filter_map(|txid| self.mempool_entry(&txid)) .collect() } + + fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option { + self.mempool_entry(txid) + } } // FIXME: do we need to repeat the entire trait implemenation? Isn't there a nicer way? @@ -456,6 +465,10 @@ impl BitcoinInterface for sync::Arc> fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec { self.lock().unwrap().mempool_spenders(outpoints) } + + fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option { + self.lock().unwrap().mempool_entry(txid) + } } // FIXME: We could avoid this type (and all the conversions entailing allocations) if bitcoind diff --git a/src/commands/mod.rs b/src/commands/mod.rs index bf473bf67..19a5aec7d 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -9,7 +9,7 @@ use crate::{ database::{Coin, DatabaseConnection, DatabaseInterface}, descriptors, spend::{ - create_spend, AddrInfo, CandidateCoin, CreateSpendRes, SpendCreationError, + create_spend, AddrInfo, AncestorInfo, CandidateCoin, CreateSpendRes, SpendCreationError, SpendOutputAddress, SpendTxFees, TxGetter, }, DaemonControl, VERSION, @@ -179,6 +179,7 @@ fn coin_to_candidate( coin: &Coin, must_select: bool, sequence: Option, + ancestor_info: Option, ) -> CandidateCoin { CandidateCoin { outpoint: coin.outpoint, @@ -187,6 +188,7 @@ fn coin_to_candidate( is_change: coin.is_change, must_select, sequence, + ancestor_info, } } @@ -460,13 +462,33 @@ impl DaemonControl { // the spend from a set of optional candidates. // Otherwise, only the specified coins will be used, all as mandatory candidates. let candidate_coins: Vec = if coins_outpoints.is_empty() { - // We only select confirmed coins for now. Including unconfirmed ones as well would - // introduce a whole bunch of additional complexity. + // From our unconfirmed coins, we only include those that are change outputs + // since unconfirmed external deposits are more at risk of being dropped + // unexpectedly from the mempool as they are beyond the user's control. db_conn - .coins(&[CoinStatus::Confirmed], &[]) - .into_values() - .map(|c| { - coin_to_candidate(&c, /*must_select=*/ false, /*sequence=*/ None) + .coins(&[CoinStatus::Unconfirmed, CoinStatus::Confirmed], &[]) + .into_iter() + .filter_map(|(op, c)| { + if c.block_info.is_some() { + Some((c, None)) // confirmed coins have no ancestor info + } else if c.is_change { + // In case the mempool_entry is None, the coin will be included without + // any ancestor info. + Some(( + c, + self.bitcoin.mempool_entry(&op.txid).map(AncestorInfo::from), + )) + } else { + None + } + }) + .map(|(c, ancestor_info)| { + coin_to_candidate( + &c, + /*must_select=*/ false, + /*sequence=*/ None, + ancestor_info, + ) }) .collect() } else { @@ -482,8 +504,23 @@ impl DaemonControl { } } coins - .into_values() - .map(|c| coin_to_candidate(&c, /*must_select=*/ true, /*sequence=*/ None)) + .into_iter() + .map(|(op, c)| { + let ancestor_info = if c.block_info.is_none() { + // We include any non-change coins here as they have been selected by the caller. + // If the unconfirmed coin's transaction is no longer in the mempool, keep the + // coin as a candidate but without any ancestor info (same as confirmed candidate). + self.bitcoin.mempool_entry(&op.txid).map(AncestorInfo::from) + } else { + None + }; + coin_to_candidate( + &c, + /*must_select=*/ true, + /*sequence=*/ None, + ancestor_info, + ) + }) .collect() }; @@ -795,7 +832,14 @@ impl DaemonControl { let mut candidate_coins: Vec = prev_coins .values() .map(|c| { - coin_to_candidate(c, /*must_select=*/ !is_cancel, /*sequence=*/ None) + // In case any previous coins are unconfirmed, we don't include their ancestor info + // in the candidate as the replacement fee and feerate will be higher and any + // additional fee to pay for ancestors should already have been taken into account + // when including these coins in the previous transaction. + coin_to_candidate( + c, /*must_select=*/ !is_cancel, /*sequence=*/ None, + /*ancestor_info=*/ None, + ) }) .collect(); let confirmed_cands: Vec = db_conn @@ -807,6 +851,7 @@ impl DaemonControl { if !prev_coins.contains_key(&c.outpoint) { Some(coin_to_candidate( &c, /*must_select=*/ false, /*sequence=*/ None, + /*ancestor_info=*/ None, )) } else { None @@ -996,6 +1041,7 @@ impl DaemonControl { &c, /*must_select=*/ true, /*sequence=*/ Some(bitcoin::Sequence::from_height(timelock)), + /*ancestor_info=*/ None, )) } else { None @@ -1384,8 +1430,8 @@ mod tests { spend_txid: None, spend_block: None, }]); - // If we try to use coin selection, the unconfirmed coin will not be used as a candidate - // and so we get a coin selection error due to insufficient funds. + // If we try to use coin selection, the unconfirmed non-change coin will not be used + // as a candidate and so we get a coin selection error due to insufficient funds. assert!(matches!( control.create_spend(&destinations, &[], 1, None), Ok(CreateSpendResult::InsufficientFunds { .. }), @@ -1616,7 +1662,7 @@ mod tests { ))) ); - // Add a confirmed unspent coin to be used for coin selection. + // Add an unconfirmed change coin to be used for coin selection. let confirmed_op_1 = bitcoin::OutPoint { txid: dummy_op.txid, vout: dummy_op.vout + 100, @@ -1624,13 +1670,10 @@ mod tests { db_conn.new_unspent_coins(&[Coin { outpoint: confirmed_op_1, is_immature: false, - block_info: Some(BlockInfo { - height: 174500, - time: 174500, - }), + block_info: None, amount: bitcoin::Amount::from_sat(80_000), derivation_index: bip32::ChildNumber::from(42), - is_change: false, + is_change: true, spend_txid: None, spend_block: None, }]); diff --git a/src/spend.rs b/src/spend.rs index 4b9435d7c..d545375e0 100644 --- a/src/spend.rs +++ b/src/spend.rs @@ -1,6 +1,10 @@ -use crate::descriptors; +use crate::{bitcoin::MempoolEntry, descriptors}; -use std::{collections::BTreeMap, convert::TryInto, fmt}; +use std::{ + collections::{BTreeMap, HashMap}, + convert::TryInto, + fmt, +}; pub use bdk_coin_select::InsufficientFunds; use bdk_coin_select::{ @@ -11,6 +15,7 @@ use miniscript::bitcoin::{ self, absolute::{Height, LockTime}, bip32, + constants::WITNESS_SCALE_FACTOR, psbt::{Input as PsbtIn, Output as PsbtOut, Psbt}, secp256k1, }; @@ -154,6 +159,29 @@ fn sanity_check_psbt( Ok(()) } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct AncestorInfo { + pub vsize: u32, + pub fee: u32, +} + +impl From for AncestorInfo { + fn from(entry: MempoolEntry) -> Self { + Self { + vsize: entry + .ancestor_vsize + .try_into() + .expect("vsize must fit in a u32"), + fee: entry + .fees + .ancestor + .to_sat() + .try_into() + .expect("fee in sats should fit in a u32"), + } + } +} + /// A candidate for coin selection when creating a transaction. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct CandidateCoin { @@ -169,6 +197,8 @@ pub struct CandidateCoin { pub must_select: bool, /// The nSequence field to set for an input spending this coin. pub sequence: Option, + /// Information about in-mempool ancestors of the coin. + pub ancestor_info: Option, } /// A coin selection result. @@ -184,6 +214,8 @@ pub struct CoinSelectionRes { /// Maximum change amount possible with the selection irrespective of any change /// policy. pub max_change_amount: bitcoin::Amount, + /// Fee added to pay for ancestors at the target feerate. + pub fee_for_ancestors: bitcoin::Amount, } /// Metric based on [`LowestFee`] that aims to minimize transaction fees @@ -291,12 +323,48 @@ fn select_coins_for_spend( base_weight = base_weight.saturating_sub(2); } let max_input_weight = TXIN_BASE_WEIGHT + max_sat_weight; + // Get feerate as u32 for calculation relating to ancestor below. + // We expect `feerate_vb` to be a positive integer, but take ceil() + // just in case to be sure we pay enough for ancestors. + let feerate_vb_u32 = feerate_vb.ceil() as u32; + let witness_factor: u32 = WITNESS_SCALE_FACTOR + .try_into() + .expect("scale factor must fit in u32"); + // This will be used to store any extra weight added to candidates. + let mut added_weights = HashMap::::with_capacity(candidate_coins.len()); let candidates: Vec = candidate_coins .iter() .map(|cand| Candidate { input_count: 1, value: cand.amount.to_sat(), - weight: max_input_weight, + weight: { + let extra = cand + .ancestor_info + .map(|info| { + // The implied ancestor vsize if the fee had been paid at our target feerate. + let ancestor_vsize_at_feerate: u32 = info + .fee + .checked_div(feerate_vb_u32) + .expect("feerate is greater than zero"); + // If the actual ancestor vsize is bigger than the implied vsize, we will need to + // pay the difference in order for the combined feerate to be at the target value. + // We multiply the vsize by 4 to get the ancestor weight, which is an upper bound + // on its true weight (vsize*4 - 3 <= weight <= vsize*4), to ensure we pay enough. + // Note that if candidates share ancestors, we may add this difference more than + // once in the resulting transaction. + info.vsize + .saturating_sub(ancestor_vsize_at_feerate) + .checked_mul(witness_factor) + .expect("weight difference must fit in u32") + }) + .unwrap_or(0); + // Store the extra weight for this candidate for use later on. + // At the same time, make sure there are no duplicate outpoints. + assert!(added_weights.insert(cand.outpoint, extra).is_none()); + max_input_weight + .checked_add(extra) + .expect("effective weight must fit in u32") + }, is_segwit: true, // We only support receiving on Segwit scripts. }) .collect(); @@ -336,9 +404,10 @@ fn select_coins_for_spend( // Finally, run the coin selection algorithm. We use an opportunistic BnB and if it couldn't // find any solution we fall back to selecting coins by descending value. + let feerate = FeeRate::from_sat_per_vb(feerate_vb); let target = Target { value: out_value_nochange, - feerate: FeeRate::from_sat_per_vb(feerate_vb), + feerate, min_fee, }; let lowest_fee = LowestFee { @@ -402,14 +471,29 @@ fn select_coins_for_spend( .try_into() .expect("value is non-negative"), ); + let mut total_added_weight: u32 = 0; + let selected = selector + .selected_indices() + .iter() + .map(|i| candidate_coins[*i]) + .inspect(|cand| { + total_added_weight = total_added_weight + .checked_add( + *added_weights + .get(&cand.outpoint) + .expect("contains added weight for all candidates"), + ) + .expect("should fit in u32") + }) + .collect(); + // Calculate added fee based on the feerate in sats/wu, which is the feerate used for coin selection. + let fee_for_ancestors = + bitcoin::Amount::from_sat(((total_added_weight as f32) * feerate.spwu()).ceil() as u64); Ok(CoinSelectionRes { - selected: selector - .selected_indices() - .iter() - .map(|i| candidate_coins[*i]) - .collect(), + selected, change_amount, max_change_amount, + fee_for_ancestors, }) } @@ -459,6 +543,7 @@ pub enum SpendTxFees { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum CreateSpendWarning { ChangeAddedToFee(u64), + AddtionalFeeForAncestors(u64), } impl fmt::Display for CreateSpendWarning { @@ -470,6 +555,12 @@ impl fmt::Display for CreateSpendWarning { amt, if *amt > 1 {"s"} else {""}, ), + CreateSpendWarning::AddtionalFeeForAncestors(amt) => write!( + f, + "An additional fee of {} sat{} has been added to pay for ancestors at the target feerate.", + amt, + if *amt > 1 {"s"} else {""}, + ), } } } @@ -589,6 +680,7 @@ pub fn create_spend( selected, change_amount, max_change_amount, + fee_for_ancestors, } = { // At this point the transaction still has no input and no change output, as expected // by the coins selection helper function. @@ -650,6 +742,12 @@ pub fn create_spend( )); } + if fee_for_ancestors.to_sat() > 0 { + warnings.push(CreateSpendWarning::AddtionalFeeForAncestors( + fee_for_ancestors.to_sat(), + )); + } + // Iterate through selected coins and add necessary information to the PSBT inputs. let mut psbt_ins = Vec::with_capacity(selected.len()); for cand in &selected { diff --git a/src/testutils.rs b/src/testutils.rs index 9034aaec2..8359b75de 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -123,6 +123,10 @@ impl BitcoinInterface for DummyBitcoind { fn mempool_spenders(&self, _: &[bitcoin::OutPoint]) -> Vec { Vec::new() } + + fn mempool_entry(&self, _: &bitcoin::Txid) -> Option { + None + } } struct DummyDbState { diff --git a/tests/test_spend.py b/tests/test_spend.py index 4ae752293..3e7224a23 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -1,6 +1,6 @@ from fixtures import * from test_framework.serializations import PSBT, uint256_from_str -from test_framework.utils import wait_for, COIN, RpcError +from test_framework.utils import sign_and_broadcast_psbt, wait_for, COIN, RpcError def test_spend_change(lianad, bitcoind): @@ -59,14 +59,6 @@ def test_spend_change(lianad, bitcoind): bitcoind.generate_block(1, wait_for_mempool=spend_txid) -def sign_and_broadcast_psbt(lianad, psbt): - txid = psbt.tx.txid().hex() - psbt = lianad.signer.sign_psbt(psbt) - lianad.rpc.updatespend(psbt.to_base64()) - lianad.rpc.broadcastspend(txid) - return txid - - def test_coin_marked_spent(lianad, bitcoind): """Test a spent coin is marked as such under various conditions.""" # Receive a coin in a single transaction @@ -179,7 +171,16 @@ def sign_and_broadcast(psbt): psbt = PSBT.from_base64(res["psbt"]) sign_and_broadcast(psbt) assert len(psbt.o) == 4 - assert len(res["warnings"]) == 0 + assert bitcoind.rpc.getmempoolentry(deposit_d)["ancestorsize"] == 165 + assert bitcoind.rpc.getmempoolentry(deposit_d)["fees"]["ancestor"] * COIN == 165 + # ancestor vsize at feerate 2 sat/vb = ancestor_fee / 2 = 165 / 2 = 82 + # extra_weight <= (extra vsize * witness factor) = (165 - 82) * 4 = 332 + # additional fee at 2 sat/vb (0.5 sat/wu) = 332 * 0.5 = 166 + assert len(res["warnings"]) == 1 + assert ( + res["warnings"][0] + == "An additional fee of 166 sats has been added to pay for ancestors at the target feerate." + ) # All the spent coins must have been detected as such all_deposits = (deposit_a, deposit_b, deposit_c, deposit_d) @@ -249,84 +250,183 @@ def test_send_to_self(lianad, bitcoind): def test_coin_selection(lianad, bitcoind): """We can create a spend using coin selection.""" # Send to an (external) address. - dest_100_000 = {bitcoind.rpc.getnewaddress(): 100_000} + dest_addr_1 = bitcoind.rpc.getnewaddress() # Coin selection is not possible if we have no coins. assert len(lianad.rpc.listcoins()["coins"]) == 0 - assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2) + assert "missing" in lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2) # Receive a coin in an unconfirmed deposit transaction. - recv_addr = lianad.rpc.getnewaddress()["address"] - deposit = bitcoind.rpc.sendtoaddress(recv_addr, 0.0008) # 80_000 sats + recv_addr_1 = lianad.rpc.getnewaddress()["address"] + deposit_1 = bitcoind.rpc.sendtoaddress(recv_addr_1, 0.0012) # 120_000 sats wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 1) - # There are still no confirmed coins to use as candidates for selection. + # There are still no confirmed coins or unconfirmed change + # to use as candidates for selection. assert len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 0 assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1 - assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2) + assert lianad.rpc.listcoins(["unconfirmed"])["coins"][0]["is_change"] is False + assert "missing" in lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2) # Confirm coin. - bitcoind.generate_block(1, wait_for_mempool=deposit) + bitcoind.generate_block(1, wait_for_mempool=deposit_1) wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1) - - # Insufficient funds for coin selection. - assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2) - - # Reduce spend amount. - dest_30_000 = {bitcoind.rpc.getnewaddress(): 30_000} - res = lianad.rpc.createspend(dest_30_000, [], 2) - assert "psbt" in res - - # The transaction must contain a change output. - spend_psbt = PSBT.from_base64(res["psbt"]) - assert len(spend_psbt.o) == 2 - assert len(spend_psbt.tx.vout) == 2 + # Coin selection now succeeds. + spend_res_1 = lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2) + assert "psbt" in spend_res_1 + assert len(spend_res_1["warnings"]) == 0 + # Increase spend amount and we have insufficient funds again even though we + # now have confirmed coins. + assert "missing" in lianad.rpc.createspend({dest_addr_1: 200_000}, [], 2) + + # The transaction contains a change output. + spend_psbt_1 = PSBT.from_base64(spend_res_1["psbt"]) + assert len(spend_psbt_1.o) == 2 + assert len(spend_psbt_1.tx.vout) == 2 # Sign and broadcast this Spend transaction. - signed_psbt = lianad.signer.sign_psbt(spend_psbt) - lianad.rpc.updatespend(signed_psbt.to_base64()) - spend_txid = signed_psbt.tx.txid().hex() - lianad.rpc.broadcastspend(spend_txid) - + spend_txid_1 = sign_and_broadcast_psbt(lianad, spend_psbt_1) wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2) - coins = lianad.rpc.listcoins()["coins"] # Check that change output is unconfirmed. assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1 + assert lianad.rpc.listcoins(["unconfirmed"])["coins"][0]["is_change"] is True assert len(lianad.rpc.listcoins(["spending"])["coins"]) == 1 - # Check we cannot use coins as candidates if they are spending/spent or unconfirmed. - assert "missing" in lianad.rpc.createspend(dest_30_000, [], 2) + # We can use unconfirmed change as candidate. + # Depending on the feerate, we'll get a warning about paying extra for the ancestor. + dest_addr_2 = bitcoind.rpc.getnewaddress() + # If feerate is higher than ancestor, we'll need to pay extra. + + # Try 10 sat/vb: + spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 10) + assert "psbt" in spend_res_2 + spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) + # The spend is using the unconfirmed change. + assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str( + bytes.fromhex(spend_txid_1)[::-1] + ) + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] == 161 + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN == 339 + # ancestor vsize at feerate 10 sat/vb = ancestor_fee / 10 = 339 / 10 = 33 + # extra_weight <= (extra vsize * witness factor) = (161 - 33) * 4 = 512 + # additional fee at 10 sat/vb (2.5 sat/wu) = 512 * 2.5 = 1280 + assert len(spend_res_2["warnings"]) == 1 + assert ( + spend_res_2["warnings"][0] + == "An additional fee of 1280 sats has been added to pay for ancestors at the target feerate." + ) - # Now confirm the Spend. - bitcoind.generate_block(1, wait_for_mempool=spend_txid) - wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1) - # But its value is not enough for this Spend. - dest_60_000 = {bitcoind.rpc.getnewaddress(): 60_000} - assert "missing" in lianad.rpc.createspend(dest_60_000, [], 2) + # Try 3 sat/vb: + spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 3) + assert "psbt" in spend_res_2 + spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) + # The spend is using the unconfirmed change. + assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str( + bytes.fromhex(spend_txid_1)[::-1] + ) + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] == 161 + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN == 339 + # ancestor vsize at feerate 3 sat/vb = ancestor_fee / 3 = 339 / 3 = 113 + # extra_weight <= (extra vsize * witness factor) = (161 - 113) * 4 = 192 + # additional fee at 3 sat/vb (0.75 sat/wu) = 192 * 0.75 = 144 + assert len(spend_res_2["warnings"]) == 1 + assert ( + spend_res_2["warnings"][0] + == "An additional fee of 144 sats has been added to pay for ancestors at the target feerate." + ) + + # 2 sat/vb is same feerate as ancestor and we have no warnings: + spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 2) + assert "psbt" in spend_res_2 + assert len(spend_res_2["warnings"]) == 0 + spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) + # The spend is using the unconfirmed change. + assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str( + bytes.fromhex(spend_txid_1)[::-1] + ) # Get another coin to check coin selection with more than one candidate. - recv_addr = lianad.rpc.getnewaddress()["address"] - deposit = bitcoind.rpc.sendtoaddress(recv_addr, 0.0002) # 20_000 sats - bitcoind.generate_block(1, wait_for_mempool=deposit) - wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 2) + recv_addr_2 = lianad.rpc.getnewaddress()["address"] + deposit_2 = bitcoind.rpc.sendtoaddress(recv_addr_2, 30_000 / COIN) + wait_for(lambda: len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 2) + assert ( + len( + [ + c + for c in lianad.rpc.listcoins(["unconfirmed"])["coins"] + if c["is_change"] + ] + ) + == 1 + ) + dest_addr_3 = bitcoind.rpc.getnewaddress() + # As only one unconfirmed coin is change, we have insufficient funds. + assert "missing" in lianad.rpc.createspend({dest_addr_3: 20_000}, [], 10) + + # If we include both unconfirmed coins manually, it will succeed. + # We'll need to pay extra for each unconfirmed coin's ancestors. + outpoints = [c["outpoint"] for c in lianad.rpc.listcoins(["unconfirmed"])["coins"]] + + spend_res_3 = lianad.rpc.createspend({dest_addr_3: 20_000}, outpoints, 10) + assert "psbt" in spend_res_3 + assert bitcoind.rpc.getmempoolentry(deposit_2)["ancestorsize"] == 165 + assert bitcoind.rpc.getmempoolentry(deposit_2)["fees"]["ancestor"] * COIN == 165 + # From above, extra fee for unconfirmed change at 10 sat/vb = 1280. + # For unconfirmed non-change: + # ancestor vsize at feerate 10 sat/vb = ancestor_fee / 10 = 165 / 10 = 16 + # extra_weight <= (extra vsize * witness factor) = (165 - 16) * 4 = 596 + # additional fee at 10 sat/vb (2.5 sat/wu) = 596 * 2.5 = 1490 + # Sum of extra ancestor fees = 1280 + 1490 = 2770. + assert len(spend_res_3["warnings"]) == 1 + assert ( + spend_res_3["warnings"][0] + == "An additional fee of 2770 sats has been added to pay for ancestors at the target feerate." + ) + spend_psbt_3 = PSBT.from_base64(spend_res_3["psbt"]) + spend_txid_3 = sign_and_broadcast_psbt(lianad, spend_psbt_3) + mempool_txid_3 = bitcoind.rpc.getmempoolentry(spend_txid_3) + # The effective feerate of new transaction plus ancestor matches the target. + # Note that in the mempool entry, "ancestor" includes spend_txid_3 itself. + assert ( + mempool_txid_3["fees"]["ancestor"] * COIN // mempool_txid_3["ancestorsize"] + == 10 + ) + # The spend_txid_3 transaction itself has a higher feerate. + assert (mempool_txid_3["fees"]["base"] * COIN) // mempool_txid_3["vsize"] > 10 + # If we subtract the extra that pays for the ancestor, the feerate is at the target value. + assert ((mempool_txid_3["fees"]["base"] * COIN) - 2770) // mempool_txid_3[ + "vsize" + ] == 10 + + # Now confirm the spend. + bitcoind.generate_block(1, wait_for_mempool=spend_txid_3) + wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1) - res = lianad.rpc.createspend(dest_60_000, [], 2) - assert "psbt" in res + # Now create the same spend with auto and manual selection: + dest_addr_4 = bitcoind.rpc.getnewaddress() + spend_res_4 = lianad.rpc.createspend({dest_addr_4: 15_000}, [], 2) + assert "psbt" in spend_res_4 + assert len(spend_res_4["warnings"]) == 0 - # The transaction must contain a change output. - auto_psbt = PSBT.from_base64(res["psbt"]) - assert len(auto_psbt.o) == 2 - assert len(auto_psbt.tx.vout) == 2 + # The transaction contains a change output. + spend_psbt_4 = PSBT.from_base64(spend_res_4["psbt"]) + assert len(spend_psbt_4.i) == 1 + assert len(spend_psbt_4.o) == 2 + assert len(spend_psbt_4.tx.vout) == 2 # Now create a transaction with manual coin selection using the same outpoints. outpoints = [ - f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in auto_psbt.tx.vin + f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in spend_psbt_4.tx.vin ] - res_manual = lianad.rpc.createspend(dest_60_000, outpoints, 2) - manual_psbt = PSBT.from_base64(res_manual["psbt"]) + assert len(outpoints) > 0 + res_manual = lianad.rpc.createspend({dest_addr_4: 15_000}, outpoints, 2) + assert len(res_manual["warnings"]) == 0 + psbt_manual = PSBT.from_base64(res_manual["psbt"]) # Recipient details are the same for both. - assert auto_psbt.tx.vout[0].nValue == manual_psbt.tx.vout[0].nValue - assert auto_psbt.tx.vout[0].scriptPubKey == manual_psbt.tx.vout[0].scriptPubKey - # Change amount is the same (change address will be different). - assert auto_psbt.tx.vout[1].nValue == manual_psbt.tx.vout[1].nValue + assert spend_psbt_4.tx.vout[0].nValue == psbt_manual.tx.vout[0].nValue + assert spend_psbt_4.tx.vout[0].scriptPubKey == psbt_manual.tx.vout[0].scriptPubKey + # Change details are also the same + # (change address is same as neither transaction has been broadcast) + assert spend_psbt_4.tx.vout[1].nValue == psbt_manual.tx.vout[1].nValue + assert spend_psbt_4.tx.vout[1].scriptPubKey == psbt_manual.tx.vout[1].scriptPubKey def test_coin_selection_changeless(lianad, bitcoind):