Removed non-safe variant of array-hash. - #9988
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
This change is |
98d8fa8 to
b681488
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 2 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on dorimedini-starkware and TomerStarkware).
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit 1751c55. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6814882cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dorimedini-starkware
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
b681488 to
06e0d00
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi resolved 1 discussion.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on dorimedini-starkware and TomerStarkware).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 06e0d00. Configure here.
No need for compatibility with old version of sha256 - as existed only to not break the existing interfaces.
06e0d00 to
1751c55
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
Summary
compute_sha512_u64_array_safehas been removed and merged intocompute_sha512_u64_array, which now directly acceptslast_input_num_bytesas au3(aBoundedInt<0, 7>) instead of au32. The oldcompute_sha512_u64_arraywrapper that performed a runtimedowncastand panicked on values greater than 7 has been eliminated. Tests have been updated to callcompute_sha512_u64_arraydirectly.Type of change
Please check one:
Why is this change needed?
Having two separate functions — one that panicked at runtime and one that enforced the bound via the type system — was redundant. The
u3type already statically guarantees thatlast_input_num_bytesis in the range0..=7, making the runtime-panicking wrapper unnecessary. Consolidating into a single function with a typed parameter removes the possibility of a runtime panic and simplifies the public API.What was the behavior or documentation before?
compute_sha512_u64_arrayaccepted au32forlast_input_num_bytesand panicked at runtime if the value exceeded 7. A separatecompute_sha512_u64_array_safefunction accepted au3to enforce the bound at the type level.What is the behavior or documentation after?
compute_sha512_u64_arraynow acceptslast_input_num_bytesas au3directly, enforcing the0..=7bound at compile time.compute_sha512_u64_array_safeno longer exists.Related issue or discussion (if any)
Additional context
The
u3type alias (BoundedInt<0, 7>) remains part of the public API insha512.