add channel <-> channel monitor consistency check

Author: joostjager

lightning/Cargo.toml

@@ -22,6 +22,7 @@ _externalize_tests = ["inventory", "_test_utils"]
 # Allow signing of local transactions that may have been revoked or will be revoked, for functional testing (e.g. justice tx handling).
 # This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
 unsafe_revoked_tx_signing = []
+safe_channels = []
 
 std = []
 

lightning/src/chain/chainmonitor.rs

@@ -1417,9 +1417,10 @@ where
 				let logger = WithChannelMonitor::from(&self.logger, &monitor, None);
 				log_trace!(
 					logger,
-					"Updating ChannelMonitor to id {} for channel {}",
+					"Updating ChannelMonitor to id {} for channel {} with updates {:#?}",
 					update.update_id,
-					log_funding_info!(monitor)
+					log_funding_info!(monitor),
+					update.updates
 				);
 
 				// We hold a `pending_monitor_updates` lock through `update_monitor` to ensure we

lightning/src/chain/channelmonitor.rs

@@ -50,7 +50,7 @@ use crate::ln::chan_utils::{
 	self, ChannelTransactionParameters, CommitmentTransaction, CounterpartyCommitmentSecrets,
 	HTLCClaim, HTLCOutputInCommitment, HolderCommitmentTransaction,
 };
-use crate::ln::channel::INITIAL_COMMITMENT_NUMBER;
+use crate::ln::channel::{read_check_data, INITIAL_COMMITMENT_NUMBER};
 use crate::ln::channel_keys::{
 	DelayedPaymentBasepoint, DelayedPaymentKey, HtlcBasepoint, HtlcKey, RevocationBasepoint,
 	RevocationKey,
@@ -111,6 +111,9 @@ pub struct ChannelMonitorUpdate {
 	/// Will be `None` for `ChannelMonitorUpdate`s constructed on LDK versions prior to 0.0.121 and
 	/// always `Some` otherwise.
 	pub channel_id: Option<ChannelId>,
+
+	/// The encoded channel data associated with this ChannelMonitor, if any.
+	pub encoded_channel: Option<Vec<u8>>,
 }
 
 impl ChannelMonitorUpdate {
@@ -159,6 +162,7 @@ impl Writeable for ChannelMonitorUpdate {
 		write_tlv_fields!(w, {
 			// 1 was previously used to store `counterparty_node_id`
 			(3, self.channel_id, option),
+			(5, self.encoded_channel, option)
 		});
 		Ok(())
 	}
@@ -176,11 +180,13 @@ impl Readable for ChannelMonitorUpdate {
 			}
 		}
 		let mut channel_id = None;
+		let mut encoded_channel = None;
 		read_tlv_fields!(r, {
 			// 1 was previously used to store `counterparty_node_id`
 			(3, channel_id, option),
+			(5, encoded_channel, option)
 		});
-		Ok(Self { update_id, updates, channel_id })
+		Ok(Self { update_id, updates, channel_id, encoded_channel })
 	}
 }
 
@@ -1402,6 +1408,8 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	/// make deciding whether to do so simple, here we track whether this monitor was last written
 	/// prior to 0.1.
 	written_by_0_1_or_later: bool,
+
+	encoded_channel: Option<Vec<u8>>,
 }
 
 // Returns a `&FundingScope` for the one we are currently observing/handling commitment transactions
@@ -1521,6 +1529,9 @@ const MIN_SERIALIZATION_VERSION: u8 = 1;
 pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
 	channel_monitor: &ChannelMonitorImpl<Signer>, _is_stub: bool, writer: &mut W,
 ) -> Result<(), Error> {
+	if let Some(ref encoded_channel) = channel_monitor.encoded_channel {
+		channel_monitor.check_encoded_channel_consistency(encoded_channel);
+	}
 	write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
 	channel_monitor.latest_update_id.write(writer)?;
@@ -1755,6 +1766,7 @@ 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.encoded_channel, option),
 	});
 
 	Ok(())
@@ -1994,6 +2006,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 			alternative_funding_confirmed: None,
 
 			written_by_0_1_or_later: true,
+			encoded_channel: None,
 		})
 	}
 
@@ -2114,6 +2127,17 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		inner.update_monitor(updates, broadcaster, fee_estimator, &logger)
 	}
 
+	/// Gets the encoded channel data, if any, associated with this ChannelMonitor.
+	pub fn get_encoded_channel(&self) -> Option<Vec<u8>> {
+		self.inner.lock().unwrap().encoded_channel.clone()
+	}
+
+	/// Updates the encoded channel data associated with this ChannelMonitor. To clear the encoded channel data (for
+	/// example after shut down of a channel), pass an empty vector.
+	pub fn update_encoded_channel(&self, encoded: Vec<u8>) {
+		self.inner.lock().unwrap().update_encoded_channel(encoded);
+	}
+
 	/// Gets the update_id from the latest ChannelMonitorUpdate which was applied to this
 	/// ChannelMonitor.
 	///
@@ -2719,6 +2743,51 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 }
 
 impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
+	fn check_encoded_channel_consistency(&self, encoded: &[u8]) {
+		let encoded_channel_reader = &mut &encoded[..];
+		let check_res = read_check_data(encoded_channel_reader);
+		if let Ok(check_data) = check_res {
+			debug_assert!(
+				check_data.cur_holder_commitment_transaction_number
+					== self.get_cur_holder_commitment_number(),
+				"cur_holder_commitment_transaction_number - channel: {} vs monitor: {}",
+				check_data.cur_holder_commitment_transaction_number,
+				self.get_cur_holder_commitment_number()
+			);
+			debug_assert!(
+				check_data.revoked_counterparty_commitment_transaction_number
+					== self.get_min_seen_secret(),
+				"revoked_counterparty_commitment_transaction_number - channel: {} vs monitor: {}",
+				check_data.revoked_counterparty_commitment_transaction_number,
+				self.get_min_seen_secret()
+			);
+			debug_assert!(
+				check_data.cur_counterparty_commitment_transaction_number
+					== self.get_cur_counterparty_commitment_number(),
+				"cur_counterparty_commitment_transaction_number - channel: {} vs monitor: {}",
+				check_data.cur_counterparty_commitment_transaction_number,
+				self.get_cur_counterparty_commitment_number()
+			);
+			debug_assert!(
+				check_data.latest_monitor_update_id == self.get_latest_update_id(),
+				"latest_monitor_update_id - channel: {} vs monitor: {}",
+				check_data.latest_monitor_update_id,
+				self.get_latest_update_id()
+			);
+		} else {
+			debug_assert!(false, "Failed to read check data from encoded channel");
+		}
+	}
+
+	fn update_encoded_channel(&mut self, encoded: Vec<u8>) {
+		if encoded.len() > 0 {
+			self.check_encoded_channel_consistency(&encoded);
+			self.encoded_channel = Some(encoded);
+		} else {
+			self.encoded_channel = None;
+		}
+	}
+
 	/// Helper for get_claimable_balances which does the work for an individual HTLC, generating up
 	/// to one `Balance` for the HTLC.
 	#[rustfmt::skip]
@@ -3475,6 +3544,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			return Err("Previous secret did not match new one");
 		}
 
+		println!("Channel monitor current commitment secret index: {}", self.commitment_secrets.get_min_seen_secret());
+
 		// Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill
 		// events for now-revoked/fulfilled HTLCs.
 		let mut removed_fulfilled_htlcs = false;
@@ -4405,9 +4476,20 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			}
 		}
 
-		if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
-			log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
-			Err(())
+		// Assume that if the updates contains no encoded channel, that the channel remained unchanged. We
+		// therefore do not update the monitor.
+		if let Some(encoded_channel) = updates.encoded_channel.as_ref() {
+			self.update_encoded_channel(encoded_channel.clone());
+		}
+
+		if ret.is_ok() {
+			if self.no_further_updates_allowed() && is_pre_close_update {
+				log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
+				Err(())
+			} else {
+
+				Ok(())
+			}
 		} else { ret }
 	}
 
@@ -6645,6 +6727,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut alternative_funding_confirmed = None;
 		let mut is_manual_broadcast = RequiredWrapper(None);
 		let mut funding_seen_onchain = RequiredWrapper(None);
+		let mut encoded_channel = None;
 		read_tlv_fields!(reader, {
 			(1, funding_spend_confirmed, option),
 			(3, htlcs_resolved_on_chain, optional_vec),
@@ -6667,6 +6750,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(34, alternative_funding_confirmed, option),
 			(35, is_manual_broadcast, (default_value, false)),
 			(37, funding_seen_onchain, (default_value, true)),
+			(39, encoded_channel, 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.
@@ -6844,6 +6928,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			alternative_funding_confirmed,
 
 			written_by_0_1_or_later,
+			encoded_channel,
 		});
 
 		if counterparty_node_id.is_none() {