Off-by-one error is making optional UTxOs selection too restrictive for coinbase outputs

[nymius]

Describe the bug
The preselect_utxos has an off-by-one error that is making the selection of optional UTxOs too restrictive, by requiring the coinbase outputs to surpass or equal coinbase maturity time at the current height of the selection, and not in the block in which the transaction may be included in the blockchain (be spent).

To Reproduce
Apply the following changes totest_spend_coinbase:

diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs
index 174a628d..ac4fa312 100644
--- a/crates/wallet/tests/wallet.rs
+++ b/crates/wallet/tests/wallet.rs
@@ -3875,8 +3875,8 @@ fn test_spend_coinbase() {
     };
     insert_anchor(&mut wallet, txid, anchor);
 
-    let not_yet_mature_time = confirmation_height + COINBASE_MATURITY - 1;
-    let maturity_time = confirmation_height + COINBASE_MATURITY;
+    let not_yet_mature_time = confirmation_height + COINBASE_MATURITY - 2;
+    let maturity_time = confirmation_height + COINBASE_MATURITY - 1;
 
     let balance = wallet.balance();
     assert_eq!(

Expected behavior
cargo test -- test_spend_coinbase should pass with the applied changes, but it fails with CoinSelection::InsufficientFunds because the only spendable UTxO is marked as immature.

Build environment

• BDK tag/commit: 43f0f8d
• OS+version: Ubuntu 24.04.1 LTS
• Rust/Cargo version: cargo 1.83.0 (5ffbef321 2024-10-29)
• Rust/Cargo target: x86_64-unknown-linux-gnu

Additional context
Why I think the correct behavior is to signal the output as mature in $h+99$¹instead of $h+100$¹?

Because as this stack overflow answer states:

A transaction that spends an output of the coinbase transaction at height $h$, is eligible to be included in block $h+100$¹ or higher.
...
This means that miners may include such a transaction in their block template when their chain tip has a height of $h+99$¹.

The bug is in these lines in wallet crate where the difference between the current height and the UTxO mining height is forced to already be of 100 blocks, effectively marking it for inclusion in chain starting from block $h+101$¹, one more than it should be.

The lines mentioned are also referencing bitcoin core code. The code is checking transaction validity after reorg. The linked lines are particularly discarding transactions if some input is spending a coinbase UTxO and the difference between the spend height (current height + 1) and the mine height of the UTxO are less than coinbase maturity. So, to consider its inclusion in a set of candidates for coin selection we should check the same condition. And this chain crate code in BDK is exactly doing that.

There is a test asserting coinbase maturity but it is faulty because of the same mistake: checking transaction can't be spend at $h+100$ and can be spend at $h+101$.
The correct test should be: can't be spend at $h+98$ (because in the block $h+99$ which would hypothetically include the transaction, the input wouldn't have 100 blocks since mined). And can be spend at h+99 because then transaction will be a candidate for inclusion starting from block $h+100$.

Footnotes

1. where h is the height at which the UTxO transaction was mined ↩ ↩² ↩³ ↩⁴ ↩⁵