Skip to content

Commit f53c5e9

Browse files
TheBlueMattTheBlueMatt
committed
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.
1 parent 4c9c211 commit f53c5e9

File tree

3 files changed

+79
-52
lines changed

3 files changed

+79
-52
lines changed

‎lightning-background-processor/src/lib.rs‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,11 +1846,13 @@ mod tests {
18461846
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
18471847
};
18481848
use lightning::util::ser::Writeable;
1849-
use lightning::util::sweep::{OutputSpendStatus, OutputSweeperSync, PRUNE_DELAY_BLOCKS};
1849+
use lightning::util::sweep::{
1850+
OutputSpendStatus, OutputSweeper, OutputSweeperSync, PRUNE_DELAY_BLOCKS,
1851+
};
18501852
use lightning::util::test_utils;
18511853
use lightning::{get_event, get_event_msg};
18521854
use lightning_liquidity::utils::time::DefaultTimeProvider;
1853-
use lightning_liquidity::{ALiquidityManagerSync, LiquidityManagerSync};
1855+
use lightning_liquidity::{ALiquidityManagerSync, LiquidityManager, LiquidityManagerSync};
18541856
use lightning_persister::fs_store::FilesystemStore;
18551857
use lightning_rapid_gossip_sync::RapidGossipSync;
18561858
use std::collections::VecDeque;
@@ -2781,6 +2783,17 @@ mod tests {
27812783
);
27822784
let kv_store = Arc::new(KVStoreSyncWrapper(kv_store_sync));
27832785

2786+
// Yes, you can unsafe { turn off the borrow checker }
2787+
let lm_async: &'static LiquidityManager<_, _, _, _, _, _> = unsafe {
2788+
&*(nodes[0].liquidity_manager.get_lm_async()
2789+
as *const LiquidityManager<_, _, _, _, _, _>)
2790+
as &'static LiquidityManager<_, _, _, _, _, _>
2791+
};
2792+
let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
2793+
&*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
2794+
as &'static OutputSweeper<_, _, _, _, _, _, _>
2795+
};
2796+
27842797
let bp_future = super::process_events_async(
27852798
kv_store,
27862799
|_: _| async { Ok(()) },
@@ -2789,8 +2802,8 @@ mod tests {
27892802
Some(Arc::clone(&nodes[0].messenger)),
27902803
nodes[0].rapid_gossip_sync(),
27912804
Arc::clone(&nodes[0].peer_manager),
2792-
Some(nodes[0].liquidity_manager.get_lm_async()),
2793-
Some(nodes[0].sweeper.sweeper_async()),
2805+
Some(lm_async),
2806+
Some(sweeper_async),
27942807
Arc::clone(&nodes[0].logger),
27952808
Some(Arc::clone(&nodes[0].scorer)),
27962809
move |dur: Duration| {
@@ -3289,6 +3302,17 @@ mod tests {
32893302
Arc::new(Persister::new(data_dir).with_graph_persistence_notifier(sender));
32903303
let kv_store = Arc::new(KVStoreSyncWrapper(kv_store_sync));
32913304

3305+
// Yes, you can unsafe { turn off the borrow checker }
3306+
let lm_async: &'static LiquidityManager<_, _, _, _, _, _> = unsafe {
3307+
&*(nodes[0].liquidity_manager.get_lm_async()
3308+
as *const LiquidityManager<_, _, _, _, _, _>)
3309+
as &'static LiquidityManager<_, _, _, _, _, _>
3310+
};
3311+
let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
3312+
&*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
3313+
as &'static OutputSweeper<_, _, _, _, _, _, _>
3314+
};
3315+
32923316
let (exit_sender, exit_receiver) = tokio::sync::watch::channel(());
32933317
let bp_future = super::process_events_async(
32943318
kv_store,
@@ -3298,8 +3322,8 @@ mod tests {
32983322
Some(Arc::clone(&nodes[0].messenger)),
32993323
nodes[0].rapid_gossip_sync(),
33003324
Arc::clone(&nodes[0].peer_manager),
3301-
Some(nodes[0].liquidity_manager.get_lm_async()),
3302-
Some(nodes[0].sweeper.sweeper_async()),
3325+
Some(lm_async),
3326+
Some(sweeper_async),
33033327
Arc::clone(&nodes[0].logger),
33043328
Some(Arc::clone(&nodes[0].scorer)),
33053329
move |dur: Duration| {
@@ -3505,6 +3529,17 @@ mod tests {
35053529

35063530
let (exit_sender, exit_receiver) = tokio::sync::watch::channel(());
35073531

3532+
// Yes, you can unsafe { turn off the borrow checker }
3533+
let lm_async: &'static LiquidityManager<_, _, _, _, _, _> = unsafe {
3534+
&*(nodes[0].liquidity_manager.get_lm_async()
3535+
as *const LiquidityManager<_, _, _, _, _, _>)
3536+
as &'static LiquidityManager<_, _, _, _, _, _>
3537+
};
3538+
let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
3539+
&*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
3540+
as &'static OutputSweeper<_, _, _, _, _, _, _>
3541+
};
3542+
35083543
let bp_future = super::process_events_async(
35093544
kv_store,
35103545
event_handler,
@@ -3513,8 +3548,8 @@ mod tests {
35133548
Some(Arc::clone(&nodes[0].messenger)),
35143549
nodes[0].no_gossip_sync(),
35153550
Arc::clone(&nodes[0].peer_manager),
3516-
Some(nodes[0].liquidity_manager.get_lm_async()),
3517-
Some(nodes[0].sweeper.sweeper_async()),
3551+
Some(lm_async),
3552+
Some(sweeper_async),
35183553
Arc::clone(&nodes[0].logger),
35193554
Some(Arc::clone(&nodes[0].scorer)),
35203555
move |dur: Duration| {

‎lightning-liquidity/src/manager.rs‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -195,15 +195,13 @@ pub trait ALiquidityManagerSync {
195195
#[cfg(any(test, feature = "_test_utils"))]
196196
fn get_lm_async(
197197
&self,
198-
) -> Arc<
199-
LiquidityManager<
200-
Self::ES,
201-
Self::NS,
202-
Self::CM,
203-
Self::C,
204-
Arc<KVStoreSyncWrapper<Self::KS>>,
205-
Self::TP,
206-
>,
198+
) -> &LiquidityManager<
199+
Self::ES,
200+
Self::NS,
201+
Self::CM,
202+
Self::C,
203+
Arc<KVStoreSyncWrapper<Self::KS>>,
204+
Self::TP,
207205
>;
208206
/// Returns a reference to the actual [`LiquidityManager`] object.
209207
fn get_lm(
@@ -243,17 +241,15 @@ where
243241
#[cfg(any(test, feature = "_test_utils"))]
244242
fn get_lm_async(
245243
&self,
246-
) -> Arc<
247-
LiquidityManager<
248-
Self::ES,
249-
Self::NS,
250-
Self::CM,
251-
Self::C,
252-
Arc<KVStoreSyncWrapper<Self::KS>>,
253-
Self::TP,
254-
>,
244+
) -> &LiquidityManager<
245+
Self::ES,
246+
Self::NS,
247+
Self::CM,
248+
Self::C,
249+
Arc<KVStoreSyncWrapper<Self::KS>>,
250+
Self::TP,
255251
> {
256-
Arc::clone(&self.inner)
252+
&self.inner
257253
}
258254
fn get_lm(&self) -> &LiquidityManagerSync<ES, NS, CM, C, KS, TP> {
259255
self
@@ -1040,7 +1036,7 @@ pub struct LiquidityManagerSync<
10401036
KS::Target: KVStoreSync,
10411037
TP::Target: TimeProvider,
10421038
{
1043-
inner: Arc<LiquidityManager<ES, NS, CM, C, Arc<KVStoreSyncWrapper<KS>>, TP>>,
1039+
inner: LiquidityManager<ES, NS, CM, C, Arc<KVStoreSyncWrapper<KS>>, TP>,
10441040
}
10451041

10461042
#[cfg(feature = "time")]
@@ -1089,7 +1085,7 @@ where
10891085
unreachable!("LiquidityManager::new should not be pending in a sync context");
10901086
},
10911087
}?;
1092-
Ok(Self { inner: Arc::new(inner) })
1088+
Ok(Self { inner })
10931089
}
10941090
}
10951091

@@ -1140,7 +1136,7 @@ where
11401136
unreachable!("LiquidityManager::new should not be pending in a sync context");
11411137
},
11421138
}?;
1143-
Ok(Self { inner: Arc::new(inner) })
1139+
Ok(Self { inner })
11441140
}
11451141

11461142
/// Returns a reference to the LSPS0 client-side handler.

‎lightning/src/util/sweep.rs‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -981,16 +981,14 @@ where
981981
L::Target: Logger,
982982
O::Target: OutputSpender,
983983
{
984-
sweeper: Arc<
985-
OutputSweeper<
986-
B,
987-
Arc<ChangeDestinationSourceSyncWrapper<D>>,
988-
E,
989-
F,
990-
Arc<KVStoreSyncWrapper<K>>,
991-
L,
992-
O,
993-
>,
984+
sweeper: OutputSweeper<
985+
B,
986+
Arc<ChangeDestinationSourceSyncWrapper<D>>,
987+
E,
988+
F,
989+
Arc<KVStoreSyncWrapper<K>>,
990+
L,
991+
O,
994992
>,
995993
}
996994

@@ -1025,7 +1023,7 @@ where
10251023
kv_store,
10261024
logger,
10271025
);
1028-
Self { sweeper: Arc::new(sweeper) }
1026+
Self { sweeper }
10291027
}
10301028

10311029
/// Regenerates and broadcasts the spending transaction for any outputs that are pending. Wraps
@@ -1074,18 +1072,16 @@ where
10741072
#[cfg(any(test, feature = "_test_utils"))]
10751073
pub fn sweeper_async(
10761074
&self,
1077-
) -> Arc<
1078-
OutputSweeper<
1079-
B,
1080-
Arc<ChangeDestinationSourceSyncWrapper<D>>,
1081-
E,
1082-
F,
1083-
Arc<KVStoreSyncWrapper<K>>,
1084-
L,
1085-
O,
1086-
>,
1075+
) -> &OutputSweeper<
1076+
B,
1077+
Arc<ChangeDestinationSourceSyncWrapper<D>>,
1078+
E,
1079+
F,
1080+
Arc<KVStoreSyncWrapper<K>>,
1081+
L,
1082+
O,
10871083
> {
1088-
Arc::clone(&self.sweeper)
1084+
&self.sweeper
10891085
}
10901086
}
10911087

0 commit comments

Comments
 (0)