fix(bdk_electrum_streaming): Keep the highest last active index per notification

[evanlinjin]

Context

Follow-up on #3 ("Fix off-by-one error in last_active_index emission").

The off-by-one from #3 is already fixed. At the time the issue was filed, handle_script_status returned this_index + 1 and that value was emitted directly as the keychain's last active index. Commit 4447f17 ("fix: Off-by-one last_active_indices update", May 2025) changed it to return this_index, and every emission site on main today reports the index of the spk that actually has history. The first commit here adds a state-level regression test that pins that behaviour so it cannot silently regress.

While covering it, I found a live bug with the same consequence (an active spk never gets revealed) — not an off-by-one, and not the one this PR originally described.

This PR was rebased onto main after the "Give the chain and anchors one owner" refactor (#13) and the State::start refactor (#17) landed. Those refactors moved last_active_indices bookkeeping from advance_spk_jobs/an inline notification handler into a unified on_spk_status + poll_spk_jobs. The original bug this PR fixed (advance_spk_jobs folding several job completions into one update via BTreeMap::extend, iterated in script-hash order) no longer exists in that form — but the refactor introduced a different bug with the same symptom, described below.

The bug on current main

on_spk_status stages a keychain's last active index with a plain BTreeMap::insert(k, i) whenever a script hash notification (or subscribe response) names history. Electrum notifies each subscribed script hash independently, in an order unrelated to derivation index — a later-derived spk can be notified before an earlier one. The last notification processed wins, so the staged index can end up lower than the highest spk that actually has history. That index is what reveal_to_target_multi reveals to, so the higher spk stays unrevealed and the wallet does not recognise its txouts as its own.

Separately, poll_spk_jobs had its own write to the same field, derived from mere job completion rather than from a notification naming history. That made it fire for every finished spk job, active or not — including every empty look-ahead spk subscribed alongside the active ones, which is the common case. on_spk_status already stages the index for every spk that actually has history, before its job even runs, so this second write was both redundant for active spks and actively wrong for inactive ones (it would tag an empty look-ahead spk as "active" at whatever index it happened to be, clobbering the real one). Removed.

Changes

test(bdk_electrum_streaming): Cover last_active_indices emission

Adds two state-level tests:

• last_active_index_is_index_of_active_spk: pins the off-by-one fix (passes on main).
• last_active_index_is_highest_regardless_of_notification_order: drives a sync where two spks of the same keychain (indices 3 and 4) each have history, and delivers their script-hash notifications with the higher index notified first. Fails on main with left: [("external", 3)], right: [("external", 4)].

fix(bdk_electrum_streaming): Keep the highest last active index per notification

Makes on_spk_status's insert a max-merge instead of an overwrite, and removes the redundant/harmful write in poll_spk_jobs. Passes with both tests above.

Testing

cargo fmt --check, cargo clippy --lib --tests -D warnings, and cargo test (lib + tests/state.rs) all pass.

tests/env.rs could not be run here: its bdk_testenv dev-dependency builds bitcoind, whose build script downloads Bitcoin Core from bitcoincore.org, which this sandbox's egress proxy blocks. That file is untouched by this PR, but it is worth a CI run.

---

Generated by Claude Code