Skip to content
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: unify all acir recursion constraints based on RecursionConstraint and proof_type #7993

Merged
merged 24 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8110a60
feat: Quick prototype of verify_proof constant
sirasistant Jul 20, 2024
04ea98f
fix bb tests
sirasistant Jul 22, 2024
1730d04
add GateCounter class and recursion constarint methods
ledwards2225 Aug 14, 2024
758d6a4
fix gate counter
ledwards2225 Aug 14, 2024
745099d
there is only one RecursionConstraint
ledwards2225 Aug 14, 2024
a58f2ca
comments
ledwards2225 Aug 14, 2024
4856474
Merge branch 'master' into lde/proof_type
ledwards2225 Aug 14, 2024
8c009ee
comment
ledwards2225 Aug 14, 2024
49adb68
updates, fix noir build
ledwards2225 Aug 15, 2024
8625ba0
format fix
ledwards2225 Aug 15, 2024
3154647
Merge branch 'master' into lde/proof_type
ledwards2225 Aug 15, 2024
1bfa5cc
add integration test for debugging; appears to be working correctly
ledwards2225 Aug 15, 2024
ae5936f
continue to use honk_recursion flag to set proof_type to avoid errors…
ledwards2225 Aug 15, 2024
c8c9de9
cleanup and remove print to maybe fix gates report?
ledwards2225 Aug 15, 2024
ed01ea6
add todo
ledwards2225 Aug 15, 2024
1ef5048
comment update
ledwards2225 Aug 15, 2024
8a18513
remove some prints
ledwards2225 Aug 15, 2024
60fb90f
Merge branch 'master' into lde/proof_type
ledwards2225 Aug 15, 2024
cb3353b
comments and an assert
ledwards2225 Aug 15, 2024
4704c64
Update barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_co…
ledwards2225 Aug 15, 2024
4e75b87
Update barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp
ledwards2225 Aug 15, 2024
0404933
comments and formatting
ledwards2225 Aug 15, 2024
44b20e0
Merge branch 'lde/proof_type' of github.com:AztecProtocol/aztec-packa…
ledwards2225 Aug 15, 2024
88e26f3
format
ledwards2225 Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
373 changes: 186 additions & 187 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct AcirFormat {
std::vector<MultiScalarMul> multi_scalar_mul_constraints;
std::vector<EcAdd> ec_add_constraints;
std::vector<RecursionConstraint> recursion_constraints;
std::vector<HonkRecursionConstraint> honk_recursion_constraints;
std::vector<RecursionConstraint> honk_recursion_constraints;
Copy link
Contributor Author

@ledwards2225 ledwards2225 Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we can probably move to a model where there is only one vector of RecursionConstraints and they are handled accordingly based on proof_type but that's going to require a larger refactor and I've already strayed a bit from the original intent of this PR :)

std::vector<BigIntFromLeBytes> bigint_from_le_bytes_constraints;
std::vector<BigIntToLeBytes> bigint_to_le_bytes_constraints;
std::vector<BigIntOperation> bigint_operations;
Expand Down Expand Up @@ -204,4 +204,49 @@ void build_constraints(
// circuit. This distinction is needed to not add the default
// aggregation object when we're not using the honk RV.

/**
* @brief Utility class for tracking the gate count of acir constraints
*
*/
template <typename Builder> class GateCounter {
public:
GateCounter(Builder* builder, bool collect_gates_per_opcode)
: builder(builder)
, collect_gates_per_opcode(collect_gates_per_opcode)
{}

size_t compute_diff()
{
if (!collect_gates_per_opcode) {
return 0;
}
size_t new_gate_count = builder->get_num_gates();
size_t diff = new_gate_count - prev_gate_count;
prev_gate_count = new_gate_count;
return diff;
}

void track_diff(std::vector<size_t>& gates_per_opcode, size_t opcode_index)
{
if (collect_gates_per_opcode) {
gates_per_opcode[opcode_index] = compute_diff();
}
}

private:
Builder* builder;
bool collect_gates_per_opcode;
size_t prev_gate_count{};
};

void process_plonk_recursion_constraints(Builder& builder,
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,
bool honk_recursion,
GateCounter<Builder>& gate_counter);

} // namespace acir_format
Original file line number Diff line number Diff line change
Expand Up @@ -543,4 +543,26 @@ TEST_F(AcirIntegrationTest, DISABLED_UpdateAcirCircuit)
EXPECT_TRUE(prove_and_verify_honk<Flavor>(circuit));
}

/**
* @brief Test recursive honk recursive verification
*
*/
TEST_F(AcirIntegrationTest, DISABLED_HonkRecursion)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a convenient way to debug honk recursion without getting bb involved

{
using Flavor = UltraFlavor;
using Builder = Flavor::CircuitBuilder;

std::string test_name = "verify_honk_proof"; // arbitrary program with RAM gates
auto acir_program = get_program_data_from_test_file(
test_name,
/*honk_recursion=*/false); // WORKTODO: TODO(https://github.com/AztecProtocol/barretenberg/issues/1013):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this false here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its false because with this new pattern, the honk_recursion flag is only used for the "this circuit will be recursively verified with honk" use case. The verifier is set to a honk verifier via the noir verify_proof_with_type call directly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a comment to this effect

// Assumes Flavor is not UltraHonk

// Construct a bberg circuit from the acir representation
auto circuit = acir_format::create_circuit<Builder>(acir_program.constraints, 0, acir_program.witness);

EXPECT_TRUE(CircuitChecker::check(circuit));
EXPECT_TRUE(prove_and_verify_honk<Flavor>(circuit));
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -415,27 +415,35 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg,
});
af.original_opcode_indices.keccak_permutations.push_back(opcode_index);
} else if constexpr (std::is_same_v<T, Program::BlackBoxFuncCall::RecursiveAggregation>) {
if (honk_recursion) { // if we're using the honk recursive verifier
auto c = HonkRecursionConstraint{
.key = map(arg.verification_key, [](auto& e) { return get_witness_from_function_input(e); }),
.proof = map(arg.proof, [](auto& e) { return get_witness_from_function_input(e); }),
.public_inputs =
map(arg.public_inputs, [](auto& e) { return get_witness_from_function_input(e); }),
};

auto input_key = get_witness_from_function_input(arg.key_hash);

auto proof_type_in = arg.proof_type;
// 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_RECURSION) {
// info("WARNING: Recursion type is not being specified correctly via noir verify_proof()!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// info("WARNING: Recursion type is not being specified correctly via noir verify_proof()!");

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having prints here seemed to make the gate_counts tests on CI complain?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was suggesting to delete this as I was surprised to see it left here with no comment

proof_type_in = HONK_RECURSION;
}

auto c = RecursionConstraint{
.key = map(arg.verification_key, [](auto& e) { return get_witness_from_function_input(e); }),
.proof = map(arg.proof, [](auto& e) { return get_witness_from_function_input(e); }),
.public_inputs = map(arg.public_inputs, [](auto& e) { return get_witness_from_function_input(e); }),
.key_hash = input_key,
.proof_type = proof_type_in,
};
// Add the recursion constraint to the appropriate container based on proof type
if (c.proof_type == PLONK_RECURSION) {
af.recursion_constraints.push_back(c);
af.original_opcode_indices.recursion_constraints.push_back(opcode_index);
} else if (c.proof_type == HONK_RECURSION) {
af.honk_recursion_constraints.push_back(c);
af.original_opcode_indices.honk_recursion_constraints.push_back(opcode_index);
} else {
auto input_key = get_witness_from_function_input(arg.key_hash);

auto c = RecursionConstraint{
.key = map(arg.verification_key, [](auto& e) { return get_witness_from_function_input(e); }),
.proof = map(arg.proof, [](auto& e) { return get_witness_from_function_input(e); }),
.public_inputs =
map(arg.public_inputs, [](auto& e) { return get_witness_from_function_input(e); }),
.key_hash = input_key,
};
af.recursion_constraints.push_back(c);
af.original_opcode_indices.recursion_constraints.push_back(opcode_index);
info("Invalid PROOF_TYPE in RecursionConstraint!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could probably print the valid proof types here vs. what was specified but this is a nit and can come later. In general, before moving to honk as the main public facing backend for noir we probably need to improve some of the error messages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave this one for a follow on since I'll be touching this code a lot in coming weeks

ASSERT(false);
}
} else if constexpr (std::is_same_v<T, Program::BlackBoxFuncCall::BigIntFromLeBytes>) {
af.bigint_from_le_bytes_constraints.push_back(BigIntFromLeBytes{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ using aggregation_state_ct = bb::stdlib::recursion::aggregation_state<bn254>;
* @param proof_fields
*/
void create_dummy_vkey_and_proof(Builder& builder,
const HonkRecursionConstraint& input,
const RecursionConstraint& input,
std::vector<field_ct>& key_fields,
std::vector<field_ct>& proof_fields)
{
Expand All @@ -36,16 +36,15 @@ void create_dummy_vkey_and_proof(Builder& builder,
// Set vkey->circuit_size correctly based on the proof size
size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs<UltraFlavor::Commitment>();
size_t num_frs_fr = bb::field_conversion::calc_num_bn254_frs<UltraFlavor::FF>();
assert((input.proof.size() - HonkRecursionConstraint::inner_public_input_offset -
UltraFlavor::NUM_WITNESS_ENTITIES * num_frs_comm - UltraFlavor::NUM_ALL_ENTITIES * num_frs_fr -
2 * num_frs_comm) %
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) ==
0);
// Note: this computation should always result in log_circuit_size = CONST_PROOF_SIZE_LOG_N
auto log_circuit_size = (input.proof.size() - HonkRecursionConstraint::inner_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);
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);
// 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
Expand Down Expand Up @@ -73,7 +72,7 @@ void create_dummy_vkey_and_proof(Builder& builder,
offset += 4;
}

offset = HonkRecursionConstraint::inner_public_input_offset;
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);
Expand Down Expand Up @@ -151,7 +150,7 @@ void create_dummy_vkey_and_proof(Builder& builder,
* or we need non-witness data to be provided as metadata in the ACIR opcode
*/
AggregationObjectIndices create_honk_recursion_constraints(Builder& builder,
const HonkRecursionConstraint& input,
const RecursionConstraint& input,
AggregationObjectIndices input_aggregation_object_indices,
bool has_valid_witness_assignments)
{
Expand Down Expand Up @@ -179,7 +178,7 @@ AggregationObjectIndices create_honk_recursion_constraints(Builder& builder,
auto field = field_ct::from_witness_index(&builder, idx);
proof_fields.emplace_back(field);
i++;
if (i == HonkRecursionConstraint::inner_public_input_offset) {
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#pragma once
#include "barretenberg/dsl/acir_format/recursion_constraint.hpp"
#include "barretenberg/stdlib/primitives/bigfield/bigfield.hpp"
#include <vector>

Expand All @@ -7,55 +8,12 @@ using Builder = bb::UltraCircuitBuilder;

using namespace bb;

/**
* @brief HonkRecursionConstraint struct contains information required to recursively verify a proof!
*
* @details The recursive verifier algorithm produces an 'aggregation object' representing 2 G1 points, expressed as 16
* witness values. The smart contract Verifier must be aware of this aggregation object in order to complete the full
* recursive verification. If the circuit verifies more than 1 proof, the recursion algorithm will update a pre-existing
* aggregation object (`input_aggregation_object`).
*
* @details We currently require that the inner circuit being verified only has a single public input. If more are
* required, the outer circuit can hash them down to 1 input.
*
* @param verification_key_data The inner circuit vkey. Is converted into circuit witness values (internal to the
* backend)
* @param proof The honk proof. Is converted into circuit witness values (internal to the backend)
* @param is_aggregation_object_nonzero A flag to tell us whether the circuit has already recursively verified proofs
* (and therefore an aggregation object is present)
* @param public_input The index of the single public input
* @param input_aggregation_object Witness indices of pre-existing aggregation object (if it exists)
* @param output_aggregation_object Witness indices of the aggregation object produced by recursive verification
* @param nested_aggregation_object Public input indices of an aggregation object inside the proof.
*
* @note If input_aggregation_object witness indices are all zero, we interpret this to mean that the inner proof does
* NOT contain a previously recursively verified proof
* @note nested_aggregation_object is used for cases where the proof being verified contains an aggregation object in
* its public inputs! If this is the case, we record the public input locations in `nested_aggregation_object`. If the
* inner proof is of a circuit that does not have a nested aggregation object, these values are all zero.
*
* To outline the interaction between the input_aggergation_object and the nested_aggregation_object take the following
* example: If we have a circuit that verifies 2 proofs A and B, the recursion constraint for B will have an
* input_aggregation_object that points to the aggregation output produced by verifying A. If circuit B also verifies a
* proof, in the above example the recursion constraint for verifying B will have a nested object that describes the
* aggregation object in B’s public inputs as well as an input aggregation object that points to the object produced by
* the previous recursion constraint in the circuit (the one that verifies A)
*
* TODO(https://github.com/AztecProtocol/barretenberg/issues/996): Update these comments for Honk.
*/
struct HonkRecursionConstraint {
// 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 inner_public_input_offset = 3;
std::vector<uint32_t> key;
std::vector<uint32_t> proof;
std::vector<uint32_t> public_inputs;

friend bool operator==(HonkRecursionConstraint const& lhs, HonkRecursionConstraint const& rhs) = default;
};
// 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 HonkRecursionConstraint& input,
const RecursionConstraint& input,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should add an assert in this function that it's actually taking in a honk recursion constraint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea. added

AggregationObjectIndices input_aggregation_object,
bool has_valid_witness_assignments = false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class AcirHonkRecursionConstraint : public ::testing::Test {
*/
Builder create_outer_circuit(std::vector<Builder>& inner_circuits)
{
std::vector<HonkRecursionConstraint> honk_recursion_constraints;
std::vector<RecursionConstraint> honk_recursion_constraints;

size_t witness_offset = 0;
std::vector<fr, ContainerSlabAllocator<fr>> witness;
Expand All @@ -155,7 +155,7 @@ class AcirHonkRecursionConstraint : public ::testing::Test {

std::vector<fr> proof_witnesses = inner_proof;
// where the inner public inputs start (after circuit_size, num_pub_inputs, pub_input_offset)
const size_t inner_public_input_offset = 3;
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<fr> inner_public_input_values(
Expand Down Expand Up @@ -206,10 +206,12 @@ class AcirHonkRecursionConstraint : public ::testing::Test {
inner_public_inputs.push_back(static_cast<uint32_t>(i + public_input_start_idx));
}

HonkRecursionConstraint honk_recursion_constraint{
RecursionConstraint honk_recursion_constraint{
.key = key_indices,
.proof = proof_indices,
.public_inputs = inner_public_inputs,
.key_hash = 0, // not used
.proof_type = HONK_RECURSION,
};
honk_recursion_constraints.push_back(honk_recursion_constraint);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace acir_format {

// Used to specify the type of recursive verifier via the proof_type specified by verify_proof() from noir
enum PROOF_TYPE { PLONK_RECURSION, HONK_RECURSION };

using namespace bb::plonk;
using Builder = bb::UltraCircuitBuilder;

Expand Down Expand Up @@ -43,6 +46,7 @@ using Builder = bb::UltraCircuitBuilder;
* aggregation object in B’s public inputs as well as an input aggregation object that points to the object produced by
* the previous recursion constraint in the circuit (the one that verifies A)
*
* TODO(https://github.com/AztecProtocol/barretenberg/issues/996): Create similar comments for Honk.
*/
struct RecursionConstraint {
// An aggregation state is represented by two G1 affine elements. Each G1 point has
Expand All @@ -52,6 +56,7 @@ struct RecursionConstraint {
std::vector<uint32_t> proof;
std::vector<uint32_t> public_inputs;
uint32_t key_hash;
uint32_t proof_type;

friend bool operator==(RecursionConstraint const& lhs, RecursionConstraint const& rhs) = default;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ Builder create_outer_circuit(std::vector<Builder>& inner_circuits)
.proof = proof_indices,
.public_inputs = inner_public_inputs,
.key_hash = key_hash_start_idx,
.proof_type = PLONK_RECURSION,
};
recursion_constraints.push_back(recursion_constraint);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,7 @@ struct BlackBoxFuncCall {
std::vector<Program::FunctionInput> proof;
std::vector<Program::FunctionInput> public_inputs;
Program::FunctionInput key_hash;
uint32_t proof_type;

friend bool operator==(const RecursiveAggregation&, const RecursiveAggregation&);
std::vector<uint8_t> bincodeSerialize() const;
Expand Down Expand Up @@ -3609,6 +3610,9 @@ inline bool operator==(const BlackBoxFuncCall::RecursiveAggregation& lhs,
if (!(lhs.key_hash == rhs.key_hash)) {
return false;
}
if (!(lhs.proof_type == rhs.proof_type)) {
return false;
}
return true;
}

Expand Down Expand Up @@ -3641,6 +3645,7 @@ void serde::Serializable<Program::BlackBoxFuncCall::RecursiveAggregation>::seria
serde::Serializable<decltype(obj.proof)>::serialize(obj.proof, serializer);
serde::Serializable<decltype(obj.public_inputs)>::serialize(obj.public_inputs, serializer);
serde::Serializable<decltype(obj.key_hash)>::serialize(obj.key_hash, serializer);
serde::Serializable<decltype(obj.proof_type)>::serialize(obj.proof_type, serializer);
}

template <>
Expand All @@ -3653,6 +3658,7 @@ Program::BlackBoxFuncCall::RecursiveAggregation serde::Deserializable<
obj.proof = serde::Deserializable<decltype(obj.proof)>::deserialize(deserializer);
obj.public_inputs = serde::Deserializable<decltype(obj.public_inputs)>::deserialize(deserializer);
obj.key_hash = serde::Deserializable<decltype(obj.key_hash)>::deserialize(deserializer);
obj.proof_type = serde::Deserializable<decltype(obj.proof_type)>::deserialize(deserializer);
return obj;
}

Expand Down
Loading
Loading