Free holding cell upon handling a counterparty tx_abort

After cad88af, a few code paths that also lead to a quiescence exit were
not accounted for. This commit addresses the path where we exit
quiescence due to processing a counterparty's `tx_abort` message.

Author: wpaulino

lightning/src/ln/channel.rs

@@ -2029,13 +2029,13 @@ where
 
 	pub fn tx_abort<L: Logger>(
 		&mut self, msg: &msgs::TxAbort, logger: &L,
-	) -> Result<(Option<msgs::TxAbort>, Option<SpliceFundingFailed>), ChannelError> {
+	) -> Result<(Option<msgs::TxAbort>, Option<SpliceFundingFailed>, bool), 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) = match &mut self.phase {
+		let (should_ack, splice_funding_failed, exited_quiescence) = 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";
@@ -2044,7 +2044,7 @@ where
 			ChannelPhase::UnfundedV2(pending_v2_channel) => {
 				let had_constructor =
 					pending_v2_channel.interactive_tx_constructor.take().is_some();
-				(had_constructor, None)
+				(had_constructor, None, false)
 			},
 			ChannelPhase::Funded(funded_channel) => {
 				if funded_channel.has_pending_splice_awaiting_signatures()
@@ -2072,11 +2072,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, splice_funding_failed, true)
 				} 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, None, false)
 				}
 			},
 		};
@@ -2092,7 +2092,7 @@ where
 			}
 		});
 
-		Ok((tx_abort, splice_funding_failed))
+		Ok((tx_abort, splice_funding_failed, exited_quiescence))
 	}
 
 	#[rustfmt::skip]

lightning/src/ln/channelmanager.rs

@@ -11632,55 +11632,68 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 	fn internal_tx_abort(
 		&self, counterparty_node_id: &PublicKey, msg: &msgs::TxAbort,
 	) -> Result<NotifyOption, MsgHandleErrInternal> {
-		let per_peer_state = self.per_peer_state.read().unwrap();
-		let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
-			debug_assert!(false);
-			MsgHandleErrInternal::no_such_peer(counterparty_node_id, msg.channel_id)
-		})?;
-		let mut peer_state_lock = peer_state_mutex.lock().unwrap();
-		let peer_state = &mut *peer_state_lock;
-		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) =
-					try_channel_entry!(self, peer_state, res, chan_entry);
+		let (result, holding_cell_res) = {
+			let per_peer_state = self.per_peer_state.read().unwrap();
+			let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
+				debug_assert!(false);
+				MsgHandleErrInternal::no_such_peer(counterparty_node_id, msg.channel_id)
+			})?;
+			let mut peer_state_lock = peer_state_mutex.lock().unwrap();
+			let peer_state = &mut *peer_state_lock;
+			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) =
+						try_channel_entry!(self, peer_state, res, chan_entry);
 
-				let persist = if tx_abort.is_some() || splice_failed.is_some() {
-					NotifyOption::DoPersist
-				} else {
-					NotifyOption::SkipPersistNoEvents
-				};
+					let persist = if tx_abort.is_some() || splice_failed.is_some() {
+						NotifyOption::DoPersist
+					} else {
+						NotifyOption::SkipPersistNoEvents
+					};
 
-				if let Some(tx_abort_msg) = tx_abort {
-					peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort {
-						node_id: *counterparty_node_id,
-						msg: tx_abort_msg,
-					});
-				}
+					if let Some(tx_abort_msg) = tx_abort {
+						peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort {
+							node_id: *counterparty_node_id,
+							msg: tx_abort_msg,
+						});
+					}
 
-				if let Some(splice_funding_failed) = splice_failed {
-					let pending_events = &mut self.pending_events.lock().unwrap();
-					pending_events.push_back((
-						events::Event::SpliceFailed {
-							channel_id: msg.channel_id,
-							counterparty_node_id: *counterparty_node_id,
-							user_channel_id: chan_entry.get().context().get_user_id(),
-							abandoned_funding_txo: splice_funding_failed.funding_txo,
-							channel_type: splice_funding_failed.channel_type,
-							contributed_inputs: splice_funding_failed.contributed_inputs,
-							contributed_outputs: splice_funding_failed.contributed_outputs,
-						},
-						None,
-					));
-				}
+					if let Some(splice_funding_failed) = splice_failed {
+						let pending_events = &mut self.pending_events.lock().unwrap();
+						pending_events.push_back((
+							events::Event::SpliceFailed {
+								channel_id: msg.channel_id,
+								counterparty_node_id: *counterparty_node_id,
+								user_channel_id: chan_entry.get().context().get_user_id(),
+								abandoned_funding_txo: splice_funding_failed.funding_txo,
+								channel_type: splice_funding_failed.channel_type,
+								contributed_inputs: splice_funding_failed.contributed_inputs,
+								contributed_outputs: splice_funding_failed.contributed_outputs,
+							},
+							None,
+						));
+					}
 
-				Ok(persist)
-			},
-			hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::no_such_channel_for_peer(
-				counterparty_node_id,
-				msg.channel_id,
-			)),
-		}
+					let holding_cell_res = if exited_quiescence {
+						self.check_free_peer_holding_cells(peer_state)
+					} else {
+						Vec::new()
+					};
+					(Ok(persist), holding_cell_res)
+				},
+				hash_map::Entry::Vacant(_) => (
+					Err(MsgHandleErrInternal::no_such_channel_for_peer(
+						counterparty_node_id,
+						msg.channel_id,
+					)),
+					Vec::new(),
+				),
+			}
+		};
+
+		self.handle_holding_cell_free_result(holding_cell_res);
+		result
 	}
 
 	#[rustfmt::skip]

lightning/src/ln/splicing_tests.rs

@@ -2037,6 +2037,13 @@ fn fail_splice_on_tx_abort() {
 		initiate_splice_in(initiator, acceptor, channel_id, Amount::from_sat(splice_in_amount));
 	let _ = complete_splice_handshake(initiator, acceptor);
 
+	// Queue an outgoing HTLC to the holding cell. It should be freed once we exit quiescence.
+	let (route, payment_hash, _payment_preimage, payment_secret) =
+		get_route_and_payment_hash!(initiator, acceptor, 1_000_000);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
+	let payment_id = PaymentId(payment_hash.0);
+	initiator.node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+
 	let tx_add_input =
 		get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor);
 	acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input);
@@ -2057,8 +2064,22 @@ fn fail_splice_on_tx_abort() {
 		_ => panic!("Expected Event::SpliceFailed"),
 	}
 
-	let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor);
-	acceptor.node.handle_tx_abort(node_id_initiator, &tx_abort);
+	// We exit quiescence upon receiving `tx_abort`, so we should see our `tx_abort` echo and the
+	// holding cell be immediately freed.
+	let msg_events = initiator.node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 2, "{msg_events:?}");
+	check_added_monitors(initiator, 1);
+	if let MessageSendEvent::SendTxAbort { msg, .. } = &msg_events[0] {
+		acceptor.node.handle_tx_abort(node_id_initiator, msg);
+	} else {
+		panic!("Unexpected event {:?}", msg_events[0]);
+	};
+	if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[1] {
+		acceptor.node.handle_update_add_htlc(node_id_initiator, &updates.update_add_htlcs[0]);
+		do_commitment_signed_dance(acceptor, initiator, &updates.commitment_signed, false, false);
+	} else {
+		panic!("Unexpected event {:?}", msg_events[1]);
+	};
 }
 
 #[test]