From: luhrhenz Date: Fri, 9 Oct 2026 14:43:14 +0000 (+0100) Subject: fix(esplora): stop paging when the server does not advance the cursor X-Git-Url: http://internal-gitweb-vhost/-script/src/example_cli/scripts/struct.EncoderStringWriter.html?a=commitdiff_plain;h=19ba4e3d67516e4a3045b9870ccf7bfd6173579e;p=bdk fix(esplora): stop paging when the server does not advance the cursor The per-script-pubkey pagination loop paged forward by asking for the transactions following the last txid of the previous page, but never checked that the cursor moved. A server that answers every request with the same full page of 25 transactions kept the cursor in place, so the loop re-requested that page forever while appending to an unbounded `Vec`, and `sync`/`full_scan` never returned. Track every cursor paged from, and return `InvalidResponse` if a full page ends on one we have already seen. This catches a cursor that repeats straight away as well as one that cycles (`A -> B -> A`). Erroring rather than breaking avoids silently handing the caller a truncated transaction history, which is how `chain_update` already treats a server it cannot reconcile with. Closes #2339 --- diff --git a/crates/esplora/Cargo.toml b/crates/esplora/Cargo.toml index 54065c96..69a4892a 100644 --- a/crates/esplora/Cargo.toml +++ b/crates/esplora/Cargo.toml @@ -26,7 +26,7 @@ serde_json = { version = "1.0", features = ["alloc"] } esplora-client = { version = "0.12.0" } bdk_chain = { path = "../chain" } bdk_testenv = { path = "../testenv" } -tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros"] } +tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros", "time"] } [features] default = ["std", "async-https", "blocking-https"] diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs index 2b382895..093604d8 100644 --- a/crates/esplora/src/async_ext.rs +++ b/crates/esplora/src/async_ext.rs @@ -333,6 +333,7 @@ where let spk = spk.spk; async move { let mut last_seen = None; + let mut seen_cursors = HashSet::new(); let mut spk_txs = Vec::new(); loop { let txs = client.scripthash_txs(&spk, last_seen).await?; @@ -342,6 +343,11 @@ where if tx_count < 25 { break; } + // Paging from a cursor we have already paged from means the server is + // not paging forward (it repeats or cycles), so we would loop forever. + if !seen_cursors.insert(last_seen) { + return Err(Box::new(esplora_client::Error::InvalidResponse)); + } } let got_txids = spk_txs.iter().map(|tx| tx.txid).collect::>(); let evicted_txids = expected_txids diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs index 225b574e..5a768190 100644 --- a/crates/esplora/src/blocking_ext.rs +++ b/crates/esplora/src/blocking_ext.rs @@ -301,6 +301,7 @@ fn fetch_txs_with_keychain_spks let spk = spk.spk; std::thread::spawn(move || -> Result { let mut last_txid = None; + let mut seen_cursors = HashSet::new(); let mut spk_txs = Vec::new(); loop { let txs = client.scripthash_txs(&spk, last_txid)?; @@ -310,6 +311,11 @@ fn fetch_txs_with_keychain_spks if tx_count < 25 { break; } + // Paging from a cursor we have already paged from means the server is + // not paging forward (it repeats or cycles), so we would loop forever. + if !seen_cursors.insert(last_txid) { + return Err(Box::new(esplora_client::Error::InvalidResponse)); + } } let got_txids = spk_txs.iter().map(|tx| tx.txid).collect::>(); let evicted_txids = expected_txids diff --git a/crates/esplora/tests/async_ext.rs b/crates/esplora/tests/async_ext.rs index 2941e2a6..c3754300 100644 --- a/crates/esplora/tests/async_ext.rs +++ b/crates/esplora/tests/async_ext.rs @@ -410,3 +410,28 @@ pub async fn test_async_stop_gap_past_last_revealed() -> anyhow::Result<()> { Ok(()) } + +// Ensure `sync` gives up on a server that keeps returning the same page of transactions. +// +// The per-spk pagination loop advances by asking for the transactions following the last txid of +// the previous page. A server that answers every request with the same full page leaves the cursor +// where it was, which used to make the loop request that page forever. See bitcoindevkit/bdk#2339. +#[tokio::test] +async fn sync_terminates_when_server_page_does_not_advance() { + let base_url = common::spawn_non_advancing_server(); + let client = Builder::new(&base_url) + .build_async() + .expect("must build client"); + + let request = SyncRequest::builder().spks([common::get_test_spk()]); + let result = tokio::time::timeout(Duration::from_secs(10), client.sync(request, 1)) + .await + .expect("`sync` must terminate"); + assert!( + matches!( + result.unwrap_err().as_ref(), + esplora_client::Error::InvalidResponse + ), + "`sync` must reject a page that does not advance the cursor" + ); +} diff --git a/crates/esplora/tests/blocking_ext.rs b/crates/esplora/tests/blocking_ext.rs index 8db59947..128f82c3 100644 --- a/crates/esplora/tests/blocking_ext.rs +++ b/crates/esplora/tests/blocking_ext.rs @@ -409,3 +409,31 @@ pub fn test_stop_gap_past_last_revealed() -> anyhow::Result<()> { Ok(()) } + +// Ensure `sync` gives up on a server that keeps returning the same page of transactions. +// +// The per-spk pagination loop advances by asking for the transactions following the last txid of +// the previous page. A server that answers every request with the same full page leaves the cursor +// where it was, which used to make the loop request that page forever. See bitcoindevkit/bdk#2339. +#[test] +fn sync_terminates_when_server_page_does_not_advance() { + let base_url = common::spawn_non_advancing_server(); + let client = Builder::new(&base_url).build_blocking(); + + let (result_tx, result_rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let request = SyncRequest::builder().spks([common::get_test_spk()]); + let _ = result_tx.send(client.sync(request, 1).map(|_| ())); + }); + + let result = result_rx + .recv_timeout(Duration::from_secs(10)) + .expect("`sync` must terminate"); + assert!( + matches!( + result.unwrap_err().as_ref(), + esplora_client::Error::InvalidResponse + ), + "`sync` must reject a page that does not advance the cursor" + ); +} diff --git a/crates/esplora/tests/common/mod.rs b/crates/esplora/tests/common/mod.rs index 9fe6dfd9..d7a78494 100644 --- a/crates/esplora/tests/common/mod.rs +++ b/crates/esplora/tests/common/mod.rs @@ -2,6 +2,8 @@ use bdk_core::bitcoin::key::{Secp256k1, UntweakedPublicKey}; use bdk_core::bitcoin::{Address, ScriptBuf}; +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; use std::str::FromStr; const PK_BYTES: &[u8] = &[ @@ -32,3 +34,73 @@ pub fn test_addresses() -> Vec
{ .map(|s| Address::from_str(s).unwrap().assume_checked()) .collect() } + +/// Number of transactions per page requested by `bdk_esplora`. +const PAGE_SIZE: u32 = 25; + +/// A full page of transactions, always ending on the same txid. +fn non_advancing_page() -> String { + let txs = (0..PAGE_SIZE) + .map(|i| { + serde_json::json!({ + "txid": format!("{i:064x}"), + "version": 2, + "locktime": 0, + "vin": [], + "vout": [], + "size": 100, + "weight": 400, + "status": { + "confirmed": false, + "block_height": null, + "block_hash": null, + "block_time": null + }, + "fee": 0 + }) + }) + .collect::>(); + serde_json::to_string(&txs).expect("page must serialize") +} + +/// Spawns a stub Esplora server which answers every request with the same full page of +/// transactions, emulating a server that never pages forward. +/// +/// Returns the base URL to point an Esplora client at. The listener is served until the test +/// binary exits. +pub fn spawn_non_advancing_server() -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("must bind to loopback"); + let base_url = format!( + "http://{}", + listener.local_addr().expect("must have local addr") + ); + let body = non_advancing_page(); + + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + // Every path gets the same answer, so there is nothing to parse. We still consume the + // request head, otherwise closing the connection could abort the client's write. + let mut reader = BufReader::new(stream.try_clone().expect("must clone stream")); + let mut line = String::new(); + while reader.read_line(&mut line).unwrap_or(0) > 0 { + if line == "\r\n" || line == "\n" { + break; + } + line.clear(); + } + let _ = write!( + stream, + "HTTP/1.1 200 OK\r\n\ + content-type: application/json\r\n\ + content-length: {}\r\n\ + connection: close\r\n\ + \r\n\ + {body}", + body.len() + ); + } + }); + + base_url +}