From: Gursharan Singh <3442979+G8XSU@users.noreply.github.com>
Date: Wed, 27 Mar 2024 15:16:14 +0000 (+0100)
Subject: Stop tracking MonitorUpdates from ChainSync in pending_monitor_updates
X-Git-Tag: v0.0.124-beta~133^2~2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=7de602a38ad68a5c75db641ee96f5a0c2693f1f9;p=rust-lightning

Stop tracking MonitorUpdates from ChainSync in pending_monitor_updates

We no longer need to track them since we no longer hold events for
pending MonitorUpdates resulting from ChainSync.
---

diff --git a/lightning-persister/src/fs_store.rs b/lightning-persister/src/fs_store.rs
index 364a3ee70..014dde3e0 100644
--- a/lightning-persister/src/fs_store.rs
+++ b/lightning-persister/src/fs_store.rs
@@ -500,7 +500,7 @@ mod tests {
 			txid: Txid::from_str("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(),
 			index: 0
 		};
-		match store.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
+		match store.persist_new_channel(test_txo, &added_monitors[0].1) {
 			ChannelMonitorUpdateStatus::UnrecoverableError => {},
 			_ => panic!("unexpected result from persisting new channel")
 		}
diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs
index 1421bd5f8..fb2bfc3fd 100644
--- a/lightning/src/chain/chainmonitor.rs
+++ b/lightning/src/chain/chainmonitor.rs
@@ -350,7 +350,6 @@ where C::Target: chain::Filter,
 			let update_id = MonitorUpdateId {
 				contents: UpdateOrigin::ChainSync(chain_sync_update_id),
 			};
-			let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 
 			log_trace!(logger, "Syncing Channel Monitor for channel {} for block-data update_id {}",
 				log_funding_info!(monitor),
@@ -364,7 +363,6 @@ where C::Target: chain::Filter,
 					),
 				ChannelMonitorUpdateStatus::InProgress => {
 					log_debug!(logger, "Channel Monitor sync for channel {} in progress.", log_funding_info!(monitor));
-					pending_monitor_updates.push(update_id);
 				},
 				ChannelMonitorUpdateStatus::UnrecoverableError => {
 					return Err(());
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 95bc2a7c6..b9672dc40 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -16,7 +16,7 @@ use crate::chain::chaininterface::ConfirmationTarget;
 #[cfg(test)]
 use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW;
 use crate::chain::chainmonitor;
-use crate::chain::chainmonitor::{MonitorUpdateId, UpdateOrigin};
+use crate::chain::chainmonitor::{MonitorUpdateId};
 use crate::chain::channelmonitor;
 use crate::chain::channelmonitor::MonitorEvent;
 use crate::chain::transaction::OutPoint;
@@ -516,7 +516,7 @@ pub struct TestPersister {
 	pub update_rets: Mutex<VecDeque<chain::ChannelMonitorUpdateStatus>>,
 	/// When we get an update_persisted_channel call with no ChannelMonitorUpdate, we insert the
 	/// MonitorUpdateId here.
-	pub chain_sync_monitor_persistences: Mutex<HashMap<OutPoint, HashSet<MonitorUpdateId>>>,
+	pub chain_sync_monitor_persistences: Mutex<VecDeque<OutPoint>>,
 	/// When we get an update_persisted_channel call *with* a ChannelMonitorUpdate, we insert the
 	/// MonitorUpdateId here.
 	pub offchain_monitor_updates: Mutex<HashMap<OutPoint, HashSet<MonitorUpdateId>>>,
@@ -525,7 +525,7 @@ impl TestPersister {
 	pub fn new() -> Self {
 		Self {
 			update_rets: Mutex::new(VecDeque::new()),
-			chain_sync_monitor_persistences: Mutex::new(new_hash_map()),
+			chain_sync_monitor_persistences: Mutex::new(VecDeque::new()),
 			offchain_monitor_updates: Mutex::new(new_hash_map()),
 		}
 	}
@@ -543,16 +543,16 @@ impl<Signer: sign::ecdsa::WriteableEcdsaChannelSigner> chainmonitor::Persist<Sig
 		chain::ChannelMonitorUpdateStatus::Completed
 	}
 
-	fn update_persisted_channel(&self, funding_txo: OutPoint, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
+	fn update_persisted_channel(&self, funding_txo: OutPoint, update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
 		let mut ret = chain::ChannelMonitorUpdateStatus::Completed;
 		if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() {
 			ret = update_ret;
 		}
-		let is_chain_sync = if let UpdateOrigin::ChainSync(_) = update_id.contents { true } else { false };
-		if is_chain_sync {
-			self.chain_sync_monitor_persistences.lock().unwrap().entry(funding_txo).or_insert(new_hash_set()).insert(update_id);
-		} else {
+
+		if update.is_some()  {
 			self.offchain_monitor_updates.lock().unwrap().entry(funding_txo).or_insert(new_hash_set()).insert(update_id);
+		} else {
+			self.chain_sync_monitor_persistences.lock().unwrap().push_back(funding_txo);
 		}
 		ret
 	}
@@ -564,7 +564,7 @@ impl<Signer: sign::ecdsa::WriteableEcdsaChannelSigner> chainmonitor::Persist<Sig
 			None => {
 				// If the channel was not in the offchain_monitor_updates map, it should be in the
 				// chain_sync_monitor_persistences map.
-				assert!(self.chain_sync_monitor_persistences.lock().unwrap().remove(&funding_txo).is_some());
+				self.chain_sync_monitor_persistences.lock().unwrap().retain(|x| x != &funding_txo);
 			}
 		};
 	}