Remove unnecessary funding tx clone & remote tx_sigs received check

We actually don't need to check if the counterparty had already sent
their `tx_signatures` in `InteractiveTxSigningSession::received_tx_signatures`.

Further, we can get rid of the clone of `funding_tx_opt` in
`Channel::tx_signatures` when setting the `Context::funding_transaction`
as we don't actually need to propagate it through to
`ChannelManager::internal_tx_complete` as we can use
`ChannelContext::unbroadcasted_funding()` which clones the
`ChannelContext::funding_transaction` anyway.

Author: dunxen

lightning/src/ln/channel.rs

@@ -5703,7 +5703,7 @@ impl<SP: Deref> Channel<SP> where
 		}
 	}
 
-	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError>
+	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<Option<msgs::TxSignatures>, ChannelError>
 		where L::Target: Logger
 	{
 		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
@@ -5740,25 +5740,23 @@ impl<SP: Deref> Channel<SP> where
 				// for spending. Doesn't seem to be anything in rust-bitcoin.
 			}
 
-			let (tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone())
+			let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone())
 				.map_err(|_| ChannelError::Warn("Witness count did not match contributed input count".to_string()))?;
+			if holder_tx_signatures_opt.is_some() && self.is_awaiting_initial_mon_persist() {
+				log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
+				self.context.monitor_pending_tx_signatures = holder_tx_signatures_opt;
+				return Ok(None);
+			}
 			if funding_tx_opt.is_some() {
+				// We have a persisted channel monitor and and a finalized funding transaction, so we can move
+				// the channel state forward, set the funding transaction and reset the signing session fields.
+				self.context.funding_transaction = funding_tx_opt;
+				self.context.next_funding_txid = None;
+				self.interactive_tx_signing_session = None;
 				self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
 			}
-			self.context.funding_transaction = funding_tx_opt.clone();
-
-			self.context.next_funding_txid = None;
-
-			// Clear out the signing session
-			self.interactive_tx_signing_session = None;
-
-			if tx_signatures_opt.is_some() && self.context.channel_state.is_monitor_update_in_progress() {
-				log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
-				self.context.monitor_pending_tx_signatures = tx_signatures_opt;
-				return Ok((None, None));
-			}
 
-			Ok((tx_signatures_opt, funding_tx_opt))
+			Ok(holder_tx_signatures_opt)
 		} else {
 			Err(ChannelError::Close((
 				"Unexpected tx_signatures. No funding transaction awaiting signatures".to_string(),

lightning/src/ln/channelmanager.rs

@@ -8415,14 +8415,14 @@ where
 				match channel_phase {
 					ChannelPhase::Funded(chan) => {
 						let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-						let (tx_signatures_opt, funding_tx_opt) = try_chan_phase_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_phase_entry);
+						let tx_signatures_opt = try_chan_phase_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_phase_entry);
 						if let Some(tx_signatures) = tx_signatures_opt {
 							peer_state.pending_msg_events.push(events::MessageSendEvent::SendTxSignatures {
 								node_id: *counterparty_node_id,
 								msg: tx_signatures,
 							});
 						}
-						if let Some(ref funding_tx) = funding_tx_opt {
+						if let Some(ref funding_tx) = chan.context.unbroadcasted_funding() {
 							self.tx_broadcaster.broadcast_transactions(&[funding_tx]);
 							{
 								let mut pending_events = self.pending_events.lock().unwrap();

lightning/src/ln/interactivetxs.rs

@@ -316,14 +316,18 @@ impl InteractiveTxSigningSession {
 
 	/// Handles a `tx_signatures` message received from the counterparty.
 	///
+	/// If the holder is required to send their `tx_signatures` message and these signatures have
+	/// already been provided to the signing session, then this return value will be `Some`, otherwise
+	/// None.
+	///
+	/// If the holder has already provided their `tx_signatures` to the signing session, a funding
+	/// transaction will be finalized and returned as Some, otherwise None.
+	///
 	/// Returns an error if the witness count does not equal the counterparty's input count in the
 	/// unsigned transaction.
 	pub fn received_tx_signatures(
 		&mut self, tx_signatures: TxSignatures,
 	) -> Result<(Option<TxSignatures>, Option<Transaction>), ()> {
-		if self.counterparty_sent_tx_signatures {
-			return Ok((None, None));
-		};
 		if self.remote_inputs_count() != tx_signatures.witnesses.len() {
 			return Err(());
 		}
@@ -336,13 +340,16 @@ impl InteractiveTxSigningSession {
 			None
 		};
 
-		let funding_tx = if self.holder_tx_signatures.is_some() {
+		// Check if the holder has provided its signatures and if so,
+		// return the finalized funding transaction.
+		let funding_tx_opt = if self.holder_tx_signatures.is_some() {
 			Some(self.finalize_funding_tx())
 		} else {
+			// This means we're still waiting for the holder to provide their signatures.
 			None
 		};
 
-		Ok((holder_tx_signatures, funding_tx))
+		Ok((holder_tx_signatures, funding_tx_opt))
 	}
 
 	/// Provides the holder witnesses for the unsigned transaction.