Retransmit commitment_signed before tx_signatures
What the developer wrote
Authored by Jeffrey Czyz
Retransmit commitment_signed before tx_signatures
During channel reestablishment, both commitment_signed and tx_signatures
messages may be retransmitted. However, commitment_signed must come
first, so re-arrange the ordering. This moves tx_abort as well to keep
the interactive tx messages together.
What changed, and why it matters
This commit fixes the order in which two Lightning protocol messages are resent when a channel connection is restored after an outage. The 'commitment_signed' message must now be sent before 'tx_signatures'. Sending them out of order could cause a peer to reject or mishandle the channel re-establishment, potentially disrupting dual-funded channels. It is a protocol correctness fix rather than a clear exploit for theft of funds.
Treat as a protocol-correctness bug worth patching. Review whether out-of-order retransmission could be induced by an attacker to stall or desynchronize dual-funded channels, and add regression tests for message ordering during reestablishment.
Security signals we found
Protocol message ordering violation in channel reestablishment
Dual-funding/interactive transaction message flow affected
Potential peer rejection or channel state inconsistency on reconnection
No explicit security impact disclosed by vendor
Evidence from the diff
In channelmanager.rs, during channel reestablishment, the code previously appended tx_signatures (and tx_abort) to pending_msg_events before the commitment_signed handling block. The patch moves the tx_signatures/tx_abort push logic to after the commitment_update handling, ensuring commitment_signed is transmitted first. The existing TODO comment about async signing and holding back tx_signatures until commitment_signed is ready is preserved and relocated. This is an ordering fix for interactive-tx/dual-funding message flow.
Changed components
lightning/src/ln/channelmanager.rsChannel reestablishment message queueDual-funding / interactive transaction negotiationInspect captured patch +14 / −13
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 5c2627b..8498313 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -8944,19 +8944,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
msg,
});
}
- // TODO(dual_funding): For async signing support we need to hold back `tx_signatures` until the `commitment_signed` is ready.
- if let Some(msg) = tx_signatures {
- pending_msg_events.push(MessageSendEvent::SendTxSignatures {
- node_id: counterparty_node_id,
- msg,
- });
- }
- if let Some(msg) = tx_abort {
- pending_msg_events.push(MessageSendEvent::SendTxAbort {
- node_id: counterparty_node_id,
- msg,
- });
- }
macro_rules! handle_cs { () => {
if let Some(update) = commitment_update {
@@ -8986,6 +8973,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
},
}
+ // TODO(dual_funding): For async signing support we need to hold back `tx_signatures` until the `commitment_signed` is ready.
+ if let Some(msg) = tx_signatures {
+ pending_msg_events.push(MessageSendEvent::SendTxSignatures {
+ node_id: counterparty_node_id,
+ msg,
+ });
+ }
+ if let Some(msg) = tx_abort {
+ pending_msg_events.push(MessageSendEvent::SendTxAbort {
+ node_id: counterparty_node_id,
+ msg,
+ });
+ }
+
if let Some(tx) = funding_broadcastable {
if channel.context.is_manual_broadcast() {
log_info!(logger, "Not broadcasting funding transaction with txid {} as it is manually managed", tx.compute_txid());
Why this scored 46/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.