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

chore: remove usage of sha256 #49

Merged
merged 1 commit into from
Mar 3, 2025
Merged

chore: remove usage of sha256 #49

merged 1 commit into from
Mar 3, 2025

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

Removing in readiness for noir-lang/noir#7477

We're just sanity checking a constant hash here which isn't worth pulling in a dependency for.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench TomAFrench requested a review from kashbrti March 3, 2025 12:54
Copy link
Contributor

github-actions bot commented Mar 3, 2025

Changes to circuit sizes

Generated at commit: e1521e307a5f58a95288fda5156c68b60d34ca4a, compared to commit: 944c159ac8e0ed14a5d381421d0dd2248bc00940

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
test_verify_sha256_pkcs1v15_2048.json -67 ✅ -0.78% -3,970 ✅ -16.14%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
test_verify_sha256_pkcs1v15_2048.json 8,535 (-67) -0.78% 20,624 (-3,970) -16.14%

@TomAFrench
Copy link
Member Author

TomAFrench commented Mar 3, 2025

hmm, interesting that this reduces the number of opcodes as I would expect that sha256 to be optimized out entirely.

Issue captured here: noir-lang/noir#7564

@TomAFrench TomAFrench merged commit 75b8c4f into main Mar 3, 2025
7 checks passed
@TomAFrench TomAFrench deleted the tf/remove-sha256-usage branch March 3, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

1 participant