From f0a21831087410687c4ca31ac00e44f380b859be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 22 Nov 2025 13:30:04 +0100 Subject: [PATCH 1/3] test: adjust `ComputeMerkleRoot` tests Update the integer fuzz test to move the vector into `ComputeMerkleRoot`, matching production usage patterns and avoiding unnecessary copies. Update `merkle_test_BlockWitness` to use an odd number of transactions to ensure the test covers the scenario where leaf duplication occurs. Also switch to `GetWitnessHash` to match `BlockWitnessMerkleRoot` semantics. The manual vector setup retains the exact-size `resize` to explicitly verify the behavior against the calculated root. --- src/test/fuzz/integer.cpp | 4 ++-- src/test/merkle_tests.cpp | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 89c29dbcb70..3effc1c19f0 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -80,8 +80,8 @@ FUZZ_TARGET(integer, .init = initialize_integer) } constexpr uint256 u256_min{"0000000000000000000000000000000000000000000000000000000000000000"}; constexpr uint256 u256_max{"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"}; - const std::vector v256{u256, u256_min, u256_max}; - (void)ComputeMerkleRoot(v256); + std::vector v256{u256, u256_min, u256_max}; + (void)ComputeMerkleRoot(std::move(v256)); (void)DecompressAmount(u64); { if (std::optional parsed = ParseMoney(FormatMoney(i64))) { diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 649da07cf7e..3a5720fedd5 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -232,8 +232,9 @@ BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness) { CBlock block; - block.vtx.resize(2); - for (std::size_t pos = 0; pos < block.vtx.size(); pos++) { + constexpr size_t vtx_count{3}; + block.vtx.resize(vtx_count); + for (std::size_t pos = 0; pos < vtx_count; pos++) { CMutableTransaction mtx; mtx.nLockTime = pos; block.vtx[pos] = MakeTransactionRef(std::move(mtx)); @@ -242,12 +243,12 @@ BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness) uint256 blockWitness = BlockWitnessMerkleRoot(block); std::vector hashes; - hashes.resize(block.vtx.size()); - hashes[0].SetNull(); - hashes[1] = block.vtx[1]->GetHash().ToUint256(); + hashes.resize(vtx_count); // Note: leaving odd count to exercise old behavior + for (size_t pos{1}; pos < vtx_count; ++pos) { + hashes[pos] = block.vtx[pos]->GetWitnessHash().ToUint256(); + } uint256 merkleRootofHashes = ComputeMerkleRoot(hashes); - BOOST_CHECK_EQUAL(merkleRootofHashes, blockWitness); } BOOST_AUTO_TEST_SUITE_END() From 7fd47e0e56087cd3b5fd76a532bdc3ac331e832e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 14 May 2025 14:32:08 +0200 Subject: [PATCH 2/3] bench: make `MerkleRoot` benchmark more representative Two versions are run now, one with the mutation calculations, the other without. To avoid unwanted compiler optimizations, we assert the expected hash, which should inhibit aggressive optimization. To make the benchmark more similar to production `ComputeMerkleRoot` call sites, the input leaves-copying is made explicit before each run. > ./build/bin/bench_bitcoin -filter='MerkleRoot.*' -min-time=1000 | ns/leaf | leaf/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 44.18 | 22,634,858.70 | 0.0% | 1.10 | `MerkleRoot` | 44.66 | 22,390,601.03 | 0.0% | 1.10 | `MerkleRootWithMutation` Massif memory measurements show the excessive memory reservations: MB 1.332^ : | # : | # : | # : | # : | # @ : | # @ : | # @ : | # @ : | # @ : | # @ : | # @ : | # @ : | #::::@::::::::::::::::::::::::::::::::::::::::::::::::::::::@:::::@:::: | #: ::@::::: :::::::: :: ::: :::::: : : :: ::: ::: : : : ::::@:::::@:::: | #: ::@::::: :::::::: :: ::: :::::: : : :: ::: ::: : : : ::::@:::::@:::: | #: ::@::::: :::::::: :: ::: :::::: : : :: ::: ::: : : : ::::@:::::@:::: | #: ::@::::: :::::::: :: ::: :::::: : : :: ::: ::: : : : ::::@:::::@:::: | #: ::@::::: :::::::: :: ::: :::::: : : :: ::: ::: : : : ::::@:::::@:::: | #: ::@::::: :::::::: :: ::: :::::: : : :: ::: ::: : : : ::::@:::::@:::: 0 +----------------------------------------------------------------------->s 0 226.2 showing the reallocations clearly in the stacks: 97.87% (1,366,841B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc. ->41.25% (576,064B) 0x969717: allocate (new_allocator.h:151) | ->41.25% (576,064B) 0x969717: allocate (allocator.h:203) | ->41.25% (576,064B) 0x969717: allocate (alloc_traits.h:614) | ->41.25% (576,064B) 0x969717: _M_allocate (stl_vector.h:387) | ->41.25% (576,064B) 0x969717: _M_realloc_append (vector.tcc:572) | ->41.25% (576,064B) 0x969717: push_back (stl_vector.h:1427) | ->41.25% (576,064B) 0x969717: ComputeMerkleRoot(std::vector >, bool*) (merkle.cpp:55) | ->41.25% (576,064B) 0x2235A7: operator() (merkle_root.cpp:31) | ->41.25% (576,064B) 0x2235A7: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run --- src/bench/merkle_root.cpp | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/bench/merkle_root.cpp b/src/bench/merkle_root.cpp index 98ce197ea58..5be592707ff 100644 --- a/src/bench/merkle_root.cpp +++ b/src/bench/merkle_root.cpp @@ -7,21 +7,33 @@ #include #include +#include #include static void MerkleRoot(benchmark::Bench& bench) { - FastRandomContext rng(true); - std::vector leaves; - leaves.resize(9001); - for (auto& item : leaves) { + FastRandomContext rng{/*fDeterministic=*/true}; + + std::vector hashes{}; + hashes.resize(9001); + for (auto& item : hashes) { item = rng.rand256(); } - bench.batch(leaves.size()).unit("leaf").run([&] { - bool mutation = false; - uint256 hash = ComputeMerkleRoot(std::vector(leaves), &mutation); - leaves[mutation] = hash; - }); + + constexpr uint256 expected_root{"d8d4dfd014a533bc3941b8663fa6e7f3a8707af124f713164d75b0c3179ecb08"}; + for (bool mutate : {false, true}) { + bench.name(mutate ? "MerkleRootWithMutation" : "MerkleRoot").batch(hashes.size()).unit("leaf").run([&] { + std::vector leaves; + leaves.resize(hashes.size()); + for (size_t s = 0; s < hashes.size(); s++) { + leaves[s] = hashes[s]; + } + + bool mutated{false}; + const uint256 root{ComputeMerkleRoot(std::move(leaves), mutate ? &mutated : nullptr)}; + assert(root == expected_root); + }); + } } BENCHMARK(MerkleRoot, benchmark::PriorityLevel::HIGH); From 3dd815f048c80c9a35f01972e0537eb42531aec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 22 Nov 2025 13:33:09 +0100 Subject: [PATCH 3/3] validation: pre-reserve leaves to prevent reallocs with odd vtx count `ComputeMerkleRoot` duplicates the last hash when the input size is odd. If the caller provides a `std::vector` whose capacity equals its size, that extra `push_back` forces a reallocation, doubling its capacity (allocating 3x the necessary memory). This affects roughly half of the created blocks (those with odd transaction counts), causing unnecessary memory fragmentation during every block validation. Fix this by pre-reserving the vector capacity to account for the odd-count duplication. The expression `(size + 1) & ~1ULL` adds 1 to the size and clears the last bit, effectively rounding up to the next even number. This syntax produces optimal assembly across x86/ARM and 32/64-bit platforms for gcc/clang, see https://godbolt.org/z/xzscoq7nv. Also switch from `resize` to `reserve` + `push_back` to eliminate the default construction of `uint256` objects that are immediately overwritten. > ./build/bin/bench_bitcoin -filter='MerkleRoot.*' -min-time=1000 | ns/leaf | leaf/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 43.73 | 22,867,350.51 | 0.0% | 1.10 | `MerkleRoot` | 44.17 | 22,640,349.14 | 0.0% | 1.10 | `MerkleRootWithMutation` Massif memory measurements after show 0.8 MB peak memory usage KB 801.4^ # | # | # | # | # | # | # | # :::::@:::::@: | #:::@@@::@:::::::::::::::@::@:@:::@@:::::::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@: 0 +----------------------------------------------------------------------->s 0 227.5 and the stacks don't show reallocs anymore: 96.37% (790,809B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc. ->35.10% (288,064B) 0x2234AF: allocate (new_allocator.h:151) | ->35.10% (288,064B) 0x2234AF: allocate (allocator.h:203) | ->35.10% (288,064B) 0x2234AF: allocate (alloc_traits.h:614) | ->35.10% (288,064B) 0x2234AF: _M_allocate (stl_vector.h:387) | ->35.10% (288,064B) 0x2234AF: reserve (vector.tcc:79) | ->35.10% (288,064B) 0x2234AF: ToMerkleLeaves, MerkleRoot(ankerl::nanobench::Bench&):::: > (merkle.h:19) | ->35.10% (288,064B) 0x2234AF: operator() (merkle_root.cpp:25) | ->35.10% (288,064B) 0x2234AF: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> --- src/bench/merkle_root.cpp | 4 ++-- src/consensus/merkle.cpp | 10 +++++----- src/signet.cpp | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bench/merkle_root.cpp b/src/bench/merkle_root.cpp index 5be592707ff..0e4c779f142 100644 --- a/src/bench/merkle_root.cpp +++ b/src/bench/merkle_root.cpp @@ -24,9 +24,9 @@ static void MerkleRoot(benchmark::Bench& bench) for (bool mutate : {false, true}) { bench.name(mutate ? "MerkleRootWithMutation" : "MerkleRoot").batch(hashes.size()).unit("leaf").run([&] { std::vector leaves; - leaves.resize(hashes.size()); + leaves.reserve((hashes.size() + 1) & ~1ULL); // capacity rounded up to even for (size_t s = 0; s < hashes.size(); s++) { - leaves[s] = hashes[s]; + leaves.push_back(hashes[s]); } bool mutated{false}; diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index 703a824e16f..b0819a1763e 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -66,9 +66,9 @@ uint256 ComputeMerkleRoot(std::vector hashes, bool* mutated) { uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) { std::vector leaves; - leaves.resize(block.vtx.size()); + leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even for (size_t s = 0; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s]->GetHash().ToUint256(); + leaves.push_back(block.vtx[s]->GetHash().ToUint256()); } return ComputeMerkleRoot(std::move(leaves), mutated); } @@ -76,10 +76,10 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) uint256 BlockWitnessMerkleRoot(const CBlock& block) { std::vector leaves; - leaves.resize(block.vtx.size()); - leaves[0].SetNull(); // The witness hash of the coinbase is 0. + leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even + leaves.emplace_back(); // The witness hash of the coinbase is 0. for (size_t s = 1; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s]->GetWitnessHash().ToUint256(); + leaves.push_back(block.vtx[s]->GetWitnessHash().ToUint256()); } return ComputeMerkleRoot(std::move(leaves)); } diff --git a/src/signet.cpp b/src/signet.cpp index 6524ebff45e..6c1df371aa9 100644 --- a/src/signet.cpp +++ b/src/signet.cpp @@ -58,10 +58,10 @@ static bool FetchAndClearCommitmentSection(const std::span header static uint256 ComputeModifiedMerkleRoot(const CMutableTransaction& cb, const CBlock& block) { std::vector leaves; - leaves.resize(block.vtx.size()); - leaves[0] = cb.GetHash().ToUint256(); + leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even + leaves.push_back(cb.GetHash().ToUint256()); for (size_t s = 1; s < block.vtx.size(); ++s) { - leaves[s] = block.vtx[s]->GetHash().ToUint256(); + leaves.push_back(block.vtx[s]->GetHash().ToUint256()); } return ComputeMerkleRoot(std::move(leaves)); }