From be270551df30dd42b4f1b664234d0c22c09be625 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 5 Nov 2025 20:57:20 +0100 Subject: [PATCH 1/3] merkle: migrate `path` arg of `MerkleComputation` to a reference There's a single call to the methods from `ComputeMerklePath` where the last parameter is always provided. This simplifies the implementation by not having to check for missing parameter. --- src/consensus/merkle.cpp | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index e274ed821a5..94e8effeaaf 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -85,9 +85,9 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) } /* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ -static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t leaf_pos, std::vector* path) +static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t leaf_pos, std::vector& path) { - if (path) path->clear(); + path.clear(); Assume(leaves.size() <= UINT32_MAX); if (leaves.size() == 0) { if (pmutated) *pmutated = false; @@ -115,13 +115,11 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot // corresponds to an inner value that existed before processing the // current leaf, and each needs a hash to combine it. for (level = 0; !(count & ((uint32_t{1}) << level)); level++) { - if (path) { - if (matchh) { - path->push_back(inner[level]); - } else if (matchlevel == level) { - path->push_back(h); - matchh = true; - } + if (matchh) { + path.push_back(inner[level]); + } else if (matchlevel == level) { + path.push_back(h); + matchh = true; } mutated |= (inner[level] == h); h = Hash(inner[level], h); @@ -147,8 +145,8 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot // If we reach this point, h is an inner value that is not the top. // We combine it with itself (Bitcoin's special rule for odd levels in // the tree) to produce a higher level one. - if (path && matchh) { - path->push_back(h); + if (matchh) { + path.push_back(h); } h = Hash(h, h); // Increment count to the value it would have if two entries at this @@ -157,13 +155,11 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot level++; // And propagate the result upwards accordingly. while (!(count & ((uint32_t{1}) << level))) { - if (path) { - if (matchh) { - path->push_back(inner[level]); - } else if (matchlevel == level) { - path->push_back(h); - matchh = true; - } + if (matchh) { + path.push_back(inner[level]); + } else if (matchlevel == level) { + path.push_back(h); + matchh = true; } h = Hash(inner[level], h); level++; @@ -176,7 +172,7 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot static std::vector ComputeMerklePath(const std::vector& leaves, uint32_t position) { std::vector ret; - MerkleComputation(leaves, nullptr, nullptr, position, &ret); + MerkleComputation(leaves, nullptr, nullptr, position, ret); return ret; } From 63d640fa6a7090b4e615153b42ebf2e48d909db0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 5 Nov 2025 21:00:24 +0100 Subject: [PATCH 2/3] merkle: remove unused `proot` and `pmutated` args from `MerkleComputation` There's a single call to the methods from `ComputeMerklePath` where these were always set to `nullptr`. --- src/consensus/merkle.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index 94e8effeaaf..d7f1435f15e 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -84,17 +84,14 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) return ComputeMerkleRoot(std::move(leaves), mutated); } -/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ -static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t leaf_pos, std::vector& path) +/* This implements a constant-space merkle path calculator, limited to 2^32 leaves. */ +static void MerkleComputation(const std::vector& leaves, uint32_t leaf_pos, std::vector& path) { path.clear(); Assume(leaves.size() <= UINT32_MAX); if (leaves.size() == 0) { - if (pmutated) *pmutated = false; - if (proot) *proot = uint256(); return; } - bool mutated = false; // count is the number of leaves processed so far. uint32_t count = 0; // inner is an array of eagerly computed subtree hashes, indexed by tree @@ -121,7 +118,6 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot path.push_back(h); matchh = true; } - mutated |= (inner[level] == h); h = Hash(inner[level], h); } // Store the resulting hash at inner position level. @@ -165,14 +161,11 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot level++; } } - // Return result. - if (pmutated) *pmutated = mutated; - if (proot) *proot = h; } static std::vector ComputeMerklePath(const std::vector& leaves, uint32_t position) { std::vector ret; - MerkleComputation(leaves, nullptr, nullptr, position, ret); + MerkleComputation(leaves, position, ret); return ret; } From 24ed820d4f0d8f7fa2f69e1909c2d98f809d2f94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Thu, 6 Nov 2025 11:26:17 +0100 Subject: [PATCH 3/3] merkle: remove unused `mutated` arg from `BlockWitnessMerkleRoot` The `mutated` parameter is never used at any call site - all callers pass `nullptr`. The explicit comment in `validation.cpp` explains the reason: // The malleation check is ignored; as the transaction tree itself // already does not permit it, it is impossible to trigger in the // witness tree. --- src/consensus/merkle.cpp | 4 ++-- src/consensus/merkle.h | 3 +-- src/test/fuzz/merkle.cpp | 4 ++-- src/validation.cpp | 4 ++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index d7f1435f15e..703a824e16f 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -73,7 +73,7 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) return ComputeMerkleRoot(std::move(leaves), mutated); } -uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) +uint256 BlockWitnessMerkleRoot(const CBlock& block) { std::vector leaves; leaves.resize(block.vtx.size()); @@ -81,7 +81,7 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) for (size_t s = 1; s < block.vtx.size(); s++) { leaves[s] = block.vtx[s]->GetWitnessHash().ToUint256(); } - return ComputeMerkleRoot(std::move(leaves), mutated); + return ComputeMerkleRoot(std::move(leaves)); } /* This implements a constant-space merkle path calculator, limited to 2^32 leaves. */ diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h index c722cbe446f..29282d217d2 100644 --- a/src/consensus/merkle.h +++ b/src/consensus/merkle.h @@ -20,9 +20,8 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated = nullptr); /* * Compute the Merkle root of the witness transactions in a block. - * *mutated is set to true if a duplicated subtree was found. */ -uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr); +uint256 BlockWitnessMerkleRoot(const CBlock& block); /** * Compute merkle path to the specified transaction diff --git a/src/test/fuzz/merkle.cpp b/src/test/fuzz/merkle.cpp index 4bb91faf0b8..9ba461bbd6b 100644 --- a/src/test/fuzz/merkle.cpp +++ b/src/test/fuzz/merkle.cpp @@ -51,7 +51,7 @@ FUZZ_TARGET(merkle) } // Test ComputeMerkleRoot - bool mutated = fuzzed_data_provider.ConsumeBool(); + bool mutated = fuzzed_data_provider.ConsumeBool(); // output param, initial value shouldn't matter const uint256 merkle_root = ComputeMerkleRoot(tx_hashes, &mutated); // Basic sanity checks for ComputeMerkleRoot @@ -66,7 +66,7 @@ FUZZ_TARGET(merkle) } if (!block->vtx.empty()){ - const uint256 block_witness_merkle_root = BlockWitnessMerkleRoot(*block, &mutated); + const uint256 block_witness_merkle_root = BlockWitnessMerkleRoot(*block); if (tx_hashes.size() == 1) { assert(block_witness_merkle_root == uint256()); } diff --git a/src/validation.cpp b/src/validation.cpp index af523b06d74..aaf80b33928 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3996,7 +3996,7 @@ static bool CheckWitnessMalleation(const CBlock& block, bool expect_witness_comm // The malleation check is ignored; as the transaction tree itself // already does not permit it, it is impossible to trigger in the // witness tree. - uint256 hash_witness = BlockWitnessMerkleRoot(block, /*mutated=*/nullptr); + uint256 hash_witness = BlockWitnessMerkleRoot(block); CHash256().Write(hash_witness).Write(witness_stack[0]).Finalize(hash_witness); if (memcmp(hash_witness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { @@ -4109,7 +4109,7 @@ std::vector ChainstateManager::GenerateCoinbaseCommitment(CBlock& int commitpos = GetWitnessCommitmentIndex(block); std::vector ret(32, 0x00); if (commitpos == NO_WITNESS_COMMITMENT) { - uint256 witnessroot = BlockWitnessMerkleRoot(block, nullptr); + uint256 witnessroot = BlockWitnessMerkleRoot(block); CHash256().Write(witnessroot).Write(ret).Finalize(witnessroot); CTxOut out; out.nValue = 0;