bitcoindevkit / bitcoindevkit/bdk
esplora: remove misleading `TODO` about maintaining a tx cache
- Dominant language
- Rust
- Stars
- 1.1k
- Forks
- 483
- Avg merge
- 20d 3h
- Merged PRs (30d)
- 3
Description
`fetch_txs_with_outpoints` carries this comment in both backends:
- `crates/esplora/src/async_ext.rs`
- `crates/esplora/src/blocking_ext.rs`
```rust
// TODO: We should maintain a tx cache (like we do with Electrum).
```
It should be removed. It invites a port of `BdkElectrumClient`'s cache that cannot pay off here, and it already cost a contributor a 600-line PR (#2254, #2250).
**Why the Electrum pattern doesn't transfer**
Electrum's cache exists because `blockchain.scripthash.get_history` returns *txids only*, forcing a follow-up `blockchain.transaction.get` per transaction. The cache eliminates that second call.
Esplora's `GET /scripthash/:hash/txs` returns full transactions — `esplora_client::Tx` carries `vin`/`vout` and `to_tx()` reconstructs the `Transaction` from the response body. There is no second call, so there is nothing for a tx cache to skip. A cache could only ever apply to the `iter_txids`/`iter_outpoints` paths, and those share `inserted_txs` with the spk phase that runs first — so in a `SyncRequest` carrying spks they are almost always already covered.
Substituting `get_tx_status` for `get_tx_info` is additionally unsafe: Esplora returns HTTP 200 `{"confirmed":false}` for an unknown txid, indistinguishable from a genuinely unconfirmed tx, so evicted transactions would have `last_seen` bumped on every sync and never be evicted.
**Suggested replacement**
Delete the TODO, or replace it with a note recording why a tx cache does *not* apply to Esplora, so this doesn't get rediscovered a third time.
If someone wants to optimize this function, the real target is the per-outpoint `get_output_status` round trips — those are not covered by the spk scan.
Contributor guide
Research direction
Open crates/esplora/src/async_ext.rs and crates/esplora/src/blocking_ext.rs and find the TODO about maintaining a transaction cache. Remove the misleading comment, or replace it with a concise explanation of why that cache does not apply to Esplora; verify both occurrences are handled and run the relevant Esplora tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 92/100