From a2086d0e370c3de2bfd7b7d2d4a37eb74cde3b66 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Sep 2026 16:33:24 -0500 Subject: [PATCH 1/6] Abort a splice when funding signing fails The signing handler previously logged and dropped both failure paths (with TODOs to abort once LDK supported it), leaving the negotiation dangling until a peer disconnect abandons it. Cancel the contributed funding instead. LDK then emits DiscardFunding, releasing whatever the wallet reserves for the contribution, and SpliceNegotiationFailed, which surfaces the failure. Cancel errors are only logged: every error case means the splice is already beyond canceling. When LDK refuses the already-signed transaction, the failure report that cancelling produces also drops the round recorded at signing time from its record: the round is gone from the channel's history and can never be broadcast, so left in place the record would wait forever on a payment that can never confirm. Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/event.rs | 52 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/src/event.rs b/src/event.rs index 4008f0701a..13ef82b897 100644 --- a/src/event.rs +++ b/src/event.rs @@ -2567,7 +2567,6 @@ where } } }, - // TODO(splicing): Revisit error handling once splicing API is settled in LDK 0.3 LdkEvent::FundingTransactionReadyForSigning { channel_id, counterparty_node_id, @@ -2609,23 +2608,58 @@ where ); }, Err(e) => { - // Either the round was reset after its history was read above — LDK - // then reports the failure through `SpliceNegotiationFailed`, whose - // handling takes the record back — or LDK rejected the witnesses, in - // which case the round stays pending in LDK, and the record with it. - // TODO(splicing): cancel the contribution here through - // `ChannelManager::cancel_funding_contributed`; a follow-up wires it. + // The signed transaction never reached LDK, so it can never be + // broadcast: cancel the splice. LDK responds with `DiscardFunding` + // (releasing whatever the wallet reserves for the contribution) and + // `SpliceNegotiationFailed` (surfacing the failure and — the round now + // gone from the channel's history — taking back the record written + // above). If LDK had already reset the round when it refused the + // transaction, that report is on its way regardless, and the cancel + // finds nothing left to cancel. log_error!( self.logger, - "LDK refused the signed funding transaction for channel {}: {:?}", + "LDK refused the signed funding transaction for channel {}, \ + aborting the splice: {:?}", channel_id, e, ); + if let Err(e) = self + .channel_manager + .cancel_funding_contributed(&channel_id, &counterparty_node_id) + { + // Every cancel error means the splice is already beyond canceling + // (e.g. the channel is gone); there is nothing further to unwind. + log_error!( + self.logger, + "Failed to cancel the splice on channel {}: {:?}", + channel_id, + e, + ); + } }, } }, Err(()) => { - log_error!(self.logger, "Failed signing funding transaction"); + // No record has been written for this transaction yet, so there is nothing to + // unwind: cancel the splice and let LDK's `DiscardFunding` and + // `SpliceNegotiationFailed` events release the contribution and surface the + // failure. + log_error!( + self.logger, + "Failed signing the funding transaction for channel {}, aborting the splice", + channel_id, + ); + if let Err(e) = self + .channel_manager + .cancel_funding_contributed(&channel_id, &counterparty_node_id) + { + log_error!( + self.logger, + "Failed to cancel the splice on channel {}: {:?}", + channel_id, + e, + ); + } }, }, LdkEvent::SpliceNegotiated { From f42ac6e229555689ef2d25762f6253ed277056d5 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 5 Nov 2026 12:27:15 -0800 Subject: [PATCH 2/6] Add reason and splice parameters to splice failure events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An application handling SpliceNegotiationFailed had nothing to act on: the event did not say why the splice failed, nor what the failed splice would have done. Both matter for deciding what to do next — a splice lost to a disconnect can simply be re-issued, while a failed fee bump leaves the splice it meant to bump in flight, possibly to confirm at the prior feerate. Attach a reason, mapped from LDK's NegotiationFailureReason onto an ldk-node-owned enum so the event's serialization and bindings do not change with LDK's, and the splice's parameters — the amount a splice-in would have added or the outputs a splice-out would have paid — read from the failed contribution LDK reports with the failure. A fee bump rebuilds the contribution of the splice it bumps, so a failed bump describes that splice, less what the higher fee takes from a splice-in of all available funds. Both fields are optional and serialized as odd TLVs: events written by LDK Node v0.7 read back as None, and v0.7 readers ignore the new fields. Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/event.rs | 309 ++++++++++++++++++++++++++++++++++++++++++++++++++- src/lib.rs | 2 +- 2 files changed, 306 insertions(+), 5 deletions(-) diff --git a/src/event.rs b/src/event.rs index 13ef82b897..3d09ec1e56 100644 --- a/src/event.rs +++ b/src/event.rs @@ -13,7 +13,7 @@ use std::sync::{Arc, Mutex}; use bitcoin::blockdata::locktime::absolute::LockTime; use bitcoin::secp256k1::PublicKey; -use bitcoin::{Amount, FeeRate, OutPoint, Txid}; +use bitcoin::{Amount, FeeRate, OutPoint, ScriptBuf, Txid}; use lightning::blinded_path::message::NextMessageHop; use lightning::chain::chaininterface::FundingCandidate; use lightning::chain::transaction::OutPoint as LdkOutPoint; @@ -22,10 +22,12 @@ use lightning::events::bump_transaction::BumpTransactionEvent; use lightning::events::PaidBolt12Invoice; use lightning::events::{ ClosureReason, Event as LdkEvent, FundingInfo, InboundHTLCLocator as LdkInboundHtlcLocator, + NegotiationFailureReason as LdkNegotiationFailureReason, OutboundHTLCLocator as LdkOutboundHtlcLocator, PaymentFailureReason, PaymentPurpose, ReplayEvent, }; use lightning::ln::channelmanager::{FailureCode, PaymentId, TrustedChannelFeatures}; +use lightning::ln::funding::FundingContribution; use lightning::ln::types::ChannelId; use lightning::routing::gossip::NodeId; use lightning::sign::{EntropySource, SpendableOutputDescriptor}; @@ -33,9 +35,13 @@ use lightning::util::config::{ChannelConfigOverrides, ChannelConfigUpdate}; use lightning::util::errors::APIError; use lightning::util::persist::KVStore; use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer}; -use lightning::{impl_writeable_tlv_based, impl_writeable_tlv_based_enum}; +use lightning::{ + impl_writeable_tlv_based, impl_writeable_tlv_based_enum, + impl_writeable_tlv_based_enum_upgradable, +}; use lightning_liquidity::lsps2::utils::compute_opening_fee; use lightning_types::payment::{PaymentHash, PaymentPreimage}; +use lightning_types::string::UntrustedString; use crate::config::{may_announce_channel, Config, PEER_RECONNECTION_INTERVAL}; use crate::connection::ConnectionManager; @@ -121,6 +127,165 @@ impl From for HTLCLocator { } } +/// The reason a channel splice failed. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] +pub enum SpliceFailureReason { + /// The reason was not available. + Unknown, + /// The peer disconnected during negotiation. The splice may be re-initiated once the peer + /// reconnects. + PeerDisconnected, + /// The counterparty explicitly aborted the negotiation. Re-initiating with the same + /// parameters is unlikely to succeed — consider adjusting them or waiting for the + /// counterparty to initiate. + CounterpartyAborted { + /// The counterparty's abort message. + /// + /// This is counterparty-provided data. Use `Display` on [`UntrustedString`] for safe + /// logging. + msg: UntrustedString, + }, + /// An error occurred during interactive transaction negotiation (e.g., the counterparty sent + /// an invalid message). The negotiation was aborted. + NegotiationError { + /// A developer-readable error message. + msg: String, + }, + /// The funding contribution was invalid (e.g., insufficient balance for the splice amount). + /// The splice may be re-initiated with adjusted parameters. + ContributionInvalid, + /// The negotiation was canceled by this node, which it does when it cannot sign the splice's + /// funding transaction or LDK refuses the signed transaction. + LocallyCanceled, + /// The channel is closing, so the negotiation cannot continue. See [`Event::ChannelClosed`] + /// for the closure reason. + ChannelClosing, + /// The contribution's feerate was too low to replace the splice's in-flight funding + /// transaction. The fee bump may be re-initiated once feerates allow it. + FeeRateTooLow, + /// A fee bump could not be initiated (e.g., a prior splice funding transaction already + /// confirmed). The channel remains operational. + CannotInitiateRbf, +} + +impl From for SpliceFailureReason { + fn from(reason: LdkNegotiationFailureReason) -> Self { + match reason { + LdkNegotiationFailureReason::Unknown => Self::Unknown, + LdkNegotiationFailureReason::PeerDisconnected => Self::PeerDisconnected, + LdkNegotiationFailureReason::CounterpartyAborted { msg } => { + Self::CounterpartyAborted { msg } + }, + LdkNegotiationFailureReason::NegotiationError { msg } => Self::NegotiationError { msg }, + LdkNegotiationFailureReason::ContributionInvalid => Self::ContributionInvalid, + LdkNegotiationFailureReason::LocallyCanceled => Self::LocallyCanceled, + LdkNegotiationFailureReason::ChannelClosing => Self::ChannelClosing, + LdkNegotiationFailureReason::FeeRateTooLow => Self::FeeRateTooLow, + LdkNegotiationFailureReason::CannotInitiateRbf => Self::CannotInitiateRbf, + } + } +} + +impl_writeable_tlv_based_enum_upgradable!(SpliceFailureReason, + (1, Unknown) => {}, + (3, PeerDisconnected) => {}, + (5, CounterpartyAborted) => { + (1, msg, required), + }, + (7, NegotiationError) => { + (1, msg, required), + }, + (9, ContributionInvalid) => {}, + (11, LocallyCanceled) => {}, + (13, ChannelClosing) => {}, + (15, FeeRateTooLow) => {}, + (17, CannotInitiateRbf) => {}, +); + +/// An output paid from a channel by a splice-out. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] +pub struct SpliceOutput { + /// The amount paid to the output, in satoshis. + pub amount_sats: u64, + /// The script the output pays to. + pub script_pubkey: ScriptBuf, +} + +impl_writeable_tlv_based!(SpliceOutput, { + (0, amount_sats, required), + (2, script_pubkey, required), +}); + +/// What a splice does with this node's on-chain funds, as asked of the [`Node`] API call that +/// initiated it. A fee bump of a splice's funding transaction rebuilds the splice's contribution +/// at a higher feerate, so a bump describes the splice it bumps: the same outputs, or the same +/// amount, less what the higher fee takes from a splice-in of all available funds. +/// +/// [`Node`]: crate::Node +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] +pub enum SpliceParameters { + /// Funds are added to the channel, via [`Node::splice_in`] or [`Node::splice_in_with_all`]. + /// + /// [`Node::splice_in`]: crate::Node::splice_in + /// [`Node::splice_in_with_all`]: crate::Node::splice_in_with_all + In { + /// The amount added to the channel, in satoshis, net of fees and change. This is what the + /// splice's contribution adds, which can exceed the amount asked of [`Node::splice_in`] by + /// what the wallet reserved for fees beyond the fee the splice pays. For + /// [`Node::splice_in_with_all`], it is the amount the available funds resolved to. + /// + /// [`Node::splice_in`]: crate::Node::splice_in + /// + /// [`Node::splice_in_with_all`]: crate::Node::splice_in_with_all + amount_sats: u64, + }, + /// Funds are removed from the channel, via [`Node::splice_out`] or [`Node::splice_out_max`]. + /// + /// [`Node::splice_out`]: crate::Node::splice_out + /// [`Node::splice_out_max`]: crate::Node::splice_out_max + Out { + /// The outputs paid from the channel. + outputs: Vec, + }, +} + +impl From<&FundingContribution> for SpliceParameters { + /// A contribution adding to the channel is a splice-in, and one adding nothing is a splice-out: + /// this node's splice-outs pay outputs from the channel balance, so they add nothing to it, + /// whereas its splice-ins add to it and pay outputs only to this node's own wallet (the anchor + /// reserve a splice-in of all available funds keeps back), which are not what the splice is + /// for. + fn from(contribution: &FundingContribution) -> Self { + let value_added = contribution.value_added(); + if value_added > Amount::ZERO { + Self::In { amount_sats: value_added.to_sat() } + } else { + Self::Out { + outputs: contribution + .outputs() + .iter() + .map(|o| SpliceOutput { + amount_sats: o.value.to_sat(), + script_pubkey: o.script_pubkey.clone(), + }) + .collect(), + } + } + } +} + +impl_writeable_tlv_based_enum_upgradable!(SpliceParameters, + (1, In) => { + (1, amount_sats, required), + }, + (3, Out) => { + (1, outputs, required_vec), + }, +); + /// An event emitted by [`Node`], which should be handled by the user. /// /// [`Node`]: [`crate::Node`] @@ -317,7 +482,11 @@ pub enum Event { /// The outpoint of the channel's splice funding transaction. new_funding_txo: OutPoint, }, - /// A channel splice negotiation round with local inputs or outputs has failed. + /// A channel splice negotiation round with local inputs or outputs, or a fee bump of a + /// splice's funding transaction, has failed. + /// + /// A failed fee bump leaves the splice it meant to bump unaffected; in particular, the + /// splice's in-flight funding transaction may still confirm. /// /// This event is not emitted when only the counterparty contributes to a splice. SpliceNegotiationFailed { @@ -327,6 +496,16 @@ pub enum Event { user_channel_id: UserChannelId, /// The `node_id` of the channel counterparty. counterparty_node_id: PublicKey, + /// The reason the splice failed. + /// + /// Will be `None` for events serialized by LDK Node v0.7. + reason: Option, + /// What the failed splice would have done with this node's funds. A failed fee bump + /// describes the splice it meant to bump. + /// + /// Will be `None` for events serialized by LDK Node v0.7, and for a failure LDK persisted + /// before an upgrade to this version and reports after it. + parameters: Option, }, } @@ -411,6 +590,8 @@ impl_writeable_tlv_based_enum!(Event, (3, counterparty_node_id, required), (5, user_channel_id, required), // TLV 7 (abandoned_funding_txo) may be set for LDK Node v0.7. + (9, reason, upgradable_option), + (11, parameters, upgradable_option), }, ); @@ -2729,7 +2910,8 @@ where channel_id, user_channel_id, counterparty_node_id, - .. + reason, + contribution, } => { log_info!( self.logger, @@ -2763,10 +2945,14 @@ where } } + let parameters = + contribution.as_ref().map(|c| SpliceParameters::from(c.contribution())); let event = Event::SpliceNegotiationFailed { channel_id, user_channel_id: UserChannelId(user_channel_id), counterparty_node_id, + reason: Some(reason.into()), + parameters, }; match self.event_queue.add_event(event).await { @@ -2992,6 +3178,11 @@ mod tests { claim_from_onchain_tx: bool, outbound_amount_forwarded_msat: Option, }, + SpliceNegotiationFailed { + channel_id: ChannelId, + user_channel_id: UserChannelId, + counterparty_node_id: PublicKey, + }, } impl_writeable_tlv_based_enum!(LegacyEvent, @@ -3009,6 +3200,11 @@ mod tests { (15, prev_htlcs, (default_value_vec, Vec::new())), (17, next_htlcs, (default_value_vec, Vec::new())), }, + (9, SpliceNegotiationFailed) => { + (1, channel_id, required), + (3, counterparty_node_id, required), + (5, user_channel_id, required), + }, ); fn encode_legacy_event_queue(event: LegacyEvent) -> Vec { @@ -3070,6 +3266,111 @@ mod tests { assert!(res.is_err()); } + #[test] + fn event_queue_reads_legacy_splice_negotiation_failed() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + let counterparty_node_id = PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(); + let channel_id = ChannelId([42u8; 32]); + let user_channel_id = UserChannelId(4242); + let legacy_event = LegacyEvent::SpliceNegotiationFailed { + channel_id, + user_channel_id, + counterparty_node_id, + }; + let persisted_bytes = encode_legacy_event_queue(legacy_event); + + let event_queue = + EventQueue::read(&mut &persisted_bytes[..], (Arc::clone(&store), logger)).unwrap(); + assert_eq!( + event_queue.next_event(), + Some(Event::SpliceNegotiationFailed { + channel_id, + user_channel_id, + counterparty_node_id, + reason: None, + parameters: None, + }) + ); + } + + #[tokio::test] + async fn splice_negotiation_failed_round_trips_reason_and_parameters() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + let event_queue = Arc::new(EventQueue::new(Arc::clone(&store), Arc::clone(&logger))); + + let expected_event = Event::SpliceNegotiationFailed { + channel_id: ChannelId([42u8; 32]), + user_channel_id: UserChannelId(4242), + counterparty_node_id: PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(), + reason: Some(SpliceFailureReason::CounterpartyAborted { + msg: UntrustedString("no thanks".to_string()), + }), + parameters: Some(SpliceParameters::Out { + outputs: vec![SpliceOutput { + amount_sats: 10_000, + script_pubkey: ScriptBuf::new(), + }], + }), + }; + event_queue.add_event(expected_event.clone()).await.unwrap(); + + let persisted_bytes = KVStore::read( + &*store, + EVENT_QUEUE_PERSISTENCE_PRIMARY_NAMESPACE, + EVENT_QUEUE_PERSISTENCE_SECONDARY_NAMESPACE, + EVENT_QUEUE_PERSISTENCE_KEY, + ) + .await + .unwrap(); + let deser_event_queue = + EventQueue::read(&mut &persisted_bytes[..], (Arc::clone(&store), logger)).unwrap(); + assert_eq!(deser_event_queue.next_event(), Some(expected_event)); + } + + #[test] + fn legacy_reader_ignores_splice_failure_reason_and_parameters() { + let counterparty_node_id = PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(); + let channel_id = ChannelId([42u8; 32]); + let user_channel_id = UserChannelId(4242); + let event = Event::SpliceNegotiationFailed { + channel_id, + user_channel_id, + counterparty_node_id, + reason: Some(SpliceFailureReason::PeerDisconnected), + parameters: Some(SpliceParameters::In { amount_sats: 10_000 }), + }; + + // The new fields use odd TLVs, so a reader without them — LDK Node v0.7 — must + // still read the event. + let mut bytes = Vec::new(); + 1u16.write(&mut bytes).unwrap(); + event.write(&mut bytes).unwrap(); + + let mut reader = &bytes[..]; + let num_events: u16 = Readable::read(&mut reader).unwrap(); + assert_eq!(num_events, 1); + let legacy_event: LegacyEvent = Readable::read(&mut reader).unwrap(); + assert_eq!( + legacy_event, + LegacyEvent::SpliceNegotiationFailed { + channel_id, + user_channel_id, + counterparty_node_id, + } + ); + } + #[test] fn event_queue_defaults_legacy_missing_forwarded_amount() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); diff --git a/src/lib.rs b/src/lib.rs index 9793081caf..f47c62e538 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -140,7 +140,7 @@ use config::{ use connection::ConnectionManager; pub use error::Error as NodeError; use error::Error; -pub use event::Event; +pub use event::{Event, SpliceFailureReason, SpliceOutput, SpliceParameters}; use event::{EventHandler, EventQueue}; use fee_estimator::{ max_funding_feerate, rbf_splice_feerates, ConfirmationTarget, FeeEstimator, OnchainFeeEstimator, From b26273cf2ded88d023967ffde4be67c722a74517 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 5 Nov 2026 12:33:21 -0800 Subject: [PATCH 3/6] Test splice failure surfacing and recovery A disconnect during the interactive negotiation fails the splice with PeerDisconnected. The first test asserts that exactly one SpliceNegotiationFailed reaches the user -- carrying the reason and the splice-in's amount -- and that a new splice initiated afterwards completes with a single funding payment. The window only exists mid-negotiation: a contribution still queued at disconnect is resumed by LDK itself on reconnect, and one awaiting signatures survives re-establishment. The test therefore synchronizes on the counterparty's splice_ack -- logged by LDK's peer handler -- and stretches the negotiation by funding the splice from many small UTXOs, each of which adds an interactive-tx round trip. A splice dropped by a restart reaches the user through the failure LDK persisted at shutdown and replays at startup -- once, with what the splice would have done. The restart tests cover a dropped splice-out, reported with its output, after which a further restart stays silent; a dropped fee bump, reported with the amount of the splice-in it meant to replace; and a splice queued behind a pending one of this node, reported with its own amount rather than the pending splice's. In each, the application re-initiates and the splice completes. Two more cases guard the payment records around these: a second splice submitted right after a zero-conf lock gets a record of its own rather than the record of the splice that just locked, and a splice confirmed while its node was offline keeps exactly one payment record, under the id recorded at signing, across the restart. The existing test of a signed round the channel's monitor never watched asserts the reason the node surfaces at the force-close, ChannelClosing, and the splice-in's amount. Written with the help of an AI tool. Co-Authored-By: Elias Rohrer Co-Authored-By: Claude Fable 5.1 --- tests/common/logging.rs | 43 +- tests/integration_tests_rust.rs | 712 +++++++++++++++++++++++++++++++- 2 files changed, 741 insertions(+), 14 deletions(-) diff --git a/tests/common/logging.rs b/tests/common/logging.rs index 5e2f2e5dcf..1f667aae4d 100644 --- a/tests/common/logging.rs +++ b/tests/common/logging.rs @@ -197,17 +197,21 @@ impl CollectingLogWriter { self.logs.lock().unwrap().clone() } - /// Waits up to ten seconds for a logged message containing `text`, returning whether one - /// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays - /// the full timeout when the line never comes. + /// Waits up to [`INTEROP_TIMEOUT_SECS`] for a logged message containing `text`, returning + /// whether one arrived. Polling beats a fixed sleep: it returns as soon as the line lands and + /// only pays the full timeout when the line never comes. + /// + /// [`INTEROP_TIMEOUT_SECS`]: super::INTEROP_TIMEOUT_SECS pub(crate) async fn wait_for(&self, text: &str) -> bool { self.wait_for_count(text, 1).await } - /// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning - /// whether they arrived. + /// Waits up to [`INTEROP_TIMEOUT_SECS`] for `occurrences` logged messages containing `text`, + /// returning whether they arrived. + /// + /// [`INTEROP_TIMEOUT_SECS`]: super::INTEROP_TIMEOUT_SECS pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool { - for _ in 0..100 { + for _ in 0..(super::INTEROP_TIMEOUT_SECS * 10) { if self.count(text) >= occurrences { return true; } @@ -222,3 +226,30 @@ impl LogWriter for CollectingLogWriter { self.logs.lock().unwrap().push(record.args.to_string()); } } + +/// Forwards every record to an inner [`CollectingLogWriter`] and signals `seen` when a record +/// contains `marker`. The signal fires from inside the logging call, so a test can react within +/// the emitting code path's timing — where the collector's polling `wait_for` (100ms granularity) +/// is too coarse. +pub(crate) struct MarkerLogWriter { + inner: Arc, + marker: &'static str, + seen: Arc, +} + +impl MarkerLogWriter { + pub(crate) fn new( + inner: Arc, marker: &'static str, seen: Arc, + ) -> Self { + Self { inner, marker, seen } + } +} + +impl LogWriter for MarkerLogWriter { + fn log(&self, record: LogRecord) { + if record.args.to_string().contains(self.marker) { + self.seen.notify_one(); + } + LogWriter::log(&*self.inner, record); + } +} diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index d53c0f3edd..691668c56a 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -21,7 +21,8 @@ use bitcoin::hashes::Hash; use bitcoin::hex::DisplayHex; use bitcoin::{Address, Amount, FeeRate, ScriptBuf, Transaction, Txid}; use common::logging::{ - init_log_logger, validate_log_entry, CollectingLogWriter, MultiNodeLogger, TestLogWriter, + init_log_logger, validate_log_entry, CollectingLogWriter, MarkerLogWriter, MultiNodeLogger, + TestLogWriter, }; use common::{ bump_fee_and_broadcast, configure_chain_source, distribute_funds_unconfirmed, @@ -49,7 +50,10 @@ use ldk_node::payment::{ ConfirmationStatus, ForwardedPaymentId, PayerProofOptions, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus, TransactionType, UnifiedPaymentResult, }; -use ldk_node::{BuildError, Builder, Event, Node, NodeError, ReserveType, UserChannelId}; +use ldk_node::{ + BuildError, Builder, Event, Node, NodeError, ReserveType, SpliceFailureReason, SpliceOutput, + SpliceParameters, UserChannelId, +}; use lightning::chain::channelmonitor::ANTI_REORG_DELAY; use lightning::ln::channelmanager::{PaymentId, BREAKDOWN_TIMEOUT}; use lightning::routing::gossip::{NodeAlias, NodeId}; @@ -60,10 +64,59 @@ use lightning_types::payment::{PaymentHash, PaymentPreimage}; use log::LevelFilter; use serde_json::json; -/// Waits until `node` has recorded the funding broadcast `funding_txid` (a channel open or splice -/// candidate) as a payment carrying a `tx_type`, syncing its wallet until it has. Wallet sync -/// records the payment when it first observes the transaction, so the sync is what settles this, -/// and a chain source can lag the broadcast, so one `sync_wallets` call may not observe it yet. +/// Pops the next event, panicking unless it is a `SpliceNegotiationFailed` from the given +/// counterparty, and returns its reason and parameters. +macro_rules! expect_splice_negotiation_failed_event { + ($node:expr, $counterparty_node_id:expr) => {{ + let event = tokio::time::timeout( + std::time::Duration::from_secs(crate::common::INTEROP_TIMEOUT_SECS), + $node.next_event_async(), + ) + .await + .unwrap_or_else(|_| { + panic!("{} timed out waiting for SpliceNegotiationFailed event", $node.node_id()) + }); + match event { + ref e @ Event::SpliceNegotiationFailed { + counterparty_node_id, + ref reason, + ref parameters, + .. + } => { + println!("{} got event {:?}", $node.node_id(), e); + assert_eq!(counterparty_node_id, $counterparty_node_id); + let reason = reason.clone(); + let parameters = parameters.clone(); + $node.event_handled().unwrap(); + (reason, parameters) + }, + ref e => { + panic!("{} got unexpected event!: {:?}", std::stringify!($node), e); + }, + } + }}; +} + +/// Asserts that `parameters` describe a splice-in of `requested_sats`. The event reports what the +/// contribution adds to the channel, which is the requested amount plus whatever the wallet +/// reserved for fees beyond the fee the splice pays: a sat or so per input, from the two fee +/// estimates rounding differently. +fn assert_splice_in_parameters(parameters: Option, requested_sats: u64) { + match parameters { + Some(SpliceParameters::In { amount_sats }) => assert!( + (requested_sats..requested_sats + 100).contains(&amount_sats), + "expected a splice-in of {} sats, got {}", + requested_sats, + amount_sats, + ), + other => panic!("expected a splice-in of {} sats, got {:?}", requested_sats, other), + } +} + +/// Waits until `node` has recorded the funding transaction `funding_txid` (a channel open or splice +/// round) as a payment carrying a `tx_type`, syncing its wallet until it has. The payment is +/// recorded when wallet sync first observes the transaction, which a sync can run too early for: +/// the chain source may lag the broadcast. async fn wait_for_classified_funding_payment(node: &Node, funding_txid: Txid) { let poll = async { loop { @@ -3124,6 +3177,73 @@ async fn splice_channel() { ); } +/// Two splices of this node in flight on a zero-conf channel — the second submitted right after +/// the first locked — are two payments: the second splice's round is recorded under an id of its +/// own rather than under the first splice's record. +/// +/// Pinned to Esplora, as the sibling below is: both wait for a record of a splice round that is +/// still unconfirmed, and wallet sync creates it from what the wallet has seen. A bitcoind chain +/// source learns an unconfirmed transaction from its mempool poll, which offers a mempool entry +/// to the wallet once, so a round that falls outside that one offer reaches the wallet only when +/// it confirms. +#[cfg(feature = "chain-esplora")] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn zero_conf_queued_splice_is_recorded_as_its_own_payment() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = TestChainSource::Esplora(&electrsd); + + let node_a = setup_node(&chain_source, random_config()); + let mut config_b = random_config(); + config_b.node_config.trusted_peers_0conf.push(node_a.node_id()); + let node_b = setup_node(&chain_source, config_b); + + // Two coins: the second splice-in below cannot spend the first one's unconfirmed change. + let address_a = node_a.onchain_payment().new_address().unwrap(); + let second_address_a = node_a.onchain_payment().new_address().unwrap(); + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a, second_address_a], + Amount::from_sat(5_000_000), + ) + .await; + node_a.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 2_000_000, false, &electrsd).await; + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 1_000_000, None).unwrap(); + let first = expect_splice_negotiated_event!(node_a, node_b.node_id()); + wait_for_classified_funding_payment(&node_a, first.txid).await; + // The zero-conf splice locks without confirmations, re-signaled as `ChannelReady`. + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + node_a.sync_wallets().unwrap(); + + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 1_000_000, None).unwrap(); + let second = expect_splice_negotiated_event!(node_a, node_b.node_id()); + wait_for_classified_funding_payment(&node_a, second.txid).await; + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + let first_payment = funding_payment(&node_a, first.txid); + let second_payment = funding_payment(&node_a, second.txid); + assert_ne!(first_payment.id, second_payment.id, "each splice must have a record of its own"); + for payment in [&first_payment, &second_payment] { + assert!(matches!( + payment.kind, + PaymentKind::Onchain { tx_type: Some(TransactionType::InteractiveFunding { .. }), .. } + )); + } + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn splice_out_max() { do_splice_out_max(false, None, false).await; @@ -4933,7 +5053,7 @@ async fn splice_round_superseded_on_an_open_channel_fails_its_payment() { /// /// LDK reports the round itself after `ChannelClosed`: a `DiscardFunding` for node A's /// contribution, whose handling reclaims its addresses, and a `SpliceNegotiationFailed` the node -/// passes on. +/// reports with reason `ChannelClosing` and the splice-in's amount. #[cfg(feature = "chain-esplora")] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn signed_splice_round_the_monitor_does_not_watch_is_dropped_at_close() { @@ -4963,7 +5083,9 @@ async fn signed_splice_round_the_monitor_does_not_watch_is_dropped_at_close() { node_a.disconnect(node_b.node_id()).unwrap(); node_a.force_close_channel(&user_channel_id_a, node_b.node_id(), None).unwrap(); expect_event!(node_a, ChannelClosed); - expect_event!(node_a, SpliceNegotiationFailed); + let (reason, parameters) = expect_splice_negotiation_failed_event!(node_a, node_b.node_id()); + assert_eq!(reason, Some(SpliceFailureReason::ChannelClosing)); + assert_splice_in_parameters(parameters, 200_000); assert!( logs_a.wait_for_count(RECLAIMED_ADDRESSES, reclaimed_a + 1).await, "node A's contribution to the discarded round was not reclaimed" @@ -5239,6 +5361,580 @@ async fn splice_rounds_discarded_while_the_channel_is_listed_fail_at_close() { node_a.stop().unwrap(); } +/// A mid-negotiation failure is surfaced to the user exactly once: the initiator disconnects +/// while the interactive negotiation is in flight, LDK fails the splice with `PeerDisconnected`, +/// and one `SpliceNegotiationFailed` — carrying the reason and the splice-in's amount — reports +/// it. The splice is not retried automatically; the application initiates a +/// new one, which completes. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn splice_failure_surfaced_after_disconnect_mid_negotiation() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // The negotiation is synchronized through a log marker: LDK's peer handler logs every received + // message, and the counterparty's `splice_ack` is the earliest point where a disconnect fails + // the splice — any sooner and the contribution is still queued, which LDK resumes on reconnect + // by itself and no failure occurs. + let logger_a = Arc::new(CollectingLogWriter::new()); + let splice_ack_seen = Arc::new(tokio::sync::Notify::new()); + let mut config_a = random_config(); + config_a.log_writer = TestLogWriter::Custom(Arc::new(MarkerLogWriter::new( + logger_a.clone(), + "Received message SpliceAck", + splice_ack_seen.clone(), + ))); + // `Node::disconnect` persists a peer-store removal before severing the connection, and the + // negotiation keeps running during that write. The default composite test store turns it into + // several fsyncs plus a cross-store comparison, wide enough to lose the race below; a plain + // SQLite store keeps it to a single quick write. + config_a.store_type = TestStoreType::Sqlite; + let node_a = setup_node(&chain_source, config_a); + let node_b = setup_node(&chain_source, random_config()); + + // Fund Node A with many small UTXOs: every input the splice contributes adds an interactive-tx + // round trip, stretching the negotiation so the disconnect below reliably lands inside it. + let addresses_a: Vec
= + (0..240).map(|_| node_a.onchain_payment().new_address().unwrap()).collect(); + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + addresses_a, + Amount::from_sat(32_000), + ) + .await; + node_a.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 1_000_000, false, &electrsd).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // The 3M target forces roughly 95 of the 32k-sat UTXOs into the contribution. + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 3_000_000, None).unwrap(); + + // Disconnect as soon as the negotiation is in flight. The negotiation keeps running while the + // disconnect is processed — `Node::disconnect` first persists a peer-store removal on the + // node's own runtime, which the negotiation is keeping busy — so in principle the negotiation + // could still complete first, the disconnect would then not fail the splice (a signed round is + // resumed on reconnect rather than failed) and the failure-event assert below would trip. The + // ~95 remaining per-input round trips make that window wide enough to survive a heavily loaded + // machine; if this ever flakes, widen the contribution further. + tokio::time::timeout(std::time::Duration::from_secs(10), splice_ack_seen.notified()) + .await + .expect("node A never received splice_ack"); + node_a.disconnect(node_b.node_id()).unwrap(); + + // ... which fails it with `PeerDisconnected`. The failure is surfaced with the reason and the + // splice-in's amount, and is not retried automatically. + let (reason, parameters) = expect_splice_negotiation_failed_event!(node_a, node_b.node_id()); + assert_eq!(reason, Some(SpliceFailureReason::PeerDisconnected)); + assert_splice_in_parameters(parameters, 3_000_000); + + let node_addr_b = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.connect(node_b.node_id(), node_addr_b, false).unwrap(); + + // The failed splice's inputs were released; the application initiates a new splice, which + // completes. A second copy of the failure event would pop here instead and panic: the failure + // is reported exactly once. + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 3_000_000, None).unwrap(); + let txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + + wait_for_classified_funding_payment(&node_a, txo.txid).await; + wait_for_tx(&electrsd.client, txo.txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + let payment = funding_payment(&node_a, txo.txid); + assert_eq!(payment.status, PaymentStatus::Succeeded); + assert!(matches!( + payment.kind, + PaymentKind::Onchain { + status: ConfirmationStatus::Confirmed { .. }, + tx_type: Some(TransactionType::InteractiveFunding { .. }), + .. + } + )); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + +/// A splice LDK dropped without ever persisting it — initiated while disconnected, then the node +/// restarts — is reported once, through the failure LDK itself persisted at shutdown and replays +/// at startup: `PeerDisconnected`, with the output the splice-out would have paid. A further +/// restart stays silent, and a new splice initiated by the application completes. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn splice_loss_surfaced_after_restart() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // Set up node_a manually so it can be restarted with the same config. + let mut config_a = random_config(); + config_a.store_type = TestStoreType::Sqlite; + let config_b = random_config(); + let node_b = setup_node(&chain_source, config_b); + + let (onchain_balance_before_sat, splice_out_address, user_channel_id_a) = { + let node_a = setup_node(&chain_source, config_a.clone()); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + let address_b = node_b.onchain_payment().new_address().unwrap(); + let premine_amount_sat = 5_000_000; + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a, address_b], + Amount::from_sat(premine_amount_sat), + ) + .await; + + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Initiate a splice-out while disconnected: LDK accepts the contribution but cannot make + // progress before the restart below drops it, having neither negotiated nor persisted + // the splice itself — only the failure event it queues for it at shutdown. + node_a.disconnect(node_b.node_id()).unwrap(); + let address = node_a.onchain_payment().new_address().unwrap(); + node_a.splice_out(&user_channel_id_a, node_b.node_id(), &address, 500_000, None).unwrap(); + + let onchain_balance_before_sat = node_a.list_balances().total_onchain_balance_sats; + node_a.stop().unwrap(); + (onchain_balance_before_sat, address, user_channel_id_a) + }; + + // The one failure surfaced is LDK's replay of the event it persisted at shutdown for the + // dropped contribution, describing the splice-out it was for. + let node_a = setup_node(&chain_source, config_a.clone()); + node_a.sync_wallets().unwrap(); + + let (reason, parameters) = expect_splice_negotiation_failed_event!(node_a, node_b.node_id()); + assert_eq!(reason, Some(SpliceFailureReason::PeerDisconnected)); + assert_eq!( + parameters, + Some(SpliceParameters::Out { + outputs: vec![SpliceOutput { + amount_sats: 500_000, + script_pubkey: splice_out_address.script_pubkey(), + }], + }) + ); + + // The replayed failure was consumed, so another restart must not report it again. + node_a.stop().unwrap(); + let node_a = setup_node(&chain_source, config_a); + node_a.sync_wallets().unwrap(); + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + assert!(node_a.next_event().is_none(), "a consumed splice failure must not be reported again"); + + // The application initiates a new splice-out, which completes. + let node_b_addr = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.connect(node_b.node_id(), node_b_addr, false).unwrap(); + node_a + .splice_out(&user_channel_id_a, node_b.node_id(), &splice_out_address, 500_000, None) + .unwrap(); + + let txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + + wait_for_tx(&electrsd.client, txo.txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + assert!( + node_a.list_balances().total_onchain_balance_sats > onchain_balance_before_sat + 400_000, + "the new splice-out should have moved ~500k sats to the on-chain balance", + ); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + +/// A splice-in of all available funds keeps the anchor reserve back as an output to this node's +/// own wallet. Its failure is still reported as a splice-in, of the amount the contribution adds, +/// not as a splice-out paying that reserve output. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn splice_in_with_all_failure_reports_the_amount_added() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // Set up node_a manually so it can be restarted with the same config. + let mut config_a = random_config(); + config_a.store_type = TestStoreType::Sqlite; + let node_b = setup_node(&chain_source, random_config()); + + let onchain_balance_sats = { + let node_a = setup_node(&chain_source, config_a.clone()); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + let address_b = node_b.onchain_payment().new_address().unwrap(); + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a, address_b], + Amount::from_sat(5_000_000), + ) + .await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + // An anchor channel, so that the splice-in of all funds below keeps the anchor reserve. + open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Initiate the splice while disconnected and restart before it is negotiated: LDK drops + // the contribution and replays the failure it persisted for it at shutdown. + let onchain_balance_sats = node_a.list_balances().total_onchain_balance_sats; + node_a.disconnect(node_b.node_id()).unwrap(); + node_a.splice_in_with_all(&user_channel_id_a, node_b.node_id(), None).unwrap(); + node_a.stop().unwrap(); + onchain_balance_sats + }; + + let node_a = setup_node(&chain_source, config_a); + node_a.sync_wallets().unwrap(); + let (reason, parameters) = expect_splice_negotiation_failed_event!(node_a, node_b.node_id()); + assert_eq!(reason, Some(SpliceFailureReason::PeerDisconnected)); + match parameters { + Some(SpliceParameters::In { amount_sats }) => assert!( + amount_sats > 0 && amount_sats < onchain_balance_sats, + "a splice-in of all funds adds less than the {} sats on hand, got {}", + onchain_balance_sats, + amount_sats, + ), + other => panic!("expected a splice-in, got {:?}", other), + } + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + +/// A fee bump initiated while disconnected and dropped by a restart leaves LDK tracking the +/// negotiated splice at the original feerate. The failure LDK persisted at shutdown for the +/// dropped bump is replayed at startup and describes the splice the bump meant to replace. A new +/// bump initiated by the application replaces the funding transaction, and once the negotiated +/// splice carries the bump, further restarts stay silent. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn splice_rbf_loss_surfaced_after_restart() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd_for_splice_rbf(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // Set up node_a manually so it can be restarted with the same config. + let mut config_a = random_config(); + config_a.store_type = TestStoreType::Sqlite; + let config_b = random_config(); + let node_b = setup_node(&chain_source, config_b); + + let (original_txo, user_channel_id_a) = { + let node_a = setup_node(&chain_source, config_a.clone()); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + let address_b = node_b.onchain_payment().new_address().unwrap(); + let premine_amount_sat = 5_000_000; + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a, address_b], + Amount::from_sat(premine_amount_sat), + ) + .await; + + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Negotiate a splice but leave its transaction unconfirmed so it can be fee-bumped. + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 500_000, None).unwrap(); + let original_txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + wait_for_tx(&electrsd.client, original_txo.txid).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + // Bump the fee while disconnected and restart before anything could be negotiated: LDK + // drops the queued bump, keeping the negotiated splice at the original feerate. + node_a.disconnect(node_b.node_id()).unwrap(); + node_a.bump_channel_funding_fee(&user_channel_id_a, node_b.node_id(), None).unwrap(); + node_a.stop().unwrap(); + (original_txo, user_channel_id_a) + }; + + // On restart, the failure LDK persisted at shutdown for the dropped bump is replayed. The bump + // rebuilt the splice-in's contribution at a higher feerate, so the failure reports the + // splice-in's amount. + let node_a = setup_node(&chain_source, config_a.clone()); + node_a.sync_wallets().unwrap(); + + let (reason, parameters) = expect_splice_negotiation_failed_event!(node_a, node_b.node_id()); + assert_eq!(reason, Some(SpliceFailureReason::PeerDisconnected)); + assert_splice_in_parameters(parameters, 500_000); + + // The application initiates a new fee bump, which replaces the funding transaction. + let node_b_addr = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.connect(node_b.node_id(), node_b_addr.clone(), false).unwrap(); + node_a.bump_channel_funding_fee(&user_channel_id_a, node_b.node_id(), None).unwrap(); + + let rbf_txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + assert_ne!(original_txo, rbf_txo, "the new fee bump should produce a different funding txo"); + + // Restarting again must stay silent: the negotiated splice now carries the bump at the + // intended feerate. + node_a.stop().unwrap(); + let node_a = setup_node(&chain_source, config_a.clone()); + node_a.sync_wallets().unwrap(); + node_a.connect(node_b.node_id(), node_b_addr.clone(), false).unwrap(); + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + assert!(node_a.next_event().is_none(), "a carried fee bump must not be reported as lost"); + + wait_for_tx(&electrsd.client, rbf_txo.txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // A locked splice leaves nothing to fail, so a further restart must stay silent. + node_a.stop().unwrap(); + let node_a = setup_node(&chain_source, config_a); + node_a.sync_wallets().unwrap(); + node_a.connect(node_b.node_id(), node_b_addr, false).unwrap(); + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + assert!(node_a.next_event().is_none(), "a locked fee bump must produce no events"); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + +/// A splice queued behind a pending splice of this node on a confirmed channel — accepted once +/// the pending round has a confirmation — is a splice of its own: a restart fails the queued +/// contribution LDK never got to negotiate, and the replayed failure carries the queued splice's +/// amount, not the pending one's. The pending splice's payment, which graduated before the +/// restart, stays that splice's one record through its lock. +/// +/// Pinned to Esplora so the nodes sync only when told to: the pending splice's lock needs the +/// counterparty's `splice_locked`, which it sends only once it has seen the confirmations. +#[cfg(feature = "chain-esplora")] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn queued_splice_failure_surfaced_after_restart() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = TestChainSource::Esplora(&electrsd); + + // Set up node_a manually so it can be restarted with the same config. + let mut config_a = random_config(); + config_a.store_type = TestStoreType::Sqlite; + let node_b = setup_node(&chain_source, random_config()); + + let (pending_txid, pending_payment_id, node_b_addr) = { + let node_a = setup_node(&chain_source, config_a.clone()); + + // Two coins for node_a: the queued splice-in cannot spend the pending one's unconfirmed + // change. + let address_a = node_a.onchain_payment().new_address().unwrap(); + let second_address_a = node_a.onchain_payment().new_address().unwrap(); + let address_b = node_b.onchain_payment().new_address().unwrap(); + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a, second_address_a, address_b], + Amount::from_sat(5_000_000), + ) + .await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 500_000, None).unwrap(); + let pending = expect_splice_negotiated_event!(node_a, node_b.node_id()); + wait_for_classified_funding_payment(&node_a, pending.txid).await; + + // With one confirmation, seen by node_a alone, LDK takes a further splice-in as a splice + // of its own, queued until the pending one locks. Queueing it starts a quiescence + // handshake LDK breaks off with a warning until then, disconnecting the peers. + wait_for_tx(&electrsd.client, pending.txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 1).await; + node_a.sync_wallets().unwrap(); + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 300_000, None).unwrap(); + + // Five more confirmations graduate the pending splice's payment on node_a, removing its + // record, while its lock still waits on node_b. + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 5).await; + node_a.sync_wallets().unwrap(); + let pending_payment = funding_payment(&node_a, pending.txid); + assert_eq!(pending_payment.status, PaymentStatus::Succeeded); + + let node_b_addr = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.stop().unwrap(); + (pending.txid, pending_payment.id, node_b_addr) + }; + + // LDK failed the queued contribution when it was last persisted and replays the failure at + // startup, with the queued splice's amount. + let node_a = setup_node(&chain_source, config_a); + node_a.sync_wallets().unwrap(); + let (reason, parameters) = expect_splice_negotiation_failed_event!(node_a, node_b.node_id()); + assert_eq!(reason, Some(SpliceFailureReason::PeerDisconnected)); + assert_splice_in_parameters(parameters, 300_000); + + // The pending splice locks once node_b catches up, under its one record: the one that + // graduated before the restart, not a second one the lock or the sync created. + node_a.connect(node_b.node_id(), node_b_addr, false).unwrap(); + node_b.sync_wallets().unwrap(); + node_a.sync_wallets().unwrap(); + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + let pending_payments = node_a.list_payments_matching( + |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == pending_txid), + ); + assert_eq!(pending_payments.len(), 1); + assert_eq!(pending_payments[0].id, pending_payment_id); + assert_eq!(pending_payments[0].status, PaymentStatus::Succeeded); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + +/// A splice confirmed while its node was offline keeps exactly one payment record, under the id +/// recorded when the round was signed, across the restart, whether or not wallet sync had +/// observed the transaction before the node stopped. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn splice_payment_tracked_across_restart_before_lock() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // Set up node_a manually so it can be restarted with the same config. + let mut config_a = random_config(); + config_a.store_type = TestStoreType::Sqlite; + let config_b = random_config(); + let node_b = setup_node(&chain_source, config_b); + + let splice_txid = { + let node_a = setup_node(&chain_source, config_a.clone()); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + let address_b = node_b.onchain_payment().new_address().unwrap(); + let premine_amount_sat = 5_000_000; + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a, address_b], + Amount::from_sat(premine_amount_sat), + ) + .await; + + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 500_000, None).unwrap(); + let txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + + // Stop node_a as soon as the splice is negotiated. node_b broadcasts the transaction + // either way, so it reaches the chain while node_a is offline. node_a recorded the round + // when it signed the funding transaction; depending on timing, its wallet sync may or may + // not also have observed the transaction before stopping — the assertions below must hold + // in both cases. + node_a.stop().unwrap(); + txo.txid + }; + + // Confirm the splice while node_a is offline, but keep it short of the depth at which it + // locks, so node_a restarts with the splice still pending. + wait_for_tx(&electrsd.client, splice_txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 1).await; + + // After the restart, wallet sync must file the confirmed transaction under the `PaymentId` + // recorded when the round was signed, whether or not it had observed the transaction before + // the stop: exactly one payment record, and not one keyed by a txid-derived id. + let node_a = setup_node(&chain_source, config_a); + node_a.sync_wallets().unwrap(); + + let splice_payments = |node: &Node| { + node.list_payments_matching( + |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == splice_txid), + ) + }; + let payments = splice_payments(&node_a); + assert_eq!( + payments.len(), + 1, + "expected exactly one payment record for the splice, got {}: {:#?}", + payments.len(), + payments, + ); + assert_ne!( + payments[0].id, + PaymentId(splice_txid.to_byte_array()), + "the splice payment must keep the id recorded at signing, not a txid-derived fallback", + ); + assert_eq!(payments[0].status, PaymentStatus::Pending); + + // Reconnect and let the splice lock: the single record graduates instead of gaining a + // duplicate. + let node_b_addr = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.connect(node_b.node_id(), node_b_addr, false).unwrap(); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 5).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + let payments = splice_payments(&node_a); + assert_eq!( + payments.len(), + 1, + "expected exactly one payment record after the splice locked, got {}: {:#?}", + payments.len(), + payments, + ); + assert_eq!(payments[0].status, PaymentStatus::Succeeded); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn simple_bolt12_send_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); From 520618407ec7a9db9e5a2bd83898801fe98ce415 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 30 Sep 2026 16:42:34 -0500 Subject: [PATCH 4/6] Refuse to sign a round this node cannot measure Recording what a signed interactive funding round is, and what this node's share of it comes to, can be refused for want of room, and the signing went ahead regardless. The transaction was then released without this node's contribution on record, so whoever first observed it recorded the wallet's view of a funding output both parties own: the whole of it read as this node's spend, a figure that is never corrected. The round's facts are now admitted however many records the store contains, as is the funding transaction this node builds for a channel it opens. The cap bounds what a counterparty drives by opening and closing channels or replacing a negotiated funding; a round this node chose to sign is not that, and its record describes the round and this node's share of it while carrying no outputs. What can still refuse the round is a record already as large as one record may be, which never shrinks, so the round is reported unmeasurable and the handler cancels the splice, as it already does where LDK refuses the signed transaction. The failure is then the splice's rather than the node's, and the round is never released with the wallet's view of a funding output both parties own standing in for this node's share. Failing the signing for the handler to replay would not do: LDK stops handling events at the first failure and leaves the failing one at the head of its queue, so everything behind it waits, claims on inbound HTLCs among them, while the same pass flags the channel manager for persistence and notifies a waiter that re-enters at once. A full store is not a condition a replay clears: room comes only from the dropping pass, which wants a record a year old whose channels have all resolved. The commit introducing the refusal said its cost was a transaction reported without a classification. That holds for a producer reporting on something it has already released, which still only logs, and the places restating it now distinguish the two. Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/config.rs | 7 +- src/event.rs | 124 ++++++++------- src/wallet/mod.rs | 323 +++++++++++++++++++++++++++++++++++---- src/wallet/provenance.rs | 32 +++- 4 files changed, 401 insertions(+), 85 deletions(-) diff --git a/src/config.rs b/src/config.rs index d524eac58e..8d7344418d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -104,8 +104,11 @@ pub(crate) const CHANNEL_TX_FACTS_MAX_RECORD_BYTES: usize = 128 * 1024; // Records are dropped only once the channels they belong to have resolved, so between prunes a // counterparty opening and closing channels, or replacing a negotiated funding again and again, // drives the store's growth. Past this many records the store is full and the transactions -// it would have described go unclassified, which is bounded loss of detail rather than unbounded -// storage. +// it would have described go unclassified, which is bounded loss of detail rather than +// unbounded storage. An interactive funding round this node is about to sign, and the funding +// transaction it builds for a channel it opens, are admitted beside them: how many of those there +// are is this node's own decision, and refusing one would cost a figure that is never corrected +// rather than a detail. pub(crate) const CHANNEL_TX_FACTS_MAX_RECORDS: usize = 100_000; // The number of pages of channel transaction provenance records one chain tip change examines. diff --git a/src/event.rs b/src/event.rs index 3d09ec1e56..12be27f5aa 100644 --- a/src/event.rs +++ b/src/event.rs @@ -70,7 +70,9 @@ use crate::types::{ Wallet, }; use crate::wallet::provenance::{ChannelOutputRole, ChannelTxFacts, FactsRecordOutcome}; -use crate::wallet::{closed_channel_tracked_rounds, funding_candidates, tracked_splice_rounds}; +use crate::wallet::{ + closed_channel_tracked_rounds, funding_candidates, tracked_splice_rounds, SignedFundingRecord, +}; use crate::{ hex_utils, BumpTransactionEventHandler, ChannelManager, Error, Graph, PeerInfo, PeerStore, UserChannelId, @@ -938,6 +940,27 @@ where Ok((payment_id, None)) } + /// Cancels a splice whose funding transaction this node will not sign, so that LDK releases + /// what it reserved for this node's contribution through `DiscardFunding` and surfaces the + /// failure through `SpliceNegotiationFailed`, which also takes back the round's record once + /// the round is gone from the channel's history. + /// + /// A refusal means the splice is already beyond cancelling — LDK reset the round itself, or + /// the channel is gone — in which case those reports are on their way regardless and there is + /// nothing further to unwind. + fn cancel_splice(&self, counterparty_node_id: PublicKey, channel_id: ChannelId) { + if let Err(e) = + self.channel_manager.cancel_funding_contributed(&channel_id, &counterparty_node_id) + { + log_error!( + self.logger, + "Failed to cancel the splice on channel {}: {:?}", + channel_id, + e, + ); + } + } + /// The channel's pending splice rounds that have a transaction, as LDK currently tracks them. fn pending_splice_rounds( &self, counterparty_node_id: PublicKey, channel_id: ChannelId, @@ -968,18 +991,23 @@ where /// Records what one of this node's channels reported about a transaction it produced, and /// classifies the transaction's payment record from it if wallet sync wrote that record first. /// - /// A failure is logged rather than reported: these facts accompany a transaction this node - /// has already released or a claim it has already made, so there is nothing left to withhold, - /// and the producing event is re-offered until the claim resolves. + /// A failure is logged rather than reported: these facts accompany a transaction this node has + /// already released or a claim it has already made, so there is nothing left to withhold, and + /// the producing event is re-offered until the claim resolves. A refusal for want of room costs + /// a transaction reported without a classification, which this node likewise cannot take back. async fn record_channel_tx_facts(&self, facts: ChannelTxFacts) { let txid = facts.txid; match self.wallet.record_channel_tx_facts(facts).await { Ok(FactsRecordOutcome::Recorded) => { self.wallet.classify_recorded_transaction(txid).await }, - // Refused for lack of room, which the wallet has logged. The report was not recorded, - // so the transaction cannot be classified from it. - Ok(FactsRecordOutcome::Incomplete) => {}, + Ok(FactsRecordOutcome::Incomplete) => { + log_error!( + self.logger, + "Reporting transaction {} without a classification: this node has no room to describe it", + txid, + ); + }, Err(e) => { log_error!( self.logger, @@ -1052,15 +1080,17 @@ where ChannelOutputRole::Funding, [vout as u32], ); - match self.wallet.record_channel_tx_facts(facts).await { + match self.wallet.record_own_funding_tx_facts(facts).await { Ok(FactsRecordOutcome::Recorded) => {}, - // Replaying would rebuild the same transaction and find the same - // full store, so the channel is funded with a transaction this - // node will report without a classification. + // Admitted however full the store is, the report can only be + // refused as too large a record, which one funding output does + // not make. Replaying would meet the same refusal, so the + // channel is funded with a transaction this node will report + // without a classification. Ok(FactsRecordOutcome::Incomplete) => { log_error!( self.logger, - "Funding channel {} with a transaction this node has no room to describe", + "Funding channel {} with a transaction this node could not describe", temporary_channel_id, ); }, @@ -2764,16 +2794,37 @@ where // re-offers the event in-session and regenerates it across restarts while the // transaction is unsigned. let candidates = self.pending_splice_rounds(counterparty_node_id, channel_id); - if let Err(e) = - self.wallet.record_signed_funding(&partially_signed_tx, &candidates).await + let record = match self + .wallet + .record_signed_funding(&partially_signed_tx, &candidates) + .await { + Ok(record) => record, + Err(e) => { + log_error!( + self.logger, + "Failed to record the signed splice round of channel {}: {}", + channel_id, + e, + ); + return Err(ReplayEvent()); + }, + }; + if record == SignedFundingRecord::Unmeasurable { + // The round was not recorded and our signatures have not left the node, so + // the splice is cancelled rather than replayed. A replay would meet the + // same refusal, and an event that fails every time it is offered holds back + // every event queued behind it — including the ones that claim inbound + // HTLCs before they expire. Bounding the loss to this splice is the cheaper + // failure. log_error!( self.logger, - "Failed to record the signed splice round for channel {}: {}", + "Not signing the funding transaction for channel {}, aborting the \ + splice: this node's share of the round is not on record", channel_id, - e, ); - return Err(ReplayEvent()); + self.cancel_splice(counterparty_node_id, channel_id); + return Ok(()); } match self.channel_manager.funding_transaction_signed( &channel_id, @@ -2790,13 +2841,8 @@ where }, Err(e) => { // The signed transaction never reached LDK, so it can never be - // broadcast: cancel the splice. LDK responds with `DiscardFunding` - // (releasing whatever the wallet reserves for the contribution) and - // `SpliceNegotiationFailed` (surfacing the failure and — the round now - // gone from the channel's history — taking back the record written - // above). If LDK had already reset the round when it refused the - // transaction, that report is on its way regardless, and the cancel - // finds nothing left to cancel. + // broadcast: cancel the splice, which also takes back the record + // written above. log_error!( self.logger, "LDK refused the signed funding transaction for channel {}, \ @@ -2804,43 +2850,19 @@ where channel_id, e, ); - if let Err(e) = self - .channel_manager - .cancel_funding_contributed(&channel_id, &counterparty_node_id) - { - // Every cancel error means the splice is already beyond canceling - // (e.g. the channel is gone); there is nothing further to unwind. - log_error!( - self.logger, - "Failed to cancel the splice on channel {}: {:?}", - channel_id, - e, - ); - } + self.cancel_splice(counterparty_node_id, channel_id); }, } }, Err(()) => { // No record has been written for this transaction yet, so there is nothing to - // unwind: cancel the splice and let LDK's `DiscardFunding` and - // `SpliceNegotiationFailed` events release the contribution and surface the - // failure. + // unwind: cancel the splice. log_error!( self.logger, "Failed signing the funding transaction for channel {}, aborting the splice", channel_id, ); - if let Err(e) = self - .channel_manager - .cancel_funding_contributed(&channel_id, &counterparty_node_id) - { - log_error!( - self.logger, - "Failed to cancel the splice on channel {}: {:?}", - channel_id, - e, - ); - } + self.cancel_splice(counterparty_node_id, channel_id); }, }, LdkEvent::SpliceNegotiated { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index c5fc5c80ee..44b612d31a 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -77,8 +77,8 @@ use crate::payment::{ use crate::runtime::Runtime; use crate::types::{Broadcaster, ChannelTxFactsStore, PaymentStore, PendingPaymentStore}; use crate::wallet::provenance::{ - ChannelLiveness, ChannelTxFacts, ChannelTxFactsRejection, FactsRecordOutcome, FactsRetention, - LocalFundingFigures, RetentionCheck, TxProvenance, + ChannelLiveness, ChannelTxFacts, ChannelTxFactsRejection, FactsAdmission, FactsRecordOutcome, + FactsRetention, LocalFundingFigures, RetentionCheck, TxProvenance, }; use crate::{ChainSource, Error}; @@ -93,6 +93,20 @@ pub(crate) enum FundingAmount { Max, } +/// What recording an interactive funding round this node is about to sign came to, as it decides +/// whether the round may be signed. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SignedFundingRecord { + /// This node's share of the round is on record, or the round has nothing to record. The round + /// may be signed. + Recorded, + /// This node's share of the round is not on record, and this node will not put it there later. + /// Signing would release a transaction whoever first observes it records with the wallet's view + /// of a funding output both parties own — the whole of it read as this node's spend, a figure + /// that is never corrected — so the round is cancelled instead. + Unmeasurable, +} + mod payment_stores; pub(crate) mod persist; pub(crate) mod provenance; @@ -270,12 +284,33 @@ impl Wallet { /// overwriting it: one of the two producers is wrong, and the recorded facts came first. /// /// A report the store has no room for is likewise refused, and reported as - /// [`FactsRecordOutcome::Incomplete`] rather than as a failure: there is nothing to retry, - /// and the consequence is a transaction this node cannot say anything about, not a lost - /// write. Only what this node has no record of at all is refused that way — a transaction it + /// [`FactsRecordOutcome::Incomplete`] rather than as a failure: the store is as it was, and + /// what the refusal costs the reporting producer is a transaction this node cannot say anything + /// about. Only what this node has no record of at all is refused that way — a transaction it /// already describes goes on being described, however full the store is. pub(crate) async fn record_channel_tx_facts( &self, facts: ChannelTxFacts, + ) -> Result { + self.record_channel_tx_facts_admitted(facts, FactsAdmission::Capped).await + } + + /// Records the funding transaction this node built for a channel it is opening, as + /// [`Self::record_channel_tx_facts`] does, admitted however many records the store contains. + /// + /// The cap bounds what a counterparty drives; a channel this node chose to fund is not that, + /// and without its funding output on record the transaction and every transaction spending + /// it, the channel's close among them, are reported without a classification. + pub(crate) async fn record_own_funding_tx_facts( + &self, facts: ChannelTxFacts, + ) -> Result { + self.record_channel_tx_facts_admitted(facts, FactsAdmission::Exempt).await + } + + /// Records what a producer reported, as [`Self::record_channel_tx_facts`] does, with + /// `admission` deciding whether the number of records the store may contain applies to a + /// transaction it has no record of at all. + async fn record_channel_tx_facts_admitted( + &self, facts: ChannelTxFacts, admission: FactsAdmission, ) -> Result { let txid = facts.txid; // Dated by the chain tip the report arrives at, which is what retention measures from. @@ -293,10 +328,10 @@ impl Wallet { None }, }, - // A transaction nothing is recorded of yet needs room of its own; one already on - // record is merged into above however full the store is, so an obligation this - // node took on is never half-kept. - None if !self.facts_retention.has_room() => { + // A transaction not yet recorded at all needs room of its own, unless the producer + // is exempt from the cap; one already on record is merged into above however full + // the store is, so an obligation this node took on is never half-kept. + None if admission == FactsAdmission::Capped && !self.facts_retention.has_room() => { rejection = Some(ChannelTxFactsRejection::NoRoom { limit: CHANNEL_TX_FACTS_MAX_RECORDS, }); @@ -320,12 +355,7 @@ impl Wallet { match rejection { Some(e) if e.is_resource_limit() => { - log_error!( - self.logger, - "Not recording what transaction {} is: {}. It will be reported without a classification", - txid, - e, - ); + log_error!(self.logger, "Not recording what transaction {} is: {}", txid, e,); Ok(FactsRecordOutcome::Incomplete) }, Some(e) => { @@ -2812,10 +2842,15 @@ impl Wallet { /// the candidate is live in the channel's splice details, and both writes are idempotent, so /// the replay completes whichever of them was lost. /// + /// The round's facts are admitted however many records the store contains, so that a store full + /// of other transactions cannot leave this node signing a round it has no measure of. A refusal + /// that a replay would only meet again reports the round [`SignedFundingRecord::Unmeasurable`], + /// without writing anything, for the caller to cancel the round rather than sign it. + /// /// [`ChannelManager::funding_transaction_signed`]: lightning::ln::channelmanager::ChannelManager::funding_transaction_signed pub(crate) async fn record_signed_funding( &self, tx: &Transaction, candidates: &[FundingCandidate], - ) -> Result<(), Error> { + ) -> Result { let txid = tx.compute_txid(); let signed_round = match candidates.iter().find(|candidate| candidate.txid == txid) { Some(round) => round, @@ -2825,7 +2860,7 @@ impl Wallet { "Not recording signed funding {}: not among the channel's pending splice rounds", txid, ); - return Ok(()); + return Ok(SignedFundingRecord::Recorded); }, }; let funding_channels: Vec = signed_round @@ -2854,7 +2889,7 @@ impl Wallet { let (figures, mut history) = match self.interactive_funding_figures(payment_id, candidates, signed_round, tx) { Some(record) => record, - None => return Ok(()), + None => return Ok(SignedFundingRecord::Recorded), }; let figures = recorded_figures.unwrap_or(figures); let direction = figures.direction; @@ -2867,7 +2902,7 @@ impl Wallet { let prior_pending = stores.pending_payment(&payment_id).await?; // A replayed signing event re-offers a transaction already recorded; nothing to add. if prior_pending.as_ref().is_some_and(|entry| entry.candidate(txid).is_some()) { - return Ok(()); + return Ok(SignedFundingRecord::Recorded); } // Merge LDK's history into the recorded one — refreshing the rounds both list, appending // the new ones — rather than replace it: LDK's history omits a recorded round it has since @@ -2891,14 +2926,27 @@ impl Wallet { // The fact goes first: it is what ties the transaction to this payment, so a failure // afterwards leaves the round attributable rather than a history pointing at a payment // the transaction would never be filed under. - self.record_channel_tx_facts( - ChannelTxFacts::new(txid) - .with_self_role(TransactionType::InteractiveFunding { - channels: funding_channels.clone(), - }) - .with_local_figures(figures), - ) - .await?; + // + // The round is admitted whatever the store's count: this node decides whether to sign it, + // and a record describing a round and this node's share of it carries no outputs, so it is + // among the smallest the store contains. What can still refuse it is a record already as + // large as one record may be, which never shrinks — so the round is reported unmeasurable + // for the caller to cancel, rather than released with the wallet's view of a funding output + // both parties own standing in for this node's share. + let facts = ChannelTxFacts::new(txid) + .with_self_role(TransactionType::InteractiveFunding { + channels: funding_channels.clone(), + }) + .with_local_figures(figures); + let outcome = self.record_channel_tx_facts_admitted(facts, FactsAdmission::Exempt).await?; + if outcome == FactsRecordOutcome::Incomplete { + log_error!( + self.logger, + "Not signing interactive funding {}: this node's share of it is not on record", + txid, + ); + return Ok(SignedFundingRecord::Unmeasurable); + } // Any round of the history may be the one that confirms, including the rounds this // node didn't contribute to. Write facts for those too, carrying this payment's id and // no share, so `find_payment_by_txid` can still map a confirming round to the payment @@ -2965,7 +3013,7 @@ impl Wallet { e, ); } - Ok(()) + Ok(SignedFundingRecord::Recorded) } /// Marks a splice round recorded when signing ([`Self::record_signed_funding`]) as broadcast @@ -4547,7 +4595,9 @@ mod tests { use crate::config::ElectrumSyncConfig; #[cfg(feature = "chain-esplora")] use crate::config::EsploraSyncConfig; - use crate::config::{CHANNEL_TX_FACTS_CACHE_CAPACITY, PAYMENT_CACHE_CAPACITY}; + use crate::config::{ + CHANNEL_TX_FACTS_CACHE_CAPACITY, CHANNEL_TX_FACTS_MAX_RECORD_BYTES, PAYMENT_CACHE_CAPACITY, + }; use crate::io::test_utils::InMemoryStore; use crate::io::{ BDK_WALLET_ADDRESS_POOL_KEY, BDK_WALLET_ADDRESS_POOL_PRIMARY_NAMESPACE, @@ -6206,6 +6256,150 @@ mod tests { ); } + /// A round this node is about to sign is measured however full the store is. The number of + /// records the store admits bounds what a counterparty drives; a round this node chose to + /// sign is admitted beside them, because without this node's share on record whoever first + /// observes the transaction records it with the wallet's view of a funding output both + /// parties own — the whole of it read as this node's spend, a figure that is never corrected. + #[tokio::test] + async fn a_full_store_still_measures_a_round_this_node_signs() { + 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))]); + + // The store contains as many records as it may, so a transaction a capped producer reports + // on is refused room. + wallet.facts_retention.walk_completed(CHANNEL_TX_FACTS_MAX_RECORDS); + assert_eq!( + wallet + .record_channel_tx_facts(ChannelTxFacts::new(Txid::from_byte_array([77u8; 32]))) + .await + .unwrap(), + FactsRecordOutcome::Incomplete, + ); + + assert_eq!( + wallet.record_signed_funding(&tx, &candidates).await.unwrap(), + SignedFundingRecord::Recorded, + "a round this node signs is measured however full the store is", + ); + let id = wallet + .find_payment_by_txid(txid) + .await + .unwrap() + .expect("the round's facts identify its payment"); + assert!( + wallet + .payment_stores + .pending_payment_store() + .get(&id) + .await + .unwrap() + .expect("entry") + .candidate(txid) + .is_some(), + "the signed round is in the channel's candidate history", + ); + + // Our signatures leave the node and the counterparty broadcasts: wallet sync is the first + // to see the transaction, and files it under this node's share of the round rather than + // under the wallet's view of a funding output both parties own. + observe_unconfirmed(&wallet, &tx).await; + + let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects; + assert_eq!(payments.len(), 1); + assert_eq!(payments[0].id, id); + assert_eq!(payments[0].amount_msat, Some(500_300_000)); + assert_eq!(payments[0].fee_paid_msat, Some(300_000)); + } + + /// A round whose facts are refused for a reason that is never undone is not signed, and is + /// reported unmeasurable so the caller cancels it. The record this round would merge into is + /// already as large as one record may be, which later reports cannot shrink, so replaying the + /// signing event would meet the same refusal forever while every event queued behind it waited. + #[tokio::test] + async fn a_round_this_node_cannot_measure_is_not_signed() { + use lightning::util::ser::Writeable; + + 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 channel = Channel { counterparty_node_id, channel_id }; + + 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))]); + + // How large the transaction's record would be once the signing added what the round is + // and this node's share of it. + let signed_length = |facts: &ChannelTxFacts| { + facts + .clone() + .with_self_role(TransactionType::InteractiveFunding { + channels: vec![channel.clone()], + }) + .with_local_figures(LocalFundingFigures { + funding_payment_id: PaymentId([0u8; 32]), + amount_msat: Some(500_300_000), + fee_paid_msat: Some(300_000), + direction: PaymentDirection::Inbound, + }) + .serialized_length() + }; + // Grow the transaction's record to the largest one that may be stored, in coarse steps + // first and single outputs after so the search stays cheap. + let mut crowded = ChannelTxFacts::new(txid); + let mut vout = 0u32; + for step in [128u32, 1] { + loop { + let grown = crowded.clone().with_outputs( + &channel, + None, + ChannelOutputRole::Htlc, + vout..vout + step, + ); + if grown.serialized_length() > CHANNEL_TX_FACTS_MAX_RECORD_BYTES { + break; + } + crowded = grown; + vout += step; + } + } + assert!( + signed_length(&crowded) > CHANNEL_TX_FACTS_MAX_RECORD_BYTES, + "the record must leave no room for what the signing adds", + ); + assert_eq!( + wallet.record_channel_tx_facts(crowded).await.unwrap(), + FactsRecordOutcome::Recorded, + ); + + assert_eq!( + wallet.record_signed_funding(&tx, &candidates).await.unwrap(), + SignedFundingRecord::Unmeasurable, + "a round this node cannot measure must be cancelled rather than replayed", + ); + assert!( + wallet.channel_tx_facts(&txid).await.expect("the record stays").local_figures.is_none(), + "this node's share was not recorded, which is what the refusal means", + ); + assert!( + wallet + .payment_stores + .pending_payment_store() + .list_filter(|entry| entry.candidate(txid).is_some()) + .await + .is_empty(), + "a round the signing refused must not be left in a candidate history", + ); + } + /// A signing event replayed after its pending-store write was lost re-derives the round's /// figures, from a contribution LDK may have adjusted the fee fields of since. The round's /// facts are immutable, so the replay adopts what is on record instead of offering a second @@ -12673,6 +12867,77 @@ mod tests { ); } + /// The funding transaction this node builds for a channel it opens is described however full + /// the store is. The number of records the store admits bounds what a counterparty drives; a + /// channel this node chose to fund is admitted beside them, because without the funding + /// output on record the funding transaction and every transaction spending it, the channel's + /// close among them, is reported without a classification. + #[tokio::test] + async fn a_full_store_still_describes_a_funding_this_node_builds() { + 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 channel = Channel { counterparty_node_id, channel_id }; + + // The store contains as many records as it may, so a transaction a capped producer + // reports on is refused room. + wallet.facts_retention.walk_completed(CHANNEL_TX_FACTS_MAX_RECORDS); + assert_eq!( + wallet + .record_channel_tx_facts(ChannelTxFacts::new(Txid::from_byte_array([48u8; 32]))) + .await + .unwrap(), + FactsRecordOutcome::Incomplete, + ); + + let funding_txid = Txid::from_byte_array([49u8; 32]); + assert_eq!( + wallet + .record_own_funding_tx_facts(ChannelTxFacts::new(funding_txid).with_outputs( + &channel, + None, + ChannelOutputRole::Funding, + [0], + )) + .await + .unwrap(), + FactsRecordOutcome::Recorded, + "a funding this node builds is described however full the store is", + ); + assert!(wallet.channel_tx_facts(&funding_txid).await.is_some()); + + // A close spending the funding output is classified from the record. + let close = tx_spending(&wallet, OutPoint { txid: funding_txid, vout: 0 }); + let close_txid = close.compute_txid(); + insert_unconfirmed_tx(&wallet, close.clone()); + wallet + .update_payment_store(vec![WalletEvent::TxUnconfirmed { + txid: close_txid, + tx: Arc::new(close), + old_block_time: None, + }]) + .await + .unwrap(); + let payment = wallet + .payment_stores + .payment_store() + .get(&PaymentId(close_txid.to_byte_array())) + .await + .unwrap() + .expect("wallet sync records the transaction"); + assert!( + matches!( + payment.kind, + PaymentKind::Onchain { + tx_type: Some(TransactionType::CooperativeClose { .. }), + .. + } + ), + "unexpected kind {:?}", + payment.kind, + ); + } + #[tokio::test] async fn a_payment_that_already_advanced_gets_no_pending_entry() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); diff --git a/src/wallet/provenance.rs b/src/wallet/provenance.rs index 94523de9d1..445f0d37b3 100644 --- a/src/wallet/provenance.rs +++ b/src/wallet/provenance.rs @@ -335,7 +335,8 @@ impl ChannelTxFacts { /// A record is written whole, so its size is the one resource a producer drives without /// creating a record of its own: every channel-controlled output of a transaction lands on /// that transaction's record, and a counterparty decides how many HTLCs a commitment - /// transaction carries. What a refused report would have described stays unclassifiable. + /// transaction carries. What a refused report would have described stays undescribed, and + /// what that costs is for the producer that reported it to weigh. pub(crate) fn size_checked(self) -> Result { let bytes = self.serialized_length(); if bytes > CHANNEL_TX_FACTS_MAX_RECORD_BYTES { @@ -354,11 +355,36 @@ pub(crate) enum FactsRecordOutcome { /// Everything reported is on record. Recorded, /// Part of what was reported is not on record, because recording it would have taken the - /// facts past the resources they are allowed. Transactions that would have been classified - /// from the missing part are reported without a classification instead. + /// facts past the resources they are allowed. Nothing was lost, so what the refusal costs + /// is the reporting producer's to weigh: a transaction reported without a classification + /// for a producer that has nothing left to withhold, a round left unsigned for one that + /// will not release a transaction it cannot measure. Incomplete, } +/// Whether a report about a transaction this node has no record of at all is subject to the +/// number of records the store may contain. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum FactsAdmission { + /// Refused once the store contains as many records as it is allowed to. + /// + /// This is what bounds the store: a counterparty opening and closing channels, or replacing + /// a negotiated funding again and again, drives records about transactions this node only + /// reports on, and each of them costs nothing to refuse beyond a transaction going + /// unclassified. + Capped, + /// Admitted beside however many records the store contains, and counted like any other, so + /// that capped reports are refused the sooner. + /// + /// This is for the reports whose refusal costs more than the record: the funding transaction + /// this node builds for a channel it opens, without which the funding and every transaction + /// spending it go unclassified, and a round this node is about to sign, which it will not + /// release without its own share of it on record. How many such records there can be is a + /// question of how many channels this node funds and rounds it signs, which is its own + /// decision. + Exempt, +} + /// One of this node's channel outputs, which only LDK will spend: the funding output of a channel /// the channel manager lists or the chain monitor watches, or an output a channel resolved to /// this node that the sweeper tracks. From dacf8c0f52bf756fc3bdff5310df0ded46a2a597 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 29 Sep 2026 21:41:30 +0200 Subject: [PATCH 5/6] Let the funding output alone identify a funding A channel becoming pending reported both the output that funds it and that the transaction is a funding of that channel. The second only repeats the first, since a funding output is what a funding transaction is recognised by. It also cost something. One transaction can open several channels, and each reports only its own output; a report stating outright what the transaction is lists one channel, so the next channel's contradicts it and is refused whole, taking its funding output with it. The payment then listed one of the channels opened instead of all of them. A channel becoming ready already reports only its funding output, so the two backstops now agree. Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/event.rs | 17 ++++----- src/wallet/mod.rs | 89 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/event.rs b/src/event.rs index 12be27f5aa..dc37a80f68 100644 --- a/src/event.rs +++ b/src/event.rs @@ -2221,15 +2221,16 @@ where "LDK Node has only ever persisted ChannelPending events from rust-lightning 0.0.115 or later", ); + // The funding output alone says what the transaction is, and says it for every + // channel a batched funding opens: each channel reports its own output, and the + // reports of one transaction are kept together. let channel = Channel { counterparty_node_id, channel_id }; - let facts = ChannelTxFacts::new(funding_txo.txid) - .with_outputs( - &channel, - Some(UserChannelId(user_channel_id)), - ChannelOutputRole::Funding, - [funding_txo.vout], - ) - .with_self_role(TransactionType::Funding { channels: vec![channel.clone()] }); + let facts = ChannelTxFacts::new(funding_txo.txid).with_outputs( + &channel, + Some(UserChannelId(user_channel_id)), + ChannelOutputRole::Funding, + [funding_txo.vout], + ); self.record_channel_tx_facts(facts).await; let event = Event::ChannelPending { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 44b612d31a..77dced3e20 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -12266,6 +12266,95 @@ mod tests { } } + /// The funding output a channel reports is what classifies the transaction that opens it; the + /// report does not have to say so separately. + #[tokio::test] + async fn a_funding_transaction_is_classified_by_the_output_it_creates() { + 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 channel = Channel { counterparty_node_id, channel_id }; + let funding = wallet_paying_tx(&wallet, 5); + let funding_txid = funding.compute_txid(); + insert_unconfirmed_tx(&wallet, funding.clone()); + + wallet + .record_channel_tx_facts(ChannelTxFacts::new(funding_txid).with_outputs( + &channel, + None, + ChannelOutputRole::Funding, + [0], + )) + .await + .unwrap(); + observe_unconfirmed(&wallet, &funding).await; + + let payment = wallet + .payment_stores + .payment_store() + .get(&PaymentId(funding_txid.to_byte_array())) + .await + .unwrap() + .expect("wallet sync records the funding transaction"); + match payment.kind { + PaymentKind::Onchain { + tx_type: Some(TransactionType::Funding { channels }), .. + } => { + assert_eq!(channels, vec![channel]); + }, + kind => panic!("unexpected kind {:?}", kind), + } + } + + /// One transaction can open several channels, each of which reports only the output that funds + /// it. The reports of one transaction are kept together, so the payment lists every channel + /// opened — which a report stating outright what the transaction is could not do, as the second + /// channel's would contradict the first's. + #[tokio::test] + async fn a_batched_funding_lists_every_channel_it_opens() { + 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 first = Channel { counterparty_node_id, channel_id }; + let second = Channel { counterparty_node_id, channel_id: ChannelId([8u8; 32]) }; + + let mut funding = wallet_paying_tx(&wallet, 6); + funding.output.push(funding.output[0].clone()); + let funding_txid = funding.compute_txid(); + insert_unconfirmed_tx(&wallet, funding.clone()); + + for (vout, channel) in [(0u32, &first), (1, &second)] { + wallet + .record_channel_tx_facts(ChannelTxFacts::new(funding_txid).with_outputs( + channel, + None, + ChannelOutputRole::Funding, + [vout], + )) + .await + .unwrap(); + } + observe_unconfirmed(&wallet, &funding).await; + + let payment = wallet + .payment_stores + .payment_store() + .get(&PaymentId(funding_txid.to_byte_array())) + .await + .unwrap() + .expect("wallet sync records the funding transaction"); + match payment.kind { + PaymentKind::Onchain { + tx_type: Some(TransactionType::Funding { channels }), .. + } => { + assert_eq!(channels, vec![first, second]); + }, + kind => panic!("unexpected kind {:?}", kind), + } + } + /// The node's channel state as a test dictates it: the channels its channel manager lists, /// the monitors its chain monitor has, and the outputs its sweeper tracks, for retention; and /// its channel outputs, for the startup pass. From 4d9d872d357e8d4e67bf33b5c1b13e9dcc663001 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 26 Oct 2026 09:54:33 -0500 Subject: [PATCH 6/6] Drop the DataStore::mutate_async wrapper It only forwarded to mutate_with, so that method is now pub(crate) and the callers that await fallible reads use it directly. Written with the help of an AI tool. Co-Authored-By: Claude Fable 5.1 --- src/data_store.rs | 24 ++++++++---------------- src/wallet/payment_stores.rs | 4 ++-- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index 747e0b7ed6..31a714a2a5 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -451,7 +451,12 @@ where self.mutate_with(id, |current| async move { Ok(f(current.as_ref())) }).await } - async fn mutate_with(&self, id: &SO::Id, f: F) -> Result, Error> + /// 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_with(&self, id: &SO::Id, f: F) -> Result, Error> where F: FnOnce(Option) -> Fut, Fut: Future, Error>>, @@ -470,19 +475,6 @@ 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; @@ -1260,7 +1252,7 @@ mod tests { } #[tokio::test] - async fn mutate_async_awaits_fallible_reads() { + async fn mutate_with_awaits_fallible_reads() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); let logger = Arc::new(TestLogger::new()); let id = TestObjectId { id: [42u8; 4] }; @@ -1285,7 +1277,7 @@ mod tests { ); let result = data_store - .mutate_async(&id, |existing| async move { + .mutate_with(&id, |existing| async move { let mut updated = existing.unwrap(); updated.data = other_store.get(&other_id).await?.unwrap().data; Ok(Some(updated)) diff --git a/src/wallet/payment_stores.rs b/src/wallet/payment_stores.rs index e88831725c..cbc7b27e58 100644 --- a/src/wallet/payment_stores.rs +++ b/src/wallet/payment_stores.rs @@ -149,7 +149,7 @@ impl PaymentStoresGuard<'_> { } /// [`Self::mutate_pending_payment`] with a transformation that awaits fallible reads; see - /// [`DataStore::mutate_async`](crate::data_store::DataStore::mutate_async). + /// [`DataStore::mutate_with`](crate::data_store::DataStore::mutate_with). pub(super) async fn mutate_pending_payment_async( &self, id: &PaymentId, f: F, ) -> Result, Error> @@ -157,6 +157,6 @@ impl PaymentStoresGuard<'_> { F: FnOnce(Option) -> Fut, Fut: Future, Error>>, { - self.stores.pending_payment_store.mutate_async(id, f).await + self.stores.pending_payment_store.mutate_with(id, f).await } }