Persist negotiated splice candidates on reload
What the developer wrote
Authored by Wilmer Paulino
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.
What changed, and why it matters
This commit fixes a bug in the Lightning Dev Kit where restarting a node during a certain type of funding negotiation (RBF splice) could accidentally throw away already-agreed splice candidates. Previously, the code avoided saving incomplete negotiations on reload, but with RBF support that also dropped earlier negotiated splice states that the channel monitor was still tracking. The fix separates the live negotiation state from the negotiated candidates during serialization, so reload cancels only the in-flight negotiation while preserving the prior candidate. The bug is a state-consistency/data-loss issue rather than a direct theft vector, but it could disrupt channel operation or funds availability after a restart.
Treat as a state-consistency bugfix worth including in release notes. Users running nodes with splicing/RBF should upgrade to avoid loss of pending splice state across restarts. No immediate emergency response is warranted because exploitation requires a local reload during an active RBF negotiation and the impact is state disruption rather than direct fund loss. Review whether any channels currently have pending splices and, if so, avoid restarting before applying the patch.
Security signals we found
State loss on node reload for pending splice candidates
Mismatch between in-memory channel state and persisted chain monitor state
RBF funding negotiation not resumable across reload
Serialization filter dropped entire PendingFunding including already-negotiated candidates
Regression test added: test_reload_resets_splice_negotiation_without_dropping_candidates
Evidence from the diff
In rust-lightning, FundedChannel::pending_splice is a PendingFunding structure containing both an active FundingNegotiation and a list of negotiated_candidates. Before RBF, serialization skipped the whole PendingFunding when a negotiation could not resume after reestablishment. With RBF, an in-flight RBF renegotiation can exist alongside a previously negotiated splice candidate that the chain monitor already tracks. The old logic filtered out pending_splice entirely, dropping negotiated_candidates and contributions. The patch introduces a PendingFundingWriteable wrapper that writes the structure with funding_negotiation reset to None (and trims the last contribution if needed) while still persisting negotiated_candidates and remaining contributions. A new regression test verifies that after reload the RBF negotiation fails but the prior splice candidate and its contribution remain available.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rsFundedChannel serialization (Writeable impl)PendingFunding TLV serializationSplicing / RBF funding negotiation stateInspect captured patch +112 / −13
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 9c16c3b..4aa9380 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2989,15 +2989,6 @@ struct PendingFunding {
contributions: Vec<FundingContribution>,
}
-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<W: Writer>(&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<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+ 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<SP: SignerProvider> Writeable for FundedChannel<SP> {
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 dfcc339..75ff238 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
Why this scored 55/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.