-
Notifications
You must be signed in to change notification settings - Fork 417
Description
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
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+1001 or higher.
...
This means that miners may include such a transaction in their block template when their chain tip has a height of h+991.
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
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
The correct test should be: can't be spend at
Footnotes
Metadata
Metadata
Assignees
Labels
Type
Projects
Status