Skip to content

Clarify zero-multiple handling in jf-utils padding helpers - #890

Open
daixihegu wants to merge 1 commit into
EspressoSystems:mainfrom
daixihegu:main
Open

daixihegu wants to merge 1 commit into
EspressoSystems:mainfrom
daixihegu:main

Conversation

@daixihegu

@daixihegu daixihegu commented Jun 24, 2026 •

Copy link
Copy Markdown

closes: #XXXX

This PR:

This PR clarifies the behavior of jf-utils padding helpers when they are called with a zero multiple.

Previously, compute_len_to_next_multiple(_, 0) panicked with Rust's modulo-by-zero message. This change adds an explicit assertion so callers get a clearer panic message, and documents the panic behavior for both compute_len_to_next_multiple and pad_with_zeros.

This PR does not:

Key places to review:

  • cargo fmt -- --check
  • cargo test -p jf-utils
  • cargo clippy -p jf-utils --all-targets -- -D warnings
  • git diff --check

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (main)
  • Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added relevant changelog entries to the CHANGELOG.md of touched crates.
  • Re-reviewed Files changed in the GitHub PR explorer

Signed-off-by: daixihegu <daixihegu@163.com>
@daixihegu
daixihegu requested a review from mrain as a code owner June 24, 2026 16:42
@CLAassistant

CLAassistant commented Jun 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request documents and clarifies the panic behavior of the padding helper functions when called with a zero multiple. Specifically, it adds an explicit assertion to compute_len_to_next_multiple to ensure multiple is non-zero, documents this behavior in the docstrings of both compute_len_to_next_multiple and pad_with_zeros, adds corresponding unit tests, and updates the CHANGELOG. There are no review comments, so we have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@daixihegu

Copy link
Copy Markdown
Author

@mrain @philippecamacho @akonring Hi, Could you please review this PR at your convenience? Thank you very much.

This branch has not been deployed

No deployments
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