rpc: fix getblock(header) returns target for tip #33446

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2025/09/getblock-target changing 5 files +43 −14
  1. Sjors commented at 4:52 PM on September 20, 2025: member

    A target field was added to the getblock and getblockheader RPC calls in #31583, but it mistakingly always used the tip value.

    This PR fixes it to return the target for the given block. Because regtest does not have difficulty adjustment, the mainnet test is expanded to cover the fix.

    A preliminary commit deals with mining block 2016 that's needed for the test. It also:

    • renames the create_coinbase retarget_period argument to halving_period. Before #31583 this was hardcoded for regtest where these values are the same.
    • drops unused fees argument from mine helper
    • expands the CPU miner instructions for generating the alternative mainnet chain

    Fixes #33440

  2. DrahtBot added the label RPC/REST/ZMQ on Sep 20, 2025
  3. DrahtBot commented at 4:52 PM on September 20, 2025: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK luke-jr, sipa, TheCharlatan, ismaelsadeeq

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. Sjors commented at 4:56 PM on September 20, 2025: member

    This should probably be backported to v29 and v30.

  5. test: add block 2016 to mock mainnet
    The next commit requires an additional mainnet block which changes the difficulty.
    
    Also fix a few minor mistakes in the test (suite):
    - rename the create_coinbase retarger_period argument to halving_period. Before bitcoin#31583 this was hardcoded for regtest where these values are the same.
    - drop unused fees argument from mine helper
    
    Finally the CPU miner instructions for generating the alternative mainnet chain are expanded.
    4c3c1f42cf
  6. rpc: fix getblock(header) returns target for tip
    A target field was added to the getblock and getblockheader RPC calls in bitcoin#31583, but it mistakingly always used the tip value.
    
    Because regtest does not have difficulty adjustment, a test is added for mainnet instead.
    bf7996cbc3
  7. Sjors force-pushed on Sep 20, 2025
  8. Sjors commented at 7:38 PM on September 20, 2025: member

    I split 88fb025c83c7aa84766a5385e6a797102dd94513 in two commits: one that adds the mock mainnet block and updates the docs, and one that's focussed on fixing the actual bug and adds test coverage for it (using this new block).

  9. ismaelsadeeq commented at 5:07 PM on September 22, 2025: member

    Concept ACK

  10. luke-jr approved
  11. luke-jr commented at 10:08 AM on September 23, 2025: contributor

    crACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188

  12. DrahtBot requested review from ismaelsadeeq on Sep 23, 2025
  13. luke-jr referenced this in commit 5ffd91a988 on Sep 23, 2025
  14. luke-jr referenced this in commit 950b6beb56 on Sep 23, 2025
  15. sipa commented at 12:12 PM on September 23, 2025: member

    utACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188

  16. maflcko added the label Needs backport (29.x) on Sep 24, 2025
  17. maflcko added the label Needs backport (30.x) on Sep 24, 2025
  18. in test/functional/data/README.md:11 in 4c3c1f42cf outdated
      10 | @@ -11,17 +11,20 @@ The alternate mainnet chain was generated as follows:
      11 |  - restart node with a faketime 2 minutes later
    


    ismaelsadeeq commented at 11:56 AM on September 24, 2025:

    In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd

    nit: commit message typo s/retarger_period/retarget_period/g


    Sjors commented at 1:03 PM on September 24, 2025:

    Will change if I need to retouch.

  19. sedited approved
  20. sedited commented at 12:24 PM on September 24, 2025: contributor

    ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188

  21. in test/functional/test_framework/blocktools.py:147 in 4c3c1f42cf outdated
     143 | @@ -144,7 +144,7 @@ def script_BIP34_coinbase_height(height):
     144 |      return CScript([CScriptNum(height)])
     145 |  
     146 |  
     147 | -def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_script=None, fees=0, nValue=50, retarget_period=REGTEST_RETARGET_PERIOD):
     148 | +def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_script=None, fees=0, nValue=50, halving_period=REGTEST_RETARGET_PERIOD):
    


    ismaelsadeeq commented at 12:24 PM on September 24, 2025:

    In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd

    This is a bit confusing @Sjors you updated the name to halving_period but still use the regtest retaget period constant.

    The constant should be updated REGTEST_SUBSIDY_HALVING_INTERVAL for regtest and also define the interval for mainet too, it can be reused in other places.


    Sjors commented at 1:01 PM on September 24, 2025:

    I could rename it if I have to retouch, but at the time didn't feel like adding another constant :-)


    Sjors commented at 12:30 PM on September 8, 2026:

    #36192 fixes this

  22. in test/functional/data/README.md:50 in 4c3c1f42cf outdated
      42 | @@ -40,3 +43,8 @@ The timestamp was not kept constant because at difficulty 1 it's not sufficient
      43 |  to only grind the nonce. Grinding the extra_nonce or version field instead
      44 |  would have required additional (stratum) software. It would also make it more
      45 |  complicated to reconstruct the blocks in this test.
      46 | +
      47 | +The `getblocktemplate` RPC code needs to be patched to ignore not being connected
      48 | +to any peers, and to ignore the IBD status check.
      49 | +
      50 | +On macOS use `faketime "@$t"` instead.
    


    ismaelsadeeq commented at 12:28 PM on September 24, 2025:

    In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd

    Is this okay to be here? Shouldn't it be in an issue to discuss the tradeoff and why those checks are added in the first place and why it should be removed?


    Sjors commented at 1:02 PM on September 24, 2025:

    It just describes how to generate the data for this test, I'm not proposing actually removing these checks in the repository.

  23. ismaelsadeeq commented at 12:36 PM on September 24, 2025: member

    Code review ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188

    Comments are on tests and doc

  24. fanquake merged this on Sep 24, 2025
  25. fanquake closed this on Sep 24, 2025

  26. fanquake referenced this in commit 4ec30d53ec on Sep 24, 2025
  27. fanquake referenced this in commit 1e348bc55a on Sep 24, 2025
  28. fanquake removed the label Needs backport (30.x) on Sep 24, 2025
  29. fanquake commented at 2:11 PM on September 24, 2025: member

    Backported to 30.x in #33473.

  30. fanquake removed the label Needs backport (29.x) on Sep 24, 2025
  31. fanquake referenced this in commit 118abf4c30 on Sep 24, 2025
  32. fanquake referenced this in commit 22ab141243 on Sep 24, 2025
  33. fanquake commented at 2:41 PM on September 24, 2025: member

    Backported to 29.x in #33474.

  34. Sjors deleted the branch on Sep 24, 2025
  35. fanquake referenced this in commit d1b5d4e9ca on Oct 3, 2025
  36. fanquake referenced this in commit d5e0077bef on Oct 6, 2025
  37. sedited referenced this in commit 3f14dd4b2a on Oct 7, 2025
  38. sedited referenced this in commit 5f9d179b8d on Oct 8, 2025
  39. sedited referenced this in commit 845b93d99e on Oct 8, 2025
  40. yuvicc referenced this in commit ccce70c31a on Oct 8, 2025
  41. sedited referenced this in commit 3b1e26d5d3 on Oct 10, 2025
  42. sedited referenced this in commit 0fa2fdaca0 on Oct 11, 2025
  43. stringintech referenced this in commit f74b275ffc on Oct 14, 2025
  44. stickies-v referenced this in commit a785fe6759 on Nov 5, 2025
  45. morozow referenced this in commit a24ef55328 on May 8, 2026
  46. morozow referenced this in commit 729b46a8ff on May 8, 2026
  47. morozow referenced this in commit 6dc0859c03 on May 8, 2026
  48. morozow referenced this in commit b29bbb0557 on May 8, 2026
  49. morozow referenced this in commit ff55cf5afd on May 8, 2026
  50. morozow referenced this in commit 73340c2fa6 on May 8, 2026
  51. Kino1994 referenced this in commit 9fa945cdee on Jun 28, 2026
  52. BigcoinBGC referenced this in commit 965829a954 on Jun 30, 2026
  53. Kino1994 referenced this in commit 7461506b61 on Aug 19, 2026

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: 2026-09-09 07:56 UTC