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,15 @@ 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 {
+			chan.maybe_fail_splice_negotiation()
+		} else {
+			None
+		};
+
+		DisconnectResult { is_resumable, splice_funding_failed }
 	}
 
 	/// Should be called when the peer re-connects, returning an initial message which we should
@@ -6800,8 +6820,15 @@ where
 	}
 
 	pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult {
-		let splice_funding_failed = self
-			.pending_splice
+		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> {
+		self.pending_splice
 			.as_mut()
 			.and_then(|pending_splice| pending_splice.funding_negotiation.take())
 			.filter(|funding_negotiation| funding_negotiation.is_initiator())
@@ -6831,11 +6858,7 @@ where
 						None
 					},
 				})
-			});
-
-		let mut shutdown_result = self.context.force_shutdown(&self.funding, closure_reason);
-		shutdown_result.splice_funding_failed = splice_funding_failed;
-		shutdown_result
+			})
 	}
 
 	fn interactive_tx_constructor_mut(&mut self) -> Option<&mut InteractiveTxConstructor> {
@@ -11672,9 +11695,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,
@@ -11684,15 +11707,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,
@@ -11716,14 +11730,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
@@ -12868,10 +12882,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;
@@ -13397,11 +13398,28 @@ 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));
+					}
+
+					if is_resumable {
 						return true;
 					}
+
 					// Clean up for removal.
 					let reason = ClosureReason::DisconnectedPeer;
 					let err = ChannelError::Close((reason.to_string(), reason));