Skip to content

refactor: add digest_file for hashing file content#6855

Merged
hanabi1224 merged 4 commits intomainfrom
hm/digest-file
Apr 7, 2026
Merged

refactor: add digest_file for hashing file content#6855
hanabi1224 merged 4 commits intomainfrom
hm/digest-file

Conversation

@hanabi1224
Copy link
Copy Markdown
Contributor

@hanabi1224 hanabi1224 commented Apr 6, 2026

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 6, 2026

Walkthrough

The PR introduces streaming hash utility functions in a new utils::hash module and migrates existing hashing code to use them. Dependencies for digest, md-5, and sha2 are upgraded to version 0.11 to support the streaming approach via the new digest-io dependency.

Changes

Cohort / File(s) Summary
Dependency Ecosystem Upgrade
Cargo.toml
Added digest = "0.11" and digest-io = "0.1" dependencies. Bumped md-5 from 0.10 to 0.11 and sha2 from 0.10 to 0.11 (keeping default-features = false).
Streaming Hash Utilities
src/utils/hash/mod.rs, src/utils/mod.rs
Introduced new public module with digest_stream<H, R> and digest_file<H> functions for streaming hash computation using digest_io::IoWrapper. Added module export in utils.
Hash Call Site Migrations
src/state_manager/utils.rs, src/utils/io/writer_checksum.rs, src/utils/net/download_file.rs, src/utils/proofs_api/parameters.rs
Updated hashing logic across multiple files to use new streaming utilities. Changed error handling in download_file.rs from Option to Result and updated checksum formatting in tests. Refactored BufReader initialization in parameters.rs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • akaladarshi
  • sudo-shashank
  • LesnyRumcajs
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring: introducing a digest_file function for hashing file content, which is the primary addition across all changed files.

✏️ 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 hm/digest-file
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch hm/digest-file

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

@hanabi1224 hanabi1224 marked this pull request as ready for review April 6, 2026 11:38
@hanabi1224 hanabi1224 requested a review from a team as a code owner April 6, 2026 11:38
@hanabi1224 hanabi1224 requested review from akaladarshi and sudo-shashank and removed request for a team April 6, 2026 11:38
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.

🧹 Nitpick comments (2)
src/utils/hash/mod.rs (2)

14-15: Minor: H: Update bound may be redundant.

In digest 0.11, the Digest trait already requires Update, so H: Digest alone should suffice. This is a very minor nit and doesn't affect correctness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/hash/mod.rs` around lines 14 - 15, The generic bound on H in the
function/impl (the clause "where H: Digest + Update") is redundant because the
Digest trait already requires Update in digest 0.11; remove the explicit
"Update" bound and leave "where H: Digest" (update any occurrences of that
where-clause on the functions/impls referencing H such as the Digest-related
function signatures) to simplify the constraints.

12-30: Consider adding doc comments for public functions.

These functions are pub and will be part of the module's API. Adding brief doc comments would improve discoverability and maintainability.

📝 Suggested doc comments
+/// Computes the digest of data read from a stream.
+///
+/// Reads from `reader` until EOF, feeding all bytes into the hasher `H`,
+/// and returns the finalized digest.
 #[allow(dead_code)]
 pub fn digest_stream<H, R>(reader: &mut R) -> std::io::Result<Output<H>>
 where
     H: Digest + Update,
     R: Read,
 {
     let mut hasher = IoWrapper(H::new());
     std::io::copy(reader, &mut hasher)?;
     Ok(hasher.0.finalize())
 }

+/// Computes the digest of a file's contents.
+///
+/// Opens the file at `path`, reads it through a buffered reader,
+/// and returns the finalized digest.
 #[allow(dead_code)]
 pub fn digest_file<H>(path: impl AsRef<Path>) -> std::io::Result<Output<H>>
 where
     H: Digest + Update,
 {
     let mut reader = BufReader::new(File::open(path)?);
     digest_stream::<H, _>(&mut reader)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/hash/mod.rs` around lines 12 - 30, Add brief doc comments for the
public functions to document their purpose, parameters and return value: add a
doc comment above pub fn digest_stream<H, R>(reader: &mut R) describing that it
reads from the given reader, computes and returns the digest Output<H> and may
return std::io::Error, and add a doc comment above pub fn digest_file<H>(path:
impl AsRef<Path>) describing that it opens the file at path, wraps it in a
BufReader, and returns the computed digest via digest_stream; mention trait
bounds (H: Digest + Update) and the possible I/O errors in both comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/utils/hash/mod.rs`:
- Around line 14-15: The generic bound on H in the function/impl (the clause
"where H: Digest + Update") is redundant because the Digest trait already
requires Update in digest 0.11; remove the explicit "Update" bound and leave
"where H: Digest" (update any occurrences of that where-clause on the
functions/impls referencing H such as the Digest-related function signatures) to
simplify the constraints.
- Around line 12-30: Add brief doc comments for the public functions to document
their purpose, parameters and return value: add a doc comment above pub fn
digest_stream<H, R>(reader: &mut R) describing that it reads from the given
reader, computes and returns the digest Output<H> and may return std::io::Error,
and add a doc comment above pub fn digest_file<H>(path: impl AsRef<Path>)
describing that it opens the file at path, wraps it in a BufReader, and returns
the computed digest via digest_stream; mention trait bounds (H: Digest + Update)
and the possible I/O errors in both comments.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fb8162e2-4755-43d8-9cdd-5a2f50e75a55

📥 Commits

Reviewing files that changed from the base of the PR and between 45f66c3 and c4b99ca.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • src/state_manager/utils.rs
  • src/utils/hash/mod.rs
  • src/utils/io/writer_checksum.rs
  • src/utils/mod.rs
  • src/utils/net/download_file.rs
  • src/utils/proofs_api/parameters.rs

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 6, 2026

Codecov Report

❌ Patch coverage is 84.37500% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.95%. Comparing base (d0dc5b8) to head (d650268).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/utils/hash/mod.rs 85.71% 0 Missing and 2 partials ⚠️
src/utils/net/download_file.rs 81.81% 0 Missing and 2 partials ⚠️
src/state_manager/utils.rs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/utils/io/writer_checksum.rs 82.40% <100.00%> (-0.17%) ⬇️
src/utils/mod.rs 82.45% <ø> (ø)
src/utils/proofs_api/parameters.rs 83.56% <100.00%> (-0.23%) ⬇️
src/state_manager/utils.rs 79.80% <0.00%> (ø)
src/utils/hash/mod.rs 85.71% <85.71%> (ø)
src/utils/net/download_file.rs 80.68% <81.81%> (-0.14%) ⬇️

... and 5 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0dc5b8...d650268. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hanabi1224 hanabi1224 added the RPC requires calibnet RPC checks to run on CI label Apr 6, 2026
@hanabi1224 hanabi1224 added this pull request to the merge queue Apr 7, 2026
Merged via the queue into main with commit 0e5709f Apr 7, 2026
37 checks passed
@hanabi1224 hanabi1224 deleted the hm/digest-file branch April 7, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPC requires calibnet RPC checks to run on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants