From b9023a981706bfa3de5402d004c12b3e08cc2add Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 27 Aug 2024 23:17:00 +0000 Subject: [PATCH 1/8] initial infrstructure of surgeon; tests passing --- .../dsl/acir_format/acir_format.cpp | 18 +-- .../acir_format/honk_recursion_constraint.cpp | 59 ++++--- .../honk_recursion_constraint.test.cpp | 51 ++---- .../dsl/acir_format/proof_surgeon.hpp | 149 ++++++++++++++++++ 4 files changed, 196 insertions(+), 81 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 8c45fb977e3..3aae57e39ad 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -5,6 +5,7 @@ #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 "proof_surgeon.hpp" #include namespace acir_format { @@ -335,20 +336,9 @@ void process_honk_recursion_constraints(Builder& builder, // Add recursion constraints for (size_t i = 0; i < constraint_system.honk_recursion_constraints.size(); ++i) { auto& constraint = constraint_system.honk_recursion_constraints.at(i); - // A proof passed into the constraint should be stripped of its inner public inputs, but not the nested - // aggregation object itself. The verifier circuit requires that the indices to a nested proof aggregation - // state are a circuit constant. The user tells us they how they want these constants set by keeping the - // nested aggregation object attached to the proof as public inputs. - for (size_t i = 0; i < bb::AGGREGATION_OBJECT_SIZE; ++i) { - // Adding the nested aggregation object to the constraint's public inputs - constraint.public_inputs.emplace_back(constraint.proof[HONK_RECURSION_PUBLIC_INPUT_OFFSET + i]); - } - // Remove the aggregation object so that they can be handled as normal public inputs - // in they way that the recursion constraint expects - constraint.proof.erase( - constraint.proof.begin() + HONK_RECURSION_PUBLIC_INPUT_OFFSET, - constraint.proof.begin() + - static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET + bb::AGGREGATION_OBJECT_SIZE)); + + ProofSurgeon::move_aggregation_object_from_proof_to_public_inputs(constraint.proof, constraint.public_inputs); + current_aggregation_object = create_honk_recursion_constraints( builder, constraint, current_aggregation_object, has_valid_witness_assignments); gate_counter.track_diff(constraint_system.gates_per_opcode, diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 5d013cd65ae..65db816000a 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -6,6 +6,7 @@ #include "barretenberg/stdlib/primitives/bigfield/constants.hpp" #include "barretenberg/stdlib/primitives/curves/bn254.hpp" #include "barretenberg/stdlib_circuit_builders/ultra_recursive_flavor.hpp" +#include "proof_surgeon.hpp" #include "recursion_constraint.hpp" namespace acir_format { @@ -27,34 +28,35 @@ using aggregation_state_ct = bb::stdlib::recursion::aggregation_state; * @param proof_fields */ void create_dummy_vkey_and_proof(Builder& builder, - const RecursionConstraint& input, - std::vector& key_fields, - std::vector& proof_fields) + const size_t proof_size, + const size_t public_inputs_size, + const std::vector& key_fields, + const std::vector& proof_fields) { - using Flavor = UltraRecursiveFlavor_; + 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(); - size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs(); - assert((input.proof.size() - HONK_RECURSION_PUBLIC_INPUT_OFFSET - UltraFlavor::NUM_WITNESS_ENTITIES * num_frs_comm - - UltraFlavor::NUM_ALL_ENTITIES * num_frs_fr - 2 * num_frs_comm) % - (num_frs_comm + num_frs_fr * UltraFlavor::BATCHED_RELATION_PARTIAL_LENGTH) == + size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs(); + size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs(); + assert((proof_size - HONK_RECURSION_PUBLIC_INPUT_OFFSET - Flavor::NUM_WITNESS_ENTITIES * num_frs_comm - + Flavor::NUM_ALL_ENTITIES * num_frs_fr - 2 * num_frs_comm) % + (num_frs_comm + num_frs_fr * Flavor::BATCHED_RELATION_PARTIAL_LENGTH) == 0); // Note: this computation should always result in log_circuit_size = CONST_PROOF_SIZE_LOG_N auto log_circuit_size = - (input.proof.size() - HONK_RECURSION_PUBLIC_INPUT_OFFSET - UltraFlavor::NUM_WITNESS_ENTITIES * num_frs_comm - - UltraFlavor::NUM_ALL_ENTITIES * num_frs_fr - 2 * num_frs_comm) / - (num_frs_comm + num_frs_fr * UltraFlavor::BATCHED_RELATION_PARTIAL_LENGTH); + (proof_size - HONK_RECURSION_PUBLIC_INPUT_OFFSET - Flavor::NUM_WITNESS_ENTITIES * num_frs_comm - + Flavor::NUM_ALL_ENTITIES * num_frs_fr - 2 * num_frs_comm) / + (num_frs_comm + num_frs_fr * Flavor::BATCHED_RELATION_PARTIAL_LENGTH); // First key field is circuit size builder.assert_equal(builder.add_variable(1 << log_circuit_size), key_fields[0].witness_index); // Second key field is number of public inputs - builder.assert_equal(builder.add_variable(input.public_inputs.size()), key_fields[1].witness_index); + builder.assert_equal(builder.add_variable(public_inputs_size), key_fields[1].witness_index); // Third key field is the pub inputs offset - builder.assert_equal(builder.add_variable(UltraFlavor::has_zero_row ? 1 : 0), key_fields[2].witness_index); + builder.assert_equal(builder.add_variable(Flavor::has_zero_row ? 1 : 0), key_fields[2].witness_index); // Fourth key field is the whether the proof contains an aggregation object. builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); uint32_t offset = 4; - size_t num_inner_public_inputs = input.public_inputs.size() - bb::AGGREGATION_OBJECT_SIZE; + size_t num_inner_public_inputs = public_inputs_size - bb::AGGREGATION_OBJECT_SIZE; // We are making the assumption that the aggregation object are behind all the inner public inputs for (size_t i = 0; i < bb::AGGREGATION_OBJECT_SIZE; i++) { @@ -75,8 +77,8 @@ void create_dummy_vkey_and_proof(Builder& builder, offset = HONK_RECURSION_PUBLIC_INPUT_OFFSET; // first 3 things builder.assert_equal(builder.add_variable(1 << log_circuit_size), proof_fields[0].witness_index); - builder.assert_equal(builder.add_variable(input.public_inputs.size()), proof_fields[1].witness_index); - builder.assert_equal(builder.add_variable(UltraFlavor::has_zero_row ? 1 : 0), proof_fields[2].witness_index); + builder.assert_equal(builder.add_variable(public_inputs_size), proof_fields[1].witness_index); + builder.assert_equal(builder.add_variable(Flavor::has_zero_row ? 1 : 0), proof_fields[2].witness_index); // the inner public inputs for (size_t i = 0; i < num_inner_public_inputs; i++) { @@ -134,7 +136,7 @@ void create_dummy_vkey_and_proof(Builder& builder, builder.assert_equal(builder.add_variable(frs[3]), proof_fields[offset + 3].witness_index); offset += 4; } - ASSERT(offset == input.proof.size() + input.public_inputs.size()); + ASSERT(offset == proof_size + public_inputs_size); } /** @@ -171,26 +173,19 @@ AggregationObjectIndices create_honk_recursion_constraints(Builder& builder, } std::vector proof_fields; - // Insert the public inputs in the middle the proof fields after 'inner_public_input_offset' because this is how the - // core barretenberg library processes proofs (with the public inputs starting at the third element and not - // separate from the rest of the proof) - proof_fields.reserve(input.proof.size() + input.public_inputs.size()); - size_t i = 0; - for (const auto& idx : input.proof) { + + // Get the witness indices for the proof with the public inputs reinserted + std::vector proof_indices = + ProofSurgeon::construct_indices_for_proof_with_public_inputs(input.proof, input.public_inputs); + proof_fields.reserve(proof_indices.size()); + for (const auto& idx : proof_indices) { auto field = field_ct::from_witness_index(&builder, idx); proof_fields.emplace_back(field); - i++; - if (i == HONK_RECURSION_PUBLIC_INPUT_OFFSET) { - for (const auto& idx : input.public_inputs) { - auto field = field_ct::from_witness_index(&builder, idx); - proof_fields.emplace_back(field); - } - } } // Populate the key fields and proof fields with dummy values to prevent issues (usually with points not being on // the curve). if (!has_valid_witness_assignments) { - create_dummy_vkey_and_proof(builder, input, key_fields, proof_fields); + create_dummy_vkey_and_proof(builder, input.proof.size(), input.public_inputs.size(), key_fields, proof_fields); } // Recursively verify the proof auto vkey = std::make_shared(builder, key_fields); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp index dc3e1222f7e..a7505043ef9 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp @@ -4,6 +4,7 @@ #include "barretenberg/sumcheck/instance/prover_instance.hpp" #include "barretenberg/ultra_honk/ultra_prover.hpp" #include "barretenberg/ultra_honk/ultra_verifier.hpp" +#include "proof_surgeon.hpp" #include #include @@ -151,35 +152,27 @@ class AcirHonkRecursionConstraint : public ::testing::Test { Verifier verifier(verification_key); auto inner_proof = prover.construct_proof(); - const size_t num_inner_public_inputs = inner_circuit.get_public_inputs().size(); - std::vector proof_witnesses = inner_proof; + std::vector key_witnesses = verification_key->to_field_elements(); + const size_t num_public_inputs_sans_agg = + inner_circuit.get_public_inputs().size() - bb::AGGREGATION_OBJECT_SIZE; + // where the inner public inputs start (after circuit_size, num_pub_inputs, pub_input_offset) const size_t inner_public_input_offset = HONK_RECURSION_PUBLIC_INPUT_OFFSET; - // - Save the public inputs so that we can set their values. - // - Then truncate them from the proof because the ACIR API expects proofs without public inputs - std::vector inner_public_input_values( - proof_witnesses.begin() + static_cast(inner_public_input_offset), - proof_witnesses.begin() + - static_cast(inner_public_input_offset + num_inner_public_inputs - - bb::AGGREGATION_OBJECT_SIZE)); - - // We want to make sure that we do not remove the nested aggregation object. - proof_witnesses.erase(proof_witnesses.begin() + static_cast(inner_public_input_offset), - proof_witnesses.begin() + - static_cast(inner_public_input_offset + num_inner_public_inputs - - bb::AGGREGATION_OBJECT_SIZE)); - std::vector key_witnesses = verification_key->to_field_elements(); + // Extract the public input witnesses contained within the proof and erase them from the proof + std::vector inner_public_input_witnesses = + ProofSurgeon::extract_and_remove_public_inputs_from_proof(proof_witnesses, num_public_inputs_sans_agg); - // This is the structure of proof_witnesses and key_witnesses concatenated, which is what we end up putting + // This is the structure of proof_witnesses and key_witnesses concatenated, which is what we end up + // putting // in witness: // [ circuit size, num_pub_inputs, pub_input_offset, public_input_0, public_input_1, agg_obj_0, // agg_obj_1, ..., agg_obj_15, rest of proof..., vkey_0, vkey_1, vkey_2, vkey_3...] const uint32_t public_input_start_idx = static_cast(inner_public_input_offset + witness_offset); // points to public_input_0 - const uint32_t proof_indices_start_idx = static_cast( - public_input_start_idx + num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE); // points to agg_obj_0 + const uint32_t proof_indices_start_idx = + static_cast(public_input_start_idx + num_public_inputs_sans_agg); // points to agg_obj_0 const uint32_t key_indices_start_idx = static_cast(proof_indices_start_idx + proof_witnesses.size() - inner_public_input_offset); // would point to vkey_3 without the - @@ -202,7 +195,7 @@ class AcirHonkRecursionConstraint : public ::testing::Test { // We keep the nested aggregation object attached to the proof, // thus we do not explicitly have to keep the public inputs while setting up the initial recursion // constraint. They will later be attached as public inputs when creating the circuit. - for (size_t i = 0; i < num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE; ++i) { + for (size_t i = 0; i < num_public_inputs_sans_agg; ++i) { inner_public_inputs.push_back(static_cast(i + public_input_start_idx)); } @@ -225,9 +218,8 @@ class AcirHonkRecursionConstraint : public ::testing::Test { idx++; if (idx == inner_public_input_offset) { // before this is true, the loop adds the first three into witness - for (size_t i = 0; i < proof_indices_start_idx - public_input_start_idx; - ++i) { // adds the inner public inputs - witness.emplace_back(0); + for (size_t i = 0; i < num_public_inputs_sans_agg; ++i) { // adds the inner public inputs + witness.emplace_back(inner_public_input_witnesses[i]); } } // after this, it adds the agg obj and rest of proof } @@ -236,18 +228,7 @@ class AcirHonkRecursionConstraint : public ::testing::Test { witness.emplace_back(wit); } - // Set the values for the inner public inputs - // TODO(maxim): check this is wrong I think - // Note: this is confusing, but we minus one here due to the fact that the - // witness values have not taken into account that zero is taken up by the zero_idx - // - // We once again have to check whether we have a nested proof, because if we do have one - // then we could get a segmentation fault as `inner_public_inputs` was never filled with values. - for (size_t i = 0; i < num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE; ++i) { - witness[inner_public_inputs[i]] = inner_public_input_values[i]; - } - - witness_offset = key_indices_start_idx + key_witnesses.size(); + witness_offset = witness.size(); } std::vector honk_recursion_opcode_indices(honk_recursion_constraints.size()); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp new file mode 100644 index 00000000000..cbbfbf53a9e --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp @@ -0,0 +1,149 @@ +#pragma once +#include "barretenberg/common/serialize.hpp" +// #include "barretenberg/dsl/acir_format/honk_recursion_constraint.hpp" +#include "barretenberg/ecc/curves/bn254/fr.hpp" +#include "barretenberg/plonk_honk_shared/types/aggregation_object_type.hpp" +#include "barretenberg/serialize/msgpack.hpp" +#include + +namespace acir_format { + +class ProofSurgeon { + + // Where the public inputs start within a proof (after circuit_size, num_pub_inputs, pub_input_offset) + static constexpr size_t HONK_RECURSION_PUBLIC_INPUT_OFFSET = 3; + + public: + /** + * @brief Move the aggregation object witness indices from a proof to the public inputs + * @details A proof passed into the constraint should be stripped of its inner public inputs, but not the nested + * aggregation object itself. The verifier circuit requires that the indices to a nested proof aggregation state are + * a circuit constant. The user tells us they how they want these constants set by keeping the nested aggregation + * object attached to the proof as public inputs. + * + * @param proof + * @param public_inputs + */ + static void move_aggregation_object_from_proof_to_public_inputs(std::vector& proof, + std::vector& public_inputs) + { + // Add aggregation object indices into the public input indices + for (size_t i = 0; i < bb::AGGREGATION_OBJECT_SIZE; ++i) { + public_inputs.emplace_back(proof[HONK_RECURSION_PUBLIC_INPUT_OFFSET + i]); + } + // Remove the aggregation object indices from the proof so that they can be handled as normal public inputs in + // they way that the recursion constraint expects + proof.erase(proof.begin() + HONK_RECURSION_PUBLIC_INPUT_OFFSET, + proof.begin() + + static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET + bb::AGGREGATION_OBJECT_SIZE)); + } + + /** + * @brief Reconstruct a bberg style proof from a acir style proof + public inputs + * @details Insert the public inputs in the middle the proof fields after 'inner_public_input_offset' because this + * is how the core barretenberg library processes proofs (with the public inputs starting at the third element and + * not separate from the rest of the proof) + * + * @param proof_in A proof stripped of its public inputs + * @param public_inputs The public inputs to be reinserted into the proof + * @return std::vector The witness indices of the complete proof + */ + static std::vector construct_indices_for_proof_with_public_inputs( + const std::vector& proof_in, const std::vector& public_inputs) + { + std::vector proof; + proof.reserve(proof_in.size() + public_inputs.size()); + + // Construct a the complete proof as the concatenation {"initial data" | public_inputs | proof_in} + proof.insert(proof.end(), proof_in.begin(), proof_in.begin() + HONK_RECURSION_PUBLIC_INPUT_OFFSET); + proof.insert(proof.end(), public_inputs.begin(), public_inputs.end()); + proof.insert(proof.end(), proof_in.begin() + HONK_RECURSION_PUBLIC_INPUT_OFFSET, proof_in.end()); + + return proof; + } + + /** + * @brief Extract then remove the public inputs from a proof (excluding the aggregation object) + * + * @param proof_witnesses Witness values of a bberg style proof containing public inputs + * @param num_public_inputs The number of public inputs contained in the proof + * @return std::vector The extracted public input witness values + */ + static std::vector extract_and_remove_public_inputs_from_proof(std::vector& proof_witnesses, + const size_t num_public_inputs_sans_agg) + { + // Construct iterators pointing to the start and end of the public inputs within the proof + auto pub_inputs_begin_itr = + proof_witnesses.begin() + static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET); + auto pub_inputs_end_itr = + proof_witnesses.begin() + + static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET + num_public_inputs_sans_agg); + + // Extract the public inputs + std::vector public_input_witnesses(pub_inputs_begin_itr, pub_inputs_end_itr); + + // Erase the public inputs from the proof (with the exception of the aggregation object) + proof_witnesses.erase(pub_inputs_begin_itr, pub_inputs_end_itr); + + return public_input_witnesses; + } + + // struct RecursionConstraintInputs { + // std::vector proof_indices; + // std::vector key_indices; + // std::vector public_inputs_indices; + // }; + + // static RecursionConstraintInputs construct_constraint_input_indices(std::vector& proof_witnesses, + // const std::vector& key_witnesses, + // const size_t num_inner_public_inputs, + // const size_t witness_offset) + // { + // std::vector proof_indices; + // std::vector key_indices; + // std::vector public_inputs_indices; + + // // where the inner public inputs start (after circuit_size, num_pub_inputs, pub_input_offset) + // const size_t public_input_offset = HONK_RECURSION_PUBLIC_INPUT_OFFSET; + + // // This is the structure of proof_witnesses and key_witnesses concatenated, which is what we end up putting + // // in witness: + // // [ circuit size, num_pub_inputs, pub_input_offset, public_input_0, public_input_1, agg_obj_0, agg_obj_1, + // ..., + // // agg_obj_15, rest of proof..., vkey_0, vkey_1, vkey_2, vkey_3...] + + // // points to public_input_0 + // const uint32_t public_input_start_idx = static_cast(public_input_offset + witness_offset); + + // // points to agg_obj_0 + // const uint32_t proof_indices_start_idx = + // static_cast(public_input_start_idx + num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE); + + // // points to vkey_0 (would point to vkey_3 without the -inner_public_input_offset) + // const uint32_t key_indices_start_idx = + // static_cast(proof_indices_start_idx + proof_witnesses.size() - public_input_offset); + + // // go over circuit size, num_pub_inputs, pub_offset + // for (size_t i = 0; i < public_input_offset; ++i) { + // proof_indices.emplace_back(static_cast(i + witness_offset)); + // } + // // goes over agg_obj_0, agg_obj_1, ..., agg_obj_15 and rest of proof + // for (size_t i = 0; i < proof_witnesses.size() - public_input_offset; ++i) { + // proof_indices.emplace_back(static_cast(i + proof_indices_start_idx)); + // } + // const size_t key_size = key_witnesses.size(); + // for (size_t i = 0; i < key_size; ++i) { + // key_indices.emplace_back(static_cast(i + key_indices_start_idx)); + // } + // // We keep the nested aggregation object attached to the proof, + // // thus we do not explicitly have to keep the public inputs while setting up the initial recursion + // // constraint. They will later be attached as public inputs when creating the circuit. + // for (size_t i = 0; i < num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE; ++i) { + // public_inputs_indices.push_back(static_cast(i + public_input_start_idx)); + // } + + // return { proof_indices, key_indices, public_inputs_indices }; + // } +}; + +} // namespace acir_format From 5599967ae398ef18760cdfdda3cc43cb172996ed Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 27 Aug 2024 23:55:49 +0000 Subject: [PATCH 2/8] massive simplification to witness construction in test --- .../honk_recursion_constraint.test.cpp | 73 +------------- .../dsl/acir_format/proof_surgeon.hpp | 98 ++++++++----------- 2 files changed, 47 insertions(+), 124 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp index a7505043ef9..b216375b53c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp @@ -141,7 +141,6 @@ class AcirHonkRecursionConstraint : public ::testing::Test { { std::vector honk_recursion_constraints; - size_t witness_offset = 0; SlabVector witness; for (auto& inner_circuit : inner_circuits) { @@ -152,52 +151,12 @@ class AcirHonkRecursionConstraint : public ::testing::Test { Verifier verifier(verification_key); auto inner_proof = prover.construct_proof(); - std::vector proof_witnesses = inner_proof; std::vector key_witnesses = verification_key->to_field_elements(); - const size_t num_public_inputs_sans_agg = - inner_circuit.get_public_inputs().size() - bb::AGGREGATION_OBJECT_SIZE; - - // where the inner public inputs start (after circuit_size, num_pub_inputs, pub_input_offset) - const size_t inner_public_input_offset = HONK_RECURSION_PUBLIC_INPUT_OFFSET; - - // Extract the public input witnesses contained within the proof and erase them from the proof - std::vector inner_public_input_witnesses = - ProofSurgeon::extract_and_remove_public_inputs_from_proof(proof_witnesses, num_public_inputs_sans_agg); - - // This is the structure of proof_witnesses and key_witnesses concatenated, which is what we end up - // putting - // in witness: - // [ circuit size, num_pub_inputs, pub_input_offset, public_input_0, public_input_1, agg_obj_0, - // agg_obj_1, ..., agg_obj_15, rest of proof..., vkey_0, vkey_1, vkey_2, vkey_3...] - const uint32_t public_input_start_idx = - static_cast(inner_public_input_offset + witness_offset); // points to public_input_0 - const uint32_t proof_indices_start_idx = - static_cast(public_input_start_idx + num_public_inputs_sans_agg); // points to agg_obj_0 - const uint32_t key_indices_start_idx = - static_cast(proof_indices_start_idx + proof_witnesses.size() - - inner_public_input_offset); // would point to vkey_3 without the - - // inner_public_input_offset, points to vkey_0 - - std::vector proof_indices; - std::vector key_indices; - std::vector inner_public_inputs; - for (size_t i = 0; i < inner_public_input_offset; ++i) { // go over circuit size, num_pub_inputs, pub_offset - proof_indices.emplace_back(static_cast(i + witness_offset)); - } - for (size_t i = 0; i < proof_witnesses.size() - inner_public_input_offset; - ++i) { // goes over agg_obj_0, agg_obj_1, ..., agg_obj_15 and rest of proof - proof_indices.emplace_back(static_cast(i + proof_indices_start_idx)); - } - const size_t key_size = key_witnesses.size(); - for (size_t i = 0; i < key_size; ++i) { - key_indices.emplace_back(static_cast(i + key_indices_start_idx)); - } - // We keep the nested aggregation object attached to the proof, - // thus we do not explicitly have to keep the public inputs while setting up the initial recursion - // constraint. They will later be attached as public inputs when creating the circuit. - for (size_t i = 0; i < num_public_inputs_sans_agg; ++i) { - inner_public_inputs.push_back(static_cast(i + public_input_start_idx)); - } + std::vector proof_witnesses = inner_proof; + const size_t num_public_inputs = inner_circuit.get_public_inputs().size(); + + auto [key_indices, proof_indices, inner_public_inputs] = ProofSurgeon::populate_recursion_witness_data( + witness, proof_witnesses, key_witnesses, num_public_inputs); RecursionConstraint honk_recursion_constraint{ .key = key_indices, @@ -207,28 +166,6 @@ class AcirHonkRecursionConstraint : public ::testing::Test { .proof_type = HONK_RECURSION, }; honk_recursion_constraints.push_back(honk_recursion_constraint); - - // Setting the witness vector which just appends proof witnesses and key witnesses. - // We need to reconstruct the proof witnesses in the same order as the proof indices, with this structure: - // [ circuit size, num_pub_inputs, pub_input_offset, public_input_0, public_input_1, agg_obj_0, - // agg_obj_1, ..., agg_obj_15, rest of proof..., vkey_0, vkey_1, vkey_2, vkey_3...] - size_t idx = 0; - for (const auto& wit : proof_witnesses) { - witness.emplace_back(wit); - idx++; - if (idx == - inner_public_input_offset) { // before this is true, the loop adds the first three into witness - for (size_t i = 0; i < num_public_inputs_sans_agg; ++i) { // adds the inner public inputs - witness.emplace_back(inner_public_input_witnesses[i]); - } - } // after this, it adds the agg obj and rest of proof - } - - for (const auto& wit : key_witnesses) { - witness.emplace_back(wit); - } - - witness_offset = witness.size(); } std::vector honk_recursion_opcode_indices(honk_recursion_constraints.size()); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp index cbbfbf53a9e..a88e3a313ad 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp @@ -88,62 +88,48 @@ class ProofSurgeon { return public_input_witnesses; } - // struct RecursionConstraintInputs { - // std::vector proof_indices; - // std::vector key_indices; - // std::vector public_inputs_indices; - // }; - - // static RecursionConstraintInputs construct_constraint_input_indices(std::vector& proof_witnesses, - // const std::vector& key_witnesses, - // const size_t num_inner_public_inputs, - // const size_t witness_offset) - // { - // std::vector proof_indices; - // std::vector key_indices; - // std::vector public_inputs_indices; - - // // where the inner public inputs start (after circuit_size, num_pub_inputs, pub_input_offset) - // const size_t public_input_offset = HONK_RECURSION_PUBLIC_INPUT_OFFSET; - - // // This is the structure of proof_witnesses and key_witnesses concatenated, which is what we end up putting - // // in witness: - // // [ circuit size, num_pub_inputs, pub_input_offset, public_input_0, public_input_1, agg_obj_0, agg_obj_1, - // ..., - // // agg_obj_15, rest of proof..., vkey_0, vkey_1, vkey_2, vkey_3...] - - // // points to public_input_0 - // const uint32_t public_input_start_idx = static_cast(public_input_offset + witness_offset); - - // // points to agg_obj_0 - // const uint32_t proof_indices_start_idx = - // static_cast(public_input_start_idx + num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE); - - // // points to vkey_0 (would point to vkey_3 without the -inner_public_input_offset) - // const uint32_t key_indices_start_idx = - // static_cast(proof_indices_start_idx + proof_witnesses.size() - public_input_offset); - - // // go over circuit size, num_pub_inputs, pub_offset - // for (size_t i = 0; i < public_input_offset; ++i) { - // proof_indices.emplace_back(static_cast(i + witness_offset)); - // } - // // goes over agg_obj_0, agg_obj_1, ..., agg_obj_15 and rest of proof - // for (size_t i = 0; i < proof_witnesses.size() - public_input_offset; ++i) { - // proof_indices.emplace_back(static_cast(i + proof_indices_start_idx)); - // } - // const size_t key_size = key_witnesses.size(); - // for (size_t i = 0; i < key_size; ++i) { - // key_indices.emplace_back(static_cast(i + key_indices_start_idx)); - // } - // // We keep the nested aggregation object attached to the proof, - // // thus we do not explicitly have to keep the public inputs while setting up the initial recursion - // // constraint. They will later be attached as public inputs when creating the circuit. - // for (size_t i = 0; i < num_inner_public_inputs - bb::AGGREGATION_OBJECT_SIZE; ++i) { - // public_inputs_indices.push_back(static_cast(i + public_input_start_idx)); - // } - - // return { proof_indices, key_indices, public_inputs_indices }; - // } + struct RecursionWitnessData { + std::vector key_indices; + std::vector proof_indices; + std::vector public_inputs_indices; + }; + + static RecursionWitnessData populate_recursion_witness_data(bb::SlabVector& witness, + std::vector& proof_witnesses, + const std::vector& key_witnesses, + const size_t num_public_inputs) + { + const size_t num_public_inputs_sans_agg = num_public_inputs - bb::AGGREGATION_OBJECT_SIZE; + + std::vector public_input_witnesses = + extract_and_remove_public_inputs_from_proof(proof_witnesses, num_public_inputs_sans_agg); + + std::vector proof_indices; + std::vector key_indices; + std::vector public_inputs_indices; + + for (size_t i = 0; i < HONK_RECURSION_PUBLIC_INPUT_OFFSET; ++i) { + witness.push_back(proof_witnesses[i]); + proof_indices.push_back(static_cast(witness.size() - 1)); + } + + for (const auto& value : public_input_witnesses) { + witness.push_back(value); + public_inputs_indices.push_back(static_cast(witness.size() - 1)); + } + + for (size_t i = HONK_RECURSION_PUBLIC_INPUT_OFFSET; i < proof_witnesses.size(); ++i) { + witness.push_back(proof_witnesses[i]); + proof_indices.push_back(static_cast(witness.size() - 1)); + } + + for (const auto& value : key_witnesses) { + witness.push_back(value); + key_indices.push_back(static_cast(witness.size() - 1)); + } + + return { key_indices, proof_indices, public_inputs_indices }; + } }; } // namespace acir_format From 8f43637e514a338891964ea15405846f043cbd93 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 28 Aug 2024 14:48:15 +0000 Subject: [PATCH 3/8] no need to move agg obj to public inputs before reconstructing proof --- .../dsl/acir_format/acir_format.cpp | 10 +- .../acir_format/honk_recursion_constraint.cpp | 9 +- .../dsl/acir_format/proof_surgeon.hpp | 98 +++++++++---------- 3 files changed, 53 insertions(+), 64 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 3aae57e39ad..e3fd79051d0 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -334,15 +334,13 @@ void process_honk_recursion_constraints(Builder& builder, stdlib::recursion::init_default_agg_obj_indices(builder); // Add recursion constraints - for (size_t i = 0; i < constraint_system.honk_recursion_constraints.size(); ++i) { - auto& constraint = constraint_system.honk_recursion_constraints.at(i); - - ProofSurgeon::move_aggregation_object_from_proof_to_public_inputs(constraint.proof, constraint.public_inputs); - + size_t idx = 0; + for (auto& constraint : constraint_system.honk_recursion_constraints) { current_aggregation_object = create_honk_recursion_constraints( builder, constraint, current_aggregation_object, has_valid_witness_assignments); + gate_counter.track_diff(constraint_system.gates_per_opcode, - constraint_system.original_opcode_indices.honk_recursion_constraints.at(i)); + constraint_system.original_opcode_indices.honk_recursion_constraints.at(idx++)); } // Now that the circuit has been completely built, we add the output aggregation as public diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 65db816000a..251aaabe4b3 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -174,19 +174,20 @@ AggregationObjectIndices create_honk_recursion_constraints(Builder& builder, std::vector proof_fields; - // Get the witness indices for the proof with the public inputs reinserted + // Create witness indices for the proof with public inputs reinserted std::vector proof_indices = - ProofSurgeon::construct_indices_for_proof_with_public_inputs(input.proof, input.public_inputs); + ProofSurgeon::create_indices_for_reconstructed_proof(input.proof, input.public_inputs); proof_fields.reserve(proof_indices.size()); for (const auto& idx : proof_indices) { auto field = field_ct::from_witness_index(&builder, idx); proof_fields.emplace_back(field); } - // Populate the key fields and proof fields with dummy values to prevent issues (usually with points not being on - // the curve). + + // Populate the key fields and proof fields with dummy values to prevent issues (e.g. points must be on curve). if (!has_valid_witness_assignments) { create_dummy_vkey_and_proof(builder, input.proof.size(), input.public_inputs.size(), key_fields, proof_fields); } + // Recursively verify the proof auto vkey = std::make_shared(builder, key_fields); RecursiveVerifier verifier(&builder, vkey); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp index a88e3a313ad..6cb57ca5fdc 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp @@ -14,30 +14,6 @@ class ProofSurgeon { static constexpr size_t HONK_RECURSION_PUBLIC_INPUT_OFFSET = 3; public: - /** - * @brief Move the aggregation object witness indices from a proof to the public inputs - * @details A proof passed into the constraint should be stripped of its inner public inputs, but not the nested - * aggregation object itself. The verifier circuit requires that the indices to a nested proof aggregation state are - * a circuit constant. The user tells us they how they want these constants set by keeping the nested aggregation - * object attached to the proof as public inputs. - * - * @param proof - * @param public_inputs - */ - static void move_aggregation_object_from_proof_to_public_inputs(std::vector& proof, - std::vector& public_inputs) - { - // Add aggregation object indices into the public input indices - for (size_t i = 0; i < bb::AGGREGATION_OBJECT_SIZE; ++i) { - public_inputs.emplace_back(proof[HONK_RECURSION_PUBLIC_INPUT_OFFSET + i]); - } - // Remove the aggregation object indices from the proof so that they can be handled as normal public inputs in - // they way that the recursion constraint expects - proof.erase(proof.begin() + HONK_RECURSION_PUBLIC_INPUT_OFFSET, - proof.begin() + - static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET + bb::AGGREGATION_OBJECT_SIZE)); - } - /** * @brief Reconstruct a bberg style proof from a acir style proof + public inputs * @details Insert the public inputs in the middle the proof fields after 'inner_public_input_offset' because this @@ -48,8 +24,8 @@ class ProofSurgeon { * @param public_inputs The public inputs to be reinserted into the proof * @return std::vector The witness indices of the complete proof */ - static std::vector construct_indices_for_proof_with_public_inputs( - const std::vector& proof_in, const std::vector& public_inputs) + static std::vector create_indices_for_reconstructed_proof(const std::vector& proof_in, + const std::vector& public_inputs) { std::vector proof; proof.reserve(proof_in.size() + public_inputs.size()); @@ -63,26 +39,26 @@ class ProofSurgeon { } /** - * @brief Extract then remove the public inputs from a proof (excluding the aggregation object) + * @brief Extract then remove a given number of public inputs from a proof * * @param proof_witnesses Witness values of a bberg style proof containing public inputs - * @param num_public_inputs The number of public inputs contained in the proof + * @param num_public_inputs The number of public inputs to extract from the proof * @return std::vector The extracted public input witness values */ - static std::vector extract_and_remove_public_inputs_from_proof(std::vector& proof_witnesses, - const size_t num_public_inputs_sans_agg) + static std::vector cut_public_inputs_from_proof(std::vector& proof_witnesses, + const size_t num_public_inputs_to_extract) { // Construct iterators pointing to the start and end of the public inputs within the proof auto pub_inputs_begin_itr = proof_witnesses.begin() + static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET); auto pub_inputs_end_itr = proof_witnesses.begin() + - static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET + num_public_inputs_sans_agg); + static_cast(HONK_RECURSION_PUBLIC_INPUT_OFFSET + num_public_inputs_to_extract); - // Extract the public inputs - std::vector public_input_witnesses(pub_inputs_begin_itr, pub_inputs_end_itr); + // Construct the isolated public inputs + std::vector public_input_witnesses{ pub_inputs_begin_itr, pub_inputs_end_itr }; - // Erase the public inputs from the proof (with the exception of the aggregation object) + // Erase the public inputs from the proof proof_witnesses.erase(pub_inputs_begin_itr, pub_inputs_end_itr); return public_input_witnesses; @@ -94,41 +70,55 @@ class ProofSurgeon { std::vector public_inputs_indices; }; + /** + * @brief Populate a witness vector with key, proof, and public inputs; track witness indices for each component + * @details This method is used to constuct acir-style inputs to a recursion constraint. It is assumed that the + * provided proof contains all of its public inputs (i.e. the conventional bberg format) which are extracted herein. + * Each component is appended to the witness which may already contain data. The order in which they are added is + * arbitrary as long as the corresponding witness indices are correct. + * + * @param witness + * @param proof_witnesses + * @param key_witnesses + * @param num_public_inputs + * @return RecursionWitnessData + */ static RecursionWitnessData populate_recursion_witness_data(bb::SlabVector& witness, std::vector& proof_witnesses, const std::vector& key_witnesses, const size_t num_public_inputs) { - const size_t num_public_inputs_sans_agg = num_public_inputs - bb::AGGREGATION_OBJECT_SIZE; - + // Extract all public inputs except for those corresponding to the aggregation object + const size_t num_public_inputs_to_extract = num_public_inputs - bb::AGGREGATION_OBJECT_SIZE; std::vector public_input_witnesses = - extract_and_remove_public_inputs_from_proof(proof_witnesses, num_public_inputs_sans_agg); + cut_public_inputs_from_proof(proof_witnesses, num_public_inputs_to_extract); - std::vector proof_indices; + // Define and reserve space for witness indices for each component std::vector key_indices; - std::vector public_inputs_indices; - - for (size_t i = 0; i < HONK_RECURSION_PUBLIC_INPUT_OFFSET; ++i) { - witness.push_back(proof_witnesses[i]); - proof_indices.push_back(static_cast(witness.size() - 1)); - } - - for (const auto& value : public_input_witnesses) { + std::vector proof_indices; + std::vector public_input_indices; + key_indices.reserve(key_witnesses.size()); + proof_indices.reserve(proof_witnesses.size()); + public_input_indices.reserve(public_input_witnesses.size()); + witness.reserve(witness.size() + key_witnesses.size() + proof_witnesses.size() + public_input_witnesses.size()); + + // Add key, proof, and public inputs to witness, tracking the corresponding witness indices for each + auto witness_idx = static_cast(witness.size()); + for (const auto& value : key_witnesses) { witness.push_back(value); - public_inputs_indices.push_back(static_cast(witness.size() - 1)); + key_indices.push_back(witness_idx++); } - - for (size_t i = HONK_RECURSION_PUBLIC_INPUT_OFFSET; i < proof_witnesses.size(); ++i) { - witness.push_back(proof_witnesses[i]); - proof_indices.push_back(static_cast(witness.size() - 1)); + for (const auto& value : proof_witnesses) { + witness.push_back(value); + proof_indices.push_back(witness_idx++); } - for (const auto& value : key_witnesses) { + for (const auto& value : public_input_witnesses) { witness.push_back(value); - key_indices.push_back(static_cast(witness.size() - 1)); + public_input_indices.push_back(witness_idx++); } - return { key_indices, proof_indices, public_inputs_indices }; + return { key_indices, proof_indices, public_input_indices }; } }; From 3df7fc0b193cf66d3bf5ee84d514684c7abf43ed Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 28 Aug 2024 15:10:18 +0000 Subject: [PATCH 4/8] simplify witness pop method --- .../dsl/acir_format/proof_surgeon.hpp | 41 ++++++++----------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp index 6cb57ca5fdc..9ca2f214605 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp @@ -93,30 +93,23 @@ class ProofSurgeon { std::vector public_input_witnesses = cut_public_inputs_from_proof(proof_witnesses, num_public_inputs_to_extract); - // Define and reserve space for witness indices for each component - std::vector key_indices; - std::vector proof_indices; - std::vector public_input_indices; - key_indices.reserve(key_witnesses.size()); - proof_indices.reserve(proof_witnesses.size()); - public_input_indices.reserve(public_input_witnesses.size()); - witness.reserve(witness.size() + key_witnesses.size() + proof_witnesses.size() + public_input_witnesses.size()); - - // Add key, proof, and public inputs to witness, tracking the corresponding witness indices for each - auto witness_idx = static_cast(witness.size()); - for (const auto& value : key_witnesses) { - witness.push_back(value); - key_indices.push_back(witness_idx++); - } - for (const auto& value : proof_witnesses) { - witness.push_back(value); - proof_indices.push_back(witness_idx++); - } - - for (const auto& value : public_input_witnesses) { - witness.push_back(value); - public_input_indices.push_back(witness_idx++); - } + // Helper to append some values to the witness vector and return their corresponding indices + auto add_to_witness_and_track_indices = [](bb::SlabVector& witness, + const std::vector& input) -> std::vector { + std::vector indices; + indices.reserve(input.size()); + auto witness_idx = static_cast(witness.size()); + for (const auto& value : input) { + witness.push_back(value); + indices.push_back(witness_idx++); + } + return indices; + }; + + // Append key, proof, and public inputs while storing the associated witness indices + std::vector key_indices = add_to_witness_and_track_indices(witness, key_witnesses); + std::vector proof_indices = add_to_witness_and_track_indices(witness, proof_witnesses); + std::vector public_input_indices = add_to_witness_and_track_indices(witness, public_input_witnesses); return { key_indices, proof_indices, public_input_indices }; } From 7dd94c2ab0f91d9cda64c423f38dc2403c05350a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 28 Aug 2024 15:36:27 +0000 Subject: [PATCH 5/8] cleanup --- .../dsl/acir_format/honk_recursion_constraint.hpp | 4 ---- .../src/barretenberg/dsl/acir_format/proof_surgeon.hpp | 8 +++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.hpp index 9fe6d5a0ca8..b3741756cbd 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.hpp @@ -8,10 +8,6 @@ using Builder = bb::UltraCircuitBuilder; using namespace bb; -// In Honk, the proof starts with circuit_size, num_public_inputs, and pub_input_offset. We use this offset to keep -// track of where the public inputs start. -static constexpr size_t HONK_RECURSION_PUBLIC_INPUT_OFFSET = 3; - AggregationObjectIndices create_honk_recursion_constraints(Builder& builder, const RecursionConstraint& input, AggregationObjectIndices input_aggregation_object, diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp index 9ca2f214605..c4eaaefad1b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp @@ -1,6 +1,5 @@ #pragma once #include "barretenberg/common/serialize.hpp" -// #include "barretenberg/dsl/acir_format/honk_recursion_constraint.hpp" #include "barretenberg/ecc/curves/bn254/fr.hpp" #include "barretenberg/plonk_honk_shared/types/aggregation_object_type.hpp" #include "barretenberg/serialize/msgpack.hpp" @@ -8,11 +7,10 @@ namespace acir_format { -class ProofSurgeon { - - // Where the public inputs start within a proof (after circuit_size, num_pub_inputs, pub_input_offset) - static constexpr size_t HONK_RECURSION_PUBLIC_INPUT_OFFSET = 3; +// Where the public inputs start within a proof (after circuit_size, num_pub_inputs, pub_input_offset) +static constexpr size_t HONK_RECURSION_PUBLIC_INPUT_OFFSET = 3; +class ProofSurgeon { public: /** * @brief Reconstruct a bberg style proof from a acir style proof + public inputs From 6204ceff419cc8689a4f0727be0e87ca0794b9bb Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 28 Aug 2024 16:55:30 +0000 Subject: [PATCH 6/8] adjust sizes in dummy construction --- .../dsl/acir_format/honk_recursion_constraint.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 251aaabe4b3..0715679c3dc 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -28,13 +28,16 @@ using aggregation_state_ct = bb::stdlib::recursion::aggregation_state; * @param proof_fields */ void create_dummy_vkey_and_proof(Builder& builder, - const size_t proof_size, - const size_t public_inputs_size, + size_t proof_size, + size_t public_inputs_size, const std::vector& key_fields, const std::vector& proof_fields) { using Flavor = UltraFlavor; + proof_size -= bb::AGGREGATION_OBJECT_SIZE; + public_inputs_size += bb::AGGREGATION_OBJECT_SIZE; + // Set vkey->circuit_size correctly based on the proof size size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs(); size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs(); From 359577568bcd4adff7afda0cbdab6906849ba542 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 28 Aug 2024 17:48:24 +0000 Subject: [PATCH 7/8] clarify comments around sizes --- .../dsl/acir_format/honk_recursion_constraint.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 0715679c3dc..85f4458577e 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -23,7 +23,8 @@ using aggregation_state_ct = bb::stdlib::recursion::aggregation_state; * aggregation object, and commitments. * * @param builder - * @param input + * @param proof_size Size of proof with NO public inputs + * @param public_inputs_size Total size of public inputs including aggregation object * @param key_fields * @param proof_fields */ @@ -35,9 +36,6 @@ void create_dummy_vkey_and_proof(Builder& builder, { using Flavor = UltraFlavor; - proof_size -= bb::AGGREGATION_OBJECT_SIZE; - public_inputs_size += bb::AGGREGATION_OBJECT_SIZE; - // Set vkey->circuit_size correctly based on the proof size size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs(); size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs(); @@ -57,7 +55,7 @@ void create_dummy_vkey_and_proof(Builder& builder, // Third key field is the pub inputs offset builder.assert_equal(builder.add_variable(Flavor::has_zero_row ? 1 : 0), key_fields[2].witness_index); // Fourth key field is the whether the proof contains an aggregation object. - builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); + builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); // WORKTODO: 4 = bug? uint32_t offset = 4; size_t num_inner_public_inputs = public_inputs_size - bb::AGGREGATION_OBJECT_SIZE; @@ -188,7 +186,12 @@ AggregationObjectIndices create_honk_recursion_constraints(Builder& builder, // Populate the key fields and proof fields with dummy values to prevent issues (e.g. points must be on curve). if (!has_valid_witness_assignments) { - create_dummy_vkey_and_proof(builder, input.proof.size(), input.public_inputs.size(), key_fields, proof_fields); + // 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::AGGREGATION_OBJECT_SIZE; + size_t total_num_public_inputs = input.public_inputs.size() + bb::AGGREGATION_OBJECT_SIZE; + create_dummy_vkey_and_proof( + builder, size_of_proof_with_no_pub_inputs, total_num_public_inputs, key_fields, proof_fields); } // Recursively verify the proof From 302cdc1692bcafc32ca6e6e514f5fc16e6776ed3 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 28 Aug 2024 20:37:35 +0000 Subject: [PATCH 8/8] fix bad index in dummy proof --- .../barretenberg/dsl/acir_format/honk_recursion_constraint.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 85f4458577e..8813b9c7614 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -55,7 +55,7 @@ void create_dummy_vkey_and_proof(Builder& builder, // Third key field is the pub inputs offset builder.assert_equal(builder.add_variable(Flavor::has_zero_row ? 1 : 0), key_fields[2].witness_index); // Fourth key field is the whether the proof contains an aggregation object. - builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); // WORKTODO: 4 = bug? + 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::AGGREGATION_OBJECT_SIZE;