Avoid force-closing 0-conf channels when funding is reorg'd

lightning/src/ln/channel.rs

@@ -11344,9 +11344,13 @@ where
 		}
 
 		// Check if the funding transaction was unconfirmed
+		let original_scid = self.funding.short_channel_id;
+		let was_confirmed = self.funding.funding_tx_confirmed_in.is_some();
 		let funding_tx_confirmations = self.funding.get_funding_tx_confirmations(height);
 		if funding_tx_confirmations == 0 {
 			self.funding.funding_tx_confirmation_height = 0;
+			self.funding.short_channel_id = None;
+			self.funding.funding_tx_confirmed_in = None;
 		}
 
 		if let Some(channel_ready) = self.check_get_channel_ready(height, logger) {
@@ -11361,18 +11365,33 @@ where
 			self.context.channel_state.is_our_channel_ready() {
 
 			// If we've sent channel_ready (or have both sent and received channel_ready), and
-			// the funding transaction has become unconfirmed,
-			// close the channel and hope we can get the latest state on chain (because presumably
-			// the funding transaction is at least still in the mempool of most nodes).
+			// the funding transaction has become unconfirmed, we'll probably get a new SCID when
+			// it re-confirms.
 			//
-			// Note that ideally we wouldn't force-close if we see *any* reorg on a 1-conf or
-			// 0-conf channel, but not doing so may lead to the
-			// `ChannelManager::short_to_chan_info` map  being inconsistent, so we currently have
-			// to.
-			if funding_tx_confirmations == 0 && self.funding.funding_tx_confirmed_in.is_some() {
-				let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.",
-					self.context.minimum_depth.unwrap(), funding_tx_confirmations);
-				return Err(ClosureReason::ProcessingError { err: err_reason });
+			// Worse, if the funding has un-confirmed we could have accepted some HTLC(s) over it
+			// and are now at risk of double-spend. While its possible, even likely, that this is
+			// just a trivial reorg and we should wait to see the new block connected in the next
+			// call, its also possible we've been double-spent. To avoid further loss of funds, we
+			// need some kind of method to freeze the channel and avoid accepting further HTLCs,
+			// but absent such a method, we just force-close.
+			//
+			// The one exception we make is for 0-conf channels, which we decided to trust anyway,
+			// in which case we simply track the previous SCID as a `historical_scids` the same as
+			// after a channel is spliced.
+			if funding_tx_confirmations == 0 && was_confirmed {
+				if let Some(scid) = original_scid {
+					self.context.historical_scids.push(scid);
+				} else {
+					debug_assert!(false);
+				}
+				if self.context.minimum_depth(&self.funding).expect("set for a ready channel") > 1 {
+					// Reset the original short_channel_id so that we'll generate a closure
+					// `channel_update` broadcast event.
+					self.funding.short_channel_id = original_scid;
+					let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.",
+						self.context.minimum_depth.unwrap(), funding_tx_confirmations);
+					return Err(ClosureReason::ProcessingError { err: err_reason });
+				}
 			}
 		} else if !self.funding.is_outbound() && self.funding.funding_tx_confirmed_in.is_none() &&
 				height >= self.context.channel_creation_height + FUNDING_CONF_DEADLINE_BLOCKS {

lightning/src/ln/functional_test_utils.rs

@@ -3224,12 +3224,13 @@ pub fn expect_probe_successful_events(
 }
 
 pub struct PaymentFailedConditions<'a> {
-	pub(crate) expected_htlc_error_data: Option<(LocalHTLCFailureReason, &'a [u8])>,
-	pub(crate) expected_blamed_scid: Option<u64>,
-	pub(crate) expected_blamed_chan_closed: Option<bool>,
-	pub(crate) expected_mpp_parts_remain: bool,
-	pub(crate) retry_expected: bool,
-	pub(crate) from_mon_update: bool,
+	pub expected_htlc_error_data: Option<(LocalHTLCFailureReason, &'a [u8])>,
+	pub expected_blamed_scid: Option<u64>,
+	pub expected_blamed_chan_closed: Option<bool>,
+	pub expected_mpp_parts_remain: bool,
+	pub retry_expected: bool,
+	pub from_mon_update: bool,
+	pub reason: Option<PaymentFailureReason>,
 }
 
 impl<'a> PaymentFailedConditions<'a> {
@@ -3241,6 +3242,7 @@ impl<'a> PaymentFailedConditions<'a> {
 			expected_mpp_parts_remain: false,
 			retry_expected: false,
 			from_mon_update: false,
+			reason: None,
 		}
 	}
 	pub fn mpp_parts_remain(mut self) -> Self {
@@ -3321,14 +3323,21 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 				*payment_failed_permanently, expected_payment_failed_permanently,
 				"unexpected payment_failed_permanently value"
 			);
-			{
-				assert!(error_code.is_some(), "expected error_code.is_some() = true");
-				assert!(error_data.is_some(), "expected error_data.is_some() = true");
-				let reason: LocalHTLCFailureReason = error_code.unwrap().into();
-				if let Some((code, data)) = conditions.expected_htlc_error_data {
-					assert_eq!(reason, code, "unexpected error code");
-					assert_eq!(&error_data.as_ref().unwrap()[..], data, "unexpected error data");
-				}
+			match failure {
+				PathFailure::OnPath { .. } => {
+					assert!(error_code.is_some(), "expected error_code.is_some() = true");
+					assert!(error_data.is_some(), "expected error_data.is_some() = true");
+					let reason: LocalHTLCFailureReason = error_code.unwrap().into();
+					if let Some((code, data)) = conditions.expected_htlc_error_data {
+						assert_eq!(reason, code, "unexpected error code");
+						assert_eq!(&error_data.as_ref().unwrap()[..], data);
+					}
+				},
+				PathFailure::InitialSend { .. } => {
+					assert!(error_code.is_none());
+					assert!(error_data.is_none());
+					assert!(conditions.expected_htlc_error_data.is_none());
+				},
 			}
 
 			if let Some(chan_closed) = conditions.expected_blamed_chan_closed {
@@ -3362,7 +3371,9 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 				assert_eq!(*payment_id, expected_payment_id);
 				assert_eq!(
 					reason.unwrap(),
-					if expected_payment_failed_permanently {
+					if let Some(expected_reason) = conditions.reason {
+						expected_reason
+					} else if expected_payment_failed_permanently {
 						PaymentFailureReason::RecipientRejected
 					} else {
 						PaymentFailureReason::RetriesExhausted
@@ -3414,7 +3425,7 @@ pub fn send_along_route_with_secret<'a, 'b, 'c>(
 	payment_id
 }
 
-fn fail_payment_along_path<'a, 'b, 'c>(expected_path: &[&Node<'a, 'b, 'c>]) {
+pub fn fail_payment_along_path<'a, 'b, 'c>(expected_path: &[&Node<'a, 'b, 'c>]) {
 	let origin_node_id = expected_path[0].node.get_our_node_id();
 
 	// iterate from the receiving node to the origin node and handle update fail htlc.