Convert sweeper to use an async ChangeDestinationSource and provide

synchronous wrappers for usage in a sync context.

Author: joostjager

lightning-background-processor/src/lib.rs

@@ -36,11 +36,18 @@ 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, OutputSpender};
+#[cfg(feature = "futures")]
+use lightning::sign::ChangeDestinationSource;
+#[cfg(feature = "std")]
+use lightning::sign::ChangeDestinationSourceSync;
+use lightning::sign::OutputSpender;
 use lightning::util::logger::Logger;
 use lightning::util::persist::{KVStore, Persister};
+#[cfg(feature = "futures")]
 use lightning::util::sweep::OutputSweeper;
 #[cfg(feature = "std")]
+use lightning::util::sweep::OutputSweeperSync;
+#[cfg(feature = "std")]
 use lightning::util::wakers::Sleeper;
 use lightning_rapid_gossip_sync::RapidGossipSync;
 
@@ -866,7 +873,7 @@ where
 		gossip_sync,
 		{
 			if let Some(ref sweeper) = sweeper {
-				let _ = sweeper.regenerate_and_broadcast_spend_if_necessary();
+				let _ = sweeper.regenerate_and_broadcast_spend_if_necessary().await;
 			}
 		},
 		logger,
@@ -992,7 +999,7 @@ impl BackgroundProcessor {
 		D: 'static + Deref,
 		O: 'static + Deref,
 		K: 'static + Deref,
-		OS: 'static + Deref<Target = OutputSweeper<T, D, F, CF, K, L, O>> + Send + Sync,
+		OS: 'static + Deref<Target = OutputSweeperSync<T, D, F, CF, K, L, O>> + Send + Sync,
 	>(
 		persister: PS, event_handler: EH, chain_monitor: M, channel_manager: CM,
 		onion_messenger: Option<OM>, gossip_sync: GossipSync<PGS, RGS, G, UL, L>, peer_manager: PM,
@@ -1010,8 +1017,8 @@ impl BackgroundProcessor {
 		OM::Target: AOnionMessenger,
 		PM::Target: APeerManager,
 		LM::Target: ALiquidityManager,
+		D::Target: ChangeDestinationSourceSync,
 		O::Target: 'static + OutputSpender,
-		D::Target: 'static + ChangeDestinationSource,
 		K::Target: 'static + KVStore,
 	{
 		let stop_thread = Arc::new(AtomicBool::new(false));
@@ -1175,7 +1182,7 @@ mod tests {
 	use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
 	use lightning::routing::router::{CandidateRouteHop, DefaultRouter, Path, RouteHop};
 	use lightning::routing::scoring::{ChannelUsage, LockableScore, ScoreLookUp, ScoreUpdate};
-	use lightning::sign::{ChangeDestinationSource, InMemorySigner, KeysManager};
+	use lightning::sign::{ChangeDestinationSourceSync, InMemorySigner, KeysManager};
 	use lightning::types::features::{ChannelFeatures, NodeFeatures};
 	use lightning::types::payment::PaymentHash;
 	use lightning::util::config::UserConfig;
@@ -1187,7 +1194,7 @@ mod tests {
 		SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
 	};
 	use lightning::util::ser::Writeable;
-	use lightning::util::sweep::{OutputSpendStatus, OutputSweeper, PRUNE_DELAY_BLOCKS};
+	use lightning::util::sweep::{OutputSpendStatus, OutputSweeperSync, PRUNE_DELAY_BLOCKS};
 	use lightning::util::test_utils;
 	use lightning::{get_event, get_event_msg};
 	use lightning_liquidity::LiquidityManager;
@@ -1313,7 +1320,7 @@ mod tests {
 		best_block: BestBlock,
 		scorer: Arc<LockingWrapper<TestScorer>>,
 		sweeper: Arc<
-			OutputSweeper<
+			OutputSweeperSync<
 				Arc<test_utils::TestBroadcaster>,
 				Arc<TestWallet>,
 				Arc<test_utils::TestFeeEstimator>,
@@ -1614,7 +1621,7 @@ mod tests {
 
 	struct TestWallet {}
 
-	impl ChangeDestinationSource for TestWallet {
+	impl ChangeDestinationSourceSync for TestWallet {
 		fn get_change_destination_script(&self) -> Result<ScriptBuf, ()> {
 			Ok(ScriptBuf::new())
 		}
@@ -1692,7 +1699,7 @@ mod tests {
 				IgnoringMessageHandler {},
 			));
 			let wallet = Arc::new(TestWallet {});
-			let sweeper = Arc::new(OutputSweeper::new(
+			let sweeper = Arc::new(OutputSweeperSync::new(
 				best_block,
 				Arc::clone(&tx_broadcaster),
 				Arc::clone(&fee_estimator),
@@ -2109,7 +2116,7 @@ mod tests {
 			nodes[0].rapid_gossip_sync(),
 			nodes[0].peer_manager.clone(),
 			Some(Arc::clone(&nodes[0].liquidity_manager)),
-			Some(nodes[0].sweeper.clone()),
+			Some(nodes[0].sweeper.sweeper_async()),
 			nodes[0].logger.clone(),
 			Some(nodes[0].scorer.clone()),
 			move |dur: Duration| {
@@ -2617,7 +2624,7 @@ mod tests {
 			nodes[0].rapid_gossip_sync(),
 			nodes[0].peer_manager.clone(),
 			Some(Arc::clone(&nodes[0].liquidity_manager)),
-			Some(nodes[0].sweeper.clone()),
+			Some(nodes[0].sweeper.sweeper_async()),
 			nodes[0].logger.clone(),
 			Some(nodes[0].scorer.clone()),
 			move |dur: Duration| {
@@ -2833,7 +2840,7 @@ mod tests {
 			nodes[0].no_gossip_sync(),
 			nodes[0].peer_manager.clone(),
 			Some(Arc::clone(&nodes[0].liquidity_manager)),
-			Some(nodes[0].sweeper.clone()),
+			Some(nodes[0].sweeper.sweeper_async()),
 			nodes[0].logger.clone(),
 			Some(nodes[0].scorer.clone()),
 			move |dur: Duration| {

lightning/src/lib.rs

@@ -30,7 +30,6 @@
 //! * `grind_signatures`
 
 #![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), deny(missing_docs))]
-#![cfg_attr(not(any(test, feature = "_test_utils")), forbid(unsafe_code))]
 
 #![deny(rustdoc::broken_intra_doc_links)]
 #![deny(rustdoc::private_intra_doc_links)]

lightning/src/sign/mod.rs

@@ -57,6 +57,7 @@ use crate::ln::msgs::{UnsignedChannelAnnouncement, UnsignedGossipMessage};
 use crate::ln::script::ShutdownScript;
 use crate::offers::invoice::UnsignedBolt12Invoice;
 use crate::types::payment::PaymentPreimage;
+use crate::util::async_poll::AsyncResult;
 use crate::util::ser::{ReadableArgs, Writeable};
 use crate::util::transaction_utils;
 
@@ -67,6 +68,7 @@ use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::sign::taproot::TaprootChannelSigner;
 use crate::util::atomic_counter::AtomicCounter;
 use core::convert::TryInto;
+use core::ops::Deref;
 use core::sync::atomic::{AtomicUsize, Ordering};
 #[cfg(taproot)]
 use musig2::types::{PartialSignature, PublicNonce};
@@ -981,11 +983,47 @@ pub trait ChangeDestinationSource {
 	/// Returns a script pubkey which can be used as a change destination for
 	/// [`OutputSpender::spend_spendable_outputs`].
 	///
+	/// This method should return a different value each time it is called, to avoid linking
+	/// on-chain funds controlled to the same user.
+	fn get_change_destination_script<'a>(&self) -> AsyncResult<'a, ScriptBuf>;
+}
+
+/// A synchronous helper trait that describes an on-chain wallet capable of returning a (change) destination script.
+pub trait ChangeDestinationSourceSync {
 	/// This method should return a different value each time it is called, to avoid linking
 	/// on-chain funds controlled to the same user.
 	fn get_change_destination_script(&self) -> Result<ScriptBuf, ()>;
 }
 
+/// A wrapper around [`ChangeDestinationSource`] to allow for async calls.
+#[cfg(any(test, feature = "_test_utils"))]
+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;
+
+impl<T: Deref> ChangeDestinationSourceSyncWrapper<T>
+where
+	T::Target: ChangeDestinationSourceSync,
+{
+	/// Creates a new [`ChangeDestinationSourceSyncWrapper`].
+	pub fn new(source: T) -> Self {
+		Self(source)
+	}
+}
+impl<T: Deref> ChangeDestinationSource for ChangeDestinationSourceSyncWrapper<T>
+where
+	T::Target: ChangeDestinationSourceSync,
+{
+	fn get_change_destination_script<'a>(&self) -> AsyncResult<'a, ScriptBuf> {
+		let script = self.0.get_change_destination_script();
+		Box::pin(async move { script })
+	}
+}
+
 mod sealed {
 	use bitcoin::secp256k1::{Scalar, SecretKey};
 

lightning/src/util/async_poll.rs

@@ -13,7 +13,7 @@ use crate::prelude::*;
 use core::future::Future;
 use core::marker::Unpin;
 use core::pin::Pin;
-use core::task::{Context, Poll};
+use core::task::{Context, Poll, RawWaker, RawWakerVTable, Waker};
 
 pub(crate) enum ResultFuture<F: Future<Output = Result<(), E>>, E: Copy + Unpin> {
 	Pending(F),
@@ -74,3 +74,27 @@ impl<F: Future<Output = Result<(), E>> + Unpin, E: Copy + Unpin> Future
 		}
 	}
 }
+
+// If we want to poll a future without an async context to figure out if it has completed or
+// not without awaiting, we need a Waker, which needs a vtable...we fill it with dummy values
+// but sadly there's a good bit of boilerplate here.
+//
+// Waker::noop() would be preferable, but requires an MSRV of 1.85.
+fn dummy_waker_clone(_: *const ()) -> RawWaker {
+	RawWaker::new(core::ptr::null(), &DUMMY_WAKER_VTABLE)
+}
+fn dummy_waker_action(_: *const ()) {}
+
+const DUMMY_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(
+	dummy_waker_clone,
+	dummy_waker_action,
+	dummy_waker_action,
+	dummy_waker_action,
+);
+
+pub(crate) fn dummy_waker() -> Waker {
+	unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &DUMMY_WAKER_VTABLE)) }
+}
+
+/// A type alias for a future that returns a result of type T.
+pub type AsyncResult<'a, T> = Pin<Box<dyn Future<Output = Result<T, ()>> + 'a + Send>>;