Skip to content

fix(esplora): stop paging when the server does not advance the cursor - #2361

Merged
evanlinjin merged 1 commit into
bitcoindevkit:masterfrom
luhrhenz:fix/esplora-pagination-non-advancing
Oct 18, 2026
Merged

evanlinjin merged 1 commit into
bitcoindevkit:masterfrom
luhrhenz:fix/esplora-pagination-non-advancing

Conversation

@luhrhenz

@luhrhenz luhrhenz commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #2339.

bdk_esplora pages each script pubkey's history by passing the previous page's last txid as the cursor, and stops only on a page of fewer than 25 txs. A server that ignores the cursor returns the same full page forever, so sync/full_scan never returns and memory grows without bound.

The loop now records every cursor it pages from and returns InvalidResponse when a full page ends on one it has already seen. This catches both a repeated page and a cycle (A -> B -> A). Applied to both async_ext.rs and blocking_ext.rs.

Notes to the reviewers

  • It returns an error rather than breaking, so that sync never reports a truncated history as success.
  • There is no page cap, so long histories aren't truncated. A correct server never repeats a cursor, so the check can't trigger on one.
  • The regression tests use a small TcpListener stub in tests/common that always returns the same page, since TestEnv can't fake a misbehaving server. tokio's time dev-dep feature is for the async test's timeout.

Changelog notice

  • Fixed bdk_esplora's pagination loop hanging when a chain source returns a page that does not advance the last_seen cursor.

Checklists

All Submissions:

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@luhrhenz
luhrhenz requested a review from a team as a code owner October 9, 2026 14:48
@notmandatory notmandatory added the bug Something isn't working label Oct 12, 2026
@notmandatory notmandatory moved this from Triage to In Progress in BDK Chain Oct 12, 2026
@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.67%. Comparing base (7f7f653) to head (19ba4e3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2361      +/-   ##
==========================================
+ Coverage   79.53%   79.67%   +0.14%     
==========================================
  Files          31       31              
  Lines        6141     6149       +8     
  Branches      290      292       +2     
==========================================
+ Hits         4884     4899      +15     
+ Misses       1182     1178       -4     
+ Partials       75       72       -3     
Flag Coverage Δ
rust 79.67% <100.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread crates/esplora/src/async_ext.rs Outdated
Comment on lines +348 to +351
if next_last_seen == last_seen {
return Err(Box::new(esplora_client::Error::InvalidResponse));
}
last_seen = next_last_seen;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This only catches a cursor that repeats straight away. A server that cycles (A → B → A, e.g. one that maps the cursor to a wrong page) still loops forever. Tracking every cursor seen catches any finite cycle for about the same amount of code:

let mut seen = HashSet::new();
// ...
let cursor = txs.last().map(|tx| tx.txid);
// ...
if !seen.insert(cursor) { return Err(...) }
last_seen = cursor;

Let's do this instead as it's more thorough.

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 bitcoindevkit#2339
@evanlinjin
evanlinjin force-pushed the fix/esplora-pagination-non-advancing branch from 07c0115 to 19ba4e3 Compare October 18, 2026 09:47

@evanlinjin evanlinjin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 19ba4e3

I had to update the PR description as it was too wordy.

@evanlinjin
evanlinjin merged commit 007d4e8 into bitcoindevkit:master Oct 18, 2026
19 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in BDK Chain Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bdk_esplora: per-script pagination loop never terminates when the server returns a non-advancing page

3 participants