Emit SpliceFailed upon disconnect while quiescent

Since quiescence is terminated upon disconnection, any outstanding
splice negotiation should result in emitting a SpliceFailed event as
long as we haven't reached FundingNegotiation::AwaitingSignatures. This
may occur if we explicitly disconnect the peer (e.g., when failing to
process splice_ack) or if the connection is lost..

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1193,6 +1193,14 @@ pub(crate) struct ShutdownResult {
 	pub(crate) splice_funding_failed: Option<SpliceFundingFailed>,
 }
 
+/// The result of a peer disconnection.
+pub(crate) struct DisconnectResult {
+	pub(crate) is_resumable: bool,
+	/// If a splice was in progress when the channel was shut down, this contains
+	/// the splice funding information for emitting a SpliceFailed event.
+	pub(crate) splice_funding_failed: Option<SpliceFundingFailed>,
+}
+
 /// Tracks the transaction number, along with current and next commitment points.
 /// This consolidates the logic to advance our commitment number and request new
 /// commitment points from our signer.
@@ -1585,11 +1593,15 @@ where
 	/// Should be called when the peer is disconnected. Returns true if the channel can be resumed
 	/// when the peer reconnects (via [`Self::peer_connected_get_handshake`]). If not, the channel
 	/// must be immediately closed.
-	#[rustfmt::skip]
-	pub fn peer_disconnected_is_resumable<L: Deref>(&mut self, logger: &L) -> bool where L::Target: Logger {
-		match &mut self.phase {
+	pub fn peer_disconnected_is_resumable<L: Deref>(&mut self, logger: &L) -> DisconnectResult
+	where
+		L::Target: Logger,
+	{
+		let is_resumable = match &mut self.phase {
 			ChannelPhase::Undefined => unreachable!(),
-			ChannelPhase::Funded(chan) => chan.remove_uncommitted_htlcs_and_mark_paused(logger).is_ok(),
+			ChannelPhase::Funded(chan) => {
+				chan.remove_uncommitted_htlcs_and_mark_paused(logger).is_ok()
+			},
 			// If we get disconnected and haven't yet committed to a funding
 			// transaction, we can replay the `open_channel` on reconnection, so don't
 			// bother dropping the channel here. However, if we already committed to
@@ -1599,7 +1611,34 @@ where
 			ChannelPhase::UnfundedOutboundV1(chan) => chan.is_resumable(),
 			ChannelPhase::UnfundedInboundV1(_) => false,
 			ChannelPhase::UnfundedV2(_) => false,
-		}
+		};
+
+		let splice_funding_failed = if let ChannelPhase::Funded(chan) = &mut self.phase {
+			// Reset any quiescence-related state as it is implicitly terminated once disconnected.
+			if matches!(chan.context.channel_state, ChannelState::ChannelReady(_)) {
+				if chan.quiescent_action.is_some() {
+					// If we were trying to get quiescent, try again after reconnection.
+					chan.context.channel_state.set_awaiting_quiescence();
+				}
+				chan.context.channel_state.clear_local_stfu_sent();
+				chan.context.channel_state.clear_remote_stfu_sent();
+				if chan.should_reset_pending_splice_funding_negotiation().unwrap_or(true) {
+					// If we were in quiescence but a splice was never negotiated, or the negotiation
+					// failed due to disconnecting, we shouldn't be quiescent anymore upon reconnecting.
+					// If there was a pending splice negotiation that has failed due to disconnecting,
+					// we also take the opportunity to clean up our state.
+					chan.reset_pending_splice_state()
+				} else {
+					None
+				}
+			} else {
+				None
+			}
+		} else {
+			None
+		};
+
+		DisconnectResult { is_resumable, splice_funding_failed }
 	}
 
 	/// Should be called when the peer re-connects, returning an initial message which we should
@@ -6808,37 +6847,38 @@ where
 	}
 
 	pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult {
-		let splice_funding_failed =
-			if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
-				self.reset_pending_splice_state().or_else(|| {
-					self.quiescent_action.take().and_then(|quiescent_action| match quiescent_action
-					{
-						QuiescentAction::Splice(instructions) => {
-							let (inputs, outputs) =
-								instructions.into_contributed_inputs_and_outputs();
-							Some(SpliceFundingFailed {
-								funding_txo: None,
-								channel_type: None,
-								contributed_inputs: inputs,
-								contributed_outputs: outputs,
-							})
-						},
-						#[cfg(any(test, fuzzing))]
-						_ => {
-							self.quiescent_action = Some(quiescent_action);
-							None
-						},
-					})
-				})
-			} else {
-				None
-			};
+		let splice_funding_failed = self.maybe_fail_splice_negotiation();
 
 		let mut shutdown_result = self.context.force_shutdown(&self.funding, closure_reason);
 		shutdown_result.splice_funding_failed = splice_funding_failed;
 		shutdown_result
 	}
 
+	fn maybe_fail_splice_negotiation(&mut self) -> Option<SpliceFundingFailed> {
+		if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
+			self.reset_pending_splice_state().or_else(|| {
+				self.quiescent_action.take().and_then(|quiescent_action| match quiescent_action {
+					QuiescentAction::Splice(instructions) => {
+						let (inputs, outputs) = instructions.into_contributed_inputs_and_outputs();
+						Some(SpliceFundingFailed {
+							funding_txo: None,
+							channel_type: None,
+							contributed_inputs: inputs,
+							contributed_outputs: outputs,
+						})
+					},
+					#[cfg(any(test, fuzzing))]
+					_ => {
+						self.quiescent_action = Some(quiescent_action);
+						None
+					},
+				})
+			})
+		} else {
+			None
+		}
+	}
+
 	fn interactive_tx_constructor_mut(&mut self) -> Option<&mut InteractiveTxConstructor> {
 		self.pending_splice
 			.as_mut()
@@ -9095,24 +9135,6 @@ where
 			}
 		}
 
-		// Reset any quiescence-related state as it is implicitly terminated once disconnected.
-		if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
-			if self.quiescent_action.is_some() {
-				// If we were trying to get quiescent, try again after reconnection.
-				self.context.channel_state.set_awaiting_quiescence();
-			}
-			self.context.channel_state.clear_local_stfu_sent();
-			self.context.channel_state.clear_remote_stfu_sent();
-			if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) {
-				// If we were in quiescence but a splice was never negotiated, or the negotiation
-				// failed due to disconnecting, we shouldn't be quiescent anymore upon reconnecting.
-				// If there was a pending splice negotiation that has failed due to disconnecting,
-				// we also take the opportunity to clean up our state.
-				self.reset_pending_splice_state();
-				debug_assert!(!self.context.channel_state.is_quiescent());
-			}
-		}
-
 		self.context.channel_state.set_peer_disconnected();
 		log_trace!(logger, "Peer disconnection resulted in {} remote-announced HTLC drops on channel {}", inbound_drop_count, &self.context.channel_id());
 		Ok(())
@@ -11779,9 +11801,9 @@ where
 			.map_err(|e| APIError::APIMisuseError { err: e.to_owned() })
 	}
 
-	fn send_splice_init(
-		&mut self, instructions: SpliceInstructions,
-	) -> Result<msgs::SpliceInit, String> {
+	fn send_splice_init(&mut self, instructions: SpliceInstructions) -> msgs::SpliceInit {
+		debug_assert!(self.pending_splice.is_none());
+
 		let SpliceInstructions {
 			adjusted_funding_contribution,
 			our_funding_inputs,
@@ -11791,15 +11813,6 @@ where
 			locktime,
 		} = instructions;
 
-		// Check if a splice has been initiated already.
-		// Note: only a single outstanding splice is supported (per spec)
-		if self.pending_splice.is_some() {
-			return Err(format!(
-				"Channel {} cannot be spliced, as it has already a splice pending",
-				self.context.channel_id(),
-			));
-		}
-
 		let prev_funding_input = self.funding.to_splice_funding_input();
 		let context = FundingNegotiationContext {
 			is_initiator: true,
@@ -11823,14 +11836,14 @@ where
 		let prev_funding_txid = self.funding.get_funding_txid();
 		let funding_pubkey = self.context.holder_pubkeys(prev_funding_txid).funding_pubkey;
 
-		Ok(msgs::SpliceInit {
+		msgs::SpliceInit {
 			channel_id: self.context.channel_id,
 			funding_contribution_satoshis: adjusted_funding_contribution.to_sat(),
 			funding_feerate_per_kw,
 			locktime,
 			funding_pubkey,
 			require_confirmed_inputs: None,
-		})
+		}
 	}
 
 	/// Checks during handling splice_init
@@ -12975,10 +12988,21 @@ where
 						"Internal Error: Didn't have anything to do after reaching quiescence".to_owned()
 					));
 				},
-				Some(QuiescentAction::Splice(_instructions)) => {
-					return self.send_splice_init(_instructions)
-						.map(|splice_init| Some(StfuResponse::SpliceInit(splice_init)))
-						.map_err(|e| ChannelError::WarnAndDisconnect(e.to_owned()));
+				Some(QuiescentAction::Splice(instructions)) => {
+					if self.pending_splice.is_some() {
+						debug_assert!(false);
+						self.quiescent_action = Some(QuiescentAction::Splice(instructions));
+
+						return Err(ChannelError::WarnAndDisconnect(
+							format!(
+								"Channel {} cannot be spliced as it already has a splice pending",
+								self.context.channel_id(),
+							),
+						));
+					}
+
+					let splice_init = self.send_splice_init(instructions);
+					return Ok(Some(StfuResponse::SpliceInit(splice_init)));
 				},
 				#[cfg(any(test, fuzzing))]
 				Some(QuiescentAction::DoNothing) => {

lightning/src/ln/channelmanager.rs

@@ -59,9 +59,10 @@ use crate::ln::chan_utils::selected_commitment_sat_per_1000_weight;
 #[cfg(any(test, fuzzing))]
 use crate::ln::channel::QuiescentAction;
 use crate::ln::channel::{
-	self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, FundedChannel,
-	InboundV1Channel, OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult,
-	SpliceFundingFailed, StfuResponse, UpdateFulfillCommitFetch, WithChannelContext,
+	self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, DisconnectResult,
+	FundedChannel, InboundV1Channel, OutboundV1Channel, PendingV2Channel, ReconnectionMsg,
+	ShutdownResult, SpliceFundingFailed, StfuResponse, UpdateFulfillCommitFetch,
+	WithChannelContext,
 };
 use crate::ln::channel_state::ChannelDetails;
 use crate::ln::funding::SpliceContribution;
@@ -13447,8 +13448,8 @@ where
 
 	#[rustfmt::skip]
 	fn peer_disconnected(&self, counterparty_node_id: PublicKey) {
-		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(
-			self, || NotifyOption::SkipPersistHandleEvents);
+		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
+		let mut persist = NotifyOption::SkipPersistHandleEvents;
 		let mut failed_channels: Vec<(Result<Infallible, _>, _)> = Vec::new();
 		let mut per_peer_state = self.per_peer_state.write().unwrap();
 		let remove_peer = {
@@ -13461,11 +13462,29 @@ where
 				let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 				let peer_state = &mut *peer_state_lock;
 				let pending_msg_events = &mut peer_state.pending_msg_events;
+				let pending_events = &mut self.pending_events.lock().unwrap();
 				peer_state.channel_by_id.retain(|_, chan| {
 					let logger = WithChannelContext::from(&self.logger, &chan.context(), None);
-					if chan.peer_disconnected_is_resumable(&&logger) {
+					let DisconnectResult { is_resumable, splice_funding_failed } =
+						chan.peer_disconnected_is_resumable(&&logger);
+
+					if let Some(splice_funding_failed) = splice_funding_failed {
+						pending_events.push_back((events::Event::SpliceFailed {
+							channel_id: chan.context().channel_id(),
+							counterparty_node_id,
+							user_channel_id: chan.context().get_user_id(),
+							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));
+						persist = NotifyOption::DoPersist;
+					}
+
+					if is_resumable {
 						return true;
 					}
+
 					// Clean up for removal.
 					let reason = ClosureReason::DisconnectedPeer;
 					let err = ChannelError::Close((reason.to_string(), reason));
@@ -13548,6 +13567,9 @@ where
 		for (err, counterparty_node_id) in failed_channels.drain(..) {
 			let _ = handle_error!(self, err, counterparty_node_id);
 		}
+
+		persist
+		});
 	}
 
 	#[rustfmt::skip]

lightning/src/ln/splicing_tests.rs

@@ -401,6 +401,8 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) {
 	} else {
 		nodes[0].node.peer_disconnected(node_id_1);
 		nodes[1].node.peer_disconnected(node_id_0);
+
+		let _event = get_event!(nodes[0], Event::SpliceFailed);
 	}
 
 	let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
@@ -466,6 +468,8 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) {
 	} else {
 		nodes[0].node.peer_disconnected(node_id_1);
 		nodes[1].node.peer_disconnected(node_id_0);
+
+		let _event = get_event!(nodes[0], Event::SpliceFailed);
 	}
 
 	let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);