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: keccak/sha3 gadgets #14

Merged
merged 3 commits into from
Jan 10, 2024
Merged

feat: keccak/sha3 gadgets #14

merged 3 commits into from
Jan 10, 2024

Conversation

huitseeker
Copy link
Member

@huitseeker huitseeker commented Jan 4, 2024

Adds a tweaked variant of the zatoichi keccak gadget, ported to bellpepper, with added sha3 for good measure.

@huitseeker huitseeker force-pushed the keccak branch 3 times, most recently from 1c0e137 to c20c66a Compare January 4, 2024 22:22
@huitseeker huitseeker changed the title feat: keccak gadget feat: keccak/sha3 gadgets Jan 7, 2024
Comment on lines +82 to +85
&a[x + 5usize],
&a[x + 10usize],
&a[x + 15usize],
&a[x + 20usize],
Copy link
Contributor

Choose a reason for hiding this comment

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

These usize type specifiers here and throughout this file seem unnecessary and just make reading/interpreting the algorithm harder.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they're voluntary: the goal is to make it clear the operand is on an index variable, not on something supposed to represent one of the u64 or Uint64 being modeled here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It has the opposite effect for me, since without the annotation this should be fine iff x is of the right type. The annotation makes it feel like some extra work is being done to coerce (although I know that's not the exact semantics either).

Copy link
Contributor

@porcuquine porcuquine 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 reasonable, and I didn't spot obvious optimization opportunities . For clarity: I didn't audit translation of the algorithm itself, just looked for low-hanging optimizations as requested.

@huitseeker huitseeker merged commit ad23b6e into lurk-lab:main Jan 10, 2024
9 checks passed
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.

None yet

2 participants