Gate interactive commitment_signed on user approval during reestablish
What changed, and why it matters
This patch fixes a logic gap in Lightning Dev Kit's interactive splicing handshake. During a channel reconnection, the code could re-send a commitment signature for a new funding transaction before the user had actually approved and provided their own transaction signatures. The fix gates that retransmission so it only happens after the user calls funding_transaction_signed, preserving the intended 'cancel up to the last moment' behavior.
Review whether any other reestablish or retransmission paths for interactive funding can emit commitment_signed or tx_signatures before holder approval. Ensure the new has_holder_witnesses gate is applied consistently across all interactive-tx flows. Consider adding tests for the abort/cancel path and for the non-initiator side.
Security signals we found
Missing authorization gate in protocol state machine
Premature cryptographic commitment retransmission on reconnection
User-intended cancellation window bypassed
Regression test added for reestablish path
Evidence from the diff
In channel.rs, the reestablish handling for an in-progress interactive funding/splicing session now checks session.has_holder_witnesses() before retransmitting a funding commitment_signed or sending holder tx_signatures. Previously, if the peer’s channel_reestablish requested retransmission of commitment_signed, the node would send it even though the holder had not yet supplied witness data via ChannelManager::funding_transaction_signed. The new test in splicing_tests.rs verifies that after dropping the acceptor’s initial commitment_signed and reconnecting, the initiator does not retransmit its commitment_signed until funding_transaction_signed is called.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rsInteractive funding/splicing handshakechannel_reestablish message handlingInspect captured patch +95 / −22
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 2341128..1588da4 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -10581,30 +10581,32 @@ where
self.context.expecting_peer_commitment_signed = true;
}
- // - if it has not received `tx_signatures` for that funding transaction:
- // - if the `commitment_signed` bit is set in `retransmit_flags`:
- if !session.has_received_tx_signatures()
- && next_funding.should_retransmit(msgs::NextFundingFlag::CommitmentSigned)
- {
- // - MUST retransmit its `commitment_signed` for that funding transaction.
- retransmit_funding_commit_sig = Some(next_funding.txid);
- }
+ if !session.has_holder_witnesses() {
+ log_debug!(logger, "Waiting for funding transaction signatures to be provided");
+ } else {
+ // - if it has not received `tx_signatures` for that funding transaction:
+ // - if the `commitment_signed` bit is set in `retransmit_flags`:
+ if !session.has_received_tx_signatures()
+ && next_funding.should_retransmit(msgs::NextFundingFlag::CommitmentSigned)
+ {
+ // - MUST retransmit its `commitment_signed` for that funding transaction.
+ retransmit_funding_commit_sig = Some(next_funding.txid);
+ }
- // - if it has already received `commitment_signed` and it should sign first
- // - MUST send its `tx_signatures` for that funding transaction.
- //
- // - if it has already received `tx_signatures` for that funding transaction:
- // - MUST send its `tx_signatures` for that funding transaction.
- if let Some(holder_tx_signatures) = session.holder_tx_signatures() {
- if self.is_awaiting_monitor_update() {
- log_debug!(logger, "Waiting for monitor update before providing funding transaction signatures");
- } else if self.context.signer_pending_funding {
- log_debug!(logger, "Waiting for signer to provide counterparty commitment_signed before releasing funding transaction signatures");
- } else {
- tx_signatures = Some(holder_tx_signatures);
+ // - if it has already received `commitment_signed` and it should sign first
+ // - MUST send its `tx_signatures` for that funding transaction.
+ //
+ // - if it has already received `tx_signatures` for that funding transaction:
+ // - MUST send its `tx_signatures` for that funding transaction.
+ if let Some(holder_tx_signatures) = session.holder_tx_signatures() {
+ if self.is_awaiting_monitor_update() {
+ log_debug!(logger, "Waiting for monitor update before providing funding transaction signatures");
+ } else if self.context.signer_pending_funding {
+ log_debug!(logger, "Waiting for signer to provide counterparty commitment_signed before releasing funding transaction signatures");
+ } else {
+ tx_signatures = Some(holder_tx_signatures);
+ }
}
- } else if !session.has_holder_witnesses() {
- log_debug!(logger, "Waiting for funding transaction signatures to be provided");
}
} else {
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 6e6af60..a8821e7 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -2676,6 +2676,77 @@ fn test_splice_locked_waits_for_channel_reestablish() {
send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
}
+#[test]
+fn test_splice_reestablish_waits_for_holder_tx_signatures_before_commitment_signed() {
+ 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 node_id_0 = nodes[0].node.get_our_node_id();
+ 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 outputs = vec![TxOut {
+ value: Amount::from_sat(initial_channel_value_sat / 4),
+ script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+ }];
+ let initiator_contribution =
+ initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs).unwrap();
+ negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution);
+
+ let signing_event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning);
+ assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+
+ // Drop the acceptor's initial `commitment_signed`. On reconnection, node 0's
+ // `channel_reestablish` should request it again, while node 1's `channel_reestablish` should
+ // not make node 0 retransmit a `commitment_signed` before holder transaction signatures are
+ // available.
+ let _ = get_htlc_update_msgs(&nodes[1], &node_id_0);
+ nodes[0].node.peer_disconnected(node_id_1);
+ nodes[1].node.peer_disconnected(node_id_0);
+
+ let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+ reconnect_args.send_announcement_sigs = (true, true);
+ reconnect_args.send_interactive_tx_commit_sig = (true, false);
+ reconnect_nodes(reconnect_args);
+
+ assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+ let unsigned_transaction = if let Event::FundingTransactionReadyForSigning {
+ unsigned_transaction,
+ ..
+ } = signing_event
+ {
+ unsigned_transaction
+ } else {
+ panic!("Expected FundingTransactionReadyForSigning event");
+ };
+ let tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap();
+ nodes[0].node.funding_transaction_signed(&channel_id, &node_id_1, tx).unwrap();
+ check_added_monitors(&nodes[0], 1);
+
+ let initiator_commit_sig = get_htlc_update_msgs(&nodes[0], &node_id_1);
+ nodes[1]
+ .node
+ .handle_commitment_signed_batch_test(node_id_0, &initiator_commit_sig.commitment_signed);
+ check_added_monitors(&nodes[1], 1);
+
+ let acceptor_tx_signatures =
+ get_event_msg!(nodes[1], MessageSendEvent::SendTxSignatures, node_id_0);
+ nodes[0].node.handle_tx_signatures(node_id_1, &acceptor_tx_signatures);
+ let initiator_tx_signatures =
+ get_event_msg!(nodes[0], MessageSendEvent::SendTxSignatures, node_id_1);
+ nodes[1].node.handle_tx_signatures(node_id_0, &initiator_tx_signatures);
+
+ expect_splice_pending_event(&nodes[0], &node_id_1);
+ expect_splice_pending_event(&nodes[1], &node_id_0);
+}
+
#[test]
fn test_splice_confirms_on_both_sides_while_disconnected() {
// Regression test: when a splice transaction confirms on both sides while peers are
Why this scored 44/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.