-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SHA256 support for variable message_size
to be hashed
#4909
Comments
@guipublic mentioned this should be approachable to implement in pure Noir now with the @michaelelliot you mentioned attempting to implement this in pure Noir but was met with poor performance some while ago. Was that before or already using |
Since I said it would be easy, I figured that I might just implement it. I confirm that the performance is good, the cost is the same as if you would hash the full message. |
# Description ## Problem\* Resolves #4909 ## Summary\* Thanks to the sha256compression opcode, we can create conditionally the blocks to be hashed. As a result there is no additional cost to support variable input size for sha256, except of course that we pay for the full input len (even if we hash less). ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [X] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
Problem
The SHA256 hash function in the stdlib doesn't currently support hashing of variable actual lengths of inputs.
This hinders use cases that rely on:
Happy Case
Extend the SHA256 library to support an additional
message_size
argument, similar to Keccak256: noir-lang/acvm#313Project Impact
Blocker
Impact Context
For projects verifying passport ownerships, sizes of data to hash varies from passport to passport.
Workaround
None
Workaround Description
No response
Additional Context
Credit to @madztheo @michaelelliot for flagging.
Would you like to submit a PR for this Issue?
None
Support Needs
No response
The text was updated successfully, but these errors were encountered: