-
Notifications
You must be signed in to change notification settings - Fork 598
feat: Refactor IPA claim handling in acir format to support them for AVM #13547
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
Changes from all commits
f0fb6f6
64bbfd4
0fa8c26
be476c9
e947f7f
445b712
dc4be31
da9c767
67fe0ed
c9a8068
c53248b
64dd336
1462446
1af7985
4777960
a9a412f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,29 +270,4 @@ template <typename Builder> class GateCounter { | |
| size_t prev_gate_count{}; | ||
| }; | ||
|
|
||
| void process_plonk_recursion_constraints(Builder& builder, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these shouldn't be externally exposed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. they also shouldn't be free floating methods but one thing at a time.. |
||
| AcirFormat& constraint_system, | ||
| bool has_valid_witness_assignments, | ||
| GateCounter<Builder>& gate_counter); | ||
| void process_honk_recursion_constraints(Builder& builder, | ||
| AcirFormat& constraint_system, | ||
| bool has_valid_witness_assignments, | ||
| GateCounter<Builder>& gate_counter, | ||
| uint32_t honk_recursion); | ||
|
|
||
| void process_ivc_recursion_constraints(MegaCircuitBuilder& builder, | ||
| AcirFormat& constraints, | ||
| ClientIVC* ivc, | ||
| bool has_valid_witness_assignments, | ||
| GateCounter<MegaCircuitBuilder>& gate_counter); | ||
|
|
||
| #ifndef DISABLE_AZTEC_VM | ||
| stdlib::recursion::aggregation_state<Builder> process_avm_recursion_constraints( | ||
| Builder& builder, | ||
| AcirFormat& constraint_system, | ||
| bool has_valid_witness_assignments, | ||
| GateCounter<Builder>& gate_counter, | ||
| stdlib::recursion::aggregation_state<Builder> current_aggregation_object); | ||
| #endif | ||
|
|
||
| } // namespace acir_format | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,11 +50,7 @@ class AvmGoblinRecursiveVerifier { | |
|
|
||
| // The structure of the final output of the goblinized AVM2 recursive verifier. The IPA data comes from recursive | ||
| // verification of the ECCVM proof as part of Goblin recursive verification. | ||
| struct RecursiveAvmGoblinOutput { | ||
| std::vector<UltraFF> ipa_proof; | ||
| OpeningClaim<stdlib::grumpkin<UltraBuilder>> ipa_claim; | ||
| stdlib::recursion::aggregation_state<UltraBuilder> aggregation_object; | ||
| }; | ||
| using RecursiveAvmGoblinOutput = stdlib::recursion::honk::UltraRecursiveVerifierOutput<UltraBuilder>; | ||
|
|
||
| // Output of prover for inner Mega-arithmetized AVM recursive verifier circuit; input to the outer verifier | ||
| struct InnerProverOutput { | ||
|
|
@@ -143,26 +139,17 @@ class AvmGoblinRecursiveVerifier { | |
| GoblinRecursiveVerifier goblin_verifier{ &ultra_builder, inner_output.goblin_vk }; | ||
| GoblinRecursiveVerifierOutput goblin_verifier_output = goblin_verifier.verify(inner_output.goblin_proof); | ||
|
|
||
| // Propagate the IPA claim via the public inputs of the outer circuit | ||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/1306): Determine the right location/entity to | ||
| // handle this IPA data propagation. | ||
|
|
||
| // TODO(jeamon): Restore ipa_claim and ipa_proof integration in the builder. While running ./bootstrap.sh, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code for adding the ipa claim/proof to builder was copied from the tube circuit, where it is assumed that thats the ONLY IPA claim/proof. The AVM RV could be just a subset of a circuit. In particular, the public base circuit has both an AVM RV and UltraRollupHonk RV, which each produce IPA claims/proofs. If we uncomment this code, we would try to add 2 IPA claims/proofs to the builder, which causes the error. The correct thing is to just return the IPA stuff and accumulate them at the acir_format level. |
||
| // for the mock-protocol-circuits, we hit an assertion at circuit_builder_base_impl.hpp:262 ("added IPA claim | ||
| // when one already exists") | ||
| // ultra_builder.add_ipa_claim(goblin_verifier_output.opening_claim.get_witness_indices()); | ||
| // ultra_builder.ipa_proof = convert_stdlib_proof_to_native(goblin_verifier_output.ipa_transcript->proof_data); | ||
| // ASSERT(ultra_builder.ipa_proof.size() && "IPA proof should not be empty"); | ||
|
|
||
| // Validate the consistency of the AVM2 verifier inputs {\pi, pub_inputs, VK}_{AVM2} between the inner (Mega) | ||
| // circuit and the outer (Ultra) by asserting equality on the independently computed hashes of this data. | ||
| const FF ultra_hash = stdlib::poseidon2<UltraBuilder>::hash(ultra_builder, hash_buffer); | ||
| mega_proof[inner_output.mega_hash_public_input_index].assert_equal(ultra_hash); | ||
|
|
||
| // Return ipa proof, ipa claim and output aggregation object produced from verifying the Mega + Goblin proofs | ||
| return RecursiveAvmGoblinOutput{ .ipa_proof = goblin_verifier_output.ipa_transcript->proof_data, | ||
| .ipa_claim = goblin_verifier_output.opening_claim, | ||
| .aggregation_object = mega_verifier_output.agg_obj }; | ||
| return RecursiveAvmGoblinOutput{ | ||
| .agg_obj = mega_verifier_output.agg_obj, | ||
| .ipa_claim = goblin_verifier_output.opening_claim, | ||
| .ipa_proof = goblin_verifier_output.ipa_transcript->proof_data, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed the template parameter to builder instead as it doesn't depend on flavor