From f3575c5dbe4d4e1020a8b5c0bd5b860715551d72 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 1 Jun 2026 15:05:33 -0700 Subject: [PATCH] Persist negotiated splice candidates on reload Prior to supporting RBF, we would avoid persisting `FundedChannel::pending_splice` when there was a pending funding negotiation that could not be resumed on channel reestablishment. With the addition of RBF support, this would cause our previously negotiated splices (but still pending) to be dropped unintentionally. --- lightning/src/ln/channel.rs | 76 +++++++++++++++++++++++++----- lightning/src/ln/splicing_tests.rs | 49 +++++++++++++++++++ 2 files changed, 112 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9c16c3b02f8..4aa9380f249 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2989,15 +2989,6 @@ struct PendingFunding { contributions: Vec, } -impl_ser_tlv_based!(PendingFunding, { - (1, funding_negotiation, upgradable_option), - (3, negotiated_candidates, required_vec), - (5, sent_funding_txid, option), - (7, received_funding_txid, option), - (8, last_funding_feerate_sat_per_1000_weight, option), - (10, contributions, optional_vec), -}); - #[derive(Debug)] enum FundingNegotiation { AwaitingAck { @@ -3037,6 +3028,57 @@ impl_writeable_tlv_based_enum_upgradable!(FundingNegotiation, unread_variants: AwaitingAck, ConstructingTransaction ); +struct PendingFundingWriteable<'a> { + pending_funding: &'a PendingFunding, + reset_funding_negotiation: bool, +} + +impl Writeable for PendingFundingWriteable<'_> { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { + let funding_negotiation = if self.reset_funding_negotiation { + None + } else { + self.pending_funding.funding_negotiation.as_ref() + }; + debug_assert!( + funding_negotiation.is_none() + || matches!( + funding_negotiation, + Some(FundingNegotiation::AwaitingSignatures { .. }) + ) + ); + let contributions_len = if self.reset_funding_negotiation + && self.pending_funding.funding_negotiation.is_some() + { + self.pending_funding.contributions.len().saturating_sub(1) + } else { + self.pending_funding.contributions.len() + }; + write_tlv_fields!(writer, { + (1, funding_negotiation, upgradable_option), + (3, self.pending_funding.negotiated_candidates, required_vec), + (5, self.pending_funding.sent_funding_txid, option), + (7, self.pending_funding.received_funding_txid, option), + (8, self.pending_funding.last_funding_feerate_sat_per_1000_weight, option), + (10, self.pending_funding.contributions[..contributions_len], optional_vec), + }); + Ok(()) + } +} + +impl Readable for PendingFunding { + fn read(reader: &mut R) -> Result { + Ok(_decode_and_build!(reader, Self, { + (1, funding_negotiation, upgradable_option), + (3, negotiated_candidates, required_vec), + (5, sent_funding_txid, option), + (7, received_funding_txid, option), + (8, last_funding_feerate_sat_per_1000_weight, option), + (10, contributions, optional_vec), + })) + } +} + impl FundingNegotiation { fn as_funding(&self) -> Option<&FundingScope> { match self { @@ -16305,10 +16347,18 @@ impl Writeable for FundedChannel { let holder_commitment_point_next = self.holder_commitment_point.next_point(); let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point; - // We don't have to worry about resetting the pending `FundingNegotiation` because we - // can only read `FundingNegotiation::AwaitingSignatures` variants anyway. - let pending_splice = - self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(true)); + // Avoid writing any negotiations that are not at the signing stage yet, as they cannot be + // resumed on reestablishment, but keep any already-negotiated candidates. + let reset_funding_negotiation = self.should_reset_pending_splice_state(true); + let should_persist_pending_splice = + !reset_funding_negotiation || !self.pending_funding().is_empty(); + let pending_splice = should_persist_pending_splice + .then(|| ()) + .and_then(|_| self.pending_splice.as_ref()) + .map(|pending_funding| PendingFundingWriteable { + pending_funding, + reset_funding_negotiation, + }); let monitor_pending_tx_signatures = self.context.monitor_pending_tx_signatures.then_some(()); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index dfcc339b83b..75ff238bc35 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1152,6 +1152,55 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); } +#[test] +fn test_reload_resets_splice_negotiation_without_dropping_candidates() { + // A reload should abort an in-flight RBF negotiation, but it must not drop the previously + // negotiated splice candidate that the monitor is still tracking. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister_0, chain_monitor_0); + let node_0; + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_1 = nodes[1].node.get_our_node_id(); + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (_splice_tx, _) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution.clone()); + + let rbf_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert_eq!(funding_template.min_rbf_feerate(), Some(rbf_feerate)); + assert!(funding_template.prior_contribution().is_some()); + + let rbf_contribution = + funding_template.with_prior_contribution(rbf_feerate, FeeRate::MAX).build().unwrap(); + nodes[0].node.funding_contributed(&channel_id, &node_id_1, rbf_contribution, None).unwrap(); + complete_rbf_handshake(&nodes[0], &nodes[1]); + + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0, + chain_monitor_0, + node_0 + ); + let _ = get_event!(&nodes[0], Event::SpliceNegotiationFailed); + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert_eq!(funding_template.min_rbf_feerate(), Some(rbf_feerate)); + assert_eq!(funding_template.prior_contribution().unwrap(), &funding_contribution); +} + #[test] fn test_config_reject_inbound_splices() { // Tests that nodes with `reject_inbound_splices` properly reject inbound splices but still