Refactor `ChainQuery` to be more flexible by evanlinjin · Pull Request #2139 · bitcoindevkit/bdk

evanlinjin · 2026-03-09T15:20:14Z

Description

This PR introduces median-time-past (MTP) on canonical views. MTP needs block timestamps, and the existing task (bdk_core::ChainQuery) interface cannot expose them.

This PR changes the task (bdk_core::ChainQuery) interface to request block data by height, with the response now a generic B. A driver such as LocalChain can return Headers (which carry timestamps), and it resolves every requested height rather than collapsing the request to a single answer (as MTP needs all the blocks in a window, not just one). Previously the request was Vec<BlockId> and the response a single ChainResponse = Option<BlockId>: only the first in-chain block among the candidates.

The new response shape could have been done by keeping next_query() -> Option<…>, but the multi-stage canonicalization is much cleaner when the driving call returns a richer TaskProgress enum. This PR reshapes the protocol to a poll-based ChainTask whose TaskProgress expresses states next_query couldn't:

TaskProgress::Blocked signals the task has issued every request it can but still needs outstanding data before it can finish. This is the hook a future streaming (non-blocking) driver would use to know when to wait.
TaskProgress::Advanced lets a task write poll as one unit of work per call instead of wrapping its whole implementation in an internal loop. It also lets a driver observe progress in finer-grained increments.

Canonical views can then compute MTP per transaction (CanonicalTx::mtp) and for the tip (Canonical::tip_mtp), opted into via with_mtp() so callers who don't need it pay nothing.

Notes to the reviewers

Naming. In the current master, LocalChain::canonicalize is the generic task runner and LocalChain::canonical_view is the high-level helper. This naming is confusing. In this PR, the runner becomes run_task and the helper becomes canonicalize.
Relationship to CheckPoint::median_time_past. This does not replace that method. CheckPoint::median_time_past computes MTP for a height directly from the in-memory LocalChain checkpoint you already hold. Canonical-view computes MTP by sourcing its blocks through ChainTask / BlockQueries so it is driver-agnostic (works for any chain source, not just LocalChain<Header>) and attaches the result to each CanonicalTx and the tip, rather than making the caller walk checkpoints per height.
TaskProgress::Blocked has no in-tree consumer yet — this is intentional. It exists for a future streaming driver (e.g. a Bitcoin Core RPC driver) that issues fetches without blocking: Query says "launch I/O", Blocked says "block until some lands". The only current driver (run_task) is synchronous and treats Blocked as unreachable!(). A comment in CanonicalTask documents the intended "anchored-leaves" batching optimization for the future driver.
The shared anchor-checking logic used by both canonicalization phases is extracted into BlockQueries::resolve_candidates (returning BlockCandidateResolution), decoupled from Anchor by taking the candidates as (payload, BlockId) pairs.

Changelog notice

bdk_core

Changed: the ChainQuery trait is reshaped into a poll-based ChainTask trait with a TaskProgress enum (Advanced / Query / Blocked / Done), generic over block type B (default BlockHash). Drivers now fetch block data by height instead of resolving candidate BlockIds.
Added: BlockQueries, a request/resolve tracker, with resolve_candidates and the BlockCandidateResolution enum.
Added: median_time_past(height, block_time), a free helper computing the BIP-113 median-time-past with the timestamp source supplied by closure. CheckPoint::median_time_past and the canonical-view MTP computation both delegate to it.
Removed: the ChainRequest / ChainResponse type aliases.

bdk_chain

Added: median-time-past on canonical views — CanonicalTx::mtp and Canonical::tip_mtp, opted into via CanonicalViewTask::with_mtp() and LocalChain::canonicalize_with_mtp().
Changed: CanonicalTask and CanonicalViewTask now implement the poll-based ChainTask (previously ChainQuery) and take a generic block-type parameter B.
Changed: LocalChain::canonicalize (the generic task runner) is renamed to LocalChain::run_task, and LocalChain::canonical_view (the high-level helper) is renamed to LocalChain::canonicalize.

Checklists

All Submissions:

I followed the contribution guidelines

New Features:

I've added tests for the new feature
I've added docs for the new feature

Bugfixes:

This pull request breaks the existing API

codecov · 2026-05-09T11:47:57Z

Codecov Report

❌ Patch coverage is 92.39631% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.70%. Comparing base (6d03fc3) to head (0ecd768).

Files with missing lines	Patch %	Lines
crates/chain/src/canonical_view_task.rs	90.76%	10 Missing and 2 partials ⚠️
crates/chain/src/canonical.rs	85.41%	7 Missing ⚠️
crates/chain/src/canonical_task.rs	93.10%	5 Missing and 1 partial ⚠️
crates/chain/src/local_chain.rs	93.75%	2 Missing and 1 partial ⚠️
crates/core/src/block_queries.rs	97.16%	3 Missing ⚠️
crates/chain/src/indexed_tx_graph.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2139      +/-   ##
==========================================
+ Coverage   78.65%   78.70%   +0.04%     
==========================================
  Files          30       31       +1     
  Lines        5909     6104     +195     
  Branches      279      279              
==========================================
+ Hits         4648     4804     +156     
- Misses       1185     1227      +42     
+ Partials       76       73       -3

Flag	Coverage Δ
rust	`78.70% <92.39%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

oleonardolima · 2026-06-15T13:14:05Z

@evanlinjin the PR dependency is solved, #2038 landed on master! 🚀 🚀
You can rebase this one, though I think just resetting and cherry-picking the last commits will be easier (sorry for the huge conflicts 😅).

Exposing median-time-past (MTP) on canonical views needs block timestamps, but the `ChainQuery` interface can't carry them: its response is `ChainResponse = Option<BlockId>` — only a hash and height. Change the interface so the driver returns block *data* by height, generic over a block type `B` (default `BlockHash`, usable as a full `Header`). The driving call also moves from `next_query() -> Option<…>` to `poll() -> TaskProgress`, whose enum expresses states `next_query` couldn't: incremental progress (`Advanced`) and a `Blocked` state that leaves room for a future streaming driver. This keeps the multi-stage canonicalization a clean poll loop rather than an internal one. With block data flowing back, `CanonicalView` gains per-transaction and tip MTP, computed optionally via `CanonicalViewTask::with_mtp()`. Mechanically: rename `ChainQuery` to `ChainTask`; remove `ChainRequest`/`ChainResponse`; add a `BlockQueries` helper for request/resolve tracking; add `LocalChain::run_task` and the `canonicalize{,_with_mtp}()` convenience methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Both canonicalization phases checked a tx's anchors against fetched blocks with identical logic: scan for a resolved, matching block; else request the missing heights; else wait on in-flight queries; else give up. The two copies had already drifted (the phase-2 copy still carried the old `has_unresolved_heights` shape with a latent empty-`Query` edge). Extract that logic as `BlockQueries::resolve_candidates`, returning a `BlockCandidateResolution` enum (`Confirmed` / `Query` / `Awaiting` / `NotConfirmed`). It lives on `BlockQueries` since it is an operation on that type, and is decoupled from `Anchor`: callers pass the candidates as `(payload, BlockId)` pairs, so it needs only a `BlockId` per candidate rather than the `Anchor` trait. Each call site now maps the four outcomes onto its own queue handling and `TaskProgress`. Adds unit tests covering all four outcomes, including the `Awaiting` branch that the synchronous driver never hits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ked` `Blocked` reads more cleanly at call sites and pairs naturally with the streaming-driver framing: `Query` says "launch I/O", `Blocked` says "block until some lands". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Covers the previously-untested median-time-past path: a `LocalChain<Header>` whose block times equal their heights (so the MTP of an 11-block window is the middle height), with a tx confirmed at a known height. Asserts both the tip MTP and the per-tx MTP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Both `CheckPoint::median_time_past` and the canonical view task's MTP computation implemented the same BIP-113 median (11-block window, higher-middle value). Extract a `bdk_core::median_time_past(height, block_time)` helper — abstracting the timestamp source via a closure — so the rule lives in one place; both sites now delegate to it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

`CanonicalTxs::view_task` is the entry point for building a view, but MTP computation is opt-in via `CanonicalViewTask::with_mtp`. Cross-reference it so it is discoverable from the view-building method. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

evanlinjin · 2026-06-19T06:34:24Z

@oleonardolima We should have done this in one go in #2038. Now reviewers need to review 2 large refactor PRs. 😢

oleonardolima · 2026-06-19T14:00:27Z

@oleonardolima We should have done this in one go in #2038. Now reviewers need to review 2 large refactor PRs. 😢

Oh, agree! Though #2038 was long overdue and getting too convoluted, I think having this follow-up won't be that bad.

I'll make this review my top priority, let me know when it's ready.

The signature takes candidates as `(payload, BlockId)` pairs, but the doc comment still described the removed `block_id` closure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…test Replace the six per-outcome tests with a single `Case`-table test that runs each scenario through a fresh `BlockQueries`, asserting both the `BlockCandidateResolution` and the outstanding heights afterward. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

evanlinjin · 2026-06-23T00:17:58Z

@oleonardolima ready for review

github-project-automation Bot added this to BDK Chain Mar 9, 2026

oleonardolima mentioned this pull request Mar 9, 2026

refactor(chain,core)!: replace CanonicalIter with sans-IO CanonicalTask + ChainQuery trait #2038

Merged

3 tasks

evanlinjin force-pushed the refactor/sans-io-mtp-chain-query branch 2 times, most recently from 6faf825 to c1ad161 Compare March 13, 2026 11:13

evanlinjin self-assigned this Apr 10, 2026

evanlinjin force-pushed the refactor/sans-io-mtp-chain-query branch 2 times, most recently from bdd7608 to 45bf03a Compare May 6, 2026 12:53

evanlinjin force-pushed the refactor/sans-io-mtp-chain-query branch 5 times, most recently from c037f35 to dfe4f4e Compare June 17, 2026 16:31

evanlinjin and others added 6 commits June 19, 2026 06:26

evanlinjin force-pushed the refactor/sans-io-mtp-chain-query branch from 2f5d193 to 5dc4b51 Compare June 19, 2026 06:32

evanlinjin and others added 2 commits June 22, 2026 23:42

docs(core): fix resolve_candidates doc for (payload, BlockId) input

37ad249

The signature takes candidates as `(payload, BlockId)` pairs, but the doc comment still described the removed `block_id` closure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

evanlinjin marked this pull request as ready for review June 23, 2026 00:16

evanlinjin requested review from ValuedMammal, luisschwab and oleonardolima as code owners June 23, 2026 00:16

oleonardolima added module-blockchain api A breaking API change labels Jun 23, 2026

oleonardolima moved this to Needs Review in BDK Chain Jun 23, 2026

oleonardolima added this to the Chain 0.24.0 milestone Jun 23, 2026

evanlinjin mentioned this pull request Jun 27, 2026

Fix #2040: Update Balance immature calculation for MTP-based maturity #2226

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor `ChainQuery` to be more flexible #2139

Refactor `ChainQuery` to be more flexible #2139
evanlinjin wants to merge 8 commits into
bitcoindevkit:masterfrom
evanlinjin:refactor/sans-io-mtp-chain-query

evanlinjin commented Mar 9, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 9, 2026 •

edited

Loading

Uh oh!

oleonardolima commented Jun 15, 2026

Uh oh!

evanlinjin commented Jun 19, 2026

Uh oh!

oleonardolima commented Jun 19, 2026

Uh oh!

evanlinjin commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

evanlinjin commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

codecov Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

oleonardolima commented Jun 15, 2026

Uh oh!

evanlinjin commented Jun 19, 2026

Uh oh!

oleonardolima commented Jun 19, 2026

Uh oh!

evanlinjin commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

evanlinjin commented Mar 9, 2026 •

edited

Loading

codecov Bot commented May 9, 2026 •

edited

Loading