From 910a00e3d0cfb8aa21b83c9876f0f0137b14bba0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 27 Sep 2023 22:26:57 +0000 Subject: [PATCH 1/5] Use `Result<_, io::Error>` over `io::Result<_>` Personally I've always found the overload of a prelude enum to be confusing, and never bothered to handle it properly in bindings as a result. To avoid needing to do so now, we simply move the newly-introduced `io::Result` usages over to `Result<_, io::Error>`. --- lightning/src/util/persist.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index dbe3ee8161e..4335a124d4d 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -16,7 +16,7 @@ use bitcoin::{BlockHash, Txid}; use crate::{io, log_error}; use crate::alloc::string::ToString; -use crate::prelude::{Vec, String}; +use crate::prelude::*; use crate::chain; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; @@ -98,11 +98,11 @@ pub trait KVStore { /// `namespace` and `sub_namespace`. /// /// [`ErrorKind::NotFound`]: io::ErrorKind::NotFound - fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> io::Result>; + fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> Result, io::Error>; /// Persists the given data under the given `key`. /// /// Will create the given `namespace` and `sub_namespace` if not already present in the store. - fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> io::Result<()>; + fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> Result<(), io::Error>; /// Removes any data that had previously been persisted under the given `key`. /// /// If the `lazy` flag is set to `true`, the backend implementation might choose to lazily @@ -117,12 +117,12 @@ pub trait KVStore { /// /// Returns successfully if no data will be stored for the given `namespace`, `sub_namespace`, and /// `key`, independently of whether it was present before its invokation or not. - fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> io::Result<()>; + fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> Result<(), io::Error>; /// Returns a list of keys that are stored under the given `sub_namespace` in `namespace`. /// /// Returns the keys in arbitrary order, so users requiring a particular order need to sort the /// returned keys. Returns an empty list if `namespace` or `sub_namespace` is unknown. - fn list(&self, namespace: &str, sub_namespace: &str) -> io::Result>; + fn list(&self, namespace: &str, sub_namespace: &str) -> Result, io::Error>; } /// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`WriteableScore`] to disk. From 18ef80f78abe0b4f9651978ad4756fd40a0635b0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 02:40:07 +0000 Subject: [PATCH 2/5] Refer to top-level persistence namespaces as `primary_namespace` This fixes a bindings build error as `namespace` is a C++ keyword which cannot be used as an argument, and while this could be fixed in the bindings rather than here, separating the term `namespace` between the concept (which refers to the primary and sub namespaces) and the primary namespace makes the documentation more readable. --- lightning/src/util/persist.rs | 53 ++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 4335a124d4d..0b3407d691a 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -74,35 +74,36 @@ pub const MONITOR_UPDATING_PERSISTER_PREPEND_SENTINEL: &[u8] = &[0xFF; 2]; /// Provides an interface that allows storage and retrieval of persisted values that are associated /// with given keys. /// -/// In order to avoid collisions the key space is segmented based on the given `namespace`s and -/// `sub_namespace`s. Implementations of this trait are free to handle them in different ways, as -/// long as per-namespace key uniqueness is asserted. +/// In order to avoid collisions the key space is segmented based on the given `primary_namespace`s +/// and `sub_namespace`s. Implementations of this trait are free to handle them in different ways, +/// as long as per-namespace key uniqueness is asserted. /// /// Keys and namespaces are required to be valid ASCII strings in the range of /// [`KVSTORE_NAMESPACE_KEY_ALPHABET`] and no longer than [`KVSTORE_NAMESPACE_KEY_MAX_LEN`]. Empty -/// namespaces and sub-namespaces (`""`) are assumed to be a valid, however, if `namespace` is -/// empty, `sub_namespace` is required to be empty, too. This means that concerns should always be -/// separated by namespace first, before sub-namespaces are used. While the number of namespaces -/// will be relatively small and is determined at compile time, there may be many sub-namespaces -/// per namespace. Note that per-namespace uniqueness needs to also hold for keys *and* -/// namespaces/sub-namespaces in any given namespace/sub-namespace, i.e., conflicts between keys -/// and equally named namespaces/sub-namespaces must be avoided. +/// primary namespaces and sub-namespaces (`""`) are assumed to be a valid, however, if +/// `primary_namespace` is empty, `sub_namespace` is required to be empty, too. This means that +/// concerns should always be separated by primary namespace first, before sub-namespaces are used. +/// While the number of primary namespaces will be relatively small and is determined at compile +/// time, there may be many sub-namespaces per primary namespace. Note that per-namespace +/// uniqueness needs to also hold for keys *and* namespaces in any given namespace, i.e., conflicts +/// between keys and equally named primary-namespaces/sub-namespaces must be avoided. /// /// **Note:** Users migrating custom persistence backends from the pre-v0.0.117 `KVStorePersister` -/// interface can use a concatenation of `[{namespace}/[{sub_namespace}/]]{key}` to recover a `key` compatible with the -/// data model previously assumed by `KVStorePersister::persist`. +/// interface can use a concatenation of `[{primary_namespace}/[{sub_namespace}/]]{key}` to recover +/// a `key` compatible with the data model previously assumed by `KVStorePersister::persist`. pub trait KVStore { - /// Returns the data stored for the given `namespace`, `sub_namespace`, and `key`. + /// Returns the data stored for the given `primary_namespace`, `sub_namespace`, and `key`. /// /// Returns an [`ErrorKind::NotFound`] if the given `key` could not be found in the given - /// `namespace` and `sub_namespace`. + /// `primary_namespace` and `sub_namespace`. /// /// [`ErrorKind::NotFound`]: io::ErrorKind::NotFound - fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> Result, io::Error>; + fn read(&self, primary_namespace: &str, sub_namespace: &str, key: &str) -> Result, io::Error>; /// Persists the given data under the given `key`. /// - /// Will create the given `namespace` and `sub_namespace` if not already present in the store. - fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> Result<(), io::Error>; + /// Will create the given `primary_namespace` and `sub_namespace` if not already present in the + /// store. + fn write(&self, primary_namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> Result<(), io::Error>; /// Removes any data that had previously been persisted under the given `key`. /// /// If the `lazy` flag is set to `true`, the backend implementation might choose to lazily @@ -115,14 +116,16 @@ pub trait KVStore { /// potentially get lost on crash after the method returns. Therefore, this flag should only be /// set for `remove` operations that can be safely replayed at a later time. /// - /// Returns successfully if no data will be stored for the given `namespace`, `sub_namespace`, and - /// `key`, independently of whether it was present before its invokation or not. - fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> Result<(), io::Error>; - /// Returns a list of keys that are stored under the given `sub_namespace` in `namespace`. + /// Returns successfully if no data will be stored for the given `primary_namespace`, + /// `sub_namespace`, and `key`, independently of whether it was present before its invokation + /// or not. + fn remove(&self, primary_namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> Result<(), io::Error>; + /// Returns a list of keys that are stored under the given `sub_namespace` in + /// `primary_namespace`. /// /// Returns the keys in arbitrary order, so users requiring a particular order need to sort the - /// returned keys. Returns an empty list if `namespace` or `sub_namespace` is unknown. - fn list(&self, namespace: &str, sub_namespace: &str) -> Result, io::Error>; + /// returned keys. Returns an empty list if `primary_namespace` or `sub_namespace` is unknown. + fn list(&self, primary_namespace: &str, sub_namespace: &str) -> Result, io::Error>; } /// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`WriteableScore`] to disk. @@ -299,7 +302,7 @@ where /// /// Each [`ChannelMonitorUpdate`] is stored in a dynamic sub-namespace, as follows: /// -/// - namespace: [`CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE`] +/// - primary-namespace: [`CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE`] /// - sub-namespace: [the monitor's encoded outpoint name] /// /// Under that sub-namespace, each update is stored with a number string, like `21`, which @@ -314,7 +317,7 @@ where /// /// `[CHANNEL_MONITOR_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1` /// -/// Updates would be stored as follows (with `/` delimiting namespace/sub-namespace/key): +/// Updates would be stored as follows (with `/` delimiting primary-namespace/sub-namespace/key): /// /// ```text /// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/1 From 676588942342844cddb0f622655a2ed82165e433 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 16:36:52 +0000 Subject: [PATCH 3/5] Rename the persistence `sub_namespace` to `secondary_namespace` With the top-level namespace now called "primary", "secondary" makes more sense than "sub". --- lightning/src/util/persist.rs | 66 ++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 0b3407d691a..f3b8187fc64 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -39,28 +39,28 @@ pub const KVSTORE_NAMESPACE_KEY_MAX_LEN: usize = 120; /// The namespace under which the [`ChannelManager`] will be persisted. pub const CHANNEL_MANAGER_PERSISTENCE_NAMESPACE: &str = ""; -/// The sub-namespace under which the [`ChannelManager`] will be persisted. +/// The secondary-namespace under which the [`ChannelManager`] will be persisted. pub const CHANNEL_MANAGER_PERSISTENCE_SUB_NAMESPACE: &str = ""; /// The key under which the [`ChannelManager`] will be persisted. pub const CHANNEL_MANAGER_PERSISTENCE_KEY: &str = "manager"; /// The namespace under which [`ChannelMonitor`]s will be persisted. pub const CHANNEL_MONITOR_PERSISTENCE_NAMESPACE: &str = "monitors"; -/// The sub-namespace under which [`ChannelMonitor`]s will be persisted. +/// The secondary-namespace under which [`ChannelMonitor`]s will be persisted. pub const CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE: &str = ""; /// The namespace under which [`ChannelMonitorUpdate`]s will be persisted. pub const CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE: &str = "monitor_updates"; /// The namespace under which the [`NetworkGraph`] will be persisted. pub const NETWORK_GRAPH_PERSISTENCE_NAMESPACE: &str = ""; -/// The sub-namespace under which the [`NetworkGraph`] will be persisted. +/// The secondary-namespace under which the [`NetworkGraph`] will be persisted. pub const NETWORK_GRAPH_PERSISTENCE_SUB_NAMESPACE: &str = ""; /// The key under which the [`NetworkGraph`] will be persisted. pub const NETWORK_GRAPH_PERSISTENCE_KEY: &str = "network_graph"; /// The namespace under which the [`WriteableScore`] will be persisted. pub const SCORER_PERSISTENCE_NAMESPACE: &str = ""; -/// The sub-namespace under which the [`WriteableScore`] will be persisted. +/// The secondary-namespace under which the [`WriteableScore`] will be persisted. pub const SCORER_PERSISTENCE_SUB_NAMESPACE: &str = ""; /// The key under which the [`WriteableScore`] will be persisted. pub const SCORER_PERSISTENCE_KEY: &str = "scorer"; @@ -75,35 +75,37 @@ pub const MONITOR_UPDATING_PERSISTER_PREPEND_SENTINEL: &[u8] = &[0xFF; 2]; /// with given keys. /// /// In order to avoid collisions the key space is segmented based on the given `primary_namespace`s -/// and `sub_namespace`s. Implementations of this trait are free to handle them in different ways, -/// as long as per-namespace key uniqueness is asserted. +/// and `secondary_namespace`s. Implementations of this trait are free to handle them in different +/// ways, as long as per-namespace key uniqueness is asserted. /// /// Keys and namespaces are required to be valid ASCII strings in the range of /// [`KVSTORE_NAMESPACE_KEY_ALPHABET`] and no longer than [`KVSTORE_NAMESPACE_KEY_MAX_LEN`]. Empty -/// primary namespaces and sub-namespaces (`""`) are assumed to be a valid, however, if -/// `primary_namespace` is empty, `sub_namespace` is required to be empty, too. This means that -/// concerns should always be separated by primary namespace first, before sub-namespaces are used. -/// While the number of primary namespaces will be relatively small and is determined at compile -/// time, there may be many sub-namespaces per primary namespace. Note that per-namespace -/// uniqueness needs to also hold for keys *and* namespaces in any given namespace, i.e., conflicts -/// between keys and equally named primary-namespaces/sub-namespaces must be avoided. +/// primary namespaces and secondary namespaces (`""`) are assumed to be a valid, however, if +/// `primary_namespace` is empty, `secondary_namespace` is required to be empty, too. This means +/// that concerns should always be separated by primary namespace first, before secondary +/// namespaces are used. While the number of primary namespaces will be relatively small and is +/// determined at compile time, there may be many secondary namespaces per primary namespace. Note +/// that per-namespace uniqueness needs to also hold for keys *and* namespaces in any given +/// namespace, i.e., conflicts between keys and equally named +/// primary namespaces/secondary namespaces must be avoided. /// /// **Note:** Users migrating custom persistence backends from the pre-v0.0.117 `KVStorePersister` -/// interface can use a concatenation of `[{primary_namespace}/[{sub_namespace}/]]{key}` to recover -/// a `key` compatible with the data model previously assumed by `KVStorePersister::persist`. +/// interface can use a concatenation of `[{primary_namespace}/[{secondary_namespace}/]]{key}` to +/// recover a `key` compatible with the data model previously assumed by `KVStorePersister::persist`. pub trait KVStore { - /// Returns the data stored for the given `primary_namespace`, `sub_namespace`, and `key`. + /// Returns the data stored for the given `primary_namespace`, `secondary_namespace`, and + /// `key`. /// /// Returns an [`ErrorKind::NotFound`] if the given `key` could not be found in the given - /// `primary_namespace` and `sub_namespace`. + /// `primary_namespace` and `secondary_namespace`. /// /// [`ErrorKind::NotFound`]: io::ErrorKind::NotFound - fn read(&self, primary_namespace: &str, sub_namespace: &str, key: &str) -> Result, io::Error>; + fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> Result, io::Error>; /// Persists the given data under the given `key`. /// - /// Will create the given `primary_namespace` and `sub_namespace` if not already present in the - /// store. - fn write(&self, primary_namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> Result<(), io::Error>; + /// Will create the given `primary_namespace` and `secondary_namespace` if not already present + /// in the store. + fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: &[u8]) -> Result<(), io::Error>; /// Removes any data that had previously been persisted under the given `key`. /// /// If the `lazy` flag is set to `true`, the backend implementation might choose to lazily @@ -117,15 +119,15 @@ pub trait KVStore { /// set for `remove` operations that can be safely replayed at a later time. /// /// Returns successfully if no data will be stored for the given `primary_namespace`, - /// `sub_namespace`, and `key`, independently of whether it was present before its invokation - /// or not. - fn remove(&self, primary_namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> Result<(), io::Error>; - /// Returns a list of keys that are stored under the given `sub_namespace` in + /// `secondary_namespace`, and `key`, independently of whether it was present before its + /// invokation or not. + fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> Result<(), io::Error>; + /// Returns a list of keys that are stored under the given `secondary_namespace` in /// `primary_namespace`. /// /// Returns the keys in arbitrary order, so users requiring a particular order need to sort the - /// returned keys. Returns an empty list if `primary_namespace` or `sub_namespace` is unknown. - fn list(&self, primary_namespace: &str, sub_namespace: &str) -> Result, io::Error>; + /// returned keys. Returns an empty list if `primary_namespace` or `secondary_namespace` is unknown. + fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> Result, io::Error>; } /// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`WriteableScore`] to disk. @@ -300,12 +302,12 @@ where /// Whole [`ChannelMonitor`]s are stored in the [`CHANNEL_MONITOR_PERSISTENCE_NAMESPACE`], using the /// familiar encoding of an [`OutPoint`] (for example, `[SOME-64-CHAR-HEX-STRING]_1`). /// -/// Each [`ChannelMonitorUpdate`] is stored in a dynamic sub-namespace, as follows: +/// Each [`ChannelMonitorUpdate`] is stored in a dynamic secondary namespace, as follows: /// -/// - primary-namespace: [`CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE`] -/// - sub-namespace: [the monitor's encoded outpoint name] +/// - primary namespace: [`CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE`] +/// - secondary namespace: [the monitor's encoded outpoint name] /// -/// Under that sub-namespace, each update is stored with a number string, like `21`, which +/// Under that secondary namespace, each update is stored with a number string, like `21`, which /// represents its `update_id` value. /// /// For example, consider this channel, named for its transaction ID and index, or [`OutPoint`]: @@ -317,7 +319,7 @@ where /// /// `[CHANNEL_MONITOR_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1` /// -/// Updates would be stored as follows (with `/` delimiting primary-namespace/sub-namespace/key): +/// Updates would be stored as follows (with `/` delimiting primary_namespace/secondary_namespace/key): /// /// ```text /// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/1 From 1cb810358dd24e20f2228ad07526f3bd40c5fd0f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 17:06:20 +0000 Subject: [PATCH 4/5] Update storage constants to new PRIMARY/SECONDARY namespace terms --- lightning-background-processor/src/lib.rs | 17 +-- lightning/src/util/persist.rs | 138 +++++++++++----------- 2 files changed, 79 insertions(+), 76 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 7ae14b4b4aa..e0c71bfcf92 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -868,7 +868,10 @@ mod tests { use lightning::util::config::UserConfig; use lightning::util::ser::Writeable; use lightning::util::test_utils; - use lightning::util::persist::{KVStore, CHANNEL_MANAGER_PERSISTENCE_NAMESPACE, CHANNEL_MANAGER_PERSISTENCE_SUB_NAMESPACE, CHANNEL_MANAGER_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SUB_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_KEY, SCORER_PERSISTENCE_NAMESPACE, SCORER_PERSISTENCE_SUB_NAMESPACE, SCORER_PERSISTENCE_KEY}; + use lightning::util::persist::{KVStore, + CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE, CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE, CHANNEL_MANAGER_PERSISTENCE_KEY, + NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_KEY, + SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY}; use lightning_persister::fs_store::FilesystemStore; use std::collections::VecDeque; use std::{fs, env}; @@ -988,8 +991,8 @@ mod tests { } fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> lightning::io::Result<()> { - if namespace == CHANNEL_MANAGER_PERSISTENCE_NAMESPACE && - sub_namespace == CHANNEL_MANAGER_PERSISTENCE_SUB_NAMESPACE && + if namespace == CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE && + sub_namespace == CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE && key == CHANNEL_MANAGER_PERSISTENCE_KEY { if let Some((error, message)) = self.manager_error { @@ -997,8 +1000,8 @@ mod tests { } } - if namespace == NETWORK_GRAPH_PERSISTENCE_NAMESPACE && - sub_namespace == NETWORK_GRAPH_PERSISTENCE_SUB_NAMESPACE && + if namespace == NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE && + sub_namespace == NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE && key == NETWORK_GRAPH_PERSISTENCE_KEY { if let Some(sender) = &self.graph_persistence_notifier { @@ -1013,8 +1016,8 @@ mod tests { } } - if namespace == SCORER_PERSISTENCE_NAMESPACE && - sub_namespace == SCORER_PERSISTENCE_SUB_NAMESPACE && + if namespace == SCORER_PERSISTENCE_PRIMARY_NAMESPACE && + sub_namespace == SCORER_PERSISTENCE_SECONDARY_NAMESPACE && key == SCORER_PERSISTENCE_KEY { if let Some((error, message)) = self.scorer_error { diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index f3b8187fc64..2a022c37cc4 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -37,31 +37,31 @@ pub const KVSTORE_NAMESPACE_KEY_ALPHABET: &str = "abcdefghijklmnopqrstuvwxyzABCD /// The maximum number of characters namespaces and keys may have. pub const KVSTORE_NAMESPACE_KEY_MAX_LEN: usize = 120; -/// The namespace under which the [`ChannelManager`] will be persisted. -pub const CHANNEL_MANAGER_PERSISTENCE_NAMESPACE: &str = ""; -/// The secondary-namespace under which the [`ChannelManager`] will be persisted. -pub const CHANNEL_MANAGER_PERSISTENCE_SUB_NAMESPACE: &str = ""; +/// The primary namespace under which the [`ChannelManager`] will be persisted. +pub const CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE: &str = ""; +/// The secondary namespace under which the [`ChannelManager`] will be persisted. +pub const CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE: &str = ""; /// The key under which the [`ChannelManager`] will be persisted. pub const CHANNEL_MANAGER_PERSISTENCE_KEY: &str = "manager"; -/// The namespace under which [`ChannelMonitor`]s will be persisted. -pub const CHANNEL_MONITOR_PERSISTENCE_NAMESPACE: &str = "monitors"; -/// The secondary-namespace under which [`ChannelMonitor`]s will be persisted. -pub const CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE: &str = ""; -/// The namespace under which [`ChannelMonitorUpdate`]s will be persisted. -pub const CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE: &str = "monitor_updates"; - -/// The namespace under which the [`NetworkGraph`] will be persisted. -pub const NETWORK_GRAPH_PERSISTENCE_NAMESPACE: &str = ""; -/// The secondary-namespace under which the [`NetworkGraph`] will be persisted. -pub const NETWORK_GRAPH_PERSISTENCE_SUB_NAMESPACE: &str = ""; +/// The primary namespace under which [`ChannelMonitor`]s will be persisted. +pub const CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE: &str = "monitors"; +/// The secondary namespace under which [`ChannelMonitor`]s will be persisted. +pub const CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE: &str = ""; +/// The primary namespace under which [`ChannelMonitorUpdate`]s will be persisted. +pub const CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE: &str = "monitor_updates"; + +/// The primary namespace under which the [`NetworkGraph`] will be persisted. +pub const NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE: &str = ""; +/// The secondary namespace under which the [`NetworkGraph`] will be persisted. +pub const NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE: &str = ""; /// The key under which the [`NetworkGraph`] will be persisted. pub const NETWORK_GRAPH_PERSISTENCE_KEY: &str = "network_graph"; -/// The namespace under which the [`WriteableScore`] will be persisted. -pub const SCORER_PERSISTENCE_NAMESPACE: &str = ""; -/// The secondary-namespace under which the [`WriteableScore`] will be persisted. -pub const SCORER_PERSISTENCE_SUB_NAMESPACE: &str = ""; +/// The primary namespace under which the [`WriteableScore`] will be persisted. +pub const SCORER_PERSISTENCE_PRIMARY_NAMESPACE: &str = ""; +/// The secondary namespace under which the [`WriteableScore`] will be persisted. +pub const SCORER_PERSISTENCE_SECONDARY_NAMESPACE: &str = ""; /// The key under which the [`WriteableScore`] will be persisted. pub const SCORER_PERSISTENCE_KEY: &str = "scorer"; @@ -164,26 +164,26 @@ impl<'a, A: KVStore, M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Der { /// Persist the given [`ChannelManager`] to disk, returning an error if persistence failed. fn persist_manager(&self, channel_manager: &ChannelManager) -> Result<(), io::Error> { - self.write(CHANNEL_MANAGER_PERSISTENCE_NAMESPACE, - CHANNEL_MANAGER_PERSISTENCE_SUB_NAMESPACE, - CHANNEL_MANAGER_PERSISTENCE_KEY, - &channel_manager.encode()) + self.write(CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE, + CHANNEL_MANAGER_PERSISTENCE_KEY, + &channel_manager.encode()) } /// Persist the given [`NetworkGraph`] to disk, returning an error if persistence failed. fn persist_graph(&self, network_graph: &NetworkGraph) -> Result<(), io::Error> { - self.write(NETWORK_GRAPH_PERSISTENCE_NAMESPACE, - NETWORK_GRAPH_PERSISTENCE_SUB_NAMESPACE, - NETWORK_GRAPH_PERSISTENCE_KEY, - &network_graph.encode()) + self.write(NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, + NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, + NETWORK_GRAPH_PERSISTENCE_KEY, + &network_graph.encode()) } /// Persist the given [`WriteableScore`] to disk, returning an error if persistence failed. fn persist_scorer(&self, scorer: &S) -> Result<(), io::Error> { - self.write(SCORER_PERSISTENCE_NAMESPACE, - SCORER_PERSISTENCE_SUB_NAMESPACE, - SCORER_PERSISTENCE_KEY, - &scorer.encode()) + self.write(SCORER_PERSISTENCE_PRIMARY_NAMESPACE, + SCORER_PERSISTENCE_SECONDARY_NAMESPACE, + SCORER_PERSISTENCE_KEY, + &scorer.encode()) } } @@ -196,8 +196,8 @@ impl Persist, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus { let key = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index); match self.write( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, &key, &monitor.encode()) { Ok(()) => chain::ChannelMonitorUpdateStatus::Completed, @@ -208,8 +208,8 @@ impl Persist, monitor: &ChannelMonitor, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus { let key = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index); match self.write( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, &key, &monitor.encode()) { Ok(()) => chain::ChannelMonitorUpdateStatus::Completed, @@ -230,7 +230,7 @@ where let mut res = Vec::new(); for stored_key in kv_store.list( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE)? + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE)? { if stored_key.len() < 66 { return Err(io::Error::new( @@ -248,7 +248,7 @@ where match <(BlockHash, ChannelMonitor<::Signer>)>::read( &mut io::Cursor::new( - kv_store.read(CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, &stored_key)?), + kv_store.read(CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, &stored_key)?), (&*entropy_source, &*signer_provider), ) { Ok((block_hash, channel_monitor)) => { @@ -299,12 +299,12 @@ where /// - [`Persist::persist_new_channel`], which persists whole [`ChannelMonitor`]s. /// - [`Persist::update_persisted_channel`], which persists only a [`ChannelMonitorUpdate`] /// -/// Whole [`ChannelMonitor`]s are stored in the [`CHANNEL_MONITOR_PERSISTENCE_NAMESPACE`], using the -/// familiar encoding of an [`OutPoint`] (for example, `[SOME-64-CHAR-HEX-STRING]_1`). +/// Whole [`ChannelMonitor`]s are stored in the [`CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE`], +/// using the familiar encoding of an [`OutPoint`] (for example, `[SOME-64-CHAR-HEX-STRING]_1`). /// /// Each [`ChannelMonitorUpdate`] is stored in a dynamic secondary namespace, as follows: /// -/// - primary namespace: [`CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE`] +/// - primary namespace: [`CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE`] /// - secondary namespace: [the monitor's encoded outpoint name] /// /// Under that secondary namespace, each update is stored with a number string, like `21`, which @@ -317,14 +317,14 @@ where /// /// Full channel monitors would be stored at a single key: /// -/// `[CHANNEL_MONITOR_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1` +/// `[CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1` /// /// Updates would be stored as follows (with `/` delimiting primary_namespace/secondary_namespace/key): /// /// ```text -/// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/1 -/// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/2 -/// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/3 +/// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/1 +/// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/2 +/// [CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE]/deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1/3 /// ``` /// ... and so on. /// @@ -426,8 +426,8 @@ where F::Target: FeeEstimator, { let monitor_list = self.kv_store.list( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, )?; let mut res = Vec::with_capacity(monitor_list.len()); for monitor_key in monitor_list { @@ -505,8 +505,8 @@ where ) -> Result<(BlockHash, ChannelMonitor<::Signer>), io::Error> { let outpoint: OutPoint = monitor_name.try_into()?; let mut monitor_cursor = io::Cursor::new(self.kv_store.read( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, monitor_name.as_str(), )?); // Discard the sentinel bytes if found. @@ -551,7 +551,7 @@ where &self, monitor_name: &MonitorName, update_name: &UpdateName, ) -> Result { let update_bytes = self.kv_store.read( - CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), update_name.as_str(), )?; @@ -559,7 +559,7 @@ where log_error!( self.logger, "Failed to read ChannelMonitorUpdate {}/{}/{}, reason: {}", - CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), update_name.as_str(), e, @@ -576,21 +576,21 @@ where /// be passed to [`KVStore::remove`]. pub fn cleanup_stale_updates(&self, lazy: bool) -> Result<(), io::Error> { let monitor_keys = self.kv_store.list( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, )?; for monitor_key in monitor_keys { let monitor_name = MonitorName::new(monitor_key)?; let (_, current_monitor) = self.read_monitor(&monitor_name)?; let updates = self .kv_store - .list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str())?; + .list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str())?; for update in updates { let update_name = UpdateName::new(update)?; // if the update_id is lower than the stored monitor, delete if update_name.0 <= current_monitor.get_latest_update_id() { self.kv_store.remove( - CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), update_name.as_str(), lazy, @@ -643,8 +643,8 @@ where monitor_bytes.extend_from_slice(MONITOR_UPDATING_PERSISTER_PREPEND_SENTINEL); monitor.write(&mut monitor_bytes).unwrap(); match self.kv_store.write( - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, monitor_name.as_str(), &monitor_bytes, ) { @@ -688,7 +688,7 @@ where // stale updates, so do nothing. } if let Err(e) = self.kv_store.remove( - CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), update_name.as_str(), true, @@ -708,8 +708,8 @@ where log_error!( self.logger, "error writing channel monitor {}/{}/{} reason: {}", - CHANNEL_MONITOR_PERSISTENCE_NAMESPACE, - CHANNEL_MONITOR_PERSISTENCE_SUB_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, + CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE, monitor_name.as_str(), e ); @@ -740,7 +740,7 @@ where let monitor_name = MonitorName::from(funding_txo); let update_name = UpdateName::from(update.update_id); match self.kv_store.write( - CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), update_name.as_str(), &update.encode(), @@ -750,7 +750,7 @@ where log_error!( self.logger, "error writing channel monitor update {}/{}/{} reason: {}", - CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), update_name.as_str(), e @@ -970,7 +970,7 @@ mod tests { let (_, cm_0) = persister_0.read_monitor(&monitor_name).unwrap(); if cm_0.get_latest_update_id() == $expected_update_id { assert_eq!( - persister_0.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + persister_0.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str()).unwrap().len(), 0, "updates stored when they shouldn't be in persister 0" @@ -986,7 +986,7 @@ mod tests { let (_, cm_1) = persister_1.read_monitor(&monitor_name).unwrap(); if cm_1.get_latest_update_id() == $expected_update_id { assert_eq!( - persister_1.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, + persister_1.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str()).unwrap().len(), 0, "updates stored when they shouldn't be in persister 1" @@ -1047,8 +1047,8 @@ mod tests { let (_, monitor) = &persisted_chan_data[0]; let monitor_name = MonitorName::from(monitor.get_funding_txo().0); // The channel should have 0 updates, as it wrote a full monitor and consolidated. - assert_eq!(persister_0.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str()).unwrap().len(), 0); - assert_eq!(persister_1.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str()).unwrap().len(), 0); + assert_eq!(persister_0.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str()).unwrap().len(), 0); + assert_eq!(persister_1.kv_store.list(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str()).unwrap().len(), 0); } // Test that if the `MonitorUpdatingPersister`'s can't actually write, trying to persist a @@ -1167,7 +1167,7 @@ mod tests { let monitor_name = MonitorName::from(monitor.get_funding_txo().0); persister_0 .kv_store - .write(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str(), UpdateName::from(1).as_str(), &[0u8; 1]) + .write(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), UpdateName::from(1).as_str(), &[0u8; 1]) .unwrap(); // Do the stale update cleanup @@ -1176,7 +1176,7 @@ mod tests { // Confirm the stale update is unreadable/gone assert!(persister_0 .kv_store - .read(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str(), UpdateName::from(1).as_str()) + .read(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), UpdateName::from(1).as_str()) .is_err()); // Force close. @@ -1188,7 +1188,7 @@ mod tests { // Write an update near u64::MAX persister_0 .kv_store - .write(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str(), UpdateName::from(u64::MAX - 1).as_str(), &[0u8; 1]) + .write(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), UpdateName::from(u64::MAX - 1).as_str(), &[0u8; 1]) .unwrap(); // Do the stale update cleanup @@ -1197,7 +1197,7 @@ mod tests { // Confirm the stale update is unreadable/gone assert!(persister_0 .kv_store - .read(CHANNEL_MONITOR_UPDATE_PERSISTENCE_NAMESPACE, monitor_name.as_str(), UpdateName::from(u64::MAX - 1).as_str()) + .read(CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE, monitor_name.as_str(), UpdateName::from(u64::MAX - 1).as_str()) .is_err()); } } From 47e1148283cd8e185569771a02be4225feca3763 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 17:28:04 +0000 Subject: [PATCH 5/5] Update remaining references to primary/secondary namespaces Update various variables, error strings, and the pending changelog entry to refer to new namespace terminology. --- lightning-background-processor/src/lib.rs | 36 ++++++++--------- lightning-persister/src/fs_store.rs | 46 +++++++++++----------- lightning-persister/src/test_utils.rs | 25 ++++++------ lightning-persister/src/utils.rs | 48 ++++++++++++----------- lightning/src/util/test_utils.rs | 32 +++++++-------- pending_changelog/kvstore.txt | 2 +- 6 files changed, 96 insertions(+), 93 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index e0c71bfcf92..efa1a42142c 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -506,10 +506,10 @@ use core::task; /// # use lightning_background_processor::{process_events_async, GossipSync}; /// # struct MyStore {} /// # impl lightning::util::persist::KVStore for MyStore { -/// # fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> io::Result> { Ok(Vec::new()) } -/// # fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> io::Result<()> { Ok(()) } -/// # fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> io::Result<()> { Ok(()) } -/// # fn list(&self, namespace: &str, sub_namespace: &str) -> io::Result> { Ok(Vec::new()) } +/// # fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> io::Result> { Ok(Vec::new()) } +/// # fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: &[u8]) -> io::Result<()> { Ok(()) } +/// # fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> io::Result<()> { Ok(()) } +/// # fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> io::Result> { Ok(Vec::new()) } /// # } /// # struct MyEventHandler {} /// # impl MyEventHandler { @@ -986,13 +986,13 @@ mod tests { } impl KVStore for Persister { - fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> lightning::io::Result> { - self.kv_store.read(namespace, sub_namespace, key) + fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> lightning::io::Result> { + self.kv_store.read(primary_namespace, secondary_namespace, key) } - fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> lightning::io::Result<()> { - if namespace == CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE && - sub_namespace == CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE && + fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: &[u8]) -> lightning::io::Result<()> { + if primary_namespace == CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE && + secondary_namespace == CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE && key == CHANNEL_MANAGER_PERSISTENCE_KEY { if let Some((error, message)) = self.manager_error { @@ -1000,8 +1000,8 @@ mod tests { } } - if namespace == NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE && - sub_namespace == NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE && + if primary_namespace == NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE && + secondary_namespace == NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE && key == NETWORK_GRAPH_PERSISTENCE_KEY { if let Some(sender) = &self.graph_persistence_notifier { @@ -1016,8 +1016,8 @@ mod tests { } } - if namespace == SCORER_PERSISTENCE_PRIMARY_NAMESPACE && - sub_namespace == SCORER_PERSISTENCE_SECONDARY_NAMESPACE && + if primary_namespace == SCORER_PERSISTENCE_PRIMARY_NAMESPACE && + secondary_namespace == SCORER_PERSISTENCE_SECONDARY_NAMESPACE && key == SCORER_PERSISTENCE_KEY { if let Some((error, message)) = self.scorer_error { @@ -1025,15 +1025,15 @@ mod tests { } } - self.kv_store.write(namespace, sub_namespace, key, buf) + self.kv_store.write(primary_namespace, secondary_namespace, key, buf) } - fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> lightning::io::Result<()> { - self.kv_store.remove(namespace, sub_namespace, key, lazy) + fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> lightning::io::Result<()> { + self.kv_store.remove(primary_namespace, secondary_namespace, key, lazy) } - fn list(&self, namespace: &str, sub_namespace: &str) -> lightning::io::Result> { - self.kv_store.list(namespace, sub_namespace) + fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> lightning::io::Result> { + self.kv_store.list(primary_namespace, secondary_namespace) } } diff --git a/lightning-persister/src/fs_store.rs b/lightning-persister/src/fs_store.rs index 42b28018fe9..3475544849f 100644 --- a/lightning-persister/src/fs_store.rs +++ b/lightning-persister/src/fs_store.rs @@ -67,7 +67,7 @@ impl FilesystemStore { } } - fn get_dest_dir_path(&self, namespace: &str, sub_namespace: &str) -> std::io::Result { + fn get_dest_dir_path(&self, primary_namespace: &str, secondary_namespace: &str) -> std::io::Result { let mut dest_dir_path = { #[cfg(target_os = "windows")] { @@ -81,9 +81,9 @@ impl FilesystemStore { } }; - dest_dir_path.push(namespace); - if !sub_namespace.is_empty() { - dest_dir_path.push(sub_namespace); + dest_dir_path.push(primary_namespace); + if !secondary_namespace.is_empty() { + dest_dir_path.push(secondary_namespace); } Ok(dest_dir_path) @@ -91,10 +91,10 @@ impl FilesystemStore { } impl KVStore for FilesystemStore { - fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> std::io::Result> { - check_namespace_key_validity(namespace, sub_namespace, Some(key), "read")?; + fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> std::io::Result> { + check_namespace_key_validity(primary_namespace, secondary_namespace, Some(key), "read")?; - let mut dest_file_path = self.get_dest_dir_path(namespace, sub_namespace)?; + let mut dest_file_path = self.get_dest_dir_path(primary_namespace, secondary_namespace)?; dest_file_path.push(key); let mut buf = Vec::new(); @@ -114,10 +114,10 @@ impl KVStore for FilesystemStore { Ok(buf) } - fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> std::io::Result<()> { - check_namespace_key_validity(namespace, sub_namespace, Some(key), "write")?; + fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: &[u8]) -> std::io::Result<()> { + check_namespace_key_validity(primary_namespace, secondary_namespace, Some(key), "write")?; - let mut dest_file_path = self.get_dest_dir_path(namespace, sub_namespace)?; + let mut dest_file_path = self.get_dest_dir_path(primary_namespace, secondary_namespace)?; dest_file_path.push(key); let parent_directory = dest_file_path @@ -201,10 +201,10 @@ impl KVStore for FilesystemStore { res } - fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool) -> std::io::Result<()> { - check_namespace_key_validity(namespace, sub_namespace, Some(key), "remove")?; + fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> std::io::Result<()> { + check_namespace_key_validity(primary_namespace, secondary_namespace, Some(key), "remove")?; - let mut dest_file_path = self.get_dest_dir_path(namespace, sub_namespace)?; + let mut dest_file_path = self.get_dest_dir_path(primary_namespace, secondary_namespace)?; dest_file_path.push(key); if !dest_file_path.is_file() { @@ -290,10 +290,10 @@ impl KVStore for FilesystemStore { Ok(()) } - fn list(&self, namespace: &str, sub_namespace: &str) -> std::io::Result> { - check_namespace_key_validity(namespace, sub_namespace, None, "list")?; + fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> std::io::Result> { + check_namespace_key_validity(primary_namespace, secondary_namespace, None, "list")?; - let prefixed_dest = self.get_dest_dir_path(namespace, sub_namespace)?; + let prefixed_dest = self.get_dest_dir_path(primary_namespace, secondary_namespace)?; let mut keys = Vec::new(); if !Path::new(&prefixed_dest).exists() { @@ -320,7 +320,7 @@ impl KVStore for FilesystemStore { let metadata = p.metadata()?; - // We allow the presence of directories in the empty namespace and just skip them. + // We allow the presence of directories in the empty primary namespace and just skip them. if metadata.is_dir() { continue; } @@ -328,9 +328,9 @@ impl KVStore for FilesystemStore { // If we otherwise don't find a file at the given path something went wrong. if !metadata.is_file() { debug_assert!(false, "Failed to list keys of {}/{}: file couldn't be accessed.", - PrintableString(namespace), PrintableString(sub_namespace)); + PrintableString(primary_namespace), PrintableString(secondary_namespace)); let msg = format!("Failed to list keys of {}/{}: file couldn't be accessed.", - PrintableString(namespace), PrintableString(sub_namespace)); + PrintableString(primary_namespace), PrintableString(secondary_namespace)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } @@ -342,17 +342,17 @@ impl KVStore for FilesystemStore { } } else { debug_assert!(false, "Failed to list keys of {}/{}: file path is not valid UTF-8", - PrintableString(namespace), PrintableString(sub_namespace)); + PrintableString(primary_namespace), PrintableString(secondary_namespace)); let msg = format!("Failed to list keys of {}/{}: file path is not valid UTF-8", - PrintableString(namespace), PrintableString(sub_namespace)); + PrintableString(primary_namespace), PrintableString(secondary_namespace)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } } Err(e) => { debug_assert!(false, "Failed to list keys of {}/{}: {}", - PrintableString(namespace), PrintableString(sub_namespace), e); + PrintableString(primary_namespace), PrintableString(secondary_namespace), e); let msg = format!("Failed to list keys of {}/{}: {}", - PrintableString(namespace), PrintableString(sub_namespace), e); + PrintableString(primary_namespace), PrintableString(secondary_namespace), e); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } } diff --git a/lightning-persister/src/test_utils.rs b/lightning-persister/src/test_utils.rs index 91557500f3d..360fa3492bf 100644 --- a/lightning-persister/src/test_utils.rs +++ b/lightning-persister/src/test_utils.rs @@ -12,34 +12,35 @@ use std::panic::RefUnwindSafe; pub(crate) fn do_read_write_remove_list_persist(kv_store: &K) { let data = [42u8; 32]; - let namespace = "testspace"; - let sub_namespace = "testsubspace"; + let primary_namespace = "testspace"; + let secondary_namespace = "testsubspace"; let key = "testkey"; // Test the basic KVStore operations. - kv_store.write(namespace, sub_namespace, key, &data).unwrap(); + kv_store.write(primary_namespace, secondary_namespace, key, &data).unwrap(); - // Test empty namespace/sub_namespace is allowed, but not empty namespace and non-empty - // sub-namespace, and not empty key. + // Test empty primary_namespace/secondary_namespace is allowed, but not empty primary_namespace + // and non-empty secondary_namespace, and not empty key. kv_store.write("", "", key, &data).unwrap(); - let res = std::panic::catch_unwind(|| kv_store.write("", sub_namespace, key, &data)); + let res = std::panic::catch_unwind(|| kv_store.write("", secondary_namespace, key, &data)); assert!(res.is_err()); - let res = std::panic::catch_unwind(|| kv_store.write(namespace, sub_namespace, "", &data)); + let res = std::panic::catch_unwind(|| kv_store.write(primary_namespace, secondary_namespace, "", &data)); assert!(res.is_err()); - let listed_keys = kv_store.list(namespace, sub_namespace).unwrap(); + let listed_keys = kv_store.list(primary_namespace, secondary_namespace).unwrap(); assert_eq!(listed_keys.len(), 1); assert_eq!(listed_keys[0], key); - let read_data = kv_store.read(namespace, sub_namespace, key).unwrap(); + let read_data = kv_store.read(primary_namespace, secondary_namespace, key).unwrap(); assert_eq!(data, &*read_data); - kv_store.remove(namespace, sub_namespace, key, false).unwrap(); + kv_store.remove(primary_namespace, secondary_namespace, key, false).unwrap(); - let listed_keys = kv_store.list(namespace, sub_namespace).unwrap(); + let listed_keys = kv_store.list(primary_namespace, secondary_namespace).unwrap(); assert_eq!(listed_keys.len(), 0); - // Ensure we have no issue operating with namespace/sub_namespace/key being KVSTORE_NAMESPACE_KEY_MAX_LEN + // Ensure we have no issue operating with primary_namespace/secondary_namespace/key being + // KVSTORE_NAMESPACE_KEY_MAX_LEN let max_chars: String = std::iter::repeat('A').take(KVSTORE_NAMESPACE_KEY_MAX_LEN).collect(); kv_store.write(&max_chars, &max_chars, &max_chars, &data).unwrap(); diff --git a/lightning-persister/src/utils.rs b/lightning-persister/src/utils.rs index 54ec230de2d..59a615937c9 100644 --- a/lightning-persister/src/utils.rs +++ b/lightning-persister/src/utils.rs @@ -6,51 +6,53 @@ pub(crate) fn is_valid_kvstore_str(key: &str) -> bool { key.len() <= KVSTORE_NAMESPACE_KEY_MAX_LEN && key.chars().all(|c| KVSTORE_NAMESPACE_KEY_ALPHABET.contains(c)) } -pub(crate) fn check_namespace_key_validity(namespace: &str, sub_namespace: &str, key: Option<&str>, operation: &str) -> Result<(), std::io::Error> { +pub(crate) fn check_namespace_key_validity( + primary_namespace: &str, secondary_namespace: &str, key: Option<&str>, operation: &str) +-> Result<(), std::io::Error> { if let Some(key) = key { if key.is_empty() { debug_assert!(false, "Failed to {} {}/{}/{}: key may not be empty.", operation, - PrintableString(namespace), PrintableString(sub_namespace), PrintableString(key)); + PrintableString(primary_namespace), PrintableString(secondary_namespace), PrintableString(key)); let msg = format!("Failed to {} {}/{}/{}: key may not be empty.", operation, - PrintableString(namespace), PrintableString(sub_namespace), PrintableString(key)); + PrintableString(primary_namespace), PrintableString(secondary_namespace), PrintableString(key)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } - if namespace.is_empty() && !sub_namespace.is_empty() { + if primary_namespace.is_empty() && !secondary_namespace.is_empty() { debug_assert!(false, - "Failed to {} {}/{}/{}: namespace may not be empty if a non-empty sub-namespace is given.", + "Failed to {} {}/{}/{}: primary namespace may not be empty if a non-empty secondary namespace is given.", operation, - PrintableString(namespace), PrintableString(sub_namespace), PrintableString(key)); + PrintableString(primary_namespace), PrintableString(secondary_namespace), PrintableString(key)); let msg = format!( - "Failed to {} {}/{}/{}: namespace may not be empty if a non-empty sub-namespace is given.", operation, - PrintableString(namespace), PrintableString(sub_namespace), PrintableString(key)); + "Failed to {} {}/{}/{}: primary namespace may not be empty if a non-empty secondary namespace is given.", operation, + PrintableString(primary_namespace), PrintableString(secondary_namespace), PrintableString(key)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } - if !is_valid_kvstore_str(namespace) || !is_valid_kvstore_str(sub_namespace) || !is_valid_kvstore_str(key) { - debug_assert!(false, "Failed to {} {}/{}/{}: namespace, sub-namespace, and key must be valid.", + if !is_valid_kvstore_str(primary_namespace) || !is_valid_kvstore_str(secondary_namespace) || !is_valid_kvstore_str(key) { + debug_assert!(false, "Failed to {} {}/{}/{}: primary namespace, secondary namespace, and key must be valid.", operation, - PrintableString(namespace), PrintableString(sub_namespace), PrintableString(key)); - let msg = format!("Failed to {} {}/{}/{}: namespace, sub-namespace, and key must be valid.", + PrintableString(primary_namespace), PrintableString(secondary_namespace), PrintableString(key)); + let msg = format!("Failed to {} {}/{}/{}: primary namespace, secondary namespace, and key must be valid.", operation, - PrintableString(namespace), PrintableString(sub_namespace), PrintableString(key)); + PrintableString(primary_namespace), PrintableString(secondary_namespace), PrintableString(key)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } } else { - if namespace.is_empty() && !sub_namespace.is_empty() { + if primary_namespace.is_empty() && !secondary_namespace.is_empty() { debug_assert!(false, - "Failed to {} {}/{}: namespace may not be empty if a non-empty sub-namespace is given.", - operation, PrintableString(namespace), PrintableString(sub_namespace)); + "Failed to {} {}/{}: primary namespace may not be empty if a non-empty secondary namespace is given.", + operation, PrintableString(primary_namespace), PrintableString(secondary_namespace)); let msg = format!( - "Failed to {} {}/{}: namespace may not be empty if a non-empty sub-namespace is given.", - operation, PrintableString(namespace), PrintableString(sub_namespace)); + "Failed to {} {}/{}: primary namespace may not be empty if a non-empty secondary namespace is given.", + operation, PrintableString(primary_namespace), PrintableString(secondary_namespace)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } - if !is_valid_kvstore_str(namespace) || !is_valid_kvstore_str(sub_namespace) { - debug_assert!(false, "Failed to {} {}/{}: namespace and sub-namespace must be valid.", - operation, PrintableString(namespace), PrintableString(sub_namespace)); - let msg = format!("Failed to {} {}/{}: namespace and sub-namespace must be valid.", - operation, PrintableString(namespace), PrintableString(sub_namespace)); + if !is_valid_kvstore_str(primary_namespace) || !is_valid_kvstore_str(secondary_namespace) { + debug_assert!(false, "Failed to {} {}/{}: primary namespace and secondary namespace must be valid.", + operation, PrintableString(primary_namespace), PrintableString(secondary_namespace)); + let msg = format!("Failed to {} {}/{}: primary namespace and secondary namespace must be valid.", + operation, PrintableString(primary_namespace), PrintableString(secondary_namespace)); return Err(std::io::Error::new(std::io::ErrorKind::Other, msg)); } } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index d53cd39b119..c5e37ca9059 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -440,12 +440,12 @@ impl TestStore { } impl KVStore for TestStore { - fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> io::Result> { + fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> io::Result> { let persisted_lock = self.persisted_bytes.lock().unwrap(); - let prefixed = if sub_namespace.is_empty() { - namespace.to_string() + let prefixed = if secondary_namespace.is_empty() { + primary_namespace.to_string() } else { - format!("{}/{}", namespace, sub_namespace) + format!("{}/{}", primary_namespace, secondary_namespace) }; if let Some(outer_ref) = persisted_lock.get(&prefixed) { @@ -460,7 +460,7 @@ impl KVStore for TestStore { } } - fn write(&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8]) -> io::Result<()> { + fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: &[u8]) -> io::Result<()> { if self.read_only { return Err(io::Error::new( io::ErrorKind::PermissionDenied, @@ -469,10 +469,10 @@ impl KVStore for TestStore { } let mut persisted_lock = self.persisted_bytes.lock().unwrap(); - let prefixed = if sub_namespace.is_empty() { - namespace.to_string() + let prefixed = if secondary_namespace.is_empty() { + primary_namespace.to_string() } else { - format!("{}/{}", namespace, sub_namespace) + format!("{}/{}", primary_namespace, secondary_namespace) }; let outer_e = persisted_lock.entry(prefixed).or_insert(HashMap::new()); let mut bytes = Vec::new(); @@ -481,7 +481,7 @@ impl KVStore for TestStore { Ok(()) } - fn remove(&self, namespace: &str, sub_namespace: &str, key: &str, _lazy: bool) -> io::Result<()> { + fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, _lazy: bool) -> io::Result<()> { if self.read_only { return Err(io::Error::new( io::ErrorKind::PermissionDenied, @@ -491,10 +491,10 @@ impl KVStore for TestStore { let mut persisted_lock = self.persisted_bytes.lock().unwrap(); - let prefixed = if sub_namespace.is_empty() { - namespace.to_string() + let prefixed = if secondary_namespace.is_empty() { + primary_namespace.to_string() } else { - format!("{}/{}", namespace, sub_namespace) + format!("{}/{}", primary_namespace, secondary_namespace) }; if let Some(outer_ref) = persisted_lock.get_mut(&prefixed) { outer_ref.remove(&key.to_string()); @@ -503,13 +503,13 @@ impl KVStore for TestStore { Ok(()) } - fn list(&self, namespace: &str, sub_namespace: &str) -> io::Result> { + fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> io::Result> { let mut persisted_lock = self.persisted_bytes.lock().unwrap(); - let prefixed = if sub_namespace.is_empty() { - namespace.to_string() + let prefixed = if secondary_namespace.is_empty() { + primary_namespace.to_string() } else { - format!("{}/{}", namespace, sub_namespace) + format!("{}/{}", primary_namespace, secondary_namespace) }; match persisted_lock.entry(prefixed) { hash_map::Entry::Occupied(e) => Ok(e.get().keys().cloned().collect()), diff --git a/pending_changelog/kvstore.txt b/pending_changelog/kvstore.txt index d96fd69371b..3fe949500e6 100644 --- a/pending_changelog/kvstore.txt +++ b/pending_changelog/kvstore.txt @@ -1,3 +1,3 @@ ## Backwards Compatibility -* Users migrating custom persistence backends from the pre-v0.0.117 `KVStorePersister` interface can use a concatenation of `[{namespace}/[{sub_namespace}/]]{key}` to recover a `key` compatible with the data model previously assumed by `KVStorePersister::persist`. +* Users migrating custom persistence backends from the pre-v0.0.117 `KVStorePersister` interface can use a concatenation of `[{primary_namespace}/[{secondary_namespace}/]]{key}` to recover a `key` compatible with the data model previously assumed by `KVStorePersister::persist`.