Skip to content

feat(esplora): add persistent tx cache to avoid re-fetching known txs - #2254

Closed
GuTS805 wants to merge 1 commit into
bitcoindevkit:masterfrom
GuTS805:fix/esplora-tx-cache
Closed

feat(esplora): add persistent tx cache to avoid re-fetching known txs#2254
GuTS805 wants to merge 1 commit into
bitcoindevkit:masterfrom
GuTS805:fix/esplora-tx-cache

Conversation

@GuTS805

@GuTS805 GuTS805 commented Aug 5, 2026

Copy link
Copy Markdown

Adds BdkEsploraClient wrapper (mirroring BdkElectrumClient) with an internal tx_cache and populate_tx_cache(). Cached txids now only trigger a lightweight get_tx_status() call instead of re-downloading the full transaction body via get_tx_info().

Fixes #2250

Description

bdk_esplora re-downloaded the full transaction body (get_tx_info) for every tracked txid on every sync, even when the tx was already fetched in a previous sync and only its confirmation status could have changed. bdk_electrum avoids this via BdkElectrumClient's persistent tx_cache; bdk_esplora had no equivalent (see the TODO in async_ext.rs:495 / blocking_ext.rs:454).

This PR adds BdkEsploraClient<C>, a wrapper around esplora_client::BlockingClient / AsyncClient that maintains a persistent in-memory tx_cache, mirroring BdkElectrumClient's pattern:

  • BdkEsploraClient::new(client) and populate_tx_cache() to pre-seed the cache from an existing TxGraph.
  • BdkEsploraClient::full_scan / sync inherent methods that use the cache.
  • For txids already in the cache, only a lightweight get_tx_status() call is made instead of re-downloading the full transaction via get_tx_info(). Only genuinely new/unseen txids trigger a full fetch.

The existing EsploraExt / EsploraAsyncExt traits on the raw esplora_client::BlockingClient / AsyncClient are unchanged, so this is fully backward compatible — the cache is opt-in via the new wrapper.

Notes to the reviewers

Couldn't run the bdk_testenv-based integration tests locally (Windows) — electrsd's build script requires std::os::unix, which doesn't exist on this platform. Confirmed this is a pre-existing limitation unrelated to this change (reproduces identically on master). Verified with cargo check --all-features and cargo clippy instead; CI should run the integration tests.

Changelog notice

Added: bdk_esplora: introduce BdkEsploraClient wrapper with a persistent transaction cache to avoid re-fetching already-downloaded transactions on sync.

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

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

Adds BdkEsploraClient wrapper (mirroring BdkElectrumClient) with an
internal tx_cache and populate_tx_cache(). Cached txids now only
trigger a lightweight get_tx_status() call instead of re-downloading
the full transaction body via get_tx_info().

Fixes bitcoindevkit#2250
@evanlinjin

Copy link
Copy Markdown
Member

Thanks for taking this on, and for the detailed writeup in both #2250 and here. Unfortunately I don't think this approach works, and I'd like to explain why in enough detail to be useful — the premise in #2250 is the part that needs revisiting, so this isn't really a matter of fixing up the patch.

The cache can't save what Esplora already sent

bdk_electrum's cache pays for itself because Electrum's blockchain.scripthash.get_history returns txids only, forcing a follow-up blockchain.transaction.get per transaction. The cache eliminates that second call.

Esplora has no equivalent second call. GET /scripthash/:hash/txs returns full transactions — esplora_client::Tx carries vin/vout, and to_tx() reconstructs the Transaction straight from the response body. The full tx is on the wire whether or not we already have it.

That shows up directly in the diff: the cache is written in fetch_txs_with_keychain_spks but never read there. Across both files there is exactly one read site — fetch_txs_with_txids (async_ext.rs:602, blocking_ext.rs:551).

Path Endpoint Cache read? Saved
full_scan (all of it) scripthash_txs no nothing
sync, spk phase scripthash_txs no nothing
sync, txid/outpoint phase get_tx_infoget_tx_status yes response bytes only

No round trips are saved anywhere. get_tx_status is still one HTTP GET per txid — identical request count to get_tx_info, just a smaller response.

And the remaining bandwidth win mostly doesn't materialize either. sync runs fetch_txs_with_spks first and shares inserted_txs; fetch_txs_with_txids skips anything already in that set. A wallet's iter_txids (unconfirmed wallet txs) and iter_outpoints (wallet UTXOs) are by construction in the history of the spks being scanned in the same call, so they've already been fetched in full and never reach the cached branch. The cache only fires for txids the spk scan missed — which for a SyncRequest carrying spks is close to none.

Worth noting the new test uses a txids-only SyncRequest with no spks, which isn't a shape wallet syncs produce, and it asserts nothing about request count or bytes.

get_tx_status silently resurrects dropped transactions

This is the part that concerns me most, and it's a correctness regression rather than a perf question.

get_tx_status uses get_response_json, not get_opt_response_json. I checked what Esplora actually returns for an unknown txid:

$ curl -s -o /dev/null -w "%{http_code}\n" https://blockstream.info/api/tx/<unknown>/status
200
$ curl -s https://blockstream.info/api/tx/<unknown>/status
{"confirmed":false}

HTTP 200 with {"confirmed":false}indistinguishable from a genuinely unconfirmed transaction. So insert_anchor_or_seen_at_from_status takes the else branch and inserts (txid, start_time) into seen_ats, and the cached body gets pushed into update.txs.

The pre-PR path returns Ok(None) on 404 from get_tx_info and inserts nothing.

Net effect: once a transaction is in the cache, it can never be evicted. Its last_seen is bumped to start_time on every single sync, forever, and it is re-inserted into the graph as a live unconfirmed tx. The txid that misses the spk scan is precisely the evicted one — so the only case where the cached branch has real work to do is the case it now gets wrong.

I reproduced this against a real Esplora instance with a transaction that was never broadcast:

let phantom = Transaction {
    version: transaction::Version::TWO,
    lock_time: absolute::LockTime::ZERO,
    input: vec![],
    output: vec![TxOut::NULL],
};
let phantom_txid = phantom.compute_txid();
assert!(client.inner.get_tx_info(&phantom_txid)?.is_none()); // esplora does not know it

client.populate_tx_cache([phantom.clone()]);
let resp = client.sync(SyncRequest::<()>::builder().txids([phantom_txid]).build(), 1)?;
txs in update:      1
seen_ats in update: {(b22593096dd416d52858c49c1778b15cf11d27e078bf2ce9a64a6917f4b233a9, 1786954489)}

A transaction that exists nowhere is reported as seen. This runs directly against the eviction-correctness work in #2240 / TxNode::is_evicted.

insert_prevouts is dropped on the cached path

The cached branch never calls insert_prevouts, so floating prevout TxOuts are missing from the update. Fee calculation degrades to None for any tx whose prevouts aren't already in the graph — reachable via populate_tx_cache from a source that doesn't carry them.

The added tests don't compile

Both new tests fail to build on this branch:

error[E0282]: type annotations needed for `SyncRequest<_>`
    --> crates/esplora/src/blocking_ext.rs:1170:13
error[E0283]: type annotations needed for `SyncRequest<_>`
    --> crates/esplora/src/async_ext.rs:1015:13
error: could not compile `bdk_esplora` (lib test) due to 2 previous errors

SyncRequest::builder() leaves I unconstrained; it needs SyncRequest::<()>::builder(). I understand the integration tests couldn't run on Windows — but this is a plain cargo test --no-run failure, and cargo check/clippy won't surface it because neither builds the test harness. Worth adding cargo test --no-run to your local loop.

On the TODO

The // TODO: We should maintain a tx cache (like we do with Electrum) comment in fetch_txs_with_outpoints is real and I don't blame you for taking it at face value — it's ours, and it's misleading. I'll open a separate issue to remove it. If there's a caching win in the Esplora backend it isn't this one; it would have to cut requests the spk scan doesn't already make.

Outcome

Closing this one, since the fix isn't a revision of the patch — the caching strategy doesn't transfer from Electrum to Esplora. #2250 should be closed alongside it for the same reason.

Genuinely, thanks for the effort here; the writeup quality was high and the investigation was easy to follow, which is what made it quick to check. If you're interested in Esplora sync performance, the more promising direction is reducing the get_output_status calls in fetch_txs_with_outpoints — those are per-outpoint round trips that the spk scan really doesn't cover.

@evanlinjin

Copy link
Copy Markdown
Member

Follow-up on the TODO: opened #2260 to remove it.

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bdk_esplora: no transaction cache, re-fetches full tx body on every sync

2 participants