-
Notifications
You must be signed in to change notification settings - Fork 271
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: modify HonkRecursionConstraint to handle IPA claims #10469
Changes from 7 commits
624642c
cd95fb4
e163856
19e6ebd
12117e5
74a67be
fae2fe6
fa045f2
616fe1d
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 |
---|---|---|
|
@@ -634,7 +634,8 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg, | |
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1074): Eventually arg.proof_type will | ||
// 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 (honk_recursion && proof_type_in != HONK && proof_type_in != AVM) { | ||
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 is foul 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. But it's not your fault and you're cleaning it up in the future 🎉 |
||
if (honk_recursion && proof_type_in != HONK && proof_type_in != AVM && proof_type_in != ROLLUP_HONK && | ||
proof_type_in != ROLLUP_ROOT_HONK) { | ||
proof_type_in = HONK; | ||
} | ||
|
||
|
@@ -653,6 +654,8 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg, | |
af.original_opcode_indices.recursion_constraints.push_back(opcode_index); | ||
break; | ||
case HONK: | ||
case ROLLUP_HONK: | ||
case ROLLUP_ROOT_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 |
---|---|---|
|
@@ -31,17 +31,17 @@ namespace { | |
* @param key_fields | ||
* @param proof_fields | ||
*/ | ||
template <typename Flavor> | ||
void create_dummy_vkey_and_proof(Builder& builder, | ||
size_t proof_size, | ||
size_t public_inputs_size, | ||
const std::vector<field_ct>& key_fields, | ||
const std::vector<field_ct>& proof_fields) | ||
const std::vector<field_ct>& proof_fields, | ||
bool is_rollup_honk_recursion_constraint) | ||
{ | ||
using Flavor = UltraFlavor; | ||
|
||
// Set vkey->circuit_size correctly based on the proof size | ||
size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs<Flavor::Commitment>(); | ||
size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs<Flavor::FF>(); | ||
size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs<typename Flavor::Commitment>(); | ||
size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs<typename Flavor::FF>(); | ||
assert((proof_size - bb::HONK_PROOF_PUBLIC_INPUT_OFFSET - Flavor::NUM_WITNESS_ENTITIES * num_frs_comm - | ||
Flavor::NUM_ALL_ENTITIES * num_frs_fr - num_frs_comm) % | ||
(num_frs_comm + num_frs_fr * (Flavor::BATCHED_RELATION_PARTIAL_LENGTH + 1)) == | ||
|
@@ -61,13 +61,26 @@ void create_dummy_vkey_and_proof(Builder& builder, | |
builder.assert_equal(builder.add_variable(1), key_fields[3].witness_index); | ||
uint32_t offset = 4; | ||
size_t num_inner_public_inputs = public_inputs_size - bb::PAIRING_POINT_ACCUMULATOR_SIZE; | ||
if (is_rollup_honk_recursion_constraint) { | ||
num_inner_public_inputs -= bb::IPA_CLAIM_SIZE; | ||
} | ||
|
||
// We are making the assumption that the aggregation object are behind all the inner public inputs | ||
// We are making the assumption that the pairing point object is behind all the inner public inputs | ||
for (size_t i = 0; i < bb::PAIRING_POINT_ACCUMULATOR_SIZE; i++) { | ||
builder.assert_equal(builder.add_variable(num_inner_public_inputs + i), key_fields[offset].witness_index); | ||
offset++; | ||
} | ||
|
||
if (is_rollup_honk_recursion_constraint) { | ||
// Key field is the whether the proof contains an aggregation object. | ||
builder.assert_equal(builder.add_variable(1), key_fields[offset++].witness_index); | ||
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. Looks like an unconstrained witness 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. Also maybe the assert_equals trap is happening here? 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. We discuss that this is quite sus, but also you are following a pattern in code that has been stable for a while. Let's mark this as a security concern. |
||
// We are making the assumption that the IPA claim is behind the inner public inputs and pairing point object | ||
for (size_t i = 0; i < bb::IPA_CLAIM_SIZE; i++) { | ||
builder.assert_equal(builder.add_variable(num_inner_public_inputs + i), key_fields[offset].witness_index); | ||
offset++; | ||
} | ||
} | ||
|
||
for (size_t i = 0; i < Flavor::NUM_PRECOMPUTED_ENTITIES; ++i) { | ||
auto comm = curve::BN254::AffineElement::one() * fr::random_element(); | ||
auto frs = field_conversion::convert_to_bn254_frs(comm); | ||
|
@@ -96,6 +109,14 @@ void create_dummy_vkey_and_proof(Builder& builder, | |
offset++; | ||
} | ||
|
||
if (is_rollup_honk_recursion_constraint) { | ||
IPAClaimIndices ipa_claim; // WORKTODO: initialize this to something? | ||
for (auto idx : ipa_claim) { | ||
builder.assert_equal(idx, proof_fields[offset].witness_index); | ||
offset++; | ||
} | ||
} | ||
|
||
// first 8 witness commitments | ||
for (size_t i = 0; i < Flavor::NUM_WITNESS_ENTITIES; i++) { | ||
auto comm = curve::BN254::AffineElement::one() * fr::random_element(); | ||
|
@@ -107,7 +128,8 @@ void create_dummy_vkey_and_proof(Builder& builder, | |
offset += 4; | ||
} | ||
|
||
// now the univariates, which can just be 0s (8*CONST_PROOF_SIZE_LOG_N Frs, where 8 is the maximum relation degree) | ||
// now the univariates, which can just be 0s (8*CONST_PROOF_SIZE_LOG_N Frs, where 8 is the maximum relation | ||
// degree) | ||
for (size_t i = 0; i < CONST_PROOF_SIZE_LOG_N * Flavor::BATCHED_RELATION_PARTIAL_LENGTH; i++) { | ||
builder.assert_equal(builder.add_variable(fr::random_element()), proof_fields[offset].witness_index); | ||
offset++; | ||
|
@@ -162,17 +184,20 @@ void create_dummy_vkey_and_proof(Builder& builder, | |
* We would either need a separate ACIR opcode where inner_proof_contains_pairing_point_accumulator = true, | ||
* or we need non-witness data to be provided as metadata in the ACIR opcode | ||
*/ | ||
PairingPointAccumulatorIndices create_honk_recursion_constraints( | ||
|
||
template <typename Flavor> | ||
HonkRecursionConstraintOutput create_honk_recursion_constraints( | ||
Builder& builder, | ||
const RecursionConstraint& input, | ||
PairingPointAccumulatorIndices input_aggregation_object_indices, | ||
bool has_valid_witness_assignments) | ||
{ | ||
using Flavor = UltraRecursiveFlavor_<Builder>; | ||
using RecursiveVerificationKey = Flavor::VerificationKey; | ||
using RecursiveVerifier = bb::stdlib::recursion::honk::UltraRecursiveVerifier_<Flavor>; | ||
|
||
ASSERT(input.proof_type == HONK); | ||
bool is_rollup_honk_recursion_constraint = | ||
(input.proof_type == ROLLUP_HONK || input.proof_type == ROLLUP_ROOT_HONK); | ||
ASSERT(input.proof_type == HONK || is_rollup_honk_recursion_constraint); | ||
|
||
// Construct an in-circuit representation of the verification key. | ||
// For now, the v-key is a circuit constant and is fixed for the circuit. | ||
|
@@ -200,21 +225,62 @@ PairingPointAccumulatorIndices create_honk_recursion_constraints( | |
// In the constraint, the agg object public inputs are still contained in the proof. To get the 'raw' size of | ||
// the proof and public_inputs we subtract and add the corresponding amount from the respective sizes. | ||
size_t size_of_proof_with_no_pub_inputs = input.proof.size() - bb::PAIRING_POINT_ACCUMULATOR_SIZE; | ||
if (is_rollup_honk_recursion_constraint) { | ||
size_of_proof_with_no_pub_inputs -= bb::IPA_CLAIM_SIZE; | ||
} | ||
size_t total_num_public_inputs = input.public_inputs.size() + bb::PAIRING_POINT_ACCUMULATOR_SIZE; | ||
create_dummy_vkey_and_proof( | ||
builder, size_of_proof_with_no_pub_inputs, total_num_public_inputs, key_fields, proof_fields); | ||
if (is_rollup_honk_recursion_constraint) { | ||
total_num_public_inputs += bb::IPA_CLAIM_SIZE; | ||
} | ||
create_dummy_vkey_and_proof<typename Flavor::NativeFlavor>(builder, | ||
size_of_proof_with_no_pub_inputs, | ||
total_num_public_inputs, | ||
key_fields, | ||
proof_fields, | ||
is_rollup_honk_recursion_constraint); | ||
} | ||
|
||
// Recursively verify the proof | ||
auto vkey = std::make_shared<RecursiveVerificationKey>(builder, key_fields); | ||
RecursiveVerifier verifier(&builder, vkey); | ||
aggregation_state_ct input_agg_obj = bb::stdlib::recursion::convert_witness_indices_to_agg_obj<Builder, bn254>( | ||
builder, input_aggregation_object_indices); | ||
UltraRecursiveVerifierOutput<Flavor> output = verifier.verify_proof(proof_fields, input_agg_obj); | ||
std::vector<field_ct> honk_proof = proof_fields; | ||
HonkRecursionConstraintOutput output; | ||
if (is_rollup_honk_recursion_constraint) { | ||
const size_t HONK_PROOF_LENGTH = 473; | ||
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 should be changed to a formula, or maybe a formula that is a property of the flavor |
||
ASSERT(input.proof.size() == | ||
HONK_PROOF_LENGTH + 65); // WORKTODO: figure out how to not involve random constants | ||
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. Maybe doc the 65? Also WORKTODO |
||
ASSERT(proof_fields.size() == HONK_PROOF_LENGTH + 65 + input.public_inputs.size()); | ||
// split out the ipa proof | ||
const std::ptrdiff_t honk_proof_with_pub_inputs_length = | ||
static_cast<std::ptrdiff_t>(HONK_PROOF_LENGTH + input.public_inputs.size()); | ||
output.ipa_proof = | ||
StdlibProof<Builder>(honk_proof.begin() + honk_proof_with_pub_inputs_length, honk_proof.end()); | ||
honk_proof = StdlibProof<Builder>(honk_proof.begin(), honk_proof.end() + honk_proof_with_pub_inputs_length); | ||
} | ||
UltraRecursiveVerifierOutput<Flavor> verifier_output = verifier.verify_proof(honk_proof, input_agg_obj); | ||
// TODO(https://github.com/AztecProtocol/barretenberg/issues/996): investigate whether assert_equal on public inputs | ||
// is important, like what the plonk recursion constraint does. | ||
|
||
return output.agg_obj.get_witness_indices(); | ||
output.agg_obj_indices = verifier_output.agg_obj.get_witness_indices(); | ||
if (is_rollup_honk_recursion_constraint) { | ||
ASSERT(HasIPAAccumulator<Flavor>); | ||
output.ipa_claim = verifier_output.ipa_opening_claim; | ||
} | ||
return output; | ||
} | ||
|
||
template HonkRecursionConstraintOutput create_honk_recursion_constraints<UltraRecursiveFlavor_<Builder>>( | ||
Builder& builder, | ||
const RecursionConstraint& input, | ||
PairingPointAccumulatorIndices input_aggregation_object_indices, | ||
bool has_valid_witness_assignments); | ||
|
||
template HonkRecursionConstraintOutput create_honk_recursion_constraints<UltraRollupRecursiveFlavor_<Builder>>( | ||
Builder& builder, | ||
const RecursionConstraint& input, | ||
PairingPointAccumulatorIndices input_aggregation_object_indices, | ||
bool has_valid_witness_assignments); | ||
|
||
} // namespace acir_format |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,26 @@ | ||
#pragma once | ||
#include "barretenberg/commitment_schemes/claim.hpp" | ||
#include "barretenberg/dsl/acir_format/recursion_constraint.hpp" | ||
#include "barretenberg/honk/proof_system/types/proof.hpp" | ||
#include "barretenberg/stdlib/primitives/bigfield/bigfield.hpp" | ||
#include "barretenberg/stdlib/primitives/curves/grumpkin.hpp" | ||
#include <vector> | ||
|
||
namespace acir_format { | ||
using Builder = bb::UltraCircuitBuilder; | ||
|
||
using namespace bb; | ||
|
||
PairingPointAccumulatorIndices create_honk_recursion_constraints( | ||
Builder& builder, | ||
const RecursionConstraint& input, | ||
PairingPointAccumulatorIndices input_aggregation_object, | ||
bool has_valid_witness_assignments = false); | ||
struct HonkRecursionConstraintOutput { | ||
PairingPointAccumulatorIndices agg_obj_indices; | ||
OpeningClaim<stdlib::grumpkin<Builder>> ipa_claim; | ||
StdlibProof<Builder> ipa_proof; | ||
}; | ||
|
||
template <typename Flavor> | ||
HonkRecursionConstraintOutput create_honk_recursion_constraints(Builder& builder, | ||
const RecursionConstraint& input, | ||
PairingPointAccumulatorIndices input_aggregation_object, | ||
bool has_valid_witness_assignments = false); | ||
|
||
} // namespace acir_format |
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.
Resolve WORKTODO