Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

feat(stdlib): Add fallback implementation of SHA256 black box function #407

Merged
merged 45 commits into from
Jul 11, 2023

Conversation

Ethan-000
Copy link
Contributor

@Ethan-000 Ethan-000 commented Jul 2, 2023

Description

Problem*

Resolves

Summary*

Additional Context

PR Checklist*

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

@Ethan-000 Ethan-000 marked this pull request as draft July 2, 2023 18:15
@Ethan-000 Ethan-000 changed the title feat: sha256 fallback feat(stdlib): sha256 fallback Jul 2, 2023
@Ethan-000 Ethan-000 marked this pull request as ready for review July 4, 2023 19:45
Copy link
Contributor

@kevaundray kevaundray left a comment

Choose a reason for hiding this comment

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

Once we resolve the nits around styling etc, I think we can merge this in under a experimental feature flag, until we get someone to comb through the logic of sha256, so the PR does not get stale.

We would only need it to be on the method that converts sha256 blackbox functions into regular gates, so tests won't be affected since they will be switched on in acvm/stdlib unconditionally.

@kevaundray
Copy link
Contributor

@Ethan-000 Please ping me when you want a re-review!

@Ethan-000
Copy link
Contributor Author

Ethan-000 commented Jul 10, 2023

I think its ready for a re-review :). I did all the suggested changes other than "using mutable reference for num_witness to avoid returning it"

@Ethan-000
Copy link
Contributor Author

also added a sha256 prop test.

Copy link
Contributor

@kevaundray kevaundray left a comment

Choose a reason for hiding this comment

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

I think this is okay to merge since its isolated and not enabled -- great work!

@kevaundray kevaundray added this pull request to the merge queue Jul 11, 2023
Merged via the queue into noir-lang:master with commit 040369a Jul 11, 2023
6 checks passed
@kevaundray kevaundray mentioned this pull request Jul 11, 2023
@Ethan-000 Ethan-000 deleted the sha256_fallback branch July 12, 2023 15:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants