From 17d3ebb9fab16111a580d0158168037407de0b36 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Sep 2026 15:46:21 -0500 Subject: [PATCH 1/6] Assign random PaymentIds to funding records Funding records were keyed by a PaymentId derived from a funding txid: the first negotiated candidate's txid when this node signed a round, the transaction's own txid when wallet sync recorded a round it could not attribute. A txid is no identity for a replaceable transaction -- the record deliberately outlives RBF rounds of its funding, so its key carried the txid of whichever round happened to come first, and code could be tempted to re-derive the id from a txid instead of resolving it. Generate the id from the OS entropy source when a signed round is first recorded, and resolve existing records through their transaction history (find_payment_by_txid) everywhere. RBF stability now comes from resolution instead of derivation. Resolution must share one lock acquisition with the record writes: resolved outside it, the id could go stale against a record wallet sync creates for the same transaction, producing a divergent record -- so the signing resolves the id under the lock it writes under. Resolution also reaches records that have graduated out of the pending store. Without that, a wallet event for a graduated record's transaction -- LDK re-broadcasting a 0conf splice whose confirmation landed while the node was offline, or a reorg after graduation -- would miss the record and create a duplicate under a fresh id. A record already failed is passed over when a newly signed round resolves its id. Wallet sync fails a funding payment whose round lost to a conflicting spend confirmed while the channel stays open, but LDK still tracks the round, so a fee bump of it is signed with the failed round among its candidates. Filed under the failed record, the bump would stay failed and untracked, so it would never graduate once it confirmed. The bump gets a record of its own instead. Only a record wallet sync created for a round it could not attribute still carries a txid-derived id, that of the transaction it saw, and the settled-record collision the wallet-sync fallback guards against arises for those records alone. The funding-record surface (candidates, stable ids) debuts in the upcoming release -- v0.7.0 shipped splice_in with no record machinery -- so changing the scheme now costs nothing, while one release later it would break payment(&PaymentId(funding_txid)) lookups for new records. Developed with assistance from Claude Code. Co-Authored-By: Claude Fable 5 Co-Authored-By: Claude Fable 5.1 --- src/wallet/mod.rs | 210 +++++++++++++++++++++++++++++--- src/wallet/payment_stores.rs | 10 ++ tests/integration_tests_rust.rs | 45 +++---- 3 files changed, 223 insertions(+), 42 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index b883ea918a..db1d00afa8 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2612,6 +2612,34 @@ impl Wallet { Ok(tx) } + /// Resolves the id under which the interactive funding with negotiated history `candidates` is + /// recorded: that of a record already tracking any of its rounds (wallet sync may record a + /// round before this node does), else a fresh one. A record already failed is passed over: + /// wallet sync fails a payment whose round lost to a conflicting spend confirmed while the + /// channel stays open, LDK still tracks the round and a fee bump of it is signed with the round + /// among its candidates, and a failed record's status is never revisited, so the bump filed + /// under it would go untracked. An id derived from a txid would tie the record's identity to + /// one round of a replaceable transaction — resolution through the record's txid history is + /// what keeps its identity stable across RBF replacements. The caller holds the cross-store + /// lock: resolved outside it, the id could go stale against a record wallet sync creates for + /// the same transaction before the caller's write. + async fn resolve_interactive_funding_id( + &self, stores: &PaymentStoresGuard<'_>, candidates: &[FundingCandidate], + ) -> Result { + for candidate in candidates.iter() { + if let Some(id) = self.find_payment_by_txid(candidate.txid).await? { + let failed = stores + .payment(&id) + .await? + .is_some_and(|payment| payment.status == PaymentStatus::Failed); + if !failed { + return Ok(id); + } + } + } + Ok(random_payment_id()) + } + /// Builds this node's share of the `active` round of an interactive funding whose negotiated /// history is `candidates`, and the per-candidate figures of that history, for recording the /// round under `payment_id`. Returns `None` when there is nothing to record: no local @@ -2692,8 +2720,9 @@ impl Wallet { /// transaction, and resolves its identity through the fact. /// /// `candidates` is the channel's pending splice history as [`funding_candidates`] lists it from - /// the channel's [`SpliceDetails`], so the history is written in full, under the first - /// candidate's txid as id. + /// the channel's [`SpliceDetails`], so the history is written in full, under the id + /// [`Self::resolve_interactive_funding_id`] resolves (that of a record already tracking any + /// round of the history, else a fresh one). /// /// Nothing is recorded for a round missing from the history (reset between the event's /// emission and its handling, so LDK will refuse the signed transaction), already recorded (a @@ -2727,9 +2756,9 @@ impl Wallet { }) .collect(); - // The reads and the writes below must share one lock acquisition, as in every - // funding-record write: read outside it, the record could change under us before the - // write. + // Resolution, the reads and the writes below must share one lock acquisition, as in every + // funding-record write: done outside it, the id could go stale against a record wallet + // sync creates for the same transaction before the write. let stores = self.payment_stores.lock().await; // The facts of a round already on record identify its payment and this node's share of // it. Those facts are immutable, so a replay adopts them rather than deriving figures @@ -2737,11 +2766,9 @@ impl Wallet { // would be refused rather than recorded, leaving the event replaying forever. let recorded_figures = self.channel_tx_facts(&txid).await.and_then(|facts| facts.local_figures); - // Anchor the `PaymentId` to the first negotiated candidate so the record stays stable - // across RBF replacements. let payment_id = match &recorded_figures { Some(figures) => figures.funding_payment_id, - None => PaymentId(candidates.first().map_or(txid, |first| first.txid).to_byte_array()), + None => self.resolve_interactive_funding_id(&stores, candidates).await?, }; let (figures, mut history) = match self.interactive_funding_figures(payment_id, candidates, signed_round, tx) { @@ -3240,7 +3267,8 @@ impl Wallet { /// payment outright, in the facts the signing recorded about it; that is the only answer that /// holds before the payment record exists. Otherwise the pending store is asked, by the /// record's own transaction, by its candidate history and by the conflicts wallet sync listed - /// for it. + /// for it, and finally the payment store itself, for a record that graduated out of the + /// pending store. async fn find_payment_by_txid(&self, target_txid: Txid) -> Result, Error> { if let Some(figures) = self.channel_tx_facts(&target_txid).await.and_then(|facts| facts.local_figures) @@ -3274,6 +3302,25 @@ impl Wallet { return Ok(Some(entry.id())); } + // The pending store only indexes in-flight records — graduation removes the entry — so a + // graduated record's transaction resolves through the payment store itself. Without this, a + // wallet event for a graduated record's transaction — a post-graduation reorg, or the first + // sight of a transaction whose confirmation landed while the node was offline — would miss + // the record and create a duplicate under the transaction's own id. + let mut page_token = None; + loop { + let page = self.payment_stores.payments_page(page_token).await?; + if let Some(payment) = page.objects.iter().find( + |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == target_txid), + ) { + return Ok(Some(payment.id)); + } + match page.next_page_token { + Some(token) => page_token = Some(token), + None => break, + } + } + Ok(None) } @@ -3838,6 +3885,15 @@ enum FundingPaymentFailure { MovedOn, } +/// Generates a fresh funding-record [`PaymentId`] from the OS entropy source. A funding record's id +/// carries no meaning beyond uniqueness: the record is found through its transaction history +/// ([`Wallet::find_payment_by_txid`]), never re-derived from a txid. +fn random_payment_id() -> PaymentId { + let mut bytes = [0u8; 32]; + getrandom::fill(&mut bytes).expect("getrandom failed"); + PaymentId(bytes) +} + /// The outcome of [`Wallet::apply_funding_status_update_locked`]. enum FundingStatusUpdate { /// The event's transaction belongs to the funding payment; its refreshed confirmation status @@ -5240,6 +5296,14 @@ mod tests { ) } + fn confirmed_status() -> ConfirmationStatus { + ConfirmationStatus::Confirmed { + block_hash: bitcoin::BlockHash::from_byte_array([8u8; 32]), + height: 100, + timestamp: 1, + } + } + /// Inserts `tx` into the BDK wallet as canonically confirmed at `height`, extending the /// local chain to that height. fn insert_confirmed_tx(wallet: &Wallet, tx: Transaction, height: u32) { @@ -5513,7 +5577,8 @@ mod tests { let wallet = new_test_wallet(Arc::clone(&store), false).await; let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); - // A counterparty round precedes ours, so the payment's id is not the round's own txid. + // A counterparty round precedes ours; the payment's id is the one the signing generated, + // which only the round's recorded facts identify. let prior_txid = Txid::from_byte_array([0xAA; 32]); let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); let txid = tx.compute_txid(); @@ -5523,7 +5588,11 @@ mod tests { &[(prior_txid, None), (txid, Some(contribution))], ); wallet.record_signed_funding(&tx, &candidates).await.unwrap(); - let id = PaymentId(prior_txid.to_byte_array()); + let id = wallet + .find_payment_by_txid(txid) + .await + .unwrap() + .expect("the round's facts identify its payment"); // Our signatures leave the node and the counterparty broadcasts: wallet sync is the first // to see the transaction. @@ -5647,6 +5716,81 @@ mod tests { ); } + /// A fee bump of a round whose payment wallet sync failed — the round lost to a conflicting + /// spend confirmed while the channel stayed open, so LDK still tracks it and offers the bump — + /// is signed with the failed round among its candidates. The failed record takes no round: its + /// status is never revisited, so the bump would go untracked under it. The bump gets a record + /// of its own. + #[tokio::test] + async fn signing_a_bump_of_a_failed_round_gets_a_record_of_its_own() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let failed_id = wallet.find_payment_by_txid(txid).await.unwrap().expect("record"); + // Wallet sync failed the payment and removed its entry. + wallet + .payment_stores + .payment_store() + .mutate(&failed_id, |existing| { + let mut update = PaymentDetailsUpdate::new(failed_id); + update.status = Some(PaymentStatus::Failed); + let mut updated = existing?.clone(); + updated.update(update).then_some(updated) + }) + .await + .unwrap(); + wallet.payment_stores.pending_payment_store().remove(&failed_id).await.unwrap(); + + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 2, 499_000, 700); + let bump_txid = bump_tx.compute_txid(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution)), (bump_txid, Some(bump_contribution))], + ); + sign_and_observe_round(&wallet, &bump_tx, &bump_candidates).await; + + let bump_id = wallet.find_payment_by_txid(bump_txid).await.unwrap().expect("a record"); + assert_ne!(bump_id, failed_id); + let payment = + wallet.payment_stores.payment_store().get(&bump_id).await.unwrap().expect("record"); + assert_eq!(payment.status, PaymentStatus::Pending); + assert!(matches!(payment.kind, PaymentKind::Onchain { txid: t, .. } if t == bump_txid)); + let entry = wallet + .payment_stores + .pending_payment_store() + .get(&bump_id) + .await + .unwrap() + .expect("entry"); + assert_eq!(entry.details(), Some(&payment)); + assert!(entry.candidate(bump_txid).expect("candidate").awaiting_broadcast); + let failed = wallet + .payment_stores + .payment_store() + .get(&failed_id) + .await + .unwrap() + .expect("the failed record stays"); + assert_eq!(failed.status, PaymentStatus::Failed); + assert!(matches!(failed.kind, PaymentKind::Onchain { txid: t, .. } if t == txid)); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&failed_id) + .await + .unwrap() + .is_none()); + } + /// Once LDK reports a round recorded at signing negotiated, there is nothing to add but the /// broadcast itself: the round's awaiting-broadcast mark is cleared and the record left as /// written. @@ -6713,8 +6857,8 @@ mod tests { } /// A middle RBF candidate must map back to the funding record: it is neither the record's - /// id (derived from the first candidate), nor its current txid (the active candidate), nor - /// in `conflicting_txids` (it never got a `TxReplaced` event of its own). + /// id (here the txid-derived id of the first candidate), nor its current txid (the active + /// candidate), nor in `conflicting_txids` (it never got a `TxReplaced` event of its own). #[tokio::test] async fn find_payment_by_txid_maps_candidate_txids() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); @@ -6911,6 +7055,30 @@ mod tests { wallet.remove_payment(&PaymentId([8u8; 32])).await.unwrap(); } + /// A graduated funding record has no pending entry — graduation removes it — so its txid must + /// resolve through the payment store itself. Without that fallback, a wallet event for the round + /// after graduation (e.g. LDK re-broadcasting a promoted 0conf splice whose confirmation landed + /// while the node was offline) would miss the record and create a duplicate under a fresh id. + #[tokio::test] + async fn find_payment_by_txid_resolves_graduated_records() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + + let txid = Txid::from_byte_array([6u8; 32]); + let payment_id = PaymentId([21u8; 32]); + let mut graduated = + interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500)); + graduated.kind = PaymentKind::Onchain { + txid, + status: confirmed_status(), + tx_type: Some(TransactionType::InteractiveFunding { channels: vec![] }), + }; + graduated.status = PaymentStatus::Succeeded; + wallet.payment_stores.payment_store().insert_or_update(graduated).await.unwrap(); + + assert_eq!(wallet.find_payment_by_txid(txid).await.unwrap(), Some(payment_id)); + } + /// A cooperative close conflicts with a pending splice's funding transaction — both spend the /// pre-splice funding outpoint — so sync records the close among the splice record's /// conflicting txids, and the close's confirmation then resolves to the splice's PaymentId. @@ -7814,18 +7982,20 @@ mod tests { assert_eq!(reads, 2, "recording an unknown transaction re-read the payment store"); } - /// A funding record's id is anchored to its first candidate's txid. Once the payment settles - /// and its entry is removed, a wallet event for that candidate no longer resolves through the - /// candidate history — the fallback keys it by its own txid, colliding with the record's id. - /// Recording the event there would merge a fresh wallet-view `Pending` payment into the - /// terminal record; such events must be skipped. + /// A funding record wallet sync created for a round this node had not recorded keeps the + /// txid-derived id of its first candidate. Once the payment settles and its entry is removed, a + /// wallet event for that candidate no longer resolves through the candidate history — the + /// fallback keys it by its own txid, colliding with the record's id. Recording the event there + /// would merge a fresh wallet-view `Pending` payment into the terminal record; such events must + /// be skipped. #[tokio::test] async fn candidate_event_does_not_resurrect_a_settled_funding_payment() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); let wallet = new_test_wallet(store, false).await; - // The record's id derives from the first candidate r1; its txid rotated to the RBF round - // r2. The payment failed and its pending entry is gone. + // The record keeps the txid-derived id of its first candidate r1, as one created for an + // unrecorded round does; its txid rotated to the RBF round r2. The payment failed and its + // pending entry is gone. let r1 = Txid::from_byte_array([2u8; 32]); let r2 = Txid::from_byte_array([4u8; 32]); let payment_id = PaymentId(r1.to_byte_array()); diff --git a/src/wallet/payment_stores.rs b/src/wallet/payment_stores.rs index e00f5d598e..59e6f61968 100644 --- a/src/wallet/payment_stores.rs +++ b/src/wallet/payment_stores.rs @@ -12,7 +12,9 @@ use std::ops::Deref; use std::sync::Arc; use lightning::ln::channelmanager::PaymentId; +use lightning::util::persist::PageToken; +use crate::data_store::DataStorePage; use crate::payment::{PaymentDetails, PendingPaymentDetails}; use crate::types::{PaymentStore, PendingPaymentStore}; use crate::Error; @@ -69,6 +71,14 @@ impl PaymentStores { self.pending_payment_store.get(id).await } + /// A page of payment records, ordered from most recently created to least recently created; + /// see [`DataStore::list_page`](crate::data_store::DataStore::list_page). + pub(super) async fn payments_page( + &self, page_token: Option, + ) -> Result, Error> { + self.payment_store.list_page(page_token).await + } + /// Whether the pending store has an entry under `id`. pub(super) async fn has_pending_payment(&self, id: &PaymentId) -> Result { self.pending_payment_store.contains_key(id).await diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 080fb8c359..e8b12bc168 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -2899,9 +2899,7 @@ async fn splice_channel() { // them to the channel balance since there may not be a change output. let expected_splice_in_lightning_balance_sat = 4_000_002; - let payments = node_b.list_all_payments(); - let payment = - payments.into_iter().find(|p| p.id == PaymentId(txo.txid.to_byte_array())).unwrap(); + let payment = funding_payment(&node_b, txo.txid); assert_eq!(payment.fee_paid_msat, Some(expected_splice_in_fee_sat * 1_000)); assert_eq!( @@ -2952,9 +2950,7 @@ async fn splice_channel() { let expected_splice_out_fee_sat = 183; - let payments = node_a.list_all_payments(); - let payment = - payments.into_iter().find(|p| p.id == PaymentId(txo.txid.to_byte_array())).unwrap(); + let payment = funding_payment(&node_a, txo.txid); assert_eq!(payment.fee_paid_msat, Some(expected_splice_out_fee_sat * 1_000)); // The splice-out graduated to a confirmed interactive-funding payment. Its `direction` is left // unasserted on purpose: the destination is our own address, so it is a self-transfer (channel @@ -3187,15 +3183,20 @@ async fn run_rbf_splice_channel_test(confirm_original: bool) { // classification — the `tx_type` assertion below catches a regression deterministically. wait_for_tx(&electrsd.client, original_txo.txid).await; wait_for_classified_funding_payment(&node_b, original_txo.txid).await; + // The record's random id is fixed at creation; capture it while the original candidate is + // current so its stability can be asserted across the RBF rounds below. + let splice_payment_id = funding_payment(&node_b, original_txo.txid).id; node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); // For `confirm_original`, capture the original candidate's fee and raw transaction now, before // the RBF replaces it, so it can be force-confirmed (instead of the RBF) further below. let original_candidate: Option<(Option, String)> = if confirm_original { - let payment_id = PaymentId(original_txo.txid.to_byte_array()); - let fee = - node_b.payment(&payment_id).unwrap().expect("splice payment exists").fee_paid_msat; + let fee = node_b + .payment(&splice_payment_id) + .unwrap() + .expect("splice payment exists") + .fee_paid_msat; let raw_tx: String = bitcoind .client .call("getrawtransaction", &[json!(original_txo.txid.to_string())]) @@ -3226,12 +3227,11 @@ async fn run_rbf_splice_channel_test(confirm_original: bool) { node_b.sync_wallets().unwrap(); // After RBF but before confirmation, node_b (the initiator) should have a single on-chain - // payment covering both candidates: id anchored to the first broadcast, `kind.txid` pointing - // at the latest (RBF) candidate, and the durable interactive-funding `tx_type` preserved across - // the replacement. + // payment covering both candidates: still under the id it was created with, `kind.txid` + // pointing at the latest (RBF) candidate, and the durable interactive-funding `tx_type` + // preserved across the replacement. let rbf_candidate_fee = { - let payment_id = PaymentId(original_txo.txid.to_byte_array()); - let payment = node_b.payment(&payment_id).unwrap().expect("splice payment exists"); + let payment = node_b.payment(&splice_payment_id).unwrap().expect("splice payment exists"); match payment.kind { PaymentKind::Onchain { txid, @@ -3305,8 +3305,8 @@ async fn run_rbf_splice_channel_test(confirm_original: bool) { // channel-lifecycle signal, not what drives payment status. Its `kind.txid` reflects the // winning RBF candidate, and `fee_paid_msat` carries this node's `FundingContribution` fee. { - let payment_id = PaymentId(original_txo.txid.to_byte_array()); - let payment = node_b.payment(&payment_id).unwrap().expect("splice payment graduated"); + let payment = + node_b.payment(&splice_payment_id).unwrap().expect("splice payment graduated"); assert_eq!(payment.status, PaymentStatus::Succeeded); match payment.kind { PaymentKind::Onchain { txid, status: ConfirmationStatus::Confirmed { .. }, .. } => { @@ -3366,8 +3366,7 @@ async fn funding_payment_graduates_without_channel_ready() { // The funding payment is `Succeeded` purely from wallet sync reaching `ANTI_REORG_DELAY` // confirmations, asserted before draining any LDK event — so graduation is not driven by the // Lightning `ChannelReady` signal. - let payment_id = PaymentId(funding_txo.txid.to_byte_array()); - let payment = node_a.payment(&payment_id).unwrap().expect("funding payment exists"); + let payment = funding_payment(&node_a, funding_txo.txid); assert_eq!(payment.status, PaymentStatus::Succeeded); match payment.kind { PaymentKind::Onchain { @@ -3430,8 +3429,8 @@ async fn splice_payment_reorged_to_unconfirmed() { generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 1).await; node_b.sync_wallets().unwrap(); - let payment_id = PaymentId(splice_txo.txid.to_byte_array()); - let payment = node_b.payment(&payment_id).unwrap().expect("splice payment exists"); + let payment = funding_payment(&node_b, splice_txo.txid); + let payment_id = payment.id; assert_eq!(payment.status, PaymentStatus::Pending); assert!(matches!( payment.kind, @@ -3601,6 +3600,8 @@ fn only_signed_round_txid(logs: &CollectingLogWriter) -> Txid { } /// `node`'s payment for the funding transaction `funding_txid`, which it must have recorded. +/// Funding records are keyed by a random id generated at creation, so they are found through their +/// transaction history rather than by deriving an id from a txid. fn funding_payment(node: &TestNode, funding_txid: Txid) -> PaymentDetails { node.list_all_payments() .into_iter() @@ -4391,9 +4392,9 @@ async fn splice_rounds_discarded_while_the_channel_is_listed_fail_at_close() { .filter(|outpoint| spends(&first_round, *outpoint)) .collect(); assert_eq!(shared, vec![funding_txo], "the bump reused an input of the first round"); - let payment_id = PaymentId(first_txo.txid.to_byte_array()); - let payment = node_a.payment(&payment_id).unwrap().expect("the splice has a payment"); + let payment = funding_payment(&node_a, bump_txo.txid); assert_eq!(payment.status, PaymentStatus::Pending); + let payment_id = payment.id; // Neither node reconnects to the other: node B closes on its own and node A learns of the // close from the chain alone. The commitment conflicts with the bump in the mempool: let it From d038d838662ba237e469379401b1986656b1dff1 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 16 Oct 2026 04:21:11 -0500 Subject: [PATCH 2/6] f - Assign random PaymentIds to funding records Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/wallet/mod.rs | 241 ++++++++++++++++++++++++++--------- src/wallet/payment_stores.rs | 14 +- 2 files changed, 188 insertions(+), 67 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index db1d00afa8..56ec714d68 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2776,6 +2776,7 @@ impl Wallet { None => return Ok(()), }; let figures = recorded_figures.unwrap_or(figures); + let direction = figures.direction; // Only the signed round awaits broadcast: LDK broadcast the others once their signatures // were exchanged. if let Some(signed) = history.iter_mut().find(|candidate| candidate.txid == txid) { @@ -2817,6 +2818,33 @@ impl Wallet { .with_local_figures(figures), ) .await?; + // The rounds this node contributed nothing to are candidates of this payment all the same, + // and one of them may be the round that confirms. Their facts carry the payment's id as + // the signed round's do, with no share of their own: the pending entry written below lists + // them, but graduation removes it, and the facts are then all that ties such a round to the + // payment (`find_payment_by_txid`). A round whose facts already carry a payment keeps it — + // the facts are immutable — so only rounds attributed to nothing yet are written. + for candidate in candidates.iter().filter(|candidate| candidate.txid != txid) { + if aggregate_local_stakes(candidate).amount_msat.is_some() { + continue; + } + let attributed = self + .channel_tx_facts(&candidate.txid) + .await + .is_some_and(|facts| facts.local_figures.is_some()); + if attributed { + continue; + } + self.record_channel_tx_facts(ChannelTxFacts::new(candidate.txid).with_local_figures( + LocalFundingFigures { + funding_payment_id: payment_id, + amount_msat: None, + fee_paid_msat: None, + direction, + }, + )) + .await?; + } stores .mutate_pending_payment(&payment_id, |existing| { @@ -3263,12 +3291,15 @@ impl Wallet { /// The payment the transaction `target_txid` belongs to, as far as anything on record says. /// - /// A transaction this node signed a round of an interactive funding for identifies its - /// payment outright, in the facts the signing recorded about it; that is the only answer that - /// holds before the payment record exists. Otherwise the pending store is asked, by the - /// record's own transaction, by its candidate history and by the conflicts wallet sync listed - /// for it, and finally the payment store itself, for a record that graduated out of the - /// pending store. + /// A round of an interactive funding identifies its payment outright, in the facts the + /// signing recorded about it — about the round it signed and about every other candidate of + /// the history, the rounds this node contributed nothing to included. That is the only answer + /// that holds before the payment record exists, and the only one left once the record + /// graduated out of the pending store. Otherwise the pending store is asked, by the record's + /// own transaction, by its candidate history and by the conflicts wallet sync listed for it. + /// The payment store itself is never listed: it is bounded, and a record wallet sync created + /// under a transaction's own id is reached through that id, which the wallet-sync arms derive + /// from the transaction when nothing resolves it. async fn find_payment_by_txid(&self, target_txid: Txid) -> Result, Error> { if let Some(figures) = self.channel_tx_facts(&target_txid).await.and_then(|facts| facts.local_figures) @@ -3302,25 +3333,6 @@ impl Wallet { return Ok(Some(entry.id())); } - // The pending store only indexes in-flight records — graduation removes the entry — so a - // graduated record's transaction resolves through the payment store itself. Without this, a - // wallet event for a graduated record's transaction — a post-graduation reorg, or the first - // sight of a transaction whose confirmation landed while the node was offline — would miss - // the record and create a duplicate under the transaction's own id. - let mut page_token = None; - loop { - let page = self.payment_stores.payments_page(page_token).await?; - if let Some(payment) = page.objects.iter().find( - |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == target_txid), - ) { - return Ok(Some(payment.id)); - } - match page.next_page_token { - Some(token) => page_token = Some(token), - None => break, - } - } - Ok(None) } @@ -5296,14 +5308,6 @@ mod tests { ) } - fn confirmed_status() -> ConfirmationStatus { - ConfirmationStatus::Confirmed { - block_hash: bitcoin::BlockHash::from_byte_array([8u8; 32]), - height: 100, - timestamp: 1, - } - } - /// Inserts `tx` into the BDK wallet as canonically confirmed at `height`, extending the /// local chain to that height. fn insert_confirmed_tx(wallet: &Wallet, tx: Transaction, height: u32) { @@ -5716,6 +5720,84 @@ mod tests { ); } + /// A round this node contributed nothing to is a candidate of the payment all the same, and + /// may be the one that confirms. Once the record graduates, the entry that listed the round + /// as a candidate is gone, so the round's facts must identify the payment, as a signed + /// round's do. + #[tokio::test] + async fn a_graduated_records_counterparty_round_still_identifies_its_payment() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + + // The counterparty's round, then one of ours signed with it in the history. + let (counterparty_tx, _) = splice_out_round(&wallet, 1, 500_000, 300); + let counterparty_txid = counterparty_tx.compute_txid(); + let (tx, contribution) = splice_out_round(&wallet, 2, 499_000, 700); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(counterparty_txid, None), (txid, Some(contribution))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let id = wallet.find_payment_by_txid(txid).await.unwrap().expect("id"); + + // Graduation: the record settles and its pending entry, with the candidate history, goes. + let mut graduated = PaymentDetailsUpdate::new(id); + graduated.status = Some(PaymentStatus::Succeeded); + wallet.payment_stores.payment_store().update(graduated).await.unwrap(); + wallet.payment_stores.pending_payment_store().remove(&id).await.unwrap(); + + assert_eq!(wallet.find_payment_by_txid(counterparty_txid).await.unwrap(), Some(id)); + assert_eq!(wallet.find_payment_by_txid(txid).await.unwrap(), Some(id)); + } + + /// The counterparty's round is the one that confirms: the record adopts it as its transaction + /// before graduating, and the round still identifies the payment afterwards. + #[tokio::test] + async fn a_graduated_record_is_identified_by_the_counterparty_round_it_adopted() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let (counterparty_tx, _) = splice_out_round(&wallet, 1, 500_000, 300); + let counterparty_txid = counterparty_tx.compute_txid(); + let (tx, contribution) = splice_out_round(&wallet, 2, 499_000, 700); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(counterparty_txid, None), (txid, Some(contribution))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let id = wallet.find_payment_by_txid(txid).await.unwrap().expect("id"); + + let confirmation_height = 100; + insert_confirmed_tx(&wallet, counterparty_tx.clone(), confirmation_height); + let event = WalletEvent::TxConfirmed { + txid: counterparty_txid, + tx: Arc::new(counterparty_tx), + block_time: confirmed_block_time(confirmation_height), + old_block_time: None, + }; + wallet.update_payment_store(vec![event]).await.unwrap(); + let payment = + wallet.payment_stores.payment_store().get(&id).await.unwrap().expect("record"); + assert!(matches!( + payment.kind, + PaymentKind::Onchain { txid, status: ConfirmationStatus::Confirmed { .. }, .. } + if txid == counterparty_txid + )); + assert_eq!(payment.amount_msat, None); + + let mut graduated = PaymentDetailsUpdate::new(id); + graduated.status = Some(PaymentStatus::Succeeded); + wallet.payment_stores.payment_store().update(graduated).await.unwrap(); + wallet.payment_stores.pending_payment_store().remove(&id).await.unwrap(); + + assert_eq!(wallet.find_payment_by_txid(counterparty_txid).await.unwrap(), Some(id)); + } + /// A fee bump of a round whose payment wallet sync failed — the round lost to a conflicting /// spend confirmed while the channel stayed open, so LDK still tracks it and offers the bump — /// is signed with the failed round among its candidates. The failed record takes no round: its @@ -5791,6 +5873,75 @@ mod tests { .is_none()); } + /// The bump's history carries a round this node contributed nothing to over from the failed + /// record, whose signing attributed the round to the failed payment. Facts are immutable, so + /// the bump's record lists the round as a candidate and leaves its facts as they are, rather + /// than failing the signing over a second attribution. + #[tokio::test] + async fn a_bump_of_a_failed_round_leaves_a_counterparty_rounds_facts_as_they_are() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let (counterparty_tx, _) = splice_out_round(&wallet, 1, 500_000, 300); + let counterparty_txid = counterparty_tx.compute_txid(); + let (tx, contribution) = splice_out_round(&wallet, 2, 499_000, 700); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(counterparty_txid, None), (txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let failed_id = wallet.find_payment_by_txid(txid).await.unwrap().expect("record"); + // Wallet sync failed the payment and removed its entry. + wallet + .payment_stores + .payment_store() + .mutate(&failed_id, |existing| { + let mut update = PaymentDetailsUpdate::new(failed_id); + update.status = Some(PaymentStatus::Failed); + let mut updated = existing?.clone(); + updated.update(update).then_some(updated) + }) + .await + .unwrap(); + wallet.payment_stores.pending_payment_store().remove(&failed_id).await.unwrap(); + + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 3, 498_000, 1_100); + let bump_txid = bump_tx.compute_txid(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[ + (counterparty_txid, None), + (txid, Some(contribution)), + (bump_txid, Some(bump_contribution)), + ], + ); + wallet + .record_signed_funding(&bump_tx, &bump_candidates) + .await + .expect("the signing is recorded"); + + let bump_id = wallet.find_payment_by_txid(bump_txid).await.unwrap().expect("a record"); + assert_ne!(bump_id, failed_id); + let figures = wallet + .channel_tx_facts(&counterparty_txid) + .await + .expect("facts") + .local_figures + .expect("figures"); + assert_eq!(figures.funding_payment_id, failed_id); + let entry = wallet + .payment_stores + .pending_payment_store() + .get(&bump_id) + .await + .unwrap() + .expect("entry"); + assert!(entry.candidate(counterparty_txid).is_some()); + } + /// Once LDK reports a round recorded at signing negotiated, there is nothing to add but the /// broadcast itself: the round's awaiting-broadcast mark is cleared and the record left as /// written. @@ -7055,30 +7206,6 @@ mod tests { wallet.remove_payment(&PaymentId([8u8; 32])).await.unwrap(); } - /// A graduated funding record has no pending entry — graduation removes it — so its txid must - /// resolve through the payment store itself. Without that fallback, a wallet event for the round - /// after graduation (e.g. LDK re-broadcasting a promoted 0conf splice whose confirmation landed - /// while the node was offline) would miss the record and create a duplicate under a fresh id. - #[tokio::test] - async fn find_payment_by_txid_resolves_graduated_records() { - let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); - let wallet = new_test_wallet(store, false).await; - - let txid = Txid::from_byte_array([6u8; 32]); - let payment_id = PaymentId([21u8; 32]); - let mut graduated = - interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500)); - graduated.kind = PaymentKind::Onchain { - txid, - status: confirmed_status(), - tx_type: Some(TransactionType::InteractiveFunding { channels: vec![] }), - }; - graduated.status = PaymentStatus::Succeeded; - wallet.payment_stores.payment_store().insert_or_update(graduated).await.unwrap(); - - assert_eq!(wallet.find_payment_by_txid(txid).await.unwrap(), Some(payment_id)); - } - /// A cooperative close conflicts with a pending splice's funding transaction — both spend the /// pre-splice funding outpoint — so sync records the close among the splice record's /// conflicting txids, and the close's confirmation then resolves to the splice's PaymentId. diff --git a/src/wallet/payment_stores.rs b/src/wallet/payment_stores.rs index 59e6f61968..b2046878b0 100644 --- a/src/wallet/payment_stores.rs +++ b/src/wallet/payment_stores.rs @@ -12,9 +12,7 @@ use std::ops::Deref; use std::sync::Arc; use lightning::ln::channelmanager::PaymentId; -use lightning::util::persist::PageToken; -use crate::data_store::DataStorePage; use crate::payment::{PaymentDetails, PendingPaymentDetails}; use crate::types::{PaymentStore, PendingPaymentStore}; use crate::Error; @@ -33,6 +31,10 @@ use crate::Error; /// write compiles only for a holder of the lock. The reads are methods of this type and take no /// lock; a caller whose write depends on what it read takes the lock first and reads through the /// guard. +/// +/// The payment store is bounded, so nothing here lists it: a transaction is mapped to its payment +/// through the pending store, which keeps every entry in memory, and through what was recorded +/// about the transaction, which is read by its id. pub(super) struct PaymentStores { payment_store: Arc, pending_payment_store: Arc, @@ -71,14 +73,6 @@ impl PaymentStores { self.pending_payment_store.get(id).await } - /// A page of payment records, ordered from most recently created to least recently created; - /// see [`DataStore::list_page`](crate::data_store::DataStore::list_page). - pub(super) async fn payments_page( - &self, page_token: Option, - ) -> Result, Error> { - self.payment_store.list_page(page_token).await - } - /// Whether the pending store has an entry under `id`. pub(super) async fn has_pending_payment(&self, id: &PaymentId) -> Result { self.pending_payment_store.contains_key(id).await From 2c2641be7464b20ee086c5ba7c621d402ef41bd4 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Sep 2026 16:01:18 -0500 Subject: [PATCH 3/6] Record a splice's intent on its pending-payment entry A user-initiated splice dropped before LDK persists it leaves no trace in LDK. Recovering whatever the splice reserved and describing later events about it in terms of the original request both require persisting the splice intent before handing it to LDK, which happens before negotiation and therefore before any funding transaction exists. Give pending-payment entries an optional splice intent, retained until the splice locks, alongside the rounds signed under the entry and the payment record wallet sync adds once it observes a transaction. Add the SpliceIntent and SpliceKind types that record what was handed to LDK and the API call that produced it. A round trip checks that a persisted intent keeps the parts its contribution inherited from the round it replaces, which the pinned LDK records in the contribution so that `reserved_inputs` and `reserved_outputs` leave them out; the contribution's equality ignores that record, so a plain round trip could lose it unnoticed. A payment-tracking merge leaves the intent alone: the writers that persist and settle splices own it, and a record built by wallet sync carries none, so letting the merge write it would clear a live intent. An entry promoted to a record once a payment exists under its id keeps its intent along with its signed rounds. The pending-store write that follows a payment's status check now re-reads the status inside the store's critical section: only Pending payments belong in the pending store, and a status read taken outside it can go stale against graduation. This is groundwork; intents are not persisted yet. The entry points that do land with the splice tracking built on this. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Fable 5 Co-Authored-By: Claude Fable 5.1 --- src/data_store.rs | 50 ++++ src/payment/pending_payment_store.rs | 362 ++++++++++++++++++++++++++- src/wallet/mod.rs | 95 +++++-- src/wallet/payment_stores.rs | 13 + 4 files changed, 487 insertions(+), 33 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index 8d51660996..747e0b7ed6 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -470,6 +470,19 @@ where Ok(Some(new_object)) } + /// Like [`Self::mutate`], but allows the transformation to await fallible reads. + /// + /// The mutation lock remains held while `f` runs. This is useful when the new state must be + /// decided from an async read of another store without letting a concurrent writer invalidate + /// that decision. Callers must keep cross-store lock ordering consistent to avoid deadlocks. + pub(crate) async fn mutate_async(&self, id: &SO::Id, f: F) -> Result, Error> + where + F: FnOnce(Option) -> Fut, + Fut: Future, Error>>, + { + self.mutate_with(id, f).await + } + /// Returns whether an object is stored under `id`. pub(crate) async fn contains_key(&self, id: &SO::Id) -> Result { let _guard = self.mutation_lock.read().await; @@ -1246,6 +1259,43 @@ mod tests { assert_eq!(Some(expected), data_store.get(&id).await.unwrap()); } + #[tokio::test] + async fn mutate_async_awaits_fallible_reads() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + let id = TestObjectId { id: [42u8; 4] }; + let other_id = TestObjectId { id: [43u8; 4] }; + let existing_object = TestObject::new(id, [23u8; 3]); + let other_object = TestObject::new(other_id, [24u8; 3]); + let data_store: DataStore> = DataStore::new( + vec![existing_object], + KeepAllEntries, + TEST_PRIMARY_NAMESPACE.to_string(), + TEST_SECONDARY_NAMESPACE.to_string(), + Arc::clone(&store), + Arc::clone(&logger), + ); + let other_store: DataStore> = DataStore::new( + vec![other_object], + KeepAllEntries, + "other_datastore_test_primary".to_string(), + "other_datastore_test_secondary".to_string(), + store, + logger, + ); + + let result = data_store + .mutate_async(&id, |existing| async move { + let mut updated = existing.unwrap(); + updated.data = other_store.get(&other_id).await?.unwrap().data; + Ok(Some(updated)) + }) + .await; + let expected = TestObject::new(id, [24u8; 3]); + assert_eq!(Ok(Some(expected)), result); + assert_eq!(Some(expected), data_store.get(&id).await.unwrap()); + } + #[tokio::test] async fn mutate_runs_the_closure_without_the_cache_lock() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 959b017d63..17f3722f25 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -5,9 +5,13 @@ // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in // accordance with one or both of these licenses. -use bitcoin::Txid; -use lightning::impl_writeable_tlv_based; +use bitcoin::secp256k1::PublicKey; +use bitcoin::{TxOut, Txid}; +use lightning::chain::transaction::OutPoint as LdkOutPoint; use lightning::ln::channelmanager::PaymentId; +use lightning::ln::funding::FundingContribution; +use lightning::ln::types::ChannelId; +use lightning::{impl_writeable_tlv_based, impl_writeable_tlv_based_enum}; use crate::data_store::{StorableObject, StorableObjectUpdate, UpdatableObject}; use crate::payment::store::{Channel, PaymentDetailsUpdate, TransactionType}; @@ -44,14 +48,76 @@ impl_writeable_tlv_based!(FundingTxCandidate, { (6, awaiting_broadcast, required), }); +/// The parameters of the API call that initiated a splice, recording what was attempted +/// independently of the contribution built from them. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum SpliceKind { + /// [`Node::splice_in`] with a resolved amount. + /// + /// [`Node::splice_in`]: crate::Node::splice_in + In { amount_sats: u64 }, + /// [`Node::splice_out`] to the given outputs. + /// + /// [`Node::splice_out`]: crate::Node::splice_out + Out { outputs: Vec }, + /// [`Node::bump_channel_funding_fee`] of a pending splice. + /// + /// [`Node::bump_channel_funding_fee`]: crate::Node::bump_channel_funding_fee + Rbf {}, +} + +impl_writeable_tlv_based_enum!(SpliceKind, + (0, In) => { + (0, amount_sats, required), + }, + (2, Out) => { + (0, outputs, required_vec), + }, + (4, Rbf) => {}, +); + +/// A user-initiated splice that has been handed to LDK but is not yet guaranteed to survive a +/// restart. LDK only persists a splice once its negotiation reaches `AwaitingSignatures`, and it +/// abandons an in-progress negotiation whenever the peer disconnects (which includes stopping the +/// node). Until the new funding transaction locks we keep enough state to recognize a splice LDK +/// no longer knows about and to describe events about it in terms of the original request. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct SpliceIntent { + /// The channel counterparty. + pub counterparty_node_id: PublicKey, + /// The channel being spliced. + pub channel_id: ChannelId, + /// The channel's funding outpoint when the splice was initiated. It only changes once a splice + /// locks, so a mismatch with the channel's current funding outpoint means the splice (or a + /// replacement) completed and the intent is stale. + pub pre_splice_funding_txo: LdkOutPoint, + /// The contribution handed to [`ChannelManager::funding_contributed`], kept to match later + /// events about the splice back to this intent. + /// + /// [`ChannelManager::funding_contributed`]: lightning::ln::channelmanager::ChannelManager::funding_contributed + pub contribution: FundingContribution, + /// The parameters of the originating API call. + pub kind: SpliceKind, +} + +impl_writeable_tlv_based!(SpliceIntent, { + (0, counterparty_node_id, required), + (2, channel_id, required), + (4, pre_splice_funding_txo, required), + (6, contribution, required), + (8, kind, required), +}); + /// A pending payment tracked by LDK Node, keyed by [`PaymentId`]. /// /// Each part of an entry is written by a different subsystem and is present on its own schedule, -/// so all of them are optional. Signing a round of an interactive funding adds the round to -/// `candidates` and lists the `funding_channels` it belongs to, still without a transaction anyone -/// has seen; wallet sync adds `details` once it observes the transaction, and records -/// `conflicting_txids` for any wallet transaction. A splice uses all of them; the fields do not -/// partition by payment type. An entry with none of them tracks no payment and is removed. +/// so all of them are optional. A user-initiated splice is persisted with nothing but its +/// [`SpliceIntent`] before its contribution is handed to LDK; signing a round of it adds the +/// round to `candidates` and lists the `funding_channels` it belongs to, still without a +/// transaction anyone has seen; wallet sync adds `details` once it observes the transaction, and +/// records `conflicting_txids` for any wallet transaction; the `ChannelReady` arm records +/// `locked_rounds`. A splice uses all of them; the fields do not partition by payment type. An +/// entry with none of them tracks no payment and is removed. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct PendingPaymentDetails { /// The payment this entry tracks. @@ -73,6 +139,12 @@ pub(crate) struct PendingPaymentDetails { /// RBF history, keyed by each candidate's txid and recorded as each round is signed. /// Empty for non-funding payments. pub candidates: Vec, + /// The live splice intent, or `None` for a non-splice payment or a splice that has + /// locked. Persisted at splice initiation and cleared once the splice locks or its failure + /// is surfaced, it outlives the rounds negotiated under it, because a fee bump is a fresh + /// negotiation LDK likewise abandons if the peer disconnects before signing, and shares the + /// bumped round's record rather than getting one of its own. + pub splice_intent: Option, /// The candidates LDK promoted to the channel's funding, as `ChannelReady` reported them. /// A zero-conf splice locks before its transaction confirms, and every later splice builds /// on it, so such a round can still confirm once the channel's funding has moved on from @@ -84,6 +156,13 @@ pub(crate) struct PendingPaymentDetails { impl PendingPaymentDetails { pub(crate) fn new( details: PaymentDetails, conflicting_txids: Vec, candidates: Vec, + ) -> Self { + Self::tracked(details, conflicting_txids, candidates, None) + } + + pub(crate) fn tracked( + details: PaymentDetails, conflicting_txids: Vec, candidates: Vec, + splice_intent: Option, ) -> Self { Self { id: details.id, @@ -91,6 +170,20 @@ impl PendingPaymentDetails { conflicting_txids, funding_channels: Vec::new(), candidates, + splice_intent, + locked_rounds: Vec::new(), + } + } + + #[cfg(test)] + pub(crate) fn pending_splice(id: PaymentId, intent: SpliceIntent) -> Self { + Self { + id, + details: None, + conflicting_txids: Vec::new(), + funding_channels: Vec::new(), + candidates: Vec::new(), + splice_intent: Some(intent), locked_rounds: Vec::new(), } } @@ -100,6 +193,7 @@ impl PendingPaymentDetails { /// record for it exists. pub(crate) fn signed_rounds( id: PaymentId, funding_channels: Vec, candidates: Vec, + splice_intent: Option, ) -> Self { Self { id, @@ -107,6 +201,7 @@ impl PendingPaymentDetails { conflicting_txids: Vec::new(), funding_channels, candidates, + splice_intent, locked_rounds: Vec::new(), } } @@ -164,7 +259,10 @@ impl PendingPaymentDetails { /// Whether this entry tracks nothing anymore and can be dropped. pub(crate) fn is_empty(&self) -> bool { - self.details.is_none() && self.candidates.is_empty() && self.locked_rounds.is_empty() + self.details.is_none() + && self.splice_intent.is_none() + && self.candidates.is_empty() + && self.locked_rounds.is_empty() } } @@ -174,6 +272,7 @@ impl_writeable_tlv_based!(PendingPaymentDetails, { (4, conflicting_txids, optional_vec), (6, funding_channels, optional_vec), (8, candidates, optional_vec), + (10, splice_intent, option), (12, locked_rounds, optional_vec), }); @@ -183,6 +282,10 @@ pub(crate) struct PendingPaymentDetailsUpdate { pub payment_update: Option, pub conflicting_txids: Option>, pub candidates: Vec, + /// The splice intent to set (`Some(Some(..))`) or clear (`Some(None)`), or `None` to leave it + /// unchanged. Clearing the intent of an entry that carries only the intent is done by removing + /// the entry, not through this field. + pub splice_intent: Option>, } impl StorableObject for PendingPaymentDetails { @@ -233,6 +336,13 @@ impl UpdatableObject for PendingPaymentDetails { updated = true; } + if let Some(new_splice_intent) = update.splice_intent { + if self.splice_intent != new_splice_intent { + self.splice_intent = new_splice_intent; + updated = true; + } + } + updated } @@ -250,12 +360,14 @@ impl StorableObjectUpdate for PendingPaymentDetailsUpdate impl From<&PendingPaymentDetails> for PendingPaymentDetailsUpdate { fn from(value: &PendingPaymentDetails) -> Self { match &value.details { - // An entry with no record yet carries nothing a payment-tracking merge could apply. + // An entry with no record yet carries nothing a payment-tracking merge could apply + // beyond its intent, which the entry containing it owns outright. None => Self { id: value.id, payment_update: None, conflicting_txids: None, candidates: value.candidates.clone(), + splice_intent: value.splice_intent.clone().map(Some), }, Some(details) => { let conflicting_txids = if value.conflicting_txids.is_empty() { @@ -263,11 +375,16 @@ impl From<&PendingPaymentDetails> for PendingPaymentDetailsUpdate { } else { Some(value.conflicting_txids.clone()) }; + // Leave the splice intent unchanged: it is owned by the writers that persist and + // settle splices, never by a payment-tracking merge. Emitting the current value + // here would let an `insert_or_update` of a payment record (e.g. from wallet sync, + // built without an intent) clobber a live intent to `None`. Self { id: details.id, payment_update: Some(details.to_update()), conflicting_txids, candidates: value.candidates.clone(), + splice_intent: None, } }, } @@ -297,6 +414,27 @@ pub(crate) fn test_funding_contribution_with_outputs( pub(crate) fn test_funding_contribution_with_parts( estimated_fee_sat: u64, feerate: u64, prevtxs: &[bitcoin::Transaction], outputs: &[bitcoin::TxOut], change_output: Option<&bitcoin::TxOut>, +) -> lightning::ln::funding::FundingContribution { + test_funding_contribution_inheriting( + estimated_fee_sat, + feerate, + prevtxs, + outputs, + change_output, + &[], + &[], + ) +} + +/// Like [`test_funding_contribution_with_parts`], but recording `inherited_inputs` and +/// `inherited_output_scripts` as parts a still-pending splice attempt reserved before this +/// contribution, as LDK records them at the hand-off of a fee bump built from the round it +/// replaces: the contribution's `reserved_inputs` and `reserved_outputs` leave them out. +#[cfg(test)] +pub(crate) fn test_funding_contribution_inheriting( + estimated_fee_sat: u64, feerate: u64, prevtxs: &[bitcoin::Transaction], + outputs: &[bitcoin::TxOut], change_output: Option<&bitcoin::TxOut>, + inherited_inputs: &[bitcoin::OutPoint], inherited_output_scripts: &[bitcoin::ScriptBuf], ) -> lightning::ln::funding::FundingContribution { use lightning::util::ser::{BigSize, Writeable}; use lightning::util::wallet_utils::ConfirmedUtxo; @@ -340,6 +478,42 @@ pub(crate) fn test_funding_contribution_with_parts( records.extend_from_slice(&[11, 8]); // (11, max_feerate) records.extend_from_slice(&feerate.to_be_bytes()); records.extend_from_slice(&[13, 1, 1]); // (13, is_splice: true) + if !inherited_inputs.is_empty() || !inherited_output_scripts.is_empty() { + // (17, pending_components): a length-prefixed TLV stream of its own. + let mut components = Vec::new(); + if !inherited_inputs.is_empty() { + let mut bytes = Vec::new(); + for outpoint in inherited_inputs { + outpoint.write(&mut bytes).expect("in-memory write must succeed"); + } + components.push(1); // (1, inputs) + BigSize(bytes.len() as u64) + .write(&mut components) + .expect("in-memory write must succeed"); + components.extend(bytes); + } + if !inherited_output_scripts.is_empty() { + let mut bytes = Vec::new(); + for script in inherited_output_scripts { + script.write(&mut bytes).expect("in-memory write must succeed"); + } + components.push(3); // (3, output_scripts) + BigSize(bytes.len() as u64) + .write(&mut components) + .expect("in-memory write must succeed"); + components.extend(bytes); + } + let mut component_bytes = Vec::new(); + BigSize(components.len() as u64) + .write(&mut component_bytes) + .expect("in-memory write must succeed"); + component_bytes.extend(components); + records.push(17); + BigSize(component_bytes.len() as u64) + .write(&mut records) + .expect("in-memory write must succeed"); + records.extend(component_bytes); + } let mut tlv_bytes = Vec::new(); // BigSize length prefix over the TLV records above. BigSize(records.len() as u64).write(&mut tlv_bytes).expect("in-memory write must succeed"); @@ -348,6 +522,23 @@ pub(crate) fn test_funding_contribution_with_parts( .expect("hand-built TLV stream must decode") } +/// Builds a [`FundingContribution`] for tests carrying just the required TLV records: a zero +/// estimated fee, the default feerate, and no contributed outputs. +/// +/// [`FundingContribution`]: lightning::ln::funding::FundingContribution +#[cfg(test)] +pub(crate) fn test_funding_contribution() -> lightning::ln::funding::FundingContribution { + test_funding_contribution_with_feerate(253) +} + +/// Like [`test_funding_contribution`], but with the given input-selection feerate in sat/kwu. +#[cfg(test)] +pub(crate) fn test_funding_contribution_with_feerate( + feerate: u64, +) -> lightning::ln::funding::FundingContribution { + test_funding_contribution_with_outputs(0, feerate, &[]) +} + #[cfg(test)] mod tests { use bitcoin::hashes::Hash; @@ -457,6 +648,150 @@ mod tests { ); } + #[test] + fn splice_kind_round_trips() { + for kind in [ + SpliceKind::In { amount_sats: 500_000 }, + SpliceKind::Out { + outputs: vec![TxOut { + value: bitcoin::Amount::from_sat(400_000), + script_pubkey: bitcoin::ScriptBuf::new(), + }], + }, + SpliceKind::Rbf {}, + ] { + let encoded = kind.encode(); + let decoded = SpliceKind::read(&mut &encoded[..]).unwrap(); + assert_eq!(kind, decoded); + } + } + + #[test] + fn pending_splice_round_trips() { + use std::str::FromStr; + + let id = PaymentId([10u8; 32]); + let intent = SpliceIntent { + counterparty_node_id: PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(), + channel_id: ChannelId([11u8; 32]), + pre_splice_funding_txo: LdkOutPoint { txid: test_txid(12), index: 0 }, + contribution: test_funding_contribution(), + kind: SpliceKind::In { amount_sats: 500_000 }, + }; + let record = PendingPaymentDetails::pending_splice(id, intent); + + let encoded = record.encode(); + let decoded = PendingPaymentDetails::read(&mut &encoded[..]).unwrap(); + assert_eq!(record, decoded); + assert_eq!(decoded.id(), id); + assert!(decoded.details().is_none()); + } + + /// A fee bump's contribution inherits the inputs and change of the round it replaces, which + /// LDK records in the contribution at the hand-off so that `reserved_inputs` and + /// `reserved_outputs` leave them out: what a failure of the bump releases, and what a retry + /// must reserve again. That record is a private field the contribution's `PartialEq` ignores, + /// so a persisted intent's round trip is checked through those accessors. + #[test] + fn pending_splice_keeps_the_contribution_reserved_parts() { + use std::str::FromStr; + + use bitcoin::{Amount, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, WPubkeyHash}; + + let prevtx = |seed: u8| Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![TxIn::default()], + output: vec![TxOut { + value: Amount::from_sat(10_000), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([seed; 20])), + }], + }; + let prevtxs = [prevtx(1), prevtx(2)]; + let outpoint = |tx: &Transaction| OutPoint { txid: tx.compute_txid(), vout: 0 }; + let script = |seed: u8| ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([seed; 20])); + let change = TxOut { value: Amount::from_sat(21_000), script_pubkey: script(9) }; + let splice_out = TxOut { value: Amount::from_sat(50_000), script_pubkey: script(8) }; + let reserved = |contribution: &FundingContribution| { + ( + contribution.reserved_inputs().map(|input| input.outpoint()).collect::>(), + contribution.reserved_outputs().cloned().collect::>(), + ) + }; + + // Without the record, every part counts as reserved. + let plain = test_funding_contribution_with_parts( + 0, + 300, + &prevtxs, + &[splice_out.clone()], + Some(&change), + ); + assert_eq!( + reserved(&plain), + (prevtxs.iter().map(outpoint).collect(), vec![splice_out.clone(), change.clone()]) + ); + + // The bump reuses the first input and the change address of the round it replaces; the + // second input and the splice-out output are its own. + let contribution = test_funding_contribution_inheriting( + 0, + 300, + &prevtxs, + &[splice_out.clone()], + Some(&change), + &[outpoint(&prevtxs[0])], + &[change.script_pubkey.clone()], + ); + let expected = (vec![outpoint(&prevtxs[1])], vec![splice_out]); + assert_eq!(reserved(&contribution), expected); + + let intent = SpliceIntent { + counterparty_node_id: PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(), + channel_id: ChannelId([11u8; 32]), + pre_splice_funding_txo: LdkOutPoint { txid: test_txid(12), index: 0 }, + contribution, + kind: SpliceKind::Rbf {}, + }; + let record = PendingPaymentDetails::pending_splice(PaymentId([10u8; 32]), intent); + + let encoded = record.encode(); + let decoded = PendingPaymentDetails::read(&mut &encoded[..]).unwrap(); + assert_eq!(record, decoded); + let intent = decoded.splice_intent.expect("a pending splice decoded without its intent"); + assert_eq!(reserved(&intent.contribution), expected); + } + + #[test] + fn tracked_payment_round_trips() { + // An entry without a payment record round-trips in `pending_splice_round_trips`; here we + // cover one carrying the record and its candidate history. + let payment_id = PaymentId([7u8; 32]); + let txid = Txid::from_byte_array([8u8; 32]); + let record = PendingPaymentDetails::new( + pending_onchain_payment(payment_id, txid), + vec![Txid::from_byte_array([9u8; 32])], + vec![FundingTxCandidate { + txid, + amount_msat: Some(1_000), + fee_paid_msat: Some(100), + awaiting_broadcast: false, + }], + ); + + let encoded = record.encode(); + let decoded = PendingPaymentDetails::read(&mut &encoded[..]).unwrap(); + assert_eq!(record, decoded); + assert_eq!(decoded.id(), payment_id); + assert!(decoded.details().is_some()); + } + /// A candidate with the given txid byte, with a stake of ours in it if `ours`. fn candidate(txid_byte: u8, ours: bool) -> FundingTxCandidate { FundingTxCandidate { @@ -473,10 +808,10 @@ mod tests { PendingPaymentDetails::new(pending_onchain_payment(payment_id, txid), vec![], candidates) } - /// An entry written by a node from before the rounds LDK promoted were kept on it reads as an - /// entry with none, everything else intact. + /// An entry written by a node from before the rounds LDK promoted and the splice's intent were + /// kept on it reads as an entry with neither, everything else intact. #[test] - fn an_entry_written_without_locked_rounds_reads_with_none() { + fn an_entry_written_without_locked_rounds_or_an_intent_reads_with_neither() { let mut stored = entry(vec![candidate(2, false), candidate(3, true)]); stored.conflicting_txids = vec![test_txid(9)]; @@ -494,8 +829,9 @@ mod tests { let written_before = write_as_before().unwrap(); let decoded: PendingPaymentDetails = Readable::read(&mut &written_before[..]) - .expect("an entry written before the rounds were kept still reads"); + .expect("an entry written before the rounds and the intent were kept still reads"); assert!(decoded.locked_rounds().is_empty()); + assert!(decoded.splice_intent.is_none()); assert_eq!(decoded, stored); } diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 56ec714d68..4327445c2d 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -643,8 +643,8 @@ impl Wallet { let pending_payments: Vec = self .payment_stores .pending_payments(|p| match p.details() { - // An entry of signed rounds whose transaction nothing has observed - // yet carries no payment and cannot graduate. + // A pre-broadcast splice intent carries no payment yet and cannot + // graduate. None => false, Some(details) => { debug_assert!( @@ -2850,7 +2850,7 @@ impl Wallet { .mutate_pending_payment(&payment_id, |existing| { let mut changed = existing.is_none(); let mut entry = existing.cloned().unwrap_or_else(|| { - PendingPaymentDetails::signed_rounds(payment_id, Vec::new(), Vec::new()) + PendingPaymentDetails::signed_rounds(payment_id, Vec::new(), Vec::new(), None) }); if entry.funding_channels.is_empty() && !funding_channels.is_empty() { entry.funding_channels = funding_channels.clone(); @@ -3168,7 +3168,7 @@ impl Wallet { stores .mutate_pending_payment(&id, |existing| { let mut entry = existing.cloned().unwrap_or_else(|| { - PendingPaymentDetails::signed_rounds(id, Vec::new(), Vec::new()) + PendingPaymentDetails::signed_rounds(id, Vec::new(), Vec::new(), None) }); entry.candidates = candidates.clone(); Some(entry) @@ -3250,22 +3250,37 @@ impl Wallet { ) -> Result<(), Error> { let id = payment.id; stores - .mutate_pending_payment(&id, |existing| match existing { - None => Some(PendingPaymentDetails::new(payment, conflicting_txids, Vec::new())), - // Promote an entry that has no record yet: wallet sync saw the splice - // transaction before this node recorded a payment for it. The entry keeps the - // rounds signed under it, and gains the record. - Some(entry) if entry.details().is_none() => { - let mut entry = entry.clone(); - entry.details = Some(payment); - entry.conflicting_txids = conflicting_txids; - Some(entry) - }, - Some(tracked) => { - let mut tracked = tracked.clone(); - let fresh = PendingPaymentDetails::new(payment, conflicting_txids, Vec::new()); - tracked.update(fresh.to_update()).then_some(tracked) - }, + .mutate_pending_payment_async(&id, move |existing| async move { + // Only `Pending` payments belong in the pending store. The authoritative + // status is re-read inside the store's critical section, where it cannot go + // stale against graduation. + let is_pending = stores + .payment(&id) + .await? + .map_or(payment.status == PaymentStatus::Pending, |recorded| { + recorded.status == PaymentStatus::Pending + }); + if !is_pending { + return Ok(None); + } + Ok(match existing { + None => { + Some(PendingPaymentDetails::new(payment, conflicting_txids, Vec::new())) + }, + // Promote an entry that has no record yet: wallet sync saw the splice + // transaction before this node recorded a payment for it. The entry keeps + // the splice intent and the rounds signed under it, and gains the record. + Some(mut entry) if entry.details().is_none() => { + entry.details = Some(payment); + entry.conflicting_txids = conflicting_txids; + Some(entry) + }, + Some(mut tracked) => { + let fresh = + PendingPaymentDetails::new(payment, conflicting_txids, Vec::new()); + tracked.update(fresh.to_update()).then_some(tracked) + }, + }) }) .await?; Ok(()) @@ -7261,6 +7276,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7479,6 +7495,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7564,6 +7581,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7657,6 +7675,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7742,6 +7761,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7833,6 +7853,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![bumped_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7886,6 +7907,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -7953,6 +7975,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![conflict_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -8253,6 +8276,7 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), + splice_intent: None, }) .await .unwrap(); @@ -10512,4 +10536,35 @@ mod tests { payment.kind, ); } + + #[tokio::test] + async fn a_payment_that_already_advanced_gets_no_pending_entry() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + + let id = PaymentId([23u8; 32]); + let txid = Txid::from_byte_array([24u8; 32]); + // Wallet sync confirmed the payment through `ANTI_REORG_DELAY` before a writer with a + // `Pending` copy read earlier got to the pending store: the payment graduated, so no entry + // belongs there. + wallet + .payment_stores + .payment_store() + .insert(funding_payment(id, txid, PaymentStatus::Succeeded)) + .await + .unwrap(); + + let stores = wallet.payment_stores.lock().await; + wallet + .upsert_pending_payment( + &stores, + funding_payment(id, txid, PaymentStatus::Pending), + Vec::new(), + ) + .await + .unwrap(); + drop(stores); + + assert!(wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().is_none()); + } } diff --git a/src/wallet/payment_stores.rs b/src/wallet/payment_stores.rs index b2046878b0..2cf8224e46 100644 --- a/src/wallet/payment_stores.rs +++ b/src/wallet/payment_stores.rs @@ -8,6 +8,7 @@ //! The wallet's payment stores behind one API, so that every write the wallet makes to them //! happens under the lock that keeps a payment record and its pending-store entry consistent. +use std::future::Future; use std::ops::Deref; use std::sync::Arc; @@ -150,4 +151,16 @@ impl PaymentStoresGuard<'_> { { self.stores.pending_payment_store.mutate(id, f).await } + + /// [`Self::mutate_pending_payment`] with a transformation that awaits fallible reads; see + /// [`DataStore::mutate_async`](crate::data_store::DataStore::mutate_async). + pub(super) async fn mutate_pending_payment_async( + &self, id: &PaymentId, f: F, + ) -> Result, Error> + where + F: FnOnce(Option) -> Fut, + Fut: Future, Error>>, + { + self.stores.pending_payment_store.mutate_async(id, f).await + } } From 473e0761bde9c5a14cf600b2ac34fe3ef5e510de Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Sep 2026 16:09:49 -0500 Subject: [PATCH 4/6] Adopt the splice-time PaymentId when recording a splice A user-initiated splice will be keyed by a PaymentId generated at splice time rather than derived from a candidate's txid, so its splice intent, funding payment, and candidate history all share one record. Teach the signing-time recording to find a pre-broadcast splice intent by its channel and reuse that id for a splice no live record tracks yet, so the intent's entry becomes the round's record and keeps the intent until the splice locks. A round already on record keeps its record, whatever id it is under: the id of the first round of the history any record tracks is adopted before the channel's intent is consulted, and a fresh id is generated only when neither yields one. A record wallet sync has already failed does not count: a failed record is never revisited, so a fee bump signed with its lost round in the history adopts the channel's intent instead, and its entry carries the intent. The intent identifies the channel, not a round, and must not decide the id of a round already on record: a splice this node joins as a fee bump of a round wallet sync recorded first converges on the record sync created, and consulting the intent first would file the bump under the intent as a second record, with wallet sync then graduating whichever of the two it finds first. Every splice round this node contributes to that the wallet records is recorded when it is signed, before our signatures are released, so the intent only ever decides the id of a splice's first signed round, or of a bump signed after wallet sync has failed every round on record before it. Splices we did not originate (counterparty-initiated or V2 dual-funded opens) have no intent. An intent submitted for a channel whose history is already on a record under another id that has not failed is never promoted and stays bare until the splice locks or fails. A splice under a generated id is no longer found by the txid-derived lookup, so it leans on find_payment_by_txid's candidate probe to map its txids back to the record. No splice intents are created yet; the splice entry points that persist them land in a follow-up -- on this branch the intent probe stays dormant. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Fable 5 Co-Authored-By: Claude Fable 5.1 --- src/payment/pending_payment_store.rs | 5 + src/wallet/mod.rs | 384 +++++++++++++++++++++++++-- 2 files changed, 360 insertions(+), 29 deletions(-) diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 17f3722f25..4c863a34aa 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -231,6 +231,11 @@ impl PendingPaymentDetails { true } + /// The splice intent this record carries, if it is a splice that has not yet locked. + pub(crate) fn splice_intent(&self) -> Option<&SpliceIntent> { + self.splice_intent.as_ref() + } + /// Returns this node's recorded funding figures for the candidate with the given txid, if any. pub(crate) fn candidate(&self, txid: Txid) -> Option<&FundingTxCandidate> { self.candidates.iter().find(|candidate| candidate.txid == txid) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 4327445c2d..1bd85f3bcb 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2612,19 +2612,49 @@ impl Wallet { Ok(tx) } - /// Resolves the id under which the interactive funding with negotiated history `candidates` is - /// recorded: that of a record already tracking any of its rounds (wallet sync may record a - /// round before this node does), else a fresh one. A record already failed is passed over: - /// wallet sync fails a payment whose round lost to a conflicting spend confirmed while the - /// channel stays open, LDK still tracks the round and a fee bump of it is signed with the round - /// among its candidates, and a failed record's status is never revisited, so the bump filed - /// under it would go untracked. An id derived from a txid would tie the record's identity to - /// one round of a replaceable transaction — resolution through the record's txid history is - /// what keeps its identity stable across RBF replacements. The caller holds the cross-store - /// lock: resolved outside it, the id could go stale against a record wallet sync creates for - /// the same transaction before the caller's write. + /// Returns the `PaymentId` of a user-initiated splice intent for one of the channels in + /// `candidate`, if any, so the first recorded round of a splice adopts the id chosen at splice + /// time rather than a fresh one. The intent identifies the channel, not the round, so it + /// decides the id only for a history no record tracks yet + /// ([`Self::resolve_interactive_funding_id`]). A fee bump reuses the channel's existing intent, + /// so at most one in-flight intent matches and the first is unambiguous. + async fn find_splice_payment_id(&self, candidate: &FundingCandidate) -> Option { + self.payment_stores + .pending_payments(|p| { + p.splice_intent().is_some_and(|intent| { + candidate.channels.iter().any(|channel| { + channel.channel_id == intent.channel_id + && channel.counterparty_node_id == intent.counterparty_node_id + }) + }) + }) + .await + .first() + .map(|p| p.id()) + } + + /// Resolves the id under which the `active` round of the interactive funding with negotiated + /// history `candidates` is recorded. A round already on record keeps its record: the id of the + /// first round of the history any record tracks is adopted (wallet sync may record a round + /// before this node does), so a replacement, a replayed signing and a sync-created record + /// converge on one record. A record already failed is passed over: wallet sync fails a payment + /// whose round lost to a conflicting spend confirmed while the channel stays open, LDK still + /// tracks the round and a fee bump of it is signed with the round among its candidates, and a + /// failed record's status is never revisited, so the bump filed under it would go untracked. + /// Only a history no live record tracks falls back to the channel's splice intent: a + /// user-initiated splice adopts the `PaymentId` generated when it was initiated, so its intent, + /// funding payment and candidate history share one record. The intent identifies the channel, + /// not the round, which is why it must not decide the id of a round already on record: a fee + /// bump this node signs of a round wallet sync recorded first must converge on the record sync + /// created, not be filed under the bump's intent as a second record. Otherwise a fresh id is + /// generated — an id derived from a txid would tie the record's identity to one round of a + /// replaceable transaction, and resolution through the record's txid history is what keeps its + /// identity stable across RBF replacements. The caller holds the cross-store lock: resolved + /// outside it, the id could go stale against a record wallet sync creates for the same + /// transaction before the caller's write. async fn resolve_interactive_funding_id( &self, stores: &PaymentStoresGuard<'_>, candidates: &[FundingCandidate], + active: &FundingCandidate, ) -> Result { for candidate in candidates.iter() { if let Some(id) = self.find_payment_by_txid(candidate.txid).await? { @@ -2637,6 +2667,9 @@ impl Wallet { } } } + if let Some(id) = self.find_splice_payment_id(active).await { + return Ok(id); + } Ok(random_payment_id()) } @@ -2722,7 +2755,7 @@ impl Wallet { /// `candidates` is the channel's pending splice history as [`funding_candidates`] lists it from /// the channel's [`SpliceDetails`], so the history is written in full, under the id /// [`Self::resolve_interactive_funding_id`] resolves (that of a record already tracking any - /// round of the history, else a fresh one). + /// round of the history, else the channel's splice intent, else a fresh one). /// /// Nothing is recorded for a round missing from the history (reset between the event's /// emission and its handling, so LDK will refuse the signed transaction), already recorded (a @@ -2768,7 +2801,7 @@ impl Wallet { self.channel_tx_facts(&txid).await.and_then(|facts| facts.local_figures); let payment_id = match &recorded_figures { Some(figures) => figures.funding_payment_id, - None => self.resolve_interactive_funding_id(&stores, candidates).await?, + None => self.resolve_interactive_funding_id(&stores, candidates, signed_round).await?, }; let (figures, mut history) = match self.interactive_funding_figures(payment_id, candidates, signed_round, tx) { @@ -3332,8 +3365,9 @@ impl Wallet { |d| matches!(d.kind, PaymentKind::Onchain { txid, .. } if txid == target_txid), ) // A middle RBF round is not the record's current txid and may never have - // received a `TxReplaced` event of its own, so map any of its candidate - // txids (an earlier RBF round may confirm) back to the record. + // received a `TxReplaced` event of its own, and a splice keyed by a generated + // PaymentId is not found by the txid-derived id above: map any of the + // candidate txids (an earlier RBF round may confirm) back to the record. || p.candidate(target_txid).is_some() }; let matches = self @@ -4283,7 +4317,8 @@ mod tests { PENDING_PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE, }; use crate::payment::pending_payment_store::{ - test_funding_contribution_with_outputs, test_funding_contribution_with_parts, + test_funding_contribution_with_outputs, test_funding_contribution_with_parts, SpliceIntent, + SpliceKind, }; use crate::types::{DynStore, DynStoreWrapper, UserChannelId}; use crate::wallet::provenance::{ @@ -5488,6 +5523,126 @@ mod tests { (tx, contribution) } + /// The intent of a user-initiated splice of `channel_id` with `counterparty_node_id`, anchored + /// at the channel's funding `pre_splice_funding` when the splice was submitted. + fn splice_intent_for( + counterparty_node_id: PublicKey, channel_id: ChannelId, pre_splice_funding: LdkOutPoint, + ) -> SpliceIntent { + SpliceIntent { + counterparty_node_id, + channel_id, + pre_splice_funding_txo: pre_splice_funding, + contribution: test_funding_contribution_with_outputs(300, 253, &[]), + kind: SpliceKind::Out { outputs: Vec::new() }, + } + } + + /// A round signed under the channel's splice intent that has since locked with zero + /// confirmations — clearing its intent — with a second splice submitted against the locked + /// funding before the round's `SpliceNegotiated` event was handled: the channel's intent no + /// longer belongs to the recorded round. + struct LockedRoundWithNewerIntent { + first_id: PaymentId, + tx: Transaction, + candidates: Vec, + second_id: PaymentId, + second_intent: SpliceIntent, + } + + async fn lock_a_signed_round_and_submit_another_splice( + wallet: &Wallet, + ) -> LockedRoundWithNewerIntent { + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let pre_splice_funding = LdkOutPoint { txid: Txid::from_byte_array([0xAA; 32]), index: 0 }; + + let first_id = PaymentId([31u8; 32]); + let first_intent = splice_intent_for(counterparty_node_id, channel_id, pre_splice_funding); + wallet + .payment_stores + .pending_payment_store() + .insert(PendingPaymentDetails::pending_splice(first_id, first_intent)) + .await + .unwrap(); + let (tx, contribution) = splice_out_round(wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = + splice_candidates(counterparty_node_id, channel_id, &[(txid, Some(contribution))]); + wallet.record_signed_funding(&tx, &candidates).await.unwrap(); + + // The round locks with zero confirmations, which clears its intent... + let cleared = PendingPaymentDetailsUpdate { + id: first_id, + payment_update: None, + conflicting_txids: None, + candidates: Vec::new(), + splice_intent: Some(None), + }; + wallet.payment_stores.pending_payment_store().update(cleared).await.unwrap(); + // ...and a second splice of the channel is submitted against the new funding. + let second_id = PaymentId([32u8; 32]); + let second_intent = + splice_intent_for(counterparty_node_id, channel_id, LdkOutPoint { txid, index: 0 }); + wallet + .payment_stores + .pending_payment_store() + .insert(PendingPaymentDetails::pending_splice(second_id, second_intent.clone())) + .await + .unwrap(); + + LockedRoundWithNewerIntent { first_id, tx, candidates, second_id, second_intent } + } + + /// A recorded round is marked broadcast in its own record once the channel carries the intent + /// of a newer splice: after a zero-conf lock, the user may submit a second splice before the + /// locked round's `SpliceNegotiated` event is handled, and the event must neither file the + /// round under the new splice as a second record nor touch the new splice's intent. + #[tokio::test] + async fn negotiation_marks_a_recorded_round_broadcast_under_a_newer_intent() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let setup = lock_a_signed_round_and_submit_another_splice(&wallet).await; + let txid = setup.tx.compute_txid(); + + let channel_id = setup.candidates[0].channels[0].channel_id; + wallet.record_broadcast_splice_round(channel_id, txid).await.unwrap(); + + let entry = wallet + .payment_stores + .pending_payment_store() + .get(&setup.first_id) + .await + .unwrap() + .expect("entry"); + assert!(!entry.candidate(txid).expect("candidate").awaiting_broadcast); + assert_eq!( + wallet.payment_stores.pending_payment_store().get(&setup.second_id).await.unwrap(), + Some(PendingPaymentDetails::pending_splice(setup.second_id, setup.second_intent)), + "the newer splice's intent must be left untouched" + ); + } + + /// The signing event of a recorded round, replayed once the channel carries the intent of a + /// newer splice, is a no-op: the round is on record, so the newer intent is not consulted. + #[tokio::test] + async fn a_replayed_signing_under_a_newer_intent_is_a_no_op() { + let fail_store = FailSwitchStore::new(); + let store: Arc = Arc::new(DynStoreWrapper(fail_store.clone())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let setup = lock_a_signed_round_and_submit_another_splice(&wallet).await; + + fail_store.fail_writes.store(true, Ordering::Release); + wallet.record_signed_funding(&setup.tx, &setup.candidates).await.unwrap(); + assert_eq!( + fail_store.failed_writes.load(Ordering::Acquire), + 0, + "a replayed signing must produce no new write" + ); + assert_eq!( + wallet.payment_stores.pending_payment_store().get(&setup.second_id).await.unwrap(), + Some(PendingPaymentDetails::pending_splice(setup.second_id, setup.second_intent)), + ); + } + /// Signing a round writes no payment record. It records what the round is, this node's share /// of it and the funding payment it belongs to, and leaves the record itself to whoever first /// observes the transaction. @@ -5595,23 +5750,21 @@ mod tests { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); let wallet = new_test_wallet(Arc::clone(&store), false).await; let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let pre_splice_funding = LdkOutPoint { txid: Txid::from_byte_array([0xAA; 32]), index: 0 }; - // A counterparty round precedes ours; the payment's id is the one the signing generated, - // which only the round's recorded facts identify. - let prior_txid = Txid::from_byte_array([0xAA; 32]); + let id = PaymentId([31u8; 32]); + let intent = splice_intent_for(counterparty_node_id, channel_id, pre_splice_funding); + wallet + .payment_stores + .pending_payment_store() + .insert(PendingPaymentDetails::pending_splice(id, intent)) + .await + .unwrap(); let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); let txid = tx.compute_txid(); - let candidates = splice_candidates( - counterparty_node_id, - channel_id, - &[(prior_txid, None), (txid, Some(contribution))], - ); + let candidates = + splice_candidates(counterparty_node_id, channel_id, &[(txid, Some(contribution))]); wallet.record_signed_funding(&tx, &candidates).await.unwrap(); - let id = wallet - .find_payment_by_txid(txid) - .await - .unwrap() - .expect("the round's facts identify its payment"); // Our signatures leave the node and the counterparty broadcasts: wallet sync is the first // to see the transaction. @@ -5813,6 +5966,179 @@ mod tests { assert_eq!(wallet.find_payment_by_txid(counterparty_txid).await.unwrap(), Some(id)); } + /// The first round of a user-initiated splice is on no record when it is signed, so it adopts + /// the id of the channel's splice intent: the bare intent entry becomes the round's record and + /// keeps carrying the intent. + #[tokio::test] + async fn signing_a_first_round_adopts_the_intent_id() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let pre_splice_funding = LdkOutPoint { txid: Txid::from_byte_array([0xAA; 32]), index: 0 }; + + let id = PaymentId([31u8; 32]); + let intent = splice_intent_for(counterparty_node_id, channel_id, pre_splice_funding); + wallet + .payment_stores + .pending_payment_store() + .insert(PendingPaymentDetails::pending_splice(id, intent.clone())) + .await + .unwrap(); + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = + splice_candidates(counterparty_node_id, channel_id, &[(txid, Some(contribution))]); + sign_and_observe_round(&wallet, &tx, &candidates).await; + + let payment = + wallet.payment_stores.payment_store().get(&id).await.unwrap().expect("record"); + assert!(matches!(payment.kind, PaymentKind::Onchain { txid: t, .. } if t == txid)); + let txid_derived_id = PaymentId(txid.to_byte_array()); + assert!(wallet + .payment_stores + .payment_store() + .get(&txid_derived_id) + .await + .unwrap() + .is_none()); + let entry = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + assert_eq!(entry.details(), Some(&payment)); + assert_eq!(entry.splice_intent(), Some(&intent)); + assert!(entry.candidate(txid).expect("candidate").awaiting_broadcast); + } + + /// A fee bump of a round whose payment wallet sync failed — the round lost to a conflicting + /// spend confirmed while the channel stayed open — adopts the channel's splice intent rather + /// than the failed record: the failed round in its history does not decide the id, so the + /// bump is recorded under the intent's id, its entry carrying the intent. + #[tokio::test] + async fn signing_a_bump_of_a_failed_round_adopts_the_channels_intent() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let pre_splice_funding = LdkOutPoint { txid: Txid::from_byte_array([0xAA; 32]), index: 0 }; + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let failed_id = wallet.find_payment_by_txid(txid).await.unwrap().expect("record"); + // Wallet sync failed the payment and removed its entry. + wallet + .payment_stores + .payment_store() + .mutate(&failed_id, |existing| { + let mut update = PaymentDetailsUpdate::new(failed_id); + update.status = Some(PaymentStatus::Failed); + let mut updated = existing?.clone(); + updated.update(update).then_some(updated) + }) + .await + .unwrap(); + wallet.payment_stores.pending_payment_store().remove(&failed_id).await.unwrap(); + + // The bump's intent, recorded at submission with no record left to join. + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 2, 499_000, 700); + let bump_txid = bump_tx.compute_txid(); + let id = PaymentId([31u8; 32]); + let intent = SpliceIntent { + contribution: bump_contribution.clone(), + kind: SpliceKind::Rbf {}, + ..splice_intent_for(counterparty_node_id, channel_id, pre_splice_funding) + }; + wallet + .payment_stores + .pending_payment_store() + .insert(PendingPaymentDetails::pending_splice(id, intent.clone())) + .await + .unwrap(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution)), (bump_txid, Some(bump_contribution))], + ); + sign_and_observe_round(&wallet, &bump_tx, &bump_candidates).await; + + let payment = + wallet.payment_stores.payment_store().get(&id).await.unwrap().expect("record"); + assert_eq!(payment.status, PaymentStatus::Pending); + assert!(matches!(payment.kind, PaymentKind::Onchain { txid: t, .. } if t == bump_txid)); + let entry = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + assert_eq!(entry.details(), Some(&payment)); + assert_eq!(entry.splice_intent(), Some(&intent)); + assert!(entry.candidate(bump_txid).expect("candidate").awaiting_broadcast); + let failed = wallet + .payment_stores + .payment_store() + .get(&failed_id) + .await + .unwrap() + .expect("the failed record stays"); + assert_eq!(failed.status, PaymentStatus::Failed); + assert!(matches!(failed.kind, PaymentKind::Onchain { txid: t, .. } if t == txid)); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&failed_id) + .await + .unwrap() + .is_none()); + } + + /// A fee bump signed while the channel's intent is still live joins the record of the round + /// it replaces: that round is on record, so the history decides the id, and the intent the + /// bump shares with the first round stays on the record. + #[tokio::test] + async fn signing_a_bump_joins_the_replaced_rounds_record() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(Arc::clone(&store), false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + let pre_splice_funding = LdkOutPoint { txid: Txid::from_byte_array([0xAA; 32]), index: 0 }; + + let id = PaymentId([31u8; 32]); + let intent = splice_intent_for(counterparty_node_id, channel_id, pre_splice_funding); + wallet + .payment_stores + .pending_payment_store() + .insert(PendingPaymentDetails::pending_splice(id, intent.clone())) + .await + .unwrap(); + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 2, 499_000, 700); + let bump_txid = bump_tx.compute_txid(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution)), (bump_txid, Some(bump_contribution))], + ); + sign_and_observe_round(&wallet, &bump_tx, &bump_candidates).await; + + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the bump must join the first round's record"); + assert_eq!(payments[0].id, id); + assert!(matches!(payments[0].kind, PaymentKind::Onchain { txid: t, .. } if t == bump_txid)); + let entry = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + assert_eq!( + entry.candidates().iter().map(|c| c.txid).collect::>(), + vec![txid, bump_txid] + ); + assert_eq!(entry.splice_intent(), Some(&intent)); + } + /// A fee bump of a round whose payment wallet sync failed — the round lost to a conflicting /// spend confirmed while the channel stayed open, so LDK still tracks it and offers the bump — /// is signed with the failed round among its candidates. The failed record takes no round: its From e52a7b719af46817aed1cba024673bb041607f14 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Sep 2026 16:16:29 -0500 Subject: [PATCH 5/6] Merge sync-created duplicates when recording a funding round Wallet sync can observe a funding round before it is recorded as a candidate: the counterparty broadcasts a round this node did not contribute to, which is not recorded until this node signs a later round of the same splice and records the channel's history with it. The funding-status gate rightly reports such a round foreign, and sync re-keys the event to the round's txid-derived id, creating an untyped duplicate record whose pending entry from then on shadows the funding record in txid resolution: even after the round is recorded as a candidate, every later event routes to the duplicate, the confirmation strands there, and the funding record never confirms or graduates. Fold the duplicate back in when its round becomes a recorded candidate: adopt its confirmation onto the funding record -- through the same status-update path wallet sync uses, so the confirmed candidate's figures land -- and remove the duplicate along with its pending entry. A duplicate for a round that never confirmed is dropped without adopting anything; the actively-broadcast candidate stays the record's current txid. The merge runs when this node signs a round and records the channel's history with it, and again when LDK reports the round negotiated, under the writer's cross-store lock acquisition, so sync cannot interleave, and is idempotent, so a replayed SpliceNegotiated event can re-run it after a partial failure. At signing time the merge is a courtesy and a failure is only logged: the signed round can have no duplicate yet, as our signatures have not left the node, and the round's SpliceNegotiated event re-runs the merge and replays on failure. The pending entry is removed before the payment record: a replay rediscovers the duplicate through the record, so a failure between the two removals can still be cleaned up, instead of orphaning a pending entry that would shadow txid resolution all over again. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5.1 --- src/wallet/mod.rs | 755 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 738 insertions(+), 17 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 1bd85f3bcb..0b0bc22b51 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2902,15 +2902,32 @@ impl Wallet { txid, candidates.len(), ); + + // The history is complete; merging the duplicates wallet sync created for earlier rounds + // is a courtesy. The signed round can have no duplicate yet, as our signatures have not + // left the node, and the round's `SpliceNegotiated` event re-runs the merge, replaying on + // failure, so a failure here is logged rather than replaying the signing. + if let Err(e) = self.merge_duplicate_candidate_records(&stores, payment_id, &recorded).await + { + log_error!( + self.logger, + "Failed to merge duplicate records into funding payment {}: {}", + payment_id, + e, + ); + } Ok(()) } /// Marks a splice round recorded when signing ([`Self::record_signed_funding`]) as broadcast /// once LDK reports the splice negotiated: `SpliceNegotiated` is emitted only once our /// `tx_signatures` for the round are ready to send, so the counterparty may have them by then - /// and may broadcast the round, which is therefore no longer dropped as abandoned. Nothing is - /// written for a round no funding payment of `channel_id` tracks (no local contribution, or no - /// wallet-level activity) or one already marked (a replayed event). + /// and may broadcast the round, which is therefore no longer dropped as abandoned. Then merges + /// the duplicate records wallet sync created for the record's candidates + /// ([`Self::merge_duplicate_candidate_records`]), completing a merge the signing left + /// unfinished. Nothing is written for a round no funding payment of `channel_id` tracks (no + /// local contribution, or no wallet-level activity); a replayed event finds the round marked + /// already and only re-runs the merge. pub(crate) async fn record_broadcast_splice_round( &self, channel_id: ChannelId, txid: Txid, ) -> Result<(), Error> { @@ -2920,13 +2937,12 @@ impl Wallet { let entries = stores .pending_payments(|entry| { - tracks_channel(entry, channel_id) - && entry.candidate(txid).is_some_and(|candidate| candidate.awaiting_broadcast) + tracks_channel(entry, channel_id) && entry.candidate(txid).is_some() }) .await; for entry in entries { let payment_id = entry.id(); - stores + let marked = stores .mutate_pending_payment(&payment_id, |existing| { let mut entry = existing?.clone(); let round = entry @@ -2937,13 +2953,19 @@ impl Wallet { Some(entry) }) .await?; - log_debug!( - self.logger, - "Marked splice round {} of channel {} as broadcast in funding payment {}", - txid, - channel_id, - payment_id, - ); + if marked.is_some() { + log_debug!( + self.logger, + "Marked splice round {} of channel {} as broadcast in funding payment {}", + txid, + channel_id, + payment_id, + ); + } + // The round's record is complete, so the duplicates wallet sync created for earlier + // rounds can be folded in. A failure replays the event, which re-runs the merge + // idempotently. + self.merge_duplicate_candidate_records(&stores, payment_id, entry.candidates()).await?; } Ok(()) } @@ -3188,9 +3210,10 @@ impl Wallet { } /// Records a funding payment the way the node does: the rounds this node signed supply the - /// candidate history, and wallet sync writes the payment record and its pending-store entry - /// once it observes the transaction. Composes that sequence for tests that need a recorded - /// funding payment to act on. + /// candidate history, wallet sync writes the payment record and its pending-store entry once + /// it observes the transaction, and the duplicates sync created for those rounds are merged + /// into the record. Composes that sequence for tests that need a recorded funding payment to + /// act on. #[cfg(test)] async fn record_funding_payment( &self, details: PaymentDetails, candidates: Vec, @@ -3209,7 +3232,72 @@ impl Wallet { .await?; } stores.insert_or_update_payment(details.clone()).await?; - self.upsert_pending_payment(&stores, details, Vec::new()).await + self.upsert_pending_payment(&stores, details, Vec::new()).await?; + self.merge_duplicate_candidate_records(&stores, id, &candidates).await + } + + /// Merges duplicate records wallet sync created for this funding payment's candidates before + /// they were recorded as such. Sync re-keys an event for a round it cannot attribute to the + /// funding record — not yet a candidate, so the funding-status gate reports it foreign — to + /// the round's txid-derived id, creating an untyped duplicate whose pending entry then + /// shadows the funding record in [`Self::find_payment_by_txid`]'s direct probe. Once the + /// round is a recorded candidate, the duplicate's confirmation (if any) belongs on the + /// funding record: adopt it, then remove the duplicate and its pending entry. + /// + /// Runs once a record's candidate history is written, so the funding-status gate accepts the + /// candidates it adopts, and under the writer's lock acquisition, so sync cannot interleave. + /// It is idempotent: a failure at signing time ([`Self::record_signed_funding`]) is left to the + /// signed round's `SpliceNegotiated` event ([`Self::record_broadcast_splice_round`]), which + /// re-runs the merge and replays on failure. The caller must hold the [`PaymentStores`] lock, + /// per [`Self::apply_funding_status_update_locked`]'s contract. + async fn merge_duplicate_candidate_records( + &self, stores: &PaymentStoresGuard<'_>, id: PaymentId, candidates: &[FundingTxCandidate], + ) -> Result<(), Error> { + for candidate in candidates { + let duplicate_id = PaymentId(candidate.txid.to_byte_array()); + if duplicate_id == id { + continue; + } + let duplicate = match stores.payment(&duplicate_id).await? { + Some(duplicate) => duplicate, + None => continue, + }; + // Only a duplicate view of this candidate's transaction qualifies: a record wallet sync + // created for the round before it was a candidate, left untyped or classified as a + // plain funding. Anything else keyed by the txid-derived id is left alone. + let status = match &duplicate.kind { + PaymentKind::Onchain { + txid, + status, + tx_type: None | Some(TransactionType::Funding { .. }), + } if *txid == candidate.txid => status.clone(), + _ => continue, + }; + // Only a confirmation is worth adopting; an unconfirmed duplicate carries nothing the + // record needs — the actively-broadcast candidate stays the record's current txid. + if matches!(status, ConfirmationStatus::Confirmed { .. }) { + let outcome = self + .apply_funding_status_update_locked(stores, id, candidate.txid, status) + .await?; + debug_assert!(matches!(outcome, FundingStatusUpdate::Applied)); + if !matches!(outcome, FundingStatusUpdate::Applied) { + // Adoption declined; keep the duplicate rather than discard its confirmation. + continue; + } + } + log_debug!( + self.logger, + "Merging duplicate payment record for funding transaction {}", + candidate.txid, + ); + // Pending entry first: the retry of a failure between these two removals rediscovers + // the duplicate through its payment record. Removed the other way around, the + // leftover pending entry would be unreachable to the retry yet keep shadowing the + // funding record in `find_payment_by_txid`'s direct probe. + stores.remove_pending_payment(&duplicate_id).await?; + stores.remove_payment(&duplicate_id).await?; + } + Ok(()) } /// Returns the wallet's view of a transaction as `(amount_msat, fee_msat, direction)`. @@ -4492,6 +4580,86 @@ mod tests { } } + /// An in-memory store that fails the next remove issued against an armed namespace, for + /// exercising cleanup paths that must survive a failure between two removals. + #[derive(Clone)] + struct FailRemoveStore { + inner: Arc, + fail_remove_in: Arc>>, + } + + impl FailRemoveStore { + fn new() -> Self { + Self { + inner: Arc::new(InMemoryStore::new()), + fail_remove_in: Arc::new(std::sync::Mutex::new(None)), + } + } + + fn fail_next_remove_in(&self, primary_namespace: &str) { + *self.fail_remove_in.lock().unwrap() = Some(primary_namespace.to_string()); + } + } + + impl KVStore for FailRemoveStore { + fn read( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, + ) -> impl Future, io::Error>> + 'static + Send { + KVStore::read(&*self.inner, primary_namespace, secondary_namespace, key) + } + + fn write( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec, + ) -> impl Future> + 'static + Send { + KVStore::write(&*self.inner, primary_namespace, secondary_namespace, key, buf) + } + + fn remove( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool, + ) -> impl Future> + 'static + Send { + let inner = Arc::clone(&self.inner); + let armed = Arc::clone(&self.fail_remove_in); + let primary_namespace = primary_namespace.to_string(); + let secondary_namespace = secondary_namespace.to_string(); + let key = key.to_string(); + async move { + let fail = { + let mut armed = armed.lock().unwrap(); + if armed.as_deref() == Some(primary_namespace.as_str()) { + *armed = None; + true + } else { + false + } + }; + if fail { + return Err(io::Error::new(io::ErrorKind::Other, "removes disabled")); + } + KVStore::remove(&*inner, &primary_namespace, &secondary_namespace, &key, lazy).await + } + } + + fn list( + &self, primary_namespace: &str, secondary_namespace: &str, + ) -> impl Future, io::Error>> + 'static + Send { + KVStore::list(&*self.inner, primary_namespace, secondary_namespace) + } + } + + impl PaginatedKVStore for FailRemoveStore { + fn list_paginated( + &self, primary_namespace: &str, secondary_namespace: &str, + page_token: Option, + ) -> impl Future> + 'static + Send { + PaginatedKVStore::list_paginated( + &*self.inner, + primary_namespace, + secondary_namespace, + page_token, + ) + } + } + /// Constructs a `Wallet` around the given store, either creating a fresh BDK wallet or /// loading the one the store already holds. async fn new_test_wallet(store: Arc, load_existing: bool) -> Arc { @@ -8630,6 +8798,559 @@ mod tests { .is_none()); } + /// Wallet sync can record a genuine replacement round before it is recorded as a candidate: + /// the counterparty broadcast a round this node did not contribute to, which is recorded only + /// when this node signs a later round of the splice. The funding-status gate then routes the + /// round's confirmation to a duplicate record keyed by the round's txid, whose pending entry + /// shadows the funding record in `find_payment_by_txid`'s direct probe. Once the round is + /// recorded as a candidate, the write must merge the duplicate — adopt its confirmation and + /// remove it — so a single record tracks the splice. + #[tokio::test] + async fn recording_a_round_merges_duplicate_records_for_its_candidates() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + + let funding_id = PaymentId([21u8; 32]); + let txid1 = Txid::from_byte_array([1u8; 32]); + let txid2 = Txid::from_byte_array([2u8; 32]); + + // Round 1 recorded normally. + let round1 = vec![FundingTxCandidate { + txid: txid1, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(500), + awaiting_broadcast: false, + }]; + let details = interactive_funding_details(funding_id, txid1, Some(1_000_000), Some(500)); + wallet.record_funding_payment(details, round1).await.unwrap(); + + // Wallet sync recorded round 2's confirmation while the round was not yet a candidate: a + // duplicate untyped record under the txid-derived id, plus its pending entry. + let duplicate_id = PaymentId(txid2.to_byte_array()); + let duplicate = PaymentDetails::new( + duplicate_id, + PaymentKind::Onchain { txid: txid2, status: confirmed_status(), tx_type: None }, + Some(999_000), + Some(999), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_stores.payment_store().insert_or_update(duplicate.clone()).await.unwrap(); + wallet + .payment_stores + .pending_payment_store() + .insert_or_update(PendingPaymentDetails::new(duplicate, Vec::new(), Vec::new())) + .await + .unwrap(); + assert_eq!(wallet.find_payment_by_txid(txid2).await.unwrap(), Some(duplicate_id)); + + // Round 2 is recorded as a candidate, with the history of a later round this node signs. + let rounds = vec![ + FundingTxCandidate { + txid: txid1, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(500), + awaiting_broadcast: false, + }, + FundingTxCandidate { + txid: txid2, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(400), + awaiting_broadcast: false, + }, + ]; + let details = interactive_funding_details(funding_id, txid2, Some(1_000_000), Some(400)); + wallet.record_funding_payment(details, rounds).await.unwrap(); + + // One record: the funding record carries the duplicate's confirmation and the confirmed + // candidate's figures; the duplicate and its pending entry are gone, so the round's txid + // resolves to the funding record again. + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the duplicate must be merged away"); + let payment = &payments[0]; + assert_eq!(payment.id, funding_id); + assert_eq!(payment.amount_msat, Some(1_000_000)); + assert_eq!(payment.fee_paid_msat, Some(400)); + match &payment.kind { + PaymentKind::Onchain { + txid, + status: ConfirmationStatus::Confirmed { .. }, + tx_type: Some(TransactionType::InteractiveFunding { .. }), + } => assert_eq!(*txid, txid2), + kind => panic!("unexpected kind {:?}", kind), + } + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_none()); + assert_eq!(wallet.find_payment_by_txid(txid2).await.unwrap(), Some(funding_id)); + } + + /// A duplicate for an *unconfirmed* round carries no state the funding record needs: the + /// merge removes it without touching the record's active txid or figures, and the round's + /// txid maps back to the funding record through its candidate history. + #[tokio::test] + async fn recording_drops_unconfirmed_duplicates_without_adopting_their_txid() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + + let funding_id = PaymentId([21u8; 32]); + let txid1 = Txid::from_byte_array([1u8; 32]); + let txid2 = Txid::from_byte_array([2u8; 32]); + + // Wallet sync saw round 1 — still unconfirmed — before any round was recorded. + let duplicate_id = PaymentId(txid1.to_byte_array()); + let duplicate = PaymentDetails::new( + duplicate_id, + PaymentKind::Onchain { + txid: txid1, + status: ConfirmationStatus::Unconfirmed, + tx_type: None, + }, + Some(999_000), + Some(999), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_stores.payment_store().insert_or_update(duplicate.clone()).await.unwrap(); + wallet + .payment_stores + .pending_payment_store() + .insert_or_update(PendingPaymentDetails::new(duplicate, Vec::new(), Vec::new())) + .await + .unwrap(); + + // Round 2 is the active broadcast; its record lists both rounds. + let rounds = vec![ + FundingTxCandidate { + txid: txid1, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(500), + awaiting_broadcast: false, + }, + FundingTxCandidate { + txid: txid2, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(400), + awaiting_broadcast: false, + }, + ]; + let details = interactive_funding_details(funding_id, txid2, Some(1_000_000), Some(400)); + wallet.record_funding_payment(details, rounds).await.unwrap(); + + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the duplicate must be merged away"); + let payment = &payments[0]; + assert_eq!(payment.id, funding_id); + // The record keeps tracking the actively-broadcast round; a duplicate that never confirmed + // has nothing to adopt. + match &payment.kind { + PaymentKind::Onchain { txid, status: ConfirmationStatus::Unconfirmed, .. } => { + assert_eq!(*txid, txid2) + }, + kind => panic!("unexpected kind {:?}", kind), + } + assert_eq!(payment.fee_paid_msat, Some(400)); + assert_eq!(wallet.find_payment_by_txid(txid1).await.unwrap(), Some(funding_id)); + } + + /// Removing the duplicate is two store writes, and the failure between them must leave a + /// state a re-run of the merge (a replayed `SpliceNegotiated` event) can finish cleaning up. + /// If the payment record went first, a failure on the pending-entry removal would orphan that + /// entry where the re-run can no longer discover it (the record lookup misses), and it would + /// keep shadowing the funding record in `find_payment_by_txid`'s direct probe — re-creating + /// the duplicate problem with no further merge coming to fix it. + #[tokio::test] + async fn a_rerun_merge_completes_a_partially_failed_duplicate_removal() { + let fail_store = FailRemoveStore::new(); + let store: Arc = Arc::new(DynStoreWrapper(fail_store.clone())); + let wallet = new_test_wallet(store, false).await; + + let funding_id = PaymentId([21u8; 32]); + let txid1 = Txid::from_byte_array([1u8; 32]); + let txid2 = Txid::from_byte_array([2u8; 32]); + + // Round 1 recorded normally. + let round1 = vec![FundingTxCandidate { + txid: txid1, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(500), + awaiting_broadcast: false, + }]; + let details = interactive_funding_details(funding_id, txid1, Some(1_000_000), Some(500)); + wallet.record_funding_payment(details, round1).await.unwrap(); + + // Wallet sync recorded round 2's confirmation while the round was not yet a candidate. + let duplicate_id = PaymentId(txid2.to_byte_array()); + let duplicate = PaymentDetails::new( + duplicate_id, + PaymentKind::Onchain { txid: txid2, status: confirmed_status(), tx_type: None }, + Some(999_000), + Some(999), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_stores.payment_store().insert_or_update(duplicate.clone()).await.unwrap(); + wallet + .payment_stores + .pending_payment_store() + .insert_or_update(PendingPaymentDetails::new(duplicate, Vec::new(), Vec::new())) + .await + .unwrap(); + + // Round 2 is recorded as a candidate, but one of the duplicate's two removals fails. + let rounds = vec![ + FundingTxCandidate { + txid: txid1, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(500), + awaiting_broadcast: false, + }, + FundingTxCandidate { + txid: txid2, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(400), + awaiting_broadcast: false, + }, + ]; + let details = interactive_funding_details(funding_id, txid2, Some(1_000_000), Some(400)); + fail_store.fail_next_remove_in(PENDING_PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE); + let res = wallet.record_funding_payment(details.clone(), rounds.clone()).await; + assert!(res.is_err(), "the injected remove failure must surface"); + + // The merge re-runs with the record's next write; it must finish the cleanup. + wallet.record_funding_payment(details, rounds).await.unwrap(); + + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the duplicate must be merged away"); + assert_eq!(payments[0].id, funding_id); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_none()); + assert_eq!(wallet.find_payment_by_txid(txid2).await.unwrap(), Some(funding_id)); + } + + /// Signing a later round merges the duplicates of earlier rounds as a courtesy: the signed + /// round itself can have no duplicate yet, as our signatures have not left the node, and the + /// round's own `SpliceNegotiated` event re-runs the merge, replaying on failure. A merge + /// failure must therefore not fail the signing, whose record is complete once both stores are + /// written, and must not leave the record half rolled back. + #[tokio::test] + async fn signing_survives_a_failed_duplicate_merge() { + let fail_store = FailRemoveStore::new(); + let store: Arc = Arc::new(DynStoreWrapper(fail_store.clone())); + let wallet = new_test_wallet(store, false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + + // Round 1 is recorded at signing; round 2 is a counterparty-initiated replacement the + // wallet observed before it was recorded as a candidate, filed as an untyped duplicate. + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let id = wallet.find_payment_by_txid(txid).await.unwrap().expect("record"); + let (replacement_tx, _) = splice_out_round(&wallet, 2, 500_000, 500); + let replacement_txid = replacement_tx.compute_txid(); + let duplicate_id = PaymentId(replacement_txid.to_byte_array()); + let duplicate = PaymentDetails::new( + duplicate_id, + PaymentKind::Onchain { + txid: replacement_txid, + status: ConfirmationStatus::Unconfirmed, + tx_type: None, + }, + Some(999_000), + Some(999), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_stores.payment_store().insert_or_update(duplicate.clone()).await.unwrap(); + wallet + .payment_stores + .pending_payment_store() + .insert_or_update(PendingPaymentDetails::new(duplicate.clone(), Vec::new(), Vec::new())) + .await + .unwrap(); + + // This node signs round 3, a bump of the replacement, but the duplicate's removal fails. + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 3, 499_000, 700); + let bump_txid = bump_tx.compute_txid(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[ + (txid, Some(contribution)), + (replacement_txid, None), + (bump_txid, Some(bump_contribution)), + ], + ); + fail_store.fail_next_remove_in(PENDING_PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE); + wallet.record_signed_funding(&bump_tx, &bump_candidates).await.unwrap(); + + // The signing is recorded in full and the duplicate is left as it was. + let entry = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + assert_eq!( + entry.candidates().iter().map(|c| c.txid).collect::>(), + vec![txid, replacement_txid, bump_txid] + ); + assert!(entry.candidate(bump_txid).expect("candidate").awaiting_broadcast); + // The signing writes no payment record: the one wallet sync made for the first round + // still describes that round, and the entry still mirrors it. + let payment = + wallet.payment_stores.payment_store().get(&id).await.unwrap().expect("payment"); + assert!(matches!(payment.kind, PaymentKind::Onchain { txid: t, .. } if t == txid)); + assert_eq!(entry.details(), Some(&payment)); + assert_eq!( + wallet.payment_stores.payment_store().get(&duplicate_id).await.unwrap(), + Some(duplicate) + ); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_some()); + + // The bump's `SpliceNegotiated` event merges the duplicate away. + wallet.record_broadcast_splice_round(channel_id, bump_txid).await.unwrap(); + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the duplicate must be merged away"); + assert_eq!(payments[0].id, id); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_none()); + assert_eq!(wallet.find_payment_by_txid(replacement_txid).await.unwrap(), Some(id)); + } + + /// A duplicate merge failing under the `SpliceNegotiated` write must fail that write: the + /// replay it triggers is the merge's only re-run. The mark, cleared before the merge, stays + /// cleared and the duplicate is left as it was; the replayed write finds the round marked + /// already and merges the duplicate away. + #[tokio::test] + async fn a_failed_duplicate_merge_fails_the_negotiation_write_until_its_replay() { + let fail_store = FailRemoveStore::new(); + let store: Arc = Arc::new(DynStoreWrapper(fail_store.clone())); + let wallet = new_test_wallet(store, false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + + // Round 1 is recorded at signing; round 2 is a counterparty-initiated replacement the + // wallet observed before it was recorded as a candidate, filed as an untyped duplicate; + // round 3, a bump this node signs, records round 2 as a candidate, but the signing's + // merge of the duplicate fails and is left to the bump's `SpliceNegotiated` event. + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let id = wallet.find_payment_by_txid(txid).await.unwrap().expect("record"); + let (replacement_tx, _) = splice_out_round(&wallet, 2, 500_000, 500); + let replacement_txid = replacement_tx.compute_txid(); + let duplicate_id = PaymentId(replacement_txid.to_byte_array()); + let duplicate = PaymentDetails::new( + duplicate_id, + PaymentKind::Onchain { + txid: replacement_txid, + status: ConfirmationStatus::Unconfirmed, + tx_type: None, + }, + Some(999_000), + Some(999), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_stores.payment_store().insert_or_update(duplicate.clone()).await.unwrap(); + wallet + .payment_stores + .pending_payment_store() + .insert_or_update(PendingPaymentDetails::new(duplicate.clone(), Vec::new(), Vec::new())) + .await + .unwrap(); + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 3, 499_000, 700); + let bump_txid = bump_tx.compute_txid(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[ + (txid, Some(contribution)), + (replacement_txid, None), + (bump_txid, Some(bump_contribution)), + ], + ); + fail_store.fail_next_remove_in(PENDING_PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE); + wallet.record_signed_funding(&bump_tx, &bump_candidates).await.unwrap(); + + // The event's write meets the same failure: it must surface, so the event is replayed, + // with the mark cleared and the duplicate untouched. + fail_store.fail_next_remove_in(PENDING_PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE); + let res = wallet.record_broadcast_splice_round(channel_id, bump_txid).await; + assert!(res.is_err(), "a failed merge must fail the write"); + let entry = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + assert!(!entry.candidate(bump_txid).expect("candidate").awaiting_broadcast); + assert_eq!( + wallet.payment_stores.payment_store().get(&duplicate_id).await.unwrap(), + Some(duplicate) + ); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_some()); + + // The replayed write finds the round marked already and merges the duplicate away. + wallet.record_broadcast_splice_round(channel_id, bump_txid).await.unwrap(); + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the duplicate must be merged away"); + assert_eq!(payments[0].id, id); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_none()); + assert_eq!(wallet.find_payment_by_txid(replacement_txid).await.unwrap(), Some(id)); + } + + /// A merge cut short between adopting a confirmed duplicate's confirmation and removing the + /// duplicate leaves the funding record confirmed on the duplicate's transaction, the pending + /// entry at its prior status and the duplicate untouched, and a re-run completes the removal: + /// the merge is idempotent, so the record's next write or a replayed `SpliceNegotiated` event + /// can finish what a failure cut short. The failure injected is the pending store's, which the + /// adoption writes after the payment store. + #[tokio::test] + async fn a_torn_duplicate_merge_is_completed_by_a_rerun() { + let fail_store = + FailSwitchStore::failing_only(PENDING_PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE); + let store: Arc = Arc::new(DynStoreWrapper(fail_store.clone())); + let wallet = new_test_wallet(store, false).await; + let (counterparty_node_id, channel_id) = test_counterparty_and_channel(); + + // Round 1 is recorded at signing, round 2 is a counterparty-initiated replacement, and + // round 3 is this node's bump of it, recorded with the channel's history when signed. + let (tx, contribution) = splice_out_round(&wallet, 1, 500_000, 300); + let txid = tx.compute_txid(); + let candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[(txid, Some(contribution.clone()))], + ); + sign_and_observe_round(&wallet, &tx, &candidates).await; + let id = wallet.find_payment_by_txid(txid).await.unwrap().expect("record"); + let (replacement_tx, _) = splice_out_round(&wallet, 2, 500_000, 500); + let replacement_txid = replacement_tx.compute_txid(); + let (bump_tx, bump_contribution) = splice_out_round(&wallet, 3, 499_000, 700); + let bump_txid = bump_tx.compute_txid(); + let bump_candidates = splice_candidates( + counterparty_node_id, + channel_id, + &[ + (txid, Some(contribution)), + (replacement_txid, None), + (bump_txid, Some(bump_contribution)), + ], + ); + wallet.record_signed_funding(&bump_tx, &bump_candidates).await.unwrap(); + + // Wallet sync filed the replacement's confirmation under an untyped record of its own, a + // duplicate of the funding record that already lists the replacement as a candidate. + let duplicate_id = PaymentId(replacement_txid.to_byte_array()); + let duplicate = PaymentDetails::new( + duplicate_id, + PaymentKind::Onchain { + txid: replacement_txid, + status: confirmed_status(), + tx_type: None, + }, + Some(999_000), + Some(999), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_stores.payment_store().insert_or_update(duplicate.clone()).await.unwrap(); + let duplicate_entry = PendingPaymentDetails::new(duplicate.clone(), Vec::new(), Vec::new()); + wallet + .payment_stores + .pending_payment_store() + .insert_or_update(duplicate_entry.clone()) + .await + .unwrap(); + let entry_before = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + let rounds = entry_before.candidates().to_vec(); + + // The merge adopts the confirmation onto the payment record, then fails to mirror it onto + // the pending entry and stops short of removing the duplicate. + fail_store.fail_writes.store(true, Ordering::Release); + { + let guard = wallet.payment_stores.lock().await; + let res = wallet.merge_duplicate_candidate_records(&guard, id, &rounds).await; + assert!(res.is_err(), "the injected pending-store failure must surface"); + } + fail_store.fail_writes.store(false, Ordering::Release); + let payment = + wallet.payment_stores.payment_store().get(&id).await.unwrap().expect("payment"); + assert!(matches!( + payment.kind, + PaymentKind::Onchain { txid: t, status: ConfirmationStatus::Confirmed { .. }, .. } + if t == replacement_txid + )); + assert_eq!( + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap(), + Some(entry_before) + ); + assert_eq!( + wallet.payment_stores.payment_store().get(&duplicate_id).await.unwrap(), + Some(duplicate) + ); + assert_eq!( + wallet.payment_stores.pending_payment_store().get(&duplicate_id).await.unwrap(), + Some(duplicate_entry) + ); + + // A re-run finds the confirmation adopted, mirrors it, and removes the duplicate. + { + let guard = wallet.payment_stores.lock().await; + wallet.merge_duplicate_candidate_records(&guard, id, &rounds).await.unwrap(); + } + let entry = + wallet.payment_stores.pending_payment_store().get(&id).await.unwrap().expect("entry"); + assert_eq!(entry.details(), Some(&payment)); + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1, "the duplicate must be merged away"); + assert_eq!(payments[0].id, id); + assert!(wallet + .payment_stores + .pending_payment_store() + .get(&duplicate_id) + .await + .unwrap() + .is_none()); + assert_eq!(wallet.find_payment_by_txid(replacement_txid).await.unwrap(), Some(id)); + } + #[tokio::test] async fn max_funding_estimate_keeps_reserved_change_address_used() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); From 31992b5a7eed096f8ca373f08338bace1f2d2ee2 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 16 Oct 2026 04:14:50 -0500 Subject: [PATCH 6/6] f - Merge sync-created duplicates when recording a funding round Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/wallet/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 0b0bc22b51..58ad1eeac8 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -5526,6 +5526,14 @@ mod tests { ) } + fn confirmed_status() -> ConfirmationStatus { + ConfirmationStatus::Confirmed { + block_hash: bitcoin::BlockHash::from_byte_array([8u8; 32]), + height: 100, + timestamp: 1, + } + } + /// Inserts `tx` into the BDK wallet as canonically confirmed at `height`, extending the /// local chain to that height. fn insert_confirmed_tx(wallet: &Wallet, tx: Transaction, height: u32) {