Reduce Boxing using impl Trait in trait methods post-MSRV-bump

.github/workflows/build.yml

@@ -62,52 +62,35 @@ jobs:
       - name: Set RUSTFLAGS to deny warnings
         if: "matrix.toolchain == '1.75.0'"
         run: echo "RUSTFLAGS=-D warnings" >> "$GITHUB_ENV"
-      - name: Run CI script
-        shell: bash # Default on Winblows is powershell
-        run: CI_ENV=1 CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tests.sh
-
-  build-tx-sync:
-    strategy:
-      fail-fast: false
-      matrix:
-        platform: [ ubuntu-latest, macos-latest ]
-        toolchain: [ stable, beta, 1.75.0 ]
-    runs-on: ${{ matrix.platform }}
-    steps:
-      - name: Checkout source code
-        uses: actions/checkout@v4
-      - name: Install Rust ${{ matrix.toolchain }} toolchain
-        run: |
-          curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain ${{ matrix.toolchain }}
-      - name: Set RUSTFLAGS to deny warnings
-        if: "matrix.toolchain == '1.75.0'"
-        run: echo "RUSTFLAGS=-D warnings" >> "$GITHUB_ENV"
       - name: Enable caching for bitcoind
+        if: matrix.platform != 'windows-latest'
         id: cache-bitcoind
         uses: actions/cache@v4
         with:
           path: bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
           key: bitcoind-${{ runner.os }}-${{ runner.arch }}
       - name: Enable caching for electrs
+        if: matrix.platform != 'windows-latest'
         id: cache-electrs
         uses: actions/cache@v4
         with:
           path: bin/electrs-${{ runner.os }}-${{ runner.arch }}
           key: electrs-${{ runner.os }}-${{ runner.arch }}
       - name: Download bitcoind/electrs
-        if: "steps.cache-bitcoind.outputs.cache-hit != 'true' || steps.cache-electrs.outputs.cache-hit != 'true'"
+        if: "matrix.platform != 'windows-latest' && (steps.cache-bitcoind.outputs.cache-hit != 'true' || steps.cache-electrs.outputs.cache-hit != 'true')"
         run: |
             source ./contrib/download_bitcoind_electrs.sh
             mkdir bin
             mv "$BITCOIND_EXE" bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
             mv "$ELECTRS_EXE" bin/electrs-${{ runner.os }}-${{ runner.arch }}
       - name: Set bitcoind/electrs environment variables
+        if: matrix.platform != 'windows-latest'
         run: |
             echo "BITCOIND_EXE=$( pwd )/bin/bitcoind-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
             echo "ELECTRS_EXE=$( pwd )/bin/electrs-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
       - name: Run CI script
         shell: bash # Default on Winblows is powershell
-        run: CI_ENV=1 CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tx-sync-tests.sh
+        run: CI_ENV=1 CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tests.sh
 
   coverage:
     needs: fuzz

Cargo.toml

@@ -16,11 +16,11 @@ members = [
     "lightning-macros",
     "lightning-dns-resolver",
     "lightning-liquidity",
+    "lightning-transaction-sync",
     "possiblyrandom",
 ]
 
 exclude = [
-    "lightning-transaction-sync",
     "lightning-tests",
     "ext-functional-test-demo",
     "no-std-check",

ci/check-lint.sh

@@ -107,7 +107,8 @@ CLIPPY() {
 		-A clippy::useless_conversion \
 		-A clippy::manual_repeat_n `# to be removed once we hit MSRV 1.86` \
 		-A clippy::manual_is_multiple_of `# to be removed once we hit MSRV 1.87` \
-		-A clippy::uninlined-format-args
+		-A clippy::uninlined-format-args \
+		-A clippy::manual-async-fn # Not really sure why this is even a warning when there's a Send bound
 }
 
 CLIPPY

ci/ci-tests.sh

@@ -2,7 +2,6 @@
 #shellcheck disable=SC2002,SC2207
 set -eox pipefail
 
-# Currently unused as we don't have to pin anything for MSRV:
 RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }')
 
 # Some crates require pinning to meet our MSRV even for our downstream users,
@@ -20,6 +19,9 @@ PIN_RELEASE_DEPS # pin the release dependencies in our main workspace
 # proptest 1.9.0 requires rustc 1.82.0
 [ "$RUSTC_MINOR_VERSION" -lt 82 ] && cargo update -p proptest --precise "1.8.0" --verbose
 
+# Starting with version 1.2.0, the `idna_adapter` crate has an MSRV of rustc 1.81.0.
+[ "$RUSTC_MINOR_VERSION" -lt 81 ] && cargo update -p idna_adapter --precise "1.1.0" --verbose
+
 export RUST_BACKTRACE=1
 
 echo -e "\n\nChecking the workspace, except lightning-transaction-sync."
@@ -57,6 +59,23 @@ cargo check -p lightning-block-sync --verbose --color always --features rpc-clie
 cargo test -p lightning-block-sync --verbose --color always --features rpc-client,rest-client,tokio
 cargo check -p lightning-block-sync --verbose --color always --features rpc-client,rest-client,tokio
 
+echo -e "\n\nChecking Transaction Sync Clients with features."
+cargo check -p lightning-transaction-sync --verbose --color always --features esplora-blocking
+cargo check -p lightning-transaction-sync --verbose --color always --features esplora-async
+cargo check -p lightning-transaction-sync --verbose --color always --features esplora-async-https
+cargo check -p lightning-transaction-sync --verbose --color always --features electrum
+
+if [ -z "$CI_ENV" ] && [[ -z "$BITCOIND_EXE" || -z "$ELECTRS_EXE" ]]; then
+	echo -e "\n\nSkipping testing Transaction Sync Clients due to BITCOIND_EXE or ELECTRS_EXE being unset."
+	cargo check -p lightning-transaction-sync --tests
+else
+	echo -e "\n\nTesting Transaction Sync Clients with features."
+	cargo test -p lightning-transaction-sync --verbose --color always --features esplora-blocking
+	cargo test -p lightning-transaction-sync --verbose --color always --features esplora-async
+	cargo test -p lightning-transaction-sync --verbose --color always --features esplora-async-https
+	cargo test -p lightning-transaction-sync --verbose --color always --features electrum
+fi
+
 echo -e "\n\nChecking and testing lightning-persister with features"
 cargo test -p lightning-persister --verbose --color always --features tokio
 cargo check -p lightning-persister --verbose --color always --features tokio

lightning-background-processor/src/lib.rs

@@ -41,6 +41,8 @@ use lightning::events::ReplayEvent;
 use lightning::events::{Event, PathFailure};
 use lightning::util::ser::Writeable;
 
+#[cfg(not(c_bindings))]
+use lightning::io::Error;
 use lightning::ln::channelmanager::AChannelManager;
 use lightning::ln::msgs::OnionMessageHandler;
 use lightning::ln::peer_handler::APeerManager;
@@ -51,6 +53,8 @@ use lightning::routing::utxo::UtxoLookup;
 use lightning::sign::{
 	ChangeDestinationSource, ChangeDestinationSourceSync, EntropySource, OutputSpender,
 };
+#[cfg(not(c_bindings))]
+use lightning::util::async_poll::MaybeSend;
 use lightning::util::logger::Logger;
 use lightning::util::persist::{
 	KVStore, KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_PERSISTENCE_KEY,
@@ -83,7 +87,11 @@ use std::time::Instant;
 #[cfg(not(feature = "std"))]
 use alloc::boxed::Box;
 #[cfg(all(not(c_bindings), not(feature = "std")))]
+use alloc::string::String;
+#[cfg(all(not(c_bindings), not(feature = "std")))]
 use alloc::sync::Arc;
+#[cfg(all(not(c_bindings), not(feature = "std")))]
+use alloc::vec::Vec;
 
 /// `BackgroundProcessor` takes care of tasks that (1) need to happen periodically to keep
 /// Rust-Lightning running properly, and (2) either can or should be run in the background. Its
@@ -416,6 +424,37 @@ pub const NO_ONION_MESSENGER: Option<
 	>,
 > = None;
 
+#[cfg(not(c_bindings))]
+/// A panicking implementation of [`KVStore`] that is used in [`NO_LIQUIDITY_MANAGER`].
+pub struct DummyKVStore;
+
+#[cfg(not(c_bindings))]
+impl KVStore for DummyKVStore {
+	fn read(
+		&self, _: &str, _: &str, _: &str,
+	) -> impl core::future::Future<Output = Result<Vec<u8>, Error>> + MaybeSend + 'static {
+		async { unimplemented!() }
+	}
+
+	fn write(
+		&self, _: &str, _: &str, _: &str, _: Vec<u8>,
+	) -> impl core::future::Future<Output = Result<(), Error>> + MaybeSend + 'static {
+		async { unimplemented!() }
+	}
+
+	fn remove(
+		&self, _: &str, _: &str, _: &str, _: bool,
+	) -> impl core::future::Future<Output = Result<(), Error>> + MaybeSend + 'static {
+		async { unimplemented!() }
+	}
+
+	fn list(
+		&self, _: &str, _: &str,
+	) -> impl core::future::Future<Output = Result<Vec<String>, Error>> + MaybeSend + 'static {
+		async { unimplemented!() }
+	}
+}
+
 /// When initializing a background processor without a liquidity manager, this can be used to avoid
 /// specifying a concrete `LiquidityManager` type.
 #[cfg(not(c_bindings))]
@@ -430,8 +469,8 @@ pub const NO_LIQUIDITY_MANAGER: Option<
 				CM = &DynChannelManager,
 				Filter = dyn chain::Filter + Send + Sync,
 				C = &(dyn chain::Filter + Send + Sync),
-				KVStore = dyn lightning::util::persist::KVStore + Send + Sync,
-				K = &(dyn lightning::util::persist::KVStore + Send + Sync),
+				KVStore = DummyKVStore,
+				K = &DummyKVStore,
 				TimeProvider = dyn lightning_liquidity::utils::time::TimeProvider + Send + Sync,
 				TP = &(dyn lightning_liquidity::utils::time::TimeProvider + Send + Sync),
 				BroadcasterInterface = dyn lightning::chain::chaininterface::BroadcasterInterface

lightning-block-sync/src/gossip.rs

@@ -2,7 +2,7 @@
 //! current UTXO set. This module defines an implementation of the LDK API required to do so
 //! against a [`BlockSource`] which implements a few additional methods for accessing the UTXO set.
 
-use crate::{AsyncBlockSourceResult, BlockData, BlockSource, BlockSourceError};
+use crate::{BlockData, BlockSource, BlockSourceError, BlockSourceResult};
 
 use bitcoin::block::Block;
 use bitcoin::constants::ChainHash;
@@ -18,7 +18,7 @@ use lightning::util::native_async::FutureSpawner;
 use std::collections::VecDeque;
 use std::future::Future;
 use std::ops::Deref;
-use std::pin::Pin;
+use std::pin::{pin, Pin};
 use std::sync::{Arc, Mutex};
 use std::task::Poll;
 
@@ -35,11 +35,13 @@ pub trait UtxoSource: BlockSource + 'static {
 	/// for gossip validation.
 	fn get_block_hash_by_height<'a>(
 		&'a self, block_height: u32,
-	) -> AsyncBlockSourceResult<'a, BlockHash>;
+	) -> impl Future<Output = BlockSourceResult<BlockHash>> + Send + 'a;
 
 	/// Returns true if the given output has *not* been spent, i.e. is a member of the current UTXO
 	/// set.
-	fn is_output_unspent<'a>(&'a self, outpoint: OutPoint) -> AsyncBlockSourceResult<'a, bool>;
+	fn is_output_unspent<'a>(
+		&'a self, outpoint: OutPoint,
+	) -> impl Future<Output = BlockSourceResult<bool>> + Send + 'a;
 }
 
 #[cfg(feature = "tokio")]
@@ -55,34 +57,37 @@ impl FutureSpawner for TokioSpawner {
 /// A trivial future which joins two other futures and polls them at the same time, returning only
 /// once both complete.
 pub(crate) struct Joiner<
-	A: Future<Output = Result<(BlockHash, Option<u32>), BlockSourceError>> + Unpin,
-	B: Future<Output = Result<BlockHash, BlockSourceError>> + Unpin,
+	'a,
+	A: Future<Output = Result<(BlockHash, Option<u32>), BlockSourceError>>,
+	B: Future<Output = Result<BlockHash, BlockSourceError>>,
 > {
-	pub a: A,
-	pub b: B,
+	pub a: Pin<&'a mut A>,
+	pub b: Pin<&'a mut B>,
 	a_res: Option<(BlockHash, Option<u32>)>,
 	b_res: Option<BlockHash>,
 }
 
 impl<
-		A: Future<Output = Result<(BlockHash, Option<u32>), BlockSourceError>> + Unpin,
-		B: Future<Output = Result<BlockHash, BlockSourceError>> + Unpin,
-	> Joiner<A, B>
+		'a,
+		A: Future<Output = Result<(BlockHash, Option<u32>), BlockSourceError>>,
+		B: Future<Output = Result<BlockHash, BlockSourceError>>,
+	> Joiner<'a, A, B>
 {
-	fn new(a: A, b: B) -> Self {
+	fn new(a: Pin<&'a mut A>, b: Pin<&'a mut B>) -> Self {
 		Self { a, b, a_res: None, b_res: None }
 	}
 }
 
 impl<
-		A: Future<Output = Result<(BlockHash, Option<u32>), BlockSourceError>> + Unpin,
-		B: Future<Output = Result<BlockHash, BlockSourceError>> + Unpin,
-	> Future for Joiner<A, B>
+		'a,
+		A: Future<Output = Result<(BlockHash, Option<u32>), BlockSourceError>>,
+		B: Future<Output = Result<BlockHash, BlockSourceError>>,
+	> Future for Joiner<'a, A, B>
 {
 	type Output = Result<((BlockHash, Option<u32>), BlockHash), BlockSourceError>;
 	fn poll(mut self: Pin<&mut Self>, ctx: &mut core::task::Context<'_>) -> Poll<Self::Output> {
 		if self.a_res.is_none() {
-			match Pin::new(&mut self.a).poll(ctx) {
+			match self.a.as_mut().poll(ctx) {
 				Poll::Ready(res) => {
 					if let Ok(ok) = res {
 						self.a_res = Some(ok);
@@ -94,7 +99,7 @@ impl<
 			}
 		}
 		if self.b_res.is_none() {
-			match Pin::new(&mut self.b).poll(ctx) {
+			match self.b.as_mut().poll(ctx) {
 				Poll::Ready(res) => {
 					if let Ok(ok) = res {
 						self.b_res = Some(ok);
@@ -200,10 +205,12 @@ where
 				}
 			}
 
-			let ((_, tip_height_opt), block_hash) =
-				Joiner::new(source.get_best_block(), source.get_block_hash_by_height(block_height))
-					.await
-					.map_err(|_| UtxoLookupError::UnknownTx)?;
+			let ((_, tip_height_opt), block_hash) = Joiner::new(
+				pin!(source.get_best_block()),
+				pin!(source.get_block_hash_by_height(block_height)),
+			)
+			.await
+			.map_err(|_| UtxoLookupError::UnknownTx)?;
 			if let Some(tip_height) = tip_height_opt {
 				// If the block doesn't yet have five confirmations, error out.
 				//

lightning-block-sync/src/lib.rs

@@ -53,7 +53,6 @@ use lightning::chain::{BestBlock, Listen};
 
 use std::future::Future;
 use std::ops::Deref;
-use std::pin::Pin;
 
 /// Abstract type for retrieving block headers and data.
 pub trait BlockSource: Sync + Send {
@@ -65,31 +64,28 @@ pub trait BlockSource: Sync + Send {
 	/// when `height_hint` is `None`.
 	fn get_header<'a>(
 		&'a self, header_hash: &'a BlockHash, height_hint: Option<u32>,
-	) -> AsyncBlockSourceResult<'a, BlockHeaderData>;
+	) -> impl Future<Output = BlockSourceResult<BlockHeaderData>> + Send + 'a;
 
 	/// Returns the block for a given hash. A headers-only block source should return a `Transient`
 	/// error.
-	fn get_block<'a>(&'a self, header_hash: &'a BlockHash)
-		-> AsyncBlockSourceResult<'a, BlockData>;
+	fn get_block<'a>(
+		&'a self, header_hash: &'a BlockHash,
+	) -> impl Future<Output = BlockSourceResult<BlockData>> + Send + 'a;
 
 	/// Returns the hash of the best block and, optionally, its height.
 	///
 	/// When polling a block source, [`Poll`] implementations may pass the height to [`get_header`]
 	/// to allow for a more efficient lookup.
 	///
 	/// [`get_header`]: Self::get_header
-	fn get_best_block(&self) -> AsyncBlockSourceResult<'_, (BlockHash, Option<u32>)>;
+	fn get_best_block<'a>(
+		&'a self,
+	) -> impl Future<Output = BlockSourceResult<(BlockHash, Option<u32>)>> + Send + 'a;
 }
 
 /// Result type for `BlockSource` requests.
 pub type BlockSourceResult<T> = Result<T, BlockSourceError>;
 
-// TODO: Replace with BlockSourceResult once `async` trait functions are supported. For details,
-// see: https://areweasyncyet.rs.
-/// Result type for asynchronous `BlockSource` requests.
-pub type AsyncBlockSourceResult<'a, T> =
-	Pin<Box<dyn Future<Output = BlockSourceResult<T>> + 'a + Send>>;
-
 /// Error type for `BlockSource` requests.
 ///
 /// Transient errors may be resolved when re-polling, but no attempt will be made to re-poll on