From cab2ca8eeb6e8e4e75ad9227f440900ba9a787cb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 1 Aug 2021 02:13:36 +0000 Subject: [PATCH 1/2] Log when a channel is closed on startup due to stale ChannelManager This is one of the riskiest parts of our API from the perspective of accidental force-closes - if users delay persisting the ChannelManager much at all after a ChannelMonitor we may hit a force-close after restart. The fact that we don't log at all when this happens is criminal. --- lightning/src/ln/channelmanager.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0aa3f1d0bef..c5db4075626 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4960,6 +4960,10 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> channel.get_cur_counterparty_commitment_transaction_number() > monitor.get_cur_counterparty_commitment_number() || channel.get_latest_monitor_update_id() < monitor.get_latest_update_id() { // But if the channel is behind of the monitor, close the channel: + log_error!(args.logger, "A ChannelManager is stale compared to the current ChannelMonitor!"); + log_error!(args.logger, " The channel will be force-closed and the latest commitment transaction from the ChannelMonitor broadcast."); + log_error!(args.logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.", + log_bytes!(channel.channel_id()), monitor.get_latest_update_id(), channel.get_latest_monitor_update_id()); let (_, mut new_failed_htlcs) = channel.force_shutdown(true); failed_htlcs.append(&mut new_failed_htlcs); monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); From 5307b5e8ceb2bb2ac9634eafd3c8b123d25da34f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 1 Aug 2021 02:42:42 +0000 Subject: [PATCH 2/2] Make BackgroundProcessor `#[must_use]` to avoid dropping immediately It is easy for users to have a bug where they drop a `BackgroundProcessor` immediately, causing it to start and then immediately stop. Instead, add a `#[must_use]` tag to provide a compiler warning for such instances. --- lightning-background-processor/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 55ac14c048c..afa3633bee6 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -39,6 +39,7 @@ use std::ops::Deref; /// then there is a risk of channels force-closing on startup when the manager realizes it's /// outdated. However, as long as `ChannelMonitor` backups are sound, no funds besides those used /// for unilateral chain closure fees are at risk. +#[must_use = "BackgroundProcessor will immediately stop on drop. It should be stored until shutdown."] pub struct BackgroundProcessor { stop_thread: Arc, thread_handle: Option>>,