From 5ea373f7d653f7322a108297113a2deb379e1400 Mon Sep 17 00:00:00 2001 From: Charlie Lye Date: Wed, 11 Oct 2023 15:53:31 +0100 Subject: [PATCH] chore: acir format cleanup (#2779) * Refactor to remove redundancy in acir format (there used to be 4 copy pastes of 1 big function. shocking). * Remove composer_ as member var from acir composer for greater safety. * Fix a bug in the RAM constraint whereby setting invalid witness to true, hard baked a 0 constant into the pk causing subsequent runs to fail. --- .../dsl/acir_format/acir_format.cpp | 134 ++++-------------- .../dsl/acir_format/block_constraint.cpp | 2 +- .../dsl/acir_proofs/acir_composer.cpp | 76 ++++------ .../dsl/acir_proofs/acir_composer.hpp | 1 - 4 files changed, 55 insertions(+), 158 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 250efe82ecd..59b5192a645 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -11,29 +11,30 @@ void read_witness(Builder& builder, WitnessVector const& witness) } } -void create_circuit(Builder& builder, acir_format const& constraint_system) +void add_public_vars(Builder& builder, acir_format const& constraint_system) { - if (constraint_system.public_inputs.size() > constraint_system.varnum) { - info("create_circuit: too many public inputs!"); - } - for (size_t i = 1; i < constraint_system.varnum; ++i) { // If the index is in the public inputs vector, then we add it as a public input if (std::find(constraint_system.public_inputs.begin(), constraint_system.public_inputs.end(), i) != constraint_system.public_inputs.end()) { + builder.add_public_variable(0); + } else { builder.add_variable(0); } } +} +void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments) +{ // Add arithmetic gates for (const auto& constraint : constraint_system.constraints) { builder.create_poly_gate(constraint); } - // Add and constraint + // Add logic constraint for (const auto& constraint : constraint_system.logic_constraints) { create_logic_gate( builder, constraint.a, constraint.b, constraint.result, constraint.num_bits, constraint.is_xor_gate); @@ -54,14 +55,14 @@ void create_circuit(Builder& builder, acir_format const& constraint_system) create_schnorr_verify_constraints(builder, constraint); } - // Add ECDSA K1 constraints + // Add ECDSA k1 constraints for (const auto& constraint : constraint_system.ecdsa_k1_constraints) { - create_ecdsa_k1_verify_constraints(builder, constraint, false); + create_ecdsa_k1_verify_constraints(builder, constraint, has_valid_witness_assignments); } - // Add ECDSA R1 constraints + // Add ECDSA r1 constraints for (const auto& constraint : constraint_system.ecdsa_r1_constraints) { - create_ecdsa_r1_verify_constraints(builder, constraint, false); + create_ecdsa_r1_verify_constraints(builder, constraint, has_valid_witness_assignments); } // Add blake2s constraints @@ -94,13 +95,13 @@ void create_circuit(Builder& builder, acir_format const& constraint_system) // Add block constraints for (const auto& constraint : constraint_system.block_constraints) { - create_block_constraints(builder, constraint, false); + create_block_constraints(builder, constraint, has_valid_witness_assignments); } // Add recursion constraints for (size_t i = 0; i < constraint_system.recursion_constraints.size(); ++i) { auto& constraint = constraint_system.recursion_constraints[i]; - create_recursion_constraints(builder, constraint); + create_recursion_constraints(builder, constraint, has_valid_witness_assignments); // make sure the verification key records the public input indices of the final recursion output // (N.B. up to the ACIR description to make sure that the final output aggregation object wires are public @@ -113,6 +114,16 @@ void create_circuit(Builder& builder, acir_format const& constraint_system) } } +void create_circuit(Builder& builder, acir_format const& constraint_system) +{ + if (constraint_system.public_inputs.size() > constraint_system.varnum) { + info("create_circuit: too many public inputs!"); + } + + add_public_vars(builder, constraint_system); + build_constraints(builder, constraint_system, false); +} + Builder create_circuit(const acir_format& constraint_system, size_t size_hint) { Builder builder(size_hint); @@ -135,104 +146,9 @@ void create_circuit_with_witness(Builder& builder, acir_format const& constraint info("create_circuit_with_witness: too many public inputs!"); } - for (size_t i = 1; i < constraint_system.varnum; ++i) { - // If the index is in the public inputs vector, then we add it as a public input - - if (std::find(constraint_system.public_inputs.begin(), constraint_system.public_inputs.end(), i) != - constraint_system.public_inputs.end()) { - - builder.add_public_variable(0); - - } else { - builder.add_variable(0); - } - } - + add_public_vars(builder, constraint_system); read_witness(builder, witness); - - // Add arithmetic gates - for (const auto& constraint : constraint_system.constraints) { - builder.create_poly_gate(constraint); - } - - // Add logic constraint - for (const auto& constraint : constraint_system.logic_constraints) { - create_logic_gate( - builder, constraint.a, constraint.b, constraint.result, constraint.num_bits, constraint.is_xor_gate); - } - - // Add range constraint - for (const auto& constraint : constraint_system.range_constraints) { - builder.create_range_constraint(constraint.witness, constraint.num_bits, ""); - } - - // Add sha256 constraints - for (const auto& constraint : constraint_system.sha256_constraints) { - create_sha256_constraints(builder, constraint); - } - - // Add schnorr constraints - for (const auto& constraint : constraint_system.schnorr_constraints) { - create_schnorr_verify_constraints(builder, constraint); - } - - // Add ECDSA k1 constraints - for (const auto& constraint : constraint_system.ecdsa_k1_constraints) { - create_ecdsa_k1_verify_constraints(builder, constraint); - } - - // Add ECDSA r1 constraints - for (const auto& constraint : constraint_system.ecdsa_r1_constraints) { - create_ecdsa_r1_verify_constraints(builder, constraint); - } - - // Add blake2s constraints - for (const auto& constraint : constraint_system.blake2s_constraints) { - create_blake2s_constraints(builder, constraint); - } - - // Add keccak constraints - for (const auto& constraint : constraint_system.keccak_constraints) { - create_keccak_constraints(builder, constraint); - } - for (const auto& constraint : constraint_system.keccak_var_constraints) { - create_keccak_var_constraints(builder, constraint); - } - - // Add pedersen constraints - for (const auto& constraint : constraint_system.pedersen_constraints) { - create_pedersen_constraint(builder, constraint); - } - - // Add fixed base scalar mul constraints - for (const auto& constraint : constraint_system.fixed_base_scalar_mul_constraints) { - create_fixed_base_constraint(builder, constraint); - } - - // Add hash to field constraints - for (const auto& constraint : constraint_system.hash_to_field_constraints) { - create_hash_to_field_constraints(builder, constraint); - } - - // Add block constraints - for (const auto& constraint : constraint_system.block_constraints) { - create_block_constraints(builder, constraint); - } - - // Add recursion constraints - for (size_t i = 0; i < constraint_system.recursion_constraints.size(); ++i) { - auto& constraint = constraint_system.recursion_constraints[i]; - create_recursion_constraints(builder, constraint, true); - - // make sure the verification key records the public input indices of the final recursion output - // (N.B. up to the ACIR description to make sure that the final output aggregation object wires are public - // inputs!) - if (i == constraint_system.recursion_constraints.size() - 1) { - std::vector proof_output_witness_indices(constraint.output_aggregation_object.begin(), - constraint.output_aggregation_object.end()); - builder.set_recursive_proof(proof_output_witness_indices); - } - } + build_constraints(builder, constraint_system, true); } } // namespace acir_format diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 9eb908f0b00..882d1ac14ed 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -55,7 +55,7 @@ void create_block_constraints(Builder& builder, const BlockConstraint constraint field_ct value = poly_to_field_ct(op.value, builder); field_ct index = poly_to_field_ct(op.index, builder); if (has_valid_witness_assignments == false) { - index = field_ct(0); + index = field_ct::from_witness(&builder, 0); } if (op.access_type == 0) { value.assert_equal(table.read(index)); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp index 8ed136b7b6e..c5afdc10046 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -12,83 +12,60 @@ namespace acir_proofs { AcirComposer::AcirComposer(size_t size_hint, bool verbose) - : composer_(/*p_key=*/0, /*v_key=*/0) - , size_hint_(size_hint) + : size_hint_(size_hint) , verbose_(verbose) {} void AcirComposer::create_circuit(acir_format::acir_format& constraint_system) { + if (builder_.get_num_gates() > 1) { + return; + } + vinfo("building circuit..."); builder_ = acir_format::create_circuit(constraint_system, size_hint_); - - // We are done with the constraint system at this point, and we need the memory slab back. - constraint_system.constraints.clear(); - constraint_system.constraints.shrink_to_fit(); - exact_circuit_size_ = builder_.get_num_gates(); total_circuit_size_ = builder_.get_total_circuit_size(); circuit_subgroup_size_ = builder_.get_circuit_subgroup_size(total_circuit_size_); size_hint_ = circuit_subgroup_size_; + vinfo("gates: ", builder_.get_total_circuit_size()); } void AcirComposer::init_proving_key(acir_format::acir_format& constraint_system) { - vinfo("building circuit... ", size_hint_); - builder_ = acir_format::Builder(size_hint_); - acir_format::create_circuit(builder_, constraint_system); - - // We are done with the constraint system at this point, and we need the memory slab back. - constraint_system.constraints.clear(); - constraint_system.constraints.shrink_to_fit(); - - exact_circuit_size_ = builder_.get_num_gates(); - total_circuit_size_ = builder_.get_total_circuit_size(); - circuit_subgroup_size_ = builder_.get_circuit_subgroup_size(total_circuit_size_); - - composer_ = acir_format::Composer(); + create_circuit(constraint_system); + acir_format::Composer composer; vinfo("computing proving key..."); - proving_key_ = composer_.compute_proving_key(builder_); + proving_key_ = composer.compute_proving_key(builder_); } std::vector AcirComposer::create_proof(acir_format::acir_format& constraint_system, acir_format::WitnessVector& witness, bool is_recursive) { - // Release prior memory first. - composer_ = acir_format::Composer(/*p_key=*/0, /*v_key=*/0); - - vinfo("building circuit..."); + vinfo("building circuit with witness..."); builder_ = acir_format::Builder(size_hint_); create_circuit_with_witness(builder_, constraint_system, witness); vinfo("gates: ", builder_.get_total_circuit_size()); - composer_ = [&]() { + auto composer = [&]() { if (proving_key_) { - auto composer = acir_format::Composer(proving_key_, nullptr); - return composer; - } else { - return acir_format::Composer(); + return acir_format::Composer(proving_key_, nullptr); } - }(); - if (!proving_key_) { + + acir_format::Composer composer; vinfo("computing proving key..."); - proving_key_ = composer_.compute_proving_key(builder_); + proving_key_ = composer.compute_proving_key(builder_); vinfo("done."); - } - - // We are done with the constraint system at this point, and we need the memory slab back. - constraint_system.constraints.clear(); - constraint_system.constraints.shrink_to_fit(); - witness.clear(); - witness.shrink_to_fit(); + return composer; + }(); vinfo("creating proof..."); std::vector proof; if (is_recursive) { - auto prover = composer_.create_prover(builder_); + auto prover = composer.create_prover(builder_); proof = prover.construct_proof().proof_data; } else { - auto prover = composer_.create_ultra_with_keccak_prover(builder_); + auto prover = composer.create_ultra_with_keccak_prover(builder_); proof = prover.construct_proof().proof_data; } vinfo("done."); @@ -97,8 +74,12 @@ std::vector AcirComposer::create_proof(acir_format::acir_format& constr std::shared_ptr AcirComposer::init_verification_key() { + if (!proving_key_) { + throw_or_abort("Compute proving key first."); + } vinfo("computing verification key..."); - verification_key_ = composer_.compute_verification_key(builder_); + acir_format::Composer composer(proving_key_, nullptr); + verification_key_ = composer.compute_verification_key(builder_); vinfo("done."); return verification_key_; } @@ -107,14 +88,15 @@ void AcirComposer::load_verification_key(proof_system::plonk::verification_key_d { verification_key_ = std::make_shared( std::move(data), srs::get_crs_factory()->get_verifier_crs()); - composer_ = acir_format::Composer(proving_key_, verification_key_); } bool AcirComposer::verify_proof(std::vector const& proof, bool is_recursive) { + acir_format::Composer composer(proving_key_, verification_key_); + if (!verification_key_) { vinfo("computing verification key..."); - verification_key_ = composer_.compute_verification_key(builder_); + verification_key_ = composer.compute_verification_key(builder_); vinfo("done."); } @@ -122,10 +104,10 @@ bool AcirComposer::verify_proof(std::vector const& proof, bool is_recur builder_.public_inputs.resize((proof.size() - 2144) / 32); if (is_recursive) { - auto verifier = composer_.create_verifier(builder_); + auto verifier = composer.create_verifier(builder_); return verifier.verify_proof({ proof }); } else { - auto verifier = composer_.create_ultra_with_keccak_verifier(builder_); + auto verifier = composer.create_ultra_with_keccak_verifier(builder_); return verifier.verify_proof({ proof }); } } diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.hpp index 58696e4ad25..32b678268e3 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.hpp @@ -38,7 +38,6 @@ class AcirComposer { private: acir_format::Builder builder_; - acir_format::Composer composer_; size_t size_hint_; size_t exact_circuit_size_; size_t total_circuit_size_;