Electrum sync forwards unrelated history transactions and its caches never evict

[noahjoeris]

Describe the bug

populate_with_spks (crates/electrum/src/bdk_electrum_client.rs:333-344) fetches every transaction listed in a script's history and pushes it to tx_update.txs. fetch_tx only checks that the returned body matches the requested txid; nothing checks that the transaction pays to or spends from the queried script, which is what the method's doc comment describes. TxGraph::apply_update inserts all txs unconditionally, so whatever the server lists ends up in the wallet's graph and persisted changeset. With fetch_prev_txouts = true, fetch_prev_txout (lines 621-647) additionally fetches the parent of every input of every such transaction and adds it as a floating txout. populate_with_outpoints (lines 385-432) fetches history entries one by one until it finds a spend; the non-matching ones are left out of the update but stay in the cache.

The client's tx_cache, block_header_cache and anchor_cache (lines 27-31) only ever grow; there is no cap or eviction for the lifetime of a BdkElectrumClient. Together this means the amount of data downloaded and retained per sync is determined by the server's history responses rather than by the wallet's own activity. A server that lists many large, unrelated transactions for a single script makes the client download all of them, keep them in memory and forward them to the wallet.

This issue was found by AI.

To Reproduce

Add crates/electrum/tests/test_unrelated_history.rs and run cargo test -p bdk_electrum --test test_unrelated_history. It runs a stub Electrum server in-process; no real server is needed:

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 lists `tx` in the history of every script it is asked about.
fn serve_history_with(listener: TcpListener, tx: Transaction) {
    let (txid, raw_tx) = (tx.compute_txid(), consensus::encode::serialize_hex(&tx));
    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":0,"tx_hash":"{txid}"}}]"#)
            } else if request.contains("blockchain.transaction.get") {
                format!(r#""{raw_tx}""#)
            } else {
                panic!("unexpected request: {request}");
            };
            writeln!(stream, r#"{{"jsonrpc":"2.0","id":{id},"result":{result}}}"#).unwrap();
        }
    }
}

#[test]
fn sync_ignores_history_entries_unrelated_to_the_script() {
    let wallet_spk = ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([1; 20]));
    let unrelated_tx = 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: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([2; 20])),
        }],
    };
    let unrelated_txid = unrelated_tx.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_history_with(listener, unrelated_tx));

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

    let txids: Vec<_> = response.tx_update.txs.iter().map(|tx| tx.compute_txid()).collect();
    assert!(!txids.contains(&unrelated_txid), "update contains unrelated tx {unrelated_txid}");
}

The assertion fails with update contains unrelated tx ad20c55e…: the transaction neither pays to the requested script nor spends from it, yet it is part of the sync update.

Expected behavior

The update should only contain transactions that involve the requested scripts or outpoints, and the data the client downloads and retains should not grow without bound based on what the server lists.

Related: #2295 (scan termination also depends only on the server's history responses).