Merge bitcoindevkit#1830: Fix off-by-one error checking coinbase matu…

…rity in optional UTxOs

03b7eca fix(wallet): off-by-one error checking coinbase maturity in optional UTxOs (nymius)

Pull request description:

  ### Description

  As I was developing the changes in bitcoindevkit#1798 I discover issue bitcoindevkit#1810. So I introduced the fixes in that PR but later I split them in two to ease the review by suggestion of @oleonardolima .

  The `preselect_utxos` method 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.

  The changes in this commit fix it by considering the maturity of the coinbase output at the spending height and not
  the transaction creation height, this means, a +1 at the considered height at the moment of building the
  transaction.

  Fixes bitcoindevkit#1810.

  ### Notes to the reviewers

  Tests for issue bitcoindevkit#1810 have not been explicitly added, as there already was a `text_spend_coinbase` test which was corrected to ensure coinbase maturation is considered in alignment with the new logic.

  Changes are not breaking but I'm modifying slightly the documentation for the public method `TxBuilder::current_height` to adjust to the fixed code. Does this merit an entry in the CHANGELOG?

  ### Changelog notice

  `Wallet` now considers a utxo originated from a coinbase transaction (`coinbase utxo`) as available for selection if it will mature in the next block after the height provided to the selection, the current height by default. The previous behavior allowed selecting a `coinbase utxo` only when the height at the moment of selection was equal to maturity height or greater.

  ### Checklists

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing
  * [x] I've updated existing tests to match the fix
  * [x] I've updated docs to match the fix logic
  * [x] This pull request DOES NOT break the existing API
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  LagginTimes:
    ACK 03b7eca
  evanlinjin:
    ACK 03b7eca

Tree-SHA512: f270b73963bd6f141c8a3e759bc9b9bf75de7c52f37fff93f0a6b8b996b449d98c58e5eeb2b56f0ee236222f0807da5c8201ade7462813743e0c4d255313e2b5

crates/wallet/src/wallet/mod.rs

@@ -2052,9 +2052,10 @@ impl Wallet {
                         match chain_position {
                             ChainPosition::Confirmed { anchor, .. } => {
                                 // https://github.com/bitcoin/bitcoin/blob/c5e67be03bb06a5d7885c55db1f016fbf2333fe3/src/validation.cpp#L373-L375
-                                spendable &= (current_height
-                                    .saturating_sub(anchor.block_id.height))
-                                    >= COINBASE_MATURITY;
+                                let spend_height = current_height + 1;
+                                let coin_age_at_spend_height =
+                                    spend_height.saturating_sub(anchor.block_id.height);
+                                spendable &= coin_age_at_spend_height >= COINBASE_MATURITY;
                             }
                             ChainPosition::Unconfirmed { .. } => spendable = false,
                         }

crates/wallet/src/wallet/tx_builder.rs

@@ -556,9 +556,9 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
     /// 1. Set the nLockTime for preventing fee sniping.
     ///    **Note**: This will be ignored if you manually specify a nlocktime using [`TxBuilder::nlocktime`].
     /// 2. Decide whether coinbase outputs are mature or not. If the coinbase outputs are not
-    ///    mature at `current_height`, we ignore them in the coin selection.
-    ///    If you want to create a transaction that spends immature coinbase inputs, manually
-    ///    add them using [`TxBuilder::add_utxos`].
+    ///    mature at spending height, which is `current_height` + 1, we ignore them in the coin
+    ///    selection. If you want to create a transaction that spends immature coinbase inputs,
+    ///    manually add them using [`TxBuilder::add_utxos`].
     ///
     /// In both cases, if you don't provide a current height, we use the last sync height.
     pub fn current_height(&mut self, height: u32) -> &mut Self {

crates/wallet/tests/wallet.rs

@@ -3875,8 +3875,16 @@ 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;
+    // NOTE: A transaction spending an output coming from the coinbase tx at height h, is eligible
+    // to be included in block h + [100 = COINBASE_MATURITY] or higher.
+    // Tx elibible to be included in the next block will be accepted in the mempool, used in block
+    // templates and relayed on the network.
+    // Miners may include such tx in a block when their chaintip is at h + [99 = COINBASE_MATURITY - 1].
+    // This means these coins are available for selection at height h + 99.
+    //
+    // By https://bitcoin.stackexchange.com/a/119017
+    let not_yet_mature_time = confirmation_height + COINBASE_MATURITY - 2;
+    let maturity_time = confirmation_height + COINBASE_MATURITY - 1;
 
     let balance = wallet.balance();
     assert_eq!(