Remove counterparty_node_id from ChannelMonitorUpdate

The `counterparty_node_id` in `ChannelMonitorUpdate`s was only tracked
to guarantee we could backfill the `ChannelMonitor`'s field once the
update is applied. Now that we require the field to already be set at
the monitor level, we can remove it from the updates without backwards
compatibility concerns as it was written as an odd TLV.

Author: wpaulino

lightning/src/chain/chainmonitor.rs

@@ -804,7 +804,7 @@ where C::Target: chain::Filter,
 		let monitors = self.monitors.read().unwrap();
 		match monitors.get(&channel_id) {
 			None => {
-				let logger = WithContext::from(&self.logger, update.counterparty_node_id, Some(channel_id), None);
+				let logger = WithContext::from(&self.logger, None, Some(channel_id), None);
 				log_error!(logger, "Failed to update channel monitor: no such monitor registered");
 
 				// We should never ever trigger this from within ChannelManager. Technically a

lightning/src/chain/channelmonitor.rs

@@ -74,15 +74,6 @@ use crate::sync::{Mutex, LockTestExt};
 #[must_use]
 pub struct ChannelMonitorUpdate {
 	pub(crate) updates: Vec<ChannelMonitorUpdateStep>,
-	/// Historically, [`ChannelMonitor`]s didn't know their counterparty node id. However,
-	/// `ChannelManager` really wants to know it so that it can easily look up the corresponding
-	/// channel. For now, this results in a temporary map in `ChannelManager` to look up channels
-	/// by only the funding outpoint.
-	///
-	/// To eventually remove that, we repeat the counterparty node id here so that we can upgrade
-	/// `ChannelMonitor`s to become aware of the counterparty node id if they were generated prior
-	/// to when it was stored directly in them.
-	pub(crate) counterparty_node_id: Option<PublicKey>,
 	/// The sequence number of this update. Updates *must* be replayed in-order according to this
 	/// sequence number (and updates may panic if they are not). The update_id values are strictly
 	/// increasing and increase by one for each new update, with two exceptions specified below.
@@ -117,7 +108,7 @@ impl Writeable for ChannelMonitorUpdate {
 			update_step.write(w)?;
 		}
 		write_tlv_fields!(w, {
-			(1, self.counterparty_node_id, option),
+			// 1 was previously used to store `counterparty_node_id`
 			(3, self.channel_id, option),
 		});
 		Ok(())
@@ -134,13 +125,12 @@ impl Readable for ChannelMonitorUpdate {
 				updates.push(upd);
 			}
 		}
-		let mut counterparty_node_id = None;
 		let mut channel_id = None;
 		read_tlv_fields!(r, {
-			(1, counterparty_node_id, option),
+			// 1 was previously used to store `counterparty_node_id`
 			(3, channel_id, option),
 		});
-		Ok(Self { update_id, counterparty_node_id, updates, channel_id })
+		Ok(Self { update_id, updates, channel_id })
 	}
 }
 
@@ -3191,10 +3181,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len());
 		}
 
-		if let Some(counterparty_node_id) = &updates.counterparty_node_id {
-			debug_assert_eq!(self.counterparty_node_id, *counterparty_node_id);
-		}
-
 		// ChannelMonitor updates may be applied after force close if we receive a preimage for a
 		// broadcasted commitment transaction HTLC output that we'd like to claim on-chain. If this
 		// is the case, we no longer have guaranteed access to the monitor's update ID, so we use a

lightning/src/ln/channel.rs

@@ -4386,7 +4386,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 				self.latest_monitor_update_id += 1;
 				Some((self.get_counterparty_node_id(), funding_txo, self.channel_id(), ChannelMonitorUpdate {
 					update_id: self.latest_monitor_update_id,
-					counterparty_node_id: Some(self.counterparty_node_id),
 					updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 					channel_id: Some(self.channel_id()),
 				}))
@@ -4957,7 +4956,6 @@ impl<SP: Deref> FundedChannel<SP> where
 		self.context.latest_monitor_update_id += 1;
 		let monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
-			counterparty_node_id: Some(self.context.counterparty_node_id),
 			updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
 				payment_preimage: payment_preimage_arg.clone(),
 				payment_info,
@@ -5665,7 +5663,6 @@ impl<SP: Deref> FundedChannel<SP> where
 		self.context.latest_monitor_update_id += 1;
 		let mut monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
-			counterparty_node_id: Some(self.context.counterparty_node_id),
 			updates: vec![ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo {
 				commitment_tx: holder_commitment_tx,
 				htlc_outputs: htlcs_and_sigs,
@@ -5759,7 +5756,6 @@ impl<SP: Deref> FundedChannel<SP> where
 
 			let mut monitor_update = ChannelMonitorUpdate {
 				update_id: self.context.latest_monitor_update_id + 1, // We don't increment this yet!
-				counterparty_node_id: Some(self.context.counterparty_node_id),
 				updates: Vec::new(),
 				channel_id: Some(self.context.channel_id()),
 			};
@@ -5952,7 +5948,6 @@ impl<SP: Deref> FundedChannel<SP> where
 		self.context.latest_monitor_update_id += 1;
 		let mut monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
-			counterparty_node_id: Some(self.context.counterparty_node_id),
 			updates: vec![ChannelMonitorUpdateStep::CommitmentSecret {
 				idx: self.context.cur_counterparty_commitment_transaction_number + 1,
 				secret: msg.per_commitment_secret,
@@ -7224,7 +7219,6 @@ impl<SP: Deref> FundedChannel<SP> where
 			self.context.latest_monitor_update_id += 1;
 			let monitor_update = ChannelMonitorUpdate {
 				update_id: self.context.latest_monitor_update_id,
-				counterparty_node_id: Some(self.context.counterparty_node_id),
 				updates: vec![ChannelMonitorUpdateStep::ShutdownScript {
 					scriptpubkey: self.get_closing_scriptpubkey(),
 				}],
@@ -8506,7 +8500,6 @@ impl<SP: Deref> FundedChannel<SP> where
 		self.context.latest_monitor_update_id += 1;
 		let monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
-			counterparty_node_id: Some(self.context.counterparty_node_id),
 			updates: vec![ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
 				commitment_txid: counterparty_commitment_txid,
 				htlc_outputs: htlcs.clone(),
@@ -8718,7 +8711,6 @@ impl<SP: Deref> FundedChannel<SP> where
 			self.context.latest_monitor_update_id += 1;
 			let monitor_update = ChannelMonitorUpdate {
 				update_id: self.context.latest_monitor_update_id,
-				counterparty_node_id: Some(self.context.counterparty_node_id),
 				updates: vec![ChannelMonitorUpdateStep::ShutdownScript {
 					scriptpubkey: self.get_closing_scriptpubkey(),
 				}],

lightning/src/ln/channelmanager.rs

@@ -7281,7 +7281,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 
 		let preimage_update = ChannelMonitorUpdate {
 			update_id,
-			counterparty_node_id: Some(counterparty_node_id),
 			updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
 				payment_preimage,
 				payment_info,
@@ -13704,7 +13703,6 @@ where
 					&channel_id);
 				let monitor_update = ChannelMonitorUpdate {
 					update_id: monitor.get_latest_update_id().saturating_add(1),
-					counterparty_node_id: Some(counterparty_node_id),
 					updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }],
 					channel_id: Some(monitor.channel_id()),
 				};