diff --git a/examples/anti_fee_sniping.rs b/examples/anti_fee_sniping.rs index 0c6a81c..0a5b4ef 100644 --- a/examples/anti_fee_sniping.rs +++ b/examples/anti_fee_sniping.rs @@ -1,8 +1,8 @@ #![allow(dead_code)] use bdk_testenv::{bitcoincore_rpc::RpcApi, TestEnv}; use bdk_tx::{ - filter_unspendable, group_by_spk, selection_algorithm_lowest_fee_bnb, FeeStrategy, Output, - PsbtParams, ScriptSource, SelectorParams, + filter_unspendable, group_by_spk, selection_algorithm_lowest_fee_bnb, Output, PsbtParams, + SelectorParams, }; use bitcoin::{absolute::LockTime, key::Secp256k1, Amount, FeeRate, Sequence}; use miniscript::Descriptor; @@ -74,15 +74,18 @@ fn main() -> anyhow::Result<()> { .filter(filter_unspendable(tip_height, Some(tip_time))) .into_selection( selection_algorithm_lowest_fee_bnb(longterm_feerate, 100_000), - SelectorParams::new( - FeeStrategy::FeeRate(FeeRate::from_sat_per_vb_unchecked(10)), - vec![Output::with_script( - recipient_addr.script_pubkey(), - Amount::from_sat(50_000_000), - )], - ScriptSource::Descriptor(Box::new(internal.at_derivation_index(0)?)), - wallet.change_policy(), - ), + SelectorParams { + // For waste optimization when deciding change. + change_longterm_feerate: Some(longterm_feerate), + ..SelectorParams::new( + FeeRate::from_sat_per_vb_unchecked(10), + vec![Output::with_script( + recipient_addr.script_pubkey(), + Amount::from_sat(50_000_000), + )], + bdk_tx::ChangeScript::from_descriptor(internal.at_derivation_index(0)?), + ) + }, )?; let fallback_locktime: LockTime = LockTime::from_consensus(tip_height.to_consensus_u32()); diff --git a/examples/common.rs b/examples/common.rs index e639906..22d3c7a 100644 --- a/examples/common.rs +++ b/examples/common.rs @@ -4,12 +4,11 @@ use bdk_bitcoind_rpc::{Emitter, NO_EXPECTED_MEMPOOL_TXIDS}; use bdk_chain::{ bdk_core, Anchor, Balance, CanonicalizationParams, ChainPosition, ConfirmationBlockTime, }; -use bdk_coin_select::{ChangePolicy, DrainWeights}; use bdk_testenv::{bitcoincore_rpc::RpcApi, TestEnv}; use bdk_tx::{ CanonicalUnspents, ConfirmationStatus, Input, InputCandidates, RbfParams, TxWithStatus, }; -use bitcoin::{absolute, Address, Amount, BlockHash, OutPoint, Transaction, TxOut, Txid}; +use bitcoin::{absolute, Address, BlockHash, OutPoint, Transaction, Txid}; use miniscript::{ plan::{Assets, Plan}, Descriptor, DescriptorPublicKey, ForEachKey, @@ -143,51 +142,6 @@ impl Wallet { .map(|c_tx| (c_tx.tx_node.tx, status_from_position(c_tx.chain_position))) } - /// Computes the weight of a change output plus the future weight to spend it. - pub fn drain_weights(&self) -> DrainWeights { - // Get descriptor of change keychain at a derivation index. - let desc = self - .graph - .index - .get_descriptor(INTERNAL) - .unwrap() - .at_derivation_index(0) - .unwrap(); - - // Compute the weight of a change output for this wallet. - let output_weight = TxOut { - script_pubkey: desc.script_pubkey(), - value: Amount::ZERO, - } - .weight() - .to_wu(); - - // The spend weight is the default input weight plus the plan satisfaction weight - // (this code assumes that we're only dealing with segwit transactions). - let plan = desc.plan(&self.assets()).expect("failed to create Plan"); - let spend_weight = - bitcoin::TxIn::default().segwit_weight().to_wu() + plan.satisfaction_weight() as u64; - - DrainWeights { - output_weight, - spend_weight, - n_outputs: 1, - } - } - - /// Get the default change policy for this wallet. - pub fn change_policy(&self) -> ChangePolicy { - let spk_0 = self - .graph - .index - .spk_at_index(INTERNAL, 0) - .expect("spk should exist in wallet"); - ChangePolicy { - min_value: spk_0.minimal_non_dust().to_sat(), - drain_weights: self.drain_weights(), - } - } - pub fn all_candidates(&self) -> bdk_tx::InputCandidates { let index = &self.graph.index; let assets = self.assets(); diff --git a/examples/synopsis.rs b/examples/synopsis.rs index 125efc0..4f8dc6c 100644 --- a/examples/synopsis.rs +++ b/examples/synopsis.rs @@ -1,7 +1,7 @@ use bdk_testenv::{bitcoincore_rpc::RpcApi, TestEnv}; use bdk_tx::{ - filter_unspendable, group_by_spk, selection_algorithm_lowest_fee_bnb, FeeStrategy, Output, - PsbtParams, ScriptSource, SelectorParams, Signer, + filter_unspendable, group_by_spk, selection_algorithm_lowest_fee_bnb, Output, PsbtParams, + SelectorParams, Signer, }; use bitcoin::{key::Secp256k1, Amount, FeeRate, Sequence}; use miniscript::Descriptor; @@ -54,15 +54,18 @@ fn main() -> anyhow::Result<()> { .filter(filter_unspendable(tip_height, Some(tip_mtp))) .into_selection( selection_algorithm_lowest_fee_bnb(longterm_feerate, 100_000), - SelectorParams::new( - FeeStrategy::FeeRate(FeeRate::from_sat_per_vb_unchecked(10)), - vec![Output::with_script( - recipient_addr.script_pubkey(), - Amount::from_sat(21_000_000), - )], - ScriptSource::Descriptor(Box::new(internal.at_derivation_index(0)?)), - wallet.change_policy(), - ), + SelectorParams { + // For waste-optimization when deciding change. + change_longterm_feerate: Some(longterm_feerate), + ..SelectorParams::new( + FeeRate::from_sat_per_vb_unchecked(10), + vec![Output::with_script( + recipient_addr.script_pubkey(), + Amount::from_sat(21_000_000), + )], + bdk_tx::ChangeScript::from_descriptor(internal.at_derivation_index(0)?), + ) + }, )?; let mut psbt = selection.create_psbt(PsbtParams { @@ -128,16 +131,19 @@ fn main() -> anyhow::Result<()> { SelectorParams { // This is just a lower-bound feerate. The actual result will be much higher to // satisfy mempool-replacement policy. - fee_strategy: FeeStrategy::FeeRate(FeeRate::from_sat_per_vb_unchecked(1)), + target_feerate: FeeRate::from_sat_per_vb_unchecked(1), // We cancel the tx by specifying no target outputs. This way, all excess returns // to our change output (unless if the prevouts picked are so small that it will // be less wasteful to have no output, however that will not be a valid tx). // If you only want to fee bump, put the original txs' recipients here. target_outputs: vec![], - change_script: ScriptSource::Descriptor(Box::new( + change_script: bdk_tx::ChangeScript::from_descriptor( internal.at_derivation_index(1)?, - )), - change_policy: wallet.change_policy(), + ), + // For waste optimization when deciding change. + change_longterm_feerate: Some(longterm_feerate), + change_min_value: None, + change_dust_relay_feerate: None, // This ensures that we satisfy mempool-replacement policy rules 4 and 6. replace: Some(rbf_params), }, diff --git a/src/input_candidates.rs b/src/input_candidates.rs index b77da7a..d257877 100644 --- a/src/input_candidates.rs +++ b/src/input_candidates.rs @@ -8,7 +8,7 @@ use miniscript::bitcoin; use crate::collections::{BTreeMap, HashSet}; use crate::{ - cs_feerate, CannotMeetTarget, Input, InputGroup, Selection, Selector, SelectorError, + CannotMeetTarget, FeeRateExt, Input, InputGroup, Selection, Selector, SelectorError, SelectorParams, }; @@ -291,10 +291,10 @@ pub fn selection_algorithm_lowest_fee_bnb( longterm_feerate: FeeRate, max_rounds: usize, ) -> impl FnMut(&mut Selector) -> Result<(), NoBnbSolution> { - let long_term_feerate = cs_feerate(longterm_feerate); + let long_term_feerate = longterm_feerate.into_cs_feerate(); move |selector| { let target = selector.target(); - let change_policy = selector.change_policy(); + let change_policy = selector.cs_change_policy(); selector .inner_mut() .run_bnb( diff --git a/src/lib.rs b/src/lib.rs index ebfa713..2699fc3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,3 +49,15 @@ pub(crate) mod collections { /// Definite descriptor. pub type DefiniteDescriptor = Descriptor; + +/// Extension trait for converting [`bitcoin::FeeRate`] to [`bdk_coin_select::FeeRate`]. +pub trait FeeRateExt { + /// Convert to a [`bdk_coin_select::FeeRate`]. + fn into_cs_feerate(self) -> bdk_coin_select::FeeRate; +} + +impl FeeRateExt for bitcoin::FeeRate { + fn into_cs_feerate(self) -> bdk_coin_select::FeeRate { + bdk_coin_select::FeeRate::from_sat_per_wu(self.to_sat_per_kwu() as f32 / 1000.0) + } +} diff --git a/src/selection.rs b/src/selection.rs index 962c686..220aa34 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -2,7 +2,6 @@ use alloc::boxed::Box; use alloc::vec::Vec; use core::fmt::{Debug, Display}; -use bdk_coin_select::FeeRate; use miniscript::bitcoin; use miniscript::bitcoin::{ absolute::{self, LockTime}, @@ -15,10 +14,6 @@ use crate::{apply_anti_fee_sniping, Finalizer, Input, Output}; const FALLBACK_SEQUENCE: bitcoin::Sequence = bitcoin::Sequence::ENABLE_LOCKTIME_NO_RBF; -pub(crate) fn cs_feerate(feerate: bitcoin::FeeRate) -> bdk_coin_select::FeeRate { - FeeRate::from_sat_per_wu(feerate.to_sat_per_kwu() as f32 / 1000.0) -} - /// Final selection of inputs and outputs. #[derive(Debug, Clone)] pub struct Selection { diff --git a/src/selector.rs b/src/selector.rs index 447000d..6fc2e8a 100644 --- a/src/selector.rs +++ b/src/selector.rs @@ -1,10 +1,13 @@ -use bdk_coin_select::{ChangePolicy, InsufficientFunds, Replace, Target, TargetFee, TargetOutputs}; -use bitcoin::{Amount, FeeRate, Transaction, Weight}; +use bdk_coin_select::{InsufficientFunds, Replace, Target, TargetFee, TargetOutputs}; +use bitcoin::{Amount, FeeRate, ScriptBuf, Transaction, Weight}; use miniscript::bitcoin; -use crate::{cs_feerate, InputCandidates, InputGroup, Output, ScriptSource, Selection}; +use crate::{ + DefiniteDescriptor, FeeRateExt, InputCandidates, InputGroup, Output, ScriptSource, Selection, +}; +use alloc::boxed::Box; use alloc::vec::Vec; -use core::fmt; +use core::fmt::{self, Debug}; /// A coin selector #[derive(Debug, Clone)] @@ -12,7 +15,7 @@ pub struct Selector<'c> { candidates: &'c InputCandidates, target_outputs: Vec, target: Target, - change_policy: ChangePolicy, + change_policy: bdk_coin_select::ChangePolicy, change_script: ScriptSource, inner: bdk_coin_select::CoinSelector<'c>, } @@ -25,44 +28,142 @@ pub struct Selector<'c> { /// * Error on anything that does not satisfy mempool policy. /// If the caller wants to create non-mempool-policy conforming txs, they can just fill in the /// fields directly. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct SelectorParams { - /// Fee targeting strategy. + /// Target feerate. /// - /// Either target a specific feerate or an absolute fee. - pub fee_strategy: FeeStrategy, + /// The actual feerate of the resulting transaction may be higher due to RBF requirements or + /// rounding. + pub target_feerate: FeeRate, /// Outputs that must be included. pub target_outputs: Vec, - /// To derive change output. + /// Source of the change output script. + /// + /// The satisfaction weight (cost of spending the change output in the future) is derived from + /// this. For descriptors it is computed automatically; for raw scripts it must be provided. + pub change_script: ChangeScript, + + /// Dust relay feerate used to calculate the dust threshold for change outputs. /// - /// Will error if this is unsatisfiable descriptor. - pub change_script: ScriptSource, + /// If `None`, defaults to 3 sat/vB (the Bitcoin Core default for `-dustrelayfee`). + pub change_dust_relay_feerate: Option, - /// The policy to determine whether we create a change output. - pub change_policy: ChangePolicy, + /// Minimum change value. + /// + /// A change value below this is forgone as fee. `None` means only the dust threshold applies. + pub change_min_value: Option, + + /// Long-term feerate for waste optimization when deciding whether to include change. + /// + /// `None` means no waste optimization - just enforce `change_min_value` (if specified) and the + /// dust threshold. + pub change_longterm_feerate: Option, /// Params for replacing tx(s). pub replace: Option, } -/// Fee targeting strategy. +/// Source of the change output script and its spending cost. /// -/// Choose `FeeRate` for standard wallet operations where fees should scale with -/// transaction size. Choose `AbsoluteFee` when you need exact fee amounts for -/// protocol-specific requirements. -#[derive(Debug, Clone)] -pub enum FeeStrategy { - /// Target a specific fee rate in sat/vB. +/// For a [`DefiniteDescriptor`], the satisfaction weight is derived automatically. For a raw +/// script (e.g. silent payments), the caller may provide it. It can be omitted if the change +/// policy does not require waste calculations. +#[derive(Debug)] +pub enum ChangeScript { + /// A raw script pubkey. + Script { + /// The output script. + script: ScriptBuf, + /// The weight of the witness/scriptSig data needed to spend this script in a future + /// transaction. + /// + /// This is the same value as + /// [`Plan::satisfaction_weight`](miniscript::plan::Plan::satisfaction_weight) and is used + /// by coin selection to estimate the cost of spending the change output. + /// + /// Can be `Weight::ZERO` if `SelectorParams::change_longterm_feerate` is unspecified. + satisfaction_weight: Weight, + }, + /// A definite descriptor from which the script and satisfaction weight are both derived. + Descriptor { + /// The descriptor. + descriptor: Box, + /// Assets available for satisfying the descriptor. + /// + /// If provided, the satisfaction weight is computed via [`Plan`](miniscript::plan::Plan) + /// for a tighter estimate. If `None`, falls back to + /// [`max_weight_to_satisfy`](DefiniteDescriptor::max_weight_to_satisfy). + satisfaction_assets: Option, + }, +} + +impl ChangeScript { + /// Create from a [`DefiniteDescriptor`]. /// - /// The actual rate can be higher. - FeeRate(bitcoin::FeeRate), - /// Pay an exact fee amount, regardless of transaction size. + /// The satisfaction weight is derived via + /// [`max_weight_to_satisfy`](DefiniteDescriptor::max_weight_to_satisfy). + pub fn from_descriptor(descriptor: DefiniteDescriptor) -> Self { + Self::Descriptor { + descriptor: Box::new(descriptor), + satisfaction_assets: None, + } + } + + /// Create from a [`DefiniteDescriptor`] with known assets. /// - /// Change outputs will be created if their value exceeds the - /// long-term cost to spend them. - AbsoluteFee(Amount), + /// The satisfaction weight is derived via [`Plan`](miniscript::plan::Plan) for a tighter + /// estimate based on the provided assets. + pub fn from_descriptor_with_assets( + descriptor: DefiniteDescriptor, + assets: miniscript::plan::Assets, + ) -> Self { + Self::Descriptor { + descriptor: Box::new(descriptor), + satisfaction_assets: Some(assets), + } + } + + /// Create from a raw script. + pub fn from_script(script: ScriptBuf, satisfaction_weight: Weight) -> Self { + Self::Script { + script, + satisfaction_weight, + } + } + + /// Convert to a [`ScriptSource`], discarding the satisfaction weight. + pub fn source(&self) -> ScriptSource { + match self { + ChangeScript::Script { script, .. } => ScriptSource::Script(script.clone()), + ChangeScript::Descriptor { descriptor, .. } => { + ScriptSource::Descriptor(descriptor.clone()) + } + } + } + + fn satisfaction_weight(&self) -> Result { + match &self { + ChangeScript::Script { + satisfaction_weight, + .. + } => Ok(*satisfaction_weight), + ChangeScript::Descriptor { + descriptor, + satisfaction_assets, + } => match satisfaction_assets { + Some(assets) => descriptor + .clone() + .plan(assets) + .map(|p| Weight::from_wu_usize(p.satisfaction_weight())) + .map_err(|_| SelectorError::InsufficientAssets), + None => descriptor + .max_weight_to_satisfy() + .map_err(SelectorError::Miniscript), + }, + } + } } /// Rbf original tx stats. @@ -120,7 +221,7 @@ impl RbfParams { pub fn to_cs_replace(&self) -> Replace { Replace { fee: self.original_txs.iter().map(|otx| otx.fee.to_sat()).sum(), - incremental_relay_feerate: cs_feerate(self.incremental_relay_feerate), + incremental_relay_feerate: self.incremental_relay_feerate.into_cs_feerate(), } } @@ -139,17 +240,18 @@ impl RbfParams { impl SelectorParams { /// With default params. pub fn new( - fee_strategy: FeeStrategy, + target_feerate: FeeRate, target_outputs: Vec, - change_script: ScriptSource, - change_policy: ChangePolicy, + change_script: ChangeScript, ) -> Self { Self { - fee_strategy, + target_feerate, target_outputs, change_script, - change_policy, + change_min_value: None, + change_longterm_feerate: None, replace: None, + change_dust_relay_feerate: None, } } @@ -159,31 +261,64 @@ impl SelectorParams { .replace .as_ref() .map_or(FeeRate::ZERO, |r| r.max_feerate()); + Target { + fee: TargetFee { + rate: self.target_feerate.max(feerate_lb).into_cs_feerate(), + replace: self.replace.as_ref().map(|r| r.to_cs_replace()), + }, + outputs: TargetOutputs::fund_outputs( + self.target_outputs + .iter() + .map(|o| (o.txout().weight().to_wu(), o.value.to_sat())), + ), + } + } - let mut target_outputs = TargetOutputs::fund_outputs( - self.target_outputs - .iter() - .map(|output| (output.txout().weight().to_wu(), output.value.to_sat())), + /// Compute the [`bdk_coin_select::ChangePolicy`] from the current params. + /// + /// # Errors + /// + /// Returns [`SelectorError::InsufficientAssets`] if the provided assets cannot satisfy the + /// change descriptor. + /// + /// Returns [`SelectorError::Miniscript`] if the change descriptor is inherently unsatisfiable. + pub fn to_cs_change_policy(&self) -> Result { + let change_script = self.change_script.source().script(); + let min_non_dust = self.change_dust_relay_feerate.map_or_else( + || change_script.minimal_non_dust(), + |r| change_script.minimal_non_dust_custom(r), ); - let fee = match &self.fee_strategy { - FeeStrategy::FeeRate(fee_rate) => TargetFee { - rate: cs_feerate(*fee_rate.max(&feerate_lb)), - replace: self.replace.as_ref().map(|r| r.to_cs_replace()), + let change_weights = bdk_coin_select::DrainWeights { + output_weight: { + let temp_txout = bitcoin::TxOut { + value: Amount::ZERO, + script_pubkey: change_script, + }; + temp_txout.weight().to_wu() }, - FeeStrategy::AbsoluteFee(amount) => { - target_outputs.value_sum += amount.to_sat(); - TargetFee { - rate: bdk_coin_select::FeeRate::ZERO, - replace: self.replace.as_ref().map(|r| r.to_cs_replace()), - } - } + // This code assumes that the change spend transaction is segwit. + spend_weight: bitcoin::TxIn::default().segwit_weight().to_wu() + + self.change_script.satisfaction_weight()?.to_wu(), + n_outputs: 1, }; - Target { - fee, - outputs: target_outputs, - } + let min_value = min_non_dust + .max(self.change_min_value.unwrap_or(Amount::ZERO)) + .to_sat(); + + Ok( + if let Some(longterm_feerate) = self.change_longterm_feerate { + bdk_coin_select::ChangePolicy::min_value_and_waste( + change_weights, + min_value, + self.target_feerate.into_cs_feerate(), + longterm_feerate.into_cs_feerate(), + ) + } else { + bdk_coin_select::ChangePolicy::min_value(change_weights, min_value) + }, + ) } } @@ -206,10 +341,12 @@ impl std::error::Error for CannotMeetTarget {} /// Selector error #[derive(Debug)] pub enum SelectorError { - /// miniscript error + /// Miniscript error (e.g. the change descriptor is inherently unsatisfiable). Miniscript(miniscript::Error), - /// meeting the target is not possible + /// Meeting the target is not possible with the input candidates. CannotMeetTarget(CannotMeetTarget), + /// The provided assets cannot satisfy the change descriptor. + InsufficientAssets, } impl fmt::Display for SelectorError { @@ -217,6 +354,9 @@ impl fmt::Display for SelectorError { match self { Self::Miniscript(err) => write!(f, "{err}"), Self::CannotMeetTarget(err) => write!(f, "{err}"), + Self::InsufficientAssets => { + write!(f, "provided assets cannot satisfy the change descriptor") + } } } } @@ -236,9 +376,9 @@ impl<'c> Selector<'c> { params: SelectorParams, ) -> Result { let target = params.to_cs_target(); - let change_policy = params.change_policy; + let change_policy = params.to_cs_change_policy()?; let target_outputs = params.target_outputs; - let change_script = params.change_script; + let change_script = params.change_script.source(); if target.value() > candidates.groups().map(|grp| grp.value().to_sat()).sum() { return Err(SelectorError::CannotMeetTarget(CannotMeetTarget)); } @@ -272,7 +412,7 @@ impl<'c> Selector<'c> { } /// Coin selection change policy. - pub fn change_policy(&self) -> ChangePolicy { + pub fn cs_change_policy(&self) -> bdk_coin_select::ChangePolicy { self.change_policy } @@ -338,57 +478,3 @@ impl<'c> Selector<'c> { }) } } - -#[cfg(test)] -mod tests { - use super::*; - use bdk_coin_select::DrainWeights; - use bdk_coin_select::FeeRate as CsFeeRate; - use bitcoin::{Amount, FeeRate, ScriptBuf}; - - fn create_output(value: u64) -> Output { - Output::with_script(ScriptBuf::new(), Amount::from_sat(value)) - } - - fn change_script() -> ScriptSource { - ScriptSource::from_script(ScriptBuf::new()) - } - - fn change_policy() -> ChangePolicy { - ChangePolicy::min_value(DrainWeights::TR_KEYSPEND, 330) - } - - #[test] - fn test_absolute_fee_vs_feerate_target_value() { - let output_value: u64 = 100_000; - let absolute_fee: u64 = 5_000; - let target_outputs = vec![create_output(output_value)]; - - // With absolute fee - let params_absolute = SelectorParams::new( - FeeStrategy::AbsoluteFee(Amount::from_sat(absolute_fee)), - target_outputs.clone(), - change_script(), - change_policy(), - ); - - // With fee rate - let params_feerate = SelectorParams::new( - FeeStrategy::FeeRate(FeeRate::from_sat_per_vb(10).unwrap()), - target_outputs, - change_script(), - change_policy(), - ); - - let target_absolute = params_absolute.to_cs_target(); - let target_feerate = params_feerate.to_cs_target(); - - assert_eq!( - target_absolute.outputs.value_sum, - output_value + absolute_fee - ); - assert_eq!(target_absolute.fee.rate, CsFeeRate::ZERO); - assert_eq!(target_feerate.outputs.value_sum, output_value); - assert!(target_feerate.fee.rate > CsFeeRate::ZERO); - } -}