Skip to content

refactor(testenv)!: make wait_until_electrum_sees_block concrete#21

Open
evanlinjin wants to merge 1 commit into
masterfrom
claude/serene-bell-dncr82
Open

refactor(testenv)!: make wait_until_electrum_sees_block concrete#21
evanlinjin wants to merge 1 commit into
masterfrom
claude/serene-bell-dncr82

Conversation

@evanlinjin

Copy link
Copy Markdown
Owner

Description

A from-scratch reimplementation of bitcoindevkit#1640 on top of the current master.

TestEnv::wait_until_electrum_sees_block previously only took a timeout and returned as soon as Electrum emitted any new-block notification. That made it ambiguous which block was actually being waited for, and it could return before Electrum had caught up to the block the test cared about.

This PR makes the wait concrete:

  • wait_until_electrum_sees_block now takes block_height: usize and block_hash: Option<BlockHash>, so callers state exactly which block Electrum must be aware of. If the notified tip has already advanced past block_height, the header at that height is fetched explicitly so the expected block_hash can still be verified.
  • A new convenience wrapper wait_until_electrum_tip_syncs_with_bitcoind waits until Electrum's tip matches bitcoind's chain tip (both height and hash). All existing call sites are migrated to it, since in every case the intent was "wait until Electrum catches up to the freshly mined/reorged tip".

Notes to the reviewers

  • Why subscribe instead of polling: polling Electrs for the header at block_height and checking its hash does not guarantee the spk histories are current. Receiving a block-tip notification does, because of how Electrs indexes. This is captured in a NOTE in the code.
  • Tip advanced past target: the original PR's reviewers asked what happens when the method is called after the chain tip has moved past the target height. This is handled explicitly: when the notified height is greater than block_height, the header at block_height is fetched directly to verify the hash, rather than relying on the notification header.
  • trigger() placement: electrsd.trigger() is kept inside the polling loop (matching the previous master behaviour) so Electrs keeps getting nudged to re-index — this is important for reorgs, where the tip changes without a height change.
  • The previous WIP artefacts from Refactor TestEnv::wait_until_electrum_sees_block to be more concrete. bitcoindevkit/bdk#1640 (a // TODO: Fails here. marker and a struck-through doc comment) are intentionally left out.

Changelog notice

  • Changed: TestEnv::wait_until_electrum_sees_block now takes a block_height and an optional block_hash (breaking change to bdk_testenv).
  • Added: TestEnv::wait_until_electrum_tip_syncs_with_bitcoind.

Checklists

All Submissions:

New Features:

  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API

https://claude.ai/code/session_016gJvi98Q65QnanBFxe2UUn


Generated by Claude Code

@evanlinjin evanlinjin closed this Jun 15, 2026
@evanlinjin evanlinjin reopened this Jun 15, 2026
Previously `TestEnv::wait_until_electrum_sees_block` only took a `timeout`
and returned as soon as Electrum emitted any new-block notification, so it
was not clear which block was actually being waited for.

Add `block_height` and an optional `block_hash` so callers state exactly
which block Electrum must be aware of. When the notified tip has already
advanced past `block_height`, the header at that height is fetched
explicitly so the expected `block_hash` can still be verified.

Introduce `TestEnv::wait_until_electrum_tip_syncs_with_bitcoind`, a
convenience wrapper that waits until Electrum's tip matches bitcoind's
chain tip (both height and hash), and migrate all existing call sites to
it.

https://claude.ai/code/session_016gJvi98Q65QnanBFxe2UUn
@evanlinjin evanlinjin force-pushed the claude/serene-bell-dncr82 branch from 0115d93 to f6a60a3 Compare June 15, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants