From b923b4e41aee9826ba7df42939ee9a13bfdf3475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 12 Aug 2026 06:54:37 +0000 Subject: [PATCH 1/2] fix!: replace Candidate input_count/is_segwit with segwit_count/legacy_count Fixes CoinSelector::input_weight undercounting candidates that group multiple legacy inputs in a segwit transaction (where each legacy input serializes a 1 WU empty witness). Tracking segwit and legacy input counts separately also allows a single Candidate to mix legacy and segwit inputs. --- CHANGELOG.md | 1 + README.md | 23 +++---- benches/coin_selector.rs | 4 +- src/coin_selector.rs | 46 +++++++++---- tests/bnb.rs | 19 +++--- tests/changeless.rs | 4 +- tests/common.rs | 15 +++-- tests/lowest_fee.rs | 29 ++++---- tests/srd.rs | 16 ++--- tests/weight.rs | 140 ++++++++++++++++++++++++++++++++++++--- 10 files changed, 222 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e5daf0..0696755 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- **Breaking:** Replace `Candidate`'s `input_count` and `is_segwit` fields with `segwit_count` and `legacy_count`, fixing `CoinSelector::input_weight` undercounting candidates that group multiple inputs: in a segwit transaction every legacy input still serializes an empty witness (1 WU), which was previously paid once per candidate instead of once per legacy input, so a group of N legacy inputs came out N-1 WU short. Splitting the count by script type also means a single candidate may now mix legacy and segwit inputs and still be priced exactly. - **Breaking:** `BnbMetric`'s `score`, `bound`, and `drain` take the `target: Target` as a parameter, and `CoinSelector::run_bnb`/`bnb_solutions` gain a leading `target` argument. Consequently `LowestFee` and `Changeless` no longer store a `target` field. This removes the target that `Changeless` previously had to keep in sync with its inner metric, and aligns the metric API with the rest of `CoinSelector`, where `target` is always passed in. - **Breaking:** `BnbMetric` metrics now decide the change output themselves. The trait gains a `drain(&mut self, cs) -> Drain` method; call it on a branch-and-bound solution (or the `LowestFee` metric directly) to get the change output the metric optimized against, instead of computing a separate `ChangePolicy`. - **Breaking:** `CoinSelector::run_bnb` now returns `(Ordf32, Drain)` instead of just `Ordf32`, handing back the change output the metric decided on for the winning selection. diff --git a/README.md b/README.md index 4335d4b..a03ab9b 100644 --- a/README.md +++ b/README.md @@ -33,23 +33,22 @@ let candidates = vec![ Candidate { // How many inputs does this candidate represents. Needed so we can // figure out the weight of the varint that encodes the number of inputs - input_count: 1, + // and whether segwit transaction fields need to be counted in. + segwit_count: 1, + legacy_count: 0, // the value of the input value: 1_000_000, // the total weight of the input(s) including their witness/scriptSig // you may need to use miniscript to figure out the correct value here. weight: TR_KEYSPEND_TXIN_WEIGHT, - // wether it's a segwit input. Needed so we know whether to include the - // segwit header in total weight calculations. - is_segwit: true }, Candidate { // A candidate can represent multiple inputs in the case where you // always want some inputs to be spent together. - input_count: 2, + segwit_count: 2, + legacy_count: 0, weight: 2*TR_KEYSPEND_TXIN_WEIGHT, value: 3_000_000, - is_segwit: true } ]; @@ -105,22 +104,22 @@ let outputs = vec![TxOut { let candidates = [ Candidate { - input_count: 1, + segwit_count: 1, + legacy_count: 0, value: 400_000, weight: TR_KEYSPEND_TXIN_WEIGHT, - is_segwit: true }, Candidate { - input_count: 1, + segwit_count: 1, + legacy_count: 0, value: 200_000, weight: TR_KEYSPEND_TXIN_WEIGHT, - is_segwit: true }, Candidate { - input_count: 1, + segwit_count: 1, + legacy_count: 0, value: 11_000, weight: TR_KEYSPEND_TXIN_WEIGHT, - is_segwit: true } ]; let drain_weights = bdk_coin_select::DrainWeights::default(); diff --git a/benches/coin_selector.rs b/benches/coin_selector.rs index c05eabb..89d8d93 100644 --- a/benches/coin_selector.rs +++ b/benches/coin_selector.rs @@ -33,8 +33,8 @@ fn make_candidates(n: usize) -> Vec { Candidate { value, weight: TXIN_BASE_WEIGHT + P2WPKH_SAT_W, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, } }) .collect() diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 604abd8..977a8e8 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -142,20 +142,23 @@ impl<'a> CoinSelector<'a> { /// The weight of the inputs including the witness header and the varint for the number of /// inputs. pub fn input_weight(&self) -> u64 { - let is_segwit_tx = self.selected().any(|(_, wv)| wv.is_segwit); + let is_segwit_tx = self.selected().any(|(_, wv)| wv.segwit_count > 0); let witness_header_extra_weight = is_segwit_tx as u64 * 2; - let input_count = self.selected().map(|(_, wv)| wv.input_count).sum::(); + let input_count = self + .selected() + .map(|(_, wv)| wv.segwit_count + wv.legacy_count) + .sum::(); let input_varint_weight = varint_size(input_count) * 4; let selected_weight: u64 = self .selected() .map(|(_, candidate)| { let mut weight = candidate.weight; - if is_segwit_tx && !candidate.is_segwit { - // non-segwit candidates do not have the witness length field included in their - // weight field so we need to add 1 here if it's in a segwit tx. - weight += 1; + if is_segwit_tx { + // Legacy inputs do not have the witness length included in their weight field + // so we need to add 1 to each if it's a segwit tx. + weight += candidate.legacy_count as u64; } weight }) @@ -889,7 +892,11 @@ impl std::error::Error for NoBnbSolution {} /// A `Candidate` represents an input candidate for [`CoinSelector`]. /// -/// This can either be a single UTXO, or a group of UTXOs that should be spent together. +/// This can either be a single UTXO, or a group of UTXOs that should be spent together. A group +/// may mix legacy and segwit inputs; set [`legacy_count`] and [`segwit_count`] accordingly. +/// +/// [`legacy_count`]: Candidate::legacy_count +/// [`segwit_count`]: Candidate::segwit_count #[derive(Debug, Clone, Copy)] pub struct Candidate { /// Total value of the UTXO(s) that this [`Candidate`] represents. @@ -897,11 +904,24 @@ pub struct Candidate { /// Total weight of including this/these UTXO(s). /// `txin` fields: `prevout`, `nSequence`, `scriptSigLen`, `scriptSig`, `scriptWitnessLen`, /// `scriptWitness` should all be included. + /// + /// For legacy inputs, do *not* include the `scriptWitnessLen` byte: a legacy input only + /// serializes an (empty) witness when the transaction has a witness section, and + /// [`CoinSelector::input_weight`] adds that 1 WU per legacy input once any segwit input is + /// selected. pub weight: u64, - /// Total number of inputs; so we can calculate extra `varint` weight due to `vin` len changes. - pub input_count: usize, - /// Whether this [`Candidate`] contains at least one segwit spend. - pub is_segwit: bool, + /// Total number of segwit inputs. + /// + /// If any selected candidate has a non-zero `segwit_count`, the transaction serializes a + /// witness section (marker + flag, 2 WU) and every input — including legacy ones — pays for + /// a witness. + pub segwit_count: usize, + /// Total number of legacy (non-segwit) inputs. + /// + /// Each legacy input serializes an empty witness (1 WU) when the transaction has a witness + /// section; [`CoinSelector::input_weight`] prices this per legacy input, so grouped legacy + /// inputs are counted exactly. + pub legacy_count: usize, } impl Candidate { @@ -920,8 +940,8 @@ impl Candidate { Candidate { value, weight, - input_count: 1, - is_segwit, + segwit_count: is_segwit as usize, + legacy_count: !is_segwit as usize, } } diff --git a/tests/bnb.rs b/tests/bnb.rs index 45a22dc..3fea68a 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -11,16 +11,16 @@ use proptest::{prelude::*, proptest, test_runner::*}; fn test_wv(mut rng: impl RngCore) -> impl Iterator { core::iter::repeat_with(move || { let value = rng.random_range(0..1_000); - let mut candidate = Candidate { + let candidate = Candidate { value, weight: 100, - input_count: rng.random_range(1..2), - is_segwit: rng.random_bool(0.5), + segwit_count: rng.random_range(1..2), + legacy_count: 0, }; - // HACK: set is_segwit = true for all these tests because you can't actually lower bound - // things easily with how segwit inputs interfere with their weights. We can't modify the - // above since that would change what we pull from rng. - candidate.is_segwit = true; + // Keep drawing the bool these tests always drew so the rng stream (and therefore the + // generated cases) is unchanged. All candidates are segwit: mixing in legacy inputs makes + // their weights context-dependent, which these tests can't lower-bound easily. + let _ = rng.random_bool(0.5); candidate }) } @@ -62,10 +62,7 @@ fn bnb_finds_an_exact_solution_in_n_iter() { let num_additional_canidates = 12; let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let mut wv = test_wv(&mut rng).map(|mut candidate| { - candidate.is_segwit = true; - candidate - }); + let mut wv = test_wv(&mut rng); let solution: Vec = (0..solution_len).map(|_| wv.next().unwrap()).collect(); let solution_weight = { diff --git a/tests/changeless.rs b/tests/changeless.rs index aac10a3..2d43cb7 100644 --- a/tests/changeless.rs +++ b/tests/changeless.rs @@ -14,8 +14,8 @@ fn test_wv(mut rng: impl RngCore) -> impl Iterator { Candidate { value, weight: rng.random_range(0..100), - input_count: rng.random_range(1..2), - is_segwit: false, + segwit_count: rng.random_range(1..2), + legacy_count: 0, } }) } diff --git a/tests/common.rs b/tests/common.rs index 273b830..e508b04 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -269,14 +269,21 @@ pub fn gen_candidates(n: usize) -> Vec { core::iter::repeat_with(move || { let value = rng.random_range(1..500_001); let weight = rng.random_range(1..2001); - let input_count = rng.random_range(1..3); - let is_segwit = rng.random_bool(0.01); + + let (mut legacy_count, mut segwit_count); + loop { + legacy_count = rng.random_range(0..3); + segwit_count = if rng.random_bool(0.01) { 1 } else { 0 }; + if legacy_count > 0 || segwit_count > 0 { + break; + } + } Candidate { value, weight, - input_count, - is_segwit, + segwit_count, + legacy_count, } }) .take(n) diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index d6b8cba..f428d76 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -1,5 +1,4 @@ #![allow(unused_imports)] - mod common; use bdk_coin_select::metrics::{Changeless, LowestFee}; use bdk_coin_select::{ @@ -85,8 +84,8 @@ proptest! { Candidate { value: 20_000, weight: (32 + 4 + 4 + 1) * 4 + 64 + 32, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }; params.n_candidates ]; @@ -238,21 +237,21 @@ fn does_not_create_change_below_spend_cost() { Candidate { value: 100_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, Candidate { value: 50_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, // NOTE: this input has negative effective value Candidate { value: 10, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, ]; @@ -321,14 +320,14 @@ fn zero_fee_tx() { Candidate { value: 100_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, Candidate { value: 50_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, ]; @@ -354,8 +353,8 @@ fn err_candidate(value: u64) -> Candidate { Candidate { value, weight: 272, // ~1 P2WPKH input - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, } } diff --git a/tests/srd.rs b/tests/srd.rs index b1b3096..e8348f1 100644 --- a/tests/srd.rs +++ b/tests/srd.rs @@ -75,20 +75,20 @@ fn srd_insufficient_funds() { Candidate { value: 50_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, Candidate { value: 50_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, Candidate { value: 50_000, weight: 100, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }, ]; let target = target(200_000, 5.0); @@ -117,8 +117,8 @@ fn srd_max_weight_exceeded() { Candidate { value: 100_000, weight: 1000, - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }; 10 ]; diff --git a/tests/weight.rs b/tests/weight.rs index 6a8dbb5..5ad619c 100644 --- a/tests/weight.rs +++ b/tests/weight.rs @@ -21,6 +21,26 @@ pub fn hex_decode(hex: &str) -> Vec { bytes } +// https://mempool.space/tx/5f231df4f73694b3cca9211e336451c20dab136e0a843c2e3166cdcb093e91f4 +const THREE_INPUT_LEGACY_TX_HEX: &str = "0100000003fe785783e14669f638ba902c26e8e3d7036fb183237bc00f8a10542191c7171300000000fdfd00004730440220418996f20477d143d02ad47e74e5949641b6c2904159ab7c592d2cfc659f9bd802205b18f18ac86b714971f84a8b74a4cb14ad5c1a5b9d0d939bb32c6ae4032f4ea10148304502210091296ff8dd87b5ebfc3d47cb82cfe4750d52c544a2b88a85970354a4d0d4b1db022069632067ee6f30f06145f649bc76d5e5d5e6404dbe985e006fcde938f778c297014c695221030502b8ade694d57a6e86998180a64f4ce993372830dc796c3d561ad8b2a504de210272b68e1c037c4630eff7ea5858640cc0748e36f5de82fb38529ef1fd0a89670d2103ba0544a3a2aa9f2314022760b78b5c833aebf6f88468a089550f93834a2886ed53aeffffffff7e048a7c53a8af656e24442c65fe4c4299b1494f6c7579fe0fd9fa741ce83e3279000000fc004730440220018fa343acccd048ed8f8f179e1b6ae27435a41b5fb2c1d96a5a772777acc6dc022074783814f2100c6fc4d4c976f941212be50825814502ca0cbe3f929db789979e0147304402206373f01b73fb09876d0f5ee3087e0614cab3be249934bc2b7eb64ee67f53dc8302200b50f8a327020172b82aaba7480c77ecf07bb32322a05f4afbc543aa97d2fde8014c69522103039d906b2494e310f6c7774c98618be552720d04781e073dd3ff25d5906f22662103d82026baa529619b103ec6341d548a7eb6d924061a8469a7416155513a3071c12102e452bc4aa726d44646ba80db70465683b30efde282a19aa35c6029ae8925df5e53aeffffffffef80f0b1cc543de4f73d59c02a3c575ae5d0af17c1e11e6be7abe3325c777507ad000000fdfd00004730440220220fee11bf836621a11a8ea9100a4600c109c13895f11468d3e2062210c5481902201c5c8a462175538e87b8248e1ed3927c3a461c66d1b46215641c875e86eb22c4014830450221008d2de8c2f20a720129c372791e595b9602b1a9bce99618497aec5266148ffc1302203a493359d700ed96323f8805ed03e909959ff0f22eff359028db6861486b1555014c6952210374a4add33567f09967592c5bcdc3db421fdbba67bac4636328f96d941da31bd221039636c2ffac90afb7499b16e265078113dfb2d77b54270e37353217c9eaeaf3052103d0bcea6d10cdd2f16018ea71572631708e26f457f67cda36a7f816a87f7791d253aeffffffff04977261000000000016001470385d054721987f41521648d7b2f5c77f735d6bee92030000000000225120d0cda1b675a0b369964cbfa381721aae3549dd2c9c6f2cf71ff67d5bc277afd3f2aaf30000000000160014ed2d41ba08313dbb2630a7106b2fedafc14aa121d4f0c70000000000220020e5c7c00d174631d2d1e365d6347b016fb87b6a0c08902d8e443989cb771fa7ec00000000"; + +/// The 3-legacy-input mainnet tx above. +fn legacy_three_input_tx() -> Transaction { + Transaction::consensus_decode(&mut hex_decode(THREE_INPUT_LEGACY_TX_HEX).as_slice()).unwrap() +} + +/// The same tx with the middle input turned into a (semi-realistic) P2WPKH segwit spend. +fn legacy_three_input_tx_mixed() -> Transaction { + let mut tx = legacy_three_input_tx(); + tx.input[1].script_sig = ScriptBuf::default(); + tx.input[1].witness = vec![ + // semi-realistic p2wpkh spend + hex_decode("3045022100bdc115b86e9c863279132b4808459cf9b266c8f6a9c14a3dfd956986b807e3320220265833b85197679687c5d5eed1b2637489b34249d44cf5d2d40bc7b514181a5101"), + hex_decode("02077741a668889ce15d59365886375aea47a7691941d7a0d301697edbc773b45b"), + ].into(); + tx +} + #[test] fn segwit_one_input_one_output() { // FROM https://mempool.space/tx/e627fbb7f775a57fd398bf9b150655d4ac3e1f8afed4255e74ee10d7a345a9cc @@ -35,8 +55,8 @@ fn segwit_one_input_one_output() { .map(|(txin, value)| Candidate { value, weight: txin.segwit_weight().to_wu(), - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }) .collect::>(); @@ -78,8 +98,8 @@ fn segwit_two_inputs_one_output() { .map(|(txin, value)| Candidate { value, weight: txin.segwit_weight().to_wu(), - input_count: 1, - is_segwit: true, + segwit_count: 1, + legacy_count: 0, }) .collect::>(); @@ -122,8 +142,8 @@ fn legacy_three_inputs() { .map(|(txin, value)| Candidate { value, weight: txin.legacy_weight().to_wu(), - input_count: 1, - is_segwit: false, + segwit_count: 0, + legacy_count: 1, }) .collect::>(); @@ -179,8 +199,8 @@ fn legacy_three_inputs_one_segwit() { txin.legacy_weight() } .to_wu(), - input_count: 1, - is_segwit, + segwit_count: is_segwit as usize, + legacy_count: !is_segwit as usize, } }) .collect::>(); @@ -200,6 +220,110 @@ fn legacy_three_inputs_one_segwit() { ); } +#[test] +fn legacy_three_inputs_grouped() { + // Same tx as `legacy_three_inputs`, but all three legacy inputs carried by a single candidate. + // No witness section is serialized, so nothing is added per legacy input — this guards the + // all-legacy path (it also passed under the old per-candidate accounting). + let tx = legacy_three_input_tx(); + let input_values = [022_680_000, 006_558_175, 006_558_200]; + + let candidates = [Candidate { + value: input_values.iter().sum(), + weight: tx + .input + .iter() + .map(|txin| txin.legacy_weight().to_wu()) + .sum(), + segwit_count: 0, + legacy_count: tx.input.len(), + }]; + + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value.to_sat()).sum(), + weight_sum: tx.output.iter().map(|output| output.weight().to_wu()).sum(), + n_outputs: tx.output.len(), + }; + + let mut coin_selector = CoinSelector::new(&candidates); + coin_selector.select_all(); + + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + tx.weight().to_wu() + ); +} + +#[test] +fn legacy_pair_grouped_with_segwit_input() { + // Same tx as `legacy_three_inputs_one_segwit`, but the two legacy inputs are grouped into a + // single candidate. In a segwit tx each legacy input still serializes an (empty) witness + // costing 1 WU, so the grouped candidate must pay 2 WU — not 1 — for its two empty witnesses. + let tx = legacy_three_input_tx_mixed(); + let input_values = [022_680_000, 006_558_175, 006_558_200]; + + let candidates = [ + Candidate { + value: input_values[0] + input_values[2], + weight: tx.input[0].legacy_weight().to_wu() + tx.input[2].legacy_weight().to_wu(), + segwit_count: 0, + legacy_count: 2, + }, + Candidate { + value: input_values[1], + weight: tx.input[1].segwit_weight().to_wu(), + segwit_count: 1, + legacy_count: 0, + }, + ]; + + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value.to_sat()).sum(), + weight_sum: tx.output.iter().map(|output| output.weight().to_wu()).sum(), + n_outputs: tx.output.len(), + }; + + let mut coin_selector = CoinSelector::new(&candidates); + coin_selector.select_all(); + + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + tx.weight().to_wu() + ); +} + +#[test] +fn mixed_group_all_inputs_one_candidate() { + // Same tx as `legacy_three_inputs_one_segwit`, with all three inputs — legacy *and* segwit — + // in a single mixed candidate. `legacy_count`/`segwit_count` price this exactly: 2 WU for the + // two empty legacy witnesses, the segwit header once, and a 3-input varint. + let tx = legacy_three_input_tx_mixed(); + let input_values = [022_680_000, 006_558_175, 006_558_200]; + + let candidates = [Candidate { + value: input_values.iter().sum(), + weight: tx.input[0].legacy_weight().to_wu() + + tx.input[1].segwit_weight().to_wu() + + tx.input[2].legacy_weight().to_wu(), + segwit_count: 1, + legacy_count: 2, + }]; + + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value.to_sat()).sum(), + weight_sum: tx.output.iter().map(|output| output.weight().to_wu()).sum(), + n_outputs: tx.output.len(), + }; + + let mut coin_selector = CoinSelector::new(&candidates); + coin_selector.select_all(); + + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + tx.weight().to_wu() + ); +} + #[test] fn new_tr_keyspend_correct_weight() { // FROM https://mempool.space/tx/4936a1a4ea1a0085b9dc2a1d5b59d361f5b1b41241772f3e465153712b6d8dc0 From 50597e4264173447aa3ae4763fa9e5fe6944ac75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 12 Aug 2026 07:24:24 +0000 Subject: [PATCH 2/2] refactor!: replace Candidate::new with Candidate::new_segwit and new_legacy Replaces the boolean is_segwit parameter in Candidate::new with explicit new_segwit and new_legacy constructors. Clarifies in doc comments that satisfaction_weight is the additional weight required beyond TXIN_BASE_WEIGHT (which already accounts for a 1-byte scriptSigLen). --- CHANGELOG.md | 2 +- src/coin_selector.rs | 36 +++++++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0696755..80a38b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Unreleased -- **Breaking:** Replace `Candidate`'s `input_count` and `is_segwit` fields with `segwit_count` and `legacy_count`, fixing `CoinSelector::input_weight` undercounting candidates that group multiple inputs: in a segwit transaction every legacy input still serializes an empty witness (1 WU), which was previously paid once per candidate instead of once per legacy input, so a group of N legacy inputs came out N-1 WU short. Splitting the count by script type also means a single candidate may now mix legacy and segwit inputs and still be priced exactly. +- **Breaking:** Replace `Candidate`'s `input_count` and `is_segwit` fields with `segwit_count` and `legacy_count`, fixing `CoinSelector::input_weight` undercounting candidates that group multiple inputs: in a segwit transaction every legacy input still serializes an empty witness (1 WU), which was previously paid once per candidate instead of once per legacy input, so a group of N legacy inputs came out N-1 WU short. Splitting the count by script type also means a single candidate may now mix legacy and segwit inputs and still be priced exactly. Replaces `Candidate::new` with `Candidate::new_segwit` and `Candidate::new_legacy`. - **Breaking:** `BnbMetric`'s `score`, `bound`, and `drain` take the `target: Target` as a parameter, and `CoinSelector::run_bnb`/`bnb_solutions` gain a leading `target` argument. Consequently `LowestFee` and `Changeless` no longer store a `target` field. This removes the target that `Changeless` previously had to keep in sync with its inner metric, and aligns the metric API with the rest of `CoinSelector`, where `target` is always passed in. - **Breaking:** `BnbMetric` metrics now decide the change output themselves. The trait gains a `drain(&mut self, cs) -> Drain` method; call it on a branch-and-bound solution (or the `LowestFee` metric directly) to get the change output the metric optimized against, instead of computing a separate `ChangePolicy`. - **Breaking:** `CoinSelector::run_bnb` now returns `(Ordf32, Drain)` instead of just `Ordf32`, handing back the change output the metric decided on for the winning selection. diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 977a8e8..2b8f542 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -928,20 +928,42 @@ impl Candidate { /// Create a [`Candidate`] input that spends a single taproot keyspend output. pub fn new_tr_keyspend(value: u64) -> Self { let weight = TR_KEYSPEND_SATISFACTION_WEIGHT; - Self::new(value, weight, true) + Self::new_segwit(value, weight) } - /// Create a new [`Candidate`] that represents a single input. + /// Create a new [`Candidate`] that represents a single segwit input. /// - /// `satisfaction_weight` is the weight of `scriptSigLen + scriptSig + scriptWitnessLen + - /// scriptWitness`. - pub fn new(value: u64, satisfaction_weight: u64, is_segwit: bool) -> Candidate { + /// `satisfaction_weight` is the additional weight (in weight units) required to satisfy the input + /// beyond [`TXIN_BASE_WEIGHT`] (e.g. `scriptWitnessLen + scriptWitness` in WU at 1 WU/byte, plus + /// any `scriptSig` data and extra `scriptSigLen` varint bytes if nested/wrapped segwit). + /// + /// Note that [`TXIN_BASE_WEIGHT`] already accounts for the outpoint, `nSequence`, and 1 byte for + /// `scriptSigLen`. + pub fn new_segwit(value: u64, satisfaction_weight: u64) -> Candidate { + let weight = TXIN_BASE_WEIGHT + satisfaction_weight; + Candidate { + value, + weight, + segwit_count: 1, + legacy_count: 0, + } + } + + /// Create a new [`Candidate`] that represents a single legacy (non-segwit) input. + /// + /// `satisfaction_weight` is the additional weight (in weight units) required to satisfy the input + /// beyond [`TXIN_BASE_WEIGHT`] (e.g. `scriptSig` at 4 WU/byte, plus 4 WU per extra `scriptSigLen` + /// varint byte if `scriptSig` exceeds 252 bytes). + /// + /// Note that [`TXIN_BASE_WEIGHT`] already accounts for the outpoint, `nSequence`, and 1 byte for + /// `scriptSigLen`. + pub fn new_legacy(value: u64, satisfaction_weight: u64) -> Candidate { let weight = TXIN_BASE_WEIGHT + satisfaction_weight; Candidate { value, weight, - segwit_count: is_segwit as usize, - legacy_count: !is_segwit as usize, + segwit_count: 0, + legacy_count: 1, } }