From 13eac47ed939fc44cffb5f7b25bbbf4684ae8fde Mon Sep 17 00:00:00 2001
From: Gursharan Singh <3442979+G8XSU@users.noreply.github.com>
Date: Thu, 28 Sep 2023 21:18:47 -0700
Subject: [PATCH] Correctly mark chain_sync updates in test_utils

We were incorrectly marking updates as chain_sync
or not in test_utils based on whether monitor_update
is None or not. Instead, use UpdateOrigin to determine it.
---
 lightning/src/chain/chainmonitor.rs | 36 +++++++++++++++++++----------
 lightning/src/util/test_utils.rs    |  5 ++--
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs
index a49b3b42..81d23b3a 100644
--- a/lightning/src/chain/chainmonitor.rs
+++ b/lightning/src/chain/chainmonitor.rs
@@ -47,23 +47,35 @@ use core::ops::Deref;
 use core::sync::atomic::{AtomicUsize, Ordering};
 use bitcoin::secp256k1::PublicKey;
 
-#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
-/// A specific update's ID stored in a `MonitorUpdateId`, separated out to make the contents
-/// entirely opaque.
-enum UpdateOrigin {
-	/// An update that was generated by the `ChannelManager` (via our `chain::Watch`
-	/// implementation). This corresponds to an actual [`ChannelMonitorUpdate::update_id`] field
-	/// and [`ChannelMonitor::get_latest_update_id`].
-	OffChain(u64),
-	/// An update that was generated during blockchain processing. The ID here is specific to the
-	/// generating [`ChainMonitor`] and does *not* correspond to any on-disk IDs.
-	ChainSync(u64),
+mod update_origin {
+	#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
+	/// A specific update's ID stored in a `MonitorUpdateId`, separated out to make the contents
+	/// entirely opaque.
+	pub(crate) enum UpdateOrigin {
+		/// An update that was generated by the `ChannelManager` (via our [`crate::chain::Watch`]
+		/// implementation). This corresponds to an actual [ChannelMonitorUpdate::update_id] field
+		/// and [ChannelMonitor::get_latest_update_id].
+		///
+		/// [ChannelMonitor::get_latest_update_id]: crate::chain::channelmonitor::ChannelMonitor::get_latest_update_id
+		/// [ChannelMonitorUpdate::update_id]: crate::chain::channelmonitor::ChannelMonitorUpdate::update_id
+		OffChain(u64),
+		/// An update that was generated during blockchain processing. The ID here is specific to the
+		/// generating [ChannelMonitor] and does *not* correspond to any on-disk IDs.
+		///
+		/// [ChannelMonitor]: crate::chain::channelmonitor::ChannelMonitor
+		ChainSync(u64),
+	}
 }
 
+#[cfg(any(feature = "_test_utils", test))]
+pub(crate) use update_origin::UpdateOrigin;
+#[cfg(not(any(feature = "_test_utils", test)))]
+use update_origin::UpdateOrigin;
+
 /// An opaque identifier describing a specific [`Persist`] method call.
 #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
 pub struct MonitorUpdateId {
-	contents: UpdateOrigin,
+	pub(crate) contents: UpdateOrigin,
 }
 
 impl MonitorUpdateId {
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 18c3db76..bc9c0a32 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -13,7 +13,7 @@ use crate::chain::chaininterface;
 use crate::chain::chaininterface::ConfirmationTarget;
 use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW;
 use crate::chain::chainmonitor;
-use crate::chain::chainmonitor::MonitorUpdateId;
+use crate::chain::chainmonitor::{MonitorUpdateId, UpdateOrigin};
 use crate::chain::channelmonitor;
 use crate::chain::channelmonitor::MonitorEvent;
 use crate::chain::transaction::OutPoint;
@@ -419,7 +419,8 @@ impl<Signer: sign::WriteableEcdsaChannelSigner> chainmonitor::Persist<Signer> fo
 		if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() {
 			ret = update_ret;
 		}
-		if update.is_none() {
+		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(HashSet::new()).insert(update_id);
 		} else {
 			self.offchain_monitor_updates.lock().unwrap().entry(funding_txo).or_insert(HashSet::new()).insert(update_id);
-- 
2.30.2