Allow passing an `OutputSweeperSync` to the sync-KVStore-async-BP
What changed, and why it matters
This commit is a code cleanup in the Lightning Dev Kit Rust library. It removes an awkward API that mixed synchronous storage with an asynchronous sweeper component, and instead lets callers pass a fully synchronous sweeper where synchronous storage is expected. There is no direct security vulnerability being fixed; it is an API-consistency and maintainability change.
No immediate security action required. Treat as a normal API refactor; downstream users relying on OutputSweeper::new_with_kv_store_sync or read_with_kv_store_sync will need to migrate to OutputSweeperSync.
Security signals we found
No memory-safety, cryptographic, or authorization issues visible in the diff
Change reduces API surface that could be misused to create blocking async futures unexpectedly
No input validation, parsing, or serialization changes that would introduce vulnerabilities
No changes to secrets, key handling, or transaction signing logic
Evidence from the diff
The patch removes OutputSweeper::new_with_kv_store_sync and OutputSweeper::read_with_kv_store_sync, which created an async OutputSweeper backed by a synchronous KVStore (causing async methods to block on I/O). It updates process_events_async_with_kv_store_sync to accept an OutputSweeperSync and expose its inner async sweeper via a new #[doc(hidden)] sweeper_async() method. It also makes ChangeDestinationSourceSyncWrapper dereference to itself and removes an unnecessary Arc wrapper. These are internal refactorings to keep sync/async API boundaries consistent.
Changed components
lightning-background-processor/src/lib.rslightning/src/sign/mod.rslightning/src/util/sweep.rsInspect captured patch +33 / −79
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index 86576e7..627b4e6 100644
--- a/lightning-background-processor/src/lib.rs
+++ b/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,
@@ -61,9 +59,7 @@ use lightning::util::persist::{
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
-use lightning::util::sweep::OutputSweeper;
-#[cfg(feature = "std")]
-use lightning::util::sweep::OutputSweeperSync;
+use lightning::util::sweep::{OutputSweeper, OutputSweeperSync};
#[cfg(feature = "std")]
use lightning::util::wakers::Sleeper;
use lightning_rapid_gossip_sync::RapidGossipSync;
@@ -1361,7 +1357,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 +1382,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 +1395,7 @@ where
gossip_sync,
peer_manager,
liquidity_manager,
- sweeper,
+ sweeper.as_ref().map(|os| os.sweeper_async()),
logger,
scorer,
sleeper,
diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
index f9db5ff..c0bbb94 100644
--- a/lightning/src/sign/mod.rs
+++ b/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};
diff --git a/lightning/src/util/sweep.rs b/lightning/src/util/sweep.rs
index 052a56d..b60d4d8 100644
--- a/lightning/src/util/sweep.rs
+++ b/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
}
}
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.