feat(dsl)!: Arbitrary depth recursion #433

vezenovm · 2023-05-05T19:35:34Z

Description

This PR was pushed for testing the full CI before merging into the main mv/noir-recursion branch. This PR adds a nested_aggregation_object to a RecursionConstraint in acir_format. It also includes tests in acir_format and acir_proofs that show full composition of recursive proofs.

This PR does a couple other things to enable interop with Noir/DSLs. The verify_recursive_proof function which writes the output aggregation object to a buffer now accepts the number of public inputs of the inner proof while also handling the input aggregation object. This will enable DSLs to simulate the result of a recursion operation without running through the proving operation.

The init_verification_key method also now writes the recursive proof indices of a given key. This is used by method looking to simulate the result of recursive verification. Our circuit require's that the recursive proof indices are circuits constants, thus we must fetch the information from the backend in some manner. I chose to keep this as the verification key and then whomever is interacting with the acir_proofs package can extract the indices from the verification key witnesses.

Checklist:

I have reviewed my diff in github, line by line.
Every change is related to the PR description.
I have linked this pull request to the issue(s) that it resolves.
There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
[] There are no circuit changes, OR specifications in /markdown/specs have been updated.
[] There are no circuit changes, OR a cryptographer has been assigned for review.
I've updated any terraform that needs updating (e.g. environment variables) for deployment.
The branch has been rebased against the head of its merge target.
I'm happy for the PR to be merged at the reviewer's next convenience.
New functions, classes, etc. have been documented according to the doxygen comment format. Classes and structs must have @brief describing the intended functionality.
If existing code has been modified, such documentation has been added or updated.

…h arbitrary depth cahnges

…s serialization test

… bugs in acir_proofs test

…e proof indices when init vkey, fix full recursive comp test in acir_proofs

zac-williamson

Looks good to me. Flagged an issue we might want to create for a future optimisation.

zac-williamson · 2023-05-16T10:26:38Z

cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp

@@ -113,12 +122,14 @@ void create_recursion_constraints(Composer& composer, const RecursionConstraint&

    // Assign the output aggregation object to the proof public inputs (16 field elements representing two
    // G1 points)
-    result.add_proof_outputs_as_public_inputs();
+    // result.add_proof_outputs_as_public_inputs();


Suggested change

// result.add_proof_outputs_as_public_inputs();

I forgot about this commented out code. Do you think I should just remove the comment above as well? It may be confusing to leave it otherwise?

I removed the entire comment for now but can add it back in the main PR if a further explanation is deemed necessary

zac-williamson · 2023-05-16T10:37:23Z

cpp/src/barretenberg/dsl/acir_proofs/acir_proofs.cpp

-    std::shared_ptr<acir_format::verification_key_ct> vkey =
-        acir_format::verification_key_ct::template from_field_pt_vector<false>(&composer, key_fields);
+    // recursively verify the proof
+    std::array<uint32_t, acir_format::RecursionConstraint::AGGREGATION_OBJECT_SIZE> nested_aggregation_object = {};


This method could be quite slow as we are building a circuit that technically we do not need to (if we made some tweaks).

One alternative to this would be to enable the native verification algorithm to output an aggregation object. i.e. the native verifier performs a pairing check in the last step. Instead of this it would return the G1 points it inputs into the pairing; these would be the new aggregation object.

Possibly complex as it may require also making the native verifier use the recursion-friendly hash function (not keccak). Maybe create this as an issue that we can address later?

It does get a little complex with determining which hash function to use, but I don't think it is too bad and may be worth it to not generate a circuit. Is it very inefficient if we are not generating a proof? Either way I can open up an issue for this.

Issue made here: #447

…blic inputs in the recursion constraint

* Added recursion constraint into dsl + tests * fix dsl ecdsa constraint. Added ecdsa dsl tests * changed dsl recursion to pass proof/key as witnesses * recursive verification key no longer encapsulates a native key recursive verification key no longer encapsulates a reference string (neither were fundamentally needed!) * added serialization test for acir_proofs * Added serialization methods into dsl for recursive proof composition Added passing serialization tests Fixed format of RecursionConstraint to be compatible with existing ACIR formatting * added verificaiton key hash into RecursionConstraint Can be used by backend to force recursive verification key to be a specific value (it is represented via witnesses and therefore is not by-default constraint to represent a specific circuit) * fixed compiler errors in stdlib_recursion_tests * feat(dsl)!: Noir recursion updates (#379) * merge master in zw/noir-recursion * add dsl files to commit * namespace stuff and debugging * c bind functions * constraint test and comment removal * revert some changes to RecursionConstraint while debugging serialization issues * dispaly error when trying to use create_circuit_with_witness * is_recursive flag as part of new_proof and verify_proof, new RecursiveProver type in dsl, and serialization changes for recursion acir_proofs methods * remove debug output from TestSerializationWithRecursion * master merge conflicts * EnvReferenceString undefined reference to error workaround * add missing recursion_constraint field from acir_format tests * add recursion_constraints field back in acir_proofs.test inner circuit * fix inner circuit in RecursionConstraint.TestRecursionConstraint * Empty-Commit * add verify_recursive_proof method for simulating recursive verification in the ACVM * add ecc module to env package to fix linking of ennv_load_prover_crs and env_load_verifier_crs * add back reference strinng to stdlib recursion key to pass sol verifier generation tests * remove prints from recursive verifier test * fix dirty free for when serializing vk to fields, was working on macbook so not caught earlier, but need more clarity on ubuntu * fix ecdsa tests after master merge * missing keccak constraints fields in acir format and proofs tests * one more missing keccak constraint * mismatched acir format structs for gcc build * missing block constraints in acir tests * feat(dsl)!: Arbitrary depth recursion (#433) * merge conflicts and small updates to get acir_proofs test passing with arbitrary depth cahnges * inline with mv/noir-recursion and working double recursion acir_proofs serialization test * cleanup and working towards supporting full nested proofs, still some bugs in acir_proofs test * full recursive composition test working in acir_proofs * use two public inptus for inner circuit * delete commented out unnecessary acir proofs method * made dummy transcript points unique to prevent point collisions * handle nested proofs when exporting dummy transcript, export recursive proof indices when init vkey, fix full recursive comp test in acir_proofs * update bindings on verify_recursive_proof to accept num public inputs * missing acir_format field in tests * missing one more correct acir_format struct in recursion constraint test * cleanup and additional comment for recursion_constraint * fix up comment in recursion_constraint * remove unnecesary comments when we were including proof outputs as public inputs in the recursion constraint --------- Co-authored-by: zac-williamson <blorktronics@gmail.com> * move export key into dsl package * chore: remove unused export key in recursion format from main proof system classes * pr review: moved from_witness comment and renamed from_field_pt_vector * moved export transcript in recursion format to DSL package * move order of acir functions * remove ecc bb_module declaration in env package * chore: remove usage of magic numbers when slicing g1::affine_element into barretenberg::fr * introduce NUM_AGGREGATION_ELEMENTS constant and use it through recursion constraint * nit ASSERT(result != -1) in get_public_input_index * remove unused found var * cast -1 * fix up verify_recursive_proof and add a test for it * moved from tempalte for has_valid_witness_assignments to a flag * chore: add comments to AcirProofs.TestVerifyRecursiveProofPass * update prove and verify exports to handle is_recursive * remove install state * remove debug output and use total_circuit_size * remove verify_recursive_proof as we might not need it for simulation * switch how we do is_recursive * remove acir recursive simulation method * changed wasi stubs and removed cout from acir_init_proving_key * removed cout outs getting runtime error for func definition * working recursion format funcs, but strangely getting errors about trying to invert zero in the field for dummy transcript and key * most recent debug output, hitting bad alloc in compute_proving_key_base * delete old comment * bb.js cmd changes * Lint and default fixes. * cleanup info and debug comments * add comment to recursion serialization methods * match package.json on cl/acir_cbinds * don't remove EventEmitter from bb_wasm.ts * delete empty exports file * use process.exit * log * add comment for why CIRCUIT_SIZE is 2**18 * mising keccak var constraints in new acir format tests * Cleanup some serialization. * Cleanup for publishing. README. --------- Co-authored-by: zac-williamson <blorktronics@gmail.com> Co-authored-by: Charlie Lye <karl.lye@gmail.com>

…t. (#471) * Initial * Separate thread build dir. * Large vector slab allocator. * format msgpack serialization and excldue msgpack-c from clang-format (#467) * patch: temporarily remove broken solidity ci (#470) * progress. * Sumcheck improvements (#455) * convert partially evaluated polynomials from vectors to Polynomials and rename * rename fold method to partially_evaluate * static constexpr barycentric arrays * change purported evaluations to claimed evaluations * specify relations in Flavor * working. but cant compute pk independently :/ * Fixed. * Better key construction separation. * Clean * tests pass * Commander * Fix. * Fix * Fix serve. * Fix * More commands. * Fixed a bug in biggroup tests (#478) * Add back custom libc++ build. * Fix. * write_vk * DSL: Add KeccakVar opcode (#476) * add initial KeccakVar code * add result field * add keccak_var_constraints to fields * Multi-constraint Relations (#444) Allow for correct and efficient batching over identities in the Sumcheck relation * Clean up srs package. Add a global crs that the wasm can init once. * Add back vk serialization for a3 * Clean * Exclude env from lib. * feat(dsl)!: DSL recursion changes w/ bb.js (#485) * Added recursion constraint into dsl + tests * fix dsl ecdsa constraint. Added ecdsa dsl tests * changed dsl recursion to pass proof/key as witnesses * recursive verification key no longer encapsulates a native key recursive verification key no longer encapsulates a reference string (neither were fundamentally needed!) * added serialization test for acir_proofs * Added serialization methods into dsl for recursive proof composition Added passing serialization tests Fixed format of RecursionConstraint to be compatible with existing ACIR formatting * added verificaiton key hash into RecursionConstraint Can be used by backend to force recursive verification key to be a specific value (it is represented via witnesses and therefore is not by-default constraint to represent a specific circuit) * fixed compiler errors in stdlib_recursion_tests * feat(dsl)!: Noir recursion updates (#379) * merge master in zw/noir-recursion * add dsl files to commit * namespace stuff and debugging * c bind functions * constraint test and comment removal * revert some changes to RecursionConstraint while debugging serialization issues * dispaly error when trying to use create_circuit_with_witness * is_recursive flag as part of new_proof and verify_proof, new RecursiveProver type in dsl, and serialization changes for recursion acir_proofs methods * remove debug output from TestSerializationWithRecursion * master merge conflicts * EnvReferenceString undefined reference to error workaround * add missing recursion_constraint field from acir_format tests * add recursion_constraints field back in acir_proofs.test inner circuit * fix inner circuit in RecursionConstraint.TestRecursionConstraint * Empty-Commit * add verify_recursive_proof method for simulating recursive verification in the ACVM * add ecc module to env package to fix linking of ennv_load_prover_crs and env_load_verifier_crs * add back reference strinng to stdlib recursion key to pass sol verifier generation tests * remove prints from recursive verifier test * fix dirty free for when serializing vk to fields, was working on macbook so not caught earlier, but need more clarity on ubuntu * fix ecdsa tests after master merge * missing keccak constraints fields in acir format and proofs tests * one more missing keccak constraint * mismatched acir format structs for gcc build * missing block constraints in acir tests * feat(dsl)!: Arbitrary depth recursion (#433) * merge conflicts and small updates to get acir_proofs test passing with arbitrary depth cahnges * inline with mv/noir-recursion and working double recursion acir_proofs serialization test * cleanup and working towards supporting full nested proofs, still some bugs in acir_proofs test * full recursive composition test working in acir_proofs * use two public inptus for inner circuit * delete commented out unnecessary acir proofs method * made dummy transcript points unique to prevent point collisions * handle nested proofs when exporting dummy transcript, export recursive proof indices when init vkey, fix full recursive comp test in acir_proofs * update bindings on verify_recursive_proof to accept num public inputs * missing acir_format field in tests * missing one more correct acir_format struct in recursion constraint test * cleanup and additional comment for recursion_constraint * fix up comment in recursion_constraint * remove unnecesary comments when we were including proof outputs as public inputs in the recursion constraint --------- Co-authored-by: zac-williamson <blorktronics@gmail.com> * move export key into dsl package * chore: remove unused export key in recursion format from main proof system classes * pr review: moved from_witness comment and renamed from_field_pt_vector * moved export transcript in recursion format to DSL package * move order of acir functions * remove ecc bb_module declaration in env package * chore: remove usage of magic numbers when slicing g1::affine_element into barretenberg::fr * introduce NUM_AGGREGATION_ELEMENTS constant and use it through recursion constraint * nit ASSERT(result != -1) in get_public_input_index * remove unused found var * cast -1 * fix up verify_recursive_proof and add a test for it * moved from tempalte for has_valid_witness_assignments to a flag * chore: add comments to AcirProofs.TestVerifyRecursiveProofPass * update prove and verify exports to handle is_recursive * remove install state * remove debug output and use total_circuit_size * remove verify_recursive_proof as we might not need it for simulation * switch how we do is_recursive * remove acir recursive simulation method * changed wasi stubs and removed cout from acir_init_proving_key * removed cout outs getting runtime error for func definition * working recursion format funcs, but strangely getting errors about trying to invert zero in the field for dummy transcript and key * most recent debug output, hitting bad alloc in compute_proving_key_base * delete old comment * bb.js cmd changes * Lint and default fixes. * cleanup info and debug comments * add comment to recursion serialization methods * match package.json on cl/acir_cbinds * don't remove EventEmitter from bb_wasm.ts * delete empty exports file * use process.exit * log * add comment for why CIRCUIT_SIZE is 2**18 * mising keccak var constraints in new acir format tests * Cleanup some serialization. * Cleanup for publishing. README. --------- Co-authored-by: zac-williamson <blorktronics@gmail.com> Co-authored-by: Charlie Lye <karl.lye@gmail.com> --------- Co-authored-by: Maxim Vezenov <mvezenov@gmail.com> Co-authored-by: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com> Co-authored-by: kevaundray <kevtheappdev@gmail.com> Co-authored-by: zac-williamson <blorktronics@gmail.com>

vezenovm and others added 9 commits May 3, 2023 19:32

merge conflicts and small updates to get acir_proofs test passing wit…

8599781

…h arbitrary depth cahnges

inline with mv/noir-recursion and working double recursion acir_proof…

c1a4367

…s serialization test

cleanup and working towards supporting full nested proofs, still some…

b434c37

… bugs in acir_proofs test

full recursive composition test working in acir_proofs

cd0c726

use two public inptus for inner circuit

fe8cec5

Merge branch 'mv/noir-recursion' into mv/noir-arbitrary-recursion

80fcac5

Merge branch 'mv/noir-recursion' into mv/noir-arbitrary-recursion

371cb4f

delete commented out unnecessary acir proofs method

ad59820

made dummy transcript points unique to prevent point collisions

e65950d

Savio-Sou mentioned this pull request May 11, 2023

Epic: UP Recursive Proofs noir-lang/noir#1098

Closed

vezenovm added 4 commits May 12, 2023 19:01

handle nested proofs when exporting dummy transcript, export recursiv…

a523906

…e proof indices when init vkey, fix full recursive comp test in acir_proofs

update bindings on verify_recursive_proof to accept num public inputs

dcfcbd9

missing acir_format field in tests

231c0be

missing one more correct acir_format struct in recursion constraint test

bc6e5c7

vezenovm marked this pull request as ready for review May 15, 2023 18:54

vezenovm added 3 commits May 15, 2023 18:56

merge conflicts with mv/noir-recursion

48c8659

cleanup and additional comment for recursion_constraint

82347ce

fix up comment in recursion_constraint

9753e44

vezenovm requested review from zac-williamson and kevaundray May 15, 2023 19:26

vezenovm mentioned this pull request May 15, 2023

feat!: Recursion bindings noir-lang/barretenberg-sys#26

Open

zac-williamson approved these changes May 16, 2023

View reviewed changes

remove unnecesary comments when we were including proof outputs as pu…

bdbafc5

…blic inputs in the recursion constraint

vezenovm mentioned this pull request May 16, 2023

Enable native verification algorithm to return an output aggregation object #447

Open

vezenovm merged commit 7ecb574 into mv/noir-recursion May 16, 2023

vezenovm deleted the mv/noir-arbitrary-recursion branch May 16, 2023 17:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dsl)!: Arbitrary depth recursion #433

feat(dsl)!: Arbitrary depth recursion #433

vezenovm commented May 5, 2023 •

edited

Loading

zac-williamson left a comment

zac-williamson May 16, 2023

vezenovm May 16, 2023 •

edited

Loading

vezenovm May 16, 2023

zac-williamson May 16, 2023

vezenovm May 16, 2023 •

edited

Loading

vezenovm May 16, 2023

feat(dsl)!: Arbitrary depth recursion #433

feat(dsl)!: Arbitrary depth recursion #433

Conversation

vezenovm commented May 5, 2023 • edited Loading

Description

Checklist:

zac-williamson left a comment

Choose a reason for hiding this comment

zac-williamson May 16, 2023

Choose a reason for hiding this comment

vezenovm May 16, 2023 • edited Loading

Choose a reason for hiding this comment

vezenovm May 16, 2023

Choose a reason for hiding this comment

zac-williamson May 16, 2023

Choose a reason for hiding this comment

vezenovm May 16, 2023 • edited Loading

Choose a reason for hiding this comment

vezenovm May 16, 2023

Choose a reason for hiding this comment

vezenovm commented May 5, 2023 •

edited

Loading

vezenovm May 16, 2023 •

edited

Loading

vezenovm May 16, 2023 •

edited

Loading