From b9e7b423826fb1bda6a8899c6af69cce76e6939a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 17 Feb 2026 17:07:22 +0000 Subject: [PATCH 1/3] refactor!: eliminate redundant satisfaction weight in SelectorParams Previously `SelectorParams` had both `change_script: ScriptSource` and `change_weight: DrainWeights`, which could disagree. Replace these with `ChangeScript` (which bundles the script with its satisfaction weight) and `ChangePolicy` (an enum instead of raw `bdk_coin_select::ChangePolicy`). `DrainWeights` is now derived internally from `ChangeScript`. Also removes `FeeStrategy` in favor of a direct `target_feerate` field, as the `AbsoluteFee` variant had a hacky implementation that conflicted with RBF logic. --- examples/anti_fee_sniping.rs | 13 +- examples/common.rs | 48 +------ examples/synopsis.rs | 23 ++-- src/input_candidates.rs | 6 +- src/lib.rs | 12 ++ src/selection.rs | 5 - src/selector.rs | 238 +++++++++++++++++++---------------- 7 files changed, 172 insertions(+), 173 deletions(-) diff --git a/examples/anti_fee_sniping.rs b/examples/anti_fee_sniping.rs index 0c6a81c..53b2334 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; @@ -75,13 +75,16 @@ fn main() -> anyhow::Result<()> { .into_selection( selection_algorithm_lowest_fee_bnb(longterm_feerate, 100_000), SelectorParams::new( - FeeStrategy::FeeRate(FeeRate::from_sat_per_vb_unchecked(10)), + 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(), + bdk_tx::ChangeScript::Descriptor(Box::new(internal.at_derivation_index(0)?)), + bdk_tx::ChangePolicy::NoDustLeastWaste { + longterm_feerate, + min_value: None, + }, ), )?; 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..c54bd9e 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; @@ -55,13 +55,16 @@ fn main() -> anyhow::Result<()> { .into_selection( selection_algorithm_lowest_fee_bnb(longterm_feerate, 100_000), SelectorParams::new( - FeeStrategy::FeeRate(FeeRate::from_sat_per_vb_unchecked(10)), + 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(), + bdk_tx::ChangeScript::Descriptor(Box::new(internal.at_derivation_index(0)?)), + bdk_tx::ChangePolicy::NoDustLeastWaste { + longterm_feerate, + min_value: None, + }, ), )?; @@ -128,18 +131,22 @@ 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::Descriptor(Box::new( internal.at_derivation_index(1)?, )), - change_policy: wallet.change_policy(), + change_policy: bdk_tx::ChangePolicy::NoDustLeastWaste { + longterm_feerate, + min_value: None, + }, // This ensures that we satisfy mempool-replacement policy rules 4 and 6. replace: Some(rbf_params), + dust_relay_feerate: None, }, )?; 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..3a3d68d 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>, } @@ -27,42 +30,79 @@ pub struct Selector<'c> { /// fields directly. #[derive(Debug, Clone)] 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. /// - /// Will error if this is unsatisfiable descriptor. - pub change_script: ScriptSource, + /// 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, /// The policy to determine whether we create a change output. pub change_policy: ChangePolicy, /// Params for replacing tx(s). pub replace: Option, + + /// Dust relay feerate used to calculate the dust threshold for change outputs. + /// + /// If `None`, defaults to 3 sat/vB (the Bitcoin Core default for `-dustrelayfee`). + pub dust_relay_feerate: 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. - /// - /// The actual rate can be higher. - FeeRate(bitcoin::FeeRate), - /// Pay an exact fee amount, regardless of transaction size. - /// - /// Change outputs will be created if their value exceeds the - /// long-term cost to spend them. - AbsoluteFee(Amount), +/// 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, Clone, PartialEq, Eq)] +pub enum ChangeScript { + /// A raw script pubkey. + Script { + /// The output script. + script: ScriptBuf, + /// Weight needed to satisfy this script in a future spending transaction. + /// + /// Can be `None` if the change policy does not require waste calculations. + satisfaction_weight: Option, + }, + /// A definite descriptor from which the script and satisfaction weight are both derived. + Descriptor(Box), +} + +impl ChangeScript { + /// 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()), + } + } +} + +/// Policy for deciding whether to create a change output. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum ChangePolicy { + /// Create a change output as long as it is not dust. + NoDust { + /// Minimum change value. If set, change below this value is forgone as fee. + min_value: Option, + }, + /// Create a change output as long as it is not dust and doing so reduces waste. + NoDustLeastWaste { + /// Long-term feerate estimate used to evaluate the future cost of spending the change. + longterm_feerate: FeeRate, + /// Minimum change value. If set, change below this value is forgone as fee. + min_value: Option, + }, } /// Rbf original tx stats. @@ -120,7 +160,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 +179,18 @@ impl RbfParams { impl SelectorParams { /// With default params. pub fn new( - fee_strategy: FeeStrategy, + target_feerate: FeeRate, target_outputs: Vec, - change_script: ScriptSource, + change_script: ChangeScript, change_policy: ChangePolicy, ) -> Self { Self { - fee_strategy, + target_feerate, target_outputs, change_script, change_policy, replace: None, + dust_relay_feerate: None, } } @@ -159,31 +200,70 @@ 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())), + /// To change output weights. + /// + /// # Error + /// + /// Fails if `change_descriptor` cannot be satisfied or the script's satisfaction weight is not + /// provided. + pub fn to_cs_change_policy(&self) -> Result { + let change_script = self.change_script.source().script(); + let min_non_dust = self.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()), + spend_weight: { + let satisfaction_weight = match &self.change_script { + ChangeScript::Script { + satisfaction_weight, + .. + } => satisfaction_weight.ok_or(miniscript::Error::CouldNotSatisfy)?, + ChangeScript::Descriptor(descriptor) => descriptor.max_weight_to_satisfy()?, } - } + .to_wu(); + // This code assumes that the change spend transaction is segwit. + bitcoin::TxIn::default().segwit_weight().to_wu() + satisfaction_weight + }, + n_outputs: 1, }; - Target { - fee, - outputs: target_outputs, - } + Ok(match self.change_policy { + ChangePolicy::NoDust { min_value } => bdk_coin_select::ChangePolicy::min_value( + change_weights, + min_non_dust.max(min_value.unwrap_or(Amount::ZERO)).to_sat(), + ), + ChangePolicy::NoDustLeastWaste { + longterm_feerate, + min_value, + } => bdk_coin_select::ChangePolicy::min_value_and_waste( + change_weights, + min_non_dust.max(min_value.unwrap_or(Amount::ZERO)).to_sat(), + self.target_feerate.into_cs_feerate(), + longterm_feerate.into_cs_feerate(), + ), + }) } } @@ -236,9 +316,11 @@ 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() + .map_err(SelectorError::Miniscript)?; 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 +354,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 +420,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); - } -} From ffd1211338aa1abc0b17fd73863b33efd53e1193 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 18 Feb 2026 13:32:56 +0000 Subject: [PATCH 2/3] feat: add constructor methods for ChangeScript and ChangePolicy Add ChangeScript::from_descriptor, from_descriptor_with_assets, and from_script constructors to reduce boilerplate. Add ChangePolicy::no_dust and no_dust_least_waste constructors with a builder-style min_value method. Also adds satisfaction_assets field to ChangeScript::Descriptor for tighter weight estimates via Plan, and folds satisfaction weight errors into SelectorError. --- examples/anti_fee_sniping.rs | 7 +- examples/synopsis.rs | 16 ++-- src/selector.rs | 164 +++++++++++++++++++++++++++++------ 3 files changed, 146 insertions(+), 41 deletions(-) diff --git a/examples/anti_fee_sniping.rs b/examples/anti_fee_sniping.rs index 53b2334..3fb52b2 100644 --- a/examples/anti_fee_sniping.rs +++ b/examples/anti_fee_sniping.rs @@ -80,11 +80,8 @@ fn main() -> anyhow::Result<()> { recipient_addr.script_pubkey(), Amount::from_sat(50_000_000), )], - bdk_tx::ChangeScript::Descriptor(Box::new(internal.at_derivation_index(0)?)), - bdk_tx::ChangePolicy::NoDustLeastWaste { - longterm_feerate, - min_value: None, - }, + bdk_tx::ChangeScript::from_descriptor(internal.at_derivation_index(0)?), + bdk_tx::ChangePolicy::no_dust_least_waste(longterm_feerate), ), )?; diff --git a/examples/synopsis.rs b/examples/synopsis.rs index c54bd9e..927c11b 100644 --- a/examples/synopsis.rs +++ b/examples/synopsis.rs @@ -60,11 +60,8 @@ fn main() -> anyhow::Result<()> { recipient_addr.script_pubkey(), Amount::from_sat(21_000_000), )], - bdk_tx::ChangeScript::Descriptor(Box::new(internal.at_derivation_index(0)?)), - bdk_tx::ChangePolicy::NoDustLeastWaste { - longterm_feerate, - min_value: None, - }, + bdk_tx::ChangeScript::from_descriptor(internal.at_derivation_index(0)?), + bdk_tx::ChangePolicy::no_dust_least_waste(longterm_feerate), ), )?; @@ -137,13 +134,10 @@ fn main() -> anyhow::Result<()> { // 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: bdk_tx::ChangeScript::Descriptor(Box::new( + change_script: bdk_tx::ChangeScript::from_descriptor( internal.at_derivation_index(1)?, - )), - change_policy: bdk_tx::ChangePolicy::NoDustLeastWaste { - longterm_feerate, - min_value: None, - }, + ), + change_policy: bdk_tx::ChangePolicy::no_dust_least_waste(longterm_feerate), // This ensures that we satisfy mempool-replacement policy rules 4 and 6. replace: Some(rbf_params), dust_relay_feerate: None, diff --git a/src/selector.rs b/src/selector.rs index 3a3d68d..0d30eab 100644 --- a/src/selector.rs +++ b/src/selector.rs @@ -28,7 +28,7 @@ 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 { /// Target feerate. /// @@ -62,27 +62,98 @@ pub struct SelectorParams { /// 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, Clone, PartialEq, Eq)] +#[derive(Debug)] pub enum ChangeScript { /// A raw script pubkey. Script { /// The output script. script: ScriptBuf, - /// Weight needed to satisfy this script in a future spending transaction. + /// 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 `None` if the change policy does not require waste calculations. satisfaction_weight: Option, }, /// A definite descriptor from which the script and satisfaction weight are both derived. - Descriptor(Box), + 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 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. + /// + /// 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: Option) -> 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()), + ChangeScript::Descriptor { descriptor, .. } => { + ScriptSource::Descriptor(descriptor.clone()) + } + } + } + + fn satisfaction_weight(&self) -> Result { + match &self { + ChangeScript::Script { + satisfaction_weight, + .. + } => satisfaction_weight.ok_or(SelectorError::MissingSatisfactionWeight), + 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), + }, } } } @@ -105,6 +176,38 @@ pub enum ChangePolicy { }, } +impl ChangePolicy { + /// Create a policy that avoids dust change outputs. + pub fn no_dust() -> Self { + Self::NoDust { min_value: None } + } + + /// Create a policy that avoids dust and minimizes waste. + pub fn no_dust_least_waste(longterm_feerate: FeeRate) -> Self { + Self::NoDustLeastWaste { + longterm_feerate, + min_value: None, + } + } + + /// Set a minimum change value. Change below this amount is forgone as fee. + #[must_use] + pub fn min_value(mut self, min_value: Amount) -> Self { + match &mut self { + Self::NoDust { min_value: mv, .. } => *mv = Some(min_value), + Self::NoDustLeastWaste { min_value: mv, .. } => *mv = Some(min_value), + } + self + } + + fn considers_waste(&self) -> bool { + match self { + ChangePolicy::NoDust { .. } => false, + ChangePolicy::NoDustLeastWaste { .. } => true, + } + } +} + /// Rbf original tx stats. #[derive(Debug, Clone, Copy)] pub struct OriginalTxStats { @@ -213,13 +316,18 @@ impl SelectorParams { } } - /// To change output weights. + /// Compute the [`bdk_coin_select::ChangePolicy`] from the current params. /// - /// # Error + /// # Errors + /// + /// Returns [`SelectorError::MissingSatisfactionWeight`] if the change script is a raw script + /// without a satisfaction weight and the change policy requires waste calculations. + /// + /// Returns [`SelectorError::InsufficientAssets`] if the provided assets cannot satisfy the + /// change descriptor. /// - /// Fails if `change_descriptor` cannot be satisfied or the script's satisfaction weight is not - /// provided. - pub fn to_cs_change_policy(&self) -> Result { + /// 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.dust_relay_feerate.map_or_else( || change_script.minimal_non_dust(), @@ -234,17 +342,13 @@ impl SelectorParams { }; temp_txout.weight().to_wu() }, - spend_weight: { - let satisfaction_weight = match &self.change_script { - ChangeScript::Script { - satisfaction_weight, - .. - } => satisfaction_weight.ok_or(miniscript::Error::CouldNotSatisfy)?, - ChangeScript::Descriptor(descriptor) => descriptor.max_weight_to_satisfy()?, - } - .to_wu(); + spend_weight: if self.change_policy.considers_waste() { // This code assumes that the change spend transaction is segwit. - bitcoin::TxIn::default().segwit_weight().to_wu() + satisfaction_weight + bitcoin::TxIn::default().segwit_weight().to_wu() + + self.change_script.satisfaction_weight()?.to_wu() + } else { + // Spend weight is not needed. + 0 }, n_outputs: 1, }; @@ -286,10 +390,15 @@ 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 change policy requires a satisfaction weight, but none was provided for the raw change + /// script. + MissingSatisfactionWeight, + /// The provided assets cannot satisfy the change descriptor. + InsufficientAssets, } impl fmt::Display for SelectorError { @@ -297,6 +406,13 @@ impl fmt::Display for SelectorError { match self { Self::Miniscript(err) => write!(f, "{err}"), Self::CannotMeetTarget(err) => write!(f, "{err}"), + Self::MissingSatisfactionWeight => write!( + f, + "change policy requires satisfaction weight, but none was provided for the raw script" + ), + Self::InsufficientAssets => { + write!(f, "provided assets cannot satisfy the change descriptor") + } } } } @@ -316,9 +432,7 @@ impl<'c> Selector<'c> { params: SelectorParams, ) -> Result { let target = params.to_cs_target(); - let change_policy = params - .to_cs_change_policy() - .map_err(SelectorError::Miniscript)?; + let change_policy = params.to_cs_change_policy()?; let target_outputs = params.target_outputs; let change_script = params.change_script.source(); if target.value() > candidates.groups().map(|grp| grp.value().to_sat()).sum() { From 8189b0317d722365185d7ae4d7ef4406f8c4312c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sat, 14 Mar 2026 11:18:45 +0000 Subject: [PATCH 3/3] refactor!: inline ChangePolicy fields into SelectorParams Remove the separate `ChangePolicy` type and inline its fields directly as `change_min_value` and `change_longterm_feerate` on `SelectorParams`. Also rename `dust_relay_feerate` to `change_dust_relay_feerate` for consistency, simplify `ChangeScript`'s satisfaction_weight from `Option` to `Weight`, and remove the now-unnecessary `MissingSatisfactionWeight` error variant. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/anti_fee_sniping.rs | 21 +++--- examples/synopsis.rs | 27 ++++--- src/selector.rs | 140 +++++++++++------------------------ 3 files changed, 70 insertions(+), 118 deletions(-) diff --git a/examples/anti_fee_sniping.rs b/examples/anti_fee_sniping.rs index 3fb52b2..0a5b4ef 100644 --- a/examples/anti_fee_sniping.rs +++ b/examples/anti_fee_sniping.rs @@ -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( - 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)?), - bdk_tx::ChangePolicy::no_dust_least_waste(longterm_feerate), - ), + 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/synopsis.rs b/examples/synopsis.rs index 927c11b..4f8dc6c 100644 --- a/examples/synopsis.rs +++ b/examples/synopsis.rs @@ -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( - 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)?), - bdk_tx::ChangePolicy::no_dust_least_waste(longterm_feerate), - ), + 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 { @@ -137,10 +140,12 @@ fn main() -> anyhow::Result<()> { change_script: bdk_tx::ChangeScript::from_descriptor( internal.at_derivation_index(1)?, ), - change_policy: bdk_tx::ChangePolicy::no_dust_least_waste(longterm_feerate), + // 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), - dust_relay_feerate: None, }, )?; diff --git a/src/selector.rs b/src/selector.rs index 0d30eab..6fc2e8a 100644 --- a/src/selector.rs +++ b/src/selector.rs @@ -45,16 +45,24 @@ pub struct SelectorParams { /// this. For descriptors it is computed automatically; for raw scripts it must be provided. pub change_script: ChangeScript, - /// The policy to determine whether we create a change output. - pub change_policy: ChangePolicy, - - /// Params for replacing tx(s). - pub replace: Option, - /// Dust relay feerate used to calculate the dust threshold for change outputs. /// /// If `None`, defaults to 3 sat/vB (the Bitcoin Core default for `-dustrelayfee`). - pub dust_relay_feerate: Option, + pub change_dust_relay_feerate: Option, + + /// 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, } /// Source of the change output script and its spending cost. @@ -75,8 +83,8 @@ pub enum ChangeScript { /// [`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 `None` if the change policy does not require waste calculations. - satisfaction_weight: Option, + /// 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 { @@ -118,7 +126,7 @@ impl ChangeScript { } /// Create from a raw script. - pub fn from_script(script: ScriptBuf, satisfaction_weight: Option) -> Self { + pub fn from_script(script: ScriptBuf, satisfaction_weight: Weight) -> Self { Self::Script { script, satisfaction_weight, @@ -140,7 +148,7 @@ impl ChangeScript { ChangeScript::Script { satisfaction_weight, .. - } => satisfaction_weight.ok_or(SelectorError::MissingSatisfactionWeight), + } => Ok(*satisfaction_weight), ChangeScript::Descriptor { descriptor, satisfaction_assets, @@ -158,56 +166,6 @@ impl ChangeScript { } } -/// Policy for deciding whether to create a change output. -#[derive(Debug, Clone, PartialEq, Eq)] -#[non_exhaustive] -pub enum ChangePolicy { - /// Create a change output as long as it is not dust. - NoDust { - /// Minimum change value. If set, change below this value is forgone as fee. - min_value: Option, - }, - /// Create a change output as long as it is not dust and doing so reduces waste. - NoDustLeastWaste { - /// Long-term feerate estimate used to evaluate the future cost of spending the change. - longterm_feerate: FeeRate, - /// Minimum change value. If set, change below this value is forgone as fee. - min_value: Option, - }, -} - -impl ChangePolicy { - /// Create a policy that avoids dust change outputs. - pub fn no_dust() -> Self { - Self::NoDust { min_value: None } - } - - /// Create a policy that avoids dust and minimizes waste. - pub fn no_dust_least_waste(longterm_feerate: FeeRate) -> Self { - Self::NoDustLeastWaste { - longterm_feerate, - min_value: None, - } - } - - /// Set a minimum change value. Change below this amount is forgone as fee. - #[must_use] - pub fn min_value(mut self, min_value: Amount) -> Self { - match &mut self { - Self::NoDust { min_value: mv, .. } => *mv = Some(min_value), - Self::NoDustLeastWaste { min_value: mv, .. } => *mv = Some(min_value), - } - self - } - - fn considers_waste(&self) -> bool { - match self { - ChangePolicy::NoDust { .. } => false, - ChangePolicy::NoDustLeastWaste { .. } => true, - } - } -} - /// Rbf original tx stats. #[derive(Debug, Clone, Copy)] pub struct OriginalTxStats { @@ -285,15 +243,15 @@ impl SelectorParams { target_feerate: FeeRate, target_outputs: Vec, change_script: ChangeScript, - change_policy: ChangePolicy, ) -> Self { Self { target_feerate, target_outputs, change_script, - change_policy, + change_min_value: None, + change_longterm_feerate: None, replace: None, - dust_relay_feerate: None, + change_dust_relay_feerate: None, } } @@ -320,16 +278,13 @@ impl SelectorParams { /// /// # Errors /// - /// Returns [`SelectorError::MissingSatisfactionWeight`] if the change script is a raw script - /// without a satisfaction weight and the change policy requires waste calculations. - /// /// 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.dust_relay_feerate.map_or_else( + 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), ); @@ -342,32 +297,28 @@ impl SelectorParams { }; temp_txout.weight().to_wu() }, - spend_weight: if self.change_policy.considers_waste() { - // This code assumes that the change spend transaction is segwit. - bitcoin::TxIn::default().segwit_weight().to_wu() - + self.change_script.satisfaction_weight()?.to_wu() - } else { - // Spend weight is not needed. - 0 - }, + // 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, }; - Ok(match self.change_policy { - ChangePolicy::NoDust { min_value } => bdk_coin_select::ChangePolicy::min_value( - change_weights, - min_non_dust.max(min_value.unwrap_or(Amount::ZERO)).to_sat(), - ), - ChangePolicy::NoDustLeastWaste { - longterm_feerate, - min_value, - } => bdk_coin_select::ChangePolicy::min_value_and_waste( - change_weights, - min_non_dust.max(min_value.unwrap_or(Amount::ZERO)).to_sat(), - self.target_feerate.into_cs_feerate(), - longterm_feerate.into_cs_feerate(), - ), - }) + 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) + }, + ) } } @@ -394,9 +345,6 @@ pub enum SelectorError { Miniscript(miniscript::Error), /// Meeting the target is not possible with the input candidates. CannotMeetTarget(CannotMeetTarget), - /// The change policy requires a satisfaction weight, but none was provided for the raw change - /// script. - MissingSatisfactionWeight, /// The provided assets cannot satisfy the change descriptor. InsufficientAssets, } @@ -406,10 +354,6 @@ impl fmt::Display for SelectorError { match self { Self::Miniscript(err) => write!(f, "{err}"), Self::CannotMeetTarget(err) => write!(f, "{err}"), - Self::MissingSatisfactionWeight => write!( - f, - "change policy requires satisfaction weight, but none was provided for the raw script" - ), Self::InsufficientAssets => { write!(f, "provided assets cannot satisfy the change descriptor") }