← Watch feed
Drop unnecessary Arcs in Sweeper and LiquidityManager sync wrappers
Public commit record
83/100 · Strong
What the developer wrote
Authored by Matt Corallo
Drop unnecessary Arcs in Sweeper and LiquidityManager sync wrappers
Both `OutputSweeperSync` and `LiquidityManagerSync` added `Arc`s to
their internal state to allow returning a reference to that
internal state in testing. While this is required to get a
forever-lifetime reference to that state, we don't actually need
that in testing.
Instead, we turn off the borrow checker in the
`lightning-background-processor` async tests where required,
avoiding the extra heap indirection.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit is a straightforward internal code cleanup. It removes unnecessary Arc (reference-counted pointer) wrappers around internal objects in two sync-wrapper types, replacing them with direct ownership. To keep existing tests compiling, the tests use a small unsafe block to create a static reference. There is no user-facing behavior change and no security fix.
Recommended action
No security action required. Treat as normal refactoring/review for correctness and test stability.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors LiquidityManagerSync and OutputSweeperSync so their inner LiquidityManager/OutputSweeper are owned directly rather than behind an Arc. Corresponding test-only accessor methods now return &T instead of Arc
Changed components
lightning-liquidity/src/manager.rslightning/src/util/sweep.rslightning-background-processor/src/lib.rsInspect captured patch +79 / −52
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index 44ce52b..b4b0186 100644
--- a/lightning-background-processor/src/lib.rs
+++ b/lightning-background-processor/src/lib.rs
@@ -1846,11 +1846,13 @@ mod tests {
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::Writeable;
- use lightning::util::sweep::{OutputSpendStatus, OutputSweeperSync, PRUNE_DELAY_BLOCKS};
+ use lightning::util::sweep::{
+ OutputSpendStatus, OutputSweeper, OutputSweeperSync, PRUNE_DELAY_BLOCKS,
+ };
use lightning::util::test_utils;
use lightning::{get_event, get_event_msg};
use lightning_liquidity::utils::time::DefaultTimeProvider;
- use lightning_liquidity::{ALiquidityManagerSync, LiquidityManagerSync};
+ use lightning_liquidity::{ALiquidityManagerSync, LiquidityManager, LiquidityManagerSync};
use lightning_persister::fs_store::FilesystemStore;
use lightning_rapid_gossip_sync::RapidGossipSync;
use std::collections::VecDeque;
@@ -2781,6 +2783,17 @@ mod tests {
);
let kv_store = Arc::new(KVStoreSyncWrapper(kv_store_sync));
+ // Yes, you can unsafe { turn off the borrow checker }
+ let lm_async: &'static LiquidityManager<_, _, _, _, _, _> = unsafe {
+ &*(nodes[0].liquidity_manager.get_lm_async()
+ as *const LiquidityManager<_, _, _, _, _, _>)
+ as &'static LiquidityManager<_, _, _, _, _, _>
+ };
+ let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
+ &*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
+ as &'static OutputSweeper<_, _, _, _, _, _, _>
+ };
+
let bp_future = super::process_events_async(
kv_store,
|_: _| async { Ok(()) },
@@ -2789,8 +2802,8 @@ mod tests {
Some(Arc::clone(&nodes[0].messenger)),
nodes[0].rapid_gossip_sync(),
Arc::clone(&nodes[0].peer_manager),
- Some(nodes[0].liquidity_manager.get_lm_async()),
- Some(nodes[0].sweeper.sweeper_async()),
+ Some(lm_async),
+ Some(sweeper_async),
Arc::clone(&nodes[0].logger),
Some(Arc::clone(&nodes[0].scorer)),
move |dur: Duration| {
@@ -3289,6 +3302,17 @@ mod tests {
Arc::new(Persister::new(data_dir).with_graph_persistence_notifier(sender));
let kv_store = Arc::new(KVStoreSyncWrapper(kv_store_sync));
+ // Yes, you can unsafe { turn off the borrow checker }
+ let lm_async: &'static LiquidityManager<_, _, _, _, _, _> = unsafe {
+ &*(nodes[0].liquidity_manager.get_lm_async()
+ as *const LiquidityManager<_, _, _, _, _, _>)
+ as &'static LiquidityManager<_, _, _, _, _, _>
+ };
+ let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
+ &*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
+ as &'static OutputSweeper<_, _, _, _, _, _, _>
+ };
+
let (exit_sender, exit_receiver) = tokio::sync::watch::channel(());
let bp_future = super::process_events_async(
kv_store,
@@ -3298,8 +3322,8 @@ mod tests {
Some(Arc::clone(&nodes[0].messenger)),
nodes[0].rapid_gossip_sync(),
Arc::clone(&nodes[0].peer_manager),
- Some(nodes[0].liquidity_manager.get_lm_async()),
- Some(nodes[0].sweeper.sweeper_async()),
+ Some(lm_async),
+ Some(sweeper_async),
Arc::clone(&nodes[0].logger),
Some(Arc::clone(&nodes[0].scorer)),
move |dur: Duration| {
@@ -3505,6 +3529,17 @@ mod tests {
let (exit_sender, exit_receiver) = tokio::sync::watch::channel(());
+ // Yes, you can unsafe { turn off the borrow checker }
+ let lm_async: &'static LiquidityManager<_, _, _, _, _, _> = unsafe {
+ &*(nodes[0].liquidity_manager.get_lm_async()
+ as *const LiquidityManager<_, _, _, _, _, _>)
+ as &'static LiquidityManager<_, _, _, _, _, _>
+ };
+ let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
+ &*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
+ as &'static OutputSweeper<_, _, _, _, _, _, _>
+ };
+
let bp_future = super::process_events_async(
kv_store,
event_handler,
@@ -3513,8 +3548,8 @@ mod tests {
Some(Arc::clone(&nodes[0].messenger)),
nodes[0].no_gossip_sync(),
Arc::clone(&nodes[0].peer_manager),
- Some(nodes[0].liquidity_manager.get_lm_async()),
- Some(nodes[0].sweeper.sweeper_async()),
+ Some(lm_async),
+ Some(sweeper_async),
Arc::clone(&nodes[0].logger),
Some(Arc::clone(&nodes[0].scorer)),
move |dur: Duration| {
diff --git a/lightning-liquidity/src/manager.rs b/lightning-liquidity/src/manager.rs
index 490ca8b..d0925e1 100644
--- a/lightning-liquidity/src/manager.rs
+++ b/lightning-liquidity/src/manager.rs
@@ -195,15 +195,13 @@ pub trait ALiquidityManagerSync {
#[cfg(any(test, feature = "_test_utils"))]
fn get_lm_async(
&self,
- ) -> Arc<
- LiquidityManager<
- Self::ES,
- Self::NS,
- Self::CM,
- Self::C,
- Arc<KVStoreSyncWrapper<Self::KS>>,
- Self::TP,
- >,
+ ) -> &LiquidityManager<
+ Self::ES,
+ Self::NS,
+ Self::CM,
+ Self::C,
+ Arc<KVStoreSyncWrapper<Self::KS>>,
+ Self::TP,
>;
/// Returns a reference to the actual [`LiquidityManager`] object.
fn get_lm(
@@ -243,17 +241,15 @@ where
#[cfg(any(test, feature = "_test_utils"))]
fn get_lm_async(
&self,
- ) -> Arc<
- LiquidityManager<
- Self::ES,
- Self::NS,
- Self::CM,
- Self::C,
- Arc<KVStoreSyncWrapper<Self::KS>>,
- Self::TP,
- >,
+ ) -> &LiquidityManager<
+ Self::ES,
+ Self::NS,
+ Self::CM,
+ Self::C,
+ Arc<KVStoreSyncWrapper<Self::KS>>,
+ Self::TP,
> {
- Arc::clone(&self.inner)
+ &self.inner
}
fn get_lm(&self) -> &LiquidityManagerSync<ES, NS, CM, C, KS, TP> {
self
@@ -1040,7 +1036,7 @@ pub struct LiquidityManagerSync<
KS::Target: KVStoreSync,
TP::Target: TimeProvider,
{
- inner: Arc<LiquidityManager<ES, NS, CM, C, Arc<KVStoreSyncWrapper<KS>>, TP>>,
+ inner: LiquidityManager<ES, NS, CM, C, Arc<KVStoreSyncWrapper<KS>>, TP>,
}
#[cfg(feature = "time")]
@@ -1089,7 +1085,7 @@ where
unreachable!("LiquidityManager::new should not be pending in a sync context");
},
}?;
- Ok(Self { inner: Arc::new(inner) })
+ Ok(Self { inner })
}
}
@@ -1140,7 +1136,7 @@ where
unreachable!("LiquidityManager::new should not be pending in a sync context");
},
}?;
- Ok(Self { inner: Arc::new(inner) })
+ Ok(Self { inner })
}
/// Returns a reference to the LSPS0 client-side handler.
diff --git a/lightning/src/util/sweep.rs b/lightning/src/util/sweep.rs
index 8f3df5e..51cb5b3 100644
--- a/lightning/src/util/sweep.rs
+++ b/lightning/src/util/sweep.rs
@@ -981,16 +981,14 @@ where
L::Target: Logger,
O::Target: OutputSpender,
{
- sweeper: Arc<
- OutputSweeper<
- B,
- Arc<ChangeDestinationSourceSyncWrapper<D>>,
- E,
- F,
- Arc<KVStoreSyncWrapper<K>>,
- L,
- O,
- >,
+ sweeper: OutputSweeper<
+ B,
+ Arc<ChangeDestinationSourceSyncWrapper<D>>,
+ E,
+ F,
+ Arc<KVStoreSyncWrapper<K>>,
+ L,
+ O,
>,
}
@@ -1025,7 +1023,7 @@ where
kv_store,
logger,
);
- Self { sweeper: Arc::new(sweeper) }
+ Self { sweeper }
}
/// Regenerates and broadcasts the spending transaction for any outputs that are pending. Wraps
@@ -1074,18 +1072,16 @@ where
#[cfg(any(test, feature = "_test_utils"))]
pub fn sweeper_async(
&self,
- ) -> Arc<
- OutputSweeper<
- B,
- Arc<ChangeDestinationSourceSyncWrapper<D>>,
- E,
- F,
- Arc<KVStoreSyncWrapper<K>>,
- L,
- O,
- >,
+ ) -> &OutputSweeper<
+ B,
+ Arc<ChangeDestinationSourceSyncWrapper<D>>,
+ E,
+ F,
+ Arc<KVStoreSyncWrapper<K>>,
+ L,
+ O,
> {
- Arc::clone(&self.sweeper)
+ &self.sweeper
}
}
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.