fix: define and document stop_gap

- changes the code implementation to "the maximum number of consecutive unused addresses"
  in esplora async and blocking extensions.
- panics if stop gap is zero (> 0).
- renames `past_gap_limit` to `gap_limit_reached` to indicate we want to break once the gap
  limit is reached and not go further in `full_scan`, as suggested in
  #1227 (comment)
- change the tests according to the new implementation.
- add notes on what `stop_gap` means and links to convergent definition in other
  Bitcoin-related software.

Closes #1227.

crates/esplora/src/async_ext.rs

@@ -52,6 +52,17 @@ pub trait EsploraAsyncExt {
     /// The full scan for each keychain stops after a gap of `stop_gap` script pubkeys with no associated
     /// transactions. `parallel_requests` specifies the max number of HTTP requests to make in
     /// parallel.
+    ///
+    /// ## Note
+    ///
+    /// `stop_gap` is defined as "the maximum number of consecutive unused addresses".
+    /// For example, with `stop_gap = 3`, `full_scan` will keep searching for script pubkeys
+    /// until it encounters 3 consecutive script pubkeys with no associated transactions.
+    ///
+    /// This follows the same approach as other Bitcoin-related software,
+    /// such as [Electrum](https://electrum.readthedocs.io/en/latest/faq.html#what-is-the-gap-limit),
+    /// [BTCPay Server](https://docs.btcpayserver.org/FAQ/Wallet/#the-gap-limit-problem),
+    /// and [Sparrow](https://www.sparrowwallet.com/docs/faq.html#ive-restored-my-wallet-but-some-of-my-funds-are-missing).
     async fn full_scan<K: Ord + Clone + Send>(
         &self,
         keychain_spks: BTreeMap<
@@ -158,6 +169,7 @@ impl EsploraAsyncExt for esplora_client::AsyncClient {
         stop_gap: usize,
         parallel_requests: usize,
     ) -> Result<(TxGraph<ConfirmationTimeHeightAnchor>, BTreeMap<K, u32>), Error> {
+        assert!(stop_gap > 0, "stop_gap must be higher than 0");
         type TxsOfSpkIndex = (u32, Vec<esplora_client::Tx>);
         let parallel_requests = Ord::max(parallel_requests, 1);
         let mut graph = TxGraph::<ConfirmationTimeHeightAnchor>::default();
@@ -226,12 +238,12 @@ impl EsploraAsyncExt for esplora_client::AsyncClient {
                 }
 
                 let last_index = last_index.expect("Must be set since handles wasn't empty.");
-                let past_gap_limit = if let Some(i) = last_active_index {
+                let gap_limit_reached = if let Some(i) = last_active_index {
                     last_index > i.saturating_add(stop_gap as u32)
                 } else {
-                    last_index >= stop_gap as u32
+                    last_index + 1 >= stop_gap as u32
                 };
-                if past_gap_limit {
+                if gap_limit_reached {
                     break;
                 }
             }

crates/esplora/src/blocking_ext.rs

@@ -50,6 +50,17 @@ pub trait EsploraExt {
     /// The full scan for each keychain stops after a gap of `stop_gap` script pubkeys with no associated
     /// transactions. `parallel_requests` specifies the max number of HTTP requests to make in
     /// parallel.
+    ///
+    /// ## Note
+    ///
+    /// `stop_gap` is defined as "the maximum number of consecutive unused addresses".
+    /// For example, with `stop_gap = 3`, `full_scan` will keep searching for script pubkeys
+    /// until it encounters 3 consecutive script pubkeys with no associated transactions.
+    ///
+    /// This follows the same approach as other Bitcoin-related software,
+    /// such as [Electrum](https://electrum.readthedocs.io/en/latest/faq.html#what-is-the-gap-limit),
+    /// [BTCPay Server](https://docs.btcpayserver.org/FAQ/Wallet/#the-gap-limit-problem),
+    /// and [Sparrow](https://www.sparrowwallet.com/docs/faq.html#ive-restored-my-wallet-but-some-of-my-funds-are-missing).
     fn full_scan<K: Ord + Clone>(
         &self,
         keychain_spks: BTreeMap<K, impl IntoIterator<Item = (u32, ScriptBuf)>>,
@@ -145,6 +156,7 @@ impl EsploraExt for esplora_client::BlockingClient {
         stop_gap: usize,
         parallel_requests: usize,
     ) -> Result<(TxGraph<ConfirmationTimeHeightAnchor>, BTreeMap<K, u32>), Error> {
+        assert!(stop_gap > 0, "stop_gap must be higher than 0");
         type TxsOfSpkIndex = (u32, Vec<esplora_client::Tx>);
         let parallel_requests = Ord::max(parallel_requests, 1);
         let mut graph = TxGraph::<ConfirmationTimeHeightAnchor>::default();
@@ -216,12 +228,12 @@ impl EsploraExt for esplora_client::BlockingClient {
                 }
 
                 let last_index = last_index.expect("Must be set since handles wasn't empty.");
-                let past_gap_limit = if let Some(i) = last_active_index {
+                let gap_limit_reached = if let Some(i) = last_active_index {
                     last_index > i.saturating_add(stop_gap as u32)
                 } else {
-                    last_index >= stop_gap as u32
+                    last_index + 1 >= stop_gap as u32
                 };
-                if past_gap_limit {
+                if gap_limit_reached {
                     break;
                 }
             }

crates/esplora/tests/async_ext.rs

@@ -139,6 +139,24 @@ pub async fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
     Ok(())
 }
 
+/// Test that stop_gap 0 should panic
+#[tokio::test]
+#[should_panic = "stop_gap must be higher than 0"]
+pub async fn test_stop_gap_0() {
+    let env = TestEnv::new().expect("could not create TestEnv");
+    let spks = vec![(
+        0_u32,
+        Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm")
+            .expect("could not convert Address from str")
+            .assume_checked()
+            .script_pubkey(),
+    )];
+
+    let mut keychains = BTreeMap::new();
+    keychains.insert(0, spks);
+    env.client.full_scan(keychains.clone(), 0, 1).await.unwrap();
+}
+
 /// Test the bounds of the address scan depending on the gap limit.
 #[tokio::test]
 pub async fn test_async_update_tx_graph_gap_limit() -> anyhow::Result<()> {
@@ -186,12 +204,12 @@ pub async fn test_async_update_tx_graph_gap_limit() -> anyhow::Result<()> {
         sleep(Duration::from_millis(10))
     }
 
-    // A scan with a gap limit of 2 won't find the transaction, but a scan with a gap limit of 3
+    // A scan with a gap limit of 3 won't find the transaction, but a scan with a gap limit of 4
     // will.
-    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 2, 1).await?;
+    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 3, 1).await?;
     assert!(graph_update.full_txs().next().is_none());
     assert!(active_indices.is_empty());
-    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 3, 1).await?;
+    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 4, 1).await?;
     assert_eq!(graph_update.full_txs().next().unwrap().txid, txid_4th_addr);
     assert_eq!(active_indices[&0], 3);
 

crates/esplora/tests/blocking_ext.rs

@@ -167,6 +167,24 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
     Ok(())
 }
 
+/// Test that stop_gap 0 should panic
+#[test]
+#[should_panic = "stop_gap must be higher than 0"]
+pub fn test_stop_gap_0() {
+    let env = TestEnv::new().expect("could not create TestEnv");
+    let spks = vec![(
+        0_u32,
+        Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm")
+            .expect("could not convert Address from str")
+            .assume_checked()
+            .script_pubkey(),
+    )];
+
+    let mut keychains = BTreeMap::new();
+    keychains.insert(0, spks);
+    env.client.full_scan(keychains.clone(), 0, 1).unwrap();
+}
+
 /// Test the bounds of the address scan depending on the gap limit.
 #[test]
 pub fn test_update_tx_graph_gap_limit() -> anyhow::Result<()> {
@@ -214,12 +232,12 @@ pub fn test_update_tx_graph_gap_limit() -> anyhow::Result<()> {
         sleep(Duration::from_millis(10))
     }
 
-    // A scan with a gap limit of 2 won't find the transaction, but a scan with a gap limit of 3
+    // A scan with a gap limit of 3 won't find the transaction, but a scan with a gap limit of 4
     // will.
-    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 2, 1)?;
+    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 3, 1)?;
     assert!(graph_update.full_txs().next().is_none());
     assert!(active_indices.is_empty());
-    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 3, 1)?;
+    let (graph_update, active_indices) = env.client.full_scan(keychains.clone(), 4, 1)?;
     assert_eq!(graph_update.full_txs().next().unwrap().txid, txid_4th_addr);
     assert_eq!(active_indices[&0], 3);