Merge pull request #2795 from TheBlueMatt/2023-11-robuster-chan-to-peer

 Move channel -> peer tracking to OutPoints from Channel IDs

Author: wpaulino

lightning/src/chain/chainmonitor.rs

@@ -35,7 +35,7 @@ use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::events;
 use crate::events::{Event, EventHandler};
 use crate::util::atomic_counter::AtomicCounter;
-use crate::util::logger::Logger;
+use crate::util::logger::{Logger, WithContext};
 use crate::util::errors::APIError;
 use crate::util::wakers::{Future, Notifier};
 use crate::ln::channelmanager::ChannelDetails;
@@ -757,7 +757,8 @@ where C::Target: chain::Filter,
 		let monitors = self.monitors.read().unwrap();
 		match monitors.get(&funding_txo) {
 			None => {
-				log_error!(self.logger, "Failed to update channel monitor: no such monitor registered");
+				let logger = WithContext::from(&self.logger, update.counterparty_node_id, Some(funding_txo.to_channel_id()));
+				log_error!(logger, "Failed to update channel monitor: no such monitor registered");
 
 				// We should never ever trigger this from within ChannelManager. Technically a
 				// user could use this object with some proxying in between which makes this

lightning/src/chain/channelmonitor.rs

@@ -71,6 +71,15 @@ 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.
@@ -107,7 +116,9 @@ impl Writeable for ChannelMonitorUpdate {
 		for update_step in self.updates.iter() {
 			update_step.write(w)?;
 		}
-		write_tlv_fields!(w, {});
+		write_tlv_fields!(w, {
+			(1, self.counterparty_node_id, option),
+		});
 		Ok(())
 	}
 }
@@ -122,8 +133,11 @@ impl Readable for ChannelMonitorUpdate {
 				updates.push(upd);
 			}
 		}
-		read_tlv_fields!(r, {});
-		Ok(Self { update_id, updates })
+		let mut counterparty_node_id = None;
+		read_tlv_fields!(r, {
+			(1, counterparty_node_id, option),
+		});
+		Ok(Self { update_id, counterparty_node_id, updates })
 	}
 }
 
@@ -2738,6 +2752,15 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			log_info!(logger, "Applying update to monitor {}, bringing update_id from {} to {} with {} change(s).",
 				log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len());
 		}
+
+		if updates.counterparty_node_id.is_some() {
+			if self.counterparty_node_id.is_none() {
+				self.counterparty_node_id = updates.counterparty_node_id;
+			} else {
+				debug_assert_eq!(self.counterparty_node_id, updates.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

@@ -2394,6 +2394,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 				self.latest_monitor_update_id = CLOSED_CHANNEL_UPDATE_ID;
 				Some((self.get_counterparty_node_id(), funding_txo, ChannelMonitorUpdate {
 					update_id: self.latest_monitor_update_id,
+					counterparty_node_id: Some(self.counterparty_node_id),
 					updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 				}))
 			} else { None }
@@ -2766,6 +2767,7 @@ impl<SP: Deref> Channel<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(),
 			}],
@@ -2997,6 +2999,20 @@ impl<SP: Deref> Channel<SP> where
 		self.context.channel_state.clear_waiting_for_batch();
 	}
 
+	/// Unsets the existing funding information.
+	///
+	/// This must only be used if the channel has not yet completed funding and has not been used.
+	///
+	/// Further, the channel must be immediately shut down after this with a call to
+	/// [`ChannelContext::force_shutdown`].
+	pub fn unset_funding_info(&mut self, temporary_channel_id: ChannelId) {
+		debug_assert!(matches!(
+			self.context.channel_state, ChannelState::AwaitingChannelReady(_)
+		));
+		self.context.channel_transaction_parameters.funding_outpoint = None;
+		self.context.channel_id = temporary_channel_id;
+	}
+
 	/// Handles a channel_ready message from our peer. If we've already sent our channel_ready
 	/// and the channel is now usable (and public), this may generate an announcement_signatures to
 	/// reply with.
@@ -3487,6 +3503,7 @@ impl<SP: Deref> Channel<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,
@@ -3566,6 +3583,7 @@ impl<SP: Deref> Channel<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(),
 			};
 
@@ -3746,6 +3764,7 @@ impl<SP: Deref> Channel<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,
@@ -4803,6 +4822,7 @@ impl<SP: Deref> Channel<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(),
 				}],
@@ -5926,6 +5946,7 @@ impl<SP: Deref> Channel<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(),
@@ -6124,6 +6145,7 @@ impl<SP: Deref> Channel<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(),
 				}],