Update block Emitter#2244
Open
ValuedMammal wants to merge 7 commits into
Open
Conversation
bitcoincore-rpc is effectively unmaintained. Swap it out for bdk_bitcoind_client (bitreq backend + corepc_types), which is maintained under the bitcoindevkit org. The Emitter and FilterIter now borrow a concrete `&Client` instead of being generic over `C: Deref<Target: RpcApi>`, since the new client does not implement the old RpcApi trait. All RPC call sites and response types are migrated to the new client while preserving emission logic unchanged. Core v28 is the default (`28_0`); `29_0`/`30_0` passthrough features let callers target newer nodes.
The Emitter struct, its impl, MempoolEvent, BlockEvent, PollResponse, the poll/poll_once helpers, NO_EXPECTED_MEMPOOL_TXS, and the inline test module are relocated from lib.rs into a new src/emitter.rs. lib.rs now declares `mod emitter` and re-exports its public items, keeping the crate's public API unchanged.
Return a bounded `EmitterError::AgreementNotFound` instead of silently resetting to the genesis checkpoint when no agreement block is found. This surfaces the wrong-network case (e.g. a testnet checkpoint polled against a mainnet node) to the caller rather than resetting emission to genesis. Agreement failures are bounded by `MAX_AGREEMENT_FAILURES` (3). Other changes: - Extract private `Emitter::raw_mempool()` for the tip-consistent mempool snapshot, replacing the mut-before-loop pattern. - Fix `BlockEvent::connected_to()` to derive the connected block from `(height - 1, header.prev_blockhash)` instead of returning itself when there is no previous checkpoint. - Convert `start_height` from a constructor parameter into a builder method. `Emitter::new` is simplified by using last_cp.height() as the default start height. - Introde `EmitterError` and use it in the definition of `next_block` and mempool methods. - Remove NO_EXPECTED_MEMPOOL_TXS, which restricted type inference at the call site.
…ADME
Rewrite the crate README with a usage overview and a runnable example, and
wire it in as the crate-level docs via `#![doc = include_str!("../README.md")]`.
Correct the mempool timestamp semantics: `MempoolEvent::update`/`evicted` are
timestamped with the `sync_time` of the call, not the time a transaction was
first seen or evicted. Document that this is a full snapshot, not a delta, and
that evictions are only reported once the emitter is at tip.
Tidy the poll state-machine docs (`PollResponse`, `poll_once`, `poll`),
`next_block`, and `connected_to`, and trim redundant field comments so the
code stays self-describing. Documentation only; no behavior change.
Rewrite the emitter integration tests with behavior-focused names and expanded coverage. Each test now documents the scenario it exercises: - blocks_emitted_in_order_and_after_reorg - unconfirmed_txs_anchored_on_confirmation - ensure_block_emitted_after_reorg_is_at_reorg_height - tx_can_become_unconfirmed_after_reorg - mempool_update_is_complete_snapshot - reorg_past_start_checkpoint - double_spent_tx_evicted_and_removed_from_canonical_set - detect_new_mempool_txs - birthday_checkpoint_skips_earlier_blocks - mempool_at_uses_provided_timestamp - start_height_reset_on_reorg_prevents_height_gaps - evictions_withheld_until_at_tip - start_height_beyond_tip_returns_rpc_error - wrong_genesis_returns_agreement_not_found New coverage includes behind-tip eviction withholding (live and wallet-restart paths), the start_height birthday guarantee, custom mempool_at sync_time semantics, and the recoverable EmitterError cases (Rpc for an out-of-range start_height, AgreementNotFound for a wrong genesis hash).
Introduce a generic block-data type parameter B on Emitter and FilterIter so callers can thread full Headers through emissions, not just BlockHash. - emitter: Emitter<'a> is now Emitter<'a, B> and BlockEvent is BlockEvent<B>. Added B: From<Header> to existing generic bounds. - bip158: FilterIter<'a> and Event are now FilterIter<'a, B> and Event<B> with the same bound on its Iterator impl. A private `as_header` helper reconstructs a bitcoin Header from GetBlockHeaderVerbose so the emitted checkpoint carries the header. - bip158: Error is now #[non_exhaustive] - test: Add `emitter_collects_header_checkpoints` to exercise Emitter with CheckPoint<Header>
The `download` feature now enables electrsd/bitcoind_30_2. This brings the downloaded bitcoind version in line with the latest version feature in bdk_bitcoind_rpc. electrsd 0.41.0 throws a compile error if no electrs version is set, causing compilation of bdk_core to fail when no testenv default features are set. We can't specify an electrs version directly as a feature of electrsd because that breaks docs.rs which has no network access when building the docs. Instead enable bdk_testenv default features in crates/core/Cargo.toml to ensure a version of electrs is set. However it's valid to note that since bdk_core only uses the test macros (not TestEnv), another option is to make electrsd an optional dependency in bdk_testenv, explicitly enabled by the download feature. That way crates like bdk_core can use bdk_testenv without pulling in the extra download dependencies.
ValuedMammal
requested review from
evanlinjin,
luisschwab,
oleonardolima and
tvpeter
as code owners
July 13, 2026 23:52
ValuedMammal
added
dependencies
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2244 +/- ##
==========================================
+ Coverage 78.32% 78.60% +0.27%
==========================================
Files 30 31 +1
Lines 5934 5922 -12
Branches 281 276 -5
==========================================
+ Hits 4648 4655 +7
+ Misses 1210 1192 -18
+ Partials 76 75 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tvpeter
reviewed
Jul 18, 2026
tvpeter
left a comment
tvpeter
left a comment
Collaborator
There was a problem hiding this comment.
Thank you ValuedMammal for working on this.
On my comment regarding the AgreementNotFound arm, my understanding is that the emitter immediately retry the node with identical request (last_cp hash) and may likely return the same outcome for all three calls, so it might be more efficient to immediately return EmitterError::AgreementNotFound immediately upon the first failure.
Also, I have checked part of this PR that replaces bitcoincore-rpc with bdk_bitcoind_client and it covers everything with the latest release, so I will be closing #2119 in favour of this PR.
Thank you.
| if self.agreement_failure_count >= MAX_AGREEMENT_FAILURES { | ||
| return Err(EmitterError::AgreementNotFound); | ||
| } | ||
| self.last_block = None; |
Collaborator
There was a problem hiding this comment.
I think for the loop to execute the AgreementNotFound arm, the self.last_block was already None, so resetting it to None does not change the outcome of the next call.
| if let Some(last_block_info) = &self.last_block { | ||
| let next_hash = if last_block_info.height + 1 < self.start_height { | ||
| // enforce start height | ||
| self.client.get_block_hash(self.start_height)? |
Collaborator
There was a problem hiding this comment.
I think the removed check on whether the Emitter is connected to the Best Chain is not covered by the block confirmations check. Can you please verify this?
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replace
bitcoincore-rpcwithbdk_bitcoind_client-EmitterandFilterIternow borrow a concrete&bdk_bitcoind_client::Clientinstead of being generic overC: Deref<Target: RpcApi>. Core v28 is the default target; passthrough features (29_0,30_0) let callers target newer nodes.Generic
CheckPoint<B>inEmitterandFilterIter- Both types now carry a block-data type parameterB: From<Header>. This lets callers thread fullHeaders through emissions rather than onlyBlockHashes.BlockEvent<B>andbip158::Event<B>follow the same bound. Anas_headerhelper inbip158.rsreconstructs aHeaderfromGetBlockHeaderVerbosefor the emitted checkpoint.Tidy poll loop and
EmitterAPI:EmitterErroris introduced;next_blockand mempool methods now return itEmitterError::AgreementNotFoundsurfaces the wrong-network case instead of silently replacing the passed-in checkpoint with a height-0 genesis checkpoint (bounded byMAX_AGREEMENT_FAILURES = 3)start_heightis converted from a constructor parameter to a builder method;Emitter::newdefaults tolast_cp.height()BlockEvent::connected_to()is fixed to derive from(height - 1, prev_blockhash)when no previous checkpoint existsraw_mempool()extracted as a private helper for tip-consistent snapshotsNO_EXPECTED_MEMPOOL_TXSremoved (was hurting type inference at call sites)Documentation — README rewritten with a usage overview and a runnable example, wired in as crate-level docs via
#![doc = include_str!("../README.md")]. Poll state-machine docs (PollResponse,poll_once,poll),next_block, andconnected_toare tidied throughout.Test rewrite —
test_emitter.rsrewritten with 14 behavior-focused tests covering: block ordering, reorgs, mempool snapshots, eviction withholding behind tip, birthday checkpoint,start_heightsemantics,AgreementNotFoundon wrong genesis, and out-of-boundsstart_height.electrsdbumped to0.41.0inbdk_coreandbdk_testenv.Fixes #2176
Notes to the reviewers
The
bitcoincore-rpc→bdk_bitcoind_clientswap is the most structurally impactful change. The emission logic itself is unchanged — only the RPC call sites and response types are migrated. The generics onEmitter<'a, B>/FilterIter<'a, B>are purely additive: existing callers usingCheckPoint<BlockHash>compile without changes (asBlockHash: From<Header>is already implemented).The
AgreementNotFounderror is a deliberate behavior change — previously a sync against a wrong network would silently replace the caller's genesis checkpoint.Changelog notice
bdk_bitcoind_rpcbitcoincore-rpcdependency withbdk_bitcoind_client;EmitterandFilterIternow borrow&bdk_bitcoind_client::ClientdirectlyEmitteris nowEmitter<'a, B>andBlockEventisBlockEvent<B>whereB: From<bitcoin::block::Header>; same forFilterIter<'a, B>andEvent<B>start_heightmoved fromEmitter::newto a new builder methodEmitter::start_heightEmitterErrorenum returned bynext_block,mempool, andmempool_atEmitterError::AgreementNotFoundsurfaces wrong-network / no-common-ancestor failures instead of silently replacing the checkpoint with genesisBlockEvent::connected_to()now correctly derives(height - 1, prev_blockhash)when no previous checkpoint existsNO_EXPECTED_MEMPOOL_TXSconstantbdk_core/bdk_testenvelectrsdto0.41.0Checklists
All Submissions:
New Features:
Bugfixes: