From ef501a63d9d65fce49bd633c424e0bbabead7ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 26 Apr 2026 19:52:20 +0200 Subject: [PATCH] consensus: document merkle mutation root invariant Document the `mutated` output flag on the `ComputeMerkleRoot` declaration and explain in the inner loop why the mutation check runs at every tree level even after a duplicate is found. Add a direct regression test for the duplicate-subtree construction described in the code comments for CVE-2012-2459: `[1,2,3,4,5,6]` and `[1,2,3,4,5,6,5,6]` produce the same root. The test also verifies that mutation detection checks equal pairs before the final pair of a tree level. The existing `merkle_test` already exercises this behavior indirectly through random duplications and old-vs-new comparisons. The new test pins it down explicitly through the `ComputeMerkleRoot` API. Both would fail under a refactor that stops the outer reduction once mutation is detected. Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> --- src/consensus/merkle.cpp | 7 +++++-- src/consensus/merkle.h | 5 +++++ src/test/merkle_tests.cpp | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index dfa23cf897d..df5f95e740a 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -49,12 +49,15 @@ known ways of changing the transactions without affecting the merkle root. */ - - uint256 ComputeMerkleRoot(std::vector hashes, bool* mutated) { bool mutation = false; while (hashes.size() > 1) { if (mutated) { + // Check every level because equal pairs can appear above the leaves, + // as in the [1,2,3,4,5,6,5,6] construction described above. + // Continuing after finding one is redundant, but mutated blocks should + // not propagate through the network anyway, and the total number of + // comparisons is the same as for an unmutated input of the same length. for (size_t pos = 0; pos + 1 < hashes.size(); pos += 2) { if (hashes[pos] == hashes[pos + 1]) mutation = true; } diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h index 446b2df75ae..dc8866bbf18 100644 --- a/src/consensus/merkle.h +++ b/src/consensus/merkle.h @@ -12,6 +12,11 @@ class CBlock; +/** + * Compute a Merkle root from the provided leaf hashes. + * If non-null, `*mutated` is set to true if two identical hashes are paired at + * any tree level before the odd-count hash duplication step, and false otherwise. + */ uint256 ComputeMerkleRoot(std::vector hashes, bool* mutated = nullptr); /* diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index a6dd23adb17..e3faffe4f78 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -200,6 +200,26 @@ BOOST_AUTO_TEST_CASE(merkle_test_OddTxWithRepeatedLastTx_block) BOOST_CHECK_EQUAL(mutated, true); } +BOOST_AUTO_TEST_CASE(merkle_test_mutated_return_value) +{ + // CVE-2012-2459 construction: [1,2,3,4,5,6] and [1,2,3,4,5,6,5,6] produce the same root. + const std::vector leaves{uint256{1}, uint256{2}, uint256{3}, uint256{4}, uint256{5}, uint256{6}}; + auto mutated_leaves{leaves}; + mutated_leaves.insert(mutated_leaves.end(), leaves.end() - 2, leaves.end()); // repeat last two elements + + bool mutated{true}; + const uint256 unmutated_root{ComputeMerkleRoot(leaves, &mutated)}; + BOOST_CHECK(!mutated); + BOOST_CHECK_EQUAL(unmutated_root, ComputeMerkleRoot(mutated_leaves, &mutated)); + BOOST_CHECK( mutated); + + const std::vector nontrailing_duplicate_leaves{uint256{1}, uint256{1}, uint256{3}, uint256{4}}; + mutated = false; + const uint256 nontrailing_duplicate_root{ComputeMerkleRoot(nontrailing_duplicate_leaves, &mutated)}; + BOOST_CHECK(nontrailing_duplicate_root == ComputeMerkleRoot(nontrailing_duplicate_leaves)); + BOOST_CHECK(mutated); +} + BOOST_AUTO_TEST_CASE(merkle_test_LeftSubtreeRightSubtree) { CBlock block, leftSubtreeBlock, rightSubtreeBlock;