rpc: collect transaction fees on generateblock #31775

pull tiagosh wants to merge 1 commits into bitcoin:master from tiagosh:2025/01/collect-fees-on-generateblock changing 4 files +19 −6
  1. tiagosh commented at 0:07 am on February 1, 2025: none

    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.

  2. DrahtBot commented at 0:07 am on February 1, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31775.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #28676 ([WIP] Cluster mempool implementation by sdaftuar)

    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.

  3. DrahtBot added the label RPC/REST/ZMQ on Feb 1, 2025
  4. tiagosh force-pushed on Feb 1, 2025
  5. DrahtBot commented at 0:21 am on February 1, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/36509660013

    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.

  6. DrahtBot added the label CI failed on Feb 1, 2025
  7. in src/node/miner.cpp:150 in 5a02fbaf86 outdated
    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);
    


    brunoerg commented at 0:56 am on February 1, 2025:
    019:24:04.713] /ci_container_base/src/node/miner.cpp:150:9: error: 'exclusive_locks_required' attribute cannot be applied to a statement
    1[19:24:04.713]   150 |         EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs);
    2[19:24:04.713]       |         ^                                      ~
    3[19:24:04.713] /ci_container_base/src/threadsafety.h:31:54: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
    4[19:24:04.713]    31 | #define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_required(__VA_ARGS__)))
    
  8. tiagosh force-pushed on Feb 1, 2025
  9. rpc: collect transaction fees on generateblock 007caf1163
  10. tiagosh force-pushed on Feb 1, 2025
  11. maflcko commented at 11:33 am on February 4, 2025: member
    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.

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-02-07 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me