feat: refactor VerifyLeafHashes, VerifyNamespace, and VerifyInclusion to expose computing root and validations outside of Proof

[gupadhyaya]

Fixes #290

Summary by CodeRabbit

• New Features
  • Enhanced data integrity verification with a refined cryptographic proof process and improved error handling.
• Refactor
  • Streamlined the underlying verification flow to simplify the validation logic and bolster system reliability.
  • Isolated root computation logic into a dedicated method for improved modularity.

[coderabbitai]

Walkthrough

The pull request refactors the namespace proof verification logic by introducing several new methods to modularize and clarify the code. It adds IsValidEmptyRangeProof for explicit empty proof validation, ComputeLeafHashes for leaf hash computation, and ValidateProofStructure, ValidateSingleNamespace, and ValidateCompleteness for comprehensive validation steps. The existing VerifyLeafHashes method is rewritten to incorporate these validations and to return both a validity boolean and error. The previously inline root computation is extracted into a new public method ComputeRoot, which reconstructs the Merkle root from leaf hashes and proof nodes with improved iteration and error handling. Overall, the changes improve separation of concerns, error reporting, and code clarity.

Changes

File(s)

Change Summary

proof.go

Refactored namespace proof verification by adding new methods: IsValidEmptyRangeProof, ComputeLeafHashes, ValidateProofStructure, ValidateSingleNamespace, and ValidateCompleteness. Rewrote VerifyLeafHashes to use these validations and return (bool, error). Extracted root computation into a new public method ComputeRoot with improved leaf range estimation and loop style. Simplified VerifyNamespace and VerifyInclusion by delegating to these new methods and removing redundant checks.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant VerifyLeafHashes
    participant Validate
    participant ComputeLeafHashes
    participant ComputeRoot

    Client->>VerifyLeafHashes: VerifyLeafHashes(nth, verifyCompleteness, nID, leafHashes, root)
    VerifyLeafHashes->>Validate: ValidateProofStructure(...)
    Validate-->>VerifyLeafHashes: Validation result
    VerifyLeafHashes->>Validate: ValidateSingleNamespace(...)
    Validate-->>VerifyLeafHashes: Validation result
    VerifyLeafHashes->>Validate: ValidateCompleteness(...) (if requested)
    Validate-->>VerifyLeafHashes: Validation result
    VerifyLeafHashes->>ComputeRoot: ComputeRoot(nth, leafHashes)
    ComputeRoot-->>VerifyLeafHashes: computedRoot or error
    VerifyLeafHashes-->>Client: (bool valid, error)

Loading

Assessment against linked issues

Objective (Issue #)	Addressed	Explanation
Expose root computation as a separate function (#290)	✅

Poem

I'm a bunny in the code garden, hopping with delight,
New ComputeRoot shines like a carrot in the light,
Old tangled logic is now trimmed and neat,
With recursive hops, the tree's roots meet,
Celebrate the modular magic—hop, skip, and byte! 🐰💻

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5dee5b and 61fbcbf.

📒 Files selected for processing (1)

• proof.go (6 hunks)

🔇 Additional comments (13)

proof.go (13)

144-164: Well-structured extraction of empty range proof validation logic.

This new method cleanly encapsulates the empty range proof validation that was previously embedded in multiple methods. The implementation correctly handles the key validation cases:

1. Structural validation (empty proof with no leaves)
2. Namespace verification (checking if nID is outside the namespace range or if the tree is empty)

---

166-194: Good abstraction for leaf hash computation.

This helper function nicely consolidates two patterns previously duplicated across the codebase:

1. Validation of existing leaf hashes with namespace checking
2. Prepending namespace IDs to raw leaf data before hashing

The error handling is comprehensive, and the memory pre-allocation in the byte appending operation is efficient.

---

219-222: Appropriate delegation to specialized helper method.

Replacing the inline empty range proof validation with a call to the dedicated helper improves readability and ensures consistent validation logic.

---

228-233: Improved leaf hash computation with better error handling.

Using the extracted ComputeLeafHashes function here removes duplication and adds proper error handling that was previously missing.

---

243-291: Comprehensive proof structure validation in a dedicated method.

This extraction centralizes all proof structure validation checks into a single method with detailed error messaging. The validation is thorough, covering:

• Proof range validity
• Leaf hash count verification
• Namespace compatibility checking
• Node format validation for proof nodes and leaf hashes

This significantly improves the maintainability of the verification logic.

---

293-303: Clean separation of namespace validation concerns.

This helper method focuses specifically on ensuring all leaf hashes belong to the queried namespace, checking both min and max namespace bounds. This single-responsibility approach makes the validation logic easier to understand and maintain.

---

305-335: Clear extraction of completeness validation logic.

The completeness check has been properly isolated into its own method, making the overall verification flow more understandable. The implementation correctly verifies that no leaves of the queried namespace exist outside the proof range.

---

337-350: Improved documentation for the ComputeRoot method.

The enhanced documentation clearly explains the method's purpose, parameters, and return values, making it more accessible for users of the package. This addresses the PR's goal of clarifying the root computation process.

---

399-400: Simplified leaf subtree size estimation with max function.

Using max(getSplitPoint(proof.end)*2, 1) improves the code by combining what was previously a conditional check into a single expression, making it more concise without sacrificing readability.

---

404-410: More idiomatic iteration over proof nodes.

The switch to a range-based loop for iterating over proof nodes is more idiomatic in Go and reduces the potential for index errors. The error handling is also more detailed now.

---

413-452: Well-structured refactoring of the VerifyLeafHashes method.

This is the core of the PR's refactoring goal - separating root computation from verification. The implementation now:

1. Delegates validation to specialized methods
2. Returns both a boolean result and a detailed error
3. Has a clear, logical flow from validation to computation to verification

This design makes the code more maintainable and the error reporting more useful for debugging.

---

463-466: Consistent use of helper method for empty range proof validation.

Similar to the change in VerifyNamespace, this properly delegates to the specialized helper, ensuring consistent validation behavior across different verification methods.

---

469-472: Improved leaf hash computation with consistent error handling.

This change completes the pattern of using the extracted ComputeLeafHashes function, ensuring consistent behavior and error handling across the package.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[rootulp]

Overall seems fine to me. Left a few optional nits and refactors that are safe to ignore if that code was just copied from the previous implementation.

It would be nice to include unit tests for the extracted helper methods.

[walldiss]

Nice changes and refactor!

Agree with above that nmt pkg will benefit from unit tests for new exported funcs.

[evan-forbes]

any remaining blocking reviews @vgonkivs ? or can we merge and flup

[vgonkivs]

LGTM

[Copilot]

Pull Request Overview

This PR refactors the core verification methods in the proof package to isolate root computation logic and expose validations externally, while also extending test coverage for various proof scenarios.

• Refactored verification functions to separate basic validation and root computation.
• Added helper functions for computing and validating leaf hashes and prefixed leaf hashes.
• Expanded tests to cover namespace mismatch, proof structure violations, and completeness conditions.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
proof_test.go	New tests covering ComputeRootWithBasicValidation, namespace and completeness validations
proof.go	Refactored proof verification APIs; added validation helper functions and improved error handling

Comments suppressed due to low confidence (1)

proof.go:249

• [nitpick] Although the proof range check is effective, consider splitting the condition into separately named variables for proof start and end to improve readability and future maintainability.

if proof.Start() < 0 || proof.Start() >= proof.End() {

[rootulp]

[no change needed] I don't understand the refactor anymore because so many lines were modified. One idea for a future refactor is to split the proof struct into multiple structs. For example: one struct for absence proof and one for inclusion proof. Then we can split this large file into two smaller files.

I don't see any issues besides the AI slop in the tests so seems safe to merge after addressing that comment.