Add flag to disable broadcasting when it's dangerous due to information loss

Author: QPotato

lightning/src/chain/channelmonitor.rs

@@ -723,6 +723,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
 	best_block: BestBlock,
 
 	secp_ctx: Secp256k1<secp256k1::All>, //TODO: dedup this a bit...
+
+	// Used to track that the channel was updated with ChannelForceClosed {should_broadcast: false}
+	// implying that it's unsafe to broadcast the latest holder commitment transaction.
+	allow_automated_broadcast: bool,
 }
 
 /// Transaction outputs to watch for on-chain spends.
@@ -1059,6 +1063,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 				best_block,
 
 				secp_ctx,
+
+				allow_automated_broadcast: true,
 			}),
 		}
 	}
@@ -1110,15 +1116,26 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 			payment_hash, payment_preimage, broadcaster, fee_estimator, logger)
 	}
 
-	pub(crate) fn broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(
+	pub(crate) fn maybe_broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(
+		&self,
+		broadcaster: &B,
+		logger: &L,
+	) where
+		B::Target: BroadcasterInterface,
+		L::Target: Logger,
+	{
+		self.inner.lock().unwrap().maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger)
+	}
+
+	pub(crate) fn force_broadcast_latest_holder_commitment_txn_unsafe<B: Deref, L: Deref>(
 		&self,
 		broadcaster: &B,
 		logger: &L,
 	) where
 		B::Target: BroadcasterInterface,
 		L::Target: Logger,
 	{
-		self.inner.lock().unwrap().broadcast_latest_holder_commitment_txn(broadcaster, logger)
+		self.inner.lock().unwrap().force_broadcast_latest_holder_commitment_txn_unsafe(broadcaster, logger)
 	}
 
 	/// Updates a ChannelMonitor on the basis of some new information provided by the Channel
@@ -1923,7 +1940,22 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		}
 	}
 
-	pub(crate) fn broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(&mut self, broadcaster: &B, logger: &L)
+	pub(crate) fn maybe_broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(&mut self, broadcaster: &B, logger: &L)
+		where B::Target: BroadcasterInterface,
+					L::Target: Logger,
+	{
+		if self.allow_automated_broadcast{
+			for tx in self.get_latest_holder_commitment_txn(logger).iter() {
+				log_info!(logger, "Broadcasting local {}", log_tx!(tx));
+				broadcaster.broadcast_transaction(tx);
+			}
+			self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
+		} else {
+			log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take");
+		}
+	}
+
+	pub(crate) fn force_broadcast_latest_holder_commitment_txn_unsafe<B: Deref, L: Deref>(&mut self, broadcaster: &B, logger: &L)
 		where B::Target: BroadcasterInterface,
 					L::Target: Logger,
 	{
@@ -1934,6 +1966,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
 	}
 
+	
+
 	pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()>
 	where B::Target: BroadcasterInterface,
 		    F::Target: FeeEstimator,
@@ -1989,8 +2023,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 					log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast);
 					self.lockdown_from_offchain = true;
 					if *should_broadcast {
-						self.broadcast_latest_holder_commitment_txn(broadcaster, logger);
+						self.maybe_broadcast_latest_holder_commitment_txn(broadcaster, logger);
 					} else if !self.holder_tx_signed {
+						self.allow_automated_broadcast = false;
 						log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take");
 					} else {
 						// If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
@@ -3397,6 +3432,8 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 				best_block,
 
 				secp_ctx,
+
+				allow_automated_broadcast: true,
 			}),
 		}))
 	}

lightning/src/ln/channelmanager.rs

@@ -5267,7 +5267,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// Channel object.
 	fn handle_init_event_channel_failures(&self, mut failed_channels: Vec<ShutdownResult>) {
 		for mut failure in failed_channels.drain(..) {
-			// Either a commitment transactions has been confirmed on-chain or
+			// Either a commitment transaction has been confirmed on-chain or
 			// Channel::block_disconnected detected that the funding transaction has been
 			// reorganized out of the main chain.
 			// We cannot broadcast our latest local state via monitor update (as
@@ -6805,7 +6805,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 						log_bytes!(channel.channel_id()), monitor.get_latest_update_id(), channel.get_latest_monitor_update_id());
 					let (_, mut new_failed_htlcs) = channel.force_shutdown(true);
 					failed_htlcs.append(&mut new_failed_htlcs);
-					monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
+					monitor.maybe_broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
 					channel_closures.push(events::Event::ChannelClosed {
 						channel_id: channel.channel_id(),
 						user_channel_id: channel.get_user_id(),
@@ -6831,7 +6831,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		for (ref funding_txo, ref mut monitor) in args.channel_monitors.iter_mut() {
 			if !funding_txo_set.contains(funding_txo) {
 				log_info!(args.logger, "Broadcasting latest holder commitment transaction for closed channel {}", log_bytes!(funding_txo.to_channel_id()));
-				monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
+				monitor.maybe_broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
 			}
 		}