Skip to content

feat: e1,e3 implementation for sha2 #1657

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

Open
wants to merge 26 commits into
base: feat/new-execution
Choose a base branch
from

Conversation

arayikhalatyan
Copy link
Contributor

@arayikhalatyan arayikhalatyan commented May 15, 2025

Implemented e1, e3 for sha2 circuit. I feel like the trace generation of sha2 is more readable now. Probably there is a room for making the inner sha256 trace generation more efficient, I'll try to make some improvements before merging.

Question: are we planning to have the restrictions on block_size (eg alignment and being power of two) in e1 and does it have to be constant? If not I could make the e1 execution to read the entire message in one read.

Resolves INT-3966

Copy link

codspeed-hq bot commented May 15, 2025

CodSpeed Instrumentation Performance Report

Merging #1657 will not alter performance

Comparing feat/sha2-rewrite (9f6f129) with feat/new-execution (00bab47)

Summary

✅ 4 untouched benchmarks

Copy link

codspeed-hq bot commented May 15, 2025

CodSpeed Walltime Performance Report

Merging #1657 will not alter performance

Comparing feat/sha2-rewrite (9f6f129) with feat/new-execution (00bab47)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

✅ 4 untouched benchmarks

@jonathanpwang
Copy link
Contributor

@arayikhalatyan we shouldn't have any block size restrictions on E1 (but likely need it for E2 in some light form). My imagination is indeed that E1 should just look like input = read_bytes(); return sha256(input)

@arayikhalatyan
Copy link
Contributor Author

@arayikhalatyan we shouldn't have any block size restrictions on E1 (but likely need it for E2 in some light form). My imagination is indeed that E1 should just look like input = read_bytes(); return sha256(input)

Yeah I agree. Then we have to make some changes to the GuestMemory interface because it expects the block size as a const generic.

@jonathanpwang jonathanpwang reopened this May 15, 2025
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.

2 participants