Fixes #31684
Currently generateblock will insert the specified transactions directly into the new block without collecting fees in the coinbase transaction.
This patch changes the code to take transaction fees into account and make it behave closer to the expected miner behavior in mainnet.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31775.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process. A summary of reviews will appear here.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed. <sub>Debug: https://github.com/bitcoin/bitcoin/runs/36509660013</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
143 | @@ -144,8 +144,16 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock() 144 | 145 | int nPackagesSelected = 0; 146 | int nDescendantsUpdated = 0; 147 | - if (m_mempool) { 148 | + if (m_options.use_mempool) { 149 | addPackageTxs(nPackagesSelected, nDescendantsUpdated); 150 | + } else if (m_options.txs.size() > 0) { 151 | + EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs);
19:24:04.713] /ci_container_base/src/node/miner.cpp:150:9: error: 'exclusive_locks_required' attribute cannot be applied to a statement
[19:24:04.713] 150 | EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs);
[19:24:04.713] | ^ ~
[19:24:04.713] /ci_container_base/src/threadsafety.h:31:54: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
[19:24:04.713] 31 | #define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_required(__VA_ARGS__)))
The code doesn't pass existing tests, and doesn't have new tests. Also, the approach won't work anyway, because the transactions may not be in the mempool. You'd have to calculate the fees based on the prevout amounts for each transaction.
I agree with @maflcko!
The block reward should include both the block subsidy and the transaction fees from all transactions included in the block.
Is there a use case for this?
generateblockis a test-only RPC, so there is no need to write extra code to collect the fees. Note that the othergenerate*calls will collect the fees.
Closing for now. For future contributions, my recommendation would be to compile the code and run the tests. Also, new tests should be included for new features.