Wait for reorg safety when promoting zero conf funding scopes

If we don't, it's possible that an alternative funding transaction
confirms instead of the one that was locked and we're not able to
broadcast a holder commitment to recover our funds.

Author: wpaulino

lightning/src/chain/channelmonitor.rs

@@ -534,7 +534,9 @@ enum OnchainEvent {
 	},
 	/// An output waiting on [`ANTI_REORG_DELAY`] confirmations before we hand the user the
 	/// [`SpendableOutputDescriptor`].
-	MaturingOutput { descriptor: SpendableOutputDescriptor },
+	MaturingOutput {
+		descriptor: SpendableOutputDescriptor,
+	},
 	/// A spend of the funding output, either a commitment transaction or a cooperative closing
 	/// transaction.
 	FundingSpendConfirmation {
@@ -578,6 +580,16 @@ enum OnchainEvent {
 		// considered locked.
 		confirmation_depth: u32,
 	},
+	// We've negotiated and locked (via a `RenegotiatedFundingLocked` monitor update) a zero conf
+	// funding transcation. If confirmations were required, we'd remove all alternative funding
+	// transactions and their associated holder commitment transactions, as we can assume they can
+	// no longer confirm. However, with zero conf funding transactions, we cannot guarantee this.
+	// Ultimately an alternative funding transaction could actually end up on chain, and we must
+	// still be able to claim our funds from it.
+	//
+	// This event will only be generated when a `RenegotiatedFundingLocked` monitor update is
+	// applied for a zero conf funding transaction.
+	ZeroConfFundingReorgSafety {},
 }
 
 impl Writeable for OnchainEventEntry {
@@ -629,6 +641,7 @@ impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
 		(0, on_local_output_csv, option),
 		(1, commitment_tx_to_counterparty_output, option),
 	},
+	(4, ZeroConfFundingReorgSafety) => {},
 	(5, HTLCSpendConfirmation) => {
 		(0, commitment_tx_output_idx, required),
 		(2, preimage, option),
@@ -1301,6 +1314,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	// commitment transactions, their ordering with respect to each other must remain the same.
 	current_holder_htlc_data: CommitmentHTLCData,
 	prev_holder_htlc_data: Option<CommitmentHTLCData>,
+
+	// If a funding transaction has been renegotiated for the channel and it requires zero
+	// confirmations to be considered locked, we wait for `ANTI_REORG_DELAY` confirmations until we
+	// no longer track alternative funding candidates.
+	wait_for_0conf_funding_reorg_safety: bool,
 }
 
 // Macro helper to access holder commitment HTLC data (including both non-dust and dust) while
@@ -1558,6 +1576,8 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
 			_ => self.pending_monitor_events.clone(),
 		};
 
+		let wait_for_0conf_funding_reorg_safety = self.wait_for_0conf_funding_reorg_safety.then(|| ());
+
 		write_tlv_fields!(writer, {
 			(1, self.funding_spend_confirmed, option),
 			(3, self.htlcs_resolved_on_chain, required_vec),
@@ -1576,6 +1596,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
 			(29, self.initial_counterparty_commitment_tx, option),
 			(31, self.funding.channel_parameters, required),
 			(32, self.pending_funding, optional_vec),
+			(34, wait_for_0conf_funding_reorg_safety, option),
 		});
 
 		Ok(())
@@ -1801,6 +1822,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 			// There are never any HTLCs in the initial commitment transaction
 			current_holder_htlc_data: CommitmentHTLCData::new(),
 			prev_holder_htlc_data: None,
+
+			wait_for_0conf_funding_reorg_safety: false,
 		})
 	}
 
@@ -3837,23 +3860,29 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		Ok(())
 	}
 
-	fn promote_funding(&mut self, new_funding_txid: Txid) -> Result<(), ()> {
+	fn promote_funding(
+		&mut self, new_funding_txid: Txid, from_monitor_update: bool,
+	) -> Result<(), ()> {
 		let new_funding = self
 			.pending_funding
 			.iter_mut()
-			.map(|pending_funding| &mut pending_funding.0)
-			.find(|funding| funding.funding_txid() == new_funding_txid);
+			.find(|pending_funding| pending_funding.0.funding_txid() == new_funding_txid);
 		if new_funding.is_none() {
 			return Err(());
 		}
-		let mut new_funding = new_funding.unwrap();
+		let (new_funding, confirmation_depth) = new_funding.unwrap();
 
-		mem::swap(&mut self.funding, &mut new_funding);
+		mem::swap(&mut self.funding, new_funding);
 		self.onchain_tx_handler.update_after_renegotiated_funding_locked(
 			self.funding.current_holder_commitment_tx.clone(),
 			self.funding.prev_holder_commitment_tx.clone(),
 		);
 
+		if from_monitor_update && *confirmation_depth == 0 {
+			self.wait_for_0conf_funding_reorg_safety = true;
+			return Ok(());
+		}
+
 		// The swap above places the previous `FundingScope` into `pending_funding`.
 		for (funding, _) in self.pending_funding.drain(..) {
 			self.outputs_to_watch.remove(&funding.funding_txid());
@@ -3984,7 +4013,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				},
 				ChannelMonitorUpdateStep::RenegotiatedFundingLocked { funding_txid } => {
 					log_trace!(logger, "Updating ChannelMonitor with locked renegotiated funding txid {}", funding_txid);
-					if let Err(_) = self.promote_funding(*funding_txid) {
+					if let Err(_) = self.promote_funding(*funding_txid, true) {
 						log_error!(logger, "Unknown funding with txid {} became locked", funding_txid);
 						ret = Err(());
 					}
@@ -4942,6 +4971,20 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				}
 			}
 
+			// A zero-conf renegotiated funding transaction has confirmed. We should have already
+			// applied the corresponding `RenegotiatedFundingLocked` monitor update for it, but
+			// we're still tracking the alternative funding transactions until we're reorg safe.
+			if self.wait_for_0conf_funding_reorg_safety && self.funding.funding_txid() == txid {
+				debug_assert!(self.funding_spend_confirmed.is_none());
+				self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
+					txid,
+					transaction: Some((*tx).clone()),
+					height,
+					block_hash: Some(block_hash),
+					event: OnchainEvent::ZeroConfFundingReorgSafety {},
+				});
+			}
+
 			// A splice transaction has confirmed. We can't promote the splice's scope until we see
 			// the corresponding monitor update for it, but we track the txid so we know which
 			// holder commitment transaction we may need to broadcast.
@@ -5214,19 +5257,35 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				},
 				OnchainEvent::AlternativeFundingConfirmation { .. } => {
 					// An alternative funding transaction has irrevocably confirmed. Locate the
-					// corresponding scope and promote it if the monitor is no longer allowing
-					// updates. Otherwise, we expect it to be promoted via
-					// [`ChannelMonitorUpdateStep::RenegotiatedFundingLocked`].
-					if self.no_further_updates_allowed() {
+					// corresponding scope and promote it. If we're still allowing monitor updates,
+					// we expect it to be promoted via
+					// [`ChannelMonitorUpdateStep::RenegotiatedFundingLocked`] instead. However, in
+					// the zero conf funding case, while we already promoted the scope, we're still
+					// tracking the alternative funding candidates until we're reorg safe.
+					if self.no_further_updates_allowed() || self.wait_for_0conf_funding_reorg_safety {
 						let funding_txid = entry.transaction
 							.expect("Transactions are always present for AlternativeFundingConfirmation entries")
 							.compute_txid();
 						debug_assert_ne!(self.funding.funding_txid(), funding_txid);
-						if let Err(_) = self.promote_funding(funding_txid) {
+						if let Err(_) = self.promote_funding(funding_txid, false) {
 							log_error!(logger, "Missing scope for alternative funding confirmation with txid {}", entry.txid);
 						}
 					}
 				},
+				OnchainEvent::ZeroConfFundingReorgSafety {} => {
+					if self.wait_for_0conf_funding_reorg_safety {
+						debug_assert_eq!(self.funding.funding_txid(), entry.txid);
+						self
+							.pending_funding
+							.drain(..)
+							.for_each(|(funding, _)| {
+								self.outputs_to_watch.remove(&funding.funding_txid());
+							});
+						self.wait_for_0conf_funding_reorg_safety = false;
+					} else {
+						debug_assert!(false, "These events can only be generated when wait_for_0conf_funding_reorg_safety is set");
+					}
+				},
 			}
 		}
 
@@ -6075,6 +6134,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut first_negotiated_funding_txo = RequiredWrapper(None);
 		let mut channel_parameters = None;
 		let mut pending_funding = None;
+		let mut wait_for_0conf_funding_reorg_safety: Option<()> = None;
 		read_tlv_fields!(reader, {
 			(1, funding_spend_confirmed, option),
 			(3, htlcs_resolved_on_chain, optional_vec),
@@ -6093,6 +6153,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(29, initial_counterparty_commitment_tx, option),
 			(31, channel_parameters, (option: ReadableArgs, None)),
 			(32, pending_funding, optional_vec),
+			(34, wait_for_0conf_funding_reorg_safety, option),
 		});
 		if let Some(payment_preimages_with_info) = payment_preimages_with_info {
 			if payment_preimages_with_info.len() != payment_preimages.len() {
@@ -6262,6 +6323,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 
 			current_holder_htlc_data,
 			prev_holder_htlc_data,
+
+			wait_for_0conf_funding_reorg_safety: wait_for_0conf_funding_reorg_safety.is_some(),
 		})))
 	}
 }