Ignore stale splice initial commitment_signed
What the developer wrote
Authored by Wilmer Paulino
Ignore stale splice initial commitment_signed
After we complete a splice negotiation and see a
`FundingTransactionReadyForSigning` event, the counterparty may already
have sent its initial `commitment_signed` for the splice funding
transaction. If we then cancel the funding contribution, our local
channel state no longer tracks the pending splice attempt and queues
`tx_abort`, but the in-flight `commitment_signed` can still arrive
first. Handling that message against the post-abort channel state
attempts to validate a signature for the now-stale splice funding
transaction and can force-close the still-live channel.
We fix this by checking the optional `funding_txid` (which we expect all
implementations to always include by default) included in
`commitment_signed` before validating the commitment signature. If it
does not match the channel's locked funding txid, we can safely ignore
the stale message.
What changed, and why it matters
This patch fixes a bug in the Lightning Dev Kit where a delayed message from a peer could accidentally force-close a live payment channel. During a failed 'splice' (a way to resize a channel), the peer might already have sent a signature for the now-canceled new funding transaction. Before this fix, the software would try to check that signature against the old channel state, fail, and wrongly close the channel. The fix makes the software first check whether the signature belongs to the currently active funding transaction and ignore it if it is stale.
Apply the patch. It is a targeted, low-risk fix with a regression test. Nodes running splice-capable LDK versions should upgrade to avoid unnecessary channel force-closures during splice cancellation races.
Security signals we found
Force-closure of a live Lightning channel due to stale signature validation
Race condition between splice cancellation and in-flight commitment_signed
Incorrect signature validation against post-abort channel state
Denial-of-service-like channel closure triggered by delayed peer message
Evidence from the diff
In lightning/src/ln/channel.rs, commitment_signed handling now checks msg.funding_txid against self.funding.get_funding_txid() before validating the commitment signature. If a funding txid is present and does not match the channel’s locked funding txid, the message is rejected with ChannelError::Ignore instead of proceeding to signature validation. This prevents a race where a stale splice-initial commitment_signed arrives after cancel_funding_contributed has already aborted the splice, which previously caused an invalid-signature force-close. A regression test in splicing_tests.rs reproduces the exact ordering.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rsLDK splice negotiation / interactive transaction exchangecommitment_signed message handlingInspect captured patch +81 / −0
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index ab9c964..9353dbe 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -8614,6 +8614,15 @@ where
"Got a single commitment_signed message when expecting a batch".to_owned(),
));
}
+ if let Some(funding_txid) = msg.funding_txid {
+ let locked_funding_txid =
+ self.funding.get_funding_txid().expect("funded channel must have known txid");
+ if funding_txid != locked_funding_txid {
+ return Err(ChannelError::Ignore(format!(
+ "Ignoring commitment_signed for stale funding txid {funding_txid}"
+ )));
+ }
+ }
let transaction_number = self.holder_commitment_point.next_transaction_number();
let commitment_point = self.holder_commitment_point.next_point();
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index f480c4e..1140e6b 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -4176,6 +4176,78 @@ fn do_cancel_funding_contributed_before_funding_transaction_signed(state: u8) {
do_commitment_signed_dance(acceptor, initiator, &update.commitment_signed, false, false);
}
+#[test]
+fn cancel_funding_contributed_then_inflight_commitment_signed_does_not_close_channel() {
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let initiator = &nodes[0];
+ let acceptor = &nodes[1];
+
+ let node_id_initiator = initiator.node.get_our_node_id();
+ let node_id_acceptor = acceptor.node.get_our_node_id();
+
+ let initial_channel_capacity = 100_000;
+ let (_, _, channel_id, _) =
+ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0);
+
+ let outputs = vec![TxOut {
+ value: Amount::from_sat(1_000),
+ script_pubkey: initiator.wallet_source.get_change_script().unwrap(),
+ }];
+ let funding_contribution =
+ initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap();
+ let new_funding_script = complete_splice_handshake(initiator, acceptor);
+ complete_interactive_funding_negotiation(
+ initiator,
+ acceptor,
+ channel_id,
+ funding_contribution.clone(),
+ new_funding_script,
+ );
+
+ // Both peers completed the interactive transaction exchange. Since only the
+ // initiator contributed splice funds, the initiator must still surface the
+ // unsigned funding transaction before it may send its initial
+ // `commitment_signed`.
+ let _ = get_event!(initiator, Event::FundingTransactionReadyForSigning);
+ assert!(acceptor.node.get_and_clear_pending_events().is_empty());
+ assert!(initiator.node.get_and_clear_pending_msg_events().is_empty());
+
+ // The acceptor has no funding contribution, so it can send its initial
+ // `commitment_signed` immediately. Hold that message to model it racing with
+ // the local caller's decision to cancel instead of sign.
+ let acceptor_commit_sig = get_htlc_update_msgs(acceptor, &node_id_initiator);
+ assert_eq!(acceptor_commit_sig.commitment_signed.len(), 1);
+
+ // Cancel before signing. This is a valid API flow: local contribution is
+ // discarded, the splice negotiation fails locally, and LDK queues a
+ // `tx_abort` for the peer.
+ initiator.node.cancel_funding_contributed(&channel_id, &node_id_acceptor).unwrap();
+ let reason = NegotiationFailureReason::LocallyCanceled;
+ expect_splice_failed_events(initiator, &channel_id, funding_contribution, reason);
+
+ // Keep our `tx_abort` queued. The fuzz failure has this exact ordering: our
+ // abort is outbound, but the acceptor's earlier `commitment_signed` reaches
+ // us first.
+ let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor);
+ assert_eq!(tx_abort.channel_id, channel_id);
+
+ initiator
+ .node
+ .handle_commitment_signed(node_id_acceptor, &acceptor_commit_sig.commitment_signed[0]);
+
+ // The delayed `commitment_signed` belonged to the splice we just aborted. It
+ // should not be validated against the post-abort channel state and should
+ // not force-close the live channel as an invalid commitment signature.
+ assert!(initiator.node.get_and_clear_pending_events().is_empty());
+ assert!(acceptor.node.get_and_clear_pending_events().is_empty());
+ assert!(initiator.node.get_and_clear_pending_msg_events().is_empty());
+ assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty());
+}
+
#[test]
fn cannot_cancel_funding_contributed_after_funding_transaction_signed() {
let chanmon_cfgs = create_chanmon_cfgs(2);
Why this scored 64/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.