refactor(chain,core)!: replace `CanonicalIter` with sans-IO `CanonicalTask` + `ChainQuery` trait by oleonardolima · Pull Request #2038 · bitcoindevkit/bdk

oleonardolima · 2025-09-18T05:59:57Z

Description

Replaces the iterator-based CanonicalIter with a two-phase sans-IO canonicalization pipeline, and introduces a generic ChainQuery trait in bdk_core to decouple canonicalization from chain sources.

Old API:

// Direct coupling between canonicalization logic and ChainOracle
let view = tx_graph.canonical_view(&chain, chain_tip, params)?;

New API:

// Option A: Two-phase (full control)
let canonical_txs = chain.canonicalize(tx_graph.canonical_task(tip, params));
let view = chain.canonicalize(canonical_txs.view_task(&tx_graph));

// Option B: Convenience method
let view = chain.canonical_view(&tx_graph, tip, params);

Phase 1: `CanonicalTask`

Determines which transactions are canonical by processing them in stages:

Assumed txs — transactions assumed canonical via CanonicalParams
Anchored txs — transactions anchored in the best chain (descending height)
Seen txs — unconfirmed transactions by descending last-seen time
Remaining txs — leftover anchored transactions not in the best chain

Produces a CanonicalTxs<A> containing each canonical transaction with its CanonicalReason.

Phase 2: `CanonicalViewTask`

Resolves CanonicalReasons into concrete ChainPositions (confirmed height or unconfirmed with last-seen), producing the final CanonicalView<A>.

Both phases implement the ChainQuery trait, so any chain source can drive them via the same next_query/resolve_query loop.

Key structural changes

ChainQuery trait added to bdk_core — a generic sans-IO interface (next_query → resolve_query → finish) for any algorithm that needs to verify blocks against a chain source.
ChainOracle trait removed — replaced by ChainQuery. LocalChain::canonicalize() now drives any ChainQuery implementor.
Canonical<A, P> generic container — CanonicalTxs<A> (phase 1 output) and CanonicalView<A> (phase 2 output) are type aliases over Canonical<A, P>.
Module split — canonical_view.rs split into canonical.rs (types: Canonical, CanonicalTx, CanonicalTxOut) and canonical_view_task.rs (phase 2 task). canonical_iter.rs replaced by canonical_task.rs.

Notes to the reviewers

The changes are split into multiple commits for easier review. Also depends on #2029.

Changelog notice

  ### Added
  - `bdk_core::ChainQuery` trait — generic sans-IO interface for chain verification queries
  - `bdk_core::ChainRequest` / `ChainResponse` type aliases
  - `CanonicalTask` — phase 1 sans-IO canonicalization (determines canonical txs)
  - `CanonicalViewTask` — phase 2 sans-IO canonicalization (resolves chain positions)
  - `Canonical<A, P>` generic container with `CanonicalTxs<A>` and `CanonicalView<A>` aliases
  - `LocalChain::canonicalize()` — drives any `ChainQuery` implementor
  - `LocalChain::canonical_view()` — convenience method for full two-phase canonicalization

  ### Changed
  - **Breaking:** Replace `TxGraph::canonical_iter()` / `TxGraph::canonical_view()` with `TxGraph::canonical_task()`
  - **Breaking:** Canonicalization now uses a two-phase sans-IO process via `ChainQuery`
  - **Breaking:** `ChainQuery`, `ChainRequest`, `ChainResponse` have no generics (use `BlockId` directly)
  - **Breaking:** Chain tip moved from `ChainRequest` to `ChainQuery::tip()`

  ### Removed
  - **Breaking:** `ChainOracle` trait and all implementations
  - **Breaking:** `CanonicalIter` type and `canonical_iter` module
  - **Breaking:** `TxGraph::try_canonical_view()` and `TxGraph::canonical_view()` methods
  - **Breaking:** `CanonicalView::new()` public constructor

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

evanlinjin

Great work.

This is my initial round of reviews.

Are you planning to introduce topological ordering in a separate PR?

evanlinjin · 2025-10-02T06:43:24Z

+    let chain_tip = chain.tip().block_id();
+    let task = graph.canonicalization_task(chain_tip, Default::default());
+    let canonical_view = chain.canonicalize(task);


What do you think about the following naming:

CanonicalizationTask -> CanonicalResolver.

TxGraph::canonicalization_task -> TxGraph::resolver.

LocalChain::canonicalize -> LocalChain::resolve.

I've been thinking about this, I agree with the CanonicalResolver, though the TxGraph::resolver and LocalChain::resolve seems a bit off.

What do you think about (?):

CanonicalResolver

TxGraph::canonical_resolver

LocalChain::canonical_resolve

Alright, this one is a bit outdated.

As of 031de40 we have:

CanonicalizationTask -> CanonicalTask; and also new CanonicalViewTask.

TxGraph::canonicalization_task -> TxGraph::canonical_task.

LocalChain::canonicalize is still the same, though we now also have LocalChain::canonical_view.

I'm fine with these names for now, though if there's no consensus on those we can discuss/change in a follow-up PR.

nymius

Most of the suggestions are nits related to first impressions from the code and code comprehension.
The only significant change I would do here, as @ValuedMammal suggested before, is the Kahn's algorithm addition.
The Canonical struct is claiming something that is not true (see linked tests) if not paired with this algorithm.
There are two chances here, to remove the claim or actually implement the sort.
I'm inclined for the second, as the implementation I've implemented with LLM help is not as complex as I thought: nymius@dbfabad

It also simplifies the code in canonical task, as some of the order related fields are unnecessary after implementing the sort algorithm.

I've also compared the implementation there with the implementation from 1a77475.
I've ported some of those test to the current implementation in nymius@25049a9.
I don't use the chain position to disambiguate order positions nor create depth based tiers in the order because I'm not aware of use cases that require this disposition.

nymius · 2026-05-12T21:22:56Z

+    /// Contruct a new [`CanonicalReason`] from the original which is transitive to `descendant`.
+    ///
+    /// This signals that either the [`ObservedIn`] or [`Anchor`] value belongs to the transaction's


This is confusing, what's the direction of the transitivity? ancestor ---> descendant or descendant ---> ancestor?

nymius · 2026-05-19T13:39:08Z

+    /// Transactions that will supersede all other transactions.
+    ///
+    /// In case of conflicting transactions within `assume_canonical`, transactions that appear
+    /// later in the list (have higher index) have precedence.


have higher index is misleading, which is the index? the Txid (I wouldn't call it an index)

nymius · 2026-05-19T14:29:04Z

+
+                if self.not_canonical.contains(&this_txid) {
+                    // Early exit if self-double-spend is detected.
+                    detected_self_double_spend = true;


When a self double spend is detected, shouldn't the code stop straight away instead of "running until finished"?

nymius · 2026-05-20T18:31:53Z

+    ///
+    /// This signals that either the [`ObservedIn`] or [`Anchor`] value belongs to the transaction's
+    /// descendant, but is transitively relevant.
+    pub fn to_transitive(&self, descendant: Txid) -> Self {


transitivity is a directed relation. Without some clues, it may be hard to understand what's the direction here. I would change to_transitive to by_descendant or something similar that shows the tx is canonical because its descendant is. So it is clear the canonical reason is transmitted from the descendant to the ancestor.

How about rename both the field and method to be via_descendant?

nymius · 2026-05-20T18:31:54Z

+        }
+    }
+
+    /// Construct a new [`CanonicalReason`] from the original which is transitive to `descendant`.


This is confusing, what's the direction of the transitivity? ancestor ---> descendant or descendant ---> ancestor`?

ValuedMammal · 2026-05-21T14:57:56Z

Thank you @nymius

oleonardolima · 2026-05-21T21:22:08Z

Most of the suggestions are nits related to first impressions from the code and code comprehension. The only significant change I would do here, as @ValuedMammal suggested before, is the Kahn's algorithm addition. The Canonical struct is claiming something that is not true (see linked tests) if not paired with this algorithm. There are two chances here, to remove the claim or actually implement the sort.

@nymius thanks for the review, I'll take a look on the comments. In my opinion, this PR's already got complex enough by itself, I'd suggest adding the topological order support as a follow-up, as it was intended by #2201, I'd say the same regarding some canonicalization algorithm fixes that I've added alongside here.

nymius · 2026-05-22T11:39:14Z

@nymius thanks for the review, I'll take a look on the comments. In my opinion, this PR's already got complex enough by itself, I'd suggest adding the topological order support as a follow-up, as it was intended by #2201, I'd say the same regarding some canonicalization algorithm fixes that I've added alongside

I saw the topological order issue is on the same milestone than this one, so I'm ok with this, as long they go together in the same release. I would still remove any topological order claims from docs before that happens.

It introduces the new `CanonicalizationTask` that's implements the canonicalization algorithm through a request/response pattern. Also, it introduces the new `ChainQuery` trait in `bdk_core`, which provides an interface for blockchain source/oracle query-based operations. Allowing sans-IO patterns for algorithm that needs a blockchain oracle, without the need for directly implement/handle I/O. Adds new API methods into `LocalChain`: `canonicalize` and `canonical_view`, adding same features as the existing `CanonicalIter` and it's APIs. Co-Authored-By: Claude <noreply@anthropic.com>

nymius

ACK dd69b6a

evanlinjin · 2026-06-13T20:20:56Z

@oleonardolima I think we accidentally added back the examples in this PR!

Edit: I'll fix it!

Updates the codebase to use the new convenience `LocalChain::canonical_view` method in order to generate the `CanonicalView`. Internally the convenience method follows the `sans-IO` approach, separating the canonicalization algorithm from i/o operations, and it's used as follows: 1. Create a new `CanonicalizationTask` with a `TxGraph`, by calling: `graph.canonicalization_task(params)` 2. Execute the canonicalization process with a chain oracle (e.g `LocalChain`, which implements `ChainOracle` trait), by calling: `chain.canonicalize(task, chain_tip)`

The codebase has been updated to the new `LocalChain::canonical_view` method. It's now safe to remove the `CanonicalIter` it's the old APIs relying on it, eg. `try_canonical_view`.

…`Canonical<A, P>` Separate concerns by splitting `CanonicalizationTask` into two phases: 1. `CanonicalTask` determines which transactions are canonical and why (`CanonicalReason`), outputting `CanonicalTxs<A>`. 2. `CanonicalViewTask` resolves reasons into `ChainPosition`s (confirmed vs unconfirmed), outputting `CanonicalView<A>`. Make `Canonical<A, P>`, `CanonicalTx<P>`, and `FullTxOut<P>` generic over the position type so the same structs serve both phases. Add `LocalChain::canonical_view()` convenience method for the common two-step pipeline. Renames: - `CanonicalizationTask` -> `CanonicalTask` - `CanonicalizationParams` -> `CanonicalParams` - `canonicalization_task()` -> `canonical_task()` - `FullTxOut::chain_position` -> `FullTxOut::pos` Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…view_task` It moves the shared types: `CanonicalTx`, `Canonical`, `CanonicalView`, `CanonicalTxs` and other convenience methods into `canonical.rs`. Keep the phase-2 task (`CanonicalViewTask` in `canonical_view_task.rs`. Also, rename `FullTxOut` to `CanonicalTxOut`, and move it to `canonical.rs`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- add new `test_canonical_view_task.rs` to handle different scenarios of chain position resolution. - fixes the assumed canonical txs chain position resolution, especially for transitively assumed canonical transactions, where there's an anchored/confirmed descendant.

evanlinjin

ACK 555f774

oleonardolima · 2026-06-15T11:35:06Z

@oleonardolima I think we accidentally added back the examples in this PR!

Edit: I'll fix it!

Oh, thanks! I must've missed that in the last rebasing.

outdated review

github-project-automation Bot added this to BDK Chain Sep 18, 2025

evanlinjin reviewed Sep 18, 2025

View reviewed changes

Comment thread crates/chain/src/canonical_task.rs Outdated

LagginTimes assigned oleonardolima Sep 18, 2025

oleonardolima added this to the Wallet 3.0.0 milestone Sep 18, 2025

notmandatory moved this to In Progress in BDK Chain Sep 18, 2025

oleonardolima mentioned this pull request Sep 19, 2025

refactor(chain)!: remove ChainOracle dependency from CanonicalIter #2034

Closed

6 tasks

oleonardolima force-pushed the refactor/canonical-iter-api branch 6 times, most recently from d851ba6 to c02636d Compare September 23, 2025 00:54

oleonardolima added module-blockchain api A breaking API change labels Sep 23, 2025

oleonardolima force-pushed the refactor/canonical-iter-api branch from c02636d to 78c0538 Compare September 23, 2025 01:08

oleonardolima requested review from LLFourn and LagginTimes September 23, 2025 01:09

oleonardolima mentioned this pull request Sep 24, 2025

refactor(chain)!: update ChainOracle and TxGraph methods to being… #2035

Closed

6 tasks

oleonardolima force-pushed the refactor/canonical-iter-api branch 3 times, most recently from 677e25a to 9e27ab1 Compare September 29, 2025 01:47

evanlinjin reviewed Oct 2, 2025

View reviewed changes

Comment thread crates/chain/src/canonical.rs

oleonardolima marked this pull request as ready for review October 2, 2025 06:18

evanlinjin reviewed Oct 2, 2025

View reviewed changes

oleonardolima force-pushed the refactor/canonical-iter-api branch from 9e27ab1 to f6c8b02 Compare October 3, 2025 00:33

evanlinjin reviewed Oct 7, 2025

View reviewed changes

Comment thread crates/core/src/chain_query.rs Outdated

Comment thread crates/chain/src/canonical_task.rs Outdated

Comment thread crates/core/src/chain_query.rs Outdated

oleonardolima force-pushed the refactor/canonical-iter-api branch 4 times, most recently from a276c37 to b7f8fba Compare October 8, 2025 04:53

evanlinjin reviewed May 20, 2026

View reviewed changes

Comment thread crates/chain/tests/test_canonical_view_task.rs

nymius requested changes May 20, 2026

View reviewed changes

nymius mentioned this pull request May 21, 2026

Add optional topological ordering stage CanonicalViewTask #2201

Open

oleonardolima force-pushed the refactor/canonical-iter-api branch 2 times, most recently from 47eb93c to 8c0e3f6 Compare June 2, 2026 20:33

oleonardolima force-pushed the refactor/canonical-iter-api branch from 8c0e3f6 to a3b2e10 Compare June 2, 2026 20:46

Dmenec mentioned this pull request Jun 3, 2026

Make TxAncestors::new_include_root public #2216

Open

oleonardolima mentioned this pull request Jun 4, 2026

refactor(chain): add CanonicalView topological ordering #2219

Open

3 tasks

Dmenec mentioned this pull request Jun 5, 2026

refactor(chain)!: remove min_confirmations parameter from CanonicalView::balance #2221

Draft

2 tasks

oleonardolima force-pushed the refactor/canonical-iter-api branch from a3b2e10 to dd69b6a Compare June 11, 2026 18:54

nymius approved these changes Jun 11, 2026

View reviewed changes

oleonardolima requested a review from evanlinjin June 11, 2026 19:44

oleonardolima and others added 6 commits June 13, 2026 20:23

refactor(chain)!: remove CanonicalIter APIs

a9f5ca4

The codebase has been updated to the new `LocalChain::canonical_view` method. It's now safe to remove the `CanonicalIter` it's the old APIs relying on it, eg. `try_canonical_view`.

chore(chain,example)!: remove ChainOracle, update docs

4f10cde

evanlinjin force-pushed the refactor/canonical-iter-api branch from dd69b6a to 555f774 Compare June 13, 2026 20:44

evanlinjin approved these changes Jun 13, 2026

View reviewed changes

oleonardolima merged commit 023fdf0 into bitcoindevkit:master Jun 15, 2026
15 of 16 checks passed

github-project-automation Bot moved this from Needs Review to Done in BDK Chain Jun 15, 2026

oleonardolima deleted the refactor/canonical-iter-api branch June 15, 2026 12:42

Conversation

oleonardolima commented Sep 18, 2025 • edited by evanlinjin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Phase 1: CanonicalTask

Phase 2: CanonicalViewTask

Key structural changes

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nymius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ValuedMammal commented May 21, 2026

Uh oh!

oleonardolima commented May 21, 2026

Uh oh!

nymius commented May 22, 2026

Uh oh!

nymius left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

oleonardolima commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

oleonardolima commented Sep 18, 2025 •

edited by evanlinjin

Loading

Phase 1: `CanonicalTask`

Phase 2: `CanonicalViewTask`

oleonardolima Mar 6, 2026 •

edited

Loading

evanlinjin commented Jun 13, 2026 •

edited

Loading