bitcoindevkit / bitcoindevkit/bdk

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

Open Beginner friendly
#2,267 1 comment 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.