feat(chain)!: taint-aware `CanonicalView::balance` by Dmenec · Pull Request #2246 · bitcoindevkit/bdk

Dmenec · 2026-07-22T18:18:11Z

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:

I followed the contribution guidelines
I ran just p before pushing

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

codecov · 2026-07-22T20:45:23Z

Codecov Report

❌ Patch coverage is 90.59829% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.80%. Comparing base (acc06e5) to head (2581fb2).

Files with missing lines	Patch %	Lines
crates/chain/src/canonical.rs	90.59%	6 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2246      +/-   ##
==========================================
+ Coverage   78.71%   78.80%   +0.09%     
==========================================
  Files          31       31              
  Lines        5966     6039      +73     
  Branches      282      285       +3     
==========================================
+ Hits         4696     4759      +63     
- Misses       1194     1202       +8     
- Partials       76       78       +2

Flag	Coverage Δ
rust	`78.80% <90.59%> (+0.09%)`	⬆️

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.

Dmenec · 2026-07-24T20:36:16Z

Thinking about some cases where settledness makes a transaction fall into a different Eligibility. With foreign inputs (e.g. a tx that needs 6 min_confirmations to be settled, while the current tip is only 1 block ahead of it) the output ends up as UntrustedPending, which doesn't really make sense: it isn't pending, just not settled. The same happens with owned inputs that falls into TrustedPending.

I think it might be clearer to call them TrustedUnsettled / UntrustedUnsettled rather than *Pending.

evanlinjin · 2026-07-27T08:46:24Z

@Dmenec Good point. What do you think about this?

pub enum Eligibility {
    Settled,
    Immature,
    Unsettled(Trust),
}

pub enum Trust {
    Trusted,
    Untrusted,
}

I think this increases clarity and makes it a bit easier for call sites that only care about whether it's unsettled or not.

evanlinjin

Thanks for pushing this forward - this is looking great.

I haven't looked too hard into the tests yet, follow-up reviews will come.

evanlinjin · 2026-07-27T09:18:13Z

+                    if txout.is_mature(tip) {
+                        Eligibility::Settled
+                    } else {
+                        Eligibility::Immature
+                    }


The coinbase maturity check lives inside the is_settled check - this is wrong. Unsettled is not the same as unconfirmed.

Example: A confirmed transaction can be unsettled because the caller requires 3 confirmations to be classified as settled. With the current logic, this transaction will become "pending" instead of "immature".

So any unconfirmed coinbase tx with an owned script pubkey should be treated as Immature, not TrustedPending, even if it doesn't have a single confirmation?

Hopefully there shouldn't be such a thing as an "unconfirmed coinbase" - the canonicalization algorithm should have considered those as non-canonical! If not, let's file a bug.

Not even if you assume it canonical?

Tried it with a coinbase without an anchor (which, as @evanlinjin said, shouldn't be possible on a real chain, but you can still construct it in a test), and if you assume it canonical it does end up in the set as unconfirmed.

bdk/crates/chain/src/canonical.rs

Lines 132 to 137 in 8897579

let conf_height = match self.pos.confirmation_height_upper_bound() {

Some(height) => height,

None => {

debug_assert!(false, "coinbase tx can never be unconfirmed");

return false;

}

As you can see above, it falls back to false there, so such an output is treated as immature anyway.

I agree with moving the maturity check out of is_settled, so it stays Immature regardless of settledness.

Moved the maturity check out of is_settled. Leaving this unresolved for now.

nymius · 2026-07-27T14:52:01Z

+                    if txout.is_mature(tip) {
+                        Eligibility::Settled
+                    } else {
+                        Eligibility::Immature
+                    }


So any unconfirmed coinbase tx with an owned script pubkey should be treated as Immature, not TrustedPending, even if it doesn't have a single confirmation?

Dmenec · 2026-08-04T23:49:43Z

Thanks a lot @evanlinjin and @nymius for reviewing this :)
I think that covers everything now, but let me know if I missed something.

nymius

In commit ff71789, change BREAKING to BREAKING CHANGE to use the most common marker.

Dmenec · 2026-08-20T16:25:29Z

I'd like to add a locked balance category on top of this (bdk_wallet's balance needs to be aware of it), for outputs that are confirmed but not yet spendable because a descriptor timelock hasn't matured yet, as raised in bitcoindevkit/bdk_wallet#180.

I think that maybe, in a future PR, a separate frozen/reserved category could also be added for outputs the user manually locks. I'd leave it as a possible future addition, which can be folded over classify_outpoints without changing Balance in wallet.

Edit: still thinking about how to do it. We could do the fold directly in the wallet with a locked category and leave Balance untouched, but it might be worth modeling it in chain too. Since each Eligibility variant has a matching Balance field, adding a Locked variant would mean adding Balance.locked as well... (same on frozen/reserved) Some thoughts @evanlinjin @nymius @110CodingP?

110CodingP

LGTM 🚀 , just a nit.
I especially love the test coverage, great work @Dmenec !

nymius · 2026-08-24T13:23:22Z

Edit: still thinking about how to do it. We could do the fold directly in the wallet with a locked category and leave Balance untouched, but it might be worth modeling it in chain too. Since each Eligibility variant has a matching Balance field, adding a Locked variant would mean adding Balance.locked as well... (same on frozen/reserved)

This is part of the protocol, not user decided, so it belongs to chain. I would do both changes in Elegibility and Balance.
Using the same criteria, I'm not sure frozen belongs here, but Wallet. There you can pre-filter utxos to mark them as frozen before passing them to chain::balance. You could even create two separated list of outpoints, one set is frozen, the other is not, and compute balance for both separatedly. You would need some extra operations to compute a common output, but is doable.

nymius

ACK 3bbdf78

noahjoeris

ACK 3bbdf78

Great job ✨

Adds classify_outpoints, which decides per-output whether it's settled, immature, or pending (trusted, untrusted, or unknown) based on its unsettled ancestry. Trust is resolved with a memoized ancestry walk: a tainting ancestor makes it untrusted, one missing from the view makes it unknown, and 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 an unsettled ancestor taints or is missing from the view), not from a per-output flag. BREAKING CHANGE: 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.

nymius

ACK 2581fb2

evanlinjin

This is quality work.

I've found a final rough edge that should be hashed out before we merge.

I haven't finished reviewing the tests yet. However, we should probably rename some of them as the min_confirmations parameter no longer exists.

evanlinjin · 2026-09-03T03:59:42Z

    // Test min_confirmations = 0: Should behave same as 1 (confirmed)
    let balance_0_conf = canonical_view.balance(
-        [((), outpoint)],
-        |_, _| true, // trust all
-        0,
+        [outpoint],
+        |_tx| false, // never taint (trust all)
+        settled(tip_height, 0),
    );
    assert_eq!(balance_0_conf.confirmed, Amount::from_sat(50_000));
    assert_eq!(balance_0_conf.trusted_pending, Amount::ZERO);
    assert_eq!(balance_0_conf, balance_1_conf);


We should remove this now since it's only testing a test helper method; settled.

evanlinjin · 2026-09-03T04:01:22Z

+}
+
 #[test]
 fn test_min_confirmations_parameter() {


There is no min_confirmations parameter anymore. Maybe we should rename to test_is_settled_boundary?

evanlinjin · 2026-09-04T02:25:22Z

+/// [`CanonicalView::classify_outpoints`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum Eligibility {
+    /// A confirmed output unlikely to be replaced.


Suggested change

/// A confirmed output unlikely to be replaced.

/// An output the caller considers settled, per the `is_settled` predicate given to

/// [`classify_outpoints`](CanonicalView::classify_outpoints). Typically confirmed deeply

/// enough to be unlikely to be replaced, but the caller decides.

Settled is not necessarily synonymous with "confirmed".

evanlinjin · 2026-09-04T02:35:34Z

-        outpoints: impl IntoIterator<Item = (O, OutPoint)> + 'v,
-        mut trust_predicate: impl FnMut(&O, &CanonicalTxOut<ChainPosition<A>>) -> bool,
-        min_confirmations: u32,
+    pub fn balance(


It's easy for a caller to expect that setting does_taint as |_| false would put all unconfirmed balance in Balance::trusted_pending. However this is not true as UTXOs categorized as Unsettled(Unknown) will end up in Balance::untrusted_pending. This is a potential footgun and even the tests in the repository got confused.

Essentially, does_taint can never achieve "trust all". We need to make this clear in the docs. Furthermore, I also propose introducing Balance::unknown_pending (which means trust cannot be determined due to missing ancestry) making the behavior even more explicit.

evanlinjin · 2026-09-04T02:46:20Z

-        |_, _| true, // trust all
-        6,
+        [outpoint],
+        |_tx| false, // never taint (trust all)


"trust all" is an incorrect framing. Maybe "leave trust to ancestry" makes more sense?

evanlinjin · 2026-09-04T03:01:33Z

-    ///     |_keychain, _script| true,  // Trust all outputs
-    ///     6,  // Require 6 confirmations
+    ///     indexer.outpoints().iter().map(|(_, op)| *op),
+    ///     |_tx| false, // Never taint (trust all)


Let's avoid doing |_| false in the examples and tests as it doesn't make sense.

Callers that actually want to "trust all" should do their own custom balance - our version is correct but opinionated.

evanlinjin · 2026-09-04T04:12:27Z

-        |_, _| false, // don't trust
-        5,
+        [outpoint],
+        |_tx| true, // taint everything


Right now, even if we switch this to |_tx| false, the test still passes for the same reasons we mentioned above; hash!("parent") does not actually point to a real parent -> Trust::Unknown -> Balance::untrusted_pending.

Dmenec requested review from ValuedMammal, evanlinjin and oleonardolima as code owners July 22, 2026 18:18

github-project-automation Bot added this to BDK Chain Jul 22, 2026

evanlinjin mentioned this pull request Jul 23, 2026

feat(chain)!: taint-aware CanonicalView::balance via canonical ancestor walk #2235

Closed

Dmenec mentioned this pull request Jul 24, 2026

refactor(wallet): derive balance trust from output ancestry bitcoindevkit/bdk_wallet#431

Draft

5 tasks

evanlinjin requested changes Jul 27, 2026

View reviewed changes

evanlinjin reviewed Jul 27, 2026

View reviewed changes

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

nymius reviewed Jul 27, 2026

View reviewed changes

Dmenec force-pushed the feat/classify-outpoints branch from 8897579 to efbb669 Compare August 4, 2026 23:14

Dmenec force-pushed the feat/classify-outpoints branch from efbb669 to 7ee5080 Compare August 5, 2026 12:26

110CodingP reviewed Aug 17, 2026

View reviewed changes

Comment thread crates/chain/src/canonical.rs

nymius reviewed Aug 17, 2026

View reviewed changes

Dmenec force-pushed the feat/classify-outpoints branch from 7ee5080 to 982244f Compare August 19, 2026 10:07

Dmenec mentioned this pull request Aug 20, 2026

feat(chain): add ChainPosition::blocks_since_conf #2263

Open

5 tasks

110CodingP approved these changes Aug 23, 2026

View reviewed changes

Comment thread crates/chain/tests/test_indexed_tx_graph.rs

Comment thread crates/chain/tests/test_indexed_tx_graph.rs Outdated

Dmenec mentioned this pull request Aug 23, 2026

feat(wallet)!: add locked balance category for unmet descriptor height-based timelocks bitcoindevkit/bdk_wallet#538

Draft

5 tasks

noahjoeris reviewed Aug 28, 2026

View reviewed changes

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

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

Dmenec force-pushed the feat/classify-outpoints branch from 982244f to 3bbdf78 Compare August 29, 2026 16:56

noahjoeris mentioned this pull request Aug 31, 2026

CanonicalView::balance misclassifies immature coinbase outputs when min_confirmations exceeds their confirmations #2267

Open

7 tasks

nymius approved these changes Aug 31, 2026

View reviewed changes

noahjoeris approved these changes Aug 31, 2026

View reviewed changes

evanlinjin mentioned this pull request Sep 3, 2026

CanonicalView admits an unconfirmed coinbase transaction #2269

Open

evanlinjin and others added 3 commits September 3, 2026 19:17

bench(chain): benchmark classify_outpoints across canonical topologies

2581fb2

Times the memoized classification over fan-in, disjoint, untrusted, diamond and deep-taint-mid-chain graphs at a range of widths and depths.

Dmenec force-pushed the feat/classify-outpoints branch from 3bbdf78 to 2581fb2 Compare September 3, 2026 17:18

nymius approved these changes Sep 3, 2026

View reviewed changes

evanlinjin requested changes Sep 4, 2026

View reviewed changes

	let conf_height = match self.pos.confirmation_height_upper_bound() {
	Some(height) => height,
	None => {
	debug_assert!(false, "coinbase tx can never be unconfirmed");
	return false;
	}

-    /// A confirmed output unlikely to be replaced.
+    /// An output the caller considers settled, per the `is_settled` predicate given to
+    /// [`classify_outpoints`](CanonicalView::classify_outpoints). Typically confirmed deeply
+    /// enough to be unlikely to be replaced, but the caller decides.

Uh oh!

Conversation

Dmenec commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

codecov Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Dmenec commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evanlinjin commented Jul 27, 2026

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Dmenec Jul 29, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Dmenec commented Aug 4, 2026

Uh oh!

Uh oh!

nymius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dmenec commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

110CodingP left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nymius commented Aug 24, 2026

Uh oh!

Uh oh!

Uh oh!

nymius left a comment

Choose a reason for hiding this comment

Dmenec commented Jul 22, 2026 •

edited

Loading

codecov Bot commented Jul 22, 2026 •

edited

Loading

Dmenec commented Jul 24, 2026 •

edited

Loading

Dmenec Jul 29, 2026 •

edited

Loading

Dmenec commented Aug 20, 2026 •

edited

Loading

evanlinjin Sep 4, 2026 •

edited

Loading