Allow passing an OutputSweeperSync to the sync-KVStore-async-BP

`OutputSweeper::new_with_kv_store_sync` is a pretty strange API -
it allows building an async `OutputSweeper` where the only `await`s
are on a sync `KVStore`, ie will immediately block until the IO
operation completes.

While this isn't broken (futures are allowed to take their time,
and async runtimes have to handle this, though they often don't
handle it particularly well), its pretty weird. It seems to exist
largely for `process_events_async_with_kv_store_sync`, which does
async `Event` handling but sync `KVStore` operations (like the
existing pre-0.2 "async" background processor).

Instead, we allow passing an `OutputSweeperSync` to
`process_events_async_with_kv_store_sync`, keeping the API
consistent such that a user would use the appropriate
`OutputSweeper` variant, but fetching the inner async
`OutputSweeper` inside the BP.

Author: TheBlueMatt

lightning-background-processor/src/lib.rs

@@ -48,11 +48,9 @@ use lightning::onion_message::messenger::AOnionMessenger;
 use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
 use lightning::routing::scoring::{ScoreUpdate, WriteableScore};
 use lightning::routing::utxo::UtxoLookup;
-use lightning::sign::ChangeDestinationSource;
-#[cfg(feature = "std")]
-use lightning::sign::ChangeDestinationSourceSync;
-use lightning::sign::EntropySource;
-use lightning::sign::OutputSpender;
+use lightning::sign::{
+	ChangeDestinationSource, ChangeDestinationSourceSync, EntropySource, OutputSpender,
+};
 use lightning::util::logger::Logger;
 use lightning::util::persist::{
 	KVStore, KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_PERSISTENCE_KEY,
@@ -1361,7 +1359,7 @@ pub async fn process_events_async_with_kv_store_sync<
 	D: Deref,
 	O: Deref,
 	K: Deref,
-	OS: Deref<Target = OutputSweeper<T, D, F, CF, KVStoreSyncWrapper<K>, L, O>>,
+	OS: Deref<Target = OutputSweeperSync<T, D, F, CF, K, L, O>>,
 	S: Deref<Target = SC> + Send + Sync,
 	SC: for<'b> WriteableScore<'b>,
 	SleepFuture: core::future::Future<Output = bool> + core::marker::Unpin,
@@ -1386,7 +1384,7 @@ where
 	PM::Target: APeerManager,
 	LM::Target: ALiquidityManager,
 	O::Target: OutputSpender,
-	D::Target: ChangeDestinationSource,
+	D::Target: ChangeDestinationSourceSync,
 	K::Target: KVStoreSync,
 {
 	let kv_store = KVStoreSyncWrapper(kv_store);
@@ -1399,7 +1397,7 @@ where
 		gossip_sync,
 		peer_manager,
 		liquidity_manager,
-		sweeper,
+		sweeper.as_ref().map(|os| os.sweeper_async()),
 		logger,
 		scorer,
 		sleeper,

lightning/src/sign/mod.rs

@@ -1052,12 +1052,11 @@ pub trait ChangeDestinationSourceSync {
 }
 
 /// A wrapper around [`ChangeDestinationSource`] to allow for async calls.
-#[cfg(any(test, feature = "_test_utils"))]
+///
+/// You should likely never use this directly but rather allow LDK to build this when required to
+/// build higher-level sync wrappers.
+#[doc(hidden)]
 pub struct ChangeDestinationSourceSyncWrapper<T: Deref>(T)
-where
-	T::Target: ChangeDestinationSourceSync;
-#[cfg(not(any(test, feature = "_test_utils")))]
-pub(crate) struct ChangeDestinationSourceSyncWrapper<T: Deref>(T)
 where
 	T::Target: ChangeDestinationSourceSync;
 
@@ -1080,6 +1079,16 @@ where
 	}
 }
 
+impl<T: Deref> Deref for ChangeDestinationSourceSyncWrapper<T>
+where
+	T::Target: ChangeDestinationSourceSync,
+{
+	type Target = Self;
+	fn deref(&self) -> &Self {
+		self
+	}
+}
+
 mod sealed {
 	use bitcoin::secp256k1::{Scalar, SecretKey};
 

lightning/src/util/sweep.rs

@@ -19,7 +19,6 @@ use crate::sign::{
 	ChangeDestinationSource, ChangeDestinationSourceSync, ChangeDestinationSourceSyncWrapper,
 	OutputSpender, SpendableOutputDescriptor,
 };
-use crate::sync::Arc;
 use crate::sync::Mutex;
 use crate::util::logger::Logger;
 use crate::util::persist::{
@@ -353,47 +352,6 @@ where
 	logger: L,
 }
 
-impl<B: Deref, D: Deref, E: Deref, F: Deref, K: Deref, L: Deref, O: Deref>
-	OutputSweeper<B, D, E, F, KVStoreSyncWrapper<K>, L, O>
-where
-	B::Target: BroadcasterInterface,
-	D::Target: ChangeDestinationSource,
-	E::Target: FeeEstimator,
-	F::Target: Filter + Send + Sync,
-	K::Target: KVStoreSync,
-	L::Target: Logger,
-	O::Target: OutputSpender,
-{
-	/// Constructs a new [`OutputSweeper`] based on a [`KVStoreSync`].
-	pub fn new_with_kv_store_sync(
-		best_block: BestBlock, broadcaster: B, fee_estimator: E, chain_data_source: Option<F>,
-		output_spender: O, change_destination_source: D, kv_store_sync: K, logger: L,
-	) -> Self {
-		let kv_store = KVStoreSyncWrapper(kv_store_sync);
-
-		Self::new(
-			best_block,
-			broadcaster,
-			fee_estimator,
-			chain_data_source,
-			output_spender,
-			change_destination_source,
-			kv_store,
-			logger,
-		)
-	}
-
-	/// Reads an [`OutputSweeper`] from the given reader and returns it with a synchronous [`KVStoreSync`].
-	pub fn read_with_kv_store_sync<R: io::Read>(
-		reader: &mut R, args: (B, E, Option<F>, O, D, K, L),
-	) -> Result<Self, DecodeError> {
-		let kv_store = KVStoreSyncWrapper(args.5);
-		let args = (args.0, args.1, args.2, args.3, args.4, kv_store, args.6);
-
-		Self::read(reader, args)
-	}
-}
-
 impl<B: Deref, D: Deref, E: Deref, F: Deref, K: Deref, L: Deref, O: Deref>
 	OutputSweeper<B, D, E, F, K, L, O>
 where
@@ -981,15 +939,8 @@ where
 	L::Target: Logger,
 	O::Target: OutputSpender,
 {
-	sweeper: OutputSweeper<
-		B,
-		Arc<ChangeDestinationSourceSyncWrapper<D>>,
-		E,
-		F,
-		KVStoreSyncWrapper<K>,
-		L,
-		O,
-	>,
+	sweeper:
+		OutputSweeper<B, ChangeDestinationSourceSyncWrapper<D>, E, F, KVStoreSyncWrapper<K>, L, O>,
 }
 
 impl<B: Deref, D: Deref, E: Deref, F: Deref, K: Deref, L: Deref, O: Deref>
@@ -1009,7 +960,7 @@ where
 		output_spender: O, change_destination_source: D, kv_store: K, logger: L,
 	) -> Self {
 		let change_destination_source =
-			Arc::new(ChangeDestinationSourceSyncWrapper::new(change_destination_source));
+			ChangeDestinationSourceSyncWrapper::new(change_destination_source);
 
 		let kv_store = KVStoreSyncWrapper(kv_store);
 
@@ -1068,19 +1019,17 @@ where
 		self.sweeper.tracked_spendable_outputs()
 	}
 
-	/// Returns the inner async sweeper for testing purposes.
-	#[cfg(any(test, feature = "_test_utils"))]
+	/// Fetch the inner async sweeper.
+	///
+	/// In general you shouldn't have much reason to use this - you have a sync [`KVStore`] backing
+	/// this [`OutputSweeperSync`], fetching an async [`OutputSweeper`] won't accomplish much, all
+	/// the async methods will hang waiting on your sync [`KVStore`] and likely confuse your async
+	/// runtime. This exists primarily for LDK-internal use, including outside of this crate.
+	#[doc(hidden)]
 	pub fn sweeper_async(
 		&self,
-	) -> &OutputSweeper<
-		B,
-		Arc<ChangeDestinationSourceSyncWrapper<D>>,
-		E,
-		F,
-		KVStoreSyncWrapper<K>,
-		L,
-		O,
-	> {
+	) -> &OutputSweeper<B, ChangeDestinationSourceSyncWrapper<D>, E, F, KVStoreSyncWrapper<K>, L, O>
+	{
 		&self.sweeper
 	}
 }