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.
This commit is contained in:
Wilmer Paulino 2025-02-06 15:23:49 -08:00
parent f0bb9001b6
commit 362ca653f7
No known key found for this signature in database
GPG key ID: 634FE5FC544DCA31
4 changed files with 4 additions and 28 deletions

View file

@ -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

View file

@ -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 })
}
}
@ -3197,10 +3187,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

View file

@ -4528,7 +4528,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()),
}))
@ -5107,7 +5106,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,
@ -5715,7 +5713,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,
htlc_outputs,
@ -5797,7 +5794,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()),
};
@ -5990,7 +5986,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,
@ -7262,7 +7257,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(),
}],
@ -8548,7 +8542,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(),
@ -8760,7 +8753,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(),
}],

View file

@ -7309,7 +7309,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,
@ -13732,7 +13731,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()),
};