From 4833af47f52e5e1e398a46d50250624c62c256bd Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 20 Dec 2023 20:40:05 +0000 Subject: [PATCH 01/20] new builder constructor from acir data --- .../dsl/acir_format/acir_format.cpp | 6 ++-- .../dsl/acir_format/acir_format.hpp | 3 ++ .../dsl/acir_proofs/acir_composer.cpp | 25 +++++++++++----- .../cpp/src/barretenberg/goblin/goblin.hpp | 1 + .../goblin_ultra_circuit_builder.hpp | 28 +++++++++++++++++ .../proof_system/instance_inspector.hpp | 30 +++++++++++++++++-- .../sumcheck/instance/prover_instance.cpp | 2 +- 7 files changed, 82 insertions(+), 13 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 6f66e74ce9f9..81f642e5ab75 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -153,7 +153,9 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b // TODO(https://github.com/AztecProtocol/barretenberg/issues/817): disable these for UGH for now since we're not yet // dealing with proper recursion if constexpr (IsGoblinBuilder) { - info("WARNING: this circuit contains recursion_constraints!"); + if (constraint_system.recursion_constraints.size() > 0) { + info("WARNING: this circuit contains recursion_constraints!"); + } } else { // These are set and modified whenever we encounter a recursion opcode // @@ -277,6 +279,6 @@ template void create_circuit_with_witness(GoblinUltra acir_format const& constraint_system, WitnessVector const& witness); template void apply_wire_index_offset(GoblinUltraCircuitBuilder& builder); -template void apply_wire_index_offset(UltraCircuitBuilder& builder); +template void build_constraints(GoblinUltraCircuitBuilder&, acir_format const&, bool); } // namespace acir_format diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp index c9c373ac3398..fa9a77e5805d 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp @@ -88,4 +88,7 @@ void create_circuit_with_witness(Builder& builder, const acir_format& constraint template void apply_wire_index_offset(Builder& builder); +template +void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments); + } // namespace acir_format 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 2901784eefea..b0ce15654aa1 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -81,17 +81,26 @@ std::vector AcirComposer::create_proof(acir_format::acir_format& constr void AcirComposer::create_goblin_circuit(acir_format::acir_format& constraint_system, acir_format::WitnessVector& witness) { - // Provide the builder with the op queue owned by the goblin instance - goblin_builder_.op_queue = goblin.op_queue; - - create_circuit_with_witness(goblin_builder_, constraint_system, witness); + // The public inputs in constraint_system do not index into "witness" but rather into the future "variables" which + // it assumes will be equal to witness but with a prepended zero. We want to remove this +1 so that public_inputs + // properly indexes into witness bc we're about to make calls like add_variable(witness[public_inputs[idx]]). Once + // the +1 is removed from noir, this correction can be removed entirely and we can use + // constraint_system.public_inputs directly. + const uint32_t pre_applied_noir_offset = 1; + std::vector corrected_public_inputs; + for (const auto& index : constraint_system.public_inputs) { + corrected_public_inputs.emplace_back(index - pre_applied_noir_offset); + } - info("after create_circuit_with_witness: num_gates = ", goblin_builder_.num_gates); + // Construct a builder using the witness and public input data from acir + goblin_builder_ = + acir_format::GoblinBuilder{ goblin.op_queue, witness, corrected_public_inputs, constraint_system.varnum }; - // Correct for the addition of const variables in the builder constructor - acir_format::apply_wire_index_offset(goblin_builder_); + // Populate constraints in the builder via the data in constraint_system + acir_format::build_constraints(goblin_builder_, constraint_system, true); - // Add some arbitrary op gates to ensure the associated polynomials are non-zero + // HACK: Add some arbitrary op gates to ensure the associated polynomials are non-zero and to give ECCVM and + // Translator some ECC ops to process. GoblinTestingUtils::construct_goblin_ecc_op_circuit(goblin_builder_); } diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 7e74349529e6..634badfb58d1 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -170,6 +170,7 @@ class Goblin { auto prover = composer.create_prover(instance); auto ultra_proof = prover.construct_proof(); instance_inspector::inspect_instance(instance); + instance_inspector::print_databus_info(instance); // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): no merge prover for now since we're not // mocking the first set of ecc ops diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index ed0e41d5ae9c..5f114930654b 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -88,6 +88,34 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui : GoblinUltraCircuitBuilder_(0, op_queue_in) {} + GoblinUltraCircuitBuilder_(std::shared_ptr op_queue_in, + auto& witness_values, + std::vector& public_inputs, + size_t varnum) + : UltraCircuitBuilder_>() + , op_queue(op_queue_in) + { + // Kev says this should virtually always be true for Noir programs. + ASSERT(witness_values.size() == varnum - 1); + + // Add the variables and public inputs known directly from acir + for (size_t idx = 0; idx < witness_values.size(); ++idx) { + auto& value = witness_values[idx]; + if (std::find(public_inputs.begin(), public_inputs.end(), idx) != public_inputs.end()) { + this->add_public_variable(value); + } else { + this->add_variable(value); + } + } + + // Set indices to constants corresponding to Goblin ECC op codes + null_op_idx = this->zero_idx; + add_accum_op_idx = this->put_constant_variable(FF(EccOpCode::ADD_ACCUM)); + mul_accum_op_idx = this->put_constant_variable(FF(EccOpCode::MUL_ACCUM)); + equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); + num_vars_added_in_constructor = this->variables.size(); + }; + void finalize_circuit(); void add_gates_to_ensure_all_polys_are_non_zero(); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp b/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp index 48216e97fbf4..63c2d61fbfed 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp @@ -30,9 +30,9 @@ void inspect_instance(auto& prover_instance) } } if (zero_polys.empty()) { - info("\nDebug Utility: All prover polynomials are non-zero."); + info("\nInstance Inspector: All prover polynomials are non-zero."); } else { - info("\nDebug Utility: The following prover polynomials are identically zero: "); + info("\nInstance Inspector: The following prover polynomials are identically zero: "); for (const std::string& label : zero_polys) { info("\t", label); } @@ -40,4 +40,30 @@ void inspect_instance(auto& prover_instance) info(); } +/** + * @brief Print some useful info about polys related to the databus lookup relation + * + * @param prover_instance + */ +void print_databus_info(auto& prover_instance) +{ + info("\nInstance Inspector: Printing databus gate info."); + auto& prover_polys = prover_instance->prover_polynomials; + for (size_t idx = 0; idx < prover_instance->proving_key->circuit_size; ++idx) { + if (prover_polys.q_busread[idx] == 1) { + info("idx = ", idx); + info("q_busread = ", prover_polys.q_busread[idx]); + info("w_l = ", prover_polys.w_l[idx]); + info("w_r = ", prover_polys.w_r[idx]); + } + if (prover_polys.calldata_read_counts[idx] > 0) { + info("idx = ", idx); + info("read_counts = ", prover_polys.calldata_read_counts[idx]); + info("calldata = ", prover_polys.calldata[idx]); + info("databus_id = ", prover_polys.databus_id[idx]); + } + } + info(); +} + } // namespace instance_inspector \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp index 836a4728bd6f..340425429f87 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp @@ -195,7 +195,7 @@ void ProverInstance_::construct_databus_polynomials(Circuit& circuit) // Note: We do not utilize a zero row for databus columns for (size_t idx = 0; idx < circuit.public_calldata.size(); ++idx) { public_calldata[idx] = circuit.get_variable(circuit.public_calldata[idx]); - calldata_read_counts[idx] = circuit.get_variable(circuit.calldata_read_counts[idx]); + calldata_read_counts[idx] = circuit.calldata_read_counts[idx]; } proving_key->calldata = public_calldata.share(); From ec262d596468c6a8d841393335e75ad9186d525c Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 18:32:22 +0000 Subject: [PATCH 02/20] try running all acir tests w goblin --- barretenberg/acir_tests/Dockerfile.bb | 2 +- .../src/barretenberg/eccvm/eccvm_verifier.cpp | 4 ++-- .../cpp/src/barretenberg/goblin/goblin.hpp | 22 +++++++++++-------- .../goblin_ultra_circuit_builder.hpp | 12 ++++++---- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/barretenberg/acir_tests/Dockerfile.bb b/barretenberg/acir_tests/Dockerfile.bb index 3dee4e370d88..62c5f4f9dd3a 100644 --- a/barretenberg/acir_tests/Dockerfile.bb +++ b/barretenberg/acir_tests/Dockerfile.bb @@ -11,6 +11,6 @@ COPY . . # This ensures we test independent pk construction through real/garbage witness data paths. RUN FLOW=prove_then_verify ./run_acir_tests.sh # TODO(https://github.com/AztecProtocol/barretenberg/issues/811) make this able to run the default test -RUN FLOW=prove_and_verify_goblin ./run_acir_tests.sh assert_statement +RUN FLOW=prove_and_verify_goblin ./run_acir_tests.sh # Run 1_mul through native bb build, all_cmds flow, to test all cli args. RUN VERBOSE=1 FLOW=all_cmds ./run_acir_tests.sh 1_mul \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp index 5de5a4a03438..32e28e62a7d8 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp @@ -199,7 +199,7 @@ template bool ECCVMVerifier_::verify_proof(const plonk if (commitment.y != 0) { batched_commitment_unshifted += commitment * rhos[commitment_idx]; } else { - info("ECCVM Verifier: point at infinity (unshifted)"); + // info("ECCVM Verifier: point at infinity (unshifted)"); } ++commitment_idx; } @@ -210,7 +210,7 @@ template bool ECCVMVerifier_::verify_proof(const plonk if (commitment.y != 0) { batched_commitment_to_be_shifted += commitment * rhos[commitment_idx]; } else { - info("ECCVM Verifier: point at infinity (to be shifted)"); + // info("ECCVM Verifier: point at infinity (to be shifted)"); } ++commitment_idx; } diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 634badfb58d1..925e15fab409 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -169,8 +169,8 @@ class Goblin { auto instance = composer.create_instance(circuit_builder); auto prover = composer.create_prover(instance); auto ultra_proof = prover.construct_proof(); - instance_inspector::inspect_instance(instance); - instance_inspector::print_databus_info(instance); + // instance_inspector::inspect_instance(instance); + // instance_inspector::print_databus_info(instance); // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): no merge prover for now since we're not // mocking the first set of ecc ops @@ -191,7 +191,7 @@ class Goblin { // ACIRHACK Proof prove_for_acir() { - info("Goblin.prove(): op_queue size = ", op_queue->ultra_ops[0].size()); + // info("Goblin.prove(): op_queue size = ", op_queue->ultra_ops[0].size()); Proof proof; proof.merge_proof = std::move(merge_proof); @@ -236,9 +236,9 @@ class Goblin { // ACIRHACK std::vector construct_proof(GoblinUltraCircuitBuilder& builder) { - info("goblin: construct_proof"); + // info("goblin: construct_proof"); accumulate_for_acir(builder); - info("accumulate complete."); + // info("accumulate complete."); std::vector goblin_proof = prove_for_acir().to_buffer(); std::vector result(accumulator.proof.proof_data.size() + goblin_proof.size()); @@ -257,15 +257,19 @@ class Goblin { const auto extract_final_kernel_proof = [&]([[maybe_unused]] auto& input_proof) { return accumulator.proof; }; GoblinUltraVerifier verifier{ accumulator.verification_key }; - info("constructed GUH verifier"); + // info("constructed GUH verifier"); bool verified = verifier.verify_proof(extract_final_kernel_proof(proof)); - info(" verified GUH proof; result: ", verified); + if (verified) { + info("GUH verification SUCCEEDED"); + } else { + info("GUH verification FAILED"); + } const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; auto goblin_proof = extract_goblin_proof(proof); - info("extracted goblin proof"); + // info("extracted goblin proof"); verified = verified && verify_for_acir(goblin_proof); - info("verified goblin proof"); + // info("verified goblin proof"); return verified; } }; diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index 5f114930654b..db8c8f5c31a5 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -95,12 +95,16 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui : UltraCircuitBuilder_>() , op_queue(op_queue_in) { - // Kev says this should virtually always be true for Noir programs. - ASSERT(witness_values.size() == varnum - 1); + // WORKTODO: clarify this in comments + // // Kev says this should virtually always be true for Noir programs. + // info("witness_values.size() = ", witness_values.size()); + // info("varnum = ", varnum); + // // ASSERT(witness_values.size() == varnum - 1); // Add the variables and public inputs known directly from acir - for (size_t idx = 0; idx < witness_values.size(); ++idx) { - auto& value = witness_values[idx]; + for (size_t idx = 0; idx < varnum; ++idx) { + // for (size_t idx = 0; idx < witness_values.size(); ++idx) { + auto value = idx < witness_values.size() ? witness_values[idx] : 0; if (std::find(public_inputs.begin(), public_inputs.end(), idx) != public_inputs.end()) { this->add_public_variable(value); } else { From 4a0c291c8db6c66040373504933ef21bc3658ad5 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 19:10:58 +0000 Subject: [PATCH 03/20] increase default srs size --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 2 +- .../src/barretenberg/commitment_schemes/commitment_key.hpp | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 35c260f8969f..9b7bb73cea7c 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -46,7 +46,7 @@ acir_proofs::AcirComposer init(acir_format::acir_format& constraint_system) void init_reference_strings() { // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode subgroup size - size_t subgroup_size = 32768; + size_t subgroup_size = 65536; // TODO(https://github.com/AztecProtocol/barretenberg/issues/811) reduce duplication with above // Must +1! diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 394ff93fdbef..2a7e4223b8e5 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -69,6 +69,12 @@ template class CommitmentKey { Commitment commit(std::span polynomial) { const size_t degree = polynomial.size(); + if (degree <= srs->get_monomial_size()) { + info("CommitmentKey:"); + info("degree = ", degree); + info("srs->get_monomial_size() = ", srs->get_monomial_size()); + } + ASSERT(degree <= srs->get_monomial_size()); return barretenberg::scalar_multiplication::pippenger_unsafe( const_cast(polynomial.data()), srs->get_monomial_points(), degree, pippenger_runtime_state); From 32a1bc0f770ce1fe9c9695d8209327fe0293256e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 19:55:02 +0000 Subject: [PATCH 04/20] fix bad print --- .../cpp/src/barretenberg/commitment_schemes/commitment_key.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 2a7e4223b8e5..fdd2ee406342 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -69,7 +69,7 @@ template class CommitmentKey { Commitment commit(std::span polynomial) { const size_t degree = polynomial.size(); - if (degree <= srs->get_monomial_size()) { + if (degree > srs->get_monomial_size()) { info("CommitmentKey:"); info("degree = ", degree); info("srs->get_monomial_size() = ", srs->get_monomial_size()); From 57bb2c04e4032e1f97db53f6bc7c96931734e706 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 20:08:52 +0000 Subject: [PATCH 05/20] bump srs size again --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 9b7bb73cea7c..274b5bea66b1 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -46,7 +46,7 @@ acir_proofs::AcirComposer init(acir_format::acir_format& constraint_system) void init_reference_strings() { // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode subgroup size - size_t subgroup_size = 65536; + size_t subgroup_size = 131072; // TODO(https://github.com/AztecProtocol/barretenberg/issues/811) reduce duplication with above // Must +1! From 72b8ff7ddd5cd8d6002f9612eee4c46b000e256a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 20:31:29 +0000 Subject: [PATCH 06/20] bump srs yet again --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 274b5bea66b1..823e0db0fb8d 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -46,7 +46,7 @@ acir_proofs::AcirComposer init(acir_format::acir_format& constraint_system) void init_reference_strings() { // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode subgroup size - size_t subgroup_size = 131072; + size_t subgroup_size = 262144; // TODO(https://github.com/AztecProtocol/barretenberg/issues/811) reduce duplication with above // Must +1! From 92b49f1792ac37dd215864e53ac2195fb95c7308 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 21:31:11 +0000 Subject: [PATCH 07/20] remove some of the old offset solution stuff --- .../dsl/acir_format/acir_format.cpp | 30 ----------------- .../dsl/acir_format/acir_format.hpp | 2 -- .../goblin_ultra_circuit_builder.hpp | 33 +++++++++---------- .../circuit_builder/ultra_circuit_builder.hpp | 3 -- 4 files changed, 16 insertions(+), 52 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 81f642e5ab75..da0e38666c68 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -241,35 +241,6 @@ void create_circuit_with_witness(Builder& builder, acir_format const& constraint build_constraints(builder, constraint_system, true); } -/** - * @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. - * - * @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 - 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). - 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) { - wire[idx] += offset; - } - } -} - template UltraCircuitBuilder create_circuit(const acir_format& constraint_system, size_t size_hint); template void create_circuit_with_witness(UltraCircuitBuilder& builder, @@ -278,7 +249,6 @@ template void create_circuit_with_witness(UltraCircuitBuild template void create_circuit_with_witness(GoblinUltraCircuitBuilder& builder, acir_format const& constraint_system, WitnessVector const& witness); -template void apply_wire_index_offset(GoblinUltraCircuitBuilder& builder); template void build_constraints(GoblinUltraCircuitBuilder&, acir_format const&, bool); } // namespace acir_format diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp index fa9a77e5805d..2860e810818c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp @@ -86,8 +86,6 @@ Builder create_circuit_with_witness(const acir_format& constraint_system, template void create_circuit_with_witness(Builder& builder, const acir_format& constraint_system, WitnessVector const& witness); -template void apply_wire_index_offset(Builder& builder); - template void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index db8c8f5c31a5..56d5d4a3fc8d 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -1,13 +1,7 @@ #pragma once -#include "barretenberg/polynomials/polynomial.hpp" #include "barretenberg/proof_system/arithmetization/arithmetization.hpp" #include "barretenberg/proof_system/op_queue/ecc_op_queue.hpp" -#include "barretenberg/proof_system/plookup_tables/plookup_tables.hpp" -#include "barretenberg/proof_system/plookup_tables/types.hpp" -#include "barretenberg/proof_system/types/merkle_hash_type.hpp" -#include "barretenberg/proof_system/types/pedersen_commitment_type.hpp" #include "ultra_circuit_builder.hpp" -#include namespace proof_system { @@ -20,8 +14,6 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui static constexpr size_t DEFAULT_NON_NATIVE_FIELD_LIMB_BITS = UltraCircuitBuilder_>::DEFAULT_NON_NATIVE_FIELD_LIMB_BITS; - size_t num_vars_added_in_constructor = 0; // needed in constructing circuit from acir - size_t num_ecc_op_gates = 0; // number of ecc op "gates" (rows); these are placed at the start of the circuit // Stores record of ecc operations and performs corresponding native operations internally @@ -82,12 +74,25 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui add_accum_op_idx = this->put_constant_variable(FF(EccOpCode::ADD_ACCUM)); mul_accum_op_idx = this->put_constant_variable(FF(EccOpCode::MUL_ACCUM)); equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); - num_vars_added_in_constructor = this->variables.size(); }; GoblinUltraCircuitBuilder_(std::shared_ptr op_queue_in) : GoblinUltraCircuitBuilder_(0, op_queue_in) {} + /** + * @brief Constructor from data generated from ACIR + * + * @param op_queue_in Op queue to which goblinized group ops will be added + * @param witness_values witnesses values known to acir + * @param public_inputs indices of public inputs in witness array + * @param varnum number of known witness + * + * @note The size of witness_values may be less than varnum. The former is the set of actual witness values known at + * the time of acir generation. The former may be larger and essentially acounts for placeholders for witnesses that + * we know will exist but whose values are not known during acir generation. Both are in general less than the total + * number of variables/witnesses that might be present for a circuit generated from acir, since many gates will + * depend on the details of the bberg implementation (or more generally on the backend used to process acir). + */ GoblinUltraCircuitBuilder_(std::shared_ptr op_queue_in, auto& witness_values, std::vector& public_inputs, @@ -95,15 +100,10 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui : UltraCircuitBuilder_>() , op_queue(op_queue_in) { - // WORKTODO: clarify this in comments - // // Kev says this should virtually always be true for Noir programs. - // info("witness_values.size() = ", witness_values.size()); - // info("varnum = ", varnum); - // // ASSERT(witness_values.size() == varnum - 1); - // Add the variables and public inputs known directly from acir for (size_t idx = 0; idx < varnum; ++idx) { - // for (size_t idx = 0; idx < witness_values.size(); ++idx) { + // Zeros are added for variables whose existence is known but whose values are not yet known. The values may + // be "set" later on via the assert_equal mechanism. auto value = idx < witness_values.size() ? witness_values[idx] : 0; if (std::find(public_inputs.begin(), public_inputs.end(), idx) != public_inputs.end()) { this->add_public_variable(value); @@ -117,7 +117,6 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui add_accum_op_idx = this->put_constant_variable(FF(EccOpCode::ADD_ACCUM)); mul_accum_op_idx = this->put_constant_variable(FF(EccOpCode::MUL_ACCUM)); equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); - num_vars_added_in_constructor = this->variables.size(); }; void finalize_circuit(); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index 86b13627e998..14c7f9cc2b9d 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -56,8 +56,6 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasezero_idx = put_constant_variable(FF::zero()); this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this - num_vars_added_in_constructor = this->variables.size(); }; UltraCircuitBuilder_(const UltraCircuitBuilder_& other) = default; UltraCircuitBuilder_(UltraCircuitBuilder_&& other) From 69c773420f7730e571e58ea3d4db5303138437b4 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 22:10:51 +0000 Subject: [PATCH 08/20] remove verbose prints --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 10 +++++----- .../circuit_builder/goblin_ultra_circuit_builder.cpp | 8 ++++++++ .../circuit_builder/goblin_ultra_circuit_builder.hpp | 11 +++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 1656ce502d96..510bf11f74e2 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -145,23 +145,23 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& witnessPath, [[maybe_unused]] bool recursive) { - info("Construct constraint_system and witness."); + // info("Construct constraint_system and witness."); auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); init_reference_strings(); - info("Construct goblin circuit from constraint system and witness."); + // info("Construct goblin circuit from constraint system and witness."); acir_proofs::AcirComposer acir_composer; acir_composer.create_goblin_circuit(constraint_system, witness); - info("Construct goblin proof."); + // info("Construct goblin proof."); auto proof = acir_composer.create_goblin_proof(); - info("verify_goblin_proof."); + // info("verify_goblin_proof."); auto verified = acir_composer.verify_goblin_proof(proof); - vinfo("verified: ", verified); + // vinfo("verified: ", verified); return verified; } diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp index 8b30c606781a..85e4a36cbb2f 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp @@ -246,6 +246,14 @@ template void GoblinUltraCircuitBuilder_::populate_ecc_op_wire num_ecc_op_gates += 2; }; +template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_code_constant_variables() +{ + null_op_idx = this->zero_idx; + add_accum_op_idx = this->put_constant_variable(FF(EccOpCode::ADD_ACCUM)); + mul_accum_op_idx = this->put_constant_variable(FF(EccOpCode::MUL_ACCUM)); + equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); +} + template void GoblinUltraCircuitBuilder_::create_poseidon2_external_gate(const poseidon2_external_gate_& in) { diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index 56d5d4a3fc8d..3d153f858dc0 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -62,6 +62,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui private: void populate_ecc_op_wires(const ecc_op_tuple& in); ecc_op_tuple decompose_ecc_operands(uint32_t op, const g1::affine_element& point, const FF& scalar = FF::zero()); + void set_goblin_ecc_op_code_constant_variables(); public: GoblinUltraCircuitBuilder_(const size_t size_hint = 0, @@ -70,10 +71,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui , op_queue(op_queue_in) { // Set indices to constants corresponding to Goblin ECC op codes - null_op_idx = this->zero_idx; - add_accum_op_idx = this->put_constant_variable(FF(EccOpCode::ADD_ACCUM)); - mul_accum_op_idx = this->put_constant_variable(FF(EccOpCode::MUL_ACCUM)); - equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); + set_goblin_ecc_op_code_constant_variables(); }; GoblinUltraCircuitBuilder_(std::shared_ptr op_queue_in) : GoblinUltraCircuitBuilder_(0, op_queue_in) @@ -113,10 +111,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui } // Set indices to constants corresponding to Goblin ECC op codes - null_op_idx = this->zero_idx; - add_accum_op_idx = this->put_constant_variable(FF(EccOpCode::ADD_ACCUM)); - mul_accum_op_idx = this->put_constant_variable(FF(EccOpCode::MUL_ACCUM)); - equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); + set_goblin_ecc_op_code_constant_variables(); }; void finalize_circuit(); From a99d49628474af24866fabbe6c309c8bee992833 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 22:40:56 +0000 Subject: [PATCH 09/20] couple of prints --- barretenberg/cpp/src/barretenberg/goblin/goblin.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 925e15fab409..f5f61e022630 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -198,14 +198,18 @@ class Goblin { eccvm_builder = std::make_unique(op_queue); eccvm_composer = std::make_unique(); + info("ECCVM: create_prover"); auto eccvm_prover = eccvm_composer->create_prover(*eccvm_builder); + info("ECCVM: construct_proof"); proof.eccvm_proof = eccvm_prover.construct_proof(); proof.translation_evaluations = eccvm_prover.translation_evaluations; translator_builder = std::make_unique( eccvm_prover.translation_batching_challenge_v, eccvm_prover.evaluation_challenge_x, op_queue); translator_composer = std::make_unique(); + info("Translator: create_prover"); auto translator_prover = translator_composer->create_prover(*translator_builder, eccvm_prover.transcript); + info("Translator: construct_proof"); proof.translator_proof = translator_prover.construct_proof(); proof_ = proof; // ACIRHACK From 4806d12cfefbb6aaead7cd5d0b1e0c4c5853a52e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 22:54:12 +0000 Subject: [PATCH 10/20] more prints --- barretenberg/cpp/src/barretenberg/goblin/goblin.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index f5f61e022630..80bed3188ac8 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -225,13 +225,18 @@ class Goblin { // bool merge_verified = merge_verifier.verify_proof(proof.merge_proof); // info("verified merge proof. result: ", merge_verified); + info("ECCVM: create_verifier"); auto eccvm_verifier = eccvm_composer->create_verifier(*eccvm_builder); + info("ECCVM: verify_proof"); bool eccvm_verified = eccvm_verifier.verify_proof(proof.eccvm_proof); + info("Translator: create_verifier"); auto translator_verifier = translator_composer->create_verifier(*translator_builder, eccvm_verifier.transcript); + info("Translator: verify_proof"); bool accumulator_construction_verified = translator_verifier.verify_proof(proof.translator_proof); // TODO(https://github.com/AztecProtocol/barretenberg/issues/799): Ensure translation_evaluations are passed // correctly + info("Translator: verify_translation"); bool translation_verified = translator_verifier.verify_translation(proof.translation_evaluations); return /* merge_verified && */ eccvm_verified && accumulator_construction_verified && translation_verified; From 149a3a773db9d88a464e2e6eb0095ebd31f9e508 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 2 Jan 2024 23:37:04 +0000 Subject: [PATCH 11/20] test vanquish prints --- .../cpp/src/barretenberg/goblin/goblin.hpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 80bed3188ac8..5aa540b5b23b 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -198,18 +198,18 @@ class Goblin { eccvm_builder = std::make_unique(op_queue); eccvm_composer = std::make_unique(); - info("ECCVM: create_prover"); + // info("ECCVM: create_prover"); auto eccvm_prover = eccvm_composer->create_prover(*eccvm_builder); - info("ECCVM: construct_proof"); + // info("ECCVM: construct_proof"); proof.eccvm_proof = eccvm_prover.construct_proof(); proof.translation_evaluations = eccvm_prover.translation_evaluations; translator_builder = std::make_unique( eccvm_prover.translation_batching_challenge_v, eccvm_prover.evaluation_challenge_x, op_queue); translator_composer = std::make_unique(); - info("Translator: create_prover"); + // info("Translator: create_prover"); auto translator_prover = translator_composer->create_prover(*translator_builder, eccvm_prover.transcript); - info("Translator: construct_proof"); + // info("Translator: construct_proof"); proof.translator_proof = translator_prover.construct_proof(); proof_ = proof; // ACIRHACK @@ -225,18 +225,18 @@ class Goblin { // bool merge_verified = merge_verifier.verify_proof(proof.merge_proof); // info("verified merge proof. result: ", merge_verified); - info("ECCVM: create_verifier"); + // info("ECCVM: create_verifier"); auto eccvm_verifier = eccvm_composer->create_verifier(*eccvm_builder); - info("ECCVM: verify_proof"); + // info("ECCVM: verify_proof"); bool eccvm_verified = eccvm_verifier.verify_proof(proof.eccvm_proof); - info("Translator: create_verifier"); + // info("Translator: create_verifier"); auto translator_verifier = translator_composer->create_verifier(*translator_builder, eccvm_verifier.transcript); - info("Translator: verify_proof"); + // info("Translator: verify_proof"); bool accumulator_construction_verified = translator_verifier.verify_proof(proof.translator_proof); // TODO(https://github.com/AztecProtocol/barretenberg/issues/799): Ensure translation_evaluations are passed // correctly - info("Translator: verify_translation"); + // info("Translator: verify_translation"); bool translation_verified = translator_verifier.verify_translation(proof.translation_evaluations); return /* merge_verified && */ eccvm_verified && accumulator_construction_verified && translation_verified; From 321e07487daf10cd862a0fd51ce8aa295fa03c01 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 00:32:12 +0000 Subject: [PATCH 12/20] test get rid of goblin component --- .../cpp/src/barretenberg/goblin/goblin.hpp | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 5aa540b5b23b..b4068111b953 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -248,14 +248,16 @@ class Goblin { // info("goblin: construct_proof"); accumulate_for_acir(builder); // info("accumulate complete."); - std::vector goblin_proof = prove_for_acir().to_buffer(); - std::vector result(accumulator.proof.proof_data.size() + goblin_proof.size()); - - const auto insert = [&result](const std::vector& buf) { - result.insert(result.end(), buf.begin(), buf.end()); - }; - insert(accumulator.proof.proof_data); - insert(goblin_proof); + // std::vector goblin_proof = prove_for_acir().to_buffer(); + // std::vector result(accumulator.proof.proof_data.size() + goblin_proof.size()); + + // const auto insert = [&result](const std::vector& buf) { + // result.insert(result.end(), buf.begin(), buf.end()); + // }; + // insert(accumulator.proof.proof_data); + // insert(goblin_proof); + // return result; + std::vector result; return result; } @@ -274,12 +276,13 @@ class Goblin { info("GUH verification FAILED"); } - const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; - auto goblin_proof = extract_goblin_proof(proof); - // info("extracted goblin proof"); - verified = verified && verify_for_acir(goblin_proof); - // info("verified goblin proof"); - return verified; + // const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; + // auto goblin_proof = extract_goblin_proof(proof); + // // info("extracted goblin proof"); + // verified = verified && verify_for_acir(goblin_proof); + // // info("verified goblin proof"); + // return verified; + return true; } }; } // namespace barretenberg \ No newline at end of file From 5514cef50568f17a5e916bdaeb560f85999fc988 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 18:44:03 +0000 Subject: [PATCH 13/20] bring back gob --- .../cpp/src/barretenberg/goblin/goblin.hpp | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index b4068111b953..8e743d2bc63a 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -248,17 +248,18 @@ class Goblin { // info("goblin: construct_proof"); accumulate_for_acir(builder); // info("accumulate complete."); - // std::vector goblin_proof = prove_for_acir().to_buffer(); - // std::vector result(accumulator.proof.proof_data.size() + goblin_proof.size()); - - // const auto insert = [&result](const std::vector& buf) { - // result.insert(result.end(), buf.begin(), buf.end()); - // }; - // insert(accumulator.proof.proof_data); - // insert(goblin_proof); - // return result; - std::vector result; + std::vector goblin_proof = prove_for_acir().to_buffer(); + std::vector result(accumulator.proof.proof_data.size() + goblin_proof.size()); + + const auto insert = [&result](const std::vector& buf) { + result.insert(result.end(), buf.begin(), buf.end()); + }; + insert(accumulator.proof.proof_data); + insert(goblin_proof); return result; + + // std::vector result; + // return result; } // ACIRHACK @@ -276,13 +277,13 @@ class Goblin { info("GUH verification FAILED"); } - // const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; - // auto goblin_proof = extract_goblin_proof(proof); - // // info("extracted goblin proof"); - // verified = verified && verify_for_acir(goblin_proof); - // // info("verified goblin proof"); - // return verified; - return true; + const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; + auto goblin_proof = extract_goblin_proof(proof); + // info("extracted goblin proof"); + verified = verified && verify_for_acir(goblin_proof); + // info("verified goblin proof"); + return verified; + // return true; } }; } // namespace barretenberg \ No newline at end of file From 4f559fc932e712706ab155a6a3b5d44c925b34ac Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 19:08:11 +0000 Subject: [PATCH 14/20] try to repro failure in isolation --- barretenberg/acir_tests/Dockerfile.bb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/acir_tests/Dockerfile.bb b/barretenberg/acir_tests/Dockerfile.bb index a20d3d53280b..656cfb5874a7 100644 --- a/barretenberg/acir_tests/Dockerfile.bb +++ b/barretenberg/acir_tests/Dockerfile.bb @@ -11,6 +11,6 @@ COPY . . # This ensures we test independent pk construction through real/garbage witness data paths. RUN FLOW=prove_then_verify ./run_acir_tests.sh # TODO(https://github.com/AztecProtocol/barretenberg/issues/811) make this able to run the default test -RUN FLOW=prove_and_verify_goblin ./run_acir_tests.sh +RUN FLOW=prove_and_verify_goblin ./run_acir_tests.sh brillig_hash_to_field # Run 1_mul through native bb build, all_cmds flow, to test all cli args. RUN VERBOSE=1 FLOW=all_cmds ./run_acir_tests.sh 1_mul From 77170fd4d73a03272ee41931ba261c8c656c94e9 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 19:43:59 +0000 Subject: [PATCH 15/20] remove goblin and all prints --- .../cpp/src/barretenberg/goblin/goblin.hpp | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 8e743d2bc63a..a66577355d79 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -245,21 +245,22 @@ class Goblin { // ACIRHACK std::vector construct_proof(GoblinUltraCircuitBuilder& builder) { - // info("goblin: construct_proof"); + // Construct a GUH proof accumulate_for_acir(builder); - // info("accumulate complete."); - std::vector goblin_proof = prove_for_acir().to_buffer(); - std::vector result(accumulator.proof.proof_data.size() + goblin_proof.size()); + + std::vector result(accumulator.proof.proof_data.size()); const auto insert = [&result](const std::vector& buf) { result.insert(result.end(), buf.begin(), buf.end()); }; + insert(accumulator.proof.proof_data); - insert(goblin_proof); - return result; - // std::vector result; - // return result; + // WORKTODO: + // std::vector goblin_proof = prove_for_acir().to_buffer(); + // insert(goblin_proof); + + return result; } // ACIRHACK @@ -269,21 +270,19 @@ class Goblin { const auto extract_final_kernel_proof = [&]([[maybe_unused]] auto& input_proof) { return accumulator.proof; }; GoblinUltraVerifier verifier{ accumulator.verification_key }; - // info("constructed GUH verifier"); bool verified = verifier.verify_proof(extract_final_kernel_proof(proof)); - if (verified) { - info("GUH verification SUCCEEDED"); - } else { - info("GUH verification FAILED"); - } + // if (verified) { + // info("GUH verification SUCCEEDED"); + // } else { + // info("GUH verification FAILED"); + // } + + // WORKTODO + // const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; + // auto goblin_proof = extract_goblin_proof(proof); + // verified = verified && verify_for_acir(goblin_proof); - const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; - auto goblin_proof = extract_goblin_proof(proof); - // info("extracted goblin proof"); - verified = verified && verify_for_acir(goblin_proof); - // info("verified goblin proof"); return verified; - // return true; } }; } // namespace barretenberg \ No newline at end of file From 7ec3a4eebf70a49f452a5473ff7a65a228c672cc Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 19:46:43 +0000 Subject: [PATCH 16/20] run full test suite on CI --- barretenberg/acir_tests/Dockerfile.bb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/acir_tests/Dockerfile.bb b/barretenberg/acir_tests/Dockerfile.bb index 656cfb5874a7..a20d3d53280b 100644 --- a/barretenberg/acir_tests/Dockerfile.bb +++ b/barretenberg/acir_tests/Dockerfile.bb @@ -11,6 +11,6 @@ COPY . . # This ensures we test independent pk construction through real/garbage witness data paths. RUN FLOW=prove_then_verify ./run_acir_tests.sh # TODO(https://github.com/AztecProtocol/barretenberg/issues/811) make this able to run the default test -RUN FLOW=prove_and_verify_goblin ./run_acir_tests.sh brillig_hash_to_field +RUN FLOW=prove_and_verify_goblin ./run_acir_tests.sh # Run 1_mul through native bb build, all_cmds flow, to test all cli args. RUN VERBOSE=1 FLOW=all_cmds ./run_acir_tests.sh 1_mul From 90de8ee6a634971682ef6fb8d92144e967c5207a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 20:04:00 +0000 Subject: [PATCH 17/20] remove some commented out prints --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 8 ++----- .../commitment_schemes/commitment_key.hpp | 6 ------ .../cpp/src/barretenberg/goblin/goblin.hpp | 21 ------------------- 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 510bf11f74e2..62637ec205b1 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -52,7 +52,8 @@ acir_proofs::AcirComposer init(acir_format::acir_format& constraint_system) void init_reference_strings() { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode subgroup size + // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode subgroup size. Currently set to + // max circuit size present in acir tests suite. size_t subgroup_size = 262144; // TODO(https://github.com/AztecProtocol/barretenberg/issues/811) reduce duplication with above @@ -145,23 +146,18 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& witnessPath, [[maybe_unused]] bool recursive) { - // info("Construct constraint_system and witness."); auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); init_reference_strings(); - // info("Construct goblin circuit from constraint system and witness."); acir_proofs::AcirComposer acir_composer; acir_composer.create_goblin_circuit(constraint_system, witness); - // info("Construct goblin proof."); auto proof = acir_composer.create_goblin_proof(); - // info("verify_goblin_proof."); auto verified = acir_composer.verify_goblin_proof(proof); - // vinfo("verified: ", verified); return verified; } diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index fdd2ee406342..394ff93fdbef 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -69,12 +69,6 @@ template class CommitmentKey { Commitment commit(std::span polynomial) { const size_t degree = polynomial.size(); - if (degree > srs->get_monomial_size()) { - info("CommitmentKey:"); - info("degree = ", degree); - info("srs->get_monomial_size() = ", srs->get_monomial_size()); - } - ASSERT(degree <= srs->get_monomial_size()); return barretenberg::scalar_multiplication::pippenger_unsafe( const_cast(polynomial.data()), srs->get_monomial_points(), degree, pippenger_runtime_state); diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index a66577355d79..a36a9c75e190 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -169,15 +169,11 @@ class Goblin { auto instance = composer.create_instance(circuit_builder); auto prover = composer.create_prover(instance); auto ultra_proof = prover.construct_proof(); - // instance_inspector::inspect_instance(instance); - // instance_inspector::print_databus_info(instance); // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): no merge prover for now since we're not // mocking the first set of ecc ops // // Construct and store the merge proof to be recursively verified on the next call to accumulate - // info("create_merge_prover"); // auto merge_prover = composer.create_merge_prover(op_queue); - // info("merge_prover.construct_proof()"); // merge_proof = merge_prover.construct_proof(); // if (!merge_proof_exists) { @@ -191,25 +187,20 @@ class Goblin { // ACIRHACK Proof prove_for_acir() { - // info("Goblin.prove(): op_queue size = ", op_queue->ultra_ops[0].size()); Proof proof; proof.merge_proof = std::move(merge_proof); eccvm_builder = std::make_unique(op_queue); eccvm_composer = std::make_unique(); - // info("ECCVM: create_prover"); auto eccvm_prover = eccvm_composer->create_prover(*eccvm_builder); - // info("ECCVM: construct_proof"); proof.eccvm_proof = eccvm_prover.construct_proof(); proof.translation_evaluations = eccvm_prover.translation_evaluations; translator_builder = std::make_unique( eccvm_prover.translation_batching_challenge_v, eccvm_prover.evaluation_challenge_x, op_queue); translator_composer = std::make_unique(); - // info("Translator: create_prover"); auto translator_prover = translator_composer->create_prover(*translator_builder, eccvm_prover.transcript); - // info("Translator: construct_proof"); proof.translator_proof = translator_prover.construct_proof(); proof_ = proof; // ACIRHACK @@ -221,22 +212,15 @@ class Goblin { { // ACIRHACK // MergeVerifier merge_verifier; - // info("constructed merge_verifier"); // bool merge_verified = merge_verifier.verify_proof(proof.merge_proof); - // info("verified merge proof. result: ", merge_verified); - // info("ECCVM: create_verifier"); auto eccvm_verifier = eccvm_composer->create_verifier(*eccvm_builder); - // info("ECCVM: verify_proof"); bool eccvm_verified = eccvm_verifier.verify_proof(proof.eccvm_proof); - // info("Translator: create_verifier"); auto translator_verifier = translator_composer->create_verifier(*translator_builder, eccvm_verifier.transcript); - // info("Translator: verify_proof"); bool accumulator_construction_verified = translator_verifier.verify_proof(proof.translator_proof); // TODO(https://github.com/AztecProtocol/barretenberg/issues/799): Ensure translation_evaluations are passed // correctly - // info("Translator: verify_translation"); bool translation_verified = translator_verifier.verify_translation(proof.translation_evaluations); return /* merge_verified && */ eccvm_verified && accumulator_construction_verified && translation_verified; @@ -271,11 +255,6 @@ class Goblin { GoblinUltraVerifier verifier{ accumulator.verification_key }; bool verified = verifier.verify_proof(extract_final_kernel_proof(proof)); - // if (verified) { - // info("GUH verification SUCCEEDED"); - // } else { - // info("GUH verification FAILED"); - // } // WORKTODO // const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; From d82176eb93d3a2210e361938796478b80be97bdd Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 20:39:42 +0000 Subject: [PATCH 18/20] add issue to TODOs --- barretenberg/cpp/src/barretenberg/goblin/goblin.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index a36a9c75e190..0586364faf7b 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -240,7 +240,7 @@ class Goblin { insert(accumulator.proof.proof_data); - // WORKTODO: + // TODO(https://github.com/AztecProtocol/barretenberg/issues/819): Skip ECCVM/Translator proof for now // std::vector goblin_proof = prove_for_acir().to_buffer(); // insert(goblin_proof); @@ -256,7 +256,7 @@ class Goblin { GoblinUltraVerifier verifier{ accumulator.verification_key }; bool verified = verifier.verify_proof(extract_final_kernel_proof(proof)); - // WORKTODO + // TODO(https://github.com/AztecProtocol/barretenberg/issues/819): Skip ECCVM/Translator verification for now // const auto extract_goblin_proof = [&]([[maybe_unused]] auto& input_proof) { return proof_; }; // auto goblin_proof = extract_goblin_proof(proof); // verified = verified && verify_for_acir(goblin_proof); From 94c060d6457e0b31b10752a3ac5a0ea67961570e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 3 Jan 2024 22:04:36 +0000 Subject: [PATCH 19/20] update js, clean up constructor --- barretenberg/acir_tests/Dockerfile.bb.js | 2 +- .../acir_tests/flows/prove_and_verify_goblin.sh | 5 ++++- .../circuit_builder/goblin_ultra_circuit_builder.hpp | 11 +++++------ barretenberg/ts/src/main.ts | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/barretenberg/acir_tests/Dockerfile.bb.js b/barretenberg/acir_tests/Dockerfile.bb.js index 52d3057ea3a4..c9d877786a02 100644 --- a/barretenberg/acir_tests/Dockerfile.bb.js +++ b/barretenberg/acir_tests/Dockerfile.bb.js @@ -15,7 +15,7 @@ ENV VERBOSE=1 # Run double_verify_proof through bb.js on node to check 512k support. RUN BIN=../ts/dest/node/main.js FLOW=prove_then_verify ./run_acir_tests.sh double_verify_proof # TODO(https://github.com/AztecProtocol/barretenberg/issues/811) make this able to run double_verify_proof -RUN BIN=../ts/dest/node/main.js FLOW=prove_and_verify_goblin ./run_acir_tests.sh assert_statement +RUN BIN=../ts/dest/node/main.js FLOW=prove_and_verify_goblin ./run_acir_tests.sh # Run 1_mul through bb.js build, all_cmds flow, to test all cli args. RUN BIN=../ts/dest/node/main.js FLOW=all_cmds ./run_acir_tests.sh 1_mul # Run double_verify_proof through bb.js on chrome testing multi-threaded browser support. diff --git a/barretenberg/acir_tests/flows/prove_and_verify_goblin.sh b/barretenberg/acir_tests/flows/prove_and_verify_goblin.sh index a5da48d27387..68a003d685b9 100755 --- a/barretenberg/acir_tests/flows/prove_and_verify_goblin.sh +++ b/barretenberg/acir_tests/flows/prove_and_verify_goblin.sh @@ -3,4 +3,7 @@ set -eu VFLAG=${VERBOSE:+-v} -$BIN prove_and_verify_goblin $VFLAG -c $CRS_PATH -b ./target/acir.gz \ No newline at end of file +$BIN prove_and_verify_goblin $VFLAG -c $CRS_PATH -b ./target/acir.gz + +# This command can be used to run all of the tests in sequence with the debugger +# lldb-16 -o run -b -- $BIN prove_and_verify_goblin $VFLAG -c $CRS_PATH -b ./target/acir.gz \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index 3d153f858dc0..f077228effc0 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -98,18 +98,17 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui : UltraCircuitBuilder_>() , op_queue(op_queue_in) { - // Add the variables and public inputs known directly from acir + // Add the witness variables known directly from acir for (size_t idx = 0; idx < varnum; ++idx) { // Zeros are added for variables whose existence is known but whose values are not yet known. The values may // be "set" later on via the assert_equal mechanism. auto value = idx < witness_values.size() ? witness_values[idx] : 0; - if (std::find(public_inputs.begin(), public_inputs.end(), idx) != public_inputs.end()) { - this->add_public_variable(value); - } else { - this->add_variable(value); - } + this->add_variable(value); } + // Add the public_inputs from acir + this->public_inputs = public_inputs; + // Set indices to constants corresponding to Goblin ECC op codes set_goblin_ecc_op_code_constant_variables(); }; diff --git a/barretenberg/ts/src/main.ts b/barretenberg/ts/src/main.ts index 5c708347104d..208c4851a1f9 100755 --- a/barretenberg/ts/src/main.ts +++ b/barretenberg/ts/src/main.ts @@ -73,7 +73,7 @@ async function init(bytecodePath: string, crsPath: string, subgroupSizeOverride async function initGoblin(bytecodePath: string, crsPath: string) { // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): remove this subgroup size hack - const hardcodedGrumpkinSubgroupSizeHack = 32768; + const hardcodedGrumpkinSubgroupSizeHack = 262144; const initData = await init(bytecodePath, crsPath, hardcodedGrumpkinSubgroupSizeHack); const { api } = initData; From 8523bf279e2497d83b1834c32f37193defcfef10 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 4 Jan 2024 15:00:04 +0000 Subject: [PATCH 20/20] respond to kevs comments --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 6 +++--- .../src/barretenberg/dsl/acir_proofs/acir_composer.cpp | 8 ++++---- .../cpp/src/barretenberg/eccvm/eccvm_verifier.cpp | 7 ++++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 62637ec205b1..3cf532f8958d 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -54,16 +54,16 @@ void init_reference_strings() { // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode subgroup size. Currently set to // max circuit size present in acir tests suite. - size_t subgroup_size = 262144; + size_t hardcoded_subgroup_size_hack = 262144; // TODO(https://github.com/AztecProtocol/barretenberg/issues/811) reduce duplication with above // Must +1! - auto g1_data = get_bn254_g1_data(CRS_PATH, subgroup_size + 1); + auto g1_data = get_bn254_g1_data(CRS_PATH, hardcoded_subgroup_size_hack + 1); auto g2_data = get_bn254_g2_data(CRS_PATH); srs::init_crs_factory(g1_data, g2_data); // Must +1! - auto grumpkin_g1_data = get_grumpkin_g1_data(CRS_PATH, subgroup_size + 1); + auto grumpkin_g1_data = get_grumpkin_g1_data(CRS_PATH, hardcoded_subgroup_size_hack + 1); srs::init_grumpkin_crs_factory(grumpkin_g1_data); } 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 b0ce15654aa1..f8098dc4c445 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -83,8 +83,8 @@ void AcirComposer::create_goblin_circuit(acir_format::acir_format& constraint_sy { // The public inputs in constraint_system do not index into "witness" but rather into the future "variables" which // it assumes will be equal to witness but with a prepended zero. We want to remove this +1 so that public_inputs - // properly indexes into witness bc we're about to make calls like add_variable(witness[public_inputs[idx]]). Once - // the +1 is removed from noir, this correction can be removed entirely and we can use + // properly indexes into witness because we're about to make calls like add_variable(witness[public_inputs[idx]]). + // Once the +1 is removed from noir, this correction can be removed entirely and we can use // constraint_system.public_inputs directly. const uint32_t pre_applied_noir_offset = 1; std::vector corrected_public_inputs; @@ -99,8 +99,8 @@ void AcirComposer::create_goblin_circuit(acir_format::acir_format& constraint_sy // Populate constraints in the builder via the data in constraint_system acir_format::build_constraints(goblin_builder_, constraint_system, true); - // HACK: Add some arbitrary op gates to ensure the associated polynomials are non-zero and to give ECCVM and - // Translator some ECC ops to process. + // TODO(https://github.com/AztecProtocol/barretenberg/issues/817): Add some arbitrary op gates to ensure the + // associated polynomials are non-zero and to give ECCVM and Translator some ECC ops to process. GoblinTestingUtils::construct_goblin_ecc_op_circuit(goblin_builder_); } diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp index 32e28e62a7d8..1d564c06a9fd 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_verifier.cpp @@ -195,11 +195,12 @@ template bool ECCVMVerifier_::verify_proof(const plonk // Construct batched commitment for NON-shifted polynomials size_t commitment_idx = 0; for (auto& commitment : commitments.get_unshifted()) { - // TODO(@zac-williamson) ensure ECCVM polynomial commitments are never points at infinity (#2214) + // TODO(@zac-williamson)(https://github.com/AztecProtocol/barretenberg/issues/820) ensure ECCVM polynomial + // commitments are never points at infinity if (commitment.y != 0) { batched_commitment_unshifted += commitment * rhos[commitment_idx]; } else { - // info("ECCVM Verifier: point at infinity (unshifted)"); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/820) } ++commitment_idx; } @@ -210,7 +211,7 @@ template bool ECCVMVerifier_::verify_proof(const plonk if (commitment.y != 0) { batched_commitment_to_be_shifted += commitment * rhos[commitment_idx]; } else { - // info("ECCVM Verifier: point at infinity (to be shifted)"); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/820) } ++commitment_idx; }