Correctly order channel_ready on channel_reestablish
What changed, and why it matters
This patch fixes the order in which Lightning messages are resent when a connection comes back after being dropped. In some channel states, the node was sending 'channel_ready' before the initial signatures it depends on, or after them when it shouldn't. The fix adds a new ordering flag so the correct sequence is used depending on whether the channel is still finalizing its funding transaction or is already operational. A wrong order could confuse a peer and prevent the channel from resuming correctly, potentially causing a denial of service or funds to be stuck.
Review whether the old ordering caused observable interoperability failures or stuck channels with specific peers. Consider adding regression tests covering re-establishment ordering for both pre-funding and post-funding states. No immediate emergency patch action is indicated beyond applying the fix.
Security signals we found
Protocol message ordering bug in channel re-establishment
Potential denial of service / channel stall on reconnection
Incorrect retransmission ordering could violate peer expectations
No explicit security advisory or CVE referenced in commit
Evidence from the diff
The commit introduces a ChannelReadyOrder enum and threads it through MonitorRestoreUpdates and ReestablishResponses. During channel_reestablish, if an interactive signing session is active or the channel is still AwaitingChannelReady, channel_ready is now queued after commitment_signed/tx_signatures (SignaturesFirst). Otherwise it is sent before commitment updates (ChannelReadyFirst). handle_channel_resumption in ChannelManager now emits channel_ready and announcement_sigs either before or after the commitment/RAA sequence based on this flag. This corrects a protocol-ordering bug in retransmission logic.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rsChannel re-establishment message orderingInteractive funding / splicing signing sessionsInspect captured patch +88 / −34
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 5724592..2b3280e 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -54,9 +54,10 @@ use crate::ln::channel_state::{
OutboundHTLCDetails, OutboundHTLCStateDetails,
};
use crate::ln::channelmanager::{
- self, FundingConfirmedMessage, HTLCFailureMsg, HTLCSource, OpenChannelMessage,
- PaymentClaimDetails, PendingHTLCInfo, PendingHTLCStatus, RAACommitmentOrder, SentHTLCId,
- BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA,
+ self, ChannelReadyOrder, FundingConfirmedMessage, HTLCFailureMsg, HTLCSource,
+ OpenChannelMessage, PaymentClaimDetails, PendingHTLCInfo, PendingHTLCStatus,
+ RAACommitmentOrder, SentHTLCId, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT,
+ MIN_CLTV_EXPIRY_DELTA,
};
use crate::ln::funding::FundingTxInput;
#[cfg(splicing)]
@@ -1181,13 +1182,14 @@ pub enum UpdateFulfillCommitFetch {
pub(super) struct MonitorRestoreUpdates {
pub raa: Option<msgs::RevokeAndACK>,
pub commitment_update: Option<msgs::CommitmentUpdate>,
- pub order: RAACommitmentOrder,
+ pub commitment_order: RAACommitmentOrder,
pub accepted_htlcs: Vec<(PendingHTLCInfo, u64)>,
pub failed_htlcs: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
pub finalized_claimed_htlcs: Vec<(HTLCSource, Option<AttributionData>)>,
pub pending_update_adds: Vec<msgs::UpdateAddHTLC>,
pub funding_broadcastable: Option<Transaction>,
pub channel_ready: Option<msgs::ChannelReady>,
+ pub channel_ready_order: ChannelReadyOrder,
pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
pub tx_signatures: Option<msgs::TxSignatures>,
}
@@ -1210,9 +1212,10 @@ pub(super) struct SignerResumeUpdates {
/// The return value of `channel_reestablish`
pub(super) struct ReestablishResponses {
pub channel_ready: Option<msgs::ChannelReady>,
+ pub channel_ready_order: ChannelReadyOrder,
pub raa: Option<msgs::RevokeAndACK>,
pub commitment_update: Option<msgs::CommitmentUpdate>,
- pub order: RAACommitmentOrder,
+ pub commitment_order: RAACommitmentOrder,
pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
pub shutdown_msg: Option<msgs::Shutdown>,
pub tx_signatures: Option<msgs::TxSignatures>,
@@ -8705,6 +8708,17 @@ where
}
}
+ // An active interactive signing session or an awaiting channel_ready state implies that a
+ // commitment_signed retransmission is an initial one for funding negotiation. Thus, the
+ // signatures should be sent before channel_ready.
+ let channel_ready_order = if self.interactive_tx_signing_session.is_some() {
+ ChannelReadyOrder::SignaturesFirst
+ } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) {
+ ChannelReadyOrder::SignaturesFirst
+ } else {
+ ChannelReadyOrder::ChannelReadyFirst
+ };
+
// We will never broadcast the funding transaction when we're in MonitorUpdateInProgress
// (and we assume the user never directly broadcasts the funding transaction and waits for
// us to do it). Thus, we can only ever hit monitor_pending_channel_ready when we're
@@ -8733,9 +8747,10 @@ where
self.context.monitor_pending_revoke_and_ack = false;
self.context.monitor_pending_commitment_signed = false;
return MonitorRestoreUpdates {
- raa: None, commitment_update: None, order: RAACommitmentOrder::RevokeAndACKFirst,
+ raa: None, commitment_update: None, commitment_order: RAACommitmentOrder::RevokeAndACKFirst,
accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, pending_update_adds,
- funding_broadcastable, channel_ready, announcement_sigs, tx_signatures: None
+ funding_broadcastable, channel_ready, announcement_sigs, tx_signatures: None,
+ channel_ready_order,
};
}
@@ -8758,14 +8773,15 @@ where
self.context.monitor_pending_revoke_and_ack = false;
self.context.monitor_pending_commitment_signed = false;
- let order = self.context.resend_order.clone();
+ let commitment_order = self.context.resend_order.clone();
log_debug!(logger, "Restored monitor updating in channel {} resulting in {}{} commitment update and {} RAA, with {} first",
&self.context.channel_id(), if funding_broadcastable.is_some() { "a funding broadcastable, " } else { "" },
if commitment_update.is_some() { "a" } else { "no" }, if raa.is_some() { "an" } else { "no" },
- match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
+ match commitment_order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
MonitorRestoreUpdates {
- raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs,
- pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures: None
+ raa, commitment_update, commitment_order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs,
+ pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures: None,
+ channel_ready_order,
}
}
@@ -9168,8 +9184,9 @@ where
// Short circuit the whole handler as there is nothing we can resend them
return Ok(ReestablishResponses {
channel_ready: None,
+ channel_ready_order: ChannelReadyOrder::ChannelReadyFirst,
raa: None, commitment_update: None,
- order: RAACommitmentOrder::CommitmentFirst,
+ commitment_order: RAACommitmentOrder::CommitmentFirst,
shutdown_msg, announcement_sigs,
tx_signatures: None,
tx_abort: None,
@@ -9179,8 +9196,9 @@ where
// We have OurChannelReady set!
return Ok(ReestablishResponses {
channel_ready: self.get_channel_ready(logger),
+ channel_ready_order: ChannelReadyOrder::ChannelReadyFirst,
raa: None, commitment_update: None,
- order: RAACommitmentOrder::CommitmentFirst,
+ commitment_order: RAACommitmentOrder::CommitmentFirst,
shutdown_msg, announcement_sigs,
tx_signatures: None,
tx_abort: None,
@@ -9306,10 +9324,13 @@ where
};
Ok(ReestablishResponses {
- channel_ready, shutdown_msg, announcement_sigs,
+ channel_ready,
+ channel_ready_order: ChannelReadyOrder::SignaturesFirst,
+ shutdown_msg,
+ announcement_sigs,
raa: required_revoke,
commitment_update,
- order: self.context.resend_order.clone(),
+ commitment_order: self.context.resend_order.clone(),
tx_signatures,
tx_abort,
})
@@ -9323,9 +9344,11 @@ where
if self.context.channel_state.is_monitor_update_in_progress() {
self.context.monitor_pending_commitment_signed = true;
Ok(ReestablishResponses {
- channel_ready, shutdown_msg, announcement_sigs,
+ channel_ready,
+ channel_ready_order: ChannelReadyOrder::ChannelReadyFirst,
+ shutdown_msg, announcement_sigs,
commitment_update: None, raa: None,
- order: self.context.resend_order.clone(),
+ commitment_order: self.context.resend_order.clone(),
tx_signatures: None,
tx_abort: None,
})
@@ -9347,9 +9370,11 @@ where
required_revoke
};
Ok(ReestablishResponses {
- channel_ready, shutdown_msg, announcement_sigs,
+ channel_ready,
+ channel_ready_order: ChannelReadyOrder::ChannelReadyFirst,
+ shutdown_msg, announcement_sigs,
raa, commitment_update,
- order: self.context.resend_order.clone(),
+ commitment_order: self.context.resend_order.clone(),
tx_signatures: None,
tx_abort: None,
})
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 8498313..a450dcc 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -917,6 +917,16 @@ pub(super) enum RAACommitmentOrder {
RevokeAndACKFirst,
}
+/// Similar to scenarios used by [`RAACommitmentOrder`], this determines whether a `channel_ready`
+/// message should be sent first (i.e., prior to a `commitment_update`) or after the initial
+/// `commitment_update` and `tx_signatures` for channel funding.
+pub(super) enum ChannelReadyOrder {
+ /// Send `channel_ready` message first.
+ ChannelReadyFirst,
+ /// Send initial `commitment_update` and `tx_signatures` first.
+ SignaturesFirst,
+}
+
/// Information about a payment which is currently being claimed.
#[derive(Clone, Debug, PartialEq, Eq)]
struct ClaimingPayment {
@@ -3394,9 +3404,10 @@ macro_rules! handle_monitor_update_completion {
let (htlc_forwards, decode_update_add_htlcs) = $self.handle_channel_resumption(
&mut $peer_state.pending_msg_events, $chan, updates.raa,
- updates.commitment_update, updates.order, updates.accepted_htlcs, updates.pending_update_adds,
- updates.funding_broadcastable, updates.channel_ready,
- updates.announcement_sigs, updates.tx_signatures, None);
+ updates.commitment_update, updates.commitment_order, updates.accepted_htlcs,
+ updates.pending_update_adds, updates.funding_broadcastable, updates.channel_ready,
+ updates.announcement_sigs, updates.tx_signatures, None, updates.channel_ready_order,
+ );
if let Some(upd) = channel_update {
$peer_state.pending_msg_events.push(upd);
}
@@ -8900,11 +8911,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
#[rustfmt::skip]
fn handle_channel_resumption(&self, pending_msg_events: &mut Vec<MessageSendEvent>,
channel: &mut FundedChannel<SP>, raa: Option<msgs::RevokeAndACK>,
- commitment_update: Option<msgs::CommitmentUpdate>, order: RAACommitmentOrder,
+ commitment_update: Option<msgs::CommitmentUpdate>, commitment_order: RAACommitmentOrder,
pending_forwards: Vec<(PendingHTLCInfo, u64)>, pending_update_adds: Vec<msgs::UpdateAddHTLC>,
funding_broadcastable: Option<Transaction>,
channel_ready: Option<msgs::ChannelReady>, announcement_sigs: Option<msgs::AnnouncementSignatures>,
tx_signatures: Option<msgs::TxSignatures>, tx_abort: Option<msgs::TxAbort>,
+ channel_ready_order: ChannelReadyOrder,
) -> (Option<(u64, PublicKey, OutPoint, ChannelId, u128, Vec<(PendingHTLCInfo, u64)>)>, Option<(u64, Vec<msgs::UpdateAddHTLC>)>) {
let logger = WithChannelContext::from(&self.logger, &channel.context, None);
log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures, {} tx_abort",
@@ -8935,14 +8947,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
decode_update_add_htlcs = Some((short_channel_id, pending_update_adds));
}
- if let Some(msg) = channel_ready {
- send_channel_ready!(self, pending_msg_events, channel, msg);
- }
- if let Some(msg) = announcement_sigs {
- pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures {
- node_id: counterparty_node_id,
- msg,
- });
+ if let ChannelReadyOrder::ChannelReadyFirst = channel_ready_order {
+ if let Some(msg) = &channel_ready {
+ send_channel_ready!(self, pending_msg_events, channel, msg.clone());
+ }
+
+ if let Some(msg) = &announcement_sigs {
+ pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures {
+ node_id: counterparty_node_id,
+ msg: msg.clone(),
+ });
+ }
}
macro_rules! handle_cs { () => {
@@ -8962,7 +8977,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
});
}
} }
- match order {
+ match commitment_order {
RAACommitmentOrder::CommitmentFirst => {
handle_cs!();
handle_raa!();
@@ -8987,6 +9002,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
});
}
+ if let ChannelReadyOrder::SignaturesFirst = channel_ready_order {
+ if let Some(msg) = channel_ready {
+ send_channel_ready!(self, pending_msg_events, channel, msg);
+ }
+
+ if let Some(msg) = announcement_sigs {
+ pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures {
+ 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());
@@ -11049,9 +11077,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
let need_lnd_workaround = chan.context.workaround_lnd_bug_4006.take();
let (htlc_forwards, decode_update_add_htlcs) = self.handle_channel_resumption(
- &mut peer_state.pending_msg_events, chan, responses.raa, responses.commitment_update, responses.order,
+ &mut peer_state.pending_msg_events, chan, responses.raa, responses.commitment_update, responses.commitment_order,
Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs,
- responses.tx_signatures, responses.tx_abort);
+ responses.tx_signatures, responses.tx_abort, responses.channel_ready_order,
+ );
debug_assert!(htlc_forwards.is_none());
debug_assert!(decode_update_add_htlcs.is_none());
if let Some(upd) = channel_update {
Why this scored 63/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.