Skip to content

feat(chain)!: taint-aware CanonicalView::balance #2246

Open
Dmenec wants to merge 4 commits into
bitcoindevkit:masterfrom
Dmenec:feat/classify-outpoints
Open

feat(chain)!: taint-aware CanonicalView::balance #2246
Dmenec wants to merge 4 commits into
bitcoindevkit:masterfrom
Dmenec:feat/classify-outpoints

Conversation

@Dmenec

@Dmenec Dmenec commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Description

Takes over #2235 (thanks @evanlinjin for the go-ahead). It reworks CanonicalView::balance to derive trust from an output's unconfirmed ancestry, and adds classify_outpoints, a per-output spend-eligibility classifier that balance becomes a thin fold over.

The old balance decided trust using a per-output trust_predicate, which cannot express transitive trust. As a result, owned outputs whose unconfirmed ancestry included foreign coins were counted as trusted. That is the root cause of the wallet trust-classification bugs (bitcoindevkit/bdk_wallet#16, bitcoindevkit/bdk_wallet#273).

The API now takes two separate predicates, one per concern:

  • does_taint(&tx) - should this transaction be considered tainted? (e.g., because it spends a
    foreign unconfirmed output)
  • is_settled(&pos) - do we consider this chain position settled / final? (generalizes
    min_confirmations)

For each unspent output, classify_outpoints reports its chain-level spend eligibility:

  • Settled if considered settled according to is_settled
  • Immature for a coinbase output that has not yet matured
  • TrustedPending if the output is pending but trusted
  • UntrustedPending if the output itself, or any of its unconfirmed ancestors, is tainted

balance then sums each output's value into the bucket corresponding to its Eligibility.

Notes to the reviewers

  • Trust is resolved through a self-contained per-output ancestry walk. The traversal is memoized: each visited transaction is cached so that already-classified transactions (and their ancestors) do not need to be walked again.
  • Kept Balance::confirmed and did not rename it to settled. That rename is out of scope here. The folding logic is slated to move into bdk_wallet, and the current balance function in chain will eventually be deprecated.
  • balance also drops the O generic and now takes plain OutPoints, since the taint predicate operates on transactions rather than per-outpoint associated data.
  • does_taint is evaluated at most once per transaction.
  • More sophisticated classification rules (e.g., for coin control or locked funds) can be built on top of classify_outpoints. Left for a follow-up.

Changelog notice

  • Breaking: CanonicalView::balance now takes does_taint: impl FnMut(&CanonicalTx) -> bool and
    is_settled: impl Fn(&ChainPosition) -> bool instead of a per-output trust predicate and
    min_confirmations, and plain OutPoints instead of (identifier, outpoint) pairs (this drops
    the O generic). Trust is now derived from an output's unconfirmed ancestry.
  • Added CanonicalView::classify_outpoints and the Eligibility enum.

Checklists

All Submissions:

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

Dmenec and others added 4 commits July 22, 2026 19:32
Adds classify_outpoints, which decides per-output whether it's settled,
immature, or pending (trusted/untrusted) based on its unsettled ancestry.
Taint is resolved with a memoized ancestry walk, so ancestors shared by
several outputs are only walked once.

Co-authored-by: 志宇 <hello@evanlinjin.me>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
balance becomes a fold over classify_outpoints. A pending output's trust is now
taken from its unsettled ancestry (untrusted if any unsettled ancestor taints),
not from a per-output flag.

BREAKING: balance takes does_taint and is_settled instead of trust_predicate
and min_confirmations, and plain OutPoints instead of (identifier, outpoint)
pairs.

Co-authored-by: Dmenec <dmenec@proton.me>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- taint propagates through a pending output's unsettled ancestry
- is_settled alone decides the settled boundary, even for unconfirmed outputs
- taint never crosses a settled ancestor
- an immature coinbase is classified apart from a settled output
- two UTXOs sharing a tainting ancestor are both untrusted (shared taint cache)
- classify_outpoints skips spent and out-of-view outpoints

Co-authored-by: 志宇 <hello@evanlinjin.me>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Times the memoized classification over fan-in, disjoint, untrusted, diamond
and deep-taint-mid-chain graphs at a range of widths and depths.
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.36%. Comparing base (1acb4d0) to head (8897579).

Files with missing lines Patch % Lines
crates/chain/src/canonical.rs 93.33% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2246      +/-   ##
==========================================
+ Coverage   78.32%   78.36%   +0.03%     
==========================================
  Files          30       30              
  Lines        5934     5995      +61     
  Branches      281      282       +1     
==========================================
+ Hits         4648     4698      +50     
- Misses       1210     1221      +11     
  Partials       76       76              
Flag Coverage Δ
rust 78.36% <93.33%> (+0.03%) ⬆️

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.

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants