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 recursive from ACIR format; add them to API and CLI #9479

Merged
merged 66 commits into from
Nov 1, 2024

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Oct 28, 2024

Resolves noir-lang/noir#6185

  • Remove the recursive field from ACIR formats and the Circuit definition
  • Add --recursive to bb CLI
  • Add recursive to main.ts, the backend API and Wasm

This is effectively undoing a lot of what was done here. Interestingly there many more Wasm methods that need the recursive parameter now: whereas previously only acir_create_proof and acir_verify_proof used it, now anything that calls create_circuit needs it because circuit creation builds all the constraints already, which depend on this.

TODO:

  • Remove the #[recursive] attribute from Noir
  • Should the "prove and verify" methods that return nothing have a recursive parameter?
  • Remove #[recursive] from noir-protocol-circuits
  • Update bb-prover where it uses uses noir-protocol-circuits to use the correct recursive flag
  • Add --recursive to cli.ts under bb-prover
  • Check all calls to executeBB and pass --recursive if it calls a bb command that needs it

@aakoshh aakoshh changed the title chore!: Remove recursive from ACIR format; add them to API and CLI chore!: Remove recursive from ACIR format; add them to API and CLI (DRAFT) Oct 28, 2024
@aakoshh aakoshh marked this pull request as ready for review October 28, 2024 16:06
@aakoshh aakoshh force-pushed the 6185-rm-recursive-attr branch from 0174639 to e0f0fe7 Compare October 28, 2024 16:58
@aakoshh aakoshh changed the title chore!: Remove recursive from ACIR format; add them to API and CLI (DRAFT) chore!: Remove recursive from ACIR format; add them to API and CLI Oct 29, 2024
@TomAFrench
Copy link
Member

Should the "prove and verify" methods that return nothing have a recursive parameter?

I think yes as we could want to test recursive proof generation (these methods are mostly used for our CI)

@aakoshh aakoshh force-pushed the 6185-rm-recursive-attr branch from 5bda036 to b57695c Compare October 29, 2024 13:01
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

Just need to re-add recursive testing for assert_statement. We should be good to go after this.

@TomAFrench
Copy link
Member

Looks like there's a bash issue with adding the recursive flag to the CLI args.

@TomAFrench TomAFrench enabled auto-merge (squash) November 1, 2024 19:00
@aakoshh aakoshh force-pushed the 6185-rm-recursive-attr branch from c5e4c46 to 0e38181 Compare November 1, 2024 19:03
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.

Deprecate the #[recursive] attribute from functions
3 participants