Remove exited_quiescence from error handling

The `exited_quiescence` field on `MsgHandleErrInternal` and
`InteractiveTxMsgError` is a leaky abstraction -- the channelmanager
error handling shouldn't know about quiescence, only whether the holding
cell needs to be released.

Infer this from the presence of a `tx_abort` instead, since exiting
quiescence via an error always produces one. Remove `exited_quiescence`
from `InteractiveTxMsgError`, `MsgHandleErrInternal`, and the return
type of `Channel::tx_abort`, along with the `with_exited_quiescence`
builder.

For unfunded v2 channels, `tx_abort` may be present without quiescence
having been exited, but the holding cell release is a no-op since an
unfunded channel won't have any HTLCs. Similarly, the unreachable
`debug_assert!(false)` branch in `fail_interactive_tx_negotiation` for
funded channels produces a `tx_abort` without exiting quiescence, but
the holding cell release is a no-op since the channel is still quiescent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: jkczyz

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> {

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()