diff --git a/bitcoin-wallet/src/wallet.rs b/bitcoin-wallet/src/wallet.rs index 033b0cc79..2150d5ec0 100644 --- a/bitcoin-wallet/src/wallet.rs +++ b/bitcoin-wallet/src/wallet.rs @@ -11,6 +11,7 @@ use bdk_wallet::bitcoin::FeeRate; use bdk_wallet::bitcoin::Network; use bdk_wallet::export::FullyNodedExport; use bdk_wallet::rusqlite::Connection; +use bdk_wallet::chain::ChainPosition; use bdk_wallet::template::{Bip84, DescriptorTemplate}; use bdk_wallet::{Balance, PersistedWallet}; use bitcoin::bip32::Xpriv; @@ -23,6 +24,7 @@ use rust_decimal::Decimal; use rust_decimal::prelude::*; use std::collections::BTreeMap; use std::collections::HashMap; +use std::collections::HashSet; use std::fmt::Debug; use std::path::Path; use std::path::PathBuf; @@ -1359,8 +1361,7 @@ where tx_builder.finish()? }; - let weight = psbt.unsigned_tx.weight(); - let fee = self.estimate_fee(weight, Some(amount)).await?; + let fee = self.estimate_fee_for_psbt(&psbt, Some(amount)).await?; self.send_to_address(address, amount, fee, change_override) .await @@ -1450,115 +1451,118 @@ where /// /// Returns a tuple of (max_giveable_amount, spending_fee). pub async fn max_giveable(&self, locking_script_size: usize) -> Result<(Amount, Amount)> { - let mut wallet = self.wallet.lock().await; - - // Construct a dummy drain transaction - let dummy_script = ScriptBuf::from(vec![0u8; locking_script_size]); - - let mut tx_builder = wallet.build_tx(); - - tx_builder.drain_to(dummy_script.clone()); - tx_builder.fee_absolute(Amount::ZERO); - tx_builder.drain_wallet(); - - // The weight WILL NOT change, even if we change the fee - // because we are draining the wallet (using all inputs) and - // always have one output of constant size - // - // The only changable part is the amount of the output. - // If we increase the fee, the output amount simply will decrease - // - // The inputs are constant, so only the output amount changes. - let (dummy_max_giveable, dummy_weight) = match tx_builder.finish() { - Ok(psbt) => { - if psbt.unsigned_tx.output.len() != 1 { - bail!("Expected a single output in the dummy transaction"); - } - - let max_giveable = psbt.unsigned_tx.output.first().expect("Expected a single output in the dummy transaction").value; - let weight = psbt.unsigned_tx.weight(); - - Ok((Some(max_giveable), weight)) - }, - Err(bdk_wallet::error::CreateTxError::CoinSelection(_)) => { - // We don't have enough funds to create a transaction (below dust limit) - // - // We still want to to return a valid fee. - // Callers of this function might want to calculate *how* large - // the next UTXO needs to be such that we can spend any funds - // - // To be able to calculate an accurate fee, we need to figure out - // the weight our drain transaction if we received another UTXO - - // We create fake deposit UTXO - // Our dummy drain transaction will spend this deposit UTXO - let mut fake_deposit_input = bitcoin::psbt::Input::default(); - - let dummy_deposit_address = wallet.peek_address(KeychainKind::External, 0); - let fake_deposit_script = dummy_deposit_address.script_pubkey(); - let fake_deposit_txout = bitcoin::blockdata::transaction::TxOut { - // The exact deposit amount does not matter - // because we only care about the weight of the transaction - // which does not depend on the amount of the input - value: DUST_AMOUNT * 5, - script_pubkey: fake_deposit_script, - }; - let fake_deposit_tx = bitcoin::Transaction { - version: bitcoin::blockdata::transaction::Version::TWO, - lock_time: bitcoin::blockdata::locktime::absolute::LockTime::ZERO, - input: vec![bitcoin::TxIn { - previous_output: bitcoin::OutPoint::null(), // or some dummy outpoint - script_sig: Default::default(), - sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Default::default(), - }], - output: vec![fake_deposit_txout.clone()], - }; - - let fake_deposit_txid = fake_deposit_tx.compute_txid(); - - fake_deposit_input.witness_utxo = Some(fake_deposit_txout); - fake_deposit_input.non_witness_utxo = Some(fake_deposit_tx); - - // Create outpoint that points to our fake transaction's output 0 - let fake_deposit_outpoint = bitcoin::OutPoint { - txid: fake_deposit_txid, - vout: 0, - }; + let (dummy_max_giveable, dummy_psbt) = { + let mut wallet = self.wallet.lock().await; - // Worst-case witness weight for our script type. - const DUMMY_SATISFACTION_WEIGHT: Weight = Weight::from_wu(107 * 10); + // Construct a dummy drain transaction + let dummy_script = ScriptBuf::from(vec![0u8; locking_script_size]); - let mut tx_builder = wallet.build_tx(); + let mut tx_builder = wallet.build_tx(); - tx_builder.drain_to(dummy_script.clone()); - tx_builder.fee_absolute(Amount::ZERO); - tx_builder.drain_wallet(); + tx_builder.drain_to(dummy_script.clone()); + tx_builder.fee_absolute(Amount::ZERO); + tx_builder.drain_wallet(); + + // The weight WILL NOT change, even if we change the fee + // because we are draining the wallet (using all inputs) and + // always have one output of constant size + // + // The only changable part is the amount of the output. + // If we increase the fee, the output amount simply will decrease + // + // The inputs are constant, so only the output amount changes. + // + // However, the effective fee may increase beyond the child transaction's + // own weight if CPFP is required (due to unconfirmed parents). + match tx_builder.finish() { + Ok(psbt) => { + if psbt.unsigned_tx.output.len() != 1 { + bail!("Expected a single output in the dummy transaction"); + } - tx_builder - .add_foreign_utxo( - fake_deposit_outpoint, - fake_deposit_input, - DUMMY_SATISFACTION_WEIGHT, - ).context("Failed to add dummy foreign utxo to calculate fee for max_giveable if we had one more utxo")?; + let max_giveable = psbt.unsigned_tx.output.first().expect("Expected a single output in the dummy transaction").value; - // Try building the dummy drain transaction with the new fake UTXO - // If we fail now, we propagate the error to the caller - let psbt = tx_builder.finish()?; - let weight = psbt.unsigned_tx.weight(); + Ok((Some(max_giveable), psbt)) + }, + Err(bdk_wallet::error::CreateTxError::CoinSelection(_)) => { + // We don't have enough funds to create a transaction (below dust limit) + // + // We still want to to return a valid fee. + // Callers of this function might want to calculate *how* large + // the next UTXO needs to be such that we can spend any funds + // + // To be able to calculate an accurate fee, we need to figure out + // the weight our drain transaction if we received another UTXO + + // We create fake deposit UTXO + // Our dummy drain transaction will spend this deposit UTXO + let mut fake_deposit_input = bitcoin::psbt::Input::default(); + + let dummy_deposit_address = wallet.peek_address(KeychainKind::External, 0); + let fake_deposit_script = dummy_deposit_address.script_pubkey(); + let fake_deposit_txout = bitcoin::blockdata::transaction::TxOut { + // The exact deposit amount does not matter + // because we only care about the weight of the transaction + // which does not depend on the amount of the input + value: DUST_AMOUNT * 5, + script_pubkey: fake_deposit_script, + }; + let fake_deposit_tx = bitcoin::Transaction { + version: bitcoin::blockdata::transaction::Version::TWO, + lock_time: bitcoin::blockdata::locktime::absolute::LockTime::ZERO, + input: vec![bitcoin::TxIn { + previous_output: bitcoin::OutPoint::null(), // or some dummy outpoint + script_sig: Default::default(), + sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Default::default(), + }], + output: vec![fake_deposit_txout.clone()], + }; + + let fake_deposit_txid = fake_deposit_tx.compute_txid(); + + fake_deposit_input.witness_utxo = Some(fake_deposit_txout); + fake_deposit_input.non_witness_utxo = Some(fake_deposit_tx); + + // Create outpoint that points to our fake transaction's output 0 + let fake_deposit_outpoint = bitcoin::OutPoint { + txid: fake_deposit_txid, + vout: 0, + }; + + // Worst-case witness weight for our script type. + const DUMMY_SATISFACTION_WEIGHT: Weight = Weight::from_wu(107 * 10); + + let mut tx_builder = wallet.build_tx(); + + tx_builder.drain_to(dummy_script.clone()); + tx_builder.fee_absolute(Amount::ZERO); + tx_builder.drain_wallet(); + + tx_builder + .add_foreign_utxo( + fake_deposit_outpoint, + fake_deposit_input, + DUMMY_SATISFACTION_WEIGHT, + ).context("Failed to add dummy foreign utxo to calculate fee for max_giveable if we had one more utxo")?; + + // Try building the dummy drain transaction with the new fake UTXO + // If we fail now, we propagate the error to the caller + let psbt = tx_builder.finish()?; - tracing::trace!( - weight = weight.to_wu(), - "Built dummy drain transaction with fake UTXO, max giveable is 0" - ); + tracing::trace!( + weight = psbt.unsigned_tx.weight().to_wu(), + "Built dummy drain transaction with fake UTXO, max giveable is 0" + ); - Ok((None, weight)) - } - Err(e) => Err(e) - }.context("Failed to build transaction to figure out max giveable")?; + Ok((None, psbt)) + } + Err(e) => Err(e) + }.context("Failed to build transaction to figure out max giveable")? + }; - // Estimate the fee rate using our real fee rate estimation - let fee = self.estimate_fee(dummy_weight, dummy_max_giveable).await?; + // Estimate fee, accounting for any unconfirmed parents (CPFP) + let fee = self.estimate_fee_for_psbt(&dummy_psbt, dummy_max_giveable).await?; Ok(match dummy_max_giveable { // If the max giveable is less than the dust amount, we return 0 @@ -1609,6 +1613,172 @@ where estimate_fee(weight, transfer_amount, fee_rate, min_relay_fee) } + + /// Estimates the fee for a transaction represented by a PSBT, taking unconfirmed parent transactions into account (CPFP package fee). + /// + /// Recursively collects all unconfirmed ancestors; if any exist, the child fee is increased so that the full package (parents + child) + /// reaches the target fee rate. Fee caps (relative and absolute) are applied with warnings. + async fn estimate_fee_for_psbt( + &self, + psbt: &Psbt, + transfer_amount: Option, + ) -> Result { + let fee_rate = self.combined_fee_rate().await?; + let min_relay_fee = self.combined_min_relay_fee().await?; + let target_fee_rate = compute_effective_fee_rate(fee_rate, min_relay_fee)?; + + let child_only_fee = estimate_fee( + psbt.unsigned_tx.weight(), + transfer_amount, + target_fee_rate, + min_relay_fee, + )?; + + let (ancestor_fee, ancestor_weight) = self.collect_unconfirmed_ancestors(psbt).await?; + + if ancestor_weight == Weight::from_wu(0) { + return Ok(child_only_fee); + } + + let total_weight = Weight::from_wu( + psbt.unsigned_tx + .weight() + .to_wu() + .saturating_add(ancestor_weight.to_wu()), + ); + + let package_fee = target_fee_rate + .checked_mul_by_weight(total_weight) + .context("Failed to compute package fee")?; + + let required_child_fee = Amount::from_sat( + package_fee + .to_sat() + .saturating_sub(ancestor_fee.to_sat()), + ); + + let fee = child_only_fee.max(required_child_fee); + Ok(Self::clamp_cpfp_fee(fee, transfer_amount)) + } + + /// Walks all unconfirmed ancestors of every input in `psbt` and returns the total fee already paid by those ancestors + their total weight. + /// Confirmed ancestors are ignored. If a parent's fee cannot be calculated, that ancestor is skipped entirely. + async fn collect_unconfirmed_ancestors( + &self, + psbt: &Psbt, + ) -> Result<(Amount, Weight)> { + const MAX_ANCESTOR_DEPTH: usize = 3; + + let mut seen = HashSet::::new(); + let mut stack: Vec<(Txid, usize)> = psbt + .unsigned_tx + .input + .iter() + .map(|input| (input.previous_output.txid, 0)) + .collect(); + + let mut ancestor_fee = Amount::ZERO; + let mut ancestor_weight = Weight::from_wu(0); + + while let Some((txid, depth)) = stack.pop() { + if depth >= MAX_ANCESTOR_DEPTH { + continue; + } + + if !seen.insert(txid) { + continue; + } + + let (parent_tx, parent_fee) = { + let wallet = self.wallet.lock().await; + + // Local wallet lookup only. This reads from the synced tx graph and doesn't hit the network. + let Some(wallet_tx) = wallet.get_tx(txid) else { + continue; + }; + + if matches!(wallet_tx.chain_position, ChainPosition::Confirmed { .. }) { + continue; + } + + let parent_tx = wallet_tx.tx_node.tx.clone(); + + let parent_fee = match wallet.calculate_fee(&parent_tx) { + Ok(fee) => fee, + Err(err) => { + tracing::debug!( + %txid, + error = %err, + "Cannot compute fee for unconfirmed ancestor; skipping it" + ); + continue; + } + }; + + (parent_tx, parent_fee) + }; + + ancestor_fee = Amount::from_sat( + ancestor_fee.to_sat().saturating_add(parent_fee.to_sat()), + ); + ancestor_weight = Weight::from_wu( + ancestor_weight.to_wu().saturating_add(parent_tx.weight().to_wu()), + ); + + stack.extend( + parent_tx + .input + .iter() + .map(|input| (input.previous_output.txid, depth + 1)), + ); + } + + Ok((ancestor_fee, ancestor_weight)) + } + + /// Clamps a CPFP fee to the allowed range (relative cap -> min absolute -> hard cap) + fn clamp_cpfp_fee(fee: Amount, transfer_amount: Option) -> Amount { + let mut final_fee = fee; + + if let Some(transfer_amount) = transfer_amount { + let relative_max = Amount::from_sat( + MAX_RELATIVE_TX_FEE + .saturating_mul(Decimal::from(transfer_amount.to_sat())) + .ceil() + .to_u64() + .expect("MAX_RELATIVE_TX_FEE * transfer_amount fits in u64"), + ); + if final_fee > relative_max { + tracing::warn!( + "CPFP fee {} exceeds relative cap {} ({}% of transfer amount). Clamping.", + final_fee.to_sat(), + relative_max.to_sat(), + MAX_RELATIVE_TX_FEE.saturating_mul(Decimal::from(100)).ceil().to_u64().unwrap(), + ); + final_fee = relative_max; + } + } + + if final_fee < MIN_ABSOLUTE_TX_FEE { + tracing::warn!( + "CPFP fee {} below absolute minimum {}. Setting to minimum.", + final_fee.to_sat(), + MIN_ABSOLUTE_TX_FEE.to_sat(), + ); + final_fee = MIN_ABSOLUTE_TX_FEE; + } + + if final_fee > MAX_ABSOLUTE_TX_FEE { + tracing::warn!( + "CPFP fee {} exceeds absolute hard cap {}. Capping.", + final_fee.to_sat(), + MAX_ABSOLUTE_TX_FEE.to_sat(), + ); + final_fee = MAX_ABSOLUTE_TX_FEE; + } + + final_fee + } } impl Client { @@ -2443,6 +2613,19 @@ pub fn trace_status_change( new } +fn compute_effective_fee_rate( + fee_rate: FeeRate, + min_relay_fee: FeeRate, +) -> Result { + FeeRate::from_sat_per_vb( + fee_rate + .to_sat_per_vb_ceil() + .max(min_relay_fee.to_sat_per_vb_ceil()) + .max(FeeRate::BROADCAST_MIN.to_sat_per_vb_ceil()), + ) + .context("Failed to compute effective fee rate") +} + /// Estimate the absolute fee for a transaction. /// /// This function takes the following parameters: @@ -2491,14 +2674,7 @@ pub fn estimate_fee( // 2. The minimum relay fee rate (comes from fee estimation source, might vary depending on mempool congestion) // 3. The broadcast minimum fee rate (hardcoded in the Bitcoin library) // We round up to the next sat/vbyte - let recommended_fee_rate = FeeRate::from_sat_per_vb( - fee_rate_estimation - .to_sat_per_vb_ceil() - .max(min_relay_fee_rate.to_sat_per_vb_ceil()) - .max(FeeRate::BROADCAST_MIN.to_sat_per_vb_ceil()), - ) - .context("Failed to compute recommended fee rate")?; - + let recommended_fee_rate = compute_effective_fee_rate(fee_rate_estimation, min_relay_fee_rate)?; if recommended_fee_rate > fee_rate_estimation { tracing::warn!( "Estimated fee was below the minimum relay fee rate. Falling back to: {} sats/vbyte", diff --git a/swap/tests/child_fee_accounts_for_unconfirmed_parent.rs b/swap/tests/child_fee_accounts_for_unconfirmed_parent.rs new file mode 100644 index 000000000..f6eb55c72 --- /dev/null +++ b/swap/tests/child_fee_accounts_for_unconfirmed_parent.rs @@ -0,0 +1,112 @@ +pub mod harness; + +use anyhow::{Context, Result}; +use bitcoin::Amount; +use harness::FastCancelConfig; + +fn psbt_fee_sats(psbt: &bitcoin::psbt::Psbt) -> u64 { + let input_value: u64 = psbt + .inputs + .iter() + .map(|input| { + input + .witness_utxo + .as_ref() + .expect("missing witness_utxo") + .value + .to_sat() + }) + .sum(); + + let output_value: u64 = psbt + .unsigned_tx + .output + .iter() + .map(|output| output.value.to_sat()) + .sum(); + + input_value.saturating_sub(output_value) +} + +#[tokio::test] +async fn child_fee_accounts_for_unconfirmed_parent() -> Result<()> { + harness::setup_test(FastCancelConfig, None, None, |mut ctx| async move { + let (bob_swap, bob_handle) = ctx.bob_swap().await; + let wallet = bob_swap.bitcoin_wallet.clone(); + + bob_handle.abort(); + + let balance = wallet.balance().await?; + let parent_fee = Amount::from_sat(200); + let parent_amount = balance + .checked_sub(parent_fee) + .context("balance too small for parent transaction")?; + + let parent_dest = wallet.new_address().await?; + let parent_psbt = wallet + .send_to_address(parent_dest, parent_amount, parent_fee, None) + .await?; + + let parent_tx = wallet.sign_and_finalize(parent_psbt).await?; + let parent_txid = parent_tx.compute_txid(); + + wallet + .ensure_broadcasted(parent_tx.clone(), "low-fee-parent") + .await?; + + wallet.sync().await?; + + let child_dest = wallet.new_address().await?; + let child_amount = Amount::from_sat(100_000); + let child_psbt = wallet + .send_to_address_dynamic_fee(child_dest, child_amount, None) + .await?; + + assert!( + child_psbt + .unsigned_tx + .input + .iter() + .any(|input| input.previous_output.txid == parent_txid), + "child tx did not spend the unconfirmed parent output" + ); + + let child_fee_sat = psbt_fee_sats(&child_psbt); + let child_vbytes = child_psbt.unsigned_tx.weight().to_vbytes_floor() as u64; + let parent_vbytes = parent_tx.weight().to_vbytes_floor() as u64; + + // Minimum child fee needed for the combined package + // (parent + child) to reach ~1 sat/vB relay feerate + let min_package_relay_child_fee = parent_vbytes + .saturating_add(child_vbytes) + .saturating_sub(parent_fee.to_sat()); + + assert!( + child_fee_sat >= min_package_relay_child_fee, + "CPFP fee too low: child_fee={} sat, expected at least {} sat (parent_vbytes={}, child_vbytes={}, parent_fee={} sat)", + child_fee_sat, + min_package_relay_child_fee, + parent_vbytes, + child_vbytes, + parent_fee.to_sat(), + ); + + let package_fee_sat = parent_fee.to_sat().saturating_add(child_fee_sat); + let package_vbytes = parent_vbytes.saturating_add(child_vbytes); + + let parent_feerate = parent_fee.to_sat() as f64 / parent_vbytes as f64; + let package_feerate = package_fee_sat as f64 / package_vbytes as f64; + + assert!( + package_feerate > parent_feerate, + "CPFP did not improve package feerate (parent={} sat/vB, package={} sat/vB)", + parent_feerate, + package_feerate, + ); + + Ok(()) + }) + .await; + + Ok(()) +}