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 6f66e74ce9f..b36ba94d33e 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -3,6 +3,7 @@ #include "barretenberg/dsl/acir_format/pedersen.hpp" #include "barretenberg/dsl/acir_format/recursion_constraint.hpp" #include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp" +#include namespace acir_format { @@ -158,10 +159,10 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b // These are set and modified whenever we encounter a recursion opcode // // These should not be set by the caller - // TODO: Check if this is always the case. ie I won't receive a proof that will set the first - // TODO input_aggregation_object to be non-zero. - // TODO: if not, we can add input_aggregation_object to the proof too for all recursive proofs - // TODO: This might be the case for proof trees where the proofs are created on different machines + // TODO(maxim): Check if this is always the case. ie I won't receive a proof that will set the first + // TODO(maxim): input_aggregation_object to be non-zero. + // TODO(maxim): if not, we can add input_aggregation_object to the proof too for all recursive proofs + // TODO(maxim): This might be the case for proof trees where the proofs are created on different machines std::array current_input_aggregation_object = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; @@ -169,13 +170,45 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; + // Get the size of proof with no public inputs prepended to it + // This is used while processing recursion constraints to determine whether + // the proof we are verifying contains a recursive proof itself + auto proof_size_no_pub_inputs = recursion_proof_size_without_public_inputs(); + // Add recursion constraints - for (size_t i = 0; i < constraint_system.recursion_constraints.size(); ++i) { - auto& constraint = constraint_system.recursion_constraints[i]; + for (auto constraint : constraint_system.recursion_constraints) { + // A proof passed into the constraint should be stripped of its public inputs, except in the case where a + // proof contains an aggregation object itself. We refer to this as the `nested_aggregation_object`. 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. As this is the only object that can prepended to the proof if the + // proof is above the expected size (with public inputs stripped) + std::array nested_aggregation_object = {}; + // If the proof has public inputs attached to it, we should handle setting the nested aggregation object + if (constraint.proof.size() > proof_size_no_pub_inputs) { + // The public inputs attached to a proof should match the aggregation object in size + ASSERT(constraint.proof.size() - proof_size_no_pub_inputs == + RecursionConstraint::AGGREGATION_OBJECT_SIZE); + for (size_t i = 0; i < RecursionConstraint::AGGREGATION_OBJECT_SIZE; ++i) { + // Set the nested aggregation object indices to the current size of the public inputs + // This way we know that the nested aggregation object indices will always be the last + // indices of the public inputs + nested_aggregation_object[i] = static_cast(constraint.public_inputs.size()); + // Attach the nested aggregation object to the end of the public inputs to fill in + // the slot where the nested aggregation object index will point into + constraint.public_inputs.emplace_back(constraint.proof[i]); + } + // Remove the aggregation object so that they can be handled as normal public inputs + // in they way taht the recursion constraint expects + constraint.proof.erase(constraint.proof.begin(), + constraint.proof.begin() + + static_cast(RecursionConstraint::AGGREGATION_OBJECT_SIZE)); + } + current_output_aggregation_object = create_recursion_constraints(builder, constraint, current_input_aggregation_object, - constraint.nested_aggregation_object, + nested_aggregation_object, has_valid_witness_assignments); current_input_aggregation_object = current_output_aggregation_object; } @@ -241,25 +274,26 @@ void create_circuit_with_witness(Builder& builder, acir_format const& constraint /** * @brief Apply an offset to the indices stored in the wires - * @details This method is needed due to the following: Noir constructs "wires" as indices into a "witness" vector. This - * is analogous to the wires and variables vectors in bberg builders. Were it not for the addition of constant variables - * in the constructors of a builder (e.g. zero), we would simply have noir.wires = builder.wires and noir.witness = - * builder.variables. To account for k-many constant variables in the first entries of the variables array, we have - * something like variables = variables.append(noir.witness). Accordingly, the indices in noir.wires have to be - * incremented to account for the offset at which noir.wires was placed into variables. + * @details This method is needed due to the following: Noir constructs "wires" as indices into a "witness" vector. + * This is analogous to the wires and variables vectors in bberg builders. Were it not for the addition of constant + * variables in the constructors of a builder (e.g. zero), we would simply have noir.wires = builder.wires and + * noir.witness = builder.variables. To account for k-many constant variables in the first entries of the variables + * array, we have something like variables = variables.append(noir.witness). Accordingly, the indices in noir.wires + * have to be incremented to account for the offset at which noir.wires was placed into variables. * * @tparam Builder * @param builder */ template void apply_wire_index_offset(Builder& builder) { - // For now, noir has a hard coded witness index offset = 1. Once this is removed, this pre-applied offset goes away + // For now, noir has a hard coded witness index offset = 1. Once this is removed, this pre-applied offset goes + // away const uint32_t pre_applied_noir_offset = 1; auto offset = static_cast(builder.num_vars_added_in_constructor - pre_applied_noir_offset); info("Applying offset = ", offset); - // Apply the offset to the indices stored the wires that were generated from acir. (Do not apply the offset to those - // values that were added in the builder constructor). + // Apply the offset to the indices stored the wires that were generated from acir. (Do not apply the offset to + // those values that were added in the builder constructor). size_t start_index = builder.num_vars_added_in_constructor; for (auto& wire : builder.wires) { for (size_t idx = start_index; idx < wire.size(); ++idx) { diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp index f01d4f60f66..813f2022745 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp @@ -33,11 +33,6 @@ std::array create_recurs Builder& builder, const RecursionConstraint& input, std::array input_aggregation_object, - // TODO: does this need to be a part of the recursion opcode? - // TODO: or can we figure it out from the vk? - // TODO: either way we could probably have the user explicitly provide it - // TODO: in Noir. - // Note: this is not being used in Noir at the moment std::array nested_aggregation_object, bool has_valid_witness_assignments) { @@ -141,6 +136,7 @@ std::array create_recurs std::shared_ptr vkey = verification_key_ct::from_field_elements( &builder, key_fields, inner_proof_contains_recursive_proof, nested_aggregation_indices); vkey->program_width = noir_recursive_settings::program_width; + Transcript_ct transcript(&builder, manifest, proof_fields, input.public_inputs.size()); aggregation_state_ct result = proof_system::plonk::stdlib::recursion::verify_proof_( &builder, vkey, transcript, previous_aggregation); @@ -358,6 +354,13 @@ std::vector export_dummy_transcript_in_recursion_format(const return fields; } +size_t recursion_proof_size_without_public_inputs() +{ + const auto manifest = Composer::create_manifest(0); + auto dummy_transcript = export_dummy_transcript_in_recursion_format(manifest, false); + return dummy_transcript.size(); +} + G1AsFields export_g1_affine_element_as_fields(const barretenberg::g1::affine_element& group_element) { const uint256_t x = group_element.x; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.hpp index 345b6744cd6..8f6618106d2 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.hpp @@ -53,13 +53,14 @@ struct RecursionConstraint { std::vector proof; std::vector public_inputs; uint32_t key_hash; - // TODO:This is now unused, but we keep it here for backwards compatibility + // TODO(maxim):This is now unused, but we keep it here for backwards compatibility std::array input_aggregation_object; - // TODO: This is now unused, but we keep it here for backwards compatibility + // TODO(maxim): This is now unused, but we keep it here for backwards compatibility std::array output_aggregation_object; - // TODO: This is currently not being used on the Noir level at all - // TODO: we don't have a way to specify that the proof we are creating contains a - // TODO: aggregation object (ie it is also verifying a proof) + // TODO(maxim): This is currently not being used on the Noir level at all, + // TODO(maxim): but we keep it here for backwards compatibility + // TODO(maxim): The object is now currently contained by the `proof` field + // TODO(maxim): and is handled when serializing ACIR to a barretenberg circuit std::array nested_aggregation_object; friend bool operator==(RecursionConstraint const& lhs, RecursionConstraint const& rhs) = default; @@ -79,6 +80,7 @@ std::vector export_dummy_key_in_recursion_format(const Polynom std::vector export_transcript_in_recursion_format(const transcript::StandardTranscript& transcript); std::vector export_dummy_transcript_in_recursion_format(const transcript::Manifest& manifest, const bool contains_recursive_proof); +size_t recursion_proof_size_without_public_inputs(); // In order to interact with a recursive aggregation state inside of a circuit, we need to represent its internal G1 // elements as field elements. This happens in multiple locations when creating a recursion constraint. The struct and diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp index b80d437e6d5..a64f65462bc 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp @@ -139,8 +139,8 @@ Builder create_outer_circuit(std::vector& inner_circuits) auto inner_verifier = inner_composer.create_verifier(inner_circuit); const bool has_nested_proof = inner_verifier.key->contains_recursive_proof; - const size_t num_inner_public_inputs = inner_circuit.get_public_inputs().size(); + const size_t num_inner_public_inputs = inner_circuit.get_public_inputs().size(); transcript::StandardTranscript transcript(inner_proof.proof_data, Composer::create_manifest(num_inner_public_inputs), transcript::HashType::PedersenBlake3s, @@ -149,11 +149,16 @@ Builder create_outer_circuit(std::vector& inner_circuits) std::vector proof_witnesses = export_transcript_in_recursion_format(transcript); // - 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(), proof_witnesses.begin() + static_cast(num_inner_public_inputs)); - proof_witnesses.erase(proof_witnesses.begin(), - proof_witnesses.begin() + static_cast(num_inner_public_inputs)); + + // We want to make sure that we do not remove the nested aggregation object in the case of the proof we want to + // recursively verify contains a recursive proof itself. We are safe to keep all the inner public inputs + // as in these tests the outer circuits do not have public inputs themselves + if (!has_nested_proof) { + proof_witnesses.erase(proof_witnesses.begin(), + proof_witnesses.begin() + static_cast(num_inner_public_inputs)); + } const std::vector key_witnesses = export_key_in_recursion_format(inner_verifier.key); @@ -187,8 +192,13 @@ Builder create_outer_circuit(std::vector& inner_circuits) for (size_t i = 0; i < key_size; ++i) { key_indices.emplace_back(static_cast(i + key_indices_start_idx)); } - for (size_t i = 0; i < num_inner_public_inputs; ++i) { - inner_public_inputs.push_back(static_cast(i + public_input_start_idx)); + // In the case of a nested proof 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. + if (!has_nested_proof) { + for (size_t i = 0; i < num_inner_public_inputs; ++i) { + inner_public_inputs.push_back(static_cast(i + public_input_start_idx)); + } } RecursionConstraint recursion_constraint{ @@ -201,21 +211,30 @@ Builder create_outer_circuit(std::vector& inner_circuits) .nested_aggregation_object = nested_aggregation_object, }; recursion_constraints.push_back(recursion_constraint); + for (size_t i = 0; i < proof_indices_start_idx - witness_offset; ++i) { witness.emplace_back(0); } for (const auto& wit : proof_witnesses) { witness.emplace_back(wit); } + for (const auto& wit : key_witnesses) { witness.emplace_back(wit); } + // Set the values for the inner public inputs // 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 - for (size_t i = 0; i < num_inner_public_inputs; ++i) { - witness[inner_public_inputs[i] - 1] = inner_public_input_values[i]; + // + // 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. + if (!has_nested_proof) { + for (size_t i = 0; i < num_inner_public_inputs; ++i) { + witness[inner_public_inputs[i] - 1] = inner_public_input_values[i]; + } } + witness_offset = key_indices_start_idx + key_witnesses.size(); circuit_idx++; }