Mining interface: getCoinbaseMerklePath() and submitSolution() #30955

pull Sjors wants to merge 3 commits into bitcoin:master from Sjors:2024/07/merkle_path changing 6 files +162 −106
  1. Sjors commented at 7:55 AM on September 24, 2024: member

    The new BlockTemplate interface introduced in #30440 allows for a more efficient way for a miner to submit the block solution. Instead of having the send the full block, it only needs to provide the nonce, timestamp, version fields and coinbase transaction.

    This PR introduces submitSolution() for that. It's currently unused.

    #29432 and https://github.com/Sjors/bitcoin/pull/48 use it to process the Stratum v2 message SubmitSolution. The method should be sufficiently generic to work with alternative mining protocols (none exist that I'm aware off).

    This PR also introduces getCoinbaseMerklePath(), which is needed in Stratum v2 to construct the merkle_path field of the NewTemplate message (see spec). The coinbase merkle path is also used in Stratum "v1", see e.g. https://bitcoin.stackexchange.com/questions/109820/questions-on-merkle-root-hashing-for-stratum-pools

    This last function uses BlockMerkleBranch which was moved to the test code in #13191. The reason back then for moving it was that it was no longer used. This PR moves it back.

    This PR does not change behaviour since both methods are unused.

  2. DrahtBot commented at 7:55 AM on September 24, 2024: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK itornaza, tdb3, ryanofsky, achow101
    Concept ACK TheCharlatan

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

  3. TheCharlatan commented at 8:19 AM on September 24, 2024: contributor

    Concept ACK

  4. tdb3 commented at 10:27 AM on September 24, 2024: contributor

    Concept ACK

  5. Panga9309 approved
  6. Move BlockMerkleBranch back to merkle.{h,cpp}
    The Mining interface uses this function in the next commit
to calculate the coinbase merkle path. Stratum v2 uses
this to send a compact work template.

This partially undoes the change in 4defdfab94504018f822dc34a313ad26cedc8255,
but is not a revert, because the implementation changed in the meantime.

This commit also documents the function.
    63d6ad7c89
  7. Add getCoinbaseMerklePath() to Mining interface 47b4875ef0
  8. Add submitSolution to BlockTemplate interface 525e9dcba0
  9. Sjors force-pushed on Sep 26, 2024
  10. Sjors commented at 8:05 AM on September 26, 2024: member

    Rebased after #30510.

  11. itornaza approved
  12. itornaza commented at 8:22 AM on September 26, 2024: contributor

    Code review ACK 525e9dcba0b8c6744bcd3725864f39786afc8ed5

    In order to verify that MerkleComputation(), ComputeMerkleBranch(), BlockMerkleBranch() were copied verbatim from their resting place at src/test/merkle_tests.cpp to their new home src/consensus/merkle.cpp, I copied the two versions of the functions in separate files and compared them with diff. The only difference I noticed is that now the function BlockMerkleBranch() is not declared as static which is correct, because we want to be using it from outside it's compilation unit, like for example from src/node/interfaces.cpp. Built the pr locally and run all unit functional and extended tests and all pass.

  13. DrahtBot requested review from TheCharlatan on Sep 26, 2024
  14. DrahtBot requested review from tdb3 on Sep 26, 2024
  15. Sjors commented at 8:24 AM on September 26, 2024: member

    @itornaza you may also find git diff --color-moved=dimmed-zebra useful.

  16. tdb3 approved
  17. tdb3 commented at 1:56 AM on October 1, 2024: contributor

    Code review and light test ACK 525e9dcba0b8c6744bcd3725864f39786afc8ed5

    Cherry-picked bitcoin-mine from #30437 to exercise some of the interface.

    git cherry-pick aec7e7d897741fe25fbf54995e1825772e0872d7
git cherry-pick 2227afac0372358287fb879f3b0bd07fd653f3f8

make -C depends NO_QT=1 MULTIPROCESS=1
HOST_PLATFORM="x86_64-pc-linux-gnu"
cmake -B build --toolchain=depends/$HOST_PLATFORM/toolchain.cmake
cmake --build build -j18

build/src/bitcoin-node -regtest -printtoconsole -debug=ipc -ipcbind=unix

    Made the following update:

    diff --git a/src/bitcoin-mine.cpp b/src/bitcoin-mine.cpp
index 47e8e4cf52..77bc1e759c 100644
--- a/src/bitcoin-mine.cpp
+++ b/src/bitcoin-mine.cpp
@@ -112,5 +112,14 @@ MAIN_FUNCTION
         tfm::format(std::cout, "Tip hash is null.\n");
     }
 
+    std::vector<unsigned char> v{0x00, 0x14, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x00};
+    CScript coinbase_script = CScript() << std::vector<unsigned char>(v.begin(), v.end());
+    std::unique_ptr<interfaces::BlockTemplate> new_block{mining->createNewBlock(coinbase_script)};
+    auto merkle_path{new_block->getCoinbaseMerklePath()};
+    tfm::format(std::cout, "Printing merkle path:\n");
+    for (auto const& b : merkle_path) {
+        tfm::format(std::cout, "%s\n", b.GetHex());
+    }
+
     return EXIT_SUCCESS;
 }

    and exercised getCoinbaseMerklePath() by viewing the updated merkle path with transactions being added to the mempool.

    $ build/src/bitcoin-mine -regtest -debug
...
Printing merkle path:
$ build/src/bitcoin-cli -named sendtoaddress address=bcrt1qctncrkr9ucfvhxkkvf70zegrkqm0k8jwxg7m9u amount=1.2 fee_rate=2
4dbc1286bcda6a3c72e3d3f0b4cbf32da6cb69c5013520ecd4203de5cb0c8678
$ build/src/bitcoin-mine -regtest -debug
...
Printing merkle path:
4dbc1286bcda6a3c72e3d3f0b4cbf32da6cb69c5013520ecd4203de5cb0c8678
$ build/src/bitcoin-cli -named sendtoaddress address=bcrt1qctncrkr9ucfvhxkkvf70zegrkqm0k8jwxg7m9u amount=1.2 fee_rate=2
dcd234ee9ce7c8023bfe39109ae1582018ec18842baf6d853147c55248a09e71
$ build/src/bitcoin-mine -regtest -debug
...
Printing merkle path:
dcd234ee9ce7c8023bfe39109ae1582018ec18842baf6d853147c55248a09e71
14cd53f2bc327792c8b8bffd0d19a7b93ed173de904f010b0261d14c243f1225
$ build/src/bitcoin-cli -named sendtoaddress address=bcrt1qctncrkr9ucfvhxkkvf70zegrkqm0k8jwxg7m9u amount=1.2 fee_rate=2
2ee486b5e338d078e46500fc304ce922253422c3301679914ffce78e210a5541
$ build/src/bitcoin-mine -regtest -debug
...
Printing merkle path:
2ee486b5e338d078e46500fc304ce922253422c3301679914ffce78e210a5541
a09d497bde7bf0561fd2c94758640b2fad831d11976d4f732c377753ee2eeca0
$ build/src/bitcoin-cli -named sendtoaddress address=bcrt1qctncrkr9ucfvhxkkvf70zegrkqm0k8jwxg7m9u amount=1.2 fee_rate=2
f670812c2bf2b625891120f46888761c216850aaefd9ca70c234c4709d5f5d9c
$ build/src/bitcoin-mine -regtest -debug
...
Printing merkle path:
2ee486b5e338d078e46500fc304ce922253422c3301679914ffce78e210a5541
a09d497bde7bf0561fd2c94758640b2fad831d11976d4f732c377753ee2eeca0
9f41cd8210ee51050fb766931f59299f12d3adb9979b9d9d6066312f71969e84
$ build/src/bitcoin-cli -named sendtoaddress address=bcrt1qctncrkr9ucfvhxkkvf70zegrkqm0k8jwxg7m9u amount=1.2 fee_rate=2
d2742062a2641b43ca532fc2b292a36b67456dd6f32877389468937331267117
$ build/src/bitcoin-mine -regtest -debug
...
Printing merkle path:
4dbc1286bcda6a3c72e3d3f0b4cbf32da6cb69c5013520ecd4203de5cb0c8678
afaf34ad1e406fdf5e22cab465098b1ebc9005686e8163940ca0015bc0e1b64e
f0453cea298e82272e4a6afdac5f88b8d8804f59d08ef2c5d2344a96c3d4db07

    Then added lines to exercise submitSolution() with some hard-coded values:

    +    // submit a dummy solution
+
+    // Create coinbase transaction.
+    CMutableTransaction coinbaseTx;
+    coinbaseTx.vin.resize(1);
+    coinbaseTx.vin[0].prevout.SetNull();
+    coinbaseTx.vout.resize(1);
+    coinbaseTx.vout[0].scriptPubKey = coinbase_script;
+    coinbaseTx.vout[0].nValue = 50 * COIN;
+    coinbaseTx.vin[0].scriptSig = CScript() << tip->height + 1 << OP_0;
+
+    bool processed{new_block->submitSolution(4, 1727746274, 0, coinbaseTx)};
+    if (processed) {
+        tfm::format(std::cout, "Successfully processed block\n");
+    } else {
+        tfm::format(std::cout, "Unable to process block\n");
+    }

    submitSolution() was returning false when, for example, proof-of-work was insufficient (2024-10-01T01:38:57Z [error] ProcessNewBlock: AcceptBlock FAILED (high-hash, proof of work failed)). I was initially confused by this because the function documentation states: @returns if the block was processed, independent of block validity but then realized this is essentially echoing ProcessNewBlock()'s documentation:

    https://github.com/bitcoin/bitcoin/blob/fc642c33ef28829eda0119a0fe39fd9bc4b84051/src/validation.h#L1204-L1206

  18. in src/consensus/merkle.cpp:87 in 63d6ad7c89 outdated 
      82 | @@ -83,3 +83,106 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated)
  83 |      return ComputeMerkleRoot(std::move(leaves), mutated);
  84 |  }
  85 |  
  86 | +/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */
  87 | +static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector<uint256>* pbranch) {
    ryanofsky commented at 4:00 PM on October 8, 2024:

    In commit "Move BlockMerkleBranch back to merkle.{h,cpp}" (63d6ad7c89cd682f6e779968c4861ea085058356)

    Not important, and this is not really an uninformed opinion, but to me the phrase "merkle path" is more descriptive than "merkle branch" and I think it might be worth adding a followup commit to replace branch with path everywhere in this file so it is using consistent terminology.

    Specifically might suggest: ComputeMerkleBranch -> ComputeMerklePath, BlockMerkleBranch -> TransactionMerklePath, pbranch -> path, branchpos -> path_pos.

    Sjors commented at 6:07 PM on October 31, 2024:

    Done in #31197, though I used leaf_pos.

  19. in src/consensus/merkle.h:35 in 63d6ad7c89 outdated 
      30 | + * @param[in] block the block
  31 | + * @param[in] position transaction for which to calculate the merkle path, defaults to coinbase
  32 | + *
  33 | + * @return merkle path ordered from the deepest
  34 | + */
  35 | +std::vector<uint256> BlockMerkleBranch(const CBlock& block, uint32_t position = 0);
    ryanofsky commented at 4:04 PM on October 8, 2024:

    In commit "Move BlockMerkleBranch back to merkle.{h,cpp}" (63d6ad7c89cd682f6e779968c4861ea085058356)

    Not important, but IMO it would be clearer to drop position = 0 default argument so transaction has to be specified explicitly, and rename s/Block/Transaction/ to be clear this is returning merkle path to a specific transaction.

    Sjors commented at 6:07 PM on October 31, 2024:

    Done in #31197

  20. in src/interfaces/mining.h:58 in 525e9dcba0 
      53 | +    /**
  54 | +     * Construct and broadcast the block.
  55 | +     *
  56 | +     * @returns if the block was processed, independent of block validity
  57 | +     */
  58 | +    virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) = 0;
    ryanofsky commented at 4:13 PM on October 8, 2024:

    In commit "Add submitSolution to BlockTemplate interface" (525e9dcba0b8c6744bcd3725864f39786afc8ed5)

    It might make more sense for this function to take a CTransactionRef parameter instead of a CMutableTransaction parameter, since it is just going to turn the CMutableTransaction into a CTransactionRef anyway.

    Sjors commented at 6:07 PM on October 31, 2024:

    Done in #31197

  21. ryanofsky approved
  22. ryanofsky commented at 4:15 PM on October 8, 2024: contributor

    Code review ACK 525e9dcba0b8c6744bcd3725864f39786afc8ed5. Left minor suggestions but none are important, and looks like this could be merged as-is

  23. achow101 commented at 12:11 AM on October 10, 2024: member

    ACK 525e9dcba0b8c6744bcd3725864f39786afc8ed5

  24. achow101 merged this on Oct 10, 2024
  25. achow101 closed this on Oct 10, 2024
  26. Sjors deleted the branch on Oct 31, 2024
  27. TheCharlatan referenced this in commit 8bb47d4c2c on Nov 2, 2024
  28. ryanofsky referenced this in commit a95a8ba3a3 on Dec 17, 2024
  29. bug-castercv502 referenced this in commit 403bebd591 on Sep 28, 2025
  30. bitcoin locked this on Oct 31, 2025
Contributors
SjorsDrahtBotTheCharlatantdb3Panga9309itornazaryanofskyachow101


TheCharlatan

Linked (view graph)
#13191 Specialized double-SHA256 with 64 byte inputs with SSE4.1 and AVX2#28722 Multiprocess tracking issue#29432 Stratum v2 Template Provider (take 3)#30440 Have createNewBlock() return a BlockTemplate interface#31002 rfc: DATUM mining interface requirements#31197 refactor: mining interface 30955 followups
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-05-02 15:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me