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

Work on SHA256 and set of related improvements #528

Merged
merged 21 commits into from
Sep 1, 2023
Merged

Conversation

nadimkobeissi
Copy link
Contributor

@nadimkobeissi nadimkobeissi commented Aug 31, 2023

This commit is a set of improvements related to work on implementing the SHA256 executor.

Changes made:

  • SHA256 JSON generation (not yet fully tested)
  • Switch Keccak and SHA256 to use GateConfig interface
  • Migrate KeccakState to the unified GateState and use GateState for SHA256 as well

While this commit implements SHA256 JSON generation, it is not yet possible to finish the implementation of the SHA256 executor pending some necessary fixes to how executors are tested. Importantly, all Keccak executor tests are broken and have been broken since well before this PR, and this must be fixed before SHA256 executor tests can be standardized. All of this will be done in a separate branch, separate PR.

PR tested as well as I could given broken tests. End-to-end test passes, SHA256 gate-based implementation and Keccak gate-based implementation tests pass as well.

I'm submitting this commit now because it's getting too big, and I've already had to rebase it on develop once.

The Keccak function was overloaded with a set of options to generate
JSON files. This commit separates this logic into `keccak_gen`, which
hopefully can also be adopted as a pattern for upcoming work on
`sm/sha256` and `sm/blake`.
This commit makes it easier to run test vectors targeting hash
functions.
This replaces KeccakState with GateState, which we can then use for all
future primitives that we implement as arithmetic gates (SHA256, BLAKE2,
and others).
Very unsure that this works correctly, need external review.
Confirmed as not functional with @fractasy on August 29th, 2023.
@cla-bot cla-bot bot added the cla-signed label Aug 31, 2023
@nadimkobeissi nadimkobeissi marked this pull request as ready for review August 31, 2023 14:55
The Keccak function was overloaded with a set of options to generate
JSON files. This commit separates this logic into `keccak_gen`, which
hopefully can also be adopted as a pattern for upcoming work on
`sm/sha256` and `sm/blake`.
This commit makes it easier to run test vectors targeting hash
functions.
This replaces KeccakState with GateState, which we can then use for all
future primitives that we implement as arithmetic gates (SHA256, BLAKE2,
and others).
Very unsure that this works correctly, need external review.
Confirmed as not functional with @fractasy on August 29th, 2023.
@nadimkobeissi nadimkobeissi changed the title nadim-sha256 work in progress Work on SHA256 and set of related improvements Sep 1, 2023
@nadimkobeissi
Copy link
Contributor Author

OK, everyone is very busy, this commit doesn't break anything, and I have a lot of stuff I need to do still, so I'm merging this myself. Hopefully I won't get yelled at.

@nadimkobeissi nadimkobeissi merged commit 0023e0e into develop Sep 1, 2023
2 checks passed
@nadimkobeissi nadimkobeissi deleted the nadim-sha256 branch September 1, 2023 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant