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.

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -1737,10 +1737,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.
@@ -3278,12 +3282,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
 	}
@@ -4227,7 +4235,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

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,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;