fixup: drop manual_broadcast_pending and use holder_tx_signed and funding_seen_onchain instead

Author: martinsaposnic

lightning/src/chain/channelmonitor.rs

@@ -1190,11 +1190,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	/// True if this channel was configured for manual funding broadcasts. Monitors written by
 	/// versions prior to introducing the flag will load with `false` until a new update persists it.
 	is_manual_broadcast: bool,
-	/// True once we've observed either funding transaction on-chain. Older monitors assume this is
-	/// `true` when absent during upgrade so holder broadcasts aren't gated unexpectedly.
+	/// True once we've observed either funding transaction on-chain. Older monitors assume
+	/// this is `true` when absent during upgrade so holder broadcasts aren't gated unexpectedly.
+	/// In manual-broadcast channels we also use this to trigger deferred holder
+	/// broadcasts once the funding transaction finally appears on-chain.
 	funding_seen_onchain: bool,
-	/// Tracks whether manual-broadcasting was requested before the funding transaction appeared on-chain.
-	manual_broadcast_pending: bool,
 
 	latest_update_id: u64,
 	commitment_transaction_number_obscure_factor: u64,
@@ -1736,7 +1736,6 @@ pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
 		(34, channel_monitor.alternative_funding_confirmed, option),
 		(35, channel_monitor.is_manual_broadcast, required),
 		(37, channel_monitor.funding_seen_onchain, required),
-		(39, channel_monitor.manual_broadcast_pending, required),
 	});
 
 	Ok(())
@@ -1914,7 +1913,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 
 			is_manual_broadcast,
 			funding_seen_onchain: false,
-			manual_broadcast_pending: false,
 
 			latest_update_id: 0,
 			commitment_transaction_number_obscure_factor,
@@ -3986,7 +3984,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		// the funding transaction on-chain, do not queue any transactions.
 		if require_funding_seen && self.is_manual_broadcast && !self.funding_seen_onchain {
 			log_info!(logger, "Not broadcasting holder commitment for manual-broadcast channel before funding appears on-chain");
-			self.manual_broadcast_pending = true;
 			return;
 		}
 		let reason = ClosureReason::HolderForceClosed {
@@ -3999,7 +3996,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			claimable_outpoints, self.best_block.height, self.best_block.height, broadcaster,
 			conf_target, &self.destination_script, fee_estimator, logger,
 		);
-		self.manual_broadcast_pending = false;
 	}
 
 	fn renegotiated_funding<L: Deref>(
@@ -5259,6 +5255,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		F::Target: FeeEstimator,
 		L::Target: Logger,
 	{
+		let funding_seen_before = self.funding_seen_onchain;
 		let txn_matched = self.filter_block(txdata);
 		for tx in &txn_matched {
 			let mut output_val = Amount::ZERO;
@@ -5281,7 +5278,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		let mut watch_outputs = Vec::new();
 		let mut claimable_outpoints = Vec::new();
 
-		if self.is_manual_broadcast && self.funding_seen_onchain && self.manual_broadcast_pending {
+		if self.is_manual_broadcast && !funding_seen_before && self.funding_seen_onchain && self.holder_tx_signed
+		{
 			self.queue_latest_holder_commitment_txn_for_broadcast(
 				&broadcaster,
 				fee_estimator,
@@ -5507,12 +5505,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			// manual-broadcast mode.
 			let (mut claimables, mut outputs) =
 				self.generate_claimable_outpoints_and_watch_outputs(None);
-			if self.is_manual_broadcast && !self.funding_seen_onchain {
-				self.manual_broadcast_pending = true;
-			} else {
+			if !(self.is_manual_broadcast && !self.funding_seen_onchain) {
 				claimable_outpoints.append(&mut claimables);
 				watch_outputs.append(&mut outputs);
-				self.manual_broadcast_pending = false;
 			}
 		}
 
@@ -5552,12 +5547,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
 			let (mut new_outpoints, mut new_outputs) =
 				self.generate_claimable_outpoints_and_watch_outputs(Some(reason));
-			if self.is_manual_broadcast && !self.funding_seen_onchain {
-				self.manual_broadcast_pending = true;
-			} else {
+			if !(self.is_manual_broadcast && !self.funding_seen_onchain) {
 				claimable_outpoints.append(&mut new_outpoints);
 				watch_outputs.append(&mut new_outputs);
-				self.manual_broadcast_pending = false;
 			}
 		}
 
@@ -6525,7 +6517,6 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut alternative_funding_confirmed = None;
 		let mut is_manual_broadcast = None;
 		let mut funding_seen_onchain = None;
-		let mut manual_broadcast_pending = None;
 		read_tlv_fields!(reader, {
 			(1, funding_spend_confirmed, option),
 			(3, htlcs_resolved_on_chain, optional_vec),
@@ -6548,7 +6539,6 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(34, alternative_funding_confirmed, option),
 			(35, is_manual_broadcast, option),
 			(37, funding_seen_onchain, option),
-			(39, manual_broadcast_pending, option),
 		});
 		// Note that `payment_preimages_with_info` was added (and is always written) in LDK 0.1, so
 		// we can use it to determine if this monitor was last written by LDK 0.1 or later.
@@ -6583,7 +6573,6 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let channel_parameters = channel_parameters.unwrap_or_else(|| {
 			onchain_tx_handler.channel_parameters().clone()
 		});
-		let manual_broadcast_pending = manual_broadcast_pending.unwrap_or(false);
 
 		// Monitors for anchor outputs channels opened in v0.0.116 suffered from a bug in which the
 		// wrong `counterparty_payment_script` was being tracked. Fix it now on deserialization to
@@ -6670,7 +6659,6 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			is_manual_broadcast: is_manual_broadcast.unwrap_or(false),
 			// Assume "seen" when absent to prevent gating holder broadcasts after upgrade.
 			funding_seen_onchain: funding_seen_onchain.unwrap_or(true),
-			manual_broadcast_pending,
 
 			latest_update_id,
 			commitment_transaction_number_obscure_factor,
@@ -7163,7 +7151,6 @@ mod tests {
 		{
 			let inner = monitor.inner.lock().unwrap();
 			assert!(!inner.funding_seen_onchain);
-			assert!(inner.manual_broadcast_pending);
 		}
 
 		// Now confirm the funding transaction via transactions_confirmed.
@@ -7180,7 +7167,7 @@ mod tests {
 		);
 		{
 			let inner = monitor.inner.lock().unwrap();
-			assert!(!inner.manual_broadcast_pending);
+			assert!(inner.funding_seen_onchain);
 		}
 
 		// Next height update should allow broadcast.