Persist unresolved ChannelMonitors on empty height change

Author: arik-so

lightning/src/chain/chainmonitor.rs

@@ -639,25 +639,30 @@ where C::Target: chain::Filter,
 	/// Depending on the implementation of [`Persist::archive_persisted_channel`] the monitor
 	/// data could be moved to an archive location or removed entirely.
 	pub fn archive_fully_resolved_channel_monitors(&self) {
-		let mut have_monitors_to_prune = false;
+		let mut have_monitors_to_persist_or_prune = false;
 		for (_, monitor_holder) in self.monitors.read().unwrap().iter() {
 			let logger = WithChannelMonitor::from(&self.logger, &monitor_holder.monitor, None);
-			if monitor_holder.monitor.is_fully_resolved(&logger) {
-				have_monitors_to_prune = true;
+			let (is_fully_resolved, needs_persistence) = monitor_holder.monitor.process_claim_resolution_status(&logger);
+			if is_fully_resolved || needs_persistence {
+				have_monitors_to_persist_or_prune = true;
 			}
 		}
-		if have_monitors_to_prune {
+		if have_monitors_to_persist_or_prune {
 			let mut monitors = self.monitors.write().unwrap();
 			monitors.retain(|funding_txo, monitor_holder| {
 				let logger = WithChannelMonitor::from(&self.logger, &monitor_holder.monitor, None);
-				if monitor_holder.monitor.is_fully_resolved(&logger) {
+				let (is_fully_resolved, needs_persistence) = monitor_holder.monitor.process_claim_resolution_status(&logger);
+				if is_fully_resolved {
 					log_info!(logger,
 						"Archiving fully resolved ChannelMonitor for funding txo {}",
 						funding_txo
 					);
 					self.persister.archive_persisted_channel(*funding_txo);
 					false
 				} else {
+					if needs_persistence {
+						self.persister.update_persisted_channel(*funding_txo, None, &monitor_holder.monitor);
+					}
 					true
 				}
 			});

lightning/src/chain/channelmonitor.rs

@@ -1994,10 +1994,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 
 	/// Checks if the monitor is fully resolved. Resolved monitor is one that has claimed all of
 	/// its outputs and balances (i.e. [`Self::get_claimable_balances`] returns an empty set).
+	/// Additionally updates the empty balances height if necessary.
 	///
-	/// This function returns true only if [`Self::get_claimable_balances`] has been empty for at least
+	/// This function returns a tuple of two booleans, the first indicating whether the monitor is
+	/// fully resolved, and the second whether the monitor needs persistence as a consequence of
+	/// checking its claim resolution status.
+	/// The first boolean is true only if [`Self::get_claimable_balances`] has been empty for at least
 	/// 4032 blocks as an additional protection against any bugs resulting in spuriously empty balance sets.
-	pub fn is_fully_resolved<L: Logger>(&self, logger: &L) -> bool {
+	pub fn process_claim_resolution_status<L: Logger>(&self, logger: &L) -> (bool, bool) {
 		let mut is_all_funds_claimed = self.get_claimable_balances().is_empty();
 		let current_height = self.current_best_block().height;
 		let mut inner = self.inner.lock().unwrap();
@@ -2011,7 +2015,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		match (inner.balances_empty_height, is_all_funds_claimed) {
 			(Some(balances_empty_height), true) => {
 				// Claimed all funds, check if reached the blocks threshold.
-				current_height >= balances_empty_height + BLOCKS_THRESHOLD
+				(current_height >= balances_empty_height + BLOCKS_THRESHOLD, false)
 			},
 			(Some(_), false) => {
 				// previously assumed we claimed all funds, but we have new funds to claim.
@@ -2021,7 +2025,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 					"WARNING: LDK thought it was done claiming all the available funds in the ChannelMonitor for channel {}, but later decided it had more to claim. This is potentially an important bug in LDK, please report it at https://github.com/lightningdevkit/rust-lightning/issues/new",
 					inner.get_funding_txo().0);
 				inner.balances_empty_height = None;
-				false
+				(false, true)
 			},
 			(None, true) => {
 				// Claimed all funds but `balances_empty_height` is None. It is set to the
@@ -2030,11 +2034,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 					"ChannelMonitor funded at {} is now fully resolved. It will become archivable in {} blocks",
 					inner.get_funding_txo().0, BLOCKS_THRESHOLD);
 				inner.balances_empty_height = Some(current_height);
-				false
+				(false, true)
 			},
 			(None, false) => {
 				// Have funds to claim.
-				false
+				(false, false)
 			},
 		}
 	}