bitcoindevkit / bitcoindevkit/bdk

Electrum outpoint sync silently ignores transaction fetch errors

Open Beginner friendly
#2,293 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
1.1k
Forks
483
Avg merge
20d 3h
Merged PRs (30d)
3

Description

**Describe the bug**

`BdkElectrumClient::sync` discards every error returned by `fetch_tx` while collecting the requested outpoints (`crates/electrum/src/bdk_electrum_client.rs:362-368`):

```rust
for op in outpoints {
if let Ok(tx) = self.fetch_tx(op.txid) {
```

Only a "transaction not found" response (`Error::Protocol`) means there is nothing to sync for that outpoint. Connection, TLS and timeout errors are swallowed too, so the outpoint is skipped and `sync` reports success without any data for it. A spend of that outpoint is missed and the wallet keeps treating the UTXO as unspent until a later sync happens to succeed.

The txid path (`populate_with_txids`) treats the same situation differently: it skips only on `Error::Protocol(_)` and propagates every other error.

This issue was found by AI.

**To Reproduce**

Add `crates/electrum/tests/test_outpoint_fetch_errors.rs` and run `cargo test -p bdk_electrum --test test_outpoint_fetch_errors`. No Electrum server is needed:

```rust
use std::net::TcpListener;

use bdk_chain::bitcoin::{hashes::Hash, OutPoint, Txid};
use bdk_chain::spk_client::SyncRequest;
use bdk_electrum::electrum_client::{Client, ConfigBuilder};
use bdk_electrum::BdkElectrumClient;

#[test]
fn outpoint_sync_reports_connection_errors() {
// A "server" that accepts the connection and immediately closes it.
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let url = format!("tcp://{}", listener.local_addr().unwrap());
std::thread::spawn(move || listener.incoming().for_each(drop));

let config = ConfigBuilder::new().retry(0).build();
let client = BdkElectrumClient::new(Client::from_config(&url, config).unwrap());
let outpoint = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
let request = SyncRequest::<()>::builder_at(0).outpoints([outpoint]);

let response = client.sync(request, 1, false);
assert!(response.is_err(), "sync dropped the outpoint and returned {response:?}");
}
```

The assertion fails with `Ok(SyncResponse { tx_update: TxUpdate { txs: [], .. }, chain_update: None })`. The same request built with `.txids([outpoint.txid])` instead of `.outpoints(..)` returns `Err(AllAttemptsErrored(..))`.

**Expected behavior**

A `sync` that could not fetch the transaction of a requested outpoint because of a network error should not be reported as successful. The outpoint and txid paths should handle fetch errors consistently.

Contributor guide

Open the contributing guide

Research direction

Start in crates/electrum/src/bdk_electrum_client.rs:362-368 and compare the outpoint handling with populate_with_txids, which already distinguishes protocol errors from other fetch failures. Run crates/electrum/tests/test_outpoint_fetch_errors.rs with cargo test -p bdk_electrum --test test_outpoint_fetch_errors. Done means network errors from requested outpoints are reported by sync while transaction-not-found responses remain skippable.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.