Skip to content

Commit 5bc1417

Browse files
committed
Simplify legacy closed-channel monitor update persistence handling
Pre-0.1, after a channel was closed we generated `ChannelMonitorUpdate`s with a static `update_id` of `u64::MAX`. In this case, when using `MonitorUpdatingPersister`, we had to read the persisted `ChannelMonitor` to figure out what range of monitor updates to remove from disk. However, now that we have a `list` method there's no reason to do this anymore, we can just use that. Simplifying code that we anticipate never hitting anymore is always a win.
1 parent c903749 commit 5bc1417

File tree

1 file changed

+29
-42
lines changed

1 file changed

+29
-42
lines changed

lightning/src/util/persist.rs

Lines changed: 29 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use alloc::sync::Arc;
1616
use bitcoin::hashes::hex::FromHex;
1717
use bitcoin::{BlockHash, Txid};
1818

19-
use core::cmp;
2019
use core::future::Future;
2120
use core::ops::Deref;
2221
use core::pin::Pin;
@@ -931,21 +930,29 @@ where
931930
for monitor_key in monitor_keys {
932931
let monitor_name = MonitorName::from_str(&monitor_key)?;
933932
let (_, current_monitor) = self.read_monitor(&monitor_name, &monitor_key).await?;
934-
let updates = self
935-
.kv_store
936-
.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_key.as_str())
937-
.await?;
938-
for update in updates {
939-
let update_name = UpdateName::new(update)?;
940-
// if the update_id is lower than the stored monitor, delete
941-
if update_name.0 <= current_monitor.get_latest_update_id() {
942-
self.kv_store.remove(
943-
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
944-
monitor_key.as_str(),
945-
update_name.as_str(),
946-
lazy,
947-
).await?;
948-
}
933+
let latest_update_id = current_monitor.get_latest_update_id();
934+
self.cleanup_stale_updates_for_monitor_to(&monitor_key, latest_update_id, lazy).await;
935+
}
936+
Ok(())
937+
}
938+
939+
async fn cleanup_stale_updates_for_monitor_to(
940+
&self, monitor_key: &str, latest_update_id: u64, lazy: bool,
941+
) -> Result<(), io::Error> {
942+
let updates = self
943+
.kv_store
944+
.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_key)
945+
.await?;
946+
for update in updates {
947+
let update_name = UpdateName::new(update)?;
948+
// if the update_id is lower than the stored monitor, delete
949+
if update_name.0 <= latest_update_id {
950+
self.kv_store.remove(
951+
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
952+
monitor_key,
953+
update_name.as_str(),
954+
lazy,
955+
).await?;
949956
}
950957
}
951958
Ok(())
@@ -994,40 +1001,19 @@ where
9941001
update.encode(),
9951002
).await
9961003
} else {
997-
// In case of channel-close monitor update, we need to read old monitor before persisting
998-
// the new one in order to determine the cleanup range.
999-
let maybe_old_monitor = match monitor.get_latest_update_id() {
1000-
LEGACY_CLOSED_CHANNEL_UPDATE_ID => {
1001-
let monitor_key = monitor_name.to_string();
1002-
self.read_monitor(&monitor_name, &monitor_key).await.ok()
1003-
},
1004-
_ => None,
1005-
};
1006-
10071004
// We could write this update, but it meets criteria of our design that calls for a full monitor write.
10081005
let write_status = self.persist_new_channel(monitor_name, monitor).await;
10091006

10101007
if let Ok(()) = write_status {
10111008
let channel_closed_legacy =
10121009
monitor.get_latest_update_id() == LEGACY_CLOSED_CHANNEL_UPDATE_ID;
1013-
let cleanup_range = if channel_closed_legacy {
1014-
// If there is an error while reading old monitor, we skip clean up.
1015-
maybe_old_monitor.map(|(_, ref old_monitor)| {
1016-
let start = old_monitor.get_latest_update_id();
1017-
// We never persist an update with the legacy closed update_id
1018-
let end = cmp::min(
1019-
start.saturating_add(self.maximum_pending_updates),
1020-
LEGACY_CLOSED_CHANNEL_UPDATE_ID - 1,
1021-
);
1022-
(start, end)
1023-
})
1010+
let latest_update_id = monitor.get_latest_update_id();
1011+
if channel_closed_legacy {
1012+
let monitor_key = monitor_name.to_string();
1013+
self.cleanup_stale_updates_for_monitor_to(&monitor_key, latest_update_id, true).await;
10241014
} else {
1025-
let end = monitor.get_latest_update_id();
1015+
let end = latest_update_id;
10261016
let start = end.saturating_sub(self.maximum_pending_updates);
1027-
Some((start, end))
1028-
};
1029-
1030-
if let Some((start, end)) = cleanup_range {
10311017
self.cleanup_in_range(monitor_name, start, end).await;
10321018
}
10331019
}
@@ -1267,6 +1253,7 @@ impl From<u64> for UpdateName {
12671253

12681254
#[cfg(test)]
12691255
mod tests {
1256+
use core::cmp;
12701257
use super::*;
12711258
use crate::chain::ChannelMonitorUpdateStatus;
12721259
use crate::events::ClosureReason;

0 commit comments

Comments
 (0)