From 16b7ed050cf34454956d4f0849d80140a120eced Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 11 Nov 2026 14:55:40 -0600 Subject: [PATCH] Remove the splice intent from pending-payment entries The splice intent was added to pending-payment entries as groundwork for tracking a user-initiated splice from its submission (#1079); the splice entry points that were to write it were part of #1080 and were dropped from it before it merged. As a result, LDK Node never writes the intent. Its one reader, the id resolution at signing time, never finds one, so the field, the types describing it, the merge rule that kept it from being overwritten, and the tests exercising them are dead. Remove them. The first signed round of a splice gets a fresh id, as it already did whenever the intent was absent. TLV type 10 of the entry is left unused; it never reached a release. The tests that set up an intent to assert it was adopted or left alone are dropped or reduced to what they still show: a replayed signing writes nothing, a counterparty's broadcast joins the record under the id the signing chose rather than the transaction's own, and a fee bump joins the record of the round it replaces. Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/payment/pending_payment_store.rs | 348 ++------------------------- src/wallet/mod.rs | 335 +++----------------------- 2 files changed, 51 insertions(+), 632 deletions(-) diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 089e43a96b..f080470e80 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -5,13 +5,9 @@ // 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::secp256k1::PublicKey; -use bitcoin::{TxOut, Txid}; -use lightning::chain::transaction::OutPoint as LdkOutPoint; +use bitcoin::Txid; +use lightning::impl_writeable_tlv_based; 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}; @@ -48,76 +44,15 @@ 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. 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. +/// so all of them are optional. Signing a round of a splice adds the round to `candidates` and +/// lists the `funding_channels` it belongs to, before any 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. @@ -139,11 +74,6 @@ 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. - /// Set when a splice is initiated and cleared once it locks or its failure is surfaced. A fee - /// bump shares the entry of the round it replaces, so the intent outlives any single round - /// negotiated under it. - 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 @@ -155,13 +85,6 @@ 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, @@ -169,20 +92,6 @@ 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(), } } @@ -192,7 +101,6 @@ impl PendingPaymentDetails { /// record for it exists. pub(crate) fn signed_rounds( id: PaymentId, funding_channels: Vec, candidates: Vec, - splice_intent: Option, ) -> Self { Self { id, @@ -200,7 +108,6 @@ impl PendingPaymentDetails { conflicting_txids: Vec::new(), funding_channels, candidates, - splice_intent, locked_rounds: Vec::new(), } } @@ -230,11 +137,6 @@ 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) @@ -263,10 +165,7 @@ impl PendingPaymentDetails { /// Whether this entry tracks nothing anymore and can be dropped. pub(crate) fn is_empty(&self) -> bool { - self.details.is_none() - && self.splice_intent.is_none() - && self.candidates.is_empty() - && self.locked_rounds.is_empty() + self.details.is_none() && self.candidates.is_empty() && self.locked_rounds.is_empty() } } @@ -276,7 +175,7 @@ impl_writeable_tlv_based!(PendingPaymentDetails, { (4, conflicting_txids, optional_vec), (6, funding_channels, optional_vec), (8, candidates, optional_vec), - (10, splice_intent, option), + // Type 10 is unused: it held a splice intent that was dropped before any release wrote it. (12, locked_rounds, optional_vec), }); @@ -286,10 +185,6 @@ 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 { @@ -340,13 +235,6 @@ 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 } @@ -364,14 +252,13 @@ impl StorableObjectUpdate for PendingPaymentDetailsUpdate impl From<&PendingPaymentDetails> for PendingPaymentDetailsUpdate { fn from(value: &PendingPaymentDetails) -> Self { match &value.details { - // An entry without a record yet doesn't carry anything a payment-tracking merge could - // apply beyond its intent, which the entry containing it owns outright. + // For an entry without a record yet, a payment-tracking merge can only apply its + // candidate history. 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() { @@ -379,16 +266,11 @@ 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, } }, } @@ -418,27 +300,6 @@ 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; @@ -482,42 +343,6 @@ pub(crate) fn test_funding_contribution_inheriting( 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"); @@ -526,23 +351,6 @@ pub(crate) fn test_funding_contribution_inheriting( .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; @@ -652,130 +460,9 @@ 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. + // An entry carrying a payment record and its candidate history. let payment_id = PaymentId([7u8; 32]); let txid = Txid::from_byte_array([8u8; 32]); let record = PendingPaymentDetails::new( @@ -812,10 +499,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 and the splice's intent were - /// kept on it reads as an entry with neither, everything else intact. + /// An entry written by a node from before the rounds LDK promoted were kept on it reads as an + /// entry without them, everything else intact. #[test] - fn an_entry_written_without_locked_rounds_or_an_intent_reads_with_neither() { + fn an_entry_written_without_locked_rounds_reads_with_none() { let mut stored = entry(vec![candidate(2, false), candidate(3, true)]); stored.conflicting_txids = vec![test_txid(9)]; @@ -833,9 +520,8 @@ mod tests { let written_before = write_as_before().unwrap(); let decoded: PendingPaymentDetails = Readable::read(&mut &written_before[..]) - .expect("an entry written before the rounds and the intent were kept still reads"); + .expect("an entry written before the rounds 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 77dced3e20..d8ab47b4ff 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -679,8 +679,8 @@ impl Wallet { let pending_payments: Vec = self .payment_stores .pending_payments(|p| match p.details() { - // A pre-broadcast splice intent carries no payment yet and cannot - // graduate. + // An entry whose transaction wallet sync has yet to observe doesn't + // have a payment record yet, so it cannot graduate. None => false, Some(details) => { debug_assert!( @@ -2696,28 +2696,7 @@ impl Wallet { Ok(()) } - /// 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 + /// Resolves the id under which a newly signed 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 @@ -2725,14 +2704,11 @@ impl Wallet { /// 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. - /// A history no live record tracks adopts the id of the channel's splice intent - /// ([`Self::find_splice_payment_id`]), so a user-initiated splice's intent, funding payment and - /// candidate history share one record. Otherwise a fresh id is generated. 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. + /// A history no live record tracks gets a fresh id. 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? { @@ -2745,9 +2721,6 @@ impl Wallet { } } } - if let Some(id) = self.find_splice_payment_id(active).await { - return Ok(id); - } Ok(random_payment_id()) } @@ -2833,7 +2806,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 the channel's splice intent, else a fresh one). + /// 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 @@ -2884,7 +2857,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, signed_round).await?, + None => self.resolve_interactive_funding_id(&stores, candidates).await?, }; let (figures, mut history) = match self.interactive_funding_figures(payment_id, candidates, signed_round, tx) { @@ -2979,7 +2952,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(), None) + PendingPaymentDetails::signed_rounds(payment_id, Vec::new(), Vec::new()) }); if entry.funding_channels.is_empty() && !funding_channels.is_empty() { entry.funding_channels = funding_channels.clone(); @@ -3372,7 +3345,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(), None) + PendingPaymentDetails::signed_rounds(id, Vec::new(), Vec::new()) }); entry.candidates = candidates.clone(); Some(entry) @@ -3592,7 +3565,7 @@ impl Wallet { }, // 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. + // 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; @@ -4609,8 +4582,7 @@ mod tests { PENDING_PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE, }; use crate::payment::pending_payment_store::{ - test_funding_contribution_with_outputs, test_funding_contribution_with_parts, SpliceIntent, - SpliceKind, + test_funding_contribution_with_outputs, test_funding_contribution_with_parts, }; use crate::types::{DynStore, DynStoreWrapper, UserChannelId}; use crate::wallet::provenance::{ @@ -6081,124 +6053,27 @@ 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 { + /// The signing event of a recorded round, replayed, is a no-op: the round is on record, so + /// the store is not written. + #[tokio::test] + async fn a_replayed_signing_writes_nothing() { + 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 (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 (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(); + wallet.record_signed_funding(&tx, &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 @@ -6452,21 +6327,17 @@ 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 }; - 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, &[(txid, Some(contribution))]); wallet.record_signed_funding(&tx, &candidates).await.unwrap(); + let id = wallet + .find_payment_by_txid(txid) + .await + .unwrap() + .expect("the signing identifies the round's payment"); // Our signatures leave the node and the counterparty broadcasts: wallet sync is the first // to see the transaction. @@ -6475,6 +6346,11 @@ mod tests { let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; assert_eq!(payments.len(), 1, "the broadcast must not mint a second record"); assert_eq!(payments[0].id, id); + assert_ne!( + id, + PaymentId(txid.to_byte_array()), + "the record is under the id the signing chose, not the transaction's own" + ); assert!(matches!( payments[0].kind, PaymentKind::Onchain { @@ -6668,148 +6544,14 @@ 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. + /// A fee bump joins the record of the round it replaces: that round is on record, so the + /// history decides the id. #[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( @@ -6818,6 +6560,7 @@ mod tests { &[(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 (bump_tx, bump_contribution) = splice_out_round(&wallet, 2, 499_000, 700); let bump_txid = bump_tx.compute_txid(); @@ -6838,7 +6581,6 @@ mod tests { 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 @@ -8598,7 +8340,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -8817,7 +8558,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -8903,7 +8643,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -8997,7 +8736,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -9083,7 +8821,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -9175,7 +8912,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![bumped_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -9229,7 +8965,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -9297,7 +9032,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![conflict_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap(); @@ -9598,7 +9332,6 @@ mod tests { payment_update: None, conflicting_txids: Some(vec![close_txid]), candidates: Vec::new(), - splice_intent: None, }) .await .unwrap();