bitcoindevkit / bitcoindevkit/bdk

Electrum: `chain_update` inserts anchor block hashes that were not checked against the tip chain

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

`chain_update` (`crates/electrum/src/bdk_electrum_client.rs`) adds a checkpoint for every anchor height missing from the tip. It prefers the hash from `latest_blocks`, but `fetch_tip_and_latest_blocks` only populates that map with the last `CHAIN_SUFFIX_LENGTH` blocks plus the heights of existing checkpoints. For any older anchor height it falls back to `anchor.block_id.hash`, which comes straight from the header the server returned for that height in `batch_fetch_anchors`.

The merkle proof only shows the transaction is included in *that* header; nothing checks that the header is actually an ancestor of the tip the same server reported. So if the server returns a header and proof for a block at height `h` that is not on its own current best chain (e.g. an inconsistent server, or a stale view around a reorg), the resulting `CheckPoint` chain contains that hash at `h` alongside the current tip, and canonicalization will treat the transaction anchored there as confirmed even though the block is not part of the chain the checkpoint claims to describe.

This issue was found by AI.

**To Reproduce**

Add to the `test` module in `crates/electrum/src/bdk_electrum_client.rs` and run `cargo test -p bdk_electrum --lib chain_update_inserts_unverified_anchor_hash`:

```rust
#[test]
fn chain_update_inserts_unverified_anchor_hash() {
let genesis_hash = constants::genesis_block(Network::Bitcoin).block_hash();
let tip_hash = constants::genesis_block(Network::Testnet).block_hash();
let orphan_hash = constants::genesis_block(Network::Regtest).block_hash();

let tip = CheckPoint::new(0, genesis_hash).insert(100, tip_hash);
// Only the tip is present in `latest_blocks`; height 50 is not.
let latest_blocks = [(100, tip_hash)].into_iter().collect();
let anchor = bdk_core::ConfirmationBlockTime {
block_id: bdk_core::BlockId { height: 50, hash: orphan_hash },
confirmation_time: 0,
};

let updated = super::chain_update(tip, &latest_blocks, [(anchor, new_tx(0).compute_txid())].into_iter()).unwrap();

// Height 50 was never cross-checked against the tip's chain, yet is now a checkpoint.
assert_eq!(updated.get(50).map(|cp| cp.hash()), None);
}
```

Current output:

```
assertion `left == right` failed
left: Some(0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206)
right: None
```

**Expected behavior**

Checkpoints derived from anchors should be consistent with the chain that `fetch_tip_and_latest_blocks` established, rather than accepting a per-height header that has not been checked against it.

**Which backend(s) are relevant (if any)?**
- [x] Electrum

Contributor guide

Open the contributing guide

Research direction

Start in crates/electrum/src/bdk_electrum_client.rs by reading chain_update, fetch_tip_and_latest_blocks, and batch_fetch_anchors. Run cargo test -p bdk_electrum --lib chain_update_inserts_unverified_anchor_hash and inspect the surrounding tests. Done means an anchor hash not verified against the established tip chain is not inserted as a checkpoint, with the test passing.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.