Skip to content

Commit cd2e169

Browse files
committed
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.
1 parent 489d70a commit cd2e169

File tree

2 files changed

+16
-8
lines changed

2 files changed

+16
-8
lines changed

lightning/src/chain/channelmonitor.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,10 +1774,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
17741774
self.inner.lock().unwrap().get_cur_holder_commitment_number()
17751775
}
17761776

1777-
/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
1778-
/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
1779-
pub(crate) fn offchain_closed(&self) -> bool {
1780-
self.inner.lock().unwrap().lockdown_from_offchain
1777+
/// Fetches whether this monitor has marked the channel as closed and will refuse any further
1778+
/// updates to the commitment transactions.
1779+
///
1780+
/// It can be marked closed in a few different ways, including via a
1781+
/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
1782+
/// on-chain.
1783+
pub(crate) fn no_further_updates_allowed(&self) -> bool {
1784+
self.inner.lock().unwrap().no_further_updates_allowed()
17811785
}
17821786

17831787
/// Gets the `node_id` of the counterparty for this channel.
@@ -3315,12 +3319,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33153319
}
33163320
}
33173321

3318-
if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
3322+
if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
33193323
log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
33203324
Err(())
33213325
} else { ret }
33223326
}
33233327

3328+
fn no_further_updates_allowed(&self) -> bool {
3329+
self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
3330+
}
3331+
33243332
fn get_latest_update_id(&self) -> u64 {
33253333
self.latest_update_id
33263334
}
@@ -4268,7 +4276,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
42684276
}
42694277
}
42704278

4271-
if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
4279+
if self.no_further_updates_allowed() {
42724280
// Fail back HTLCs on backwards channels if they expire within
42734281
// `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
42744282
// point where no further off-chain updates will be accepted). If we haven't seen the

lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13781,8 +13781,8 @@ where
1378113781
// claim.
1378213782
// Note that a `ChannelMonitor` is created with `update_id` 0 and after we
1378313783
// provide it with a closure update its `update_id` will be at 1.
13784-
if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
13785-
should_queue_fc_update = !monitor.offchain_closed();
13784+
if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
13785+
should_queue_fc_update = !monitor.no_further_updates_allowed();
1378613786
let mut latest_update_id = monitor.get_latest_update_id();
1378713787
if should_queue_fc_update {
1378813788
latest_update_id += 1;

0 commit comments

Comments
 (0)