Skip to content

Commit b331ab3

Browse files
committed
Merge #72: refactor(selection)!: encapsulate Selection and fail-fast on mixed-unit locktimes
96b9f88 test(selector): cover mixed absolute-locktime-unit rejection (志宇) 5a6f34c refactor!: move locktime-unit check from create_psbt to selector (志宇) d158cbf feat(selection): add sort/shuffle methods for inputs and outputs (志宇) 4ce801d refactor(selection)!: make fields private, add InputMut handle (志宇) d43e81a feat: Add `fisher_yates_shuffle` method and refactor workspace (志宇) Pull request description: ### Description Detects mixed-unit absolute timelock requirements at `Selector::new` instead of at PSBT creation, and prevents external code from constructing or rearranging a `Selection` in ways that would bypass the check. Closes #58 ### Changes 1. **Fail-fast in the selector.** `Selector::new` now returns `SelectorError::LockTypeMismatch` when candidate inputs mix height-based and time-based absolute timelocks. The corresponding `CreatePsbtError::LockTypeMismatch` is removed and `Selection::create_psbt`'s internal locktime accumulator becomes infallible (with a `debug_assert!` guarding the upstream invariant). 2. **Encapsulate `Selection`.** `Selection.inputs` / `Selection.outputs` are now private. Access is via `inputs()` / `outputs()`. Per-input mutation goes through a new `InputMut<'_>` handle (returned by `input_mut(outpoint)` / `inputs_mut()`) which derefs to `&Input` for reads and exposes only `set_sequence`, preventing whole-input replacement that would silently break coin-selection invariants. 3. **Reorder methods.** Adds `sort_inputs_by`, `shuffle_inputs`, `sort_outputs_by`, `shuffle_outputs` on `Selection` so callers can apply BIP-69 or chain-analysis-resistant ordering without raw mutable slice access. ### Scope creep Bundled in this PR (let me know if you'd prefer these split out): - **Workspace refactor.** `src/utils.rs` is split into `src/afs.rs` (AFS types) and `src/no_std_rand.rs` (no-std rand helpers). `no_std_rand` is now crate-private since it has no external consumers. - **`fisher_yates_shuffle` helper.** Added to `no_std_rand` and used to back `Selection::shuffle_inputs` / `shuffle_outputs`. ### Changelog Notice ```md Added: - `SelectorError::LockTypeMismatch` — raised by `Selector::new` when candidate inputs mix height-based and time-based absolute timelocks. - `Selection::inputs` / `Selection::outputs` accessors (replacing the public fields). - `Selection::input_mut` / `Selection::inputs_mut` returning a new `InputMut<'_>` handle that only permits `set_sequence`. - `Selection::sort_inputs_by`, `Selection::shuffle_inputs`, `Selection::sort_outputs_by`, `Selection::shuffle_outputs` for reordering without exposing raw mutable access. Removed: - `CreatePsbtError::LockTypeMismatch` — superseded by `SelectorError::LockTypeMismatch`. Changed: - `Selection.inputs` and `Selection.outputs` are no longer public fields. External construction of `Selection` is no longer possible; obtain one from `Selector::try_finalize`. ``` ### Checklist - [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) - [x] I ran `cargo fmt` and `cargo clippy` before committing - [x] I've added tests for the new feature - [x] I've added docs for the new feature - [x] I'm linking the issue being fixed by this PR ACKs for top commit: nymius: ACK 96b9f88 noahjoeris: ACK 96b9f88 Tree-SHA512: 013e0da33f2960841c74d89faa05aa4819507bdebbd279549ce09daf380abacbb6f5ec5437a9aa1893e42c30e7832e41097e160c04676b2afedfff43e513c268
2 parents 8b51d07 + 96b9f88 commit b331ab3

9 files changed

Lines changed: 341 additions & 144 deletions

File tree

examples/anti_fee_sniping.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ fn main() -> anyhow::Result<()> {
8888
},
8989
)?;
9090

91-
let selection_inputs = selection.inputs.clone();
91+
let selection_inputs = selection.inputs().to_vec();
9292

9393
let psbt = selection.create_psbt(PsbtParams {
9494
anti_fee_sniping: Some(tip_height),

examples/synopsis.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ fn main() -> anyhow::Result<()> {
150150
println!(
151151
"selected inputs: {:?}",
152152
selection
153-
.inputs
153+
.inputs()
154154
.iter()
155155
.map(|input| input.prev_outpoint())
156156
.collect::<Vec<_>>()

src/utils.rs renamed to src/afs.rs

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use crate::Input;
1+
use crate::{
2+
no_std_rand::{random_probability, random_range},
3+
Input,
4+
};
25
use alloc::vec::Vec;
36
use miniscript::bitcoin::{
47
absolute::{self, LockTime},
@@ -157,20 +160,3 @@ pub(crate) fn apply_anti_fee_sniping(
157160

158161
Ok(())
159162
}
160-
161-
/// Returns true with probability 1/n.
162-
fn random_probability(rng: &mut impl RngCore, n: u32) -> bool {
163-
random_range(rng, n) == 0
164-
}
165-
166-
/// Returns a random value in the range [0, n) using unbiased rejection sampling.
167-
fn random_range(rng: &mut impl RngCore, n: u32) -> u32 {
168-
let threshold = n.wrapping_neg() % n;
169-
170-
loop {
171-
let value = rng.next_u32();
172-
if value >= threshold {
173-
return value % n;
174-
}
175-
}
176-
}

src/finalizer.rs

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use miniscript::{bitcoin, plan::Plan, psbt::PsbtInputSatisfier};
2727
/// # use bdk_tx::PsbtParams;
2828
/// # let secp = bitcoin::secp256k1::Secp256k1::new();
2929
/// # let keymap = std::collections::BTreeMap::new();
30-
/// # let selection = bdk_tx::Selection { inputs: vec![], outputs: vec![] };
30+
/// # let selection: bdk_tx::Selection = unimplemented!();
3131
/// // Create PSBT from a selection of inputs and outputs.
3232
/// let mut psbt = selection.create_psbt(PsbtParams::default())?;
3333
///
@@ -217,10 +217,7 @@ mod tests {
217217
fn test_finalize_single_input() -> anyhow::Result<()> {
218218
let (input, keymap) = create_input_from_descriptor_at(TR_XPRV, 0)?;
219219
let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000));
220-
let selection = Selection {
221-
inputs: vec![input],
222-
outputs: vec![output],
223-
};
220+
let selection = Selection::new(vec![input], vec![output]);
224221

225222
let mut psbt = selection.create_psbt(PsbtParams::default())?;
226223
let finalizer = selection.into_finalizer();
@@ -240,10 +237,7 @@ mod tests {
240237
fn test_finalize_sets_final_script_sig() -> anyhow::Result<()> {
241238
let (input, keymap) = create_input_from_descriptor_at(PKH_XPRV, 0)?;
242239
let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000));
243-
let selection = Selection {
244-
inputs: vec![input],
245-
outputs: vec![output],
246-
};
240+
let selection = Selection::new(vec![input], vec![output]);
247241

248242
let mut psbt = selection.create_psbt(PsbtParams::default())?;
249243
let finalizer = selection.into_finalizer();
@@ -266,13 +260,13 @@ mod tests {
266260
let taproot_output_descriptor = derive_descriptor_at(TR_XPRV, 10)?;
267261
let wpkh_output_descriptor = derive_descriptor_at(WPKH_XPRV, 11)?;
268262

269-
let selection = Selection {
270-
inputs: vec![input_0, input_1, input_2],
271-
outputs: vec![
263+
let selection = Selection::new(
264+
vec![input_0, input_1, input_2],
265+
vec![
272266
Output::with_descriptor(taproot_output_descriptor, Amount::from_sat(20_000)),
273267
Output::with_descriptor(wpkh_output_descriptor, Amount::from_sat(22_000)),
274268
],
275-
};
269+
);
276270

277271
let mut psbt = selection.create_psbt(PsbtParams::default())?;
278272
let finalizer = selection.into_finalizer();
@@ -321,13 +315,13 @@ mod tests {
321315
input_0.plan().cloned().expect("plan must exist"),
322316
)]);
323317

324-
let selection = Selection {
325-
inputs: vec![input_0, input_1],
326-
outputs: vec![
318+
let selection = Selection::new(
319+
vec![input_0, input_1],
320+
vec![
327321
Output::with_descriptor(taproot_output_descriptor, Amount::from_sat(20_000)),
328322
Output::with_descriptor(wpkh_output_descriptor, Amount::from_sat(22_000)),
329323
],
330-
};
324+
);
331325

332326
let mut psbt = selection.create_psbt(PsbtParams::default())?;
333327

@@ -361,13 +355,13 @@ mod tests {
361355
let (input, _) = create_input_from_descriptor_at(TR_XPRV, 0)?;
362356
let taproot_output_descriptor = derive_descriptor_at(TR_XPRV, 10)?;
363357
let wpkh_output_descriptor = derive_descriptor_at(WPKH_XPRV, 11)?;
364-
let selection = Selection {
365-
inputs: vec![input],
366-
outputs: vec![
358+
let selection = Selection::new(
359+
vec![input],
360+
vec![
367361
Output::with_descriptor(taproot_output_descriptor, Amount::from_sat(20_000)),
368362
Output::with_descriptor(wpkh_output_descriptor, Amount::from_sat(22_000)),
369363
],
370-
};
364+
);
371365

372366
let mut psbt = selection.create_psbt(PsbtParams::default())?;
373367
let finalizer = selection.into_finalizer();
@@ -395,10 +389,7 @@ mod tests {
395389
fn test_already_finalized_input() -> anyhow::Result<()> {
396390
let (input, keymap) = create_input_from_descriptor_at(TR_XPRV, 0)?;
397391
let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000));
398-
let selection = Selection {
399-
inputs: vec![input],
400-
outputs: vec![output],
401-
};
392+
let selection = Selection::new(vec![input], vec![output]);
402393

403394
let mut psbt = selection.create_psbt(PsbtParams::default())?;
404395
let finalizer = selection.into_finalizer();

src/input.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,39 @@ impl Input {
665665
}
666666
}
667667

668+
/// Mutable handle to an [`Input`] held inside a [`Selection`].
669+
///
670+
/// Returned by [`Selection::input_mut`] and [`Selection::inputs_mut`]. This wrapper restricts
671+
/// mutation to operations that preserve [`Selection`]'s coin-selection invariants.
672+
///
673+
/// Read-only access to the underlying [`Input`] is available via [`Deref`].
674+
///
675+
/// [`Selection`]: crate::Selection
676+
/// [`Selection::input_mut`]: crate::Selection::input_mut
677+
/// [`Selection::inputs_mut`]: crate::Selection::inputs_mut
678+
/// [`Deref`]: core::ops::Deref
679+
#[derive(Debug)]
680+
pub struct InputMut<'a>(&'a mut Input);
681+
682+
impl<'a> InputMut<'a> {
683+
pub(crate) fn new(input: &'a mut Input) -> Self {
684+
Self(input)
685+
}
686+
687+
/// See [`Input::set_sequence`].
688+
pub fn set_sequence(&mut self, sequence: Sequence) -> Result<(), SetSequenceError> {
689+
self.0.set_sequence(sequence)
690+
}
691+
}
692+
693+
impl core::ops::Deref for InputMut<'_> {
694+
type Target = Input;
695+
696+
fn deref(&self) -> &Input {
697+
self.0
698+
}
699+
}
700+
668701
/// Input group. Cannot be empty.
669702
#[derive(Debug, Clone)]
670703
pub struct InputGroup(Vec<Input>);

src/lib.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,32 @@ extern crate alloc;
99
#[cfg(feature = "std")]
1010
extern crate std;
1111

12+
mod afs;
1213
mod canonical_unspents;
1314
mod finalizer;
1415
mod input;
1516
mod input_candidates;
17+
mod no_std_rand;
1618
mod output;
1719
mod rbf;
1820
mod selection;
1921
mod selector;
2022
mod signer;
21-
mod utils;
2223

24+
pub use afs::*;
2325
pub use canonical_unspents::*;
2426
pub use finalizer::*;
2527
pub use input::*;
2628
pub use input_candidates::*;
2729
pub use miniscript;
2830
pub use miniscript::bitcoin;
2931
use miniscript::{DefiniteDescriptorKey, Descriptor};
32+
use no_std_rand::*;
3033
pub use output::*;
3134
pub use rbf::*;
3235
pub use selection::*;
3336
pub use selector::*;
3437
pub use signer::*;
35-
pub use utils::*;
3638

3739
#[cfg(feature = "std")]
3840
pub(crate) mod collections {

src/no_std_rand.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
use rand_core::RngCore;
2+
3+
/// Returns true with probability 1/n.
4+
pub(crate) fn random_probability(rng: &mut impl RngCore, n: u32) -> bool {
5+
random_range(rng, n) == 0
6+
}
7+
8+
/// Returns a random value in the range [0, n) using unbiased rejection sampling.
9+
pub(crate) fn random_range(rng: &mut impl RngCore, n: u32) -> u32 {
10+
let threshold = n.wrapping_neg() % n;
11+
12+
loop {
13+
let value = rng.next_u32();
14+
if value >= threshold {
15+
return value % n;
16+
}
17+
}
18+
}
19+
20+
/// Fisher-Yates in-place shuffle using unbiased rejection sampling.
21+
pub(crate) fn fisher_yates_shuffle<T>(slice: &mut [T], rng: &mut impl RngCore) {
22+
for i in (1..slice.len()).rev() {
23+
// Unbiased index in [0, i+1) via rejection sampling.
24+
let j = random_range(rng, (i + 1) as u32) as usize;
25+
slice.swap(i, j);
26+
}
27+
}
28+
29+
#[cfg_attr(coverage_nightly, coverage(off))]
30+
#[cfg(test)]
31+
mod tests {
32+
use super::*;
33+
use alloc::vec::Vec;
34+
use rand_core::OsRng;
35+
36+
#[test]
37+
fn test_fisher_yates_shuffle_preserves_multiset() {
38+
let original: Vec<u32> = (0..32).collect();
39+
let mut shuffled = original.clone();
40+
fisher_yates_shuffle(&mut shuffled, &mut OsRng);
41+
shuffled.sort();
42+
assert_eq!(shuffled, original);
43+
}
44+
}

0 commit comments

Comments
 (0)