test: improve HashBuilder test coverage #123
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add property-based tests to verify correctness of optimized implementations: HashBuilder tests: - arbitrary_trie_root_raw_keys: Fixed-length keys match reference implementation - arbitrary_trie_root_with_updates: Updates enabled produces same root - arbitrary_deterministic_root: Building twice produces identical results - arbitrary_branch_updates_valid: Branch node masks satisfy invariants - arbitrary_common_prefix_stress: Keys with shared prefixes handled correctly - arbitrary_single_leaf: Single leaf produces valid root - arbitrary_add_leaf_unchecked_equivalence: Unchecked API matches checked API Node tests: - unpack_path_to_nibbles_equivalence: Optimized prepend matches original join - unpack_path_roundtrip: encode_path_leaf and unpack are inverses Amp-Thread-ID: https://ampcode.com/threads/T-019bf5e4-6d0c-76ab-b9e6-008403664f8a Co-authored-by: Amp <[email protected]>
Add tests for previously untested modules: - src/mask.rs: TrieMask tests for all methods including new, get, from_nibble, is_bit_set, is_empty, count_bits, first_set_bit_index, set_bit, unset_bit, is_subset_of, bitwise ops, shift ops, and out-of-range panic behavior. Added proptests for invariants. - src/nodes/rlp.rs: RlpNode tests for from_raw, from_raw_rlp, from_rlp, word_rlp, is_hash, as_hash, boundary conditions (31 vs 32 bytes), max length (33), decode error paths, and proptests. - src/proof/proof_nodes.rs: ProofNodes tests for matching_nodes_iter, matching_nodes, matching_nodes_sorted, insert, nodes_sorted, into_nodes_sorted, into_inner, extend, extend_from, overwrite semantics, empty key behavior, and proptests. Coverage improved from ~80% to 84.62% overall, with newly tested files at 96-100% line coverage. Amp-Thread-ID: https://ampcode.com/threads/T-019bf5fc-c2bc-749a-9deb-a958ea57e01d Co-authored-by: Amp <[email protected]>
Add proptest coverage improvements to prevent vacuous test passes: 1. arbitrary_branch_updates_complete (ignored): - New test that verifies branch updates are complete for multi-leaf tries - Marked #[ignore] because it exposes the store_branch_node bug (PR #25) - Uses prop_assume!(len >= 2) to focus on meaningful cases 2. Prevent empty/trivial input issues in existing proptests: - arbitrary_hashed_root: require non-empty state - arbitrary_trie_root_raw_keys: require >= 2 entries - arbitrary_trie_root_with_updates: require >= 2 entries - arbitrary_deterministic_root: require >= 2 entries - arbitrary_common_prefix_stress: require >= 2 entries - arbitrary_add_leaf_unchecked_equivalence: require >= 2 entries These changes ensure proptests exercise meaningful code paths (branching, updates machinery) rather than passing vacuously on empty/singleton inputs. Empty trie behavior is already covered by the dedicated empty() unit test. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019bf62a-fcc3-770a-b813-cf3151d34bf1
zerosnacks
changed the title
zerosnacks
changed the title
zerosnacks
force-pushed
the
zerosnacks/proptest-coverage
branch
from
c7bef22 to
7f64ea9
Compare
Investigate and validate two potential issues raised by oracle code review: Issue 1: store_branch_node unconditionally sets parent's hash_masks bit - NOT A BUG: hash_mask means 'child is a branch node' for BranchNodeCompact - NOT 'child is RLP-hashed (>=32 bytes)' - Confirmed by test_hash_mask_large_leaf_value Issue 2: update_masks uses is_empty() check instead of specific bit - NOT A BUG: check is scoped to current subtree path, not global - Confirmed by multiple tests showing no mask leakage Tests added: - test_tree_mask_no_sibling_contamination - test_tree_mask_isolation_multiple_branches - test_hash_mask_semantics_inlined_branch - test_hash_mask_branch_vs_leaf_children - test_tree_mask_propagation_through_extensions - test_tree_mask_complex_nesting - test_hash_mask_large_leaf_value - test_mask_isolation_successive_subtrees Documented mask semantics: - state_mask: slot is occupied (child exists) - tree_mask: child is stored in database - hash_mask: child is a branch node (hash in BranchNodeCompact.hashes) Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019bf62a-fcc3-770a-b813-cf3151d34bf1
zerosnacks
force-pushed
the
zerosnacks/proptest-coverage
branch
from
7f64ea9 to
3fc8c1e
Compare
Amp-Thread-ID: https://ampcode.com/threads/T-019bf64d-c82f-7682-9933-a37c563468a6 Co-authored-by: Amp <[email protected]>
zerosnacks
commented
Jan 28, 2026
zerosnacks
commented
Jan 28, 2026
zerosnacks
commented
Jan 28, 2026
zerosnacks
commented
Jan 28, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
I was learning more about MPTs this weekend and noticed the tests are lacking. In order to do any future optimizations safely we need to first expand the test suite.
Summary
This PR adds comprehensive test coverage for the
HashBuilderand related components. No source code is modified - all changes are within#[cfg(test)]modules.Test Coverage Added
Proptest coverage (HashBuilder):
arbitrary_hashed_root- Verify root hash matches reference implementationarbitrary_trie_root_raw_keys- Test with raw 32-byte keys (2-100 entries)arbitrary_trie_root_with_updates- Verify updates don't change rootarbitrary_deterministic_root- Building twice produces identical rootsarbitrary_branch_updates_complete- Completeness check for multi-leaf triesarbitrary_branch_updates_valid- Validate mask invariants on updatesarbitrary_common_prefix_stress- Stress test with shared prefixesarbitrary_single_leaf- Single leaf root verificationarbitrary_add_leaf_unchecked_equivalence- Verify unchecked API equivalenceMask semantics tests (validated via oracle review):
test_tree_mask_no_sibling_contamination- tree_mask isolationtest_tree_mask_isolation_multiple_branches- stored vs non-stored branchestest_tree_mask_propagation_through_extensions- propagation through extensionstest_tree_mask_complex_nesting- multi-depth scenariostest_hash_mask_semantics_inlined_branch- inlined node behaviortest_hash_mask_branch_vs_leaf_children- confirms hash_mask = "branch child"test_hash_mask_large_leaf_value- large leaf doesn't set hash_masktest_mask_isolation_successive_subtrees- no leakage across insertsAdditional unit tests:
TrieMask- bit operations, properties, roundtrips (20 tests)RlpNode- encoding, hashing, properties (19 tests)ProofNodes- iteration, matching, extension (17 tests)HashBuilderValue- roundtrip properties (10 tests)root- ordered trie root functions (8 tests)Documented Mask Semantics
Based on investigation, the masks have these semantics:
state_mask: "slot is occupied" (child exists at nibble)tree_mask: "child is stored in database" (stored_in_database=true)hash_mask: "child is a branch node" (hash stored inBranchNodeCompact.hashes)Note:
hash_maskdoes NOT mean "child is RLP-hashed (≥32 bytes)" - it tracks branch children for the compact format.Test Results