Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esplora: fix incorrect gap limit check in blocking client #1225

Merged
merged 4 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion crates/esplora/src/async_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,13 @@ impl EsploraAsyncExt for esplora_client::AsyncClient {
}
}

if last_index > last_active_index.map(|i| i.saturating_add(stop_gap as u32)) {
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)
} else {
last_index >= stop_gap as u32
};
if past_gap_limit {
break;
}
}
Expand Down
8 changes: 7 additions & 1 deletion crates/esplora/src/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,13 @@ impl EsploraExt for esplora_client::BlockingClient {
}
}

if last_index > last_active_index.map(|i| i.saturating_add(stop_gap as u32)) {
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)
} else {
last_index >= stop_gap as u32
};
if past_gap_limit {
break;
}
}
Expand Down
119 changes: 119 additions & 0 deletions crates/esplora/tests/async_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use electrsd::bitcoind::bitcoincore_rpc::RpcApi;
use electrsd::bitcoind::{self, anyhow, BitcoinD};
use electrsd::{Conf, ElectrsD};
use esplora_client::{self, AsyncClient, Builder};
use std::collections::{BTreeMap, HashSet};
use std::str::FromStr;
use std::thread::sleep;
use std::time::Duration;
Expand Down Expand Up @@ -115,3 +116,121 @@ pub async fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
assert_eq!(graph_update_txids, expected_txids);
Ok(())
}

/// 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<()> {
let env = TestEnv::new()?;
let _block_hashes = env.mine_blocks(101, None)?;

// Now let's test the gap limit. First of all get a chain of 10 addresses.
let addresses = [
"bcrt1qj9f7r8r3p2y0sqf4r3r62qysmkuh0fzep473d2ar7rcz64wqvhssjgf0z4",
"bcrt1qmm5t0ch7vh2hryx9ctq3mswexcugqe4atkpkl2tetm8merqkthas3w7q30",
"bcrt1qut9p7ej7l7lhyvekj28xknn8gnugtym4d5qvnp5shrsr4nksmfqsmyn87g",
"bcrt1qqz0xtn3m235p2k96f5wa2dqukg6shxn9n3txe8arlrhjh5p744hsd957ww",
"bcrt1q9c0t62a8l6wfytmf2t9lfj35avadk3mm8g4p3l84tp6rl66m48sqrme7wu",
"bcrt1qkmh8yrk2v47cklt8dytk8f3ammcwa4q7dzattedzfhqzvfwwgyzsg59zrh",
"bcrt1qvgrsrzy07gjkkfr5luplt0azxtfwmwq5t62gum5jr7zwcvep2acs8hhnp2",
"bcrt1qw57edarcg50ansq8mk3guyrk78rk0fwvrds5xvqeupteu848zayq549av8",
"bcrt1qvtve5ekf6e5kzs68knvnt2phfw6a0yjqrlgat392m6zt9jsvyxhqfx67ef",
"bcrt1qw03ddumfs9z0kcu76ln7jrjfdwam20qtffmkcral3qtza90sp9kqm787uk",
];
let addresses: Vec<_> = addresses
.into_iter()
.map(|s| Address::from_str(s).unwrap().assume_checked())
.collect();
let spks: Vec<_> = addresses
.iter()
.enumerate()
.map(|(i, addr)| (i as u32, addr.script_pubkey()))
.collect();
let mut keychains = BTreeMap::new();
keychains.insert(0, spks);

// Then receive coins on the 4th address.
let txid_4th_addr = env.bitcoind.client.send_to_address(
&addresses[3],
Amount::from_sat(10000),
None,
None,
None,
None,
Some(1),
None,
)?;
let _block_hashes = env.mine_blocks(1, None)?;
while env.client.get_height().await.unwrap() < 103 {
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
// will.
let (graph_update, active_indices) = env
.client
.scan_txs_with_keychains(
keychains.clone(),
vec![].into_iter(),
vec![].into_iter(),
2,
1,
)
.await?;
assert!(graph_update.full_txs().next().is_none());
assert!(active_indices.is_empty());
let (graph_update, active_indices) = env
.client
.scan_txs_with_keychains(
keychains.clone(),
vec![].into_iter(),
vec![].into_iter(),
3,
1,
)
.await?;
assert_eq!(graph_update.full_txs().next().unwrap().txid, txid_4th_addr);
assert_eq!(active_indices[&0], 3);

// Now receive a coin on the last address.
let txid_last_addr = env.bitcoind.client.send_to_address(
&addresses[addresses.len() - 1],
Amount::from_sat(10000),
None,
None,
None,
None,
Some(1),
None,
)?;
let _block_hashes = env.mine_blocks(1, None)?;
while env.client.get_height().await.unwrap() < 104 {
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.
// The last active indice won't be updated in the first case but will in the second one.
let (graph_update, active_indices) = env
.client
.scan_txs_with_keychains(
keychains.clone(),
vec![].into_iter(),
vec![].into_iter(),
4,
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) = env
.client
.scan_txs_with_keychains(keychains, vec![].into_iter(), vec![].into_iter(), 5, 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));
assert_eq!(active_indices[&0], 9);

Ok(())
}
114 changes: 114 additions & 0 deletions crates/esplora/tests/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use electrsd::bitcoind::bitcoincore_rpc::RpcApi;
use electrsd::bitcoind::{self, anyhow, BitcoinD};
use electrsd::{Conf, ElectrsD};
use esplora_client::{self, BlockingClient, Builder};
use std::collections::{BTreeMap, HashSet};
use std::str::FromStr;
use std::thread::sleep;
use std::time::Duration;
Expand Down Expand Up @@ -110,5 +111,118 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
let mut expected_txids = vec![txid1, txid2];
expected_txids.sort();
assert_eq!(graph_update_txids, expected_txids);

Ok(())
}

/// Test the bounds of the address scan depending on the gap limit.
#[test]
pub fn test_update_tx_graph_gap_limit() -> anyhow::Result<()> {
let env = TestEnv::new()?;
let _block_hashes = env.mine_blocks(101, None)?;

// Now let's test the gap limit. First of all get a chain of 10 addresses.
let addresses = [
"bcrt1qj9f7r8r3p2y0sqf4r3r62qysmkuh0fzep473d2ar7rcz64wqvhssjgf0z4",
"bcrt1qmm5t0ch7vh2hryx9ctq3mswexcugqe4atkpkl2tetm8merqkthas3w7q30",
"bcrt1qut9p7ej7l7lhyvekj28xknn8gnugtym4d5qvnp5shrsr4nksmfqsmyn87g",
"bcrt1qqz0xtn3m235p2k96f5wa2dqukg6shxn9n3txe8arlrhjh5p744hsd957ww",
"bcrt1q9c0t62a8l6wfytmf2t9lfj35avadk3mm8g4p3l84tp6rl66m48sqrme7wu",
"bcrt1qkmh8yrk2v47cklt8dytk8f3ammcwa4q7dzattedzfhqzvfwwgyzsg59zrh",
"bcrt1qvgrsrzy07gjkkfr5luplt0azxtfwmwq5t62gum5jr7zwcvep2acs8hhnp2",
"bcrt1qw57edarcg50ansq8mk3guyrk78rk0fwvrds5xvqeupteu848zayq549av8",
"bcrt1qvtve5ekf6e5kzs68knvnt2phfw6a0yjqrlgat392m6zt9jsvyxhqfx67ef",
"bcrt1qw03ddumfs9z0kcu76ln7jrjfdwam20qtffmkcral3qtza90sp9kqm787uk",
];
let addresses: Vec<_> = addresses
.into_iter()
.map(|s| Address::from_str(s).unwrap().assume_checked())
.collect();
let spks: Vec<_> = addresses
.iter()
.enumerate()
.map(|(i, addr)| (i as u32, addr.script_pubkey()))
.collect();
let mut keychains = BTreeMap::new();
keychains.insert(0, spks);

// Then receive coins on the 4th address.
let txid_4th_addr = env.bitcoind.client.send_to_address(
&addresses[3],
Amount::from_sat(10000),
None,
None,
None,
None,
Some(1),
None,
)?;
let _block_hashes = env.mine_blocks(1, None)?;
while env.client.get_height().unwrap() < 103 {
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
// will.
let (graph_update, active_indices) = env.client.scan_txs_with_keychains(
keychains.clone(),
vec![].into_iter(),
vec![].into_iter(),
2,
1,
)?;
assert!(graph_update.full_txs().next().is_none());
assert!(active_indices.is_empty());
let (graph_update, active_indices) = env.client.scan_txs_with_keychains(
keychains.clone(),
vec![].into_iter(),
vec![].into_iter(),
3,
1,
)?;
assert_eq!(graph_update.full_txs().next().unwrap().txid, txid_4th_addr);
assert_eq!(active_indices[&0], 3);

// Now receive a coin on the last address.
let txid_last_addr = env.bitcoind.client.send_to_address(
&addresses[addresses.len() - 1],
Amount::from_sat(10000),
None,
None,
None,
None,
Some(1),
None,
)?;
let _block_hashes = env.mine_blocks(1, None)?;
while env.client.get_height().unwrap() < 104 {
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.
// The last active indice won't be updated in the first case but will in the second one.
let (graph_update, active_indices) = env.client.scan_txs_with_keychains(
keychains.clone(),
vec![].into_iter(),
vec![].into_iter(),
4,
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) = env.client.scan_txs_with_keychains(
keychains,
vec![].into_iter(),
vec![].into_iter(),
5,
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));
assert_eq!(active_indices[&0], 9);

Ok(())
}
Loading