Only generate a post-close lock ChannelMonitorUpdate if we need one

If a channel is closed on startup, but we find that the
`ChannelMonitor` isn't aware of this, we generate a
`ChannelMonitorUpdate` containing a
`ChannelMonitorUpdateStep::ChannelForceClosed`. This ensures that
the `ChannelMonitor` will not accept any future updates in case we
somehow load up a previous `ChannelManager` (though that really
shouldn't happen).

Previously, we'd apply this update only if we detected that the
`ChannelManager` had not yet informed the `ChannelMonitor` about
the channel's closure, even if the `ChannelMonitor` would already
refuse any other updates because it detected a channel closure
on chain.

This doesn't accomplish anything but an extra I/O write, so we
remove it here.

Further, a user reported that, in regtest, they could:
 (a) coop close a channel (not generating a `ChannelMonitorUpdate`)
 (b) wait just under 4032 blocks (on regtest, taking only a day)
 (c) restart the `ChannelManager`, generating the above update
 (d) connect a block or two (during the startup sequence), making
     the `ChannelMonitor` eligible for archival,
 (d) restart the `ChannelManager` again (without applying the
     update from (c), but after having archived the
     `ChannelMonitor`, leading to a failure to deserialize as we
     have a pending `ChannelMonitorUpdate` for a `ChannelMonitor`
     that has been archived.

Though it seems very unlikely this would happen on mainnet, it is
theoretically possible.
This commit is contained in:
Matt Corallo 2025-02-25 01:42:30 +00:00
parent 489d70a208
commit cd2e1698c8
2 changed files with 16 additions and 8 deletions

View file

@ -1774,10 +1774,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
self.inner.lock().unwrap().get_cur_holder_commitment_number()
}
/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
pub(crate) fn offchain_closed(&self) -> bool {
self.inner.lock().unwrap().lockdown_from_offchain
/// Fetches whether this monitor has marked the channel as closed and will refuse any further
/// updates to the commitment transactions.
///
/// It can be marked closed in a few different ways, including via a
/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
/// on-chain.
pub(crate) fn no_further_updates_allowed(&self) -> bool {
self.inner.lock().unwrap().no_further_updates_allowed()
}
/// Gets the `node_id` of the counterparty for this channel.
@ -3315,12 +3319,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}
if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
Err(())
} else { ret }
}
fn no_further_updates_allowed(&self) -> bool {
self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
}
fn get_latest_update_id(&self) -> u64 {
self.latest_update_id
}
@ -4268,7 +4276,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}
if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
if self.no_further_updates_allowed() {
// Fail back HTLCs on backwards channels if they expire within
// `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
// point where no further off-chain updates will be accepted). If we haven't seen the

View file

@ -13781,8 +13781,8 @@ where
// claim.
// Note that a `ChannelMonitor` is created with `update_id` 0 and after we
// provide it with a closure update its `update_id` will be at 1.
if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
should_queue_fc_update = !monitor.offchain_closed();
if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
should_queue_fc_update = !monitor.no_further_updates_allowed();
let mut latest_update_id = monitor.get_latest_update_id();
if should_queue_fc_update {
latest_update_id += 1;