Skip to content

Fix off-by-one error checking coinbase maturity in optional UTxOs #1830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
evanlinjin merged 1 commit into from
Feb 14, 2025
Merged

Fix off-by-one error checking coinbase maturity in optional UTxOs #1830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions crates/wallet/src/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

@evanlinjin evanlinjin Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think a better name would be inclusion_height.

Copy link
Contributor Author

@nymius nymius Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't be needed if #1798 is merged.

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,
}
Expand Down
6 changes: 3 additions & 3 deletions crates/wallet/src/wallet/tx_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions crates/wallet/tests/wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
Loading