fix(full_scan): gate --scan-stop-gap to backends that use it

The RPC and CBF backends never use `stop_gap`, so accepting the flag silently
ignored it. Gate the option to the Electrum and Esplora backends and reject it
at runtime for RPC and CBF wallets in multi-backend builds, where the backend is
chosen from the wallet config rather than at compile time.

Read `STOP_GAP` in the handler rather than through clap so that a globally
exported value no longer turns an RPC or CBF scan into an error when the flag
was never passed, while supported backends keep the environment behaviour.

Keep the default of 20 in one place (`DEFAULT_STOP_GAP`) and document it, with
the env var, in `--help`.

Related to #340.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 45cf4fcd7ce81189cf8d175640af61fd9b474867..d477c1e9e2284ab3b1e9f890d2c3dc27b71156d4 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@ page. See [DEVELOPMENT_CYCLE.md](DEVELOPMENT_CYCLE.md) for more details.
 - Fixed `create_tx`, `create_dns_tx`, `create_sp_tx` and `bump_fee` building transactions over Bitcoin Core's 400,000 wu (100,000 vB) standard weight limit, which no default node relays
 - Fixed `key restore` ignoring `--password`, which restored passphrase-protected mnemonics to the wrong key
 - Fixed `key generate` silently producing a 24-word mnemonic for any `--entropy` value other than 12; values other than 12 and 24 are now rejected
+- Gated `full_scan`'s `--scan-stop-gap` to the Electrum and Esplora backends that use it; RPC and CBF wallets now reject the flag instead of silently ignoring it, while a `STOP_GAP` environment variable keeps working for supported backends and no longer breaks unsupported ones
 
 ## [4.0.0]
 

diff --git a/src/handlers/online.rs b/src/handlers/online.rs
index 25baf889711d04cfaed8da127c4cb41079e7d583..0b7342b8be5083616db34b02abf1acccbcfa7b01 100644 (file)
--- a/src/handlers/online.rs
+++ b/src/handlers/online.rs
@@ -82,12 +82,45 @@ impl OnlineWalletSubCommand {
 #[derive(Parser, Debug, PartialEq, Clone, Eq)]
 pub struct FullScanCommand {
     /// Stop searching addresses for transactions after finding an unused gap of this length.
-    #[arg(env = "STOP_GAP", long = "scan-stop-gap", default_value = "20")]
-    stop_gap: usize,
+    /// Only available with the `electrum` and `esplora` backends. When the flag is omitted,
+    /// the `STOP_GAP` environment variable is read if it is set. [default: 20]
+    #[cfg(any(feature = "electrum", feature = "esplora"))]
+    #[arg(long = "scan-stop-gap")]
+    stop_gap: Option<usize>,
+
     #[clap(long, default_value = "5")]
     pub parallel_request: usize,
 }
 
+#[cfg(any(feature = "electrum", feature = "esplora"))]
+const DEFAULT_STOP_GAP: usize = 20;
+
+#[cfg(any(feature = "electrum", feature = "esplora"))]
+impl FullScanCommand {
+    /// Resolves the stop gap, in order, from `--scan-stop-gap`, the `STOP_GAP` environment
+    /// variable, or the default.
+    ///
+    /// `STOP_GAP` is read here instead of through clap so that a globally exported value is
+    /// ignored by backends that do not support the option (RPC and CBF in a multi-backend
+    /// build) instead of turning their scans into an error.
+    fn stop_gap(&self) -> Result<usize, Error> {
+        if let Some(stop_gap) = self.stop_gap {
+            return Ok(stop_gap);
+        }
+        match std::env::var("STOP_GAP") {
+            Ok(value) => value.parse().map_err(|_| {
+                Error::Generic(format!(
+                    "`STOP_GAP` must be a positive integer, got `{value}`"
+                ))
+            }),
+            Err(std::env::VarError::NotPresent) => Ok(DEFAULT_STOP_GAP),
+            Err(std::env::VarError::NotUnicode(_)) => Err(Error::Generic(
+                "`STOP_GAP` is not valid unicode".to_string(),
+            )),
+        }
+    }
+}
+
 #[cfg(any(
     feature = "electrum",
     feature = "esplora",
@@ -121,7 +154,7 @@ impl AsyncAppCommand<AppContext<OnlineOperations<'_>>> for FullScanCommand {
             #[cfg(feature = "electrum")]
             Electrum { client, batch_size } => {
                 client.populate_tx_cache(wallet.tx_graph().full_txs().map(|tx_node| tx_node.tx));
-                let update = client.full_scan(request, self.stop_gap, *batch_size, false)?;
+                let update = client.full_scan(request, self.stop_gap()?, *batch_size, false)?;
                 let events = wallet.apply_update_events(update)?;
                 print_wallet_events(&events);
             }
@@ -132,7 +165,7 @@ impl AsyncAppCommand<AppContext<OnlineOperations<'_>>> for FullScanCommand {
                 parallel_requests,
             } => {
                 let update = client
-                    .full_scan(request, self.stop_gap, *parallel_requests)
+                    .full_scan(request, self.stop_gap()?, *parallel_requests)
                     .await
                     .map_err(|e| *e)?;
                 let events = wallet.apply_update_events(update)?;
@@ -140,6 +173,14 @@ impl AsyncAppCommand<AppContext<OnlineOperations<'_>>> for FullScanCommand {
             }
             #[cfg(feature = "rpc")]
             RpcClient { client } => {
+                #[cfg(any(feature = "electrum", feature = "esplora"))]
+                if self.stop_gap.is_some() {
+                    return Err(Error::Generic(
+                        "`--scan-stop-gap` is only supported by the electrum and esplora backends"
+                            .into(),
+                    ));
+                }
+
                 let blockchain_info = client.get_blockchain_info()?;
 
                 let genesis_block = bdk_wallet::bitcoin::constants::genesis_block(wallet.network());
@@ -187,6 +228,14 @@ impl AsyncAppCommand<AppContext<OnlineOperations<'_>>> for FullScanCommand {
 
             #[cfg(feature = "cbf")]
             KyotoClient { client } => {
+                #[cfg(any(feature = "electrum", feature = "esplora"))]
+                if self.stop_gap.is_some() {
+                    return Err(Error::Generic(
+                        "`--scan-stop-gap` is only supported by the electrum and esplora backends"
+                            .into(),
+                    ));
+                }
+
                 sync_kyoto_client(wallet, client).await?;
             }
         }
@@ -196,6 +245,27 @@ impl AsyncAppCommand<AppContext<OnlineOperations<'_>>> for FullScanCommand {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::FullScanCommand;
+    use clap::Parser;
+
+    #[test]
+    fn stop_gap_is_gated_to_supported_backends() {
+        #[cfg(any(feature = "electrum", feature = "esplora"))]
+        {
+            let default = FullScanCommand::try_parse_from(["bdk-cli"]).unwrap();
+            assert_eq!(default.stop_gap, None);
+            let custom =
+                FullScanCommand::try_parse_from(["bdk-cli", "--scan-stop-gap", "42"]).unwrap();
+            assert_eq!(custom.stop_gap, Some(42));
+        }
+
+        #[cfg(not(any(feature = "electrum", feature = "esplora")))]
+        assert!(FullScanCommand::try_parse_from(["bdk-cli", "--scan-stop-gap", "42"]).is_err());
+    }
+}
+
 #[derive(Parser, Debug, PartialEq, Eq, Clone)]
 pub struct SyncCommand;