bitcoindevkit / bitcoindevkit/bdk
Electrum outpoint sync silently ignores transaction fetch errors
- 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
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