Set `counterparty_node_id` on `ChannelMonitor`s as they're updated
authorMatt Corallo <git@bluematt.me>
Wed, 29 Nov 2023 21:59:38 +0000 (21:59 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 12 Dec 2023 02:08:36 +0000 (02:08 +0000)
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
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs

index 1dab9c8c8ffba1456c78671b8150323a595ee181..9961b822ecb6089978dd45e8a7b622f18b424b4e 100644 (file)
@@ -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 })
        }
 }
 
@@ -2697,6 +2711,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
index 8cff537bbfc1417c5b0fb64aa097c3cf179aeaf1..c5f2e0c7d43f423ad30ad4a5a58800a8ff522586 100644 (file)
@@ -2389,6 +2389,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 }
@@ -2698,6 +2699,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(),
                        }],
@@ -3408,6 +3410,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,
@@ -3487,6 +3490,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(),
                        };
 
@@ -3653,6 +3657,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,
@@ -4705,6 +4710,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(),
                                }],
@@ -5828,6 +5834,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(),
@@ -6026,6 +6033,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(),
                                }],
index cffae48e620b92b4bb8c4a89c56ea454167547a3..9d5cf7e903ea088da51dac9fc951115f8aab49b3 100644 (file)
@@ -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)));