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

sha256 circuit #756

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

Brechtpd
Copy link
Collaborator

@Brechtpd Brechtpd commented Sep 9, 2022

Adds a sha256 circuit with the same lookup interface as the keccak circuit.

  • Requires 72 rows/sha256 permutation (64 bytes absorbed per permutation)
  • Uses ~130 columns
  • Would be a bit faster when making the circuit x4 wider (and then of course also using x4 fewer rows)
  • Current expression degree is 4, should be possible to lower this to 3 after the no-zk halo2 is usable
  • Is around 4x faster than keccak for same length inputs (keccak absorbs ~2x the amount of data per permutation so sha256 needs ~2x the number of permutations to process a similar amount of data)

@github-actions github-actions bot added crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bench Type: benchmark improvements labels Sep 9, 2022
@CPerezz CPerezz self-requested a review September 9, 2022 12:31
@str4d
Copy link

str4d commented Sep 14, 2022

Ooh, nice! I would love to see this adapted to implement the Sha256Instructions trait from halo2_gadgets, as then circuit developers would have a very easy time switching between our 10-column chip and this ~130-column one.

@CPerezz
Copy link
Member

CPerezz commented Nov 23, 2022

Hey @Brechtpd I'm adding the challenge API to all three keccak implementations we currently have in #925. I hope this does not collide with that (Otherwise, I'll code it with features or an option or whatever).

Please, ping me to review it! Since reviewed the last 3 I guess I'd be a bit faster!

Thanks for working on this!!!! 😄

@Brechtpd
Copy link
Collaborator Author

I don't think there will be any conflict with the keccak implementations, and will update the code confirming to #925 if it's decided this implementation will be used for the sha256 precompile (or if useful regardless of that to e.g. compare between different implementations). :)

@CPerezz
Copy link
Member

CPerezz commented Nov 24, 2022

this implementation will be used for the sha256 precompile (or if useful regardless of that to e.g. compare between different implementations). :)

That's nice!! Thanks a lot!! I'd also consider the 4 options and stick to only 1 or 2. Otherwise we need to mantain a lot more code. WDYT?

@Brechtpd
Copy link
Collaborator Author

What do you mean 4 options? I think for sha256 there's only like 2 I guess (this one or the halo2 one). Or are you talking about keccak?

@CPerezz
Copy link
Member

CPerezz commented Nov 24, 2022

What do you mean 4 options? I think for sha256 there's only like 2 I guess (this one or the halo2 one). Or are you talking about keccak?

Indeed mate.. My brain classified SHA256 and keccak as the same thing xD

Apologies, we already discussed the 3 different keccak implementations privately. 🙏

@ChihChengLiang ChihChengLiang mentioned this pull request Jul 5, 2023
30 tasks
lispc pushed a commit that referenced this pull request Aug 28, 2023
* fix: several fixes | wip debuging

* remove unnecessary part

* fix: assert equal for op output and success

* fix: G2 coeffs

* chore: remove info log
@KimiWu123 KimiWu123 added this to the Feature Completeness milestone Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bench Type: benchmark improvements
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

4 participants