From 07b5355b7573cfb8ee46769286dbd0eeb6d42ec9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 29 Nov 2023 21:59:38 +0000 Subject: [PATCH] Set `counterparty_node_id` on `ChannelMonitor`s as they're updated Historically, `ChannelMonitor`s had idea who their counterparty was. This was fine, until `ChannelManager` started indexing by peer, at which point it needed to know the counterparty when it saw a `ChannelMonitorUpdate` complete. To address this, a "temporary" map from channel ID to peer was added, but no upgrade path was created for existing `ChannelMonitor`s to not rely on this map. This commit adds such an upgrade path, setting the `counterparty_node_id` on all `ChannelMonitor`s as they're updated, allowing us to eventually break backwards compatibility and remove `ChannelManager::outpoint_to_peer`. --- lightning/src/chain/channelmonitor.rs | 29 ++++++++++++++++++++++++--- lightning/src/ln/channel.rs | 8 ++++++++ lightning/src/ln/channelmanager.rs | 2 ++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 1dab9c8c8..9961b822e 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -71,6 +71,15 @@ use crate::sync::{Mutex, LockTestExt}; #[must_use] pub struct ChannelMonitorUpdate { pub(crate) updates: Vec, + /// 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, /// 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 }) } } @@ -2697,6 +2711,15 @@ impl ChannelMonitorImpl { 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 diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8cff537bb..c5f2e0c7d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2389,6 +2389,7 @@ impl ChannelContext 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 } @@ -2698,6 +2699,7 @@ impl Channel 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(), }], @@ -3408,6 +3410,7 @@ impl Channel 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, @@ -3487,6 +3490,7 @@ impl Channel 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(), }; @@ -3653,6 +3657,7 @@ impl Channel 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, @@ -4705,6 +4710,7 @@ impl Channel 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(), }], @@ -5828,6 +5834,7 @@ impl Channel 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(), @@ -6026,6 +6033,7 @@ impl Channel 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(), }], diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cffae48e6..9d5cf7e90 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5549,6 +5549,7 @@ where } let preimage_update = ChannelMonitorUpdate { update_id: CLOSED_CHANNEL_UPDATE_ID, + counterparty_node_id: None, updates: vec![ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage, }], @@ -10202,6 +10203,7 @@ where &funding_txo.to_channel_id()); let monitor_update = ChannelMonitorUpdate { update_id: CLOSED_CHANNEL_UPDATE_ID, + counterparty_node_id: None, updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }], }; close_background_events.push(BackgroundEvent::ClosedMonitorUpdateRegeneratedOnStartup((*funding_txo, monitor_update))); -- 2.39.5