bitcoindevkit / bitcoindevkit/bdk
`full_scan` never terminates when the server reports history for every script
- Dominant language
- Rust
- Stars
- 1.1k
- Forks
- 483
- Avg merge
- 20d 3h
- Merged PRs (30d)
- 3
Description
**Describe the bug**
`full_scan` walks a keychain's unbounded script iterator and stops only after `stop_gap` *consecutive* scripts with an empty history. The consecutive-unused counter is reset to zero whenever a script has any history:
- `bdk_electrum`: `populate_with_spks` (`crates/electrum/src/bdk_electrum_client.rs:312-320`)
- `bdk_esplora`: `fetch_txs_with_keychain_spks` in `blocking_ext.rs:330-335` and `async_ext.rs` (same structure)
A buggy or misbehaving server that reports a nonempty history for every script (one unconfirmed entry is enough; nothing is verified for height 0) therefore keeps the scan running indefinitely. Every entry also fetches a full transaction, so the in-memory update grows along with it (`bdk_electrum` pushes to `tx_update.txs` once per history entry, even for a repeated txid). There is no client-side bound on scripts scanned, requests made or update size: termination depends entirely on the server's answers. This is the restore/import path, so the caller has no prior state to notice that the reported history is implausible.
This issue was found by AI.
**To Reproduce**
Add `crates/electrum/tests/test_full_scan_never_ends.rs` and run `cargo test -p bdk_electrum --test test_full_scan_never_ends`. It runs a stub Electrum server in-process; no real server is needed:
```rust
use std::io::{BufRead, BufReader, Write};
use std::net::TcpListener;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;
use std::time::Duration;
use bdk_chain::bitcoin::{absolute, consensus, transaction, Amount, ScriptBuf, Transaction, TxIn, TxOut};
use bdk_chain::spk_client::FullScanRequest;
use bdk_electrum::electrum_client::{Client, ConfigBuilder};
use bdk_electrum::BdkElectrumClient;
/// Electrum stub that reports the same unconfirmed transaction for every script.
fn serve_history_for_every_script(listener: TcpListener, histories_served: Arc) {
let tx = Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::ZERO,
input: vec![TxIn::default()],
output: vec![TxOut { value: Amount::ZERO, script_pubkey: ScriptBuf::new() }],
};
let (txid, raw_tx) = (tx.compute_txid(), consensus::encode::serialize_hex(&tx));
for stream in listener.incoming() {
let mut stream = stream.unwrap();
for request in BufReader::new(stream.try_clone().unwrap()).lines() {
let request = request.unwrap();
let id_start = request.find("\"id\":").unwrap() + 5;
let id: String = request[id_start..].chars().take_while(char::is_ascii_digit).collect();
let result = if request.contains("blockchain.scripthash.get_history") {
histories_served.fetch_add(1, Ordering::Relaxed);
format!(r#"[{{"height":0,"tx_hash":"{txid}"}}]"#)
} else if request.contains("blockchain.transaction.get") {
format!(r#""{raw_tx}""#)
} else {
panic!("unexpected request: {request}");
};
writeln!(stream, r#"{{"jsonrpc":"2.0","id":{id},"result":{result}}}"#).unwrap();
}
}
}
#[test]
fn full_scan_terminates_when_every_script_has_history() {
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let url = format!("tcp://{}", listener.local_addr().unwrap());
let histories_served = Arc::new(AtomicUsize::new(0));
let counter = Arc::clone(&histories_served);
std::thread::spawn(move || serve_history_for_every_script(listener, counter));
let config = ConfigBuilder::new().retry(0).build();
let client = BdkElectrumClient::new(Client::from_config(&url, config).unwrap());
let spks = (0u32..).map(|i| (i, ScriptBuf::from_bytes(i.to_le_bytes().to_vec())));
let request = FullScanRequest::builder_at(0).spks_for_keychain(0u32, spks);
let scan = std::thread::spawn(move || client.full_scan(request, 10, 5, false));
std::thread::sleep(Duration::from_secs(2));
let served = histories_served.load(Ordering::Relaxed);
assert!(scan.is_finished(), "still scanning after {served} script histories");
}
```
The assertion fails with `still scanning after 68690 script histories` (`stop_gap = 10`, `batch_size = 5`, two seconds). The Esplora implementations share the same loop structure.
**Expected behavior**
A `full_scan` should not be able to run indefinitely based solely on what the server reports.
Contributor guide
Research direction
Run crates/electrum/tests/test_full_scan_never_ends.rs with `cargo test -p bdk_electrum --test test_full_scan_never_ends`, then inspect `populate_with_spks` in crates/electrum/src/bdk_electrum_client.rs and the matching loops in crates/esplora/src/blocking_ext.rs and async_ext.rs. Determine how full_scan should terminate without relying solely on server history, and verify the regression test completes while the shared behavior remains consistent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100