Move tx_complete phase transition to Channel

Now that ChannelPhase is encapsulated in Channel, phase transitions can
be moved from ChannelManager to Channel. Update the tx_complete phase
transition accordingly. This allows for simpler logic in ChannelManager
since the channel does not need to removed and then re-added into the
channel_by_id map.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1243,14 +1243,6 @@ impl<SP: Deref> Channel<SP> where
 		}
 	}
 
-	pub fn into_unfunded_v2(self) -> Option<PendingV2Channel<SP>> {
-		if let ChannelPhase::UnfundedV2(channel) = self.phase {
-			Some(channel)
-		} else {
-			None
-		}
-	}
-
 	pub fn signer_maybe_unblocked<L: Deref>(
 		&mut self, chain_hash: ChainHash, logger: &L,
 	) -> Option<SignerResumeUpdates> where L::Target: Logger {
@@ -1425,6 +1417,34 @@ impl<SP: Deref> Channel<SP> where
 
 		debug_assert!(!matches!(self.phase, ChannelPhase::Undefined));
 	}
+
+	pub fn funding_tx_constructed<L: Deref>(
+		&mut self, signing_session: InteractiveTxSigningSession, logger: &L
+	) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
+	where
+		L::Target: Logger
+	{
+		let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
+		let result = if let ChannelPhase::UnfundedV2(chan) = 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)
+				},
+			}
+		} 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
+	}
 }
 
 impl<SP: Deref> From<OutboundV1Channel<SP>> for Channel<SP>
@@ -2073,8 +2093,8 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	}
 
 	pub fn funding_tx_constructed<L: Deref>(
-		&mut self, signing_session: &mut InteractiveTxSigningSession, logger: &L
-	) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
+		mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
+	) -> Result<(FundedChannel<SP>, msgs::CommitmentSigned, Option<Event>), (PendingV2Channel<SP>, ChannelError)>
 	where
 		L::Target: Logger
 	{
@@ -2090,7 +2110,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);
 			}
@@ -2102,7 +2122,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);
@@ -2117,6 +2137,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));
 			},
 		};
 
@@ -2151,7 +2172,24 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 		// Clear the interactive transaction constructor
 		self.interactive_tx_constructor.take();
 
-		Ok((commitment_signed, funding_ready_for_sig_event))
+		match self.unfunded_context.holder_commitment_point {
+			Some(holder_commitment_point) => {
+				let funded_chan = FundedChannel {
+					context: self.context,
+					interactive_tx_signing_session: Some(signing_session),
+					holder_commitment_point,
+				};
+				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))
+			},
+		}
 	}
 }
 
@@ -9422,19 +9460,6 @@ 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_channel(self, signing_session: InteractiveTxSigningSession) -> Result<FundedChannel<SP>, ChannelError>{
-		let holder_commitment_point = self.unfunded_context.holder_commitment_point.ok_or(ChannelError::close(
-			format!("Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
-			self.context.channel_id())))?;
-		let channel = FundedChannel {
-			context: self.context,
-			interactive_tx_signing_session: Some(signing_session),
-			holder_commitment_point,
-		};
-
-		Ok(channel)
-	}
 }
 
 // Unfunded channel utilities

lightning/src/ln/channelmanager.rs

@@ -8291,26 +8291,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				if let Some(msg_send_event) = msg_send_event_opt {
 					peer_state.pending_msg_events.push(msg_send_event);
 				};
-				if let Some(mut signing_session) = signing_session_opt {
-					let (commitment_signed, funding_ready_for_sig_event_opt) = match chan_entry.get_mut().as_unfunded_v2_mut() {
-						Some(chan) => {
-							chan.funding_tx_constructed(&mut signing_session, &self.logger)
-						},
-						None => Err(ChannelError::Warn(
-							"Got a tx_complete message with no interactive transaction construction expected or in-progress"
-							.into())),
-					}.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?;
-					let (channel_id, channel) = chan_entry.remove_entry();
-					let channel = match channel.into_unfunded_v2() {
-						Some(chan) => chan.into_channel(signing_session),
-						None => {
-							debug_assert!(false); // It cannot be another variant as we are in the `Ok` branch of the above match.
-							Err(ChannelError::Warn(
-								"Got a tx_complete message with no interactive transaction construction expected or in-progress"
-									.into()))
-						},
-					}.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?;
-					peer_state.channel_by_id.insert(channel_id, Channel::from(channel));
+				if let Some(signing_session) = signing_session_opt {
+					let (commitment_signed, funding_ready_for_sig_event_opt) = chan_entry
+						.get_mut()
+						.funding_tx_constructed(signing_session, &self.logger)
+						.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?;
 					if let Some(funding_ready_for_sig_event) = funding_ready_for_sig_event_opt {
 						let mut pending_events = self.pending_events.lock().unwrap();
 						pending_events.push_back((funding_ready_for_sig_event, None));