f wip: handling async monitor persist and checks for tx_signatures

Author: dunxen

lightning/src/ln/channel.rs

@@ -902,6 +902,7 @@ pub(super) struct MonitorRestoreUpdates {
 	pub funding_broadcastable: Option<Transaction>,
 	pub channel_ready: Option<msgs::ChannelReady>,
 	pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
+	pub tx_signatures: Option<msgs::TxSignatures>,
 }
 
 /// The return value of `signer_maybe_unblocked`
@@ -1253,6 +1254,7 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
 	monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
 	monitor_pending_finalized_fulfills: Vec<HTLCSource>,
 	monitor_pending_update_adds: Vec<msgs::UpdateAddHTLC>,
+	monitor_pending_tx_signatures: Option<msgs::TxSignatures>,
 
 	/// If we went to send a revoke_and_ack but our signer was unable to give us a signature,
 	/// we should retry at some point in the future when the signer indicates it may have a
@@ -2092,6 +2094,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			monitor_pending_failures: Vec::new(),
 			monitor_pending_finalized_fulfills: Vec::new(),
 			monitor_pending_update_adds: Vec::new(),
+			monitor_pending_tx_signatures: None,
 
 			signer_pending_revoke_and_ack: false,
 			signer_pending_commitment_update: false,
@@ -2328,6 +2331,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			monitor_pending_failures: Vec::new(),
 			monitor_pending_finalized_fulfills: Vec::new(),
 			monitor_pending_update_adds: Vec::new(),
+			monitor_pending_tx_signatures: None,
 
 			signer_pending_revoke_and_ack: false,
 			signer_pending_commitment_update: false,
@@ -5663,7 +5667,13 @@ impl<SP: Deref> Channel<SP> where
 		}
 	}
 
-	pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError> {
+	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError>
+		where L::Target: Logger
+	{
+		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
+			return Err(ChannelError::close("Received tx_signatures in strange state!".to_owned()));
+		}
+
 		if let Some(ref mut signing_session) = self.interactive_tx_signing_session {
 			if msg.witnesses.len() != signing_session.remote_inputs_count() {
 				return Err(ChannelError::Close(
@@ -5707,13 +5717,18 @@ impl<SP: Deref> Channel<SP> where
 			// 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))
 		} else {
-			return Err(ChannelError::Close(
-				(
+			Err(ChannelError::Close((
 					"Unexpected tx_signatures. No funding transaction awaiting signatures".to_string(),
 					ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-				)));
+				)))
 		}
 	}
 
@@ -5950,14 +5965,18 @@ impl<SP: Deref> Channel<SP> where
 		mem::swap(&mut finalized_claimed_htlcs, &mut self.context.monitor_pending_finalized_fulfills);
 		let mut pending_update_adds = Vec::new();
 		mem::swap(&mut pending_update_adds, &mut self.context.monitor_pending_update_adds);
+		// For channels established with V2 establishment we won't send a `tx_signatures` when we're in
+		// MonitorUpdateInProgress (and we assume the user will never directly broadcast the funding
+		// transaction and waits for us to do it).
+		let tx_signatures = self.context.monitor_pending_tx_signatures.take();
 
 		if self.context.channel_state.is_peer_disconnected() {
 			self.context.monitor_pending_revoke_and_ack = false;
 			self.context.monitor_pending_commitment_signed = false;
 			return MonitorRestoreUpdates {
 				raa: None, commitment_update: None, order: RAACommitmentOrder::RevokeAndACKFirst,
 				accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, pending_update_adds,
-				funding_broadcastable, channel_ready, announcement_sigs
+				funding_broadcastable, channel_ready, announcement_sigs, tx_signatures
 			};
 		}
 
@@ -5991,7 +6010,7 @@ impl<SP: Deref> Channel<SP> where
 			match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
 		MonitorRestoreUpdates {
 			raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs,
-			pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs
+			pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures
 		}
 	}
 
@@ -9989,6 +10008,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 				monitor_pending_failures,
 				monitor_pending_finalized_fulfills: monitor_pending_finalized_fulfills.unwrap(),
 				monitor_pending_update_adds: monitor_pending_update_adds.unwrap_or_default(),
+				monitor_pending_tx_signatures: None,
 
 				signer_pending_revoke_and_ack: false,
 				signer_pending_commitment_update: false,

lightning/src/ln/channelmanager.rs

@@ -3108,7 +3108,7 @@ macro_rules! handle_monitor_update_completion {
 			&mut $peer_state.pending_msg_events, $chan, updates.raa,
 			updates.commitment_update, updates.order, updates.accepted_htlcs, updates.pending_update_adds,
 			updates.funding_broadcastable, updates.channel_ready,
-			updates.announcement_sigs);
+			updates.announcement_sigs, updates.tx_signatures);
 		if let Some(upd) = channel_update {
 			$peer_state.pending_msg_events.push(upd);
 		}
@@ -7415,17 +7415,20 @@ where
 		commitment_update: Option<msgs::CommitmentUpdate>, order: RAACommitmentOrder,
 		pending_forwards: Vec<(PendingHTLCInfo, u64)>, pending_update_adds: Vec<msgs::UpdateAddHTLC>,
 		funding_broadcastable: Option<Transaction>,
-		channel_ready: Option<msgs::ChannelReady>, announcement_sigs: Option<msgs::AnnouncementSignatures>)
-	-> (Option<(u64, Option<PublicKey>, OutPoint, ChannelId, u128, Vec<(PendingHTLCInfo, u64)>)>, Option<(u64, Vec<msgs::UpdateAddHTLC>)>) {
+		channel_ready: Option<msgs::ChannelReady>, announcement_sigs: Option<msgs::AnnouncementSignatures>,
+		tx_signatures: Option<msgs::TxSignatures>
+	) -> (Option<(u64, Option<PublicKey>, OutPoint, ChannelId, u128, Vec<(PendingHTLCInfo, u64)>)>, Option<(u64, Vec<msgs::UpdateAddHTLC>)>) {
 		let logger = WithChannelContext::from(&self.logger, &channel.context, None);
-		log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement",
+		log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures",
 			&channel.context.channel_id(),
 			if raa.is_some() { "an" } else { "no" },
 			if commitment_update.is_some() { "a" } else { "no" },
 			pending_forwards.len(), pending_update_adds.len(),
 			if funding_broadcastable.is_some() { "" } else { "not " },
 			if channel_ready.is_some() { "sending" } else { "without" },
-			if announcement_sigs.is_some() { "sending" } else { "without" });
+			if announcement_sigs.is_some() { "sending" } else { "without" },
+			if tx_signatures.is_some() { "sending" } else { "without" },
+		);
 
 		let counterparty_node_id = channel.context.get_counterparty_node_id();
 		let short_channel_id = channel.context.get_short_channel_id().unwrap_or(channel.context.outbound_scid_alias());
@@ -7452,6 +7455,12 @@ where
 				msg,
 			});
 		}
+		if let Some(msg) = tx_signatures {
+			pending_msg_events.push(events::MessageSendEvent::SendTxSignatures {
+				node_id: counterparty_node_id,
+				msg,
+			});
+		}
 
 		macro_rules! handle_cs { () => {
 			if let Some(update) = commitment_update {
@@ -8343,7 +8352,8 @@ where
 				let channel_phase = chan_phase_entry.get_mut();
 				match channel_phase {
 					ChannelPhase::Funded(chan) => {
-						let (tx_signatures_opt, funding_tx_opt) = try_chan_phase_entry!(self, chan.tx_signatures(msg), chan_phase_entry);
+						let logger = WithChannelContext::from(&self.logger, &chan.context, None);
+						let (tx_signatures_opt, funding_tx_opt) = try_chan_phase_entry!(self, 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,
@@ -9221,7 +9231,7 @@ where
 						let need_lnd_workaround = chan.context.workaround_lnd_bug_4006.take();
 						let (htlc_forwards, decode_update_add_htlcs) = self.handle_channel_resumption(
 							&mut peer_state.pending_msg_events, chan, responses.raa, responses.commitment_update, responses.order,
-							Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs);
+							Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs, None);
 						debug_assert!(htlc_forwards.is_none());
 						debug_assert!(decode_update_add_htlcs.is_none());
 						if let Some(upd) = channel_update {