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

[oleonardolima]

fixes #1816

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:

1. Assumed txs — transactions assumed canonical via CanonicalParams
2. Anchored txs — transactions anchored in the best chain (descending height)
3. Seen txs — unconfirmed transactions by descending last-seen time
4. 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]

What do you think about the following naming:

• CanonicalizationTask -> CanonicalResolver.
• TxGraph::canonicalization_task -> TxGraph::resolver.
• LocalChain::canonicalize -> LocalChain::resolve.

[oleonardolima]

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

[oleonardolima]

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.

[evanlinjin]

Can we get rid of ChainOracle in the same PR?

Edit: Removed in 37eb136

[evanlinjin]

ACK 37eb136

[evanlinjin]

I've also updated the PR description.

[oleonardolima]

Can we get rid of ChainOracle in the same PR?

Edit: Removed in 37eb136

I'm fine with removing it in the same PR, thanks for the commit.

I've also updated the PR description.

Great, thank you!

[ValuedMammal]

You seem to be ignoring the descendant, but it may be the case that the descendant has a direct anchor, making this tx Confirmed transitively.

In case the reason is Assumed, to determine the ChainPosition you would want to:

• First look for a direct anchor of the current tx. If a direct anchor exists, the position must be Confirmed (and transitively None)
• Otherwise look at the descendant txid. If the descendant has a direct anchor, then the position is Confirmed transitively by the descendant's anchor
• If the descendant has no direct anchor, then the position is Unconfirmed. Likewise Unconfirmed if there's no anchor and no descendant

[nymius]

You seem to be ignoring the descendant, but it may be the case that the descendant has a direct anchor, making this tx Confirmed transitively.

I think this depends of the priority we give to the different CanonicalReasons. As it is expressed right now, CanonicalReason::Asumed has higher priority than the others.

In case the reason is Assumed, to determine the ChainPosition you would want to:

• First look for a direct anchor of the current tx. If a direct anchor exists, the position must be Confirmed (and transitively None)

In case we raise CanonicalReason::Anchor in the hierarchy, then is as easy as to make AssumedTxs also return a query for these txids.

• Otherwise look at the descendant txid. If the descendant has a direct anchor, then the position is Confirmed transitively by the descendant's anchor

Again, in case we raise CanonicalReason::Anchor in the hierarchy, this case is partially covered by mark_canonical when walking up the Ancestor queue:

            |_: usize, tx: Arc<Transaction>| -> Option<Txid> {
                let this_txid = tx.compute_txid();
                let this_reason = if is_starting_tx {
                    is_starting_tx = false;
                    reason.clone()
                } else {
                    // This is an ancestor being marked transitively
                    reason.to_transitive(starting_txid)
                };

                use crate::collections::hash_map::Entry;
                let canonical_entry = match self.canonical.entry(this_txid) {
                    // Already visited tx before, exit early.
                    Entry::Occupied(_) => return None, // Here we can compare and replace `CanonicalReason::Assumed` by `this_reason` if `this_reason` is `CanonicalReason::Anchor`. 
                    Entry::Vacant(entry) => entry,
                };

• If the descendant has no direct anchor, then the position is Unconfirmed. Likewise Unconfirmed if there's no anchor and no descendant

Are you proposing a different category here? Or with Unconfirmed you refer to CanonicalReason::Assumed?

[evanlinjin]

@nymius I think you're conflating two different things here. ValuedMammal's comment is about phase 2 position resolution (ChainPosition), not about phase 1 canonicality priority (CanonicalReason).

By the time we're in CanonicalViewTask::finish(), canonicality is already decided — we know the tx is canonical and we know its reason is Assumed. The question is purely: what is its ChainPosition — Confirmed or Unconfirmed?

An Assumed tx can still be Confirmed if it or its descendant has an anchor in the best chain. The current code checks direct_anchors.get(txid) (the tx's own anchor) but ignores the descendant field entirely. If the descendant has a resolved anchor in direct_anchors, the tx should be Confirmed { anchor: descendant_anchor, transitively: Some(descendant) }.

This has nothing to do with raising CanonicalReason::Anchor in the hierarchy or changing phase 1 processing. And "Unconfirmed" in ValuedMammal's comment refers to ChainPosition::Unconfirmed, not a CanonicalReason.

[evanlinjin]

@ValuedMammal thanks for pointing out that bug! I do see a logical flaw with your proposed fix though.

Given a scenario where we have txs (A, B and C), where B spends A and C spends B, C is assumed to be canonical, C is unconfirmed and B is anchored to the best chain.

With your logic when looking at A, the descendant field always points to the original assumed tx (C), not to the intermediate tx (B) that actually has the anchor. So checking direct_anchors.get(descendant) only works when the assumed tx itself has the anchor, not when an intermediate ancestor does.

[ValuedMammal]

You'd have to initiate a walk_descendants routine to find the first descendant having a direct anchor, if one exists, otherwise accept that the ChainPosition can't be relied on for Assumed txs.

[oleonardolima]

@ValuedMammal Great, thanks for finding this one! I noticed that unfortunately we also have this issue in both master and release/chain-0.23.x where the scenario above would resolve to: txA->unconfirmed ; txB->confirmed ; txC->unconfirmed.

I'm working on a test to assert for these scenarios.

You'd have to initiate a walk_descendants routine to find the first descendant having a direct anchor, if one exists, otherwise accept that the ChainPosition can't be relied on for Assumed txs.

I'll give a try to this one, but it's valid to note that we can only walk the descendants that are included in the canonical txs.

[oleonardolima]

I think this depends of the priority we give to the different CanonicalReasons. As it is expressed right now, CanonicalReason::Asumed has higher priority than the others.

I have been thinking in depth about this, and @nymius has a good comment here. At least for now we can focus on handling these scenarios in the position resolution.

However we probably need to discuss/review the primitives for the canonicalization algorithm in the future, in these scenarios where a given txA has been resolved to CanonicalReason::Assumed but could also resolved to CanonicalReason::Anchored (both transitively), should the assumed always have higher priority ?

[oleonardolima]

I've pushed the initial fix in 6f8ef26

[evanlinjin]

The initial use-case of assumed_canonical was for crafting RBF and CPFP transactions. We want to assume that the transaction we are replacing or adding a child to is available for coin selection (CPFP) or excluded from coin selection (RBF).

• If confirmed transactions have a higher priority, and a conflict is confirmed, the "assumed canonical" transaction will be non-canonical.
• If assumed transactions have a higher priority, confirmed transactions that conflict will be considered non-canonical.

I think there are practical (but niche) scenarios where we want "assumed canonical" transactions to supersede confirmed transactions.

• The caller may want to create a CPFP transaction that bumps the fee of a transaction which was evicted by a confirmed conflict (in case the confirmed tx gets unconfirmed).
• If the wallet detects a confirmed tx is at risk of being reorged (e.g. very few confirmations), the caller might want to assume a pre-signed replacement is canonical so coin selection uses the replacement's outputs preemptively.

Additionally, "assumed canonical" is simpler to understand than "assumed canonical, unless a conflict is confirmed".

[ValuedMammal]

The canonical task and the view task only differ in the stages of canonicalization they handle and the finished output. You should reduce this to a single task architecture. The rationale in 6d9a7d6 is to separate concerns, but it's really the same concern, canonicalizing the TxGraph into a consistent view. That'll prevent having the algorithm spread across multiple modules.

[nymius]

I agree with this, the canonicalization task doesn't have a meaning without the view task.

[evanlinjin]

I disagree that these are the same concern. They solve different problems and are diverging further over time.

Phase 1 (CanonicalTask) is conflict resolution — it walks the tx DAG, detects conflicts/self-double-spends, and determines which transactions are canonical and why (CanonicalReason). This is the complex part (~500 lines).

Phase 2 (CanonicalViewTask) is view construction — it takes the resolved canonical set and builds the final CanonicalView with concrete ChainPositions. This is a simpler transformation (~200 lines) that is growing its own responsibilities: #2139 adds MTP computation (fetching headers, computing median-time-past per confirmed height), and we plan to add topological sorting here too. None of that belongs in the conflict resolution phase.

The intermediate CanonicalTxs output is also intentionally useful on its own — callers who want to inspect canonicalization reasons without paying for full view construction can stop after phase 1.

A merged task would need 6 stages (AssumedTxs → AnchoredTxs → SeenTxs → LeftOverTxs → ResolvingPositions → Finished), a larger state machine mixing conflict resolution state with position resolution state, and a more complex finish(). The net result would be lines of interleaved concerns instead of two focused types.

[ValuedMammal]

ResolvingPositions is just generating more block queries as in the first task. Instead, when marking a tx canonical, anchors of transitive and assumed txs can be added to the back of the unprocessed anchor queue. In resolve_query if the tx is already canonical, then it can have its reason upgraded to include the best anchor.

In finish you can fetch the TxNode from the tx graph to be passed to the Canonical set. To complete the view just have CanonicalTxs::view -> CanonicalView.

[evanlinjin]

@ValuedMammal My arguments are the following:

1. The anchors aren't free. During canonicalization, you only have CanonicalReason — resolving actual anchors/confirmation heights requires additional is_block_in_chain queries (more IO). Merging the tasks doesn't eliminate that work, it just convolutes chain-position-fetching with canonicalization.

2. Simplicity through focus. Task 1 answers "is this tx canonical?" Task 2 answers "at what height is it confirmed?" These are distinct questions with distinct outputs. Keeping them separate makes each easier to understand.

3. Not all callers need confirmation heights. Some consumers only care about the canonical set, not positions. The split lets them skip the second pass entirely.

4. The duplicate-query cost is solvable orthogonally. PR Refactor ChainQuery to be more flexible  #2139 shows that caching is_block_in_chain results addresses the efficiency concern without coupling the two phases together.

[ValuedMammal]

I can take another look at it.

[ValuedMammal]

While reviewing I found it easier to structure the commits like this.

• feat: Introduce CanonicalTask

Introduces CanonicalTask, a new type that encapsulates block-anchor verification as an explicit query/response loop. The task walks the transaction graph in priority-ordered stages (assumed, anchored, seen, leftover) and emits batches of BlockIds for the caller to verify against a chain oracle, accepting responses via resolve_query. Once driven to completion, finish() returns a CanonicalTxs which can be converted to a CanonicalView via .view().

• feat!: Update usage of CanonicalTask

Migrates the entire codebase to the new CanonicalTask-based API, removing the now-superseded canonical_iter.rs and canonical_view.rs modules. Introduces canonical.rs to house the unified Canonical<A, T, P> type (aliased as CanonicalTxs and CanonicalView) along with the CanonicalReason-to-ChainPosition resolution in .view(). All consumers — tests, benchmarks, examples, and chain-sync integrations — are updated accordingly.

• refactor!: Remove ChainOracle

Removes the ChainOracle trait, which is no longer needed now that canonicalization is driven through CanonicalTask's explicit query/response interface rather than a generic trait. LocalChain's is_block_in_chain and get_chain_tip methods are retained as plain inherent methods (returning Option instead of Result<Option, Infallible>), simplifying the API surface without losing any functionality.

[evanlinjin]

This will stop when a descendant doesn't have a direct anchor which is incorrect as a direct anchor will follow after.

What should happen

• The closure should return everything (no filter).
• .next should stop once it hits a direct anchor.

What should be figured out in a follow up

Although TxDescendants does a BFS (which means we will find the "shallowest" confirmed anchor) - this does not guarantee it will be the "earliest confirmed" anchor (which is the ideal value to have).

[evanlinjin]

Nit

Suggested change

	CanonicalView::new(self.tip, view_order, view_txs, self.spends.clone())
	CanonicalView::new(self.tip, view_order, view_txs, self.spends)

[evanlinjin]

Suggested by Claude

Suggested change

	fn next_query(&mut self) -> Option<ChainRequest>;
	#[must_use]
	fn next_query(&mut self) -> Option<ChainRequest>;