diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs index 4d6e0dfa83..33cef0a17a 100644 --- a/crates/esplora/src/async_ext.rs +++ b/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( &self, keychain_spks: BTreeMap< @@ -158,6 +169,7 @@ impl EsploraAsyncExt for esplora_client::AsyncClient { stop_gap: usize, parallel_requests: usize, ) -> Result<(TxGraph, BTreeMap), Error> { + assert!(stop_gap > 0, "stop_gap must be higher than 0"); type TxsOfSpkIndex = (u32, Vec); let parallel_requests = Ord::max(parallel_requests, 1); let mut graph = TxGraph::::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; } } diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs index 993e33ac0b..a0a50ca011 100644 --- a/crates/esplora/src/blocking_ext.rs +++ b/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( &self, keychain_spks: BTreeMap>, @@ -145,6 +156,7 @@ impl EsploraExt for esplora_client::BlockingClient { stop_gap: usize, parallel_requests: usize, ) -> Result<(TxGraph, BTreeMap), Error> { + assert!(stop_gap > 0, "stop_gap must be higher than 0"); type TxsOfSpkIndex = (u32, Vec); let parallel_requests = Ord::max(parallel_requests, 1); let mut graph = TxGraph::::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; } } diff --git a/crates/esplora/tests/async_ext.rs b/crates/esplora/tests/async_ext.rs index baae1d11b0..1de1cbad3e 100644 --- a/crates/esplora/tests/async_ext.rs +++ b/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); diff --git a/crates/esplora/tests/blocking_ext.rs b/crates/esplora/tests/blocking_ext.rs index 54c367e76c..34da4b16e4 100644 --- a/crates/esplora/tests/blocking_ext.rs +++ b/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);