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.

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

feat: Add sha512 patches #1

Merged
merged 1 commit into from
Sep 30, 2024
Merged

feat: Add sha512 patches #1

merged 1 commit into from
Sep 30, 2024

Conversation

wwared
Copy link

@wwared wwared commented Sep 27, 2024

Companion PR: argumentcomputer/sphinx#179 and argumentcomputer/zk-light-clients#256

Note: The Sphinx PR should be merged before this one

Copy link

@adr1anh adr1anh left a comment

Choose a reason for hiding this comment

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

Looks good, and again noting an optimization that can be implemented later on.

zkvm_state[8] = 0; // set initial `i`
for _ in 0..80 {
unsafe {
syscall_sha512_compress(w.as_mut_ptr(), zkvm_state.as_mut_ptr());
Copy link

Choose a reason for hiding this comment

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

We can replace zkvm_state with a pointer to (i, &k[i])

Copy link
Author

Choose a reason for hiding this comment

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

I've opted to instead use a global ZKVM_STATE array for now, which enables sharing of the K constants between all compress calls.

@wwared wwared force-pushed the zkvm-next branch 3 times, most recently from 6c9f3b1 to 1d397ef Compare September 30, 2024 12:39
@wwared wwared marked this pull request as ready for review September 30, 2024 12:48
@storojs72 storojs72 self-requested a review September 30, 2024 13:59
@wwared wwared merged commit 03ff816 into zkvm Sep 30, 2024
@wwared wwared deleted the zkvm-next branch September 30, 2024 16:21
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.

3 participants