bitcoindevkit / bitcoindevkit/bdk

Electrum sync leaves a transaction without anchor or `seen_at` when its merkle proof fails

Open
#2,304 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**

In `populate_with_spks`, a history entry with `height > 0` only queues the transaction for `batch_fetch_anchors` and does not record a `seen_at` (`crates/electrum/src/bdk_electrum_client.rs:335-343`). If the merkle proof then fails to validate against the fetched header, also after the one retry, `batch_fetch_anchors` simply produces no anchor (lines 573-613). The transaction still ends up in `tx_update.txs`, but with neither an anchor nor a `seen_at`, so `TxGraph::apply_update` stores it without temporal context and it is never canonical.

For a transaction the wallet has not seen before, the server reports it as confirmed yet the wallet shows nothing: no confirmed or pending balance, no entry in the transaction list and no error. This lasts as long as the proof keeps failing. #1508 settled on not returning an error for a failed proof because a stale header can cause one legitimately; in that situation the transaction is currently hidden entirely rather than, for example, treated as unconfirmed until the proof validates.

This issue was found by AI.

**To Reproduce**

Add `crates/electrum/tests/test_bad_proof.rs` and run `cargo test -p bdk_electrum --test test_bad_proof`. It runs a stub Electrum server in-process that lists the transaction at height 100 and serves a merkle proof that does not match the header it serves:

```rust
use std::io::{BufRead, BufReader, Write};
use std::net::TcpListener;

use bdk_chain::bitcoin::{
absolute, consensus, hashes::Hash, transaction, Amount, ScriptBuf, Transaction, TxIn, TxOut,
WPubkeyHash,
};
use bdk_chain::spk_client::SyncRequest;
use bdk_electrum::electrum_client::{Client, ConfigBuilder};
use bdk_electrum::BdkElectrumClient;

/// Electrum stub that reports `tx` as confirmed at height 100 but serves a merkle proof that does
/// not match the block header it serves.
fn serve_confirmed_with_bad_proof(listener: TcpListener, tx: Transaction) {
let (txid, raw_tx) = (tx.compute_txid(), consensus::encode::serialize_hex(&tx));
let raw_header = "00".repeat(80);
for stream in listener.incoming() {
let mut stream = stream.unwrap();
for request in BufReader::new(stream.try_clone().unwrap()).lines() {
let request = request.unwrap();
let id_start = request.find("\"id\":").unwrap() + 5;
let id: String = request[id_start..].chars().take_while(char::is_ascii_digit).collect();
let result = if request.contains("blockchain.scripthash.get_history") {
format!(r#"[{{"height":100,"tx_hash":"{txid}"}}]"#)
} else if request.contains("blockchain.transaction.get_merkle") {
r#"{"block_height":100,"pos":0,"merkle":[]}"#.to_string()
} else if request.contains("blockchain.transaction.get") {
format!(r#""{raw_tx}""#)
} else if request.contains("blockchain.block.header") {
format!(r#""{raw_header}""#)
} else {
panic!("unexpected request: {request}");
};
writeln!(stream, r#"{{"jsonrpc":"2.0","id":{id},"result":{result}}}"#).unwrap();
}
}
}

#[test]
fn confirmed_tx_with_failing_proof_keeps_temporal_context() {
let wallet_spk = ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([1; 20]));
let payment = Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::ZERO,
input: vec![TxIn::default()],
output: vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: wallet_spk.clone() }],
};
let txid = payment.compute_txid();

let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let url = format!("tcp://{}", listener.local_addr().unwrap());
std::thread::spawn(move || serve_confirmed_with_bad_proof(listener, payment));

let config = ConfigBuilder::new().retry(0).build();
let client = BdkElectrumClient::new(Client::from_config(&url, config).unwrap());
let response = client.sync(SyncRequest::builder().spks([wallet_spk]), 1, false).unwrap();
let update = response.tx_update;

assert!(update.txs.iter().any(|tx| tx.compute_txid() == txid), "tx not in update");
let anchored = update.anchors.iter().any(|(_, t)| *t == txid);
let seen = update.seen_ats.iter().any(|(t, _)| *t == txid);
assert!(anchored || seen, "tx {txid} has neither an anchor nor a seen_at");
}
```

The last assertion fails with `tx 267ca521… has neither an anchor nor a seen_at`.

**Expected behavior**

A transaction that the server lists in a script's history should not silently end up without any temporal context because its merkle proof could not be validated.

Contributor guide

Open the contributing guide

Research direction

Start in crates/electrum/src/bdk_electrum_client.rs:335-343 and 573-613, then run the reproducer in crates/electrum/tests/test_bad_proof.rs with cargo test -p bdk_electrum --test test_bad_proof. Trace how a failed merkle proof is represented in tx_update. Done means the listed transaction retains either an anchor or a seen_at, and the test assertion passes.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.