e2dad373b0e2023a58c67afb073406eeaf028374 bench(chain): benchmark classify_outpoints across canonical topologies (Dmenec)
01b466018c98af9aa3b7335b31c1ea23de0e3f42 test(chain): cover taint propagation and eligibility classification (Dmenec)
3f7c1bde852826cd3df0a12dd0ae8023a6f0bb07 feat(chain)!: derive balance trust from output ancestry (志宇)
98a7d5b88e975d529a4c8e6446ef8c3a4314281e feat(chain): classify outpoints by chain-level spend eligibility (Dmenec)
Pull request description:
Solves #2267
### 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
- `Unsettled(Trust)` otherwise, where `Trust` is:
- `Trusted` if the whole unconfirmed ancestry only spends owned coins
- `Untrusted` if the output itself, or any unconfirmed ancestor, is tainted
- `Unknown` if part of the ancestry is missing from the view, so trust can't be determined
`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 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 `OutPoint`s, 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 `OutPoint`s instead of `(identifier, outpoint)` pairs (this drops the `O` generic). Trust is now derived from an output's unconfirmed ancestry.
- **Breaking**: added `Balance::unknown_pending`, where outputs whose trust can't be determined are counted instead of being lumped into `untrusted_pending`.
- Added `CanonicalView::classify_outpoints` and the `Eligibility` enum.
### Checklists
#### All Submissions:
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `just p` before pushing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
#### Bugfixes:
* [x] This pull request breaks the existing API
ACKs for top commit:
evanlinjin:
ACK
e2dad373b0e2023a58c67afb073406eeaf028374
nymius:
ACK
e2dad373b0e2023a58c67afb073406eeaf028374
Tree-SHA512: 30db16f27b3068f7ed2e0ac43b199438437622a66ff1a1a8d9fd854d22895384979a3e2305139ca7955137087bde832c64d31807069f60f08293c5e194234693