Remove exited_quiescence from error handling
What changed, and why it matters
This commit is a code-cleanup refactor in a Lightning network library. It removes a redundant flag called `exited_quiescence` and instead decides whether to release queued payments by checking whether a `tx_abort` message was produced. The change is described by the authors as fixing a 'leaky abstraction' where error-handling code knew too much about channel quiescence. It is not a security patch for an exploitable vulnerability, but it touches logic that controls when HTLCs (payments) held during channel quiescence are released, so a bug here could theoretically affect payment liveness or state consistency.
Treat as a normal code-quality / defensive refactor. Reviewers should verify that every path producing a `tx_abort` either legitimately exits quiescence or is a documented no-op, and that no path that should release the holding cell fails to produce a `tx_abort`. No urgent security response is indicated by the supplied materials.
Security signals we found
Refactor of error-handling state machine for interactive transaction negotiation (splicing / quiescence)
Holding-cell release logic now inferred from presence of `tx_abort` rather than explicit flag
Edge cases acknowledged where `tx_abort` may not imply exited quiescence, but release is no-op
No bounds checks, memory safety, cryptographic, or authentication changes observed
No vendor security disclosure, CVE, or researcher attribution in commit materials
Evidence from the diff
The patch removes the exited_quiescence boolean from InteractiveTxMsgError, MsgHandleErrInternal, and the return tuple of Channel::tx_abort. It introduces MsgHandleErrInternal::needs_holding_cell_release(), which returns self.tx_abort.is_some(). All previous uses of exited_quiescence to trigger check_free_peer_holding_cells are replaced with this inferred condition. The commit message explicitly notes two edge cases where tx_abort may be present without actually having exited quiescence, but where releasing the holding cell is a no-op: unfunded v2 channels and an unreachable debug_assert!(false) branch for funded channels. This is a defensive refactor to reduce coupling; no CVE or vendor security disclosure is present.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rsInteractiveTxMsgErrorMsgHandleErrInternalChannel::tx_abortInteractive transaction negotiation / splicing quiescence handlingInspect captured patch +33 / −62
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 93ef2b8..b2c6b60 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -1172,9 +1172,6 @@ pub(super) struct InteractiveTxMsgError {
/// If a splice was in progress when processing the message, this contains the splice funding
/// information for emitting a `SpliceFailed` event.
pub(super) splice_funding_failed: Option<SpliceFundingFailed>,
- /// Whether we were quiescent when we received the message, and are no longer due to aborting
- /// the session.
- pub(super) exited_quiescence: bool,
}
/// The return value of `monitor_updating_restored`
@@ -1818,30 +1815,24 @@ where
let logger = WithChannelContext::from(logger, &self.context(), None);
log_info!(logger, "Failed interactive transaction negotiation: {reason}");
- let (splice_funding_failed, exited_quiescence) = match &mut self.phase {
+ let splice_funding_failed = match &mut self.phase {
ChannelPhase::Undefined => unreachable!(),
- ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {
- (None, false)
- },
+ ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => None,
ChannelPhase::UnfundedV2(pending_v2_channel) => {
pending_v2_channel.interactive_tx_constructor.take();
- (None, false)
+ None
},
ChannelPhase::Funded(funded_channel) => {
if funded_channel.should_reset_pending_splice_state(false) {
- (funded_channel.reset_pending_splice_state(), true)
+ funded_channel.reset_pending_splice_state()
} else {
debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures");
- (None, false)
+ None
}
},
};
- InteractiveTxMsgError {
- err: ChannelError::Abort(reason),
- splice_funding_failed,
- exited_quiescence,
- }
+ InteractiveTxMsgError { err: ChannelError::Abort(reason), splice_funding_failed }
}
pub fn tx_add_input<L: Logger>(
@@ -1856,7 +1847,6 @@ where
"Received unexpected interactive transaction negotiation message".to_owned(),
),
splice_funding_failed: None,
- exited_quiescence: false,
}),
}
}
@@ -1873,7 +1863,6 @@ where
"Received unexpected interactive transaction negotiation message".to_owned(),
),
splice_funding_failed: None,
- exited_quiescence: false,
}),
}
}
@@ -1890,7 +1879,6 @@ where
"Received unexpected interactive transaction negotiation message".to_owned(),
),
splice_funding_failed: None,
- exited_quiescence: false,
}),
}
}
@@ -1907,7 +1895,6 @@ where
"Received unexpected interactive transaction negotiation message".to_owned(),
),
splice_funding_failed: None,
- exited_quiescence: false,
}),
}
}
@@ -1924,7 +1911,6 @@ where
return Err(InteractiveTxMsgError {
err: ChannelError::WarnAndDisconnect(err.to_owned()),
splice_funding_failed: None,
- exited_quiescence: false,
});
},
};
@@ -1985,13 +1971,13 @@ where
pub fn tx_abort<L: Logger>(
&mut self, msg: &msgs::TxAbort, logger: &L,
- ) -> Result<(Option<msgs::TxAbort>, Option<SpliceFundingFailed>, bool), ChannelError> {
+ ) -> Result<(Option<msgs::TxAbort>, Option<SpliceFundingFailed>), ChannelError> {
// If we have not sent a `tx_abort` message for this negotiation previously, we need to echo
// back a tx_abort message according to the spec:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561
// For rationale why we echo back `tx_abort`:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580
- let (should_ack, splice_funding_failed, exited_quiescence) = match &mut self.phase {
+ let (should_ack, splice_funding_failed) = match &mut self.phase {
ChannelPhase::Undefined => unreachable!(),
ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {
let err = "Got an unexpected tx_abort message: This is an unfunded channel created with V1 channel establishment";
@@ -2000,7 +1986,7 @@ where
ChannelPhase::UnfundedV2(pending_v2_channel) => {
let had_constructor =
pending_v2_channel.interactive_tx_constructor.take().is_some();
- (had_constructor, None, false)
+ (had_constructor, None)
},
ChannelPhase::Funded(funded_channel) => {
if funded_channel.has_pending_splice_awaiting_signatures()
@@ -2028,11 +2014,11 @@ where
.unwrap_or(false);
debug_assert!(has_funding_negotiation);
let splice_funding_failed = funded_channel.reset_pending_splice_state();
- (true, splice_funding_failed, true)
+ (true, splice_funding_failed)
} else {
// We were not tracking the pending funding negotiation state anymore, likely
// due to a disconnection or already having sent our own `tx_abort`.
- (false, None, false)
+ (false, None)
}
},
};
@@ -2048,7 +2034,7 @@ where
}
});
- Ok((tx_abort, splice_funding_failed, exited_quiescence))
+ Ok((tx_abort, splice_funding_failed))
}
#[rustfmt::skip]
@@ -14286,13 +14272,11 @@ where
}
fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
- let exited_quiescence = if matches!(err, ChannelError::Abort(_)) {
+ if matches!(err, ChannelError::Abort(_)) {
debug_assert!(self.context.channel_state.is_quiescent());
- self.exit_quiescence()
- } else {
- false
- };
- InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
+ self.exit_quiescence();
+ }
+ InteractiveTxMsgError { err, splice_funding_failed: None }
}
pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> {
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7ea1497..8e14b47 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1073,7 +1073,6 @@ struct MsgHandleErrInternal {
closes_channel: bool,
shutdown_finish: Option<(ShutdownResult, Option<(msgs::ChannelUpdate, NodeId, NodeId)>)>,
tx_abort: Option<msgs::TxAbort>,
- exited_quiescence: bool,
}
impl MsgHandleErrInternal {
@@ -1088,7 +1087,6 @@ impl MsgHandleErrInternal {
closes_channel: false,
shutdown_finish: None,
tx_abort: None,
- exited_quiescence: false,
}
}
@@ -1108,13 +1106,7 @@ impl MsgHandleErrInternal {
}
fn from_no_close(err: msgs::LightningError) -> Self {
- Self {
- err,
- closes_channel: false,
- shutdown_finish: None,
- tx_abort: None,
- exited_quiescence: false,
- }
+ Self { err, closes_channel: false, shutdown_finish: None, tx_abort: None }
}
fn from_finish_shutdown(
@@ -1135,7 +1127,6 @@ impl MsgHandleErrInternal {
closes_channel: true,
shutdown_finish: Some((shutdown_res, channel_update)),
tx_abort: None,
- exited_quiescence: false,
}
}
@@ -1171,13 +1162,7 @@ impl MsgHandleErrInternal {
},
},
};
- Self {
- err,
- closes_channel: false,
- shutdown_finish: None,
- tx_abort,
- exited_quiescence: false,
- }
+ Self { err, closes_channel: false, shutdown_finish: None, tx_abort }
}
fn dont_send_error_message(&mut self) {
@@ -1194,9 +1179,11 @@ impl MsgHandleErrInternal {
self.closes_channel
}
- fn with_exited_quiescence(mut self, exited_quiescence: bool) -> Self {
- self.exited_quiescence = exited_quiescence;
- self
+ /// Whether the holding cell should be released after handling this error. This is inferred
+ /// from the presence of a `tx_abort`, which is sent when aborting an interactive transaction
+ /// negotiation that was conducted during quiescence.
+ fn needs_holding_cell_release(&self) -> bool {
+ self.tx_abort.is_some()
}
}
@@ -4635,6 +4622,7 @@ impl<
internal.map_err(|err_internal| {
let mut msg_event = None;
+ let needs_holding_cell_release = err_internal.needs_holding_cell_release();
if let Some((shutdown_res, update_option)) = err_internal.shutdown_finish {
let counterparty_node_id = shutdown_res.counterparty_node_id;
@@ -4676,7 +4664,7 @@ impl<
}
let mut holding_cell_res = None;
- if msg_event.is_some() || err_internal.exited_quiescence {
+ if msg_event.is_some() || needs_holding_cell_release {
let per_peer_state = self.per_peer_state.read().unwrap();
if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
let mut peer_state = peer_state_mutex.lock().unwrap();
@@ -4687,8 +4675,7 @@ impl<
}
// We need to enqueue the `tx_abort` in `pending_msg_events` above before we
// enqueue any commitment updates generated by freeing holding cell HTLCs.
- holding_cell_res = err_internal
- .exited_quiescence
+ holding_cell_res = needs_holding_cell_release
.then(|| self.check_free_peer_holding_cells(&mut peer_state));
}
}
@@ -12007,10 +11994,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
None,
));
}
- debug_assert!(!err.exited_quiescence || matches!(err.err, ChannelError::Abort(_)));
-
MsgHandleErrInternal::from_chan_no_close(err.err, channel_id)
- .with_exited_quiescence(err.exited_quiescence)
}
fn internal_tx_msg<
@@ -12247,7 +12231,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
// We consider a splice negotiated when we exchange `tx_signatures`,
// which also terminates quiescence.
- let exited_quiescence = splice_negotiated.is_some();
+ let needs_holding_cell_release = splice_negotiated.is_some();
if let Some(splice_negotiated) = splice_negotiated {
self.pending_events.lock().unwrap().push_back((
events::Event::SplicePending {
@@ -12262,7 +12246,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
None,
));
}
- let holding_cell_res = if exited_quiescence {
+ let holding_cell_res = if needs_holding_cell_release {
self.check_free_peer_holding_cells(peer_state)
} else {
Vec::new()
@@ -12304,7 +12288,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
let res = chan_entry.get_mut().tx_abort(msg, &self.logger);
- let (tx_abort, splice_failed, exited_quiescence) =
+ let (tx_abort, splice_failed) =
try_channel_entry!(self, peer_state, res, chan_entry);
let persist = if tx_abort.is_some() || splice_failed.is_some() {
@@ -12313,6 +12297,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
NotifyOption::SkipPersistNoEvents
};
+ // Release any HTLCs held during quiescence now that we're
+ // exiting via tx_abort.
+ let needs_holding_cell_release = tx_abort.is_some();
if let Some(tx_abort_msg) = tx_abort {
peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort {
node_id: *counterparty_node_id,
@@ -12344,7 +12331,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
));
}
- let holding_cell_res = if exited_quiescence {
+ let holding_cell_res = if needs_holding_cell_release {
self.check_free_peer_holding_cells(peer_state)
} else {
Vec::new()
Why this scored 28/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.