From 3accbff67db05e76ced386e4776fdaf3357de487 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:57:22 +0300 Subject: [PATCH 1/7] feat(key-wallet): let a build fund from only the inputs it was given MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `add_funding` unions the funding account's entire unreserved UTXO set into the candidate pool, and `SelectionStrategy::All` then takes all of it. A caller that splits a large account into batches of at most MAX_STANDARD_TX_INPUTS and seeds one batch per build therefore achieves nothing: every build still sees the whole account and fails with TooManyInputs, so an account above the cap cannot be drained at all, however the caller chunks it. That is the iOS CoinJoin sweep. A wallet with 589 mixed UTXOs reports "Too many inputs for a standard transaction: 589 (max 500)" on every attempt and every retry, and the coins cannot be moved by any route the app offers. `use_only_added_inputs()` opts a build into funding from its seeded inputs alone. `add_funding` still records the reservation bookkeeping and supplies the change address, so whichever seeded outpoints selection picks are reserved by the account that holds them; it just contributes no candidates of its own. It also drops a seeded input the account has since reserved for another in-flight build. `add_inputs` does not consult the reservation set, while the normal funding path guarantees every candidate is unreserved — without this the opt-in would introduce a double-spend the default path cannot produce. Both directions are pinned by tests: a 589-UTXO account fails with TooManyInputs unbounded, and builds its 500-input chunk with the opt-in. --- .../transaction_builder.rs | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 3d145b3fa..bb936b363 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -95,6 +95,9 @@ pub struct TransactionBuilder { /// account that holds the UTXO, so each account reserves its own share of /// the chosen inputs — all under the one token this build is stamped with. funding: Vec<(ReservationSet, HashSet)>, + /// When set, `add_funding` contributes no candidates of its own — see + /// [`Self::use_only_added_inputs`]. + only_added_inputs: bool, } impl Default for TransactionBuilder { @@ -119,6 +122,7 @@ impl TransactionBuilder { special_payload: None, payload_finalizer: None, funding: Vec::new(), + only_added_inputs: false, } } @@ -183,8 +187,20 @@ impl TransactionBuilder { if present.contains(&utxo.outpoint) { continue; } + if self.only_added_inputs { + continue; + } candidates.push(utxo.clone()); } + // `add_inputs` does not consult the reservation set, so a seeded + // outpoint this account has since reserved for another in-flight build + // would be selectable here — a double-spend the normal funding path + // cannot produce, because every candidate it offers is unreserved. + if self.only_added_inputs { + self.inputs.retain(|utxo| { + !(reserved.contains(&utxo.outpoint) && funds_acc.utxos.contains_key(&utxo.outpoint)) + }); + } self.funding.push((funds_acc.reservations().clone(), owned)); self.inputs.extend(candidates); if self.change_addr.is_none() { @@ -203,6 +219,27 @@ impl TransactionBuilder { self } + /// Restrict coin selection to the inputs [`Self::add_inputs`] supplied: + /// `add_funding` keeps doing its reservation bookkeeping and still supplies + /// the change address, but contributes no candidates of its own. + /// + /// Without this, `add_funding` unions the funding account's entire + /// unreserved UTXO set into the candidate pool, and + /// [`SelectionStrategy::All`] then takes all of it. A caller that splits a + /// large account into batches of at most `MAX_STANDARD_TX_INPUTS` and + /// seeds one batch per build therefore has no effect at all: every build + /// sees the whole account and fails with [`BuilderError::TooManyInputs`], + /// so an account above the cap can never be drained. That is exactly what a + /// chunked CoinJoin sweep does. + /// + /// Reservation bookkeeping is unchanged: `owned` still covers every + /// unreserved UTXO of the account, so whichever seeded outpoints selection + /// picks are reserved by the account that holds them. + pub fn use_only_added_inputs(mut self) -> Self { + self.only_added_inputs = true; + self + } + /// Add an output to a specific address /// /// Note: by default outputs are sorted according to BIP-69 when the transaction is built: @@ -1982,6 +2019,126 @@ mod tests { assert!(candidates.contains(&free.outpoint)); } + #[test] + fn use_only_added_inputs_keeps_selection_to_the_seeded_batch() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let seeded = Utxo::dummy(0x01, 500_000, 100, false, true); + let other = Utxo::dummy(0x02, 500_000, 100, false, true); + funds.utxos.insert(seeded.outpoint, seeded.clone()); + funds.utxos.insert(other.outpoint, other.clone()); + + let builder = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .use_only_added_inputs() + .add_inputs(vec![seeded.clone()]) + .add_funding(&mut funds, &account) + .add_output(&ctx.receive_address, 100_000); + + let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + assert_eq!( + candidates, + vec![seeded.outpoint], + "add_funding must contribute no candidates of its own, got {candidates:?}" + ); + + let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); + assert_eq!(prevouts, vec![seeded.outpoint], "only the seeded input may be spent"); + + // Reservation bookkeeping is unchanged: the account that owns the + // seeded input still reserves it. + assert!(funds.reservations().reserved(200).contains(&seeded.outpoint)); + } + + #[test] + fn only_added_inputs_drops_a_seeded_input_the_account_already_reserved() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let free = Utxo::dummy(0x01, 500_000, 100, false, true); + let taken = Utxo::dummy(0x02, 500_000, 100, false, true); + funds.utxos.insert(free.outpoint, free.clone()); + funds.utxos.insert(taken.outpoint, taken.clone()); + + // Another in-flight build already holds one of the outpoints the caller + // seeds. `add_inputs` does not consult the reservation set, so without + // the filter this build would select it too and double-spend it. + funds.reservations().reserve(&[taken.outpoint], 200, ReservationToken::next()); + + let builder = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .use_only_added_inputs() + .add_inputs(vec![free.clone(), taken.clone()]) + .add_funding(&mut funds, &account) + .add_output(&ctx.receive_address, 100_000); + + let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + assert_eq!( + candidates, + vec![free.outpoint], + "a seeded input reserved by another build must be dropped, got {candidates:?}" + ); + } + + #[test] + fn only_added_inputs_lets_a_chunked_drain_clear_the_input_cap() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + // An account above MAX_STANDARD_TX_INPUTS, like a heavily mixed + // CoinJoin account: 589 UTXOs was the figure from ticket 32081. + // `Utxo::dummy` only varies the txid, which caps it at 256 distinct + // outpoints — vary the vout to get past the input limit. + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let unique: Vec = (0..589u32) + .map(|i| { + let mut utxo = Utxo::dummy((i / 256) as u8, 500_000, 100, false, true); + utxo.outpoint.vout = i; + utxo + }) + .collect(); + for utxo in &unique { + funds.utxos.insert(utxo.outpoint, utxo.clone()); + } + assert_eq!(funds.utxos.len(), 589, "the fixture must exceed the cap"); + let chunk: Vec = unique.iter().take(MAX_STANDARD_TX_INPUTS).cloned().collect(); + + // Without the opt-in the whole account is pulled in and the build dies + // on the cap, however small the seeded chunk is. + let unbounded = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .add_inputs(chunk.clone()) + .add_funding(&mut funds.clone(), &account) + .add_output(&ctx.receive_address, 100_000) + .build_unsigned_reserved(); + assert!( + matches!(unbounded, Err(BuilderError::TooManyInputs { .. })), + "expected the unbounded build to hit the cap, got {unbounded:?}" + ); + + // With it, the seeded chunk is exactly what gets spent. + let (tx, _fee, _token) = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .use_only_added_inputs() + .add_inputs(chunk.clone()) + .add_funding(&mut funds, &account) + .add_output(&ctx.receive_address, 100_000) + .build_unsigned_reserved() + .expect("chunked drain builds"); + assert_eq!(tx.input.len(), chunk.len(), "the chunk is spent whole and alone"); + } + /// A UTXO seeded with `add_inputs` and then offered again by `add_funding` /// must appear ONCE. Additive funding otherwise pushes a second candidate /// for the same outpoint, and since coin selection does not deduplicate, From 8c5163ac076c1ea68903b215039248d88c1a9b9c Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:09:07 +0300 Subject: [PATCH 2/7] fix(key-wallet): apply the input restriction whatever order the caller builds in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `use_only_added_inputs` only gated later `add_funding` calls, so a caller that funded first kept the account's whole unreserved set in the candidate pool and `SelectionStrategy::All` still tripped the cap — the very failure the option exists to prevent. `add_inputs` now records the outpoints it supplied, and enabling the option discards everything else already in the pool. Both orders now build the same transaction, which a regression test pins. --- .../transaction_builder.rs | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index bb936b363..2e2957e4e 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -98,6 +98,9 @@ pub struct TransactionBuilder { /// When set, `add_funding` contributes no candidates of its own — see /// [`Self::use_only_added_inputs`]. only_added_inputs: bool, + /// Outpoints supplied through [`Self::add_inputs`]. Kept so the restriction + /// can be applied whatever order the caller builds in. + seeded_inputs: HashSet, } impl Default for TransactionBuilder { @@ -123,6 +126,7 @@ impl TransactionBuilder { payload_finalizer: None, funding: Vec::new(), only_added_inputs: false, + seeded_inputs: HashSet::new(), } } @@ -215,7 +219,10 @@ impl TransactionBuilder { } pub fn add_inputs(mut self, inputs: impl IntoIterator) -> Self { - self.inputs.extend(inputs); + for utxo in inputs { + self.seeded_inputs.insert(utxo.outpoint); + self.inputs.push(utxo); + } self } @@ -235,8 +242,21 @@ impl TransactionBuilder { /// Reservation bookkeeping is unchanged: `owned` still covers every /// unreserved UTXO of the account, so whichever seeded outpoints selection /// picks are reserved by the account that holds them. + /// + /// Independent of call order: enabling it also discards candidates an + /// earlier `add_funding` contributed, so `add_funding(..).use_only_added_inputs()` + /// and `use_only_added_inputs().add_funding(..)` build the same transaction. pub fn use_only_added_inputs(mut self) -> Self { self.only_added_inputs = true; + // Order-independent: an `add_funding` that already ran left the + // account's candidates in `inputs`, and they have to go too, or the + // build still selects the whole account and still trips the cap. + let Self { + inputs, + seeded_inputs, + .. + } = &mut self; + inputs.retain(|utxo| seeded_inputs.contains(&utxo.outpoint)); self } @@ -2055,6 +2075,40 @@ mod tests { assert!(funds.reservations().reserved(200).contains(&seeded.outpoint)); } + #[test] + fn only_added_inputs_is_independent_of_builder_call_order() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let seeded = Utxo::dummy(0x01, 500_000, 100, false, true); + let other = Utxo::dummy(0x02, 500_000, 100, false, true); + funds.utxos.insert(seeded.outpoint, seeded.clone()); + funds.utxos.insert(other.outpoint, other.clone()); + + // The opt-in comes AFTER funding, so `add_funding` has already put the + // account's whole unreserved set into the candidate pool. + let builder = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .add_inputs(vec![seeded.clone()]) + .add_funding(&mut funds, &account) + .use_only_added_inputs() + .add_output(&ctx.receive_address, 100_000); + + let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + assert_eq!( + candidates, + vec![seeded.outpoint], + "enabling the option must discard candidates an earlier add_funding added, got {candidates:?}" + ); + + let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); + assert_eq!(prevouts, vec![seeded.outpoint]); + } + #[test] fn only_added_inputs_drops_a_seeded_input_the_account_already_reserved() { let ctx = TestWalletContext::new_random(); From 747058a60fef94fe6ab541eb2d3b19e22c9e4743 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:19:26 +0300 Subject: [PATCH 3/7] fix(key-wallet): collapse an outpoint both add_inputs and add_funding offered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seeding an outpoint an earlier `add_funding` already contributed left two candidates for it, and the restriction kept both — coin selection does not deduplicate, so `SelectionStrategy::All` spent it twice and Core rejects the duplicate prevouts. The restriction now deduplicates as it filters. Scoped to the opt-in, like the reserved-input filter beside it: `add_inputs` can duplicate on the default path too, but deduplicating there changes behaviour for every current caller — and several existing tests seed the same outpoint repeatedly and rely on each copy counting, which is a fixture bug worth its own change rather than a silent one here. --- .../transaction_builder.rs | 48 +++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 2e2957e4e..f7f1ac73d 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -244,8 +244,9 @@ impl TransactionBuilder { /// picks are reserved by the account that holds them. /// /// Independent of call order: enabling it also discards candidates an - /// earlier `add_funding` contributed, so `add_funding(..).use_only_added_inputs()` - /// and `use_only_added_inputs().add_funding(..)` build the same transaction. + /// earlier `add_funding` contributed, and collapses an outpoint that both + /// supplied to a single candidate, so every ordering builds the same + /// transaction. pub fn use_only_added_inputs(mut self) -> Self { self.only_added_inputs = true; // Order-independent: an `add_funding` that already ran left the @@ -256,7 +257,12 @@ impl TransactionBuilder { seeded_inputs, .. } = &mut self; - inputs.retain(|utxo| seeded_inputs.contains(&utxo.outpoint)); + // Deduplicated as well as restricted: seeding an outpoint an earlier + // `add_funding` already offered leaves two candidates for it, and coin + // selection does not deduplicate, so `SelectionStrategy::All` would + // spend it twice and Core rejects the duplicate prevouts. + let mut kept: HashSet = HashSet::new(); + inputs.retain(|utxo| seeded_inputs.contains(&utxo.outpoint) && kept.insert(utxo.outpoint)); self } @@ -2075,6 +2081,42 @@ mod tests { assert!(funds.reservations().reserved(200).contains(&seeded.outpoint)); } + #[test] + fn add_inputs_after_add_funding_does_not_duplicate_a_candidate() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let shared = Utxo::dummy(0x01, 500_000, 100, false, true); + let other = Utxo::dummy(0x02, 500_000, 100, false, true); + funds.utxos.insert(shared.outpoint, shared.clone()); + funds.utxos.insert(other.outpoint, other.clone()); + + // Funding first, then seeding the SAME outpoint: without dedup the pool + // holds it twice, and the opt-in keeps both copies. + let builder = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .add_funding(&mut funds, &account) + .add_inputs(vec![shared.clone()]) + .use_only_added_inputs() + .add_output(&ctx.receive_address, 100_000); + + let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + assert_eq!( + candidates, + vec![shared.outpoint], + "the shared outpoint must be offered exactly once, got {candidates:?}" + ); + + let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); + let mut deduped = prevouts.clone(); + deduped.dedup(); + assert_eq!(prevouts, deduped, "transaction must not contain duplicate prevouts"); + } + #[test] fn only_added_inputs_is_independent_of_builder_call_order() { let ctx = TestWalletContext::new_random(); From bd85d750d26661ebf480233ee21b87bd9b2e20a8 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:59:13 +0300 Subject: [PATCH 4/7] fix(key-wallet): apply the input restriction at selection time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The restriction ran wherever it was configured, so a later call could still slip a candidate past it: `add_inputs` after `use_only_added_inputs` could seed an outpoint another build had reserved, and nothing dropped it — `add_funding` had already excluded it, so it is not in that account's `owned` set and the build would not reserve it either, leaving coin selection free to spend it into a conflicting transaction. All three concerns now run once, immediately before coin selection, beside the `require_final_inputs` filter: keep only what `add_inputs` seeded, drop what a funding account has reserved, and collapse an outpoint offered twice. Call order stops mattering, and `add_funding` and `use_only_added_inputs` go back to being a plain accumulator and a plain setter. The tests now assert on the built transaction rather than the intermediate pool, which is where the contract actually lives, and cover both orderings. --- .../transaction_builder.rs | 163 +++++++++--------- 1 file changed, 86 insertions(+), 77 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index f7f1ac73d..0b9ba792b 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -191,20 +191,8 @@ impl TransactionBuilder { if present.contains(&utxo.outpoint) { continue; } - if self.only_added_inputs { - continue; - } candidates.push(utxo.clone()); } - // `add_inputs` does not consult the reservation set, so a seeded - // outpoint this account has since reserved for another in-flight build - // would be selectable here — a double-spend the normal funding path - // cannot produce, because every candidate it offers is unreserved. - if self.only_added_inputs { - self.inputs.retain(|utxo| { - !(reserved.contains(&utxo.outpoint) && funds_acc.utxos.contains_key(&utxo.outpoint)) - }); - } self.funding.push((funds_acc.reservations().clone(), owned)); self.inputs.extend(candidates); if self.change_addr.is_none() { @@ -243,26 +231,13 @@ impl TransactionBuilder { /// unreserved UTXO of the account, so whichever seeded outpoints selection /// picks are reserved by the account that holds them. /// - /// Independent of call order: enabling it also discards candidates an - /// earlier `add_funding` contributed, and collapses an outpoint that both - /// supplied to a single candidate, so every ordering builds the same - /// transaction. + /// Independent of call order: the restriction is applied immediately before + /// coin selection, so it does not matter when this is called relative to + /// `add_inputs` and `add_funding` — every ordering builds the same + /// transaction. Seeded outpoints another in-flight build has reserved are + /// dropped there too. pub fn use_only_added_inputs(mut self) -> Self { self.only_added_inputs = true; - // Order-independent: an `add_funding` that already ran left the - // account's candidates in `inputs`, and they have to go too, or the - // build still selects the whole account and still trips the cap. - let Self { - inputs, - seeded_inputs, - .. - } = &mut self; - // Deduplicated as well as restricted: seeding an outpoint an earlier - // `add_funding` already offered leaves two candidates for it, and coin - // selection does not deduplicate, so `SelectionStrategy::All` would - // spend it twice and Core rejects the duplicate prevouts. - let mut kept: HashSet = HashSet::new(); - inputs.retain(|utxo| seeded_inputs.contains(&utxo.outpoint) && kept.insert(utxo.outpoint)); self } @@ -586,6 +561,32 @@ impl TransactionBuilder { self.inputs.retain(|utxo| utxo.is_confirmed || utxo.is_instantlocked); } + if self.only_added_inputs { + // Applied here rather than where the option is set, so no call + // order can slip a candidate past it: `add_inputs` may run after + // `use_only_added_inputs`, and `add_funding` either side of it. + // + // Three things at once: drop what `add_funding` contributed, drop a + // seeded outpoint another in-flight build has reserved (`add_inputs` + // does not consult the reservation set, while every candidate + // `add_funding` offers is unreserved), and collapse an outpoint both + // supplied to one candidate — coin selection does not deduplicate, + // so a second copy is spent twice and Core rejects the transaction. + let reserved: HashSet = self + .funding + .iter() + .flat_map(|(reservations, _)| reservations.reserved(self.current_height)) + .collect(); + let seeded = core::mem::take(&mut self.seeded_inputs); + let mut kept: HashSet = HashSet::new(); + self.inputs.retain(|utxo| { + seeded.contains(&utxo.outpoint) + && !reserved.contains(&utxo.outpoint) + && kept.insert(utxo.outpoint) + }); + self.seeded_inputs = seeded; + } + // Must match `calculate_base_size`, including the conservative VIN0 routing-script size. let change_output_size = self.estimated_change_output_size(); @@ -2065,17 +2066,14 @@ mod tests { .add_funding(&mut funds, &account) .add_output(&ctx.receive_address, 100_000); - let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); assert_eq!( - candidates, + prevouts, vec![seeded.outpoint], - "add_funding must contribute no candidates of its own, got {candidates:?}" + "add_funding must contribute nothing of its own, got {prevouts:?}" ); - let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); - let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); - assert_eq!(prevouts, vec![seeded.outpoint], "only the seeded input may be spent"); - // Reservation bookkeeping is unchanged: the account that owns the // seeded input still reserves it. assert!(funds.reservations().reserved(200).contains(&seeded.outpoint)); @@ -2103,18 +2101,13 @@ mod tests { .use_only_added_inputs() .add_output(&ctx.receive_address, 100_000); - let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); assert_eq!( - candidates, + prevouts, vec![shared.outpoint], - "the shared outpoint must be offered exactly once, got {candidates:?}" + "the shared outpoint must be spent exactly once, got {prevouts:?}" ); - - let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); - let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); - let mut deduped = prevouts.clone(); - deduped.dedup(); - assert_eq!(prevouts, deduped, "transaction must not contain duplicate prevouts"); } #[test] @@ -2139,16 +2132,13 @@ mod tests { .use_only_added_inputs() .add_output(&ctx.receive_address, 100_000); - let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); + let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); assert_eq!( - candidates, + prevouts, vec![seeded.outpoint], - "enabling the option must discard candidates an earlier add_funding added, got {candidates:?}" + "candidates an earlier add_funding added must be discarded, got {prevouts:?}" ); - - let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); - let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); - assert_eq!(prevouts, vec![seeded.outpoint]); } #[test] @@ -2157,31 +2147,50 @@ mod tests { let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); - let mut funds = ManagedCoreFundsAccount::dummy_bip44(); - let free = Utxo::dummy(0x01, 500_000, 100, false, true); - let taken = Utxo::dummy(0x02, 500_000, 100, false, true); - funds.utxos.insert(free.outpoint, free.clone()); - funds.utxos.insert(taken.outpoint, taken.clone()); - - // Another in-flight build already holds one of the outpoints the caller - // seeds. `add_inputs` does not consult the reservation set, so without - // the filter this build would select it too and double-spend it. - funds.reservations().reserve(&[taken.outpoint], 200, ReservationToken::next()); - - let builder = TransactionBuilder::new() - .set_current_height(200) - .set_selection_strategy(SelectionStrategy::All) - .use_only_added_inputs() - .add_inputs(vec![free.clone(), taken.clone()]) - .add_funding(&mut funds, &account) - .add_output(&ctx.receive_address, 100_000); + // Both orders: seeding before the opt-in, and seeding after it — the + // second is what a call-order-sensitive filter would miss. + // + // A fresh account per case: `ReservationSet` has interior mutability, so + // cloning it would share the reservations one build stamps with the next. + for seeded_last in [false, true] { + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let free = Utxo::dummy(0x01, 500_000, 100, false, true); + let taken = Utxo::dummy(0x02, 500_000, 100, false, true); + funds.utxos.insert(free.outpoint, free.clone()); + funds.utxos.insert(taken.outpoint, taken.clone()); + + // Another in-flight build already holds one of the outpoints the + // caller seeds. `add_inputs` does not consult the reservation set, + // so without the check this build would select it too. + funds.reservations().reserve(&[taken.outpoint], 200, ReservationToken::next()); + + let builder = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All); + let builder = if seeded_last { + builder + .add_funding(&mut funds, &account) + .use_only_added_inputs() + .add_inputs(vec![free.clone(), taken.clone()]) + } else { + builder + .use_only_added_inputs() + .add_inputs(vec![free.clone(), taken.clone()]) + .add_funding(&mut funds, &account) + }; - let candidates: Vec = builder.inputs.iter().map(|utxo| utxo.outpoint).collect(); - assert_eq!( - candidates, - vec![free.outpoint], - "a seeded input reserved by another build must be dropped, got {candidates:?}" - ); + let (tx, _fee, _token) = builder + .add_output(&ctx.receive_address, 100_000) + .build_unsigned_reserved() + .expect("build"); + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); + assert_eq!( + prevouts, + vec![free.outpoint], + "a seeded input reserved by another build must be dropped \ + (seeded_last = {seeded_last}), got {prevouts:?}" + ); + } } #[test] From 67ada224acc59c113a4e94bf46b55fe317977ea6 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:33:18 +0300 Subject: [PATCH 5/7] refactor(key-wallet): make it a funding call, not a builder-wide flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: `add_funding_reservation_only` takes on the account's reservation bookkeeping and change address without offering its UTXOs as candidates. Nothing is contributed, so nothing has to be undone — the builder-wide flag, the seeded- outpoint tracking and the order-independence handling all go away, and the option is now per funding account rather than per build. One check does not move to the call: `add_inputs` may run after it and does not consult a reservation set, so a seeded outpoint another in-flight build holds would still be selectable. That is revalidated before selection, against the reservation sets of the accounts funded this way — a double-spend the candidate path cannot produce, since every UTXO it offers is unreserved. --- .../transaction_builder.rs | 147 ++++++++---------- 1 file changed, 68 insertions(+), 79 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 0b9ba792b..5ef249fdc 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -95,12 +95,12 @@ pub struct TransactionBuilder { /// account that holds the UTXO, so each account reserves its own share of /// the chosen inputs — all under the one token this build is stamped with. funding: Vec<(ReservationSet, HashSet)>, - /// When set, `add_funding` contributes no candidates of its own — see - /// [`Self::use_only_added_inputs`]. - only_added_inputs: bool, - /// Outpoints supplied through [`Self::add_inputs`]. Kept so the restriction - /// can be applied whatever order the caller builds in. - seeded_inputs: HashSet, + /// Reservation sets of the accounts added through + /// [`Self::add_funding_reservation_only`]. Those calls contribute no + /// candidates, so the only inputs they can cover are seeded ones — and + /// `add_inputs` does not consult a reservation set, so seeded outpoints are + /// revalidated against these before selection. + reservation_only_funding: Vec, } impl Default for TransactionBuilder { @@ -125,8 +125,7 @@ impl TransactionBuilder { special_payload: None, payload_finalizer: None, funding: Vec::new(), - only_added_inputs: false, - seeded_inputs: HashSet::new(), + reservation_only_funding: Vec::new(), } } @@ -165,7 +164,39 @@ impl TransactionBuilder { /// must therefore not be held across an `await` between `add_funding` and /// `build_signed` or `assemble_unsigned`, since suspending there reopens the /// read-then-reserve window for a concurrent build. - pub fn add_funding(mut self, funds_acc: &mut ManagedCoreFundsAccount, acc: &Account) -> Self { + pub fn add_funding(self, funds_acc: &mut ManagedCoreFundsAccount, acc: &Account) -> Self { + self.fund_from(funds_acc, acc, true) + } + + /// Take on the account's reservation bookkeeping and change address without + /// offering any of its UTXOs as candidates, so the build spends only what + /// [`Self::add_inputs`] supplied. + /// + /// `add_funding` offers every unreserved UTXO the account holds, and + /// [`SelectionStrategy::All`] takes all of them, so seeding a subset does + /// not restrict anything: a caller splitting a large account into batches of + /// at most `MAX_STANDARD_TX_INPUTS` still has every batch see the whole + /// account and fail with [`BuilderError::TooManyInputs`], and an account + /// above the cap can never be drained. That is what a chunked CoinJoin sweep + /// does. + /// + /// Reservation bookkeeping is unchanged: `owned` still covers every + /// unreserved UTXO of the account, so whichever seeded outpoints selection + /// picks are reserved by the account that holds them. + pub fn add_funding_reservation_only( + self, + funds_acc: &mut ManagedCoreFundsAccount, + acc: &Account, + ) -> Self { + self.fund_from(funds_acc, acc, false) + } + + fn fund_from( + mut self, + funds_acc: &mut ManagedCoreFundsAccount, + acc: &Account, + contribute_candidates: bool, + ) -> Self { let reserved = funds_acc.reservations().reserved(self.current_height); // An outpoint the builder already holds — seeded by `add_inputs`, or // offered by an earlier `add_funding` of an overlapping account — must @@ -191,9 +222,14 @@ impl TransactionBuilder { if present.contains(&utxo.outpoint) { continue; } - candidates.push(utxo.clone()); + if contribute_candidates { + candidates.push(utxo.clone()); + } } self.funding.push((funds_acc.reservations().clone(), owned)); + if !contribute_candidates { + self.reservation_only_funding.push(funds_acc.reservations().clone()); + } self.inputs.extend(candidates); if self.change_addr.is_none() { self.change_addr = funds_acc.next_change_address(Some(&acc.account_xpub), true).ok(); @@ -207,37 +243,7 @@ impl TransactionBuilder { } pub fn add_inputs(mut self, inputs: impl IntoIterator) -> Self { - for utxo in inputs { - self.seeded_inputs.insert(utxo.outpoint); - self.inputs.push(utxo); - } - self - } - - /// Restrict coin selection to the inputs [`Self::add_inputs`] supplied: - /// `add_funding` keeps doing its reservation bookkeeping and still supplies - /// the change address, but contributes no candidates of its own. - /// - /// Without this, `add_funding` unions the funding account's entire - /// unreserved UTXO set into the candidate pool, and - /// [`SelectionStrategy::All`] then takes all of it. A caller that splits a - /// large account into batches of at most `MAX_STANDARD_TX_INPUTS` and - /// seeds one batch per build therefore has no effect at all: every build - /// sees the whole account and fails with [`BuilderError::TooManyInputs`], - /// so an account above the cap can never be drained. That is exactly what a - /// chunked CoinJoin sweep does. - /// - /// Reservation bookkeeping is unchanged: `owned` still covers every - /// unreserved UTXO of the account, so whichever seeded outpoints selection - /// picks are reserved by the account that holds them. - /// - /// Independent of call order: the restriction is applied immediately before - /// coin selection, so it does not matter when this is called relative to - /// `add_inputs` and `add_funding` — every ordering builds the same - /// transaction. Seeded outpoints another in-flight build has reserved are - /// dropped there too. - pub fn use_only_added_inputs(mut self) -> Self { - self.only_added_inputs = true; + self.inputs.extend(inputs); self } @@ -561,30 +567,19 @@ impl TransactionBuilder { self.inputs.retain(|utxo| utxo.is_confirmed || utxo.is_instantlocked); } - if self.only_added_inputs { - // Applied here rather than where the option is set, so no call - // order can slip a candidate past it: `add_inputs` may run after - // `use_only_added_inputs`, and `add_funding` either side of it. - // - // Three things at once: drop what `add_funding` contributed, drop a - // seeded outpoint another in-flight build has reserved (`add_inputs` - // does not consult the reservation set, while every candidate - // `add_funding` offers is unreserved), and collapse an outpoint both - // supplied to one candidate — coin selection does not deduplicate, - // so a second copy is spent twice and Core rejects the transaction. + if !self.reservation_only_funding.is_empty() { + // The one check that cannot move to the funding call: `add_inputs` + // may run after it, and it does not consult a reservation set, so a + // seeded outpoint another in-flight build holds would be selectable + // here — a double-spend the candidate path cannot produce, since + // every UTXO it offers is unreserved. + let height = self.current_height; let reserved: HashSet = self - .funding + .reservation_only_funding .iter() - .flat_map(|(reservations, _)| reservations.reserved(self.current_height)) + .flat_map(|reservations| reservations.reserved(height)) .collect(); - let seeded = core::mem::take(&mut self.seeded_inputs); - let mut kept: HashSet = HashSet::new(); - self.inputs.retain(|utxo| { - seeded.contains(&utxo.outpoint) - && !reserved.contains(&utxo.outpoint) - && kept.insert(utxo.outpoint) - }); - self.seeded_inputs = seeded; + self.inputs.retain(|utxo| !reserved.contains(&utxo.outpoint)); } // Must match `calculate_base_size`, including the conservative VIN0 routing-script size. @@ -2047,7 +2042,7 @@ mod tests { } #[test] - fn use_only_added_inputs_keeps_selection_to_the_seeded_batch() { + fn reservation_only_funding_contributes_no_candidates() { let ctx = TestWalletContext::new_random(); let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); @@ -2061,9 +2056,8 @@ mod tests { let builder = TransactionBuilder::new() .set_current_height(200) .set_selection_strategy(SelectionStrategy::All) - .use_only_added_inputs() .add_inputs(vec![seeded.clone()]) - .add_funding(&mut funds, &account) + .add_funding_reservation_only(&mut funds, &account) .add_output(&ctx.receive_address, 100_000); let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); @@ -2080,7 +2074,7 @@ mod tests { } #[test] - fn add_inputs_after_add_funding_does_not_duplicate_a_candidate() { + fn reservation_only_funding_cannot_duplicate_a_seeded_candidate() { let ctx = TestWalletContext::new_random(); let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); @@ -2096,9 +2090,8 @@ mod tests { let builder = TransactionBuilder::new() .set_current_height(200) .set_selection_strategy(SelectionStrategy::All) - .add_funding(&mut funds, &account) + .add_funding_reservation_only(&mut funds, &account) .add_inputs(vec![shared.clone()]) - .use_only_added_inputs() .add_output(&ctx.receive_address, 100_000); let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); @@ -2111,7 +2104,7 @@ mod tests { } #[test] - fn only_added_inputs_is_independent_of_builder_call_order() { + fn reservation_only_funding_is_independent_of_builder_call_order() { let ctx = TestWalletContext::new_random(); let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); @@ -2128,8 +2121,7 @@ mod tests { .set_current_height(200) .set_selection_strategy(SelectionStrategy::All) .add_inputs(vec![seeded.clone()]) - .add_funding(&mut funds, &account) - .use_only_added_inputs() + .add_funding_reservation_only(&mut funds, &account) .add_output(&ctx.receive_address, 100_000); let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); @@ -2142,7 +2134,7 @@ mod tests { } #[test] - fn only_added_inputs_drops_a_seeded_input_the_account_already_reserved() { + fn reservation_only_funding_drops_a_seeded_input_it_has_reserved() { let ctx = TestWalletContext::new_random(); let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); @@ -2169,14 +2161,12 @@ mod tests { .set_selection_strategy(SelectionStrategy::All); let builder = if seeded_last { builder - .add_funding(&mut funds, &account) - .use_only_added_inputs() + .add_funding_reservation_only(&mut funds, &account) .add_inputs(vec![free.clone(), taken.clone()]) } else { builder - .use_only_added_inputs() .add_inputs(vec![free.clone(), taken.clone()]) - .add_funding(&mut funds, &account) + .add_funding_reservation_only(&mut funds, &account) }; let (tx, _fee, _token) = builder @@ -2194,7 +2184,7 @@ mod tests { } #[test] - fn only_added_inputs_lets_a_chunked_drain_clear_the_input_cap() { + fn reservation_only_funding_lets_a_chunked_drain_clear_the_input_cap() { let ctx = TestWalletContext::new_random(); let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); @@ -2235,9 +2225,8 @@ mod tests { let (tx, _fee, _token) = TransactionBuilder::new() .set_current_height(200) .set_selection_strategy(SelectionStrategy::All) - .use_only_added_inputs() .add_inputs(chunk.clone()) - .add_funding(&mut funds, &account) + .add_funding_reservation_only(&mut funds, &account) .add_output(&ctx.receive_address, 100_000) .build_unsigned_reserved() .expect("chunked drain builds"); From d9b81f1925de79f2905442a054481fecb95283a5 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 1 Sep 2026 09:40:49 +0300 Subject: [PATCH 6/7] refactor(key-wallet): derive the reserved-input check from the funding entries Per review: `funding` already carries a `ReservationSet` per account, so the separate `reservation_only_funding` field was duplicating what the builder knows. Deriving from `funding` also widens the guarantee, deliberately: the check now covers plain `add_funding` too. That path never offers a reserved UTXO of its own, but `add_inputs` can still seed one, and letting it through would spend an outpoint another in-flight build holds. No path into the builder can do that now, which a test pins. --- .../transaction_builder.rs | 58 +++++++++++++------ 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 5ef249fdc..bbbafcdf4 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -95,12 +95,6 @@ pub struct TransactionBuilder { /// account that holds the UTXO, so each account reserves its own share of /// the chosen inputs — all under the one token this build is stamped with. funding: Vec<(ReservationSet, HashSet)>, - /// Reservation sets of the accounts added through - /// [`Self::add_funding_reservation_only`]. Those calls contribute no - /// candidates, so the only inputs they can cover are seeded ones — and - /// `add_inputs` does not consult a reservation set, so seeded outpoints are - /// revalidated against these before selection. - reservation_only_funding: Vec, } impl Default for TransactionBuilder { @@ -125,7 +119,6 @@ impl TransactionBuilder { special_payload: None, payload_finalizer: None, funding: Vec::new(), - reservation_only_funding: Vec::new(), } } @@ -227,9 +220,6 @@ impl TransactionBuilder { } } self.funding.push((funds_acc.reservations().clone(), owned)); - if !contribute_candidates { - self.reservation_only_funding.push(funds_acc.reservations().clone()); - } self.inputs.extend(candidates); if self.change_addr.is_none() { self.change_addr = funds_acc.next_change_address(Some(&acc.account_xpub), true).ok(); @@ -567,17 +557,17 @@ impl TransactionBuilder { self.inputs.retain(|utxo| utxo.is_confirmed || utxo.is_instantlocked); } - if !self.reservation_only_funding.is_empty() { - // The one check that cannot move to the funding call: `add_inputs` - // may run after it, and it does not consult a reservation set, so a - // seeded outpoint another in-flight build holds would be selectable - // here — a double-spend the candidate path cannot produce, since - // every UTXO it offers is unreserved. + if !self.funding.is_empty() { + // Every UTXO a funding account offers is unreserved, but a seeded + // one need not be: `add_inputs` does not consult a reservation set, + // and may run after the funding call. Drop those here so no path + // into the builder can spend an outpoint another in-flight build + // holds. let height = self.current_height; let reserved: HashSet = self - .reservation_only_funding + .funding .iter() - .flat_map(|reservations| reservations.reserved(height)) + .flat_map(|(reservations, _)| reservations.reserved(height)) .collect(); self.inputs.retain(|utxo| !reserved.contains(&utxo.outpoint)); } @@ -2133,6 +2123,38 @@ mod tests { ); } + #[test] + fn plain_funding_also_drops_a_seeded_input_the_account_has_reserved() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let free = Utxo::dummy(0x01, 500_000, 100, false, true); + let taken = Utxo::dummy(0x02, 500_000, 100, false, true); + funds.utxos.insert(free.outpoint, free.clone()); + funds.utxos.insert(taken.outpoint, taken.clone()); + funds.reservations().reserve(&[taken.outpoint], 200, ReservationToken::next()); + + // Not the reservation-only path: `add_funding` never offers a reserved + // UTXO, but `add_inputs` can still seed one, and that must not become + // spendable either. + let (tx, _fee, _token) = TransactionBuilder::new() + .set_current_height(200) + .set_selection_strategy(SelectionStrategy::All) + .add_inputs(vec![taken.clone()]) + .add_funding(&mut funds, &account) + .add_output(&ctx.receive_address, 100_000) + .build_unsigned_reserved() + .expect("build"); + + let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); + assert!( + !prevouts.contains(&taken.outpoint), + "an outpoint another build reserved must not be spent, got {prevouts:?}" + ); + } + #[test] fn reservation_only_funding_drops_a_seeded_input_it_has_reserved() { let ctx = TestWalletContext::new_random(); From 87983afc4464e082a123f9e082e9a3db6754e462 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:01:34 +0300 Subject: [PATCH 7/7] test(key-wallet): drop the redundant cases and fix comments left from the flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: - `reservation_only_funding_is_independent_of_builder_call_order` was the same chain, fixture and assertion as `reservation_only_funding_contributes_no_candidates`. - `reservation_only_funding_cannot_duplicate_a_seeded_candidate` asserted something the shape now makes impossible: reservation-only funding contributes no candidates, so there is nothing for a seeded outpoint to duplicate against. - Comments still described the builder-wide flag as "the opt-in". The `reserved` filter in `fund_from` stays. It reads as belt-and-braces now that the pass before selection drops reserved outpoints, but it is also what keeps them out of the candidate pool in the first place, and `set_funding_skips_reserved_utxos` pins that at the pool level — removing the filter fails it. --- .../transaction_builder.rs | 69 ++----------------- 1 file changed, 5 insertions(+), 64 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index bbbafcdf4..2a456ca91 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -2063,66 +2063,6 @@ mod tests { assert!(funds.reservations().reserved(200).contains(&seeded.outpoint)); } - #[test] - fn reservation_only_funding_cannot_duplicate_a_seeded_candidate() { - let ctx = TestWalletContext::new_random(); - let account = - ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); - - let mut funds = ManagedCoreFundsAccount::dummy_bip44(); - let shared = Utxo::dummy(0x01, 500_000, 100, false, true); - let other = Utxo::dummy(0x02, 500_000, 100, false, true); - funds.utxos.insert(shared.outpoint, shared.clone()); - funds.utxos.insert(other.outpoint, other.clone()); - - // Funding first, then seeding the SAME outpoint: without dedup the pool - // holds it twice, and the opt-in keeps both copies. - let builder = TransactionBuilder::new() - .set_current_height(200) - .set_selection_strategy(SelectionStrategy::All) - .add_funding_reservation_only(&mut funds, &account) - .add_inputs(vec![shared.clone()]) - .add_output(&ctx.receive_address, 100_000); - - let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); - let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); - assert_eq!( - prevouts, - vec![shared.outpoint], - "the shared outpoint must be spent exactly once, got {prevouts:?}" - ); - } - - #[test] - fn reservation_only_funding_is_independent_of_builder_call_order() { - let ctx = TestWalletContext::new_random(); - let account = - ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); - - let mut funds = ManagedCoreFundsAccount::dummy_bip44(); - let seeded = Utxo::dummy(0x01, 500_000, 100, false, true); - let other = Utxo::dummy(0x02, 500_000, 100, false, true); - funds.utxos.insert(seeded.outpoint, seeded.clone()); - funds.utxos.insert(other.outpoint, other.clone()); - - // The opt-in comes AFTER funding, so `add_funding` has already put the - // account's whole unreserved set into the candidate pool. - let builder = TransactionBuilder::new() - .set_current_height(200) - .set_selection_strategy(SelectionStrategy::All) - .add_inputs(vec![seeded.clone()]) - .add_funding_reservation_only(&mut funds, &account) - .add_output(&ctx.receive_address, 100_000); - - let (tx, _fee, _token) = builder.build_unsigned_reserved().expect("build"); - let prevouts: Vec = tx.input.iter().map(|i| i.previous_output).collect(); - assert_eq!( - prevouts, - vec![seeded.outpoint], - "candidates an earlier add_funding added must be discarded, got {prevouts:?}" - ); - } - #[test] fn plain_funding_also_drops_a_seeded_input_the_account_has_reserved() { let ctx = TestWalletContext::new_random(); @@ -2161,8 +2101,9 @@ mod tests { let account = ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); - // Both orders: seeding before the opt-in, and seeding after it — the - // second is what a call-order-sensitive filter would miss. + // Both orders: seeding before the funding call and after it — the + // second is what a call-order-sensitive check would miss, since + // `add_inputs` never consults a reservation set. // // A fresh account per case: `ReservationSet` has interior mutability, so // cloning it would share the reservations one build stamps with the next. @@ -2229,8 +2170,8 @@ mod tests { assert_eq!(funds.utxos.len(), 589, "the fixture must exceed the cap"); let chunk: Vec = unique.iter().take(MAX_STANDARD_TX_INPUTS).cloned().collect(); - // Without the opt-in the whole account is pulled in and the build dies - // on the cap, however small the seeded chunk is. + // Funded the ordinary way the whole account is pulled in and the build + // dies on the cap, however small the seeded chunk is. let unbounded = TransactionBuilder::new() .set_current_height(200) .set_selection_strategy(SelectionStrategy::All)