Skip to content

bugfix: restore node_db_prefix's previous prefix/suffix calculation#476

Merged
malcolmgreaves merged 1 commit into
mainfrom
mg/restore_node_db_prefix_behavior
Apr 21, 2026
Merged

bugfix: restore node_db_prefix's previous prefix/suffix calculation#476
malcolmgreaves merged 1 commit into
mainfrom
mg/restore_node_db_prefix_behavior

Conversation

@malcolmgreaves
Copy link
Copy Markdown
Collaborator

@malcolmgreaves malcolmgreaves commented Apr 21, 2026

The hexadecimal formatting for MerkleHash does not fix the size of the resulting string.
Leading 0s are dropped. This means that node_db_prefix could index into a string that
is shorter than 4 characters, causing a panic. This PR restores the previous hash directory
prefix and suffix calculation. Hash values that are smaller than 4 hex characters can result
in malformed or empty-string named directories.

Additionally, the test_hex_hash_conversions_and_node_db_prefix test was fixed to
correctly run 1k randomized MerkleHash -> HexHash conversions. Before, it was only
running a single randomized conversion.

@malcolmgreaves malcolmgreaves force-pushed the mg/restore_node_db_prefix_behavior branch from a30e20e to 7553912 Compare April 21, 2026 22:22
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f589ba41-28e9-4712-96b7-0b968498bfeb

📥 Commits

Reviewing files that changed from the base of the PR and between 7553912 and 774bb26.

📒 Files selected for processing (1)
  • crates/lib/src/model/merkle_tree/merkle_hash.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/lib/src/model/merkle_tree/merkle_hash.rs

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Improved internal string handling in the merkle tree hash computation module for better code maintainability and consistency.
    • Updated test structure formatting.

Walkthrough

Refactors HexHash::node_db_prefix to build directory-name components by iterating over the hash string's characters and collecting Strings, replacing prior byte-index substring slicing; a test loop syntax was simplified from for _ in [0..1000] to for _ in 0..1000.

Changes

Cohort / File(s) Summary
Merkle Hash Refactor
crates/lib/src/model/merkle_tree/merkle_hash.rs
Replaced byte-offset slicing (&hash_str[0..3], [3..]) with character-based construction (chars().take(...).collect::<String>(), chars().skip(...).collect::<String>()) for directory components; updated Path::new(...).join(...) to use the new String refs. Minor test loop syntax cleaned (for _ in 0..1000).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • CleanCut

Poem

🐰 I hopped through hex with careful art,
Snipped by chars instead of byte by byte,
Paths still lead where they should start,
Clean joins under moonlight. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'bugfix: restore node_db_prefix's previous prefix/suffix calculation' clearly and specifically summarizes the main change—restoring previous hash directory calculation logic to fix a panic bug.
Description check ✅ Passed The description is directly related to the changeset, explaining the bug (dropped leading zeros causing string indexing panics), the fix (restoring previous calculation), and the test improvement (1k conversions instead of one).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mg/restore_node_db_prefix_behavior

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/lib/src/model/merkle_tree/merkle_hash.rs`:
- Around line 128-130: node_db_prefix() can return a single-segment path when
hash_str is too short; ensure a true 2-level path by guaranteeing dir_suffix is
never empty: if hash_str.len() <= DIR_PREFIX_LEN then left-pad the hex string
with leading '0's up to DIR_PREFIX_LEN + 1 before computing dir_prefix and
dir_suffix (use the existing DIR_PREFIX_LEN, node_db_prefix(), dir_prefix and
dir_suffix symbols to locate the logic). Also add deterministic unit tests for
short values (< 0x1000) such as 0x0, 0xa, 0xff, 0xabc to assert the returned
Path always has two segments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a160176-2481-47f4-8032-9b85377e8fb9

📥 Commits

Reviewing files that changed from the base of the PR and between c5ea20a and 7553912.

📒 Files selected for processing (1)
  • crates/lib/src/model/merkle_tree/merkle_hash.rs

Comment thread crates/lib/src/model/merkle_tree/merkle_hash.rs
@malcolmgreaves malcolmgreaves force-pushed the mg/restore_node_db_prefix_behavior branch from 7553912 to 23517f3 Compare April 21, 2026 22:33
The hexadecimal formatting for `MerkleHash` does not fix the size of the resulting string.
Leading 0s are dropped. This means that `node_db_prefix` could index into a string that
is shorter than 4 characters, causing a panic. This PR restores the previous hash directory
prefix and suffix calculation. Hash values that are smaller than 4 hex characters can result
in malformed or empty-string named directories.

Additionally, the `test_hex_hash_conversions_and_node_db_prefix` test was fixed to
correctly run 1k randomized `MerkleHash` -> `HexHash` conversions. Before, it was only
running a single randomized conversion.
@malcolmgreaves malcolmgreaves force-pushed the mg/restore_node_db_prefix_behavior branch from 23517f3 to 774bb26 Compare April 21, 2026 22:33
@malcolmgreaves malcolmgreaves enabled auto-merge (squash) April 21, 2026 22:52
@malcolmgreaves malcolmgreaves merged commit a47434e into main Apr 21, 2026
9 checks passed
@malcolmgreaves malcolmgreaves deleted the mg/restore_node_db_prefix_behavior branch April 21, 2026 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants