From: nymius <155548262+nymius@users.noreply.github.com> Date: Wed, 12 Feb 2025 19:44:15 +0000 (-0300) Subject: fix(wallet): off-by-one error checking coinbase maturity in optional UTxOs X-Git-Tag: wallet-1.2.0~29^2 X-Git-Url: http://internal-gitweb-vhost/script/%22https:/database/scripts/struct.Block.html?a=commitdiff_plain;h=03b7ecaea1176f55c6792d8798ead72b23082af7;p=bdk fix(wallet): off-by-one error checking coinbase maturity in optional UTxOs 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. --- diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 3d60aa2e..6e00b109 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/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, } diff --git a/crates/wallet/src/wallet/tx_builder.rs b/crates/wallet/src/wallet/tx_builder.rs index 88d07cbb..da41c6b0 100644 --- a/crates/wallet/src/wallet/tx_builder.rs +++ b/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 { diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 174a628d..f42f0bcd 100644 --- a/crates/wallet/tests/wallet.rs +++ b/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!(