fix: define and document stop_gap

- changes the code implementation to "the maximum number of consecutive unused addresses"
  in esplora async and blocking extjensions.
- treat `stop_gap = 0` as `stop_gap = 1` for all purposes.
- 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
  https://github.com/bitcoindevkit/bdk/issues/1227#issuecomment-1859040463
- 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
This commit is contained in:
Jose Storopoli 2024-03-26 12:44:03 -03:00
parent 80e190b3e7
commit 7c1861aab9
No known key found for this signature in database
GPG Key ID: 01BE853A206518CC
4 changed files with 52 additions and 24 deletions

View File

@ -52,6 +52,19 @@ 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 a `stop_gap` of 3, `full_scan` will keep scanning
/// 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).
///
/// A `stop_gap` of 0 will be treated as a `stop_gap` of 1.
async fn full_scan<K: Ord + Clone + Send>(
&self,
keychain_spks: BTreeMap<
@ -162,6 +175,7 @@ impl EsploraAsyncExt for esplora_client::AsyncClient {
let parallel_requests = Ord::max(parallel_requests, 1);
let mut graph = TxGraph::<ConfirmationTimeHeightAnchor>::default();
let mut last_active_indexes = BTreeMap::<K, u32>::new();
let stop_gap = Ord::max(stop_gap, 1);
for (keychain, spks) in keychain_spks {
let mut spks = spks.into_iter();
@ -226,12 +240,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 {
last_index > i.saturating_add(stop_gap as u32)
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;
}
}

View File

@ -50,6 +50,19 @@ 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 a `stop_gap` of 3, `full_scan` will keep scanning
/// 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).
///
/// A `stop_gap` of 0 will be treated as a `stop_gap` of 1.
fn full_scan<K: Ord + Clone>(
&self,
keychain_spks: BTreeMap<K, impl IntoIterator<Item = (u32, ScriptBuf)>>,
@ -149,6 +162,7 @@ impl EsploraExt for esplora_client::BlockingClient {
let parallel_requests = Ord::max(parallel_requests, 1);
let mut graph = TxGraph::<ConfirmationTimeHeightAnchor>::default();
let mut last_active_indexes = BTreeMap::<K, u32>::new();
let stop_gap = Ord::max(stop_gap, 1);
for (keychain, spks) in keychain_spks {
let mut spks = spks.into_iter();
@ -216,12 +230,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 {
last_index > i.saturating_add(stop_gap as u32)
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;
}
}

View File

@ -91,9 +91,9 @@ pub async fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
Ok(())
}
/// Test the bounds of the address scan depending on the gap limit.
/// Test the bounds of the address scan depending on the `stop_gap`.
#[tokio::test]
pub async fn test_async_update_tx_graph_gap_limit() -> anyhow::Result<()> {
pub async fn test_async_update_tx_graph_stop_gap() -> anyhow::Result<()> {
let env = TestEnv::new()?;
let base_url = format!("http://{}", &env.electrsd.esplora_url.clone().unwrap());
let client = Builder::new(base_url.as_str()).build_async()?;
@ -140,12 +140,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) = client.full_scan(keychains.clone(), 2, 1).await?;
let (graph_update, active_indices) = 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) = client.full_scan(keychains.clone(), 3, 1).await?;
let (graph_update, active_indices) = 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);
@ -165,14 +165,14 @@ pub async fn test_async_update_tx_graph_gap_limit() -> anyhow::Result<()> {
sleep(Duration::from_millis(10))
}
// A scan with gap limit 4 won't find the second transaction, but a scan with gap limit 5 will.
// 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 (graph_update, active_indices) = client.full_scan(keychains.clone(), 4, 1).await?;
let (graph_update, active_indices) = client.full_scan(keychains.clone(), 5, 1).await?;
let txs: HashSet<_> = graph_update.full_txs().map(|tx| tx.txid).collect();
assert_eq!(txs.len(), 1);
assert!(txs.contains(&txid_4th_addr));
assert_eq!(active_indices[&0], 3);
let (graph_update, active_indices) = client.full_scan(keychains, 5, 1).await?;
let (graph_update, active_indices) = client.full_scan(keychains, 6, 1).await?;
let txs: HashSet<_> = graph_update.full_txs().map(|tx| tx.txid).collect();
assert_eq!(txs.len(), 2);
assert!(txs.contains(&txid_4th_addr) && txs.contains(&txid_last_addr));

View File

@ -106,9 +106,9 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
Ok(())
}
/// Test the bounds of the address scan depending on the gap limit.
/// Test the bounds of the address scan depending on the `stop_gap`.
#[test]
pub fn test_update_tx_graph_gap_limit() -> anyhow::Result<()> {
pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> {
let env = TestEnv::new()?;
let base_url = format!("http://{}", &env.electrsd.esplora_url.clone().unwrap());
let client = Builder::new(base_url.as_str()).build_blocking()?;
@ -155,12 +155,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 stop_gap of 3 won't find the transaction, but a scan with a gap limit of 4
// will.
let (graph_update, active_indices) = client.full_scan(keychains.clone(), 2, 1)?;
let (graph_update, active_indices) = 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) = client.full_scan(keychains.clone(), 3, 1)?;
let (graph_update, active_indices) = 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);
@ -180,14 +180,14 @@ pub fn test_update_tx_graph_gap_limit() -> anyhow::Result<()> {
sleep(Duration::from_millis(10))
}
// A scan with gap limit 4 won't find the second transaction, but a scan with gap limit 5 will.
// 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 (graph_update, active_indices) = client.full_scan(keychains.clone(), 4, 1)?;
let (graph_update, active_indices) = client.full_scan(keychains.clone(), 5, 1)?;
let txs: HashSet<_> = graph_update.full_txs().map(|tx| tx.txid).collect();
assert_eq!(txs.len(), 1);
assert!(txs.contains(&txid_4th_addr));
assert_eq!(active_indices[&0], 3);
let (graph_update, active_indices) = client.full_scan(keychains, 5, 1)?;
let (graph_update, active_indices) = client.full_scan(keychains, 6, 1)?;
let txs: HashSet<_> = graph_update.full_txs().map(|tx| tx.txid).collect();
assert_eq!(txs.len(), 2);
assert!(txs.contains(&txid_4th_addr) && txs.contains(&txid_last_addr));