Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add SHA512 #4

Merged
merged 12 commits into from
Oct 20, 2023
Merged

Add SHA512 #4

merged 12 commits into from
Oct 20, 2023

Conversation

varunthakore
Copy link
Contributor

bellpepper-sha512 crate implements circuit for SHA-512 hash function and circuit representation of u64.

@avras
Copy link
Collaborator

avras commented Sep 28, 2023

I have reviewed this code before, when Varun submitted the SHA512 gadget to the ZKP MOOC hackathon.

It's ready to be merged.

avras
avras previously approved these changes Sep 28, 2023
View reviewed changes
huitseeker
huitseeker reviewed Oct 16, 2023
View reviewed changes
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

This looks great overall! thank you so much!
I left a few comments here and there that should improve testing & maintainability a bit.

crates/bellpepper-sha512/src/uint64.rs Outdated Show resolved Hide resolved
crates/bellpepper-sha512/src/sha512.rs Outdated Show resolved Hide resolved
crates/bellpepper-sha512/README.md Outdated Show resolved Hide resolved
crates/bellpepper-sha512/src/uint64.rs Outdated Show resolved Hide resolved
@avras
Copy link
Collaborator

avras commented Oct 17, 2023

Thanks @huitseeker for the comments. Sorry about the missing the right SHA-512 reference in the README.

@varunthakore Can you also change the directory name from bellperson-sha512 to sha512? Like the changes in #5. Thanks.

@varunthakore
Copy link
Contributor Author

Changed the directory name to sha512 and made changes as suggested by @huitseeker.

huitseeker
huitseeker reviewed Oct 19, 2023
View reviewed changes
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

Thanks a bunch! Just one cosmetic change, and we can get this one in!

crates/sha512/src/sha512.rs Outdated Show resolved Hide resolved
huitseeker
huitseeker approved these changes Oct 20, 2023
View reviewed changes
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

🙏 Thanks a lot!!

@huitseeker huitseeker merged commit db5efa4 into lurk-lab:main Oct 20, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@huitseeker huitseeker huitseeker approved these changes

@avras avras Awaiting requested review from avras

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@varunthakore @avras @huitseeker