bitcoindevkit / bitcoindevkit/bdk
CanonicalView::balance misclassifies immature coinbase outputs when min_confirmations exceeds their confirmations
- Dominant language
- Rust
- Stars
- 1.1k
- Forks
- 483
- Avg merge
- 20d 3h
- Merged PRs (30d)
- 3
Description
**Describe the bug**
`CanonicalView::balance` can misclassify an **immature confirmed coinbase output** as `trusted_pending` or `untrusted_pending` when its current confirmation count is below the caller-provided `min_confirmations`.
The issue is in `crates/chain/src/canonical.rs`, inside the `ChainPosition::Confirmed` branch of `CanonicalView::balance`.
The current logic checks `confirmations < min_confirmations` **before** checking whether the output is mature:
```rust
if confirmations < min_confirmations {
if trust_predicate(&spk_i, &txout) {
trusted_pending += txout.txout.value;
} else {
untrusted_pending += txout.txout.value;
}
} else if txout.is_confirmed_and_spendable(self.tip.height) {
confirmed += txout.txout.value;
} else if !txout.is_mature(self.tip.height) {
immature += txout.txout.value;
}
```
This means that when an immature coinbase satisfies:
```text
confirmations < min_confirmations
```
the first branch is taken and the maturity check is never reached.
As a result, the immature coinbase value is placed into `trusted_pending` or `untrusted_pending` instead of `Balance::immature`.
This can also affect the semantics of `Balance::trusted_spendable()`, since `trusted_pending` contributes to that value while an immature coinbase output is not yet spendable.
---
**To Reproduce**
Consider a confirmed coinbase output with:
```text
coinbase confirmation height = 10
tip height = 50
min_confirmations = 50
```
Its current confirmation count is:
```text
confirmations = 50 - 10 + 1 = 41
```
The coinbase is still immature, but:
```text
41 < 50
```
causes the `confirmations < min_confirmations` branch to execute first.
With `trust_predicate` returning `true`, the current implementation produces:
```text
immature = 0
trusted_pending = coinbase_value
untrusted_pending = 0
confirmed = 0
```
A minimal regression test for this case can assert:
```rust
assert_eq!(balance.immature, coinbase_value);
assert_eq!(balance.trusted_pending, Amount::ZERO);
assert_eq!(balance.untrusted_pending, Amount::ZERO);
assert_eq!(balance.confirmed, Amount::ZERO);
```
The `immature` assertion fails with the current condition ordering because the value is instead classified as `trusted_pending`.
The same issue occurs with `trust_predicate = false`, except the value is classified as `untrusted_pending`.
---
**Expected behavior**
An immature confirmed coinbase output should always be classified under `Balance::immature`, regardless of the caller-provided `min_confirmations` threshold.
Coinbase maturity should therefore be evaluated before applying the `min_confirmations` pending classification.
Conceptually, the ordering could be changed to:
```rust
if !txout.is_mature(self.tip.height) {
immature += txout.txout.value;
} else if confirmations < min_confirmations {
if trust_predicate(&spk_i, &txout) {
trusted_pending += txout.txout.value;
} else {
untrusted_pending += txout.txout.value;
}
} else if txout.is_confirmed_and_spendable(self.tip.height) {
confirmed += txout.txout.value;
}
```
For non-coinbase outputs, `is_mature()` evaluates as mature, so they should continue through the existing `min_confirmations` classification path.
---
**Build environment**
* BDK tag/commit: `456f9b7bbf510eefdf3e7a164a5d6a2be74ee800`
* OS+version: Windows
* Rust/Cargo version: ``
* Rust/Cargo target: ``
**Which backend(s) are relevant (if any)?**
* [ ] Electrum
* [ ] Esplora
* [ ] Bitcoin Core RPC
* [x] None / not backend-related (e.g. `bdk_chain`, `bdk_core`)
* [ ] Other (please specify): `____`
**Is this blocking production use?**
* [ ] Yes
* [x] No
**Project or organization (optional)**
N/A
**Additional context**
`Balance::immature` is documented as containing coinbase outputs that have not yet matured. The current condition ordering allows the caller-defined `min_confirmations` threshold to take precedence over that maturity classification.
Current classification flow:
```text
Confirmed output
|
v
confirmations < min_confirmations?
|
YES
|
+---- trust = true ----> trusted_pending
|
+---- trust = false ---> untrusted_pending
|
v
maturity check is skipped
```
Expected classification flow:
```text
Confirmed output
|
v
Is output immature?
|
YES ----------> immature
|
NO
v
confirmations < min_confirmations?
|
YES ----------> trusted/untrusted pending
|
NO
v
confirmed/spendable
```
A focused regression test covering an immature coinbase where `confirmations < min_confirmations` should prevent this classification from regressing.
Contributor guide
Research direction
Open crates/chain/src/canonical.rs and inspect the ChainPosition::Confirmed branch of CanonicalView::balance, focusing on the ordering of maturity and min_confirmations checks. Add a focused regression test for an immature coinbase whose confirmations are below min_confirmations. Done means the value is counted in Balance::immature, with zero trusted_pending, untrusted_pending, and confirmed amounts, while non-coinbase outputs retain their existing classification.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100