From d43e81ac1c89235de2b93b4ad3af0683779a0f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 19 May 2026 12:09:12 +0000 Subject: [PATCH 1/5] feat: Add `fisher_yates_shuffle` method and refactor workspace * move no-std rand functions to `no_std_rand.rs` * move AFS types to `afs.rs` * Add `fisher_yates_shuffle` function that shuffles list elements --- src/{utils.rs => afs.rs} | 22 ++++---------------- src/lib.rs | 6 ++++-- src/no_std_rand.rs | 44 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 20 deletions(-) rename src/{utils.rs => afs.rs} (92%) create mode 100644 src/no_std_rand.rs diff --git a/src/utils.rs b/src/afs.rs similarity index 92% rename from src/utils.rs rename to src/afs.rs index 2fd5eaa..9fd2bb0 100644 --- a/src/utils.rs +++ b/src/afs.rs @@ -1,4 +1,7 @@ -use crate::Input; +use crate::{ + no_std_rand::{random_probability, random_range}, + Input, +}; use alloc::vec::Vec; use miniscript::bitcoin::{ absolute::{self, LockTime}, @@ -157,20 +160,3 @@ pub(crate) fn apply_anti_fee_sniping( Ok(()) } - -/// Returns true with probability 1/n. -fn random_probability(rng: &mut impl RngCore, n: u32) -> bool { - random_range(rng, n) == 0 -} - -/// Returns a random value in the range [0, n) using unbiased rejection sampling. -fn random_range(rng: &mut impl RngCore, n: u32) -> u32 { - let threshold = n.wrapping_neg() % n; - - loop { - let value = rng.next_u32(); - if value >= threshold { - return value % n; - } - } -} diff --git a/src/lib.rs b/src/lib.rs index 0587350..eea52ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,17 +9,19 @@ extern crate alloc; #[cfg(feature = "std")] extern crate std; +mod afs; mod canonical_unspents; mod finalizer; mod input; mod input_candidates; +mod no_std_rand; mod output; mod rbf; mod selection; mod selector; mod signer; -mod utils; +pub use afs::*; pub use canonical_unspents::*; pub use finalizer::*; pub use input::*; @@ -27,12 +29,12 @@ pub use input_candidates::*; pub use miniscript; pub use miniscript::bitcoin; use miniscript::{DefiniteDescriptorKey, Descriptor}; +use no_std_rand::*; pub use output::*; pub use rbf::*; pub use selection::*; pub use selector::*; pub use signer::*; -pub use utils::*; #[cfg(feature = "std")] pub(crate) mod collections { diff --git a/src/no_std_rand.rs b/src/no_std_rand.rs new file mode 100644 index 0000000..b07908f --- /dev/null +++ b/src/no_std_rand.rs @@ -0,0 +1,44 @@ +use rand_core::RngCore; + +/// Returns true with probability 1/n. +pub(crate) fn random_probability(rng: &mut impl RngCore, n: u32) -> bool { + random_range(rng, n) == 0 +} + +/// Returns a random value in the range [0, n) using unbiased rejection sampling. +pub(crate) fn random_range(rng: &mut impl RngCore, n: u32) -> u32 { + let threshold = n.wrapping_neg() % n; + + loop { + let value = rng.next_u32(); + if value >= threshold { + return value % n; + } + } +} + +/// Fisher-Yates in-place shuffle using unbiased rejection sampling. +pub(crate) fn fisher_yates_shuffle(slice: &mut [T], rng: &mut impl RngCore) { + for i in (1..slice.len()).rev() { + // Unbiased index in [0, i+1) via rejection sampling. + let j = random_range(rng, (i + 1) as u32) as usize; + slice.swap(i, j); + } +} + +#[cfg_attr(coverage_nightly, coverage(off))] +#[cfg(test)] +mod tests { + use super::*; + use alloc::vec::Vec; + use rand_core::OsRng; + + #[test] + fn test_fisher_yates_shuffle_preserves_multiset() { + let original: Vec = (0..32).collect(); + let mut shuffled = original.clone(); + fisher_yates_shuffle(&mut shuffled, &mut OsRng); + shuffled.sort(); + assert_eq!(shuffled, original); + } +} From 4ce801d00238cfec33a377e2767f3034ae2a3c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sun, 17 May 2026 14:13:38 +0000 Subject: [PATCH 2/5] refactor(selection)!: make fields private, add InputMut handle `Selection.inputs` / `Selection.outputs` are now private; access through `inputs()` / `outputs()`. Mutation goes through `InputMut`, returned by `input_mut(outpoint)` and `inputs_mut()`. `InputMut` derefs to `&Input` and only exposes `set_sequence`, preventing whole-input replacement that would silently break coin-selection invariants. --- examples/anti_fee_sniping.rs | 2 +- examples/synopsis.rs | 2 +- src/finalizer.rs | 41 ++++++-------- src/input.rs | 33 +++++++++++ src/selection.rs | 103 ++++++++++++++++++++++------------- src/selector.rs | 34 +++++------- 6 files changed, 131 insertions(+), 84 deletions(-) diff --git a/examples/anti_fee_sniping.rs b/examples/anti_fee_sniping.rs index 82174ca..da318b9 100644 --- a/examples/anti_fee_sniping.rs +++ b/examples/anti_fee_sniping.rs @@ -88,7 +88,7 @@ fn main() -> anyhow::Result<()> { }, )?; - let selection_inputs = selection.inputs.clone(); + let selection_inputs = selection.inputs().to_vec(); let psbt = selection.create_psbt(PsbtParams { anti_fee_sniping: Some(tip_height), diff --git a/examples/synopsis.rs b/examples/synopsis.rs index db238b3..2d629ba 100644 --- a/examples/synopsis.rs +++ b/examples/synopsis.rs @@ -150,7 +150,7 @@ fn main() -> anyhow::Result<()> { println!( "selected inputs: {:?}", selection - .inputs + .inputs() .iter() .map(|input| input.prev_outpoint()) .collect::>() diff --git a/src/finalizer.rs b/src/finalizer.rs index cffa850..582494f 100644 --- a/src/finalizer.rs +++ b/src/finalizer.rs @@ -27,7 +27,7 @@ use miniscript::{bitcoin, plan::Plan, psbt::PsbtInputSatisfier}; /// # use bdk_tx::PsbtParams; /// # let secp = bitcoin::secp256k1::Secp256k1::new(); /// # let keymap = std::collections::BTreeMap::new(); -/// # let selection = bdk_tx::Selection { inputs: vec![], outputs: vec![] }; +/// # let selection: bdk_tx::Selection = unimplemented!(); /// // Create PSBT from a selection of inputs and outputs. /// let mut psbt = selection.create_psbt(PsbtParams::default())?; /// @@ -217,10 +217,7 @@ mod tests { fn test_finalize_single_input() -> anyhow::Result<()> { let (input, keymap) = create_input_from_descriptor_at(TR_XPRV, 0)?; let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); - let selection = Selection { - inputs: vec![input], - outputs: vec![output], - }; + let selection = Selection::new(vec![input], vec![output]); let mut psbt = selection.create_psbt(PsbtParams::default())?; let finalizer = selection.into_finalizer(); @@ -240,10 +237,7 @@ mod tests { fn test_finalize_sets_final_script_sig() -> anyhow::Result<()> { let (input, keymap) = create_input_from_descriptor_at(PKH_XPRV, 0)?; let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); - let selection = Selection { - inputs: vec![input], - outputs: vec![output], - }; + let selection = Selection::new(vec![input], vec![output]); let mut psbt = selection.create_psbt(PsbtParams::default())?; let finalizer = selection.into_finalizer(); @@ -266,13 +260,13 @@ mod tests { let taproot_output_descriptor = derive_descriptor_at(TR_XPRV, 10)?; let wpkh_output_descriptor = derive_descriptor_at(WPKH_XPRV, 11)?; - let selection = Selection { - inputs: vec![input_0, input_1, input_2], - outputs: vec![ + let selection = Selection::new( + vec![input_0, input_1, input_2], + vec![ Output::with_descriptor(taproot_output_descriptor, Amount::from_sat(20_000)), Output::with_descriptor(wpkh_output_descriptor, Amount::from_sat(22_000)), ], - }; + ); let mut psbt = selection.create_psbt(PsbtParams::default())?; let finalizer = selection.into_finalizer(); @@ -321,13 +315,13 @@ mod tests { input_0.plan().cloned().expect("plan must exist"), )]); - let selection = Selection { - inputs: vec![input_0, input_1], - outputs: vec![ + let selection = Selection::new( + vec![input_0, input_1], + vec![ Output::with_descriptor(taproot_output_descriptor, Amount::from_sat(20_000)), Output::with_descriptor(wpkh_output_descriptor, Amount::from_sat(22_000)), ], - }; + ); let mut psbt = selection.create_psbt(PsbtParams::default())?; @@ -361,13 +355,13 @@ mod tests { let (input, _) = create_input_from_descriptor_at(TR_XPRV, 0)?; let taproot_output_descriptor = derive_descriptor_at(TR_XPRV, 10)?; let wpkh_output_descriptor = derive_descriptor_at(WPKH_XPRV, 11)?; - let selection = Selection { - inputs: vec![input], - outputs: vec![ + let selection = Selection::new( + vec![input], + vec![ Output::with_descriptor(taproot_output_descriptor, Amount::from_sat(20_000)), Output::with_descriptor(wpkh_output_descriptor, Amount::from_sat(22_000)), ], - }; + ); let mut psbt = selection.create_psbt(PsbtParams::default())?; let finalizer = selection.into_finalizer(); @@ -395,10 +389,7 @@ mod tests { fn test_already_finalized_input() -> anyhow::Result<()> { let (input, keymap) = create_input_from_descriptor_at(TR_XPRV, 0)?; let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); - let selection = Selection { - inputs: vec![input], - outputs: vec![output], - }; + let selection = Selection::new(vec![input], vec![output]); let mut psbt = selection.create_psbt(PsbtParams::default())?; let finalizer = selection.into_finalizer(); diff --git a/src/input.rs b/src/input.rs index b949b01..dfe632c 100644 --- a/src/input.rs +++ b/src/input.rs @@ -665,6 +665,39 @@ impl Input { } } +/// Mutable handle to an [`Input`] held inside a [`Selection`]. +/// +/// Returned by [`Selection::input_mut`] and [`Selection::inputs_mut`]. This wrapper restricts +/// mutation to operations that preserve [`Selection`]'s coin-selection invariants. +/// +/// Read-only access to the underlying [`Input`] is available via [`Deref`]. +/// +/// [`Selection`]: crate::Selection +/// [`Selection::input_mut`]: crate::Selection::input_mut +/// [`Selection::inputs_mut`]: crate::Selection::inputs_mut +/// [`Deref`]: core::ops::Deref +#[derive(Debug)] +pub struct InputMut<'a>(&'a mut Input); + +impl<'a> InputMut<'a> { + pub(crate) fn new(input: &'a mut Input) -> Self { + Self(input) + } + + /// See [`Input::set_sequence`]. + pub fn set_sequence(&mut self, sequence: Sequence) -> Result<(), SetSequenceError> { + self.0.set_sequence(sequence) + } +} + +impl core::ops::Deref for InputMut<'_> { + type Target = Input; + + fn deref(&self) -> &Input { + self.0 + } +} + /// Input group. Cannot be empty. #[derive(Debug, Clone)] pub struct InputGroup(Vec); diff --git a/src/selection.rs b/src/selection.rs index 48cb851..8d3965b 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -3,19 +3,18 @@ use alloc::vec::Vec; use core::fmt::{Debug, Display}; use miniscript::bitcoin; -use miniscript::bitcoin::{absolute, transaction, Psbt, Sequence}; +use miniscript::bitcoin::{absolute, transaction, OutPoint, Psbt, Sequence}; use miniscript::psbt::PsbtExt; use rand_core::RngCore; -use crate::{apply_anti_fee_sniping, AntiFeeSnipingError, Finalizer, Input, Output}; +use crate::{apply_anti_fee_sniping, AntiFeeSnipingError, Finalizer, Input, InputMut, Output}; /// Final selection of inputs and outputs. #[derive(Debug, Clone)] +#[must_use] pub struct Selection { - /// Inputs in this selection. - pub inputs: Vec, - /// Outputs in this selection. - pub outputs: Vec, + inputs: Vec, + outputs: Vec, } /// Parameters for creating a psbt. @@ -140,6 +139,40 @@ impl core::fmt::Display for CreatePsbtError { impl std::error::Error for CreatePsbtError {} impl Selection { + pub(crate) fn new(inputs: Vec, outputs: Vec) -> Self { + Self { inputs, outputs } + } + + /// Inputs in this selection. + pub fn inputs(&self) -> &[Input] { + &self.inputs + } + + /// Outputs in this selection. + pub fn outputs(&self) -> &[Output] { + &self.outputs + } + + /// Mutable handle to the input spending `outpoint`, if any. + /// + /// Returns [`None`] if no input in this selection spends `outpoint`. The returned + /// [`InputMut`] only permits mutations that preserve the selection's coin-selection + /// invariants — see [`InputMut`] for the available operations. + pub fn input_mut(&mut self, outpoint: OutPoint) -> Option> { + self.inputs + .iter_mut() + .find(|input| input.prev_outpoint() == outpoint) + .map(InputMut::new) + } + + /// Iterator yielding a mutable handle to every input in this selection. + /// + /// Each yielded [`InputMut`] only permits mutations that preserve the selection's + /// coin-selection invariants — see [`InputMut`] for the available operations. + pub fn inputs_mut(&mut self) -> impl Iterator> { + self.inputs.iter_mut().map(InputMut::new) + } + /// Accumulates the maximum locktime from an iterator of input-required locktimes. /// /// Returns the `min_locktime` if the locktimes iterator is empty, `Ok(lock_time)` with @@ -327,13 +360,13 @@ mod tests { let (input, desc) = setup_cltv_input(abs_locktime)?; - let selection = Selection { - inputs: vec![input], - outputs: vec![Output::with_descriptor( + let selection = Selection::new( + vec![input], + vec![Output::with_descriptor( desc.at_derivation_index(1)?, Amount::from_sat(1000), )], - }; + ); struct TestCase { name: &'static str, @@ -387,13 +420,13 @@ mod tests { let (input, desc) = setup_cltv_input(time_locktime)?; - let selection = Selection { - inputs: vec![input], - outputs: vec![Output::with_descriptor( + let selection = Selection::new( + vec![input], + vec![Output::with_descriptor( desc.at_derivation_index(1)?, Amount::from_sat(1000), )], - }; + ); // Default `min_locktime` is height 0 (block-height unit). It is incompatible with // the time-based CLTV requirement, so it must be ignored. @@ -454,10 +487,7 @@ mod tests { let current_height = 2_500; let input = setup_test_input(2_000).unwrap(); let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); - let selection = Selection { - inputs: vec![input], - outputs: vec![output], - }; + let selection = Selection::new(vec![input], vec![output]); // Disabled - default behavior is disable let psbt = selection.create_psbt(PsbtParams { @@ -482,10 +512,7 @@ mod tests { while !used_locktime || !used_sequence { let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); - let selection = Selection { - inputs: vec![input.clone()], - outputs: vec![output], - }; + let selection = Selection::new(vec![input.clone()], vec![output]); let psbt = selection.create_psbt(PsbtParams { anti_fee_sniping: Some(tip), @@ -534,10 +561,10 @@ mod tests { let mut loops = 0; while !used_locktime || !used_sequence { - let selection = Selection { - inputs: vec![input1.clone(), input2.clone(), input3.clone()], - outputs: vec![output.clone()], - }; + let selection = Selection::new( + vec![input1.clone(), input2.clone(), input3.clone()], + vec![output.clone()], + ); let psbt = selection .create_psbt(PsbtParams { anti_fee_sniping: Some(tip), @@ -576,13 +603,13 @@ mod tests { // Tip is well below the input's CLTV requirement. let tip = absolute::Height::from_consensus(50_000)?; - let selection = Selection { - inputs: vec![input], - outputs: vec![Output::with_descriptor( + let selection = Selection::new( + vec![input], + vec![Output::with_descriptor( desc.at_derivation_index(1)?, Amount::from_sat(1000), )], - }; + ); // The input is wsh (not Taproot), so AFS deterministically takes the locktime path; loop a // few times anyway as cheap insurance against future control-flow changes. @@ -650,10 +677,10 @@ mod tests { let mut observed_sequence_path = false; for _ in 0..100 { - let selection = Selection { - inputs: vec![regular_input.clone(), csv_input.clone()], - outputs: vec![output.clone()], - }; + let selection = Selection::new( + vec![regular_input.clone(), csv_input.clone()], + vec![output.clone()], + ); let psbt = selection.create_psbt(PsbtParams { anti_fee_sniping: Some(tip), ..Default::default() @@ -697,13 +724,13 @@ mod tests { let (input, desc) = setup_cltv_input(time_locktime)?; let tip = absolute::Height::from_consensus(800_000)?; - let selection = Selection { - inputs: vec![input], - outputs: vec![Output::with_descriptor( + let selection = Selection::new( + vec![input], + vec![Output::with_descriptor( desc.at_derivation_index(1)?, Amount::from_sat(1000), )], - }; + ); let result = selection.create_psbt(PsbtParams { anti_fee_sniping: Some(tip), diff --git a/src/selector.rs b/src/selector.rs index 7a6e4e7..d907cca 100644 --- a/src/selector.rs +++ b/src/selector.rs @@ -465,24 +465,20 @@ impl<'c> Selector<'c> { } let maybe_change = self.inner.drain(self.target, self.change_policy); let to_apply = self.candidates.groups().collect::>(); - Some(Selection { - inputs: self - .inner - .apply_selection(&to_apply) - .copied() - .flat_map(InputGroup::inputs) - .cloned() - .collect(), - outputs: { - let mut outputs = self.target_outputs.clone(); - if maybe_change.is_some() { - outputs.push(Output::from(( - self.change_script.clone(), - Amount::from_sat(maybe_change.value), - ))); - } - outputs - }, - }) + let inputs = self + .inner + .apply_selection(&to_apply) + .copied() + .flat_map(InputGroup::inputs) + .cloned() + .collect(); + let mut outputs = self.target_outputs.clone(); + if maybe_change.is_some() { + outputs.push(Output::from(( + self.change_script.clone(), + Amount::from_sat(maybe_change.value), + ))); + } + Some(Selection::new(inputs, outputs)) } } From d158cbf5129ae9fb8d7af8b983511f16ae376c9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sun, 17 May 2026 14:53:21 +0000 Subject: [PATCH 3/5] feat(selection): add sort/shuffle methods for inputs and outputs Adds `sort_inputs_by`, `shuffle_inputs`, `sort_outputs_by`, `shuffle_outputs` on `Selection`. Covers BIP-69 ordering (via sort_by) and chain-analysis shuffling without exposing raw mutable slots that would let callers replace inputs/outputs and silently break coin-selection invariants. --- src/selection.rs | 52 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/selection.rs b/src/selection.rs index 8d3965b..19d3e68 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -1,5 +1,6 @@ use alloc::boxed::Box; use alloc::vec::Vec; +use core::cmp::Ordering; use core::fmt::{Debug, Display}; use miniscript::bitcoin; @@ -7,7 +8,10 @@ use miniscript::bitcoin::{absolute, transaction, OutPoint, Psbt, Sequence}; use miniscript::psbt::PsbtExt; use rand_core::RngCore; -use crate::{apply_anti_fee_sniping, AntiFeeSnipingError, Finalizer, Input, InputMut, Output}; +use crate::{ + apply_anti_fee_sniping, fisher_yates_shuffle, AntiFeeSnipingError, Finalizer, Input, InputMut, + Output, +}; /// Final selection of inputs and outputs. #[derive(Debug, Clone)] @@ -173,6 +177,43 @@ impl Selection { self.inputs.iter_mut().map(InputMut::new) } + /// Reorder inputs in-place using `compare`. + /// + /// Uses a stable sort: inputs that compare equal retain their relative order. + /// Typical use is BIP-69 lexicographic ordering by previous outpoint. + pub fn sort_inputs_by(&mut self, compare: F) + where + F: FnMut(&Input, &Input) -> Ordering, + { + self.inputs.sort_by(compare); + } + + /// Randomly shuffle inputs in-place using `rng`. + /// + /// Useful for chain-analysis resistance when no deterministic ordering is required. + pub fn shuffle_inputs(&mut self, rng: &mut R) { + fisher_yates_shuffle(&mut self.inputs, rng); + } + + /// Reorder outputs in-place using `compare`. + /// + /// Uses a stable sort: outputs that compare equal retain their relative order. + /// Typical use is BIP-69 (ascending by amount, then by `script_pubkey`). + pub fn sort_outputs_by(&mut self, compare: F) + where + F: FnMut(&Output, &Output) -> Ordering, + { + self.outputs.sort_by(compare); + } + + /// Randomly shuffle outputs in-place using `rng`. + /// + /// Useful for chain-analysis resistance — in particular, hiding which output + /// is the change. + pub fn shuffle_outputs(&mut self, rng: &mut R) { + fisher_yates_shuffle(&mut self.outputs, rng); + } + /// Accumulates the maximum locktime from an iterator of input-required locktimes. /// /// Returns the `min_locktime` if the locktimes iterator is empty, `Ok(lock_time)` with @@ -770,4 +811,13 @@ mod tests { "should return UnsupportedVersion error for version < 2" ); } + + #[test] + fn test_fisher_yates_shuffle_preserves_multiset() { + let original: Vec = (0..32).collect(); + let mut shuffled = original.clone(); + fisher_yates_shuffle(&mut shuffled, &mut OsRng); + shuffled.sort(); + assert_eq!(shuffled, original); + } } From 5a6f34c811935ba7c26e9ad3267eeae59bdbd83c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sun, 17 May 2026 15:32:38 +0000 Subject: [PATCH 4/5] refactor!: move locktime-unit check from create_psbt to selector `SelectorError::LockTypeMismatch` now rejects candidate inputs with mixed absolute-timelock units up front, in `Selector::new`. `CreatePsbtError::LockTypeMismatch` is removed and `accumulate_max_locktime` becomes infallible (with a `debug_assert!` guarding the upstream invariant). Catches the failure at the earliest point and lets PSBT creation assume the invariant. --- src/selection.rs | 67 ++++++++++++++++++++---------------------------- src/selector.rs | 29 ++++++++++++++++++++- 2 files changed, 56 insertions(+), 40 deletions(-) diff --git a/src/selection.rs b/src/selection.rs index 19d3e68..581c3a7 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -96,8 +96,6 @@ impl Default for PsbtParams { /// Occurs when creating a psbt fails. #[derive(Debug)] pub enum CreatePsbtError { - /// Attempted to mix locktime types. - LockTypeMismatch, /// Missing tx for legacy input. MissingFullTxForLegacyInput(Box), /// Missing tx for segwit v0 input. @@ -119,7 +117,6 @@ impl From for CreatePsbtError { impl core::fmt::Display for CreatePsbtError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - CreatePsbtError::LockTypeMismatch => write!(f, "cannot mix locktime units"), CreatePsbtError::MissingFullTxForLegacyInput(input) => write!( f, "legacy input that spends {} requires PSBT_IN_NON_WITNESS_UTXO", @@ -216,45 +213,37 @@ impl Selection { /// Accumulates the maximum locktime from an iterator of input-required locktimes. /// - /// Returns the `min_locktime` if the locktimes iterator is empty, `Ok(lock_time)` with - /// the maximum locktime if all items share the same unit. Errors if there is a mismatch of - /// lock type units among the required locktimes. + /// Returns `min_locktime` if the locktimes iterator is empty, otherwise the maximum locktime + /// across the inputs (with `min_locktime` only applied when compatible with the inputs' unit). + /// + /// # Panics + /// + /// In debug builds, panics if `locktimes` contains values with different units (height vs. + /// time). `Selector::new` rejects such candidates upstream, so this should never fire in + /// practice. fn accumulate_max_locktime( locktimes: impl IntoIterator, min_locktime: absolute::LockTime, - ) -> Result { - // Accumulate locktimes required by inputs. An input-vs-input unit mismatch is an error. - // `min_locktime` is only used when it is compatible with the input requirements. - // If it is a different unit from the required locktime it is intentionally ignored - // so that a height-based `min_locktime` does not conflict with a time-based CLTV - // requirement. - let mut acc = Option::::None; - for locktime in locktimes { - match &mut acc { - Some(acc) => { - if !acc.is_same_unit(locktime) { - return Err(CreatePsbtError::LockTypeMismatch); - } - if acc.is_implied_by(locktime) { - *acc = locktime; - } - } - acc => *acc = Some(locktime), - }; - } - match acc { - // No required locktimes from inputs: use `min_locktime` directly. - None => Ok(min_locktime), - // Same unit as `min_locktime`: take the maximum of required and `min_locktime`. - Some(lock_time) if lock_time.is_same_unit(min_locktime) => { - if lock_time.is_implied_by(min_locktime) { - Ok(min_locktime) - } else { - Ok(lock_time) - } + ) -> absolute::LockTime { + // Accumulate locktimes required by inputs. An input-vs-input unit mismatch is rejected + // upstream by `Selector::new`. `min_locktime` is only used when it is compatible with + // the input requirements; a different unit is intentionally ignored so that, e.g., a + // height-based `min_locktime` does not conflict with a time-based CLTV requirement. + let inputs_max = locktimes.into_iter().reduce(|a, b| { + debug_assert!( + a.is_same_unit(b), + "Selector::new should reject mixed-unit candidates", + ); + if a.is_implied_by(b) { + b + } else { + a } - // `min_locktime` is a different unit: use required locktime and ignore it. - Some(lock_time) => Ok(lock_time), + }); + match inputs_max { + Some(lt) if lt.is_implied_by(min_locktime) => min_locktime, + Some(lt) => lt, + None => min_locktime, } } @@ -277,7 +266,7 @@ impl Selection { .iter() .filter_map(|input| input.absolute_timelock()), params.min_locktime, - )?, + ), input: self .inputs .iter() diff --git a/src/selector.rs b/src/selector.rs index d907cca..c784266 100644 --- a/src/selector.rs +++ b/src/selector.rs @@ -3,7 +3,8 @@ use bitcoin::{Amount, FeeRate, ScriptBuf, Transaction, Weight}; use miniscript::bitcoin; use crate::{ - DefiniteDescriptor, FeeRateExt, InputCandidates, InputGroup, Output, ScriptSource, Selection, + DefiniteDescriptor, FeeRateExt, Input, InputCandidates, InputGroup, Output, ScriptSource, + Selection, }; use alloc::boxed::Box; use alloc::vec::Vec; @@ -355,6 +356,13 @@ pub enum SelectorError { CannotMeetTarget(CannotMeetTarget), /// The provided assets cannot satisfy the change descriptor. InsufficientAssets, + /// Input candidates have absolute timelocks of mixed units (some height-based, others + /// time-based). + /// + /// Such a set is unbuildable since `nLockTime` is a single field on a transaction. + /// Filter the [`InputCandidates`] down to a single-unit subset before constructing the + /// [`Selector`]. + LockTypeMismatch, } impl fmt::Display for SelectorError { @@ -365,6 +373,9 @@ impl fmt::Display for SelectorError { Self::InsufficientAssets => { write!(f, "provided assets cannot satisfy the change descriptor") } + Self::LockTypeMismatch => { + write!(f, "input candidates have absolute timelocks of mixed units") + } } } } @@ -387,9 +398,25 @@ impl<'c> Selector<'c> { 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() { return Err(SelectorError::CannotMeetTarget(CannotMeetTarget)); } + + // Verify that all inputs agree on absolute timelock unit (height vs time). + // Downstream stages (create_psbt, apply_anti_fee_sniping) rely on this invariant. + let mut unit: Option = None; + for lt in candidates.inputs().filter_map(Input::absolute_timelock) { + match unit { + Some(existing_unit) => { + if !existing_unit.is_same_unit(lt) { + return Err(SelectorError::LockTypeMismatch); + } + } + None => unit = Some(lt), + } + } + let mut inner = bdk_coin_select::CoinSelector::new(candidates.coin_select_candidates()); if candidates.must_select().is_some() { inner.select_next(); From 96b9f883fcb0e6636c7dd50ba043e40d4eb2cf1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 19 May 2026 14:34:55 +0000 Subject: [PATCH 5/5] test(selector): cover mixed absolute-locktime-unit rejection Co-authored-by: Noah Joeris --- src/selector.rs | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/selector.rs b/src/selector.rs index c784266..4c98ec8 100644 --- a/src/selector.rs +++ b/src/selector.rs @@ -509,3 +509,55 @@ impl<'c> Selector<'c> { Some(Selection::new(inputs, outputs)) } } + +#[cfg_attr(coverage_nightly, coverage(off))] +#[cfg(test)] +mod tests { + use crate::*; + use bitcoin::{ + absolute, key::Secp256k1, secp256k1::SecretKey, transaction, Amount, FeeRate, PrivateKey, + ScriptBuf, Transaction, TxIn, TxOut, Weight, + }; + use miniscript::{plan::Assets, DescriptorPublicKey}; + use std::string::ToString; + + fn setup_cltv_input(cltv: absolute::LockTime) -> anyhow::Result { + let secp = Secp256k1::new(); + let secret_key = SecretKey::from_slice(&[1_u8; 32])?; + let public_key = PrivateKey::new(secret_key, bitcoin::Network::Regtest).public_key(&secp); + let desc_str = format!("wsh(and_v(v:pk({public_key}),after({cltv})))"); + let desc_pk: DescriptorPublicKey = public_key.to_string().parse()?; + let (desc, _) = Descriptor::parse_descriptor(&secp, &desc_str)?; + let plan = desc + .at_derivation_index(0)? + .plan(&Assets::new().add(desc_pk).after(cltv)) + .expect("locktime asset must satisfy descriptor"); + let prev_tx = Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![TxIn::default()], + output: vec![TxOut { + script_pubkey: desc.at_derivation_index(0)?.script_pubkey(), + value: Amount::ONE_BTC, + }], + }; + Ok(Input::from_prev_tx(plan, prev_tx, 0, None)?) + } + + #[test] + fn test_selector_rejects_mixed_absolute_locktime_units() -> anyhow::Result<()> { + let height_locked_input = setup_cltv_input(absolute::LockTime::from_consensus(10_000))?; + let time_locked_input = setup_cltv_input(absolute::LockTime::from_consensus(500_000_001))?; + let candidates = InputCandidates::new([], [height_locked_input, time_locked_input]); + let params = SelectorParams::new( + FeeRate::ZERO, + vec![], + ChangeScript::from_script(ScriptBuf::new(), Weight::ZERO), + ); + assert!(matches!( + Selector::new(&candidates, params), + Err(SelectorError::LockTypeMismatch) + )); + Ok(()) + } +}