-
Notifications
You must be signed in to change notification settings - Fork 275
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
feat: Use full IPA recursive verifier in root rollup #10962
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,17 @@ | |
#include "barretenberg/common/throw_or_abort.hpp" | ||
#include "barretenberg/dsl/acir_format/ivc_recursion_constraint.hpp" | ||
#include "barretenberg/flavor/flavor.hpp" | ||
#include "barretenberg/stdlib/eccvm_verifier/verifier_commitment_key.hpp" | ||
#include "barretenberg/stdlib/plonk_recursion/aggregation_state/aggregation_state.hpp" | ||
#include "barretenberg/stdlib/primitives/curves/grumpkin.hpp" | ||
#include "barretenberg/stdlib/primitives/field/field_conversion.hpp" | ||
#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp" | ||
#include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" | ||
#include "barretenberg/transcript/transcript.hpp" | ||
#include "proof_surgeon.hpp" | ||
#include <cstddef> | ||
#include <cstdint> | ||
#include <memory> | ||
|
||
namespace acir_format { | ||
|
||
|
@@ -270,9 +273,8 @@ void build_constraints(Builder& builder, AcirProgram& program, const ProgramMeta | |
// final recursion output. | ||
builder.add_pairing_point_accumulator(current_aggregation_object); | ||
} | ||
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1183): This assertion should be true, except for | ||
// the root rollup as of now since the root rollup will not output a ipa proof. | ||
// ASSERT((metadata.honk_recursion == 2) == (output.ipa_proof.size() > 0)); | ||
// If we are proving with UltraRollupFlavor, the IPA proof should have nonzero size. | ||
ASSERT((metadata.honk_recursion == 2) == (output.ipa_proof.size() > 0)); | ||
if (metadata.honk_recursion == 2) { | ||
builder.add_ipa_claim(output.ipa_claim.get_witness_indices()); | ||
builder.ipa_proof = output.ipa_proof; | ||
|
@@ -379,13 +381,17 @@ HonkRecursionConstraintsOutput<Builder> process_honk_recursion_constraints( | |
size_t idx = 0; | ||
std::vector<OpeningClaim<stdlib::grumpkin<Builder>>> nested_ipa_claims; | ||
std::vector<StdlibProof<Builder>> nested_ipa_proofs; | ||
bool is_root_rollup = false; | ||
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 bool checks to see if any of the honk recursion constraints set the proof type to be ROOT ROLLUP HONK and that's how we know whether to run the full IPA recursive verifier or not. |
||
for (auto& constraint : constraint_system.honk_recursion_constraints) { | ||
if (constraint.proof_type == HONK) { | ||
auto [next_aggregation_object, _ipa_claim, _ipa_proof] = | ||
create_honk_recursion_constraints<UltraRecursiveFlavor_<Builder>>( | ||
builder, constraint, current_aggregation_object, has_valid_witness_assignments); | ||
current_aggregation_object = next_aggregation_object; | ||
} else if (constraint.proof_type == ROLLUP_HONK || constraint.proof_type == ROLLUP_ROOT_HONK) { | ||
} else if (constraint.proof_type == ROLLUP_HONK || constraint.proof_type == ROOT_ROLLUP_HONK) { | ||
if (constraint.proof_type == ROOT_ROLLUP_HONK) { | ||
is_root_rollup = true; | ||
} | ||
auto [next_aggregation_object, ipa_claim, ipa_proof] = | ||
create_honk_recursion_constraints<UltraRollupRecursiveFlavor_<Builder>>( | ||
builder, constraint, current_aggregation_object, has_valid_witness_assignments); | ||
|
@@ -400,6 +406,7 @@ HonkRecursionConstraintsOutput<Builder> process_honk_recursion_constraints( | |
gate_counter.track_diff(constraint_system.gates_per_opcode, | ||
constraint_system.original_opcode_indices.honk_recursion_constraints.at(idx++)); | ||
} | ||
ASSERT(!(is_root_rollup && nested_ipa_claims.size() != 2) && "Root rollup must accumulate two IPA proofs."); | ||
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. just checking to make sure that if its the root rollup, we have 2 nested IPA claims to accumulate |
||
// Accumulate the claims | ||
if (nested_ipa_claims.size() == 2) { | ||
auto commitment_key = std::make_shared<CommitmentKey<curve::Grumpkin>>(1 << CONST_ECCVM_LOG_N); | ||
|
@@ -409,8 +416,21 @@ HonkRecursionConstraintsOutput<Builder> process_honk_recursion_constraints( | |
auto ipa_transcript_2 = std::make_shared<StdlibTranscript>(nested_ipa_proofs[1]); | ||
auto [ipa_claim, ipa_proof] = IPA<stdlib::grumpkin<Builder>>::accumulate( | ||
commitment_key, ipa_transcript_1, nested_ipa_claims[0], ipa_transcript_2, nested_ipa_claims[1]); | ||
output.ipa_claim = ipa_claim; | ||
output.ipa_proof = ipa_proof; | ||
// If this is the root rollup, do full IPA verification | ||
if (is_root_rollup) { | ||
auto verifier_commitment_key = std::make_shared<VerifierCommitmentKey<stdlib::grumpkin<Builder>>>( | ||
&builder, | ||
1 << CONST_ECCVM_LOG_N, | ||
std::make_shared<VerifierCommitmentKey<curve::Grumpkin>>(1 << CONST_ECCVM_LOG_N)); | ||
// do full IPA verification | ||
auto accumulated_ipa_transcript = | ||
std::make_shared<StdlibTranscript>(convert_native_proof_to_stdlib(&builder, ipa_proof)); | ||
IPA<stdlib::grumpkin<Builder>>::full_verify_recursive( | ||
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. if root rollup, run full IPA recursive verifier, otherwise return the ipa claim and proof as output |
||
verifier_commitment_key, ipa_claim, accumulated_ipa_transcript); | ||
} else { | ||
output.ipa_claim = ipa_claim; | ||
output.ipa_proof = ipa_proof; | ||
} | ||
} else if (nested_ipa_claims.size() == 1) { | ||
output.ipa_claim = nested_ipa_claims[0]; | ||
// This conversion looks suspicious but there's no need to make this an output of the circuit since its a proof | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -635,7 +635,7 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg, | |
// be the only means for setting the proof type. use of honk_recursion flag in this context can go | ||
// away once all noir programs (e.g. protocol circuits) are updated to use the new pattern. | ||
if (proof_type_in != HONK && proof_type_in != AVM && proof_type_in != ROLLUP_HONK && | ||
proof_type_in != ROLLUP_ROOT_HONK) { | ||
proof_type_in != ROOT_ROLLUP_HONK) { | ||
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. just a renaming |
||
if (honk_recursion == 1) { | ||
proof_type_in = HONK; | ||
} else if (honk_recursion == 2) { | ||
|
@@ -659,7 +659,7 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg, | |
break; | ||
case HONK: | ||
case ROLLUP_HONK: | ||
case ROLLUP_ROOT_HONK: | ||
case ROOT_ROLLUP_HONK: | ||
af.honk_recursion_constraints.push_back(c); | ||
af.original_opcode_indices.honk_recursion_constraints.push_back(opcode_index); | ||
break; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,9 @@ | ||
use crate::abis::block_root_or_block_merge_public_inputs::BlockRootOrBlockMergePublicInputs; | ||
use dep::types::{ | ||
constants::{PROOF_TYPE_ROLLUP_HONK, VK_TREE_HEIGHT}, | ||
constants::VK_TREE_HEIGHT, | ||
merkle_tree::{membership::assert_check_membership, MembershipWitness}, | ||
proof::{ | ||
rollup_recursive_proof::NestedRecursiveProof, | ||
traits::Verifiable, | ||
verification_key::{RollupHonkVerificationKey, VerificationKey}, | ||
}, | ||
traits::Empty, | ||
|
@@ -18,8 +17,8 @@ pub struct PreviousRollupBlockData { | |
pub vk_witness: MembershipWitness<VK_TREE_HEIGHT>, | ||
} | ||
|
||
impl Verifiable for PreviousRollupBlockData { | ||
fn verify(self) { | ||
impl PreviousRollupBlockData { | ||
fn verify(self, proof_type_id: u32) { | ||
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. adding an extra argument because this function is shared between the root rollup and the block merge |
||
let inputs = BlockRootOrBlockMergePublicInputs::serialize( | ||
self.block_root_or_block_merge_public_inputs, | ||
); | ||
|
@@ -28,7 +27,7 @@ impl Verifiable for PreviousRollupBlockData { | |
self.proof.fields, | ||
inputs, | ||
self.vk.hash, | ||
PROOF_TYPE_ROLLUP_HONK, | ||
proof_type_id, | ||
); | ||
} | ||
} | ||
|
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.
honk_recursion is 2 for the tube, base, merge, block root, block merge circuits, but NOT the root rollup.
This assert previously wasn't true because the root rollup wasn't running the full verifier and therefore still spit out an ipa claim and proof, but now that it does run full verification, it will output nothing.