wallet: make coinbase that will mature on the next block available for selection #32123
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32123. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
f36e683 to
35b2f01
Compare
|
I understand what you are saying about correctness but @sipa 's concerns with propagation issues are also hard to ignore. Bitcoin Core just tends to be on the more conservative side when it comes to protecting users and BDK might leave protections like this to be handled by the actual wallet implemeters that use BDK. A potential middle ground could be that current behavior is maintained but users get a useful hint in return why they can't spend their funds yet. Then there could be a flag added that allows to spend the funds ( |
|
@luisschwab There are other examples of coins which the Bitcoin Core wallet won't let users spend, for example, unconfirmed coins (unless they are self-created/change). Just like here, that's not a protocol rule, but it's a bad idea, and it's perfectly possible to bypass by using raw transactions. Now, given how unlikely it is in 2025 for any user to use the Bitcoin Core wallet directly for mining payouts, the odds that this is still actually protecting anyone are probably negligible. So @fjahr , I think offering a way to disable this behavior would be huge overkill. It probably doesn't matter either way in practice. But on the other hand, I just see no reason to change this. If direct user mining payouts were common, this would be a useful rule. There is nothing incorrect about the current behavior, and I categorically disagree that the wallet should just allow anything that's legal in the protocol without question. If someone really wants to bypass this, they can always use raw transactions. |
|
What do you think @murchandamus? |
|
#32098 (comment) could be a reason that this is fine to change, but the CI is failing on this pull |
35b2f01 to
913e4ca
Compare
Not sure what is going on with CI, other PRs are failing on Windows as well. edit: #32184 fixes it |
…able for selection
913e4ca to
4619b73
Compare
|
-0 I agree with the rationale given by the others to not make this change. |
|
Concept ACK |
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
|
Closing as it failed to reach consensus. |
Closes #32098.
Currently, only coinbase UTXOs that are mature at height h are available for coin selection. This PR makes UTXOs that mature at height h+1 available for selection.
Since a transaction spending this output will be accepted by the mempool, it should be available for selection.
BDK implements this logic1, which I believe to be the correct one.
$ bitcoin-cli -regtest createwallet wallet { "name": "wallet" } $ export ADDRESS=$(bitcoin-cli -regtest -rpcwallet=wallet getnewaddress) $ bitcoin-cli -regtest generatetoaddress 101 $ADDRESS [ "467fd440d0ed89d0cad64fa64d0320968b33a32c681b6c3f5a5ed1d1c016f45a", "1b2246eaa35689710acc5d1bb2923197993233fb9186f0bd561cb546c7667ca7", ... "5d5ee44ada395ab1d978a77596ed3893be750840229e7234a85a01984057798a", "7f8a573a6824edc5342cf86e5e7c0a6ea5664305ef21808456fc98b2621b41cf" ] $ bitcoin-cli -regtest -rpcwallet=wallet getbalance - 50.00000000 + 100.00000000