Merge bitcoindevkit#1478: Allow opting out of getting LocalChain up…

…dates with `FullScanRequest`/`SyncRequest` structures

6d77e2e refactor(chain)!: Rename `spks_with_labels` to `spks_with_indices` (志宇)
584b10a docs(esplora): README example, uncomment async import (志宇)
3eb5dd1 fix(chain): correct `Iterator::size_hint` impl (志宇)
96023c0 docs(chain): improve `SyncRequestBuilder::spks_with_labels` docs (志宇)
0234f70 docs(esplora): Fix typo (志宇)
38f86fe fix: no premature collect (志宇)
44e2a79 feat!: rework `FullScanRequest` and `SyncRequest` (志宇)
16c1c2c docs(esplora): Simplify crate docs (志宇)
c93e6fd feat(esplora): always fetch prevouts (志宇)
cad3533 feat(esplora): make ext traits more flexible (志宇)

Pull request description:

  Closes bitcoindevkit#1528

  ### Description

  Some users use `bdk_esplora` to update `bdk_chain` structures *without* a `LocalChain`. ~~This PR introduces "low-level" methods to `EsploraExt` and `EsploraAsyncExt` which populates a `TxGraph` update with associated data.~~

  We change `FullScanRequest`/`SyncRequest` to take in the `chain_tip` parameter as an option. Spk-based chain sources (`bdk_electrum` and `bdk_esplora`) will not fetch a chain-update if `chain_tip` is `None`, allowing callers to opt-out of receiving updates for `LocalChain`.

  We change `FullScanRequest`/`SyncRequest` to have better ergonomics when inspecting the progress of syncing (refer to bitcoindevkit#1528).

  We change `FullScanRequest`/`SyncRequest` to be constructed with a builder pattern. This is a better API since we separate request-construction and request-consumption.

  Additionally, much of the `bdk_esplora` logic has been made more efficient (less calls to Esplora) by utilizing the `/tx/:txid` endpoint (`get_tx_info` method). This method returns the tx and tx_status in one call. The logic for fetching updates for outpoints has been reworked to support parallelism.

  Documentation has also been updated.

  ### Notes to reviewers

  This PR has evolved somewhat. Initially, we were adding more methods on `EsploraExt`/`EsploraAsyncExt` to make syncing/scanning more modular. However, it turns out we can just make the `chain_tip` parameter of the request structures optional. Since we are changing the request structures, we might as well go further and improve the ergonomics of the whole structures (refer to bitcoindevkit#1528). This is where we are at with this PR.

  Unfortunately, the changes are now breaking. I think this is an okay tradeoff for the API improvements (before we get to stable).

  ### Changelog notice

  * Change request structures in `bdk_chain::spk_client` to be constructed via a builder pattern, make providing a `chain_tip` optional, and have better ergonomics for inspecting progress while syncing.
  * Change `bdk_esplora` to be more efficient by reducing the number of calls via the `/tx/:txid` endpoint. The logic for fetching outpoint updates has been reworked to support parallelism.
  * Change `bdk_esplora` to always add prev-txouts to the `TxGraph` update.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [x] I've added docs for the new feature

ACKs for top commit:
  ValuedMammal:
    ACK 6d77e2e
  notmandatory:
    ACK 6d77e2e

Tree-SHA512: 806cb159a8801f4e33846d18e6053b65d105e452b0f3f9d639b0c3f2e48fb665e632898bf42977901526834587223b0d7ec7ba1f73bb796b5fd8fe91e6f287f7

crates/electrum/src/bdk_electrum_client.rs

@@ -126,31 +126,43 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
     /// [`Wallet.calculate_fee_rate`]: https://docs.rs/bdk_wallet/latest/bdk_wallet/struct.Wallet.html#method.calculate_fee_rate
     pub fn full_scan<K: Ord + Clone>(
         &self,
-        request: FullScanRequest<K>,
+        request: impl Into<FullScanRequest<K>>,
         stop_gap: usize,
         batch_size: usize,
         fetch_prev_txouts: bool,
     ) -> Result<FullScanResult<K>, Error> {
-        let (tip, latest_blocks) =
-            fetch_tip_and_latest_blocks(&self.inner, request.chain_tip.clone())?;
-        let mut graph_update = TxGraph::<ConfirmationBlockTime>::default();
-        let mut last_active_indices = BTreeMap::<K, u32>::new();
+        let mut request: FullScanRequest<K> = request.into();
+
+        let tip_and_latest_blocks = match request.chain_tip() {
+            Some(chain_tip) => Some(fetch_tip_and_latest_blocks(&self.inner, chain_tip)?),
+            None => None,
+        };
 
-        for (keychain, spks) in request.spks_by_keychain {
+        let mut graph_update = TxGraph::<ConfirmationBlockTime>::default();
+        let mut last_active_indices = BTreeMap::<K, u32>::default();
+        for keychain in request.keychains() {
+            let spks = request.iter_spks(keychain.clone());
             if let Some(last_active_index) =
                 self.populate_with_spks(&mut graph_update, spks, stop_gap, batch_size)?
             {
                 last_active_indices.insert(keychain, last_active_index);
             }
         }
 
-        let chain_update = chain_update(tip, &latest_blocks, graph_update.all_anchors())?;
-
         // Fetch previous `TxOut`s for fee calculation if flag is enabled.
         if fetch_prev_txouts {
             self.fetch_prev_txout(&mut graph_update)?;
         }
 
+        let chain_update = match tip_and_latest_blocks {
+            Some((chain_tip, latest_blocks)) => Some(chain_update(
+                chain_tip,
+                &latest_blocks,
+                graph_update.all_anchors(),
+            )?),
+            _ => None,
+        };
+
         Ok(FullScanResult {
             graph_update,
             chain_update,
@@ -180,35 +192,49 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
     /// [`CalculateFeeError::MissingTxOut`]: bdk_chain::tx_graph::CalculateFeeError::MissingTxOut
     /// [`Wallet.calculate_fee`]: https://docs.rs/bdk_wallet/latest/bdk_wallet/struct.Wallet.html#method.calculate_fee
     /// [`Wallet.calculate_fee_rate`]: https://docs.rs/bdk_wallet/latest/bdk_wallet/struct.Wallet.html#method.calculate_fee_rate
-    pub fn sync(
+    pub fn sync<I: 'static>(
         &self,
-        request: SyncRequest,
+        request: impl Into<SyncRequest<I>>,
         batch_size: usize,
         fetch_prev_txouts: bool,
     ) -> Result<SyncResult, Error> {
-        let full_scan_req = FullScanRequest::from_chain_tip(request.chain_tip.clone())
-            .set_spks_for_keychain((), request.spks.enumerate().map(|(i, spk)| (i as u32, spk)));
-        let mut full_scan_res = self.full_scan(full_scan_req, usize::MAX, batch_size, false)?;
-        let (tip, latest_blocks) =
-            fetch_tip_and_latest_blocks(&self.inner, request.chain_tip.clone())?;
-
-        self.populate_with_txids(&mut full_scan_res.graph_update, request.txids)?;
-        self.populate_with_outpoints(&mut full_scan_res.graph_update, request.outpoints)?;
-
-        let chain_update = chain_update(
-            tip,
-            &latest_blocks,
-            full_scan_res.graph_update.all_anchors(),
+        let mut request: SyncRequest<I> = request.into();
+
+        let tip_and_latest_blocks = match request.chain_tip() {
+            Some(chain_tip) => Some(fetch_tip_and_latest_blocks(&self.inner, chain_tip)?),
+            None => None,
+        };
+
+        let mut graph_update = TxGraph::<ConfirmationBlockTime>::default();
+        self.populate_with_spks(
+            &mut graph_update,
+            request
+                .iter_spks()
+                .enumerate()
+                .map(|(i, spk)| (i as u32, spk)),
+            usize::MAX,
+            batch_size,
         )?;
+        self.populate_with_txids(&mut graph_update, request.iter_txids())?;
+        self.populate_with_outpoints(&mut graph_update, request.iter_outpoints())?;
 
         // Fetch previous `TxOut`s for fee calculation if flag is enabled.
         if fetch_prev_txouts {
-            self.fetch_prev_txout(&mut full_scan_res.graph_update)?;
+            self.fetch_prev_txout(&mut graph_update)?;
         }
 
+        let chain_update = match tip_and_latest_blocks {
+            Some((chain_tip, latest_blocks)) => Some(chain_update(
+                chain_tip,
+                &latest_blocks,
+                graph_update.all_anchors(),
+            )?),
+            None => None,
+        };
+
         Ok(SyncResult {
+            graph_update,
             chain_update,
-            graph_update: full_scan_res.graph_update,
         })
     }
 

crates/electrum/tests/test_electrum.rs

@@ -39,7 +39,7 @@ where
     Spks::IntoIter: ExactSizeIterator + Send + 'static,
 {
     let mut update = client.sync(
-        SyncRequest::from_chain_tip(chain.tip()).chain_spks(spks),
+        SyncRequest::builder().chain_tip(chain.tip()).spks(spks),
         BATCH_SIZE,
         true,
     )?;
@@ -51,9 +51,11 @@ where
         .as_secs();
     let _ = update.graph_update.update_last_seen_unconfirmed(now);
 
-    let _ = chain
-        .apply_update(update.chain_update.clone())
-        .map_err(|err| anyhow::anyhow!("LocalChain update error: {:?}", err))?;
+    if let Some(chain_update) = update.chain_update.clone() {
+        let _ = chain
+            .apply_update(chain_update)
+            .map_err(|err| anyhow::anyhow!("LocalChain update error: {:?}", err))?;
+    }
     let _ = graph.apply_update(update.graph_update.clone());
 
     Ok(update)
@@ -103,7 +105,9 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
     let cp_tip = env.make_checkpoint_tip();
 
     let sync_update = {
-        let request = SyncRequest::from_chain_tip(cp_tip.clone()).set_spks(misc_spks);
+        let request = SyncRequest::builder()
+            .chain_tip(cp_tip.clone())
+            .spks(misc_spks);
         client.sync(request, 1, true)?
     };
 
@@ -207,15 +211,17 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> {
     // A scan with a stop_gap of 3 won't find the transaction, but a scan with a gap limit of 4
     // will.
     let full_scan_update = {
-        let request =
-            FullScanRequest::from_chain_tip(cp_tip.clone()).set_spks_for_keychain(0, spks.clone());
+        let request = FullScanRequest::builder()
+            .chain_tip(cp_tip.clone())
+            .spks_for_keychain(0, spks.clone());
         client.full_scan(request, 3, 1, false)?
     };
     assert!(full_scan_update.graph_update.full_txs().next().is_none());
     assert!(full_scan_update.last_active_indices.is_empty());
     let full_scan_update = {
-        let request =
-            FullScanRequest::from_chain_tip(cp_tip.clone()).set_spks_for_keychain(0, spks.clone());
+        let request = FullScanRequest::builder()
+            .chain_tip(cp_tip.clone())
+            .spks_for_keychain(0, spks.clone());
         client.full_scan(request, 4, 1, false)?
     };
     assert_eq!(
@@ -246,8 +252,9 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> {
     // A scan with gap limit 5 won't find the second transaction, but a scan with gap limit 6 will.
     // The last active indice won't be updated in the first case but will in the second one.
     let full_scan_update = {
-        let request =
-            FullScanRequest::from_chain_tip(cp_tip.clone()).set_spks_for_keychain(0, spks.clone());
+        let request = FullScanRequest::builder()
+            .chain_tip(cp_tip.clone())
+            .spks_for_keychain(0, spks.clone());
         client.full_scan(request, 5, 1, false)?
     };
     let txs: HashSet<_> = full_scan_update
@@ -259,8 +266,9 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> {
     assert!(txs.contains(&txid_4th_addr));
     assert_eq!(full_scan_update.last_active_indices[&0], 3);
     let full_scan_update = {
-        let request =
-            FullScanRequest::from_chain_tip(cp_tip.clone()).set_spks_for_keychain(0, spks.clone());
+        let request = FullScanRequest::builder()
+            .chain_tip(cp_tip.clone())
+            .spks_for_keychain(0, spks.clone());
         client.full_scan(request, 6, 1, false)?
     };
     let txs: HashSet<_> = full_scan_update
@@ -311,7 +319,7 @@ fn test_sync() -> anyhow::Result<()> {
     let txid = env.send(&addr_to_track, SEND_AMOUNT)?;
     env.wait_until_electrum_sees_txid(txid, Duration::from_secs(6))?;
 
-    sync_with_electrum(
+    let _ = sync_with_electrum(
         &client,
         [spk_to_track.clone()],
         &mut recv_chain,
@@ -332,7 +340,7 @@ fn test_sync() -> anyhow::Result<()> {
     env.mine_blocks(1, None)?;
     env.wait_until_electrum_sees_block(Duration::from_secs(6))?;
 
-    sync_with_electrum(
+    let _ = sync_with_electrum(
         &client,
         [spk_to_track.clone()],
         &mut recv_chain,
@@ -353,7 +361,7 @@ fn test_sync() -> anyhow::Result<()> {
     env.reorg_empty_blocks(1)?;
     env.wait_until_electrum_sees_block(Duration::from_secs(6))?;
 
-    sync_with_electrum(
+    let _ = sync_with_electrum(
         &client,
         [spk_to_track.clone()],
         &mut recv_chain,
@@ -373,7 +381,7 @@ fn test_sync() -> anyhow::Result<()> {
     env.mine_blocks(1, None)?;
     env.wait_until_electrum_sees_block(Duration::from_secs(6))?;
 
-    sync_with_electrum(&client, [spk_to_track], &mut recv_chain, &mut recv_graph)?;
+    let _ = sync_with_electrum(&client, [spk_to_track], &mut recv_chain, &mut recv_graph)?;
 
     // Check if balance is correct once transaction is confirmed again.
     assert_eq!(

crates/esplora/Cargo.toml

@@ -13,11 +13,9 @@ readme = "README.md"
 
 [dependencies]
 bdk_chain = { path = "../chain", version = "0.17.0", default-features = false }
-esplora-client = { version = "0.8.0", default-features = false }
+esplora-client = { version = "0.9.0", default-features = false }
 async-trait = { version = "0.1.66", optional = true }
 futures = { version = "0.3.26", optional = true }
-
-bitcoin = { version = "0.32.0", optional = true, default-features = false }
 miniscript = { version = "12.0.0", optional = true, default-features = false }
 
 [dev-dependencies]

crates/esplora/README.md

@@ -1,11 +1,12 @@
 # BDK Esplora
 
-BDK Esplora extends [`esplora-client`] to update [`bdk_chain`] structures
-from an Esplora server.
+BDK Esplora extends [`esplora-client`] (with extension traits: [`EsploraExt`] and
+[`EsploraAsyncExt`]) to update [`bdk_chain`] structures from an Esplora server.
 
-## Usage
+The extension traits are primarily intended to satisfy [`SyncRequest`]s with [`sync`] and
+[`FullScanRequest`]s with [`full_scan`].
 
-There are two versions of the extension trait (blocking and async).
+## Usage
 
 For blocking-only:
 ```toml
@@ -27,10 +28,16 @@ To use the extension traits:
 // for blocking
 use bdk_esplora::EsploraExt;
 // for async
-// use bdk_esplora::EsploraAsyncExt;
+use bdk_esplora::EsploraAsyncExt;
 ```
 
 For full examples, refer to [`example-crates/wallet_esplora_blocking`](https://github.com/bitcoindevkit/bdk/tree/master/example-crates/wallet_esplora_blocking) and [`example-crates/wallet_esplora_async`](https://github.com/bitcoindevkit/bdk/tree/master/example-crates/wallet_esplora_async).
 
 [`esplora-client`]: https://docs.rs/esplora-client/
 [`bdk_chain`]: https://docs.rs/bdk-chain/
+[`EsploraExt`]: crate::EsploraExt
+[`EsploraAsyncExt`]: crate::EsploraAsyncExt
+[`SyncRequest`]: bdk_chain::spk_client::SyncRequest
+[`FullScanRequest`]: bdk_chain::spk_client::FullScanRequest
+[`sync`]: crate::EsploraExt::sync
+[`full_scan`]: crate::EsploraExt::full_scan