feat(chain)!: taint-aware CanonicalView::balance #2246
Open
Dmenec wants to merge 4 commits into
Open
Conversation
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.
Dmenec
requested review from
ValuedMammal,
evanlinjin and
oleonardolima
as code owners
July 22, 2026 18:18
Codecov Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Takes over #2235 (thanks @evanlinjin for the go-ahead). It reworks
CanonicalView::balanceto derive trust from an output's unconfirmed ancestry, and addsclassify_outpoints, a per-output spend-eligibility classifier thatbalancebecomes a thin fold over.The old
balancedecided trust using a per-outputtrust_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 aforeign unconfirmed output)
is_settled(&pos)- do we consider this chain position settled / final? (generalizesmin_confirmations)For each unspent output,
classify_outpointsreports its chain-level spend eligibility:Settledif considered settled according tois_settledImmaturefor a coinbase output that has not yet maturedTrustedPendingif the output is pending but trustedUntrustedPendingif the output itself, or any of its unconfirmed ancestors, is taintedbalancethen sums each output's value into the bucket corresponding to itsEligibility.Notes to the reviewers
Balance::confirmedand did not rename it tosettled. That rename is out of scope here. The folding logic is slated to move intobdk_wallet, and the currentbalancefunction inchainwill eventually be deprecated.balancealso drops theOgeneric and now takes plainOutPoints, since the taint predicate operates on transactions rather than per-outpoint associated data.does_taintis evaluated at most once per transaction.classify_outpoints. Left for a follow-up.Changelog notice
CanonicalView::balancenow takesdoes_taint: impl FnMut(&CanonicalTx) -> boolandis_settled: impl Fn(&ChainPosition) -> boolinstead of a per-output trust predicate andmin_confirmations, and plainOutPoints instead of(identifier, outpoint)pairs (this dropsthe
Ogeneric). Trust is now derived from an output's unconfirmed ancestry.CanonicalView::classify_outpointsand theEligibilityenum.Checklists
All Submissions:
New Features:
Bugfixes: