Promote V2 channels to FundedChannel on initial commitment_signed receipt

dunxen · dunxen · commit 2630da8a9008 · 2025-02-26T20:55:02.000+02:00
Before this commit, unfunded V2 channels were promoted to `FundedChannel`s in
`PendingV2Channel::funding_tx_constructed`.
Since a monitor is only created upon receipt of an initial
`commitment_signed`, this would cause a crash if the channel was read
from persisted data between those two events.

Consequently, we also need to hold an `interactive_tx_signing_session`
for both of our unfunded V2 channel structs.
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -1478,32 +1478,56 @@ impl<SP: Deref> Channel<SP> where
 	where
 		L::Target: Logger
 	{
-		let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
-		let result = if let ChannelPhase::UnfundedV2(chan) = phase {
+		let result = if let ChannelPhase::UnfundedV2(chan) = &mut self.phase {
 			let logger = WithChannelContext::from(logger, &chan.context, None);
-			match chan.funding_tx_constructed(signing_session, &&logger) {
-				Ok((chan, commitment_signed, event)) => {
-					self.phase = ChannelPhase::Funded(chan);
-					Ok((commitment_signed, event))
-				},
-				Err((chan, e)) => {
-					self.phase = ChannelPhase::UnfundedV2(chan);
-					Err(e)
-				},
-			}
+			chan.funding_tx_constructed(signing_session, &&logger)
 		} else {
-			self.phase = phase;
 			Err(ChannelError::Warn("Got a tx_complete message with no interactive transaction construction expected or in-progress".to_owned()))
 		};
 
-		debug_assert!(!matches!(self.phase, ChannelPhase::Undefined));
 		result
 	}
 
 	pub fn force_shutdown(&mut self, should_broadcast: bool, closure_reason: ClosureReason) -> ShutdownResult {
 		let (funding, context) = self.funding_and_context_mut();
 		context.force_shutdown(funding, should_broadcast, closure_reason)
 	}
+
+	pub fn commitment_signed<L: Deref>(
+		&mut self, msg: &msgs::CommitmentSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
+	) -> Result<(Option<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>>, Option<ChannelMonitorUpdate>), ChannelError>
+	where
+		L::Target: Logger
+	{
+		let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
+		match phase {
+			ChannelPhase::UnfundedV2(chan) => {
+				let mut funded_channel = match chan.into_funded_channel() {
+					Ok(funded_channel) => funded_channel,
+					Err((pending_channel, err)) => {
+						self.phase = ChannelPhase::UnfundedV2(pending_channel);
+						return Err(err);
+					}
+				};
+				let res = match funded_channel.commitment_signed_initial_v2(msg, best_block, signer_provider, logger) {
+					Ok(monitor) => {
+						Ok((Some(monitor), None))
+					},
+					Err(err) => {
+						Err(err)
+					}
+				};
+				self.phase = ChannelPhase::Funded(funded_channel);
+				res
+			},
+			ChannelPhase::Funded(mut funded_channel) => {
+				let res = funded_channel.commitment_signed(msg, logger).map(|monitor_update_opt| (None, monitor_update_opt));
+				self.phase = ChannelPhase::Funded(funded_channel);
+				res
+			},
+			_ => Err(ChannelError::close("Got a commitment_signed message for an unfunded V1 channel!".into())),
+		}
+	}
 }
 
 impl<SP: Deref> From<OutboundV1Channel<SP>> for Channel<SP>
@@ -2194,8 +2218,8 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	}
 
 	pub fn funding_tx_constructed<L: Deref>(
-		mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
-	) -> Result<(FundedChannel<SP>, msgs::CommitmentSigned, Option<Event>), (PendingV2Channel<SP>, ChannelError)>
+		&mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
+	) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
 	where
 		L::Target: Logger
 	{
@@ -2211,7 +2235,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 						(
 							"Multiple outputs matched the expected script and value".to_owned(),
 							ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-						))).map_err(|e| (self, e));
+						)));
 				}
 				output_index = Some(idx as u16);
 			}
@@ -2223,7 +2247,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 				(
 					"No output matched the funding script_pubkey".to_owned(),
 					ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-				))).map_err(|e| (self, e));
+				)));
 		};
 		self.context.channel_transaction_parameters.funding_outpoint = Some(outpoint);
 		self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
@@ -2237,8 +2261,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			},
 			Err(err) => {
 				self.context.channel_transaction_parameters.funding_outpoint = None;
-				return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })))
-					.map_err(|e| (self, e));
+				return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })));
 			},
 		};
 
@@ -2249,10 +2272,10 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 					false,
 					"Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found",
 				);
-				return Err((self, ChannelError::Close((
+				return Err(ChannelError::Close((
 					"V2 channel rejected due to sender error".into(),
 					ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
-				))));
+				)));
 			}
 			None
 		} else {
@@ -2274,37 +2297,19 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 				false,
 				"We don't support users providing inputs but somehow we had more than zero inputs",
 			);
-			return Err((self, ChannelError::Close((
+			return Err(ChannelError::Close((
 				"V2 channel rejected due to sender error".into(),
 				ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
-			))));
+			)));
 		};
 
 		self.context.channel_state = ChannelState::FundingNegotiated;
 
 		// Clear the interactive transaction constructor
 		self.interactive_tx_constructor.take();
+		self.interactive_tx_signing_session = Some(signing_session);
 
-		match self.unfunded_context.holder_commitment_point {
-			Some(holder_commitment_point) => {
-				let funded_chan = FundedChannel {
-					funding: self.funding,
-					context: self.context,
-					interactive_tx_signing_session: Some(signing_session),
-					holder_commitment_point,
-					is_v2_established: true,
-				};
-				Ok((funded_chan, commitment_signed, funding_ready_for_sig_event))
-			},
-			None => {
-				Err(ChannelError::close(
-					format!(
-						"Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
-						self.context.channel_id(),
-					)))
-					.map_err(|e| (self, e))
-			},
-		}
+		Ok((commitment_signed, funding_ready_for_sig_event))
 	}
 }
 
@@ -9511,6 +9516,8 @@ pub(super) struct PendingV2Channel<SP: Deref> where SP::Target: SignerProvider {
 	pub dual_funding_context: DualFundingChannelContext,
 	/// The current interactive transaction construction session under negotiation.
 	pub interactive_tx_constructor: Option<InteractiveTxConstructor>,
+	/// The signing session created after `tx_complete` handling
+	pub interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
 }
 
 impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
@@ -9576,6 +9583,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 				our_funding_inputs: funding_inputs,
 			},
 			interactive_tx_constructor: None,
+			interactive_tx_signing_session: None,
 		};
 		Ok(chan)
 	}
@@ -9747,6 +9755,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			context,
 			dual_funding_context,
 			interactive_tx_constructor,
+			interactive_tx_signing_session: None,
 			unfunded_context,
 		})
 	}
@@ -9824,6 +9833,25 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	pub fn get_accept_channel_v2_message(&self) -> msgs::AcceptChannelV2 {
 		self.generate_accept_channel_v2_message()
 	}
+
+	pub fn into_funded_channel(self) -> Result<FundedChannel<SP>, (PendingV2Channel<SP>, ChannelError)> {
+		let holder_commitment_point = match self.unfunded_context.holder_commitment_point {
+			Some(point) => point,
+			None => {
+				let channel_id = self.context.channel_id();
+				return Err((self, ChannelError::close(
+					format!("Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
+						channel_id))));
+			}
+		};
+		Ok(FundedChannel {
+			funding: self.funding,
+			context: self.context,
+			interactive_tx_signing_session: self.interactive_tx_signing_session,
+			holder_commitment_point,
+			is_v2_established: true,
+		})
+	}
 }
 
 // Unfunded channel utilities
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -8950,14 +8950,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		let peer_state = &mut *peer_state_lock;
 		match peer_state.channel_by_id.entry(msg.channel_id) {
 			hash_map::Entry::Occupied(mut chan_entry) => {
-				if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
-					let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-					let funding_txo = chan.context.get_funding_txo();
-
-					if chan.interactive_tx_signing_session.is_some() {
-						let monitor = try_channel_entry!(
-							self, peer_state, chan.commitment_signed_initial_v2(msg, best_block, &self.signer_provider, &&logger),
-							chan_entry);
+				let chan = chan_entry.get_mut();
+				let logger = WithChannelContext::from(&self.logger, &chan.context(), None);
+				let funding_txo = chan.context().get_funding_txo();
+				let (monitor_opt, monitor_update_opt) = try_channel_entry!(
+					self, peer_state, chan.commitment_signed(msg, best_block, &self.signer_provider, &&logger),
+					chan_entry);
+
+				if let Some(chan) = chan.as_funded_mut() {
+					if let Some(monitor) = monitor_opt {
 						let monitor_res = self.chain_monitor.watch_channel(monitor.channel_id(), monitor);
 						if let Ok(persist_state) = monitor_res {
 							handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
@@ -8972,19 +8973,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 								)
 							)), chan_entry)
 						}
-					} else {
-						let monitor_update_opt = try_channel_entry!(
-							self, peer_state, chan.commitment_signed(msg, &&logger), chan_entry);
-						if let Some(monitor_update) = monitor_update_opt {
-							handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
-								peer_state, per_peer_state, chan);
-						}
+					} else if let Some(monitor_update) = monitor_update_opt {
+						handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
+							peer_state, per_peer_state, chan);
 					}
-					Ok(())
-				} else {
-					return try_channel_entry!(self, peer_state, Err(ChannelError::close(
-						"Got a commitment_signed message for an unfunded channel!".into())), chan_entry);
 				}
+				Ok(())
 			},
 			hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
 		}
