Skip to content

fix: full_scan covers revealed range before applying stop_gap #2182

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.

Sign up for GitHub

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

Open
noahjoeris wants to merge 1 commit into
base: master
Choose a base branch
from
+226 −66
Open

fix: full_scan covers revealed range before applying stop_gap #2182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion crates/chain/src/indexer/keychain_txout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,11 @@ pub trait FullScanRequestBuilderExt<K> {
impl<K: Clone + Ord + core::fmt::Debug> FullScanRequestBuilderExt<K> for FullScanRequestBuilder<K> {
fn spks_from_indexer(mut self, indexer: &KeychainTxOutIndex<K>) -> Self {
for (keychain, spks) in indexer.all_unbounded_spk_iters() {
self = self.spks_for_keychain(keychain, spks);
let last_revealed = indexer.last_revealed_index(keychain.clone());
self = self.spks_for_keychain(keychain.clone(), spks);
if let Some(index) = last_revealed {
self = self.last_revealed_for_keychain(keychain, index);
}
}
self
}
Expand Down
22 changes: 22 additions & 0 deletions crates/core/src/spk_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,18 @@ impl<K: Ord, D> FullScanRequestBuilder<K, D> {
self
}

/// Record the last revealed script pubkey `index` for a given `keychain`.
///
/// `full_scan` covers `0..=index` for this keychain; `stop_gap`
/// applies only to indices past `index`. Keychains without a recorded last revealed
/// index fall back to applying `stop_gap` from index 0.
/// Users working with a `KeychainTxOutIndex` usually don't call this directly,
/// `spks_from_indexer` (from `bdk_chain`) populates it automatically.
pub fn last_revealed_for_keychain(mut self, keychain: K, index: u32) -> Self {
self.inner.last_revealed.insert(keychain, index);
self
}

/// Set the closure that will inspect every sync item visited.
pub fn inspect<F>(mut self, inspect: F) -> Self
where
Expand Down Expand Up @@ -483,6 +495,7 @@ pub struct FullScanRequest<K, D = BlockHash> {
start_time: u64,
chain_tip: Option<CheckPoint<D>>,
spks_by_keychain: BTreeMap<K, Box<dyn Iterator<Item = Indexed<ScriptBuf>> + Send>>,
last_revealed: BTreeMap<K, u32>,
inspect: Box<InspectFullScan<K>>,
}

Expand All @@ -507,6 +520,7 @@ impl<K: Ord + Clone, D> FullScanRequest<K, D> {
start_time,
chain_tip: None,
spks_by_keychain: BTreeMap::new(),
last_revealed: BTreeMap::new(),
inspect: Box::new(|_, _, _| ()),
},
}
Expand Down Expand Up @@ -541,6 +555,14 @@ impl<K: Ord + Clone, D> FullScanRequest<K, D> {
self.spks_by_keychain.keys().cloned().collect()
}

/// Get the last revealed script pubkey index for `keychain` (if set).
///
/// Chain sources use this to scan `0..=last_revealed` before applying
/// `stop_gap` to further discovery.
pub fn last_revealed(&self, keychain: &K) -> Option<u32> {
self.last_revealed.get(keychain).copied()
}

/// Advances the full scan request and returns the next indexed [`ScriptBuf`] of the given
/// `keychain`.
pub fn next_spk(&mut self, keychain: K) -> Option<Indexed<ScriptBuf>> {
Expand Down
19 changes: 13 additions & 6 deletions crates/electrum/src/bdk_electrum_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
let mut last_active_indices = BTreeMap::<K, u32>::default();
let mut pending_anchors = Vec::new();
for keychain in request.keychains() {
let last_revealed = request.last_revealed(&keychain);
let spks = request
.iter_spks(keychain.clone())
.map(|(spk_i, spk)| (spk_i, SpkWithExpectedTxids::from(spk)));
Expand All @@ -138,6 +139,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
&mut tx_update,
spks,
stop_gap,
last_revealed,
batch_size,
&mut pending_anchors,
)? {
Expand Down Expand Up @@ -219,6 +221,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
.enumerate()
.map(|(i, spk)| (i as u32, spk)),
usize::MAX,
None,
batch_size,
&mut pending_anchors,
)?;
Expand Down Expand Up @@ -267,12 +270,14 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
/// Transactions that contains an output with requested spk, or spends form an output with
/// requested spk will be added to `tx_update`. Anchors of the aforementioned transactions are
/// also included.
#[allow(clippy::too_many_arguments)]
fn populate_with_spks(
&self,
start_time: u64,
tx_update: &mut TxUpdate<ConfirmationBlockTime>,
mut spks_with_expected_txids: impl Iterator<Item = (u32, SpkWithExpectedTxids)>,
stop_gap: usize,
last_revealed: Option<u32>,
Comment on lines +273 to +280
Copy link
Copy Markdown
Author

@noahjoeris noahjoeris Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we combine some parameters to avoid suppressing clippy::too_many_arguments? e.g.

struct StopGapExt {
  stop_gap: usize
  last_revealed Option<u32>
}

batch_size: usize,
pending_anchors: &mut Vec<(Txid, usize)>,
) -> Result<Option<u32>, Error> {
Expand All @@ -292,14 +297,16 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
.batch_script_get_history(spks.iter().map(|(_, s)| s.spk.as_script()))?;

for ((spk_index, spk), spk_history) in spks.into_iter().zip(spk_histories) {
if spk_history.is_empty() {
match unused_spk_count.checked_add(1) {
Some(i) if i < stop_gap => unused_spk_count = i,
_ => return Ok(last_active_index),
};
} else {
let beyond_revealed = last_revealed.is_none_or(|lr| spk_index > lr);

if !spk_history.is_empty() {
last_active_index = Some(spk_index);
unused_spk_count = 0;
} else if beyond_revealed {
unused_spk_count = unused_spk_count.saturating_add(1);
if unused_spk_count >= stop_gap {
return Ok(last_active_index);
}
}

let spk_history_set = spk_history
Expand Down
77 changes: 60 additions & 17 deletions crates/electrum/tests/test_electrum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,24 @@ pub fn get_test_spk() -> ScriptBuf {
ScriptBuf::new_p2tr(&secp, pk, None)
}

pub fn test_addresses() -> Vec<Address> {
[
"bcrt1qj9f7r8r3p2y0sqf4r3r62qysmkuh0fzep473d2ar7rcz64wqvhssjgf0z4",
"bcrt1qmm5t0ch7vh2hryx9ctq3mswexcugqe4atkpkl2tetm8merqkthas3w7q30",
"bcrt1qut9p7ej7l7lhyvekj28xknn8gnugtym4d5qvnp5shrsr4nksmfqsmyn87g",
"bcrt1qqz0xtn3m235p2k96f5wa2dqukg6shxn9n3txe8arlrhjh5p744hsd957ww",
"bcrt1q9c0t62a8l6wfytmf2t9lfj35avadk3mm8g4p3l84tp6rl66m48sqrme7wu",
"bcrt1qkmh8yrk2v47cklt8dytk8f3ammcwa4q7dzattedzfhqzvfwwgyzsg59zrh",
"bcrt1qvgrsrzy07gjkkfr5luplt0azxtfwmwq5t62gum5jr7zwcvep2acs8hhnp2",
"bcrt1qw57edarcg50ansq8mk3guyrk78rk0fwvrds5xvqeupteu848zayq549av8",
"bcrt1qvtve5ekf6e5kzs68knvnt2phfw6a0yjqrlgat392m6zt9jsvyxhqfx67ef",
"bcrt1qw03ddumfs9z0kcu76ln7jrjfdwam20qtffmkcral3qtza90sp9kqm787uk",
]
.into_iter()
.map(|s| Address::from_str(s).unwrap().assume_checked())
.collect()
}

fn get_balance(
recv_chain: &LocalChain,
recv_graph: &IndexedTxGraph<ConfirmationBlockTime, SpkTxOutIndex<()>>,
Expand Down Expand Up @@ -383,23 +401,7 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> {
let client = BdkElectrumClient::new(electrum_client);
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 addresses = test_addresses();
let spks: Vec<_> = addresses
.iter()
.enumerate()
Expand Down Expand Up @@ -490,6 +492,47 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> {
Ok(())
}

/// Test that `full_scan` always scans the revealed range before applying `stop_gap`.
#[test]
pub fn test_stop_gap_past_last_revealed() -> anyhow::Result<()> {
let env = TestEnv::new()?;
let electrum_client = electrum_client::Client::new(env.electrsd.electrum_url.as_str())?;
let client = BdkElectrumClient::new(electrum_client);
let _block_hashes = env.mine_blocks(101, None)?;

let addresses = test_addresses();
let spks: Vec<_> = addresses
.iter()
.enumerate()
.map(|(i, addr)| (i as u32, addr.script_pubkey()))
.collect();

// Receive coins beyond stop_gap of 3.
let txid_last_addr = env
.bitcoind
.client
.send_to_address(&addresses[9], Amount::from_sat(10000))?
.txid()?;
env.mine_blocks(1, None)?;
env.wait_until_electrum_sees_block(Duration::from_secs(6))?;

let cp_tip = env.make_checkpoint_tip();

let request = FullScanRequest::builder()
.chain_tip(cp_tip.clone())
.spks_for_keychain(0, spks.clone())
.last_revealed_for_keychain(0, 9);
let response = client.full_scan(request, 3, 1, false)?;

assert_eq!(
response.tx_update.txs.first().unwrap().compute_txid(),
txid_last_addr
);
assert_eq!(response.last_active_indices[&0], 9);

Ok(())
}

/// Ensure that [`BdkElectrumClient::sync`] can confirm previously unconfirmed transactions in both
/// reorg and no-reorg situations. After the transaction is confirmed after reorg, check if floating
/// txouts for previous outputs were inserted for transaction fee calculation.
Expand Down
11 changes: 8 additions & 3 deletions crates/esplora/src/async_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ where
let mut inserted_txs = HashSet::<Txid>::new();
let mut last_active_indices = BTreeMap::<K, u32>::new();
for keychain in keychains {
let last_revealed = request.last_revealed(&keychain);
let keychain_spks = request
.iter_spks(keychain.clone())
.map(|(spk_i, spk)| (spk_i, spk.into()));
Expand All @@ -88,6 +89,7 @@ where
&mut inserted_txs,
keychain_spks,
stop_gap,
last_revealed,
parallel_requests,
)
.await?;
Expand Down Expand Up @@ -305,6 +307,7 @@ async fn fetch_txs_with_keychain_spks<I, S>(
inserted_txs: &mut HashSet<Txid>,
mut keychain_spks: I,
stop_gap: usize,
last_revealed: Option<u32>,
parallel_requests: usize,
) -> Result<(TxUpdate<ConfirmationBlockTime>, Option<u32>), Error>
where
Expand Down Expand Up @@ -355,12 +358,13 @@ where
}

for (index, txs, evicted) in handles.try_collect::<Vec<TxsOfSpkIndex>>().await? {
if txs.is_empty() {
consecutive_unused = consecutive_unused.saturating_add(1);
} else {
if !txs.is_empty() {
consecutive_unused = 0;
last_active_index = Some(index);
} else if last_revealed.is_none_or(|lr| index > lr) {
consecutive_unused = consecutive_unused.saturating_add(1);
}

for tx in txs {
if inserted_txs.insert(tx.txid) {
update.txs.push(tx.to_tx().into());
Expand Down Expand Up @@ -407,6 +411,7 @@ where
inserted_txs,
spks.into_iter().enumerate().map(|(i, spk)| (i as u32, spk)),
usize::MAX,
None,
parallel_requests,
)
.await
Expand Down
11 changes: 8 additions & 3 deletions crates/esplora/src/blocking_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl EsploraExt for esplora_client::BlockingClient {
let mut inserted_txs = HashSet::<Txid>::new();
let mut last_active_indices = BTreeMap::<K, u32>::new();
for keychain in request.keychains() {
let last_revealed = request.last_revealed(&keychain);
let keychain_spks = request
.iter_spks(keychain.clone())
.map(|(spk_i, spk)| (spk_i, spk.into()));
Expand All @@ -78,6 +79,7 @@ impl EsploraExt for esplora_client::BlockingClient {
&mut inserted_txs,
keychain_spks,
stop_gap,
last_revealed,
parallel_requests,
)?;
tx_update.extend(update);
Expand Down Expand Up @@ -277,6 +279,7 @@ fn fetch_txs_with_keychain_spks<I: Iterator<Item = Indexed<SpkWithExpectedTxids>
inserted_txs: &mut HashSet<Txid>,
mut keychain_spks: I,
stop_gap: usize,
last_revealed: Option<u32>,
parallel_requests: usize,
) -> Result<(TxUpdate<ConfirmationBlockTime>, Option<u32>), Error> {
type TxsOfSpkIndex = (u32, Vec<esplora_client::Tx>, HashSet<Txid>);
Expand Down Expand Up @@ -324,12 +327,13 @@ fn fetch_txs_with_keychain_spks<I: Iterator<Item = Indexed<SpkWithExpectedTxids>

for handle in handles {
let (index, txs, evicted) = handle.join().expect("thread must not panic")?;
if txs.is_empty() {
consecutive_unused = consecutive_unused.saturating_add(1);
} else {
if !txs.is_empty() {
consecutive_unused = 0;
last_active_index = Some(index);
} else if last_revealed.is_none_or(|lr| index > lr) {
consecutive_unused = consecutive_unused.saturating_add(1);
}

for tx in txs {
if inserted_txs.insert(tx.txid) {
update.txs.push(tx.to_tx().into());
Expand Down Expand Up @@ -371,6 +375,7 @@ fn fetch_txs_with_spks<I: IntoIterator<Item = SpkWithExpectedTxids>>(
inserted_txs,
spks.into_iter().enumerate().map(|(i, spk)| (i as u32, spk)),
usize::MAX,
None,
parallel_requests,
)
.map(|(update, _)| update)
Expand Down
62 changes: 45 additions & 17 deletions crates/esplora/tests/async_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,7 @@ pub async fn test_async_update_tx_graph_stop_gap() -> anyhow::Result<()> {
let client = Builder::new(base_url.as_str()).build_async()?;
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 addresses = common::test_addresses();
let spks: Vec<_> = addresses
.iter()
.enumerate()
Expand Down Expand Up @@ -369,3 +353,47 @@ pub async fn test_async_update_tx_graph_stop_gap() -> anyhow::Result<()> {

Ok(())
}

/// Test that `full_scan` always scans the revealed range before applying `stop_gap`.
#[tokio::test]
pub async fn test_async_stop_gap_past_last_revealed() -> 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()?;
let _block_hashes = env.mine_blocks(101, None)?;

let addresses = common::test_addresses();
let spks: Vec<_> = addresses
.iter()
.enumerate()
.map(|(i, addr)| (i as u32, addr.script_pubkey()))
.collect();

// Receive coins beyond stop_gap of 3
let txid_last_addr = env
.bitcoind
.client
.send_to_address(&addresses[9], Amount::from_sat(10000))?
.txid()?;
let _block_hashes = env.mine_blocks(1, None)?;
while client.get_height().await.unwrap() < 103 {
sleep(Duration::from_millis(10))
}

let cp_tip = env.make_checkpoint_tip();

// the scan covers 0..=9 despite stop_gap=3
let request = FullScanRequest::builder()
.chain_tip(cp_tip.clone())
.spks_for_keychain(0, spks.clone())
.last_revealed_for_keychain(0, 9);
let response = client.full_scan(request, 3, 1).await?;

assert_eq!(
response.tx_update.txs.first().unwrap().compute_txid(),
txid_last_addr
);
assert_eq!(response.last_active_indices[&0], 9);

Ok(())
}
Loading
Loading