Skip to content

docs(esplora): replace misleading tx-cache TODO with explanation - #2261

Open
Arowolokehinde wants to merge 1 commit into
bitcoindevkit:masterfrom
Arowolokehinde:fix/esplora-remove-tx-cache-todo
Open

docs(esplora): replace misleading tx-cache TODO with explanation#2261
Arowolokehinde wants to merge 1 commit into
bitcoindevkit:masterfrom
Arowolokehinde:fix/esplora-remove-tx-cache-todo

Conversation

@Arowolokehinde

@Arowolokehinde Arowolokehinde commented Aug 18, 2026

Copy link
Copy Markdown

Description

fetch_txs_with_outpoints carried a misleading // TODO: We should maintain a tx cache (like we do with Electrum). comment in both async_ext.rs and blocking_ext.rs. Electrum needs a cache because its history call returns txids only, forcing a follow-up fetch per tx. Esplora's scan endpoint already returns full tx bodies, so there's no equivalent fetch for a cache to skip — and inserted_txs already dedupes against what the spk phase fetched earlier in the same sync.

This PR replaces the TODO with a comment explaining why, and why the obvious "fix" — using get_tx_status instead of get_tx_info for cache hits — is unsafe: unknown txids return HTTP 200 {"confirmed":false}, indistinguishable from a real unconfirmed tx, so evicted transactions would never be evicted.

Fixes #2260

Notes to the reviewers

The TODO already cost a contributor a 600-line PR (#2254, closing #2250) implementing exactly this cache, which was caught in review for the get_tx_status eviction bug described above. This comment exists to stop that from being rediscovered a third time.

Changelog notice

Removed a misleading TODO in fetch_txs_with_outpoints suggesting a tx cache be added; replaced with a comment explaining why the Electrum-style cache pattern does not apply to Esplora.

Checklists

All Submissions:

@luisschwab luisschwab added the documentation Improvements or additions to documentation label Aug 18, 2026
@luisschwab luisschwab moved this to Needs Review in BDK Chain Aug 18, 2026
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.36%. Comparing base (456f9b7) to head (1aa2ece).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2261   +/-   ##
=======================================
  Coverage   78.36%   78.36%           
=======================================
  Files          30       30           
  Lines        5945     5945           
  Branches      281      281           
=======================================
  Hits         4659     4659           
  Misses       1210     1210           
  Partials       76       76           
Flag Coverage Δ
rust 78.36% <ø> (ø)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

esplora: remove misleading TODO about maintaining a tx cache

2 participants