test: add unit test coverage for the empty leaves path in MerkleComputation #33858

pull frankomosh wants to merge 1 commits into bitcoin:master from frankomosh:merkle-empty-path changing 1 files +5 −0

frankomosh commented at 12:08 AM on November 12, 2025: contributor

As noted in #32243 (comment), the early return inside MerkleComputation when leaves.size() == 0 was only exercised by fuzz tests.

The existing merkle_test_empty_block calls BlockMerkleRoot, which uses ComputeMerkleRoot, but does not exercise the TransactionMerklePath → ComputeMerklePath → MerkleComputation code path.

Coverage before adding test: <img width="2459" height="66" alt="before" src="https://github.com/user-attachments/assets/ca94015a-d7c2-4281-ac60-13b22f177b67" />

Coverage after adding test: <img width="2459" height="66" alt="after" src="https://github.com/user-attachments/assets/b1d4e1bb-af72-46ab-8898-f18db39dd2fb" />
test: exercise TransactionMerklePath with empty block; targets the MerkleComputation empty-leaves path that was only reached by fuzz tests ffcae82a68
DrahtBot added the label Tests on Nov 12, 2025
DrahtBot commented at 12:08 AM on November 12, 2025: contributor



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



Code Coverage & Benchmarks

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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK kevkevinpal, brunoerg, maflcko, sedited

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
maflcko commented at 8:14 AM on November 12, 2025: member

Coverage before adding test: <img alt="before" width="2000" height="66" src="https://github.com/user-attachments/assets/28cb35b1-5127-4f82-ba74-5891bd36553c">

Coverage after adding test: <img alt="after" width="2000" height="66" src="https://github.com/user-attachments/assets/41ebbd66-872e-4ba4-b550-84ff9bc61752">

I don't think those links work. Also, this is redundant to the corecheck result anyway?
frankomosh commented at 11:16 AM on November 12, 2025: contributor

I don't think those links work.

I'm not sure why they didn't render correctly here. [Edit: I think it renders now]

Also, this is redundant to the corecheck result anyway?

I included them mainly to visually show % increase, but you’re right that they may be redundant. (also, I think you have been against adding tests without showing any proof of coverage)
kevkevinpal commented at 3:07 PM on November 12, 2025: contributor

ACK ffcae82

According to corecheck it looks like there was an increase in coverage https://corecheck.dev/bitcoin/bitcoin/pulls/33858

Type	Reviewers
ACK	kevkevinpal, brunoerg, maflcko, sedited

in src/test/merkle_tests.cpp:160 in ffcae82a68

 153 | @@ -154,6 +154,11 @@ BOOST_AUTO_TEST_CASE(merkle_test_empty_block)
 154 |  
 155 |      BOOST_CHECK_EQUAL(root.IsNull(), true);
 156 |      BOOST_CHECK_EQUAL(mutated, false);
 157 | +
 158 | +    // Verify TransactionMerklePath handles empty block correctly
 159 | +    // This tests the early-return path in MerkleComputation
 160 | +    std::vector<uint256> merkle_path = TransactionMerklePath(block, 0);

brunoerg commented at 7:24 PM on November 20, 2025:

nit:

    std::vector<uint256> merkle_path = TransactionMerklePath(block, /*position=*/0);

frankomosh commented at 7:17 PM on November 24, 2025:

Thanks for suggestion. Can we keep it this way for now?

brunoerg approved
brunoerg commented at 7:28 PM on November 20, 2025: contributor

code review ACK ffcae82a68104c1992964b26c592b62cbca391bf

It's just a simple test addition to exercise MerkleComputation with an empty vector of leaves, it's ok. Note that it just checks the path is empty which is fine since there is a proposal to remove the unused pmutated and proot (https://github.com/bitcoin/bitcoin/pull/33805).
maflcko commented at 10:26 AM on November 21, 2025: member

lgtm ACK ffcae82a68104c1992964b26c592b62cbca391bf
sedited approved
sedited commented at 1:35 PM on December 4, 2025: contributor

ACK ffcae82a68104c1992964b26c592b62cbca391bf

rfm?
frankomosh commented at 12:22 PM on December 5, 2025: contributor

rfm?

I think it is
fanquake merged this on Dec 5, 2025
fanquake closed this on Dec 5, 2025
frankomosh deleted the branch on Dec 6, 2025

Contributors

Labels

Linked (view graph)

#32243 test: added fuzz coverage for consensus/merkle.cpp

test: add unit test coverage for the empty leaves path in MerkleComputation #33858

Code Coverage & Benchmarks

Reviews