fix(esplora): treat parallel_requests of 0 as 1

A `parallel_requests` of 0 made `full_scan` and `sync` skip every request
and return an empty `Ok`, because each fetch loop takes its next batch with
`.take(parallel_requests)` and stops when the batch is empty.

Clamp it to 1 in each fetch loop, for both the blocking and async clients,
as `stop_gap` already is. Document this on `full_scan` and `sync`, and add
tests covering the spk, txid and outpoint paths.

Fixes #2291

diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs
index 093604d8a6e69b81a9d1956c04f827c8bf316342..80fa025330c760930ac483e1efc5691080fb63b7 100644 (file)
--- a/crates/esplora/src/async_ext.rs
+++ b/crates/esplora/src/async_ext.rs
@@ -27,7 +27,8 @@ pub trait EsploraAsyncExt {
     /// `request` provides the data required to perform a script-pubkey-based full scan
     /// (see [`FullScanRequest`]). The full scan for each keychain (`K`) stops after a gap of
     /// `stop_gap` script pubkeys with no associated transactions. `parallel_requests` specifies
-    /// the maximum number of HTTP requests to make in parallel.
+    /// the maximum number of HTTP requests to make in parallel. A `parallel_requests` of 0 is
+    /// treated as 1.
     ///
     /// Refer to [crate-level docs](crate) for more.
     async fn full_scan<K: Ord + Clone + Send, R: Into<FullScanRequest<K>> + Send>(
@@ -41,7 +42,7 @@ pub trait EsploraAsyncExt {
     ///
     /// `request` provides the data required to perform a script-pubkey-based sync (see
     /// [`SyncRequest`]). `parallel_requests` specifies the maximum number of HTTP requests to make
-    /// in parallel.
+    /// in parallel. A `parallel_requests` of 0 is treated as 1.
     ///
     /// Refer to [crate-level docs](crate) for more.
     async fn sync<I: Send, R: Into<SyncRequest<I>> + Send>(
@@ -322,6 +323,7 @@ where
     let mut consecutive_unused = 0usize;
     // Treat stop_gap = 0 as 1 while preserving original semantics for other values.
     let gap_limit = stop_gap.max(1);
+    let parallel_requests = parallel_requests.max(1);
 
     loop {
         let handles = keychain_spks
@@ -449,6 +451,7 @@ where
         .filter(|txid| !inserted_txs.contains(txid))
         .collect::<Vec<Txid>>()
         .into_iter();
+    let parallel_requests = parallel_requests.max(1);
     loop {
         let handles = txids
             .by_ref()
@@ -494,6 +497,7 @@ where
     I::IntoIter: Send,
     S: Sleeper + Clone + Send + Sync,
 {
+    let parallel_requests = parallel_requests.max(1);
     let outpoints = outpoints.into_iter().collect::<Vec<_>>();
     let mut update = TxUpdate::<ConfirmationBlockTime>::default();
 

diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs
index 5a768190b58b1ff388c3576d674c467b67695a40..2bd9722e4e2150b967686c57d1696b8621a30ca9 100644 (file)
--- a/crates/esplora/src/blocking_ext.rs
+++ b/crates/esplora/src/blocking_ext.rs
@@ -24,7 +24,8 @@ pub trait EsploraExt {
     /// `request` provides the data required to perform a script-pubkey-based full scan
     /// (see [`FullScanRequest`]). The full scan for each keychain (`K`) stops after a gap of
     /// `stop_gap` script pubkeys with no associated transactions. `parallel_requests` specifies
-    /// the maximum number of HTTP requests to make in parallel.
+    /// the maximum number of HTTP requests to make in parallel. A `parallel_requests` of 0 is
+    /// treated as 1.
     ///
     /// Refer to [crate-level docs](crate) for more.
     fn full_scan<K: Ord + Clone, R: Into<FullScanRequest<K>>>(
@@ -38,7 +39,7 @@ pub trait EsploraExt {
     ///
     /// `request` provides the data required to perform a script-pubkey-based sync (see
     /// [`SyncRequest`]). `parallel_requests` specifies the maximum number of HTTP requests to make
-    /// in parallel.
+    /// in parallel. A `parallel_requests` of 0 is treated as 1.
     ///
     /// Refer to [crate-level docs](crate) for more.
     fn sync<I: 'static, R: Into<SyncRequest<I>>>(
@@ -290,6 +291,7 @@ fn fetch_txs_with_keychain_spks<I: Iterator<Item = Indexed<SpkWithExpectedTxids>
     let mut consecutive_unused = 0usize;
     // Treat stop_gap = 0 as 1 while preserving original semantics for other values.
     let gap_limit = stop_gap.max(1);
+    let parallel_requests = parallel_requests.max(1);
 
     loop {
         let handles = keychain_spks
@@ -407,6 +409,7 @@ fn fetch_txs_with_txids<I: IntoIterator<Item = Txid>>(
         .filter(|txid| !inserted_txs.contains(txid))
         .collect::<Vec<Txid>>()
         .into_iter();
+    let parallel_requests = parallel_requests.max(1);
     loop {
         let handles = txids
             .by_ref()
@@ -453,6 +456,7 @@ fn fetch_txs_with_outpoints<I: IntoIterator<Item = OutPoint>>(
     outpoints: I,
     parallel_requests: usize,
 ) -> Result<TxUpdate<ConfirmationBlockTime>, Error> {
+    let parallel_requests = parallel_requests.max(1);
     let outpoints = outpoints.into_iter().collect::<Vec<_>>();
     let mut update = TxUpdate::<ConfirmationBlockTime>::default();
 

diff --git a/crates/esplora/tests/async_ext.rs b/crates/esplora/tests/async_ext.rs
index c37543009574588666c425f422b5e5637b9e26da..a14a8397c9f0334904e536c72efe078d4e8d85c9 100644 (file)
--- a/crates/esplora/tests/async_ext.rs
+++ b/crates/esplora/tests/async_ext.rs
@@ -1,4 +1,4 @@
-use bdk_chain::bitcoin::{Address, Amount};
+use bdk_chain::bitcoin::{hashes::Hash, Address, Amount, OutPoint, ScriptBuf, Txid};
 use bdk_chain::local_chain::LocalChain;
 use bdk_chain::spk_client::{FullScanRequest, SyncRequest};
 use bdk_chain::spk_txout::SpkTxOutIndex;
@@ -435,3 +435,43 @@ async fn sync_terminates_when_server_page_does_not_advance() {
         "`sync` must reject a page that does not advance the cursor"
     );
 }
+
+/// A `parallel_requests` of 0 must not make `full_scan` skip its scripts.
+///
+/// The client points at a closed port, so a scan that actually requests something returns an
+/// error. If no request is made, the scan returns an empty `Ok`.
+#[tokio::test]
+pub async fn test_full_scan_with_zero_parallel_requests() -> anyhow::Result<()> {
+    let client = Builder::new("http://127.0.0.1:1").build_async()?;
+    let request = FullScanRequest::builder().spks_for_keychain(0u32, [(0, ScriptBuf::new())]);
+
+    let response = client.full_scan(request, 3, 0).await;
+    assert!(
+        response.is_err(),
+        "scan skipped its script and returned {response:?}"
+    );
+    Ok(())
+}
+
+/// A `parallel_requests` of 0 must not make `sync` skip its spks, txids or outpoints (each goes
+/// through a different fetch loop).
+#[tokio::test]
+pub async fn test_sync_with_zero_parallel_requests() -> anyhow::Result<()> {
+    let client = Builder::new("http://127.0.0.1:1").build_async()?;
+    let requests = [
+        ("spks", SyncRequest::builder().spks([ScriptBuf::new()])),
+        ("txids", SyncRequest::builder().txids([Txid::all_zeros()])),
+        (
+            "outpoints",
+            SyncRequest::builder().outpoints([OutPoint::new(Txid::all_zeros(), 0)]),
+        ),
+    ];
+    for (kind, request) in requests {
+        let response = client.sync(request, 0).await;
+        assert!(
+            response.is_err(),
+            "sync skipped its {kind} and returned {response:?}"
+        );
+    }
+    Ok(())
+}

diff --git a/crates/esplora/tests/blocking_ext.rs b/crates/esplora/tests/blocking_ext.rs
index 128f82c32d8074466924d59535cf2e051d5c9ea5..1095f15255217466157f38f0050b3c2753111e71 100644 (file)
--- a/crates/esplora/tests/blocking_ext.rs
+++ b/crates/esplora/tests/blocking_ext.rs
@@ -1,4 +1,4 @@
-use bdk_chain::bitcoin::{Address, Amount};
+use bdk_chain::bitcoin::{hashes::Hash, Address, Amount, OutPoint, ScriptBuf, Txid};
 use bdk_chain::local_chain::LocalChain;
 use bdk_chain::spk_client::{FullScanRequest, SyncRequest};
 use bdk_chain::spk_txout::SpkTxOutIndex;
@@ -437,3 +437,41 @@ fn sync_terminates_when_server_page_does_not_advance() {
         "`sync` must reject a page that does not advance the cursor"
     );
 }
+
+/// A `parallel_requests` of 0 must not make `full_scan` skip its scripts.
+///
+/// The client points at a closed port, so a scan that actually requests something returns an
+/// error. If no request is made, the scan returns an empty `Ok`.
+#[test]
+pub fn test_full_scan_with_zero_parallel_requests() {
+    let client = Builder::new("http://127.0.0.1:1").build_blocking();
+    let request = FullScanRequest::builder().spks_for_keychain(0u32, [(0, ScriptBuf::new())]);
+
+    let response = client.full_scan(request, 3, 0);
+    assert!(
+        response.is_err(),
+        "scan skipped its script and returned {response:?}"
+    );
+}
+
+/// A `parallel_requests` of 0 must not make `sync` skip its spks, txids or outpoints (each goes
+/// through a different fetch loop).
+#[test]
+pub fn test_sync_with_zero_parallel_requests() {
+    let client = Builder::new("http://127.0.0.1:1").build_blocking();
+    let requests = [
+        ("spks", SyncRequest::builder().spks([ScriptBuf::new()])),
+        ("txids", SyncRequest::builder().txids([Txid::all_zeros()])),
+        (
+            "outpoints",
+            SyncRequest::builder().outpoints([OutPoint::new(Txid::all_zeros(), 0)]),
+        ),
+    ];
+    for (kind, request) in requests {
+        let response = client.sync(request, 0);
+        assert!(
+            response.is_err(),
+            "sync skipped its {kind} and returned {response:?}"
+        );
+    }
+}