Rename KVStore trait to KVStoreSync

Author: joostjager

lightning-background-processor/src/lib.rs

@@ -40,7 +40,7 @@ use lightning::sign::ChangeDestinationSourceSync;
 use lightning::sign::EntropySource;
 use lightning::sign::OutputSpender;
 use lightning::util::logger::Logger;
-use lightning::util::persist::{KVStore, Persister};
+use lightning::util::persist::{KVStoreSync, Persister};
 use lightning::util::sweep::OutputSweeper;
 #[cfg(feature = "std")]
 use lightning::util::sweep::OutputSweeperSync;
@@ -822,7 +822,7 @@ where
 	LM::Target: ALiquidityManager,
 	O::Target: 'static + OutputSpender,
 	D::Target: 'static + ChangeDestinationSource,
-	K::Target: 'static + KVStore,
+	K::Target: 'static + KVStoreSync,
 {
 	let mut should_break = false;
 	let async_event_handler = |event| {
@@ -1018,7 +1018,7 @@ impl BackgroundProcessor {
 		LM::Target: ALiquidityManager,
 		D::Target: ChangeDestinationSourceSync,
 		O::Target: 'static + OutputSpender,
-		K::Target: 'static + KVStore,
+		K::Target: 'static + KVStoreSync,
 	{
 		let stop_thread = Arc::new(AtomicBool::new(false));
 		let stop_thread_clone = Arc::clone(&stop_thread);
@@ -1186,7 +1186,8 @@ mod tests {
 	use lightning::types::payment::PaymentHash;
 	use lightning::util::config::UserConfig;
 	use lightning::util::persist::{
-		KVStore, CHANNEL_MANAGER_PERSISTENCE_KEY, CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
+		KVStoreSync, CHANNEL_MANAGER_PERSISTENCE_KEY,
+		CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
 		CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_KEY,
 		NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
 		SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -1420,7 +1421,7 @@ mod tests {
 		}
 	}
 
-	impl KVStore for Persister {
+	impl KVStoreSync for Persister {
 		fn read(
 			&self, primary_namespace: &str, secondary_namespace: &str, key: &str,
 		) -> lightning::io::Result<Vec<u8>> {

lightning-liquidity/tests/common/mod.rs

@@ -27,7 +27,7 @@ use lightning::routing::scoring::{ChannelUsage, ScoreLookUp, ScoreUpdate};
 use lightning::sign::{InMemorySigner, KeysManager};
 use lightning::util::config::UserConfig;
 use lightning::util::persist::{
-	KVStore, CHANNEL_MANAGER_PERSISTENCE_KEY, CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
+	KVStoreSync, CHANNEL_MANAGER_PERSISTENCE_KEY, CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
 	CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_KEY,
 	NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
 	SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -194,7 +194,7 @@ impl Persister {
 	}
 }
 
-impl KVStore for Persister {
+impl KVStoreSync for Persister {
 	fn read(
 		&self, primary_namespace: &str, secondary_namespace: &str, key: &str,
 	) -> lightning::io::Result<Vec<u8>> {

lightning-persister/src/fs_store.rs

@@ -1,7 +1,7 @@
 //! Objects related to [`FilesystemStore`] live here.
 use crate::utils::{check_namespace_key_validity, is_valid_kvstore_str};
 
-use lightning::util::persist::{KVStore, MigratableKVStore};
+use lightning::util::persist::{KVStoreSync, MigratableKVStore};
 use lightning::util::string::PrintableString;
 
 use std::collections::HashMap;
@@ -33,7 +33,7 @@ fn path_to_windows_str<T: AsRef<OsStr>>(path: &T) -> Vec<u16> {
 // The number of read/write/remove/list operations after which we clean up our `locks` HashMap.
 const GC_LOCK_INTERVAL: usize = 25;
 
-/// A [`KVStore`] implementation that writes to and reads from the file system.
+/// A [`KVStoreSync`] implementation that writes to and reads from the file system.
 pub struct FilesystemStore {
 	data_dir: PathBuf,
 	tmp_file_counter: AtomicUsize,
@@ -92,7 +92,7 @@ impl FilesystemStore {
 	}
 }
 
-impl KVStore for FilesystemStore {
+impl KVStoreSync for FilesystemStore {
 	fn read(
 		&self, primary_namespace: &str, secondary_namespace: &str, key: &str,
 	) -> lightning::io::Result<Vec<u8>> {

lightning-persister/src/test_utils.rs

@@ -4,15 +4,15 @@ use lightning::ln::functional_test_utils::{
 	create_network, create_node_cfgs, create_node_chanmgrs, send_payment,
 };
 use lightning::util::persist::{
-	migrate_kv_store_data, read_channel_monitors, KVStore, MigratableKVStore,
+	migrate_kv_store_data, read_channel_monitors, KVStoreSync, MigratableKVStore,
 	KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
 };
 use lightning::util::test_utils;
 use lightning::{check_added_monitors, check_closed_broadcast, check_closed_event};
 
 use std::panic::RefUnwindSafe;
 
-pub(crate) fn do_read_write_remove_list_persist<K: KVStore + RefUnwindSafe>(kv_store: &K) {
+pub(crate) fn do_read_write_remove_list_persist<K: KVStoreSync + RefUnwindSafe>(kv_store: &K) {
 	let data = [42u8; 32];
 
 	let primary_namespace = "testspace";
@@ -113,7 +113,7 @@ pub(crate) fn do_test_data_migration<S: MigratableKVStore, T: MigratableKVStore>
 
 // Integration-test the given KVStore implementation. Test relaying a few payments and check that
 // the persisted data is updated the appropriate number of times.
-pub(crate) fn do_test_store<K: KVStore + Sync>(store_0: &K, store_1: &K) {
+pub(crate) fn do_test_store<K: KVStoreSync + Sync>(store_0: &K, store_1: &K) {
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let chain_mon_0 = test_utils::TestChainMonitor::new(

lightning/src/ln/channelmanager.rs

@@ -1886,7 +1886,7 @@ where
 /// - Perform any periodic channel and payment checks by calling [`timer_tick_occurred`] roughly
 ///   every minute
 /// - Persist to disk whenever [`get_and_clear_needs_persistence`] returns `true` using a
-///   [`Persister`] such as a [`KVStore`] implementation
+///   [`Persister`] such as a [`KVStoreSync`] implementation
 /// - Handle [`Event`]s obtained via its [`EventsProvider`] implementation
 ///
 /// The [`Future`] returned by [`get_event_or_persistence_needed_future`] is useful in determining
@@ -2469,7 +2469,7 @@ where
 /// [`timer_tick_occurred`]: Self::timer_tick_occurred
 /// [`get_and_clear_needs_persistence`]: Self::get_and_clear_needs_persistence
 /// [`Persister`]: crate::util::persist::Persister
-/// [`KVStore`]: crate::util::persist::KVStore
+/// [`KVStoreSync`]: crate::util::persist::KVStoreSync
 /// [`get_event_or_persistence_needed_future`]: Self::get_event_or_persistence_needed_future
 /// [`lightning-block-sync`]: https://docs.rs/lightning_block_sync/latest/lightning_block_sync
 /// [`lightning-transaction-sync`]: https://docs.rs/lightning_transaction_sync/latest/lightning_transaction_sync

lightning/src/util/persist.rs

@@ -4,7 +4,7 @@
 // You may not use this file except in accordance with one or both of these
 // licenses.
 
-//! This module contains a simple key-value store trait [`KVStore`] that
+//! This module contains a simple key-value store trait [`KVStoreSync`] that
 //! allows one to implement the persistence for [`ChannelManager`], [`NetworkGraph`],
 //! and [`ChannelMonitor`] all in one place.
 //!
@@ -119,7 +119,7 @@ pub const MONITOR_UPDATING_PERSISTER_PREPEND_SENTINEL: &[u8] = &[0xFF; 2];
 /// **Note:** 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`.
-pub trait KVStore {
+pub trait KVStoreSync {
 	/// Returns the data stored for the given `primary_namespace`, `secondary_namespace`, and
 	/// `key`.
 	///
@@ -142,7 +142,7 @@ pub trait KVStore {
 	/// If the `lazy` flag is set to `true`, the backend implementation might choose to lazily
 	/// remove the given `key` at some point in time after the method returns, e.g., as part of an
 	/// eventual batch deletion of multiple keys. As a consequence, subsequent calls to
-	/// [`KVStore::list`] might include the removed key until the changes are actually persisted.
+	/// [`KVStoreSync::list`] might include the removed key until the changes are actually persisted.
 	///
 	/// Note that while setting the `lazy` flag reduces the I/O burden of multiple subsequent
 	/// `remove` calls, it also influences the atomicity guarantees as lazy `remove`s could
@@ -165,12 +165,12 @@ pub trait KVStore {
 	) -> Result<Vec<String>, io::Error>;
 }
 
-/// Provides additional interface methods that are required for [`KVStore`]-to-[`KVStore`]
+/// Provides additional interface methods that are required for [`KVStoreSync`]-to-[`KVStoreSync`]
 /// data migration.
-pub trait MigratableKVStore: KVStore {
+pub trait MigratableKVStore: KVStoreSync {
 	/// Returns *all* known keys as a list of `primary_namespace`, `secondary_namespace`, `key` tuples.
 	///
-	/// This is useful for migrating data from [`KVStore`] implementation to [`KVStore`]
+	/// This is useful for migrating data from [`KVStoreSync`] implementation to [`KVStoreSync`]
 	/// implementation.
 	///
 	/// Must exhaustively return all entries known to the store to ensure no data is missed, but
@@ -220,7 +220,7 @@ where
 	fn persist_scorer(&self, scorer: &S) -> Result<(), io::Error>;
 }
 
-impl<'a, A: KVStore + ?Sized, CM: Deref, L: Deref, S: Deref> Persister<'a, CM, L, S> for A
+impl<'a, A: KVStoreSync + ?Sized, CM: Deref, L: Deref, S: Deref> Persister<'a, CM, L, S> for A
 where
 	CM::Target: 'static + AChannelManager,
 	L::Target: 'static + Logger,
@@ -254,7 +254,7 @@ where
 	}
 }
 
-impl<ChannelSigner: EcdsaChannelSigner, K: KVStore + ?Sized> Persist<ChannelSigner> for K {
+impl<ChannelSigner: EcdsaChannelSigner, K: KVStoreSync + ?Sized> Persist<ChannelSigner> for K {
 	// TODO: We really need a way for the persister to inform the user that its time to crash/shut
 	// down once these start returning failure.
 	// Then we should return InProgress rather than UnrecoverableError, implying we should probably
@@ -322,7 +322,7 @@ pub fn read_channel_monitors<K: Deref, ES: Deref, SP: Deref>(
 	kv_store: K, entropy_source: ES, signer_provider: SP,
 ) -> Result<Vec<(BlockHash, ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>)>, io::Error>
 where
-	K::Target: KVStore,
+	K::Target: KVStoreSync,
 	ES::Target: EntropySource + Sized,
 	SP::Target: SignerProvider + Sized,
 {
@@ -367,15 +367,15 @@ where
 ///
 /// # Overview
 ///
-/// The main benefit this provides over the [`KVStore`]'s [`Persist`] implementation is decreased
+/// The main benefit this provides over the [`KVStoreSync`]'s [`Persist`] implementation is decreased
 /// I/O bandwidth and storage churn, at the expense of more IOPS (including listing, reading, and
 /// deleting) and complexity. This is because it writes channel monitor differential updates,
 /// whereas the other (default) implementation rewrites the entire monitor on each update. For
 /// routing nodes, updates can happen many times per second to a channel, and monitors can be tens
 /// of megabytes (or more). Updates can be as small as a few hundred bytes.
 ///
 /// Note that monitors written with `MonitorUpdatingPersister` are _not_ backward-compatible with
-/// the default [`KVStore`]'s [`Persist`] implementation. They have a prepended byte sequence,
+/// the default [`KVStoreSync`]'s [`Persist`] implementation. They have a prepended byte sequence,
 /// [`MONITOR_UPDATING_PERSISTER_PREPEND_SENTINEL`], applied to prevent deserialization with other
 /// persisters. This is because monitors written by this struct _may_ have unapplied updates. In
 /// order to downgrade, you must ensure that all updates are applied to the monitor, and remove the
@@ -427,7 +427,7 @@ where
 ///
 /// ## EXTREMELY IMPORTANT
 ///
-/// It is extremely important that your [`KVStore::read`] implementation uses the
+/// It is extremely important that your [`KVStoreSync::read`] implementation uses the
 /// [`io::ErrorKind::NotFound`] variant correctly: that is, when a file is not found, and _only_ in
 /// that circumstance (not when there is really a permissions error, for example). This is because
 /// neither channel monitor reading function lists updates. Instead, either reads the monitor, and
@@ -439,7 +439,7 @@ where
 /// Stale updates are pruned when the consolidation threshold is reached according to `maximum_pending_updates`.
 /// Monitor updates in the range between the latest `update_id` and `update_id - maximum_pending_updates`
 /// are deleted.
-/// The `lazy` flag is used on the [`KVStore::remove`] method, so there are no guarantees that the deletions
+/// The `lazy` flag is used on the [`KVStoreSync::remove`] method, so there are no guarantees that the deletions
 /// will complete. However, stale updates are not a problem for data integrity, since updates are
 /// only read that are higher than the stored [`ChannelMonitor`]'s `update_id`.
 ///
@@ -448,7 +448,7 @@ where
 /// [`MonitorUpdatingPersister::cleanup_stale_updates`] function.
 pub struct MonitorUpdatingPersister<K: Deref, L: Deref, ES: Deref, SP: Deref, BI: Deref, FE: Deref>
 where
-	K::Target: KVStore,
+	K::Target: KVStoreSync,
 	L::Target: Logger,
 	ES::Target: EntropySource + Sized,
 	SP::Target: SignerProvider + Sized,
@@ -468,7 +468,7 @@ where
 impl<K: Deref, L: Deref, ES: Deref, SP: Deref, BI: Deref, FE: Deref>
 	MonitorUpdatingPersister<K, L, ES, SP, BI, FE>
 where
-	K::Target: KVStore,
+	K::Target: KVStoreSync,
 	L::Target: Logger,
 	ES::Target: EntropySource + Sized,
 	SP::Target: SignerProvider + Sized,
@@ -508,7 +508,7 @@ where
 
 	/// Reads all stored channel monitors, along with any stored updates for them.
 	///
-	/// It is extremely important that your [`KVStore::read`] implementation uses the
+	/// It is extremely important that your [`KVStoreSync::read`] implementation uses the
 	/// [`io::ErrorKind::NotFound`] variant correctly. For more information, please see the
 	/// documentation for [`MonitorUpdatingPersister`].
 	pub fn read_all_channel_monitors_with_updates(
@@ -530,7 +530,7 @@ where
 
 	/// Read a single channel monitor, along with any stored updates for it.
 	///
-	/// It is extremely important that your [`KVStore::read`] implementation uses the
+	/// It is extremely important that your [`KVStoreSync::read`] implementation uses the
 	/// [`io::ErrorKind::NotFound`] variant correctly. For more information, please see the
 	/// documentation for [`MonitorUpdatingPersister`].
 	///
@@ -657,7 +657,7 @@ where
 	/// This function works by first listing all monitors, and then for each of them, listing all
 	/// updates. The updates that have an `update_id` less than or equal to than the stored monitor
 	/// are deleted. The deletion can either be lazy or non-lazy based on the `lazy` flag; this will
-	/// be passed to [`KVStore::remove`].
+	/// be passed to [`KVStoreSync::remove`].
 	pub fn cleanup_stale_updates(&self, lazy: bool) -> Result<(), io::Error> {
 		let monitor_keys = self.kv_store.list(
 			CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -696,15 +696,15 @@ impl<
 		FE: Deref,
 	> Persist<ChannelSigner> for MonitorUpdatingPersister<K, L, ES, SP, BI, FE>
 where
-	K::Target: KVStore,
+	K::Target: KVStoreSync,
 	L::Target: Logger,
 	ES::Target: EntropySource + Sized,
 	SP::Target: SignerProvider + Sized,
 	BI::Target: BroadcasterInterface,
 	FE::Target: FeeEstimator,
 {
 	/// Persists a new channel. This means writing the entire monitor to the
-	/// parametrized [`KVStore`].
+	/// parametrized [`KVStoreSync`].
 	fn persist_new_channel(
 		&self, monitor_name: MonitorName, monitor: &ChannelMonitor<ChannelSigner>,
 	) -> chain::ChannelMonitorUpdateStatus {
@@ -737,11 +737,11 @@ where
 		}
 	}
 
-	/// Persists a channel update, writing only the update to the parameterized [`KVStore`] if possible.
+	/// Persists a channel update, writing only the update to the parameterized [`KVStoreSync`] if possible.
 	///
 	/// In some cases, this will forward to [`MonitorUpdatingPersister::persist_new_channel`]:
 	///
-	///   - No full monitor is found in [`KVStore`]
+	///   - No full monitor is found in [`KVStoreSync`]
 	///   - The number of pending updates exceeds `maximum_pending_updates` as given to [`Self::new`]
 	///   - LDK commands re-persisting the entire monitor through this function, specifically when
 	///	    `update` is `None`.
@@ -851,7 +851,7 @@ impl<K: Deref, L: Deref, ES: Deref, SP: Deref, BI: Deref, FE: Deref>
 	MonitorUpdatingPersister<K, L, ES, SP, BI, FE>
 where
 	ES::Target: EntropySource + Sized,
-	K::Target: KVStore,
+	K::Target: KVStoreSync,
 	L::Target: Logger,
 	SP::Target: SignerProvider + Sized,
 	BI::Target: BroadcasterInterface,
@@ -932,7 +932,7 @@ pub enum MonitorName {
 }
 
 impl MonitorName {
-	/// Attempts to construct a `MonitorName` from a storage key returned by [`KVStore::list`].
+	/// Attempts to construct a `MonitorName` from a storage key returned by [`KVStoreSync::list`].
 	///
 	/// This is useful when you need to reconstruct the original data the key represents.
 	fn from_str(monitor_key: &str) -> Result<Self, io::Error> {
@@ -1461,7 +1461,7 @@ mod tests {
 
 	#[test]
 	fn kvstore_trait_object_usage() {
-		let store: Arc<dyn KVStore + Send + Sync> = Arc::new(TestStore::new(false));
+		let store: Arc<dyn KVStoreSync + Send + Sync> = Arc::new(TestStore::new(false));
 		assert!(persist_fn::<_, TestChannelSigner>(Arc::clone(&store)));
 	}
 }