Skip to content

Commit 48ddd56

Browse files
committed
Move the pub wait methods from ChannelManager to Future
Rather than having three ways to await a `ChannelManager` being persistable, this moves to just exposing the awaitable `Future` and having sleep functions on that.
1 parent 50b3079 commit 48ddd56

File tree

3 files changed

+39
-58
lines changed

3 files changed

+39
-58
lines changed

lightning-background-processor/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ impl BackgroundProcessor {
597597
define_run_body!(persister, chain_monitor, chain_monitor.process_pending_events(&event_handler),
598598
channel_manager, channel_manager.process_pending_events(&event_handler),
599599
gossip_sync, peer_manager, logger, scorer, stop_thread.load(Ordering::Acquire),
600-
channel_manager.await_persistable_update_timeout(Duration::from_millis(100)),
600+
channel_manager.get_persistable_update_future().wait_timeout(Duration::from_millis(100)),
601601
|_| Instant::now(), |time: &Instant, dur| time.elapsed().as_secs() > dur)
602602
});
603603
Self { stop_thread: stop_thread_clone, thread_handle: Some(handle) }

lightning/src/ln/channelmanager.rs

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6086,34 +6086,11 @@ where
60866086
}
60876087
}
60886088

6089-
/// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool
6090-
/// indicating whether persistence is necessary. Only one listener on
6091-
/// [`await_persistable_update`], [`await_persistable_update_timeout`], or a future returned by
6092-
/// [`get_persistable_update_future`] is guaranteed to be woken up.
6089+
/// Gets a [`Future`] that completes when this [`ChannelManager`] needs to be persisted.
60936090
///
6094-
/// Note that this method is not available with the `no-std` feature.
6091+
/// Note that callbacks registered on the [`Future`] MUST NOT call back into this
6092+
/// [`ChannelManager`] and should instead register actions to be taken later.
60956093
///
6096-
/// [`await_persistable_update`]: Self::await_persistable_update
6097-
/// [`await_persistable_update_timeout`]: Self::await_persistable_update_timeout
6098-
/// [`get_persistable_update_future`]: Self::get_persistable_update_future
6099-
#[cfg(any(test, feature = "std"))]
6100-
pub fn await_persistable_update_timeout(&self, max_wait: Duration) -> bool {
6101-
self.persistence_notifier.wait_timeout(max_wait)
6102-
}
6103-
6104-
/// Blocks until ChannelManager needs to be persisted. Only one listener on
6105-
/// [`await_persistable_update`], `await_persistable_update_timeout`, or a future returned by
6106-
/// [`get_persistable_update_future`] is guaranteed to be woken up.
6107-
///
6108-
/// [`await_persistable_update`]: Self::await_persistable_update
6109-
/// [`get_persistable_update_future`]: Self::get_persistable_update_future
6110-
pub fn await_persistable_update(&self) {
6111-
self.persistence_notifier.wait()
6112-
}
6113-
6114-
/// Gets a [`Future`] that completes when a persistable update is available. Note that
6115-
/// callbacks registered on the [`Future`] MUST NOT call back into this [`ChannelManager`] and
6116-
/// should instead register actions to be taken later.
61176094
pub fn get_persistable_update_future(&self) -> Future {
61186095
self.persistence_notifier.get_future()
61196096
}
@@ -7870,9 +7847,9 @@ mod tests {
78707847

78717848
// All nodes start with a persistable update pending as `create_network` connects each node
78727849
// with all other nodes to make most tests simpler.
7873-
assert!(nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
7874-
assert!(nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
7875-
assert!(nodes[2].node.await_persistable_update_timeout(Duration::from_millis(1)));
7850+
assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7851+
assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7852+
assert!(nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
78767853

78777854
let mut chan = create_announced_chan_between_nodes(&nodes, 0, 1);
78787855

@@ -7886,28 +7863,28 @@ mod tests {
78867863
&nodes[0].node.get_our_node_id()).pop().unwrap();
78877864

78887865
// The first two nodes (which opened a channel) should now require fresh persistence
7889-
assert!(nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
7890-
assert!(nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
7866+
assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7867+
assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
78917868
// ... but the last node should not.
7892-
assert!(!nodes[2].node.await_persistable_update_timeout(Duration::from_millis(1)));
7869+
assert!(!nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
78937870
// After persisting the first two nodes they should no longer need fresh persistence.
7894-
assert!(!nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
7895-
assert!(!nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
7871+
assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7872+
assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
78967873

78977874
// Node 3, unrelated to the only channel, shouldn't care if it receives a channel_update
78987875
// about the channel.
78997876
nodes[2].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &chan.0);
79007877
nodes[2].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &chan.1);
7901-
assert!(!nodes[2].node.await_persistable_update_timeout(Duration::from_millis(1)));
7878+
assert!(!nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
79027879

79037880
// The nodes which are a party to the channel should also ignore messages from unrelated
79047881
// parties.
79057882
nodes[0].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.0);
79067883
nodes[0].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.1);
79077884
nodes[1].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.0);
79087885
nodes[1].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.1);
7909-
assert!(!nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
7910-
assert!(!nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
7886+
assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7887+
assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
79117888

79127889
// At this point the channel info given by peers should still be the same.
79137890
assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
@@ -7924,17 +7901,17 @@ mod tests {
79247901
// persisted and that its channel info remains the same.
79257902
nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &as_update);
79267903
nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &bs_update);
7927-
assert!(!nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
7928-
assert!(!nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
7904+
assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7905+
assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
79297906
assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
79307907
assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info);
79317908

79327909
// Finally, deliver the other peers' message, ensuring each node needs to be persisted and
79337910
// the channel info has updated.
79347911
nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_update);
79357912
nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_update);
7936-
assert!(nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
7937-
assert!(nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
7913+
assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7914+
assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
79387915
assert_ne!(nodes[0].node.list_channels()[0], node_a_chan_info);
79397916
assert_ne!(nodes[1].node.list_channels()[0], node_b_chan_info);
79407917
}

lightning/src/util/wakers.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,6 @@ impl Notifier {
3939
}
4040
}
4141

42-
pub(crate) fn wait(&self) {
43-
Sleeper::from_single_future(self.get_future()).wait();
44-
}
45-
46-
#[cfg(any(test, feature = "std"))]
47-
pub(crate) fn wait_timeout(&self, max_wait: Duration) -> bool {
48-
Sleeper::from_single_future(self.get_future()).wait_timeout(max_wait)
49-
}
50-
5142
/// Wake waiters, tracking that wake needs to occur even if there are currently no waiters.
5243
pub(crate) fn notify(&self) {
5344
let mut lock = self.notify_pending.lock().unwrap();
@@ -167,6 +158,19 @@ impl Future {
167158
pub fn register_callback_fn<F: 'static + FutureCallback>(&self, callback: F) {
168159
self.register_callback(Box::new(callback));
169160
}
161+
162+
/// Waits until this [`Future`] completes.
163+
pub fn wait(self) {
164+
Sleeper::from_single_future(self).wait();
165+
}
166+
167+
/// Waits until this [`Future`] completes or the given amount of time has elapsed.
168+
///
169+
/// Returns true if the [`Future`] completed, false if the time elapsed.
170+
#[cfg(any(test, feature = "std"))]
171+
pub fn wait_timeout(self, max_wait: Duration) -> bool {
172+
Sleeper::from_single_future(self).wait_timeout(max_wait)
173+
}
170174
}
171175

172176
use core::task::Waker;
@@ -377,12 +381,12 @@ mod tests {
377381
});
378382

379383
// Check that we can block indefinitely until updates are available.
380-
let _ = persistence_notifier.wait();
384+
let _ = persistence_notifier.get_future().wait();
381385

382386
// Check that the Notifier will return after the given duration if updates are
383387
// available.
384388
loop {
385-
if persistence_notifier.wait_timeout(Duration::from_millis(100)) {
389+
if persistence_notifier.get_future().wait_timeout(Duration::from_millis(100)) {
386390
break
387391
}
388392
}
@@ -392,7 +396,7 @@ mod tests {
392396
// Check that the Notifier will return after the given duration even if no updates
393397
// are available.
394398
loop {
395-
if !persistence_notifier.wait_timeout(Duration::from_millis(100)) {
399+
if !persistence_notifier.get_future().wait_timeout(Duration::from_millis(100)) {
396400
break
397401
}
398402
}
@@ -490,8 +494,8 @@ mod tests {
490494

491495
// If we get a future and don't touch it we're definitely still notify-required.
492496
notifier.get_future();
493-
assert!(notifier.wait_timeout(Duration::from_millis(1)));
494-
assert!(!notifier.wait_timeout(Duration::from_millis(1)));
497+
assert!(notifier.get_future().wait_timeout(Duration::from_millis(1)));
498+
assert!(!notifier.get_future().wait_timeout(Duration::from_millis(1)));
495499

496500
// Even if we poll'd once but didn't observe a `Ready`, we should be notify-required.
497501
let mut future = notifier.get_future();
@@ -500,7 +504,7 @@ mod tests {
500504

501505
notifier.notify();
502506
assert!(woken.load(Ordering::SeqCst));
503-
assert!(notifier.wait_timeout(Duration::from_millis(1)));
507+
assert!(notifier.get_future().wait_timeout(Duration::from_millis(1)));
504508

505509
// However, once we do poll `Ready` it should wipe the notify-required flag.
506510
let mut future = notifier.get_future();
@@ -510,7 +514,7 @@ mod tests {
510514
notifier.notify();
511515
assert!(woken.load(Ordering::SeqCst));
512516
assert_eq!(Pin::new(&mut future).poll(&mut Context::from_waker(&waker)), Poll::Ready(()));
513-
assert!(!notifier.wait_timeout(Duration::from_millis(1)));
517+
assert!(!notifier.get_future().wait_timeout(Duration::from_millis(1)));
514518
}
515519

516520
#[test]

0 commit comments

Comments
 (0)