From 7de602a38ad68a5c75db641ee96f5a0c2693f1f9 Mon Sep 17 00:00:00 2001 From: Gursharan Singh <3442979+G8XSU@users.noreply.github.com> Date: Wed, 27 Mar 2024 16:16:14 +0100 Subject: [PATCH] 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. --- lightning-persister/src/fs_store.rs | 2 +- lightning/src/chain/chainmonitor.rs | 2 -- lightning/src/util/test_utils.rs | 18 +++++++++--------- 3 files changed, 10 insertions(+), 12 deletions(-) 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>, /// When we get an update_persisted_channel call with no ChannelMonitorUpdate, we insert the /// MonitorUpdateId here. - pub chain_sync_monitor_persistences: Mutex>>, + pub chain_sync_monitor_persistences: Mutex>, /// When we get an update_persisted_channel call *with* a ChannelMonitorUpdate, we insert the /// MonitorUpdateId here. pub offchain_monitor_updates: Mutex>>, @@ -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 chainmonitor::Persist, _data: &channelmonitor::ChannelMonitor, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus { + fn update_persisted_channel(&self, funding_txo: OutPoint, update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor, 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 chainmonitor::Persist { // 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); } }; } -- 2.39.5