Drop the dummy no-std Condvar which never sleeps

In `no-std`, we exposed `wait` functions which rely on a dummy
`Condvar` which never actually sleeps. This is somwhat nonsensical,
not to mention confusing to users. Instead, we simply remove the
`wait` methods in `no-std` builds.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -7822,6 +7822,7 @@ mod tests {
 	use bitcoin::hashes::Hash;
 	use bitcoin::hashes::sha256::Hash as Sha256;
 	use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+	#[cfg(feature = "std")]
 	use core::time::Duration;
 	use core::sync::atomic::Ordering;
 	use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
@@ -7847,9 +7848,9 @@ mod tests {
 
 		// All nodes start with a persistable update pending as `create_network` connects each node
 		// with all other nodes to make most tests simpler.
-		assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(nodes[0].node.get_persistable_update_future().poll_is_complete());
+		assert!(nodes[1].node.get_persistable_update_future().poll_is_complete());
+		assert!(nodes[2].node.get_persistable_update_future().poll_is_complete());
 
 		let mut chan = create_announced_chan_between_nodes(&nodes, 0, 1);
 
@@ -7863,28 +7864,28 @@ mod tests {
 			&nodes[0].node.get_our_node_id()).pop().unwrap();
 
 		// The first two nodes (which opened a channel) should now require fresh persistence
-		assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(nodes[0].node.get_persistable_update_future().poll_is_complete());
+		assert!(nodes[1].node.get_persistable_update_future().poll_is_complete());
 		// ... but the last node should not.
-		assert!(!nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(!nodes[2].node.get_persistable_update_future().poll_is_complete());
 		// After persisting the first two nodes they should no longer need fresh persistence.
-		assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(!nodes[0].node.get_persistable_update_future().poll_is_complete());
+		assert!(!nodes[1].node.get_persistable_update_future().poll_is_complete());
 
 		// Node 3, unrelated to the only channel, shouldn't care if it receives a channel_update
 		// about the channel.
 		nodes[2].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &chan.0);
 		nodes[2].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &chan.1);
-		assert!(!nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(!nodes[2].node.get_persistable_update_future().poll_is_complete());
 
 		// The nodes which are a party to the channel should also ignore messages from unrelated
 		// parties.
 		nodes[0].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.0);
 		nodes[0].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.1);
 		nodes[1].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.0);
 		nodes[1].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.1);
-		assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(!nodes[0].node.get_persistable_update_future().poll_is_complete());
+		assert!(!nodes[1].node.get_persistable_update_future().poll_is_complete());
 
 		// At this point the channel info given by peers should still be the same.
 		assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
@@ -7901,17 +7902,17 @@ mod tests {
 		// persisted and that its channel info remains the same.
 		nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &as_update);
 		nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &bs_update);
-		assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(!nodes[0].node.get_persistable_update_future().poll_is_complete());
+		assert!(!nodes[1].node.get_persistable_update_future().poll_is_complete());
 		assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
 		assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info);
 
 		// Finally, deliver the other peers' message, ensuring each node needs to be persisted and
 		// the channel info has updated.
 		nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_update);
 		nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_update);
-		assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
-		assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
+		assert!(nodes[0].node.get_persistable_update_future().poll_is_complete());
+		assert!(nodes[1].node.get_persistable_update_future().poll_is_complete());
 		assert_ne!(nodes[0].node.list_channels()[0], node_a_chan_info);
 		assert_ne!(nodes[1].node.list_channels()[0], node_b_chan_info);
 	}

lightning/src/sync/nostd_sync.rs

@@ -1,30 +1,10 @@
 pub use ::alloc::sync::Arc;
 use core::ops::{Deref, DerefMut};
-use core::time::Duration;
 use core::cell::{RefCell, Ref, RefMut};
 use super::{LockTestExt, LockHeldState};
 
 pub type LockResult<Guard> = Result<Guard, ()>;
 
-pub struct Condvar {}
-
-impl Condvar {
-	pub fn new() -> Condvar {
-		Condvar { }
-	}
-
-	pub fn wait<'a, T>(&'a self, guard: MutexGuard<'a, T>) -> LockResult<MutexGuard<'a, T>> {
-		Ok(guard)
-	}
-
-	#[allow(unused)]
-	pub fn wait_timeout<'a, T>(&'a self, guard: MutexGuard<'a, T>, _dur: Duration) -> LockResult<(MutexGuard<'a, T>, ())> {
-		Ok((guard, ()))
-	}
-
-	pub fn notify_all(&self) {}
-}
-
 pub struct Mutex<T: ?Sized> {
 	inner: RefCell<T>
 }

lightning/src/util/wakers.rs

@@ -15,11 +15,13 @@
 
 use alloc::sync::Arc;
 use core::mem;
-use crate::sync::{Condvar, Mutex, MutexGuard};
+use crate::sync::Mutex;
 
 use crate::prelude::*;
 
-#[cfg(any(test, feature = "std"))]
+#[cfg(feature = "std")]
+use crate::sync::{Condvar, MutexGuard};
+#[cfg(feature = "std")]
 use std::time::{Duration, Instant};
 
 use core::future::Future as StdFuture;
@@ -160,17 +162,27 @@ impl Future {
 	}
 
 	/// Waits until this [`Future`] completes.
+	#[cfg(feature = "std")]
 	pub fn wait(self) {
 		Sleeper::from_single_future(self).wait();
 	}
 
 	/// Waits until this [`Future`] completes or the given amount of time has elapsed.
 	///
 	/// Returns true if the [`Future`] completed, false if the time elapsed.
-	#[cfg(any(test, feature = "std"))]
+	#[cfg(feature = "std")]
 	pub fn wait_timeout(self, max_wait: Duration) -> bool {
 		Sleeper::from_single_future(self).wait_timeout(max_wait)
 	}
+
+	#[cfg(test)]
+	pub fn poll_is_complete(&self) -> bool {
+		let mut state = self.state.lock().unwrap();
+		if state.complete {
+			state.callbacks_made = true;
+			true
+		} else { false }
+	}
 }
 
 use core::task::Waker;
@@ -198,10 +210,12 @@ impl<'a> StdFuture for Future {
 
 /// A struct which can be used to select across many [`Future`]s at once without relying on a full
 /// async context.
+#[cfg(feature = "std")]
 pub struct Sleeper {
 	notifiers: Vec<Arc<Mutex<FutureState>>>,
 }
 
+#[cfg(feature = "std")]
 impl Sleeper {
 	/// Constructs a new sleeper from one future, allowing blocking on it.
 	pub fn from_single_future(future: Future) -> Self {
@@ -244,32 +258,23 @@ impl Sleeper {
 	/// Wait until one of the [`Future`]s registered with this [`Sleeper`] has completed.
 	pub fn wait(&self) {
 		let (cv, notified_fut_mtx) = self.setup_wait();
-		let notified_fut = loop {
-			let mut notified_fut_lck = cv.wait(notified_fut_mtx.lock().unwrap()).unwrap();
-			if let Some(notified_fut) = notified_fut_lck.take() {
-				break notified_fut;
-			}
-		};
+		let notified_fut = cv.wait_while(notified_fut_mtx.lock().unwrap(), |fut_opt| fut_opt.is_none())
+			.unwrap().take().expect("CV wait shouldn't have returned until the notifying future was set");
 		notified_fut.lock().unwrap().callbacks_made = true;
 	}
 
 	/// Wait until one of the [`Future`]s registered with this [`Sleeper`] has completed or the
 	/// given amount of time has elapsed. Returns true if a [`Future`] completed, false if the time
 	/// elapsed.
-	#[cfg(any(test, feature = "std"))]
 	pub fn wait_timeout(&self, max_wait: Duration) -> bool {
-		let start_time = Instant::now();
 		let (cv, notified_fut_mtx) = self.setup_wait();
-		let notified_fut = loop {
-			let wait_duration = max_wait.saturating_sub(start_time.elapsed());
-			if wait_duration == Duration::from_secs(0) { return false; }
-			match cv.wait_timeout(notified_fut_mtx.lock().unwrap(), wait_duration) {
-				Ok((mut notified_fut, _)) if notified_fut.is_some() =>
-					break notified_fut.take().unwrap(),
-				Ok((notified_fut_lck, _)) => continue,
+		let notified_fut =
+			match cv.wait_timeout_while(notified_fut_mtx.lock().unwrap(), max_wait, |fut_opt| fut_opt.is_none()) {
+				Ok((_, e)) if e.timed_out() => return false,
+				Ok((mut notified_fut, _)) =>
+					notified_fut.take().expect("CV wait shouldn't have returned until the notifying future was set"),
 				Err(_) => panic!("Previous panic while a lock was held led to a lock panic"),
 			};
-		};
 		notified_fut.lock().unwrap().callbacks_made = true;
 		true
 	}
@@ -486,6 +491,7 @@ mod tests {
 	}
 
 	#[test]
+	#[cfg(feature = "std")]
 	fn test_dropped_future_doesnt_count() {
 		// Tests that if a Future gets drop'd before it is poll()ed `Ready` it doesn't count as
 		// having been woken, leaving the notify-required flag set.
@@ -577,6 +583,7 @@ mod tests {
 	}
 
 	#[test]
+	#[cfg(feature = "std")]
 	fn test_multi_future_sleep() {
 		// Tests the `Sleeper` with multiple futures.
 		let notifier_a = Notifier::new();