Skip to content

refactor(chain): add CanonicalView topological ordering#2219

Open
oleonardolima wants to merge 1 commit into
bitcoindevkit:masterfrom
oleonardolima:refactor/add-canonical-view-topological-ordering
Open

refactor(chain): add CanonicalView topological ordering#2219
oleonardolima wants to merge 1 commit into
bitcoindevkit:masterfrom
oleonardolima:refactor/add-canonical-view-topological-ordering

Conversation

@oleonardolima

@oleonardolima oleonardolima commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

depends on #2038
closes #2201

Description

Add topological ordering to CanonicalView so that parent transactions always appear before their children when iterating canonical transactions. Uses Kahn's algorithm in CanonicalViewTask::finish().

Notes to the reviewers

  • sort_topologically is a standalone function in canonical_view_task.rs that operates over the existing spends map, no new data structures needed.
  • Only edges where both parent and child are canonical are considered.
  • Order among unrelated transactions is non-deterministic (HashMap iteration order).

Changelog notice

#### Changed

- `CanonicalView` now returns transactions in topological-spending order (parents before children).
- `Canonical` struct documentation updated to reflect the ordering guarantee.

Checklists

All Submissions:

New Features:

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

@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.76%. Comparing base (023fdf0) to head (88b7de2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2219      +/-   ##
==========================================
+ Coverage   78.65%   78.76%   +0.10%     
==========================================
  Files          30       30              
  Lines        5909     5938      +29     
  Branches      279      283       +4     
==========================================
+ Hits         4648     4677      +29     
  Misses       1185     1185              
  Partials       76       76              
Flag Coverage Δ
rust 78.76% <100.00%> (+0.10%) ⬆️

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 oleonardolima force-pushed the refactor/add-canonical-view-topological-ordering branch 2 times, most recently from 3fa1da6 to 27b4acf Compare June 11, 2026 19:26
@oleonardolima oleonardolima self-assigned this Jun 11, 2026
@oleonardolima oleonardolima moved this to In Progress in BDK Chain Jun 11, 2026
@oleonardolima oleonardolima added this to the Chain 0.24.0 milestone Jun 11, 2026
- add `sort_topologically` to `CanonicalViewTask::finish()` using
  Kahn's algorithm, ensuring `CanonicalView` returns `Txid`s in
  topological order (parents before children)
- add `test_list_ordered_canonical_txs` with scenarios covering
  various transaction graph shapes
@oleonardolima oleonardolima force-pushed the refactor/add-canonical-view-topological-ordering branch from 27b4acf to 88b7de2 Compare June 15, 2026 12:55
@oleonardolima oleonardolima marked this pull request as ready for review June 15, 2026 13:07
@oleonardolima

Copy link
Copy Markdown
Collaborator Author

@nymius As a heads up I also tested with your tests: nymius@bafd2c9 and nymius@b13a540 and it worked as expected.

#[derive(Debug)]
pub struct Canonical<A, P> {
/// List of canonical transaction IDs.
/// Ordered list of transaction IDs in topological-spending order.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only true for CanonicalView right? In CanonicalTxs, it's just ordered by the order that the txs got canonicalized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Add optional topological ordering stage CanonicalViewTask

2 participants