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

Keccak256 in Miden Assembly #154

Merged
merged 12 commits into from
Mar 25, 2022

Conversation

itzmeanjan
Copy link
Contributor

@itzmeanjan itzmeanjan commented Mar 18, 2022

Progress

@itzmeanjan itzmeanjan changed the base branch from main to next March 18, 2022 06:41
@itzmeanjan
Copy link
Contributor Author

itzmeanjan commented Mar 20, 2022

VM cycle count when applying keccak-p[1600, 24]: 134,355

@itzmeanjan
Copy link
Contributor Author

VM cycle count for computing keccak256 2-to-1 hash: 149,370

@itzmeanjan itzmeanjan marked this pull request as ready for review March 21, 2022 15:02
@itzmeanjan
Copy link
Contributor Author

For understanding bit interleaving technique, which is leveraged in this implementation so that using only 32 -bit bitwise operations keccak256 hash can be computed, I suggest you read section 2.1 of https://keccak.team/files/Keccak-implementation-3.2.pdf

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Thank you! Looks great! One small thing:

I'm assuming testing this would take quite a bit of time. Could we add the ignore attribute for the test so that it is run only when we want to run it explicitly? We can remove this attribute later once we improve script compilation times.

Also, some optimization thoughts for the future:

  1. Once we have u64 module fully implemented in stdlib we might simplify the code a bit.
  2. I noticed that pattern of popw followed by pushw is used quite frequently. We could reduce the number of cycles by using storew followed by loadw instead.
  3. In some cases, it might make sense to use load and store (for single elements) rather the variants for full words followed by stack manipulations.

@itzmeanjan
Copy link
Contributor Author

Thanks for reviewing !

I'm assuming testing this would take quite a bit of time. Could we add the ignore attribute for the test so that it is run only when we want to run it explicitly? We can remove this attribute later once we improve script compilation times.

Fixed here 4bf82a9

Also, some optimization thoughts for the future:

Noted down, I'll apply them when I revisit the problem.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

All looks good! thank you!

@bobbinth bobbinth merged commit b823554 into 0xPolygonMiden:next Mar 25, 2022
@itzmeanjan itzmeanjan deleted the itzmeanjan/keccak256 branch June 28, 2022 12:16
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