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

[Dmenec]

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]

Codecov Report

❌ Patch coverage is 90.59829% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.46%. Comparing base (337e9d6) to head (7ee5080).

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.36%   78.46%   +0.09%     
==========================================
  Files          30       30              
  Lines        5945     6018      +73     
  Branches      281      284       +3     
==========================================
+ Hits         4659     4722      +63     
- Misses       1210     1218       +8     
- Partials       76       78       +2     

Flag	Coverage Δ
rust	78.46% <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]

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]

@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]

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".

[nymius]

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?

[evanlinjin]

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.

[nymius]

Not even if you assume it canonical?

[Dmenec]

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.

[Dmenec]

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

[nymius]

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]

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