From 2fc7f28fabb59a85e7bb665021baf734a563064c Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 00:41:53 +0000 Subject: [PATCH 01/11] simplify create circuit with witness --- .../barretenberg/dsl/acir_format/acir_format.cpp | 14 +------------- .../barretenberg/dsl/acir_format/acir_format.hpp | 3 --- .../barretenberg/dsl/acir_proofs/acir_composer.cpp | 4 ++-- 3 files changed, 3 insertions(+), 18 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 428af52015f..ce8b3a7198a 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -257,13 +257,6 @@ Builder create_circuit_with_witness(acir_format const& constraint_system, size_t size_hint) { Builder builder(size_hint); - create_circuit_with_witness(builder, constraint_system, witness); - return builder; -} - -template -void create_circuit_with_witness(Builder& builder, acir_format const& constraint_system, WitnessVector const& witness) -{ if (constraint_system.public_inputs.size() > constraint_system.varnum) { info("create_circuit_with_witness: too many public inputs!"); } @@ -272,16 +265,11 @@ void create_circuit_with_witness(Builder& builder, acir_format const& constraint populate_variables_and_public_inputs(builder, witness, constraint_system); build_constraints(builder, constraint_system, true); + return builder; } template UltraCircuitBuilder create_circuit(const acir_format& constraint_system, size_t size_hint); -template void create_circuit_with_witness(UltraCircuitBuilder& builder, - acir_format const& constraint_system, - WitnessVector const& witness); -template void create_circuit_with_witness(GoblinUltraCircuitBuilder& builder, - acir_format const& constraint_system, - WitnessVector const& witness); 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 2860e810818..e426b7b3f42 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp @@ -83,9 +83,6 @@ Builder create_circuit_with_witness(const acir_format& constraint_system, WitnessVector const& witness, size_t size_hint = 0); -template -void create_circuit_with_witness(Builder& builder, const acir_format& constraint_system, WitnessVector const& witness); - template void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments); 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 f23cb2a77a8..c97a968ea64 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -50,8 +50,8 @@ std::vector AcirComposer::create_proof(acir_format::acir_format& constr bool is_recursive) { vinfo("building circuit with witness..."); - builder_ = acir_format::Builder(size_hint_); - create_circuit_with_witness(builder_, constraint_system, witness); + builder_ = create_circuit_with_witness(constraint_system, witness, size_hint_); + vinfo("gates: ", builder_.get_total_circuit_size()); auto composer = [&]() { From 4d2565b19b6d1c54dc22db6cb88cd756b118dcfe Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 05:05:28 +0000 Subject: [PATCH 02/11] delete and simplify --- .../dsl/acir_format/acir_format.cpp | 51 +++---------------- .../dsl/acir_format/acir_format.hpp | 4 -- 2 files changed, 7 insertions(+), 48 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 ce8b3a7198a..f370ae6e0f4 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -21,37 +21,11 @@ void populate_variables_and_public_inputs(Builder& builder, WitnessVector const& witness, acir_format const& constraint_system) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Decrement the indices in - // constraint_system.public_inputs by one to account for the +1 added by default to account for a const zero - // variable in noir. This entire block can be removed once the +1 is removed from noir. - 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); - } - for (size_t idx = 0; idx < constraint_system.varnum; ++idx) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/815) why is this needed? fr value = idx < witness.size() ? witness[idx] : 0; - if (std::find(corrected_public_inputs.begin(), corrected_public_inputs.end(), idx) != - corrected_public_inputs.end()) { - builder.add_public_variable(value); - } else { - builder.add_variable(value); - } - } -} - -template void read_witness(Builder& builder, WitnessVector const& witness) -{ - builder.variables[0] = - 0; // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): This the constant 0 hacked in. Bad. - for (size_t i = 0; i < witness.size(); ++i) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): The i+1 accounts for the fact that 0 is added - // as a constant in variables in the UCB constructor. "witness" only contains the values that came directly from - // acir. - builder.variables[i + 1] = witness[i]; + builder.add_variable(value); } + builder.public_inputs = constraint_system.public_inputs; } // TODO(https://github.com/AztecProtocol/barretenberg/issues/815): This function does two things: 1) emplaces back @@ -62,17 +36,9 @@ template void add_public_vars(Builder& builder, acir_format c { // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): i = 1 acounting for const 0 in first position? 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); - } + builder.add_variable(0); } + builder.public_inputs = constraint_system.public_inputs; } template @@ -235,20 +201,17 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b } } -template void create_circuit(Builder& builder, acir_format const& constraint_system) +template Builder create_circuit(const acir_format& constraint_system, size_t size_hint) { + Builder builder(size_hint); + 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); -} -template Builder create_circuit(const acir_format& constraint_system, size_t size_hint) -{ - Builder builder(size_hint); - create_circuit(builder, constraint_system); return builder; } 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 e426b7b3f42..293750a931c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp @@ -72,10 +72,6 @@ struct acir_format { using WitnessVector = std::vector>; -template void read_witness(Builder& builder, std::vector const& witness); - -template void create_circuit(Builder& builder, const acir_format& constraint_system); - template Builder create_circuit(const acir_format& constraint_system, size_t size_hint = 0); From 6a1e10cb6ed77b18e0032186d57da2ba1d68bae2 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 14:30:45 +0000 Subject: [PATCH 03/11] consolidate pop vars and pi --- .../dsl/acir_format/acir_format.cpp | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 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 f370ae6e0f4..2c7b58f43b1 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -18,9 +18,13 @@ namespace acir_format { */ template void populate_variables_and_public_inputs(Builder& builder, - WitnessVector const& witness, - acir_format const& constraint_system) + acir_format const& constraint_system, + WitnessVector const& witness = {}) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/815): Since we're using add_var, the pre-offset wire + // indices will be correct. Also, were using the pre-offset public_inputs (not the corrected) to set + // builder.public_inputs. TO remove the pre-offset in noir, we'll need to instead construct a builder directly from + // this data as we now do with GUH. for (size_t idx = 0; idx < constraint_system.varnum; ++idx) { fr value = idx < witness.size() ? witness[idx] : 0; builder.add_variable(value); @@ -28,19 +32,6 @@ void populate_variables_and_public_inputs(Builder& builder, builder.public_inputs = constraint_system.public_inputs; } -// TODO(https://github.com/AztecProtocol/barretenberg/issues/815): This function does two things: 1) emplaces back -// varnum-many 0s into builder.variables (.. why), and (2) populates builder.public_inputs with the correct indices into -// the variables vector (which at this stage will be populated with zeros). The actual entries of the variables vector -// are populated in "read_witness" -template void add_public_vars(Builder& builder, acir_format const& constraint_system) -{ - // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): i = 1 acounting for const 0 in first position? - for (size_t i = 1; i < constraint_system.varnum; ++i) { - builder.add_variable(0); - } - builder.public_inputs = constraint_system.public_inputs; -} - template void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments) { @@ -209,7 +200,7 @@ template Builder create_circuit(const acir_format& constraint info("create_circuit: too many public inputs!"); } - add_public_vars(builder, constraint_system); + populate_variables_and_public_inputs(builder, constraint_system); build_constraints(builder, constraint_system, false); return builder; @@ -225,7 +216,7 @@ Builder create_circuit_with_witness(acir_format const& constraint_system, } // Populate builder.variables and buider.public_inputs - populate_variables_and_public_inputs(builder, witness, constraint_system); + populate_variables_and_public_inputs(builder, constraint_system, witness); build_constraints(builder, constraint_system, true); return builder; From 8927f2062e676ed0911dd1b06aea9402c90aa5ea Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 15:23:55 +0000 Subject: [PATCH 04/11] consolidate create circuit --- .../acir_tests/flows/prove_and_verify.sh | 3 +- .../dsl/acir_format/acir_format.cpp | 35 +++++++++---------- .../dsl/acir_format/acir_format.hpp | 6 +--- .../dsl/acir_format/acir_format.test.cpp | 23 ++++++------ .../dsl/acir_format/block_constraint.test.cpp | 2 +- .../dsl/acir_format/ecdsa_secp256k1.test.cpp | 4 +-- .../dsl/acir_format/ecdsa_secp256r1.test.cpp | 6 ++-- .../acir_format/recursion_constraint.test.cpp | 15 +++----- .../dsl/acir_proofs/acir_composer.cpp | 2 +- 9 files changed, 41 insertions(+), 55 deletions(-) diff --git a/barretenberg/acir_tests/flows/prove_and_verify.sh b/barretenberg/acir_tests/flows/prove_and_verify.sh index 091a6d57946..c34194d3d77 100755 --- a/barretenberg/acir_tests/flows/prove_and_verify.sh +++ b/barretenberg/acir_tests/flows/prove_and_verify.sh @@ -5,4 +5,5 @@ VFLAG=${VERBOSE:+-v} # This is the fastest flow, because it only generates pk/vk once, gate count once, etc. # It may not catch all class of bugs. -$BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz \ No newline at end of file +$BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz +# lldb-16 -- $BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz \ No newline at end of file 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 2c7b58f43b1..699e57bb8a1 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -192,7 +192,17 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b } } -template Builder create_circuit(const acir_format& constraint_system, size_t size_hint) +/** + * @brief Create a circuit from acir constraints and optionally a witness + * + * @tparam Builder + * @param constraint_system + * @param size_hint + * @param witness + * @return Builder + */ +template +Builder create_circuit(const acir_format& constraint_system, size_t size_hint, WitnessVector const& witness) { Builder builder(size_hint); @@ -200,30 +210,17 @@ template Builder create_circuit(const acir_format& constraint info("create_circuit: too many public inputs!"); } - populate_variables_and_public_inputs(builder, constraint_system); - build_constraints(builder, constraint_system, false); - - return builder; -} - -Builder create_circuit_with_witness(acir_format const& constraint_system, - WitnessVector const& witness, - size_t size_hint) -{ - Builder builder(size_hint); - if (constraint_system.public_inputs.size() > constraint_system.varnum) { - info("create_circuit_with_witness: too many public inputs!"); - } - - // Populate builder.variables and buider.public_inputs populate_variables_and_public_inputs(builder, constraint_system, witness); - build_constraints(builder, constraint_system, true); + bool has_valid_witness_assignments = !witness.empty(); + build_constraints(builder, constraint_system, has_valid_witness_assignments); + return builder; } template UltraCircuitBuilder create_circuit(const acir_format& constraint_system, - size_t size_hint); + size_t size_hint, + WitnessVector const& witness); 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 293750a931c..d39e17e093f 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp @@ -73,11 +73,7 @@ struct acir_format { using WitnessVector = std::vector>; template -Builder create_circuit(const acir_format& constraint_system, size_t size_hint = 0); - -Builder create_circuit_with_witness(const acir_format& constraint_system, - WitnessVector const& witness, - size_t size_hint = 0); +Builder create_circuit(const acir_format& constraint_system, size_t size_hint = 0, WitnessVector const& witness = {}); template void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp index b0711582deb..4c21313fbc3 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp @@ -48,7 +48,8 @@ TEST_F(AcirFormatTests, TestASingleConstraintNoPubInputs) .block_constraints = {}, }; - auto builder = create_circuit_with_witness(constraint_system, { 0, 0, 1 }); + WitnessVector witness{ 0, 0, 1 }; + auto builder = create_circuit(constraint_system, 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -155,15 +156,10 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) .block_constraints = {} }; uint256_t inverse_of_five = fr(5).invert(); - auto builder = create_circuit_with_witness(constraint_system, - { - 5, - 10, - 15, - 5, - inverse_of_five, - 1, - }); + WitnessVector witness{ + 5, 10, 15, 5, inverse_of_five, 1, + }; + auto builder = create_circuit(constraint_system, 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -250,7 +246,7 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass) witness[i] = message_string[i]; } - auto builder = create_circuit_with_witness(constraint_system, witness); + auto builder = create_circuit(constraint_system, 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -340,7 +336,7 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange) } // TODO: actually sign a schnorr signature! - auto builder = create_circuit_with_witness(constraint_system, witness); + auto builder = create_circuit(constraint_system, 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -415,7 +411,8 @@ TEST_F(AcirFormatTests, TestVarKeccak) .block_constraints = {}, }; - auto builder = create_circuit_with_witness(constraint_system, { 4, 2, 6, 2 }); + WitnessVector witness{ 4, 2, 6, 2 }; + auto builder = create_circuit(constraint_system, 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp index 37c7aa784f2..fe16365ea65 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp @@ -129,7 +129,7 @@ TEST_F(UltraPlonkRAM, TestBlockConstraint) .block_constraints = { block }, }; - auto builder = create_circuit_with_witness(constraint_system, witness_values); + auto builder = create_circuit(constraint_system, 0, witness_values); auto composer = Composer(); auto prover = composer.create_prover(builder); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp index 57576bd763f..8bc19c2b64e 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp @@ -108,7 +108,7 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintSucceed) .block_constraints = {}, }; - auto builder = create_circuit_with_witness(constraint_system, witness_values); + auto builder = create_circuit(constraint_system, 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_k1_constraint.result), 1); @@ -185,7 +185,7 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintFail) .block_constraints = {}, }; - auto builder = create_circuit_with_witness(constraint_system, witness_values); + auto builder = create_circuit(constraint_system, 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_k1_constraint.result), 0); auto composer = Composer(); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp index 0f8764c3bc8..d787162a66a 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp @@ -147,7 +147,7 @@ TEST(ECDSASecp256r1, test_hardcoded) message, pub_key, signature); EXPECT_EQ(we_ballin, true); - auto builder = create_circuit_with_witness(constraint_system, witness_values); + auto builder = create_circuit(constraint_system, 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_r1_constraint.result), 1); auto composer = Composer(); @@ -184,7 +184,7 @@ TEST(ECDSASecp256r1, TestECDSAConstraintSucceed) .block_constraints = {}, }; - auto builder = create_circuit_with_witness(constraint_system, witness_values); + auto builder = create_circuit(constraint_system, 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_r1_constraint.result), 1); auto composer = Composer(); @@ -259,7 +259,7 @@ TEST(ECDSASecp256r1, TestECDSAConstraintFail) .block_constraints = {}, }; - auto builder = create_circuit_with_witness(constraint_system, witness_values); + auto builder = create_circuit(constraint_system, 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_r1_constraint.result), 0); auto composer = Composer(); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp index a64f65462bc..7bc1438f1e2 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp @@ -101,15 +101,10 @@ Builder create_inner_circuit() .block_constraints = {} }; uint256_t inverse_of_five = fr(5).invert(); - auto builder = create_circuit_with_witness(constraint_system, - { - 5, - 10, - 15, - 5, - inverse_of_five, - 1, - }); + WitnessVector witness{ + 5, 10, 15, 5, inverse_of_five, 1, + }; + auto builder = create_circuit(constraint_system, 0, witness); return builder; } @@ -258,7 +253,7 @@ Builder create_outer_circuit(std::vector& inner_circuits) .constraints = {}, .block_constraints = {} }; - auto outer_circuit = create_circuit_with_witness(constraint_system, witness); + auto outer_circuit = create_circuit(constraint_system, 0, witness); return outer_circuit; } 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 c97a968ea64..f90d6095b97 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -50,7 +50,7 @@ std::vector AcirComposer::create_proof(acir_format::acir_format& constr bool is_recursive) { vinfo("building circuit with witness..."); - builder_ = create_circuit_with_witness(constraint_system, witness, size_hint_); + builder_ = acir_format::create_circuit(constraint_system, size_hint_, witness); vinfo("gates: ", builder_.get_total_circuit_size()); From 433c676950aae3f722baef71370d7458b0f2825e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 16:28:04 +0000 Subject: [PATCH 05/11] mv ftcn --- .../dsl/acir_format/acir_format.cpp | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 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 699e57bb8a1..f005d156019 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -7,31 +7,6 @@ namespace acir_format { -/** - * @brief Populate variables and public_inputs in builder given witness and constraint_system - * @details This method replaces consecutive calls to add_public_vars then read_witness. - * - * @tparam Builder - * @param builder - * @param witness - * @param constraint_system - */ -template -void populate_variables_and_public_inputs(Builder& builder, - acir_format const& constraint_system, - WitnessVector const& witness = {}) -{ - // TODO(https://github.com/AztecProtocol/barretenberg/issues/815): Since we're using add_var, the pre-offset wire - // indices will be correct. Also, were using the pre-offset public_inputs (not the corrected) to set - // builder.public_inputs. TO remove the pre-offset in noir, we'll need to instead construct a builder directly from - // this data as we now do with GUH. - for (size_t idx = 0; idx < constraint_system.varnum; ++idx) { - fr value = idx < witness.size() ? witness[idx] : 0; - builder.add_variable(value); - } - builder.public_inputs = constraint_system.public_inputs; -} - template void build_constraints(Builder& builder, acir_format const& constraint_system, bool has_valid_witness_assignments) { @@ -192,6 +167,31 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b } } +/** + * @brief Populate variables and public_inputs in builder given witness and constraint_system + * @details This method replaces consecutive calls to add_public_vars then read_witness. + * + * @tparam Builder + * @param builder + * @param witness + * @param constraint_system + */ +template +void populate_variables_and_public_inputs(Builder& builder, + acir_format const& constraint_system, + WitnessVector const& witness = {}) +{ + // TODO(https://github.com/AztecProtocol/barretenberg/issues/815): Since we're using add_var, the pre-offset wire + // indices will be correct. Also, were using the pre-offset public_inputs (not the corrected) to set + // builder.public_inputs. TO remove the pre-offset in noir, we'll need to instead construct a builder directly from + // this data as we now do with GUH. + for (size_t idx = 0; idx < constraint_system.varnum; ++idx) { + fr value = idx < witness.size() ? witness[idx] : 0; + builder.add_variable(value); + } + builder.public_inputs = constraint_system.public_inputs; +} + /** * @brief Create a circuit from acir constraints and optionally a witness * From 41a3724c6ebb9f924aa81ee3b6ec6325fe9a85ca Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 8 Jan 2024 17:58:05 +0000 Subject: [PATCH 06/11] use new UCB constructor in create circuit --- .../dsl/acir_format/acir_format.cpp | 8 +--- .../goblin_ultra_circuit_builder.hpp | 3 ++ .../circuit_builder/ultra_circuit_builder.hpp | 37 +++++++++++++++++++ noir/test_programs/.gitignore | 1 + 4 files changed, 42 insertions(+), 7 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 f005d156019..161234aa591 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -204,13 +204,7 @@ void populate_variables_and_public_inputs(Builder& builder, template Builder create_circuit(const acir_format& constraint_system, size_t size_hint, WitnessVector const& witness) { - Builder builder(size_hint); - - if (constraint_system.public_inputs.size() > constraint_system.varnum) { - info("create_circuit: too many public inputs!"); - } - - populate_variables_and_public_inputs(builder, constraint_system, witness); + Builder builder{ size_hint, witness, constraint_system.public_inputs, constraint_system.varnum }; bool has_valid_witness_assignments = !witness.empty(); build_constraints(builder, constraint_system, 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 f077228effc..d7de442b0b8 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,6 +98,9 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui : UltraCircuitBuilder_>() , op_queue(op_queue_in) { + // WORKTODO: NOTE: This still works even though the witness indices in the explicit acir gates are +1 offset + // because we're still adding the const 0 before anything else via the UCB constructor. Once we remove the +1 + // from Noir, we'll need to update the UCB by moving that const 0 to get these tests to pass again. // 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 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 14c7f9cc2b9..677fd6a6269 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 @@ -656,6 +656,43 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasezero_idx = put_constant_variable(FF::zero()); this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this }; + /** + * @brief Constructor from data generated from ACIR + * + * @param size_hint + * @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). + */ + UltraCircuitBuilder_(const size_t size_hint, + auto& witness_values, + const std::vector& public_inputs, + size_t varnum) + : CircuitBuilderBase(size_hint) + { + selectors.reserve(size_hint); + w_l().reserve(size_hint); + w_r().reserve(size_hint); + w_o().reserve(size_hint); + w_4().reserve(size_hint); + this->zero_idx = put_constant_variable(FF::zero()); + this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this + 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; + this->add_variable(value); + } + + // Add the public_inputs from acir + this->public_inputs = public_inputs; + }; UltraCircuitBuilder_(const UltraCircuitBuilder_& other) = default; UltraCircuitBuilder_(UltraCircuitBuilder_&& other) : CircuitBuilderBase(std::move(other)) diff --git a/noir/test_programs/.gitignore b/noir/test_programs/.gitignore index 01a3426160c..a229df6197f 100644 --- a/noir/test_programs/.gitignore +++ b/noir/test_programs/.gitignore @@ -1 +1,2 @@ acir_artifacts +execution_success/**/crs \ No newline at end of file From e9070ae2aef97d20b252e74dbf8dfdbf3b1794c3 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 8 Jan 2024 18:16:48 +0000 Subject: [PATCH 07/11] delete no longer needed fctn --- .../dsl/acir_format/acir_format.cpp | 25 ------------------- 1 file changed, 25 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 161234aa591..16258d916b9 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -167,31 +167,6 @@ void build_constraints(Builder& builder, acir_format const& constraint_system, b } } -/** - * @brief Populate variables and public_inputs in builder given witness and constraint_system - * @details This method replaces consecutive calls to add_public_vars then read_witness. - * - * @tparam Builder - * @param builder - * @param witness - * @param constraint_system - */ -template -void populate_variables_and_public_inputs(Builder& builder, - acir_format const& constraint_system, - WitnessVector const& witness = {}) -{ - // TODO(https://github.com/AztecProtocol/barretenberg/issues/815): Since we're using add_var, the pre-offset wire - // indices will be correct. Also, were using the pre-offset public_inputs (not the corrected) to set - // builder.public_inputs. TO remove the pre-offset in noir, we'll need to instead construct a builder directly from - // this data as we now do with GUH. - for (size_t idx = 0; idx < constraint_system.varnum; ++idx) { - fr value = idx < witness.size() ? witness[idx] : 0; - builder.add_variable(value); - } - builder.public_inputs = constraint_system.public_inputs; -} - /** * @brief Create a circuit from acir constraints and optionally a witness * From 1fda3228ad0843b66f535729cb972d5282cf52e7 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 8 Jan 2024 19:07:59 +0000 Subject: [PATCH 08/11] comments --- barretenberg/acir_tests/flows/prove_and_verify.sh | 3 +-- .../barretenberg/dsl/acir_format/acir_format.test.cpp | 10 +++++----- .../dsl/acir_format/block_constraint.test.cpp | 2 +- .../dsl/acir_format/ecdsa_secp256k1.test.cpp | 4 ++-- .../dsl/acir_format/ecdsa_secp256r1.test.cpp | 6 +++--- .../dsl/acir_format/recursion_constraint.test.cpp | 4 ++-- .../src/barretenberg/dsl/acir_proofs/acir_composer.cpp | 10 +++++----- .../circuit_builder/goblin_ultra_circuit_builder.hpp | 8 ++++---- .../circuit_builder/ultra_circuit_builder.hpp | 3 +++ 9 files changed, 26 insertions(+), 24 deletions(-) diff --git a/barretenberg/acir_tests/flows/prove_and_verify.sh b/barretenberg/acir_tests/flows/prove_and_verify.sh index c34194d3d77..091a6d57946 100755 --- a/barretenberg/acir_tests/flows/prove_and_verify.sh +++ b/barretenberg/acir_tests/flows/prove_and_verify.sh @@ -5,5 +5,4 @@ VFLAG=${VERBOSE:+-v} # This is the fastest flow, because it only generates pk/vk once, gate count once, etc. # It may not catch all class of bugs. -$BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz -# lldb-16 -- $BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz \ No newline at end of file +$BIN prove_and_verify $VFLAG -c $CRS_PATH -b ./target/acir.gz \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp index 4c21313fbc3..a3e87e1c93b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp @@ -49,7 +49,7 @@ TEST_F(AcirFormatTests, TestASingleConstraintNoPubInputs) }; WitnessVector witness{ 0, 0, 1 }; - auto builder = create_circuit(constraint_system, 0, witness); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -159,7 +159,7 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) WitnessVector witness{ 5, 10, 15, 5, inverse_of_five, 1, }; - auto builder = create_circuit(constraint_system, 0, witness); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -246,7 +246,7 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass) witness[i] = message_string[i]; } - auto builder = create_circuit(constraint_system, 0, witness); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -336,7 +336,7 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange) } // TODO: actually sign a schnorr signature! - auto builder = create_circuit(constraint_system, 0, witness); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); @@ -412,7 +412,7 @@ TEST_F(AcirFormatTests, TestVarKeccak) }; WitnessVector witness{ 4, 2, 6, 2 }; - auto builder = create_circuit(constraint_system, 0, witness); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp index fe16365ea65..54793c2bc81 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp @@ -129,7 +129,7 @@ TEST_F(UltraPlonkRAM, TestBlockConstraint) .block_constraints = { block }, }; - auto builder = create_circuit(constraint_system, 0, witness_values); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); auto composer = Composer(); auto prover = composer.create_prover(builder); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp index 8bc19c2b64e..d0ecf46a6d2 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp @@ -108,7 +108,7 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintSucceed) .block_constraints = {}, }; - auto builder = create_circuit(constraint_system, 0, witness_values); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_k1_constraint.result), 1); @@ -185,7 +185,7 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintFail) .block_constraints = {}, }; - auto builder = create_circuit(constraint_system, 0, witness_values); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_k1_constraint.result), 0); auto composer = Composer(); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp index d787162a66a..0987bf4054b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp @@ -147,7 +147,7 @@ TEST(ECDSASecp256r1, test_hardcoded) message, pub_key, signature); EXPECT_EQ(we_ballin, true); - auto builder = create_circuit(constraint_system, 0, witness_values); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_r1_constraint.result), 1); auto composer = Composer(); @@ -184,7 +184,7 @@ TEST(ECDSASecp256r1, TestECDSAConstraintSucceed) .block_constraints = {}, }; - auto builder = create_circuit(constraint_system, 0, witness_values); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_r1_constraint.result), 1); auto composer = Composer(); @@ -259,7 +259,7 @@ TEST(ECDSASecp256r1, TestECDSAConstraintFail) .block_constraints = {}, }; - auto builder = create_circuit(constraint_system, 0, witness_values); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); EXPECT_EQ(builder.get_variable(ecdsa_r1_constraint.result), 0); auto composer = Composer(); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp index 7bc1438f1e2..107e64a9c12 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.test.cpp @@ -104,7 +104,7 @@ Builder create_inner_circuit() WitnessVector witness{ 5, 10, 15, 5, inverse_of_five, 1, }; - auto builder = create_circuit(constraint_system, 0, witness); + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness); return builder; } @@ -253,7 +253,7 @@ Builder create_outer_circuit(std::vector& inner_circuits) .constraints = {}, .block_constraints = {} }; - auto outer_circuit = create_circuit(constraint_system, 0, witness); + auto outer_circuit = create_circuit(constraint_system, /*size_hint*/ 0, witness); return outer_circuit; } 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 f90d6095b97..5eb044f1491 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -82,11 +82,11 @@ 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) { - // 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 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. + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): 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 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; for (const auto& index : constraint_system.public_inputs) { 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 d7de442b0b8..7502c7aec07 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,10 +98,10 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui : UltraCircuitBuilder_>() , op_queue(op_queue_in) { - // WORKTODO: NOTE: This still works even though the witness indices in the explicit acir gates are +1 offset - // because we're still adding the const 0 before anything else via the UCB constructor. Once we remove the +1 - // from Noir, we'll need to update the UCB by moving that const 0 to get these tests to pass again. - // Add the witness variables known directly from acir + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): NOTE: This still works even though the + // witness indices in the explicit acir gates are +1 offset because we're still adding the const 0 before + // anything else via the UCB constructor. Once we remove the +1 from Noir, we'll need to update the UCB by + // moving that const 0 to get these tests to pass again. 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. 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 677fd6a6269..01b8e20e85c 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 @@ -681,6 +681,9 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasezero_idx = put_constant_variable(FF::zero()); this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this for (size_t idx = 0; idx < varnum; ++idx) { From a248f882c2a6a4c205771f35a6a3faca05a5710f Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 8 Jan 2024 21:46:17 +0000 Subject: [PATCH 09/11] fix merge conflict --- .../cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp index 4549f245001..d01f3bd947b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp @@ -459,7 +459,7 @@ TEST_F(AcirFormatTests, TestKeccakPermutation) 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50 }; - auto builder = create_circuit_with_witness(constraint_system, witness); + auto builder = create_circuit(constraint_system, /*size_hint=*/0, witness); auto composer = Composer(); auto prover = composer.create_ultra_with_keccak_prover(builder); From a7838395bcad8e743bdacb267862befc69792794 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 10 Jan 2024 20:20:54 +0000 Subject: [PATCH 10/11] comments and new serialize arith fctn --- .../acir_format/acir_to_constraint_buf.hpp | 89 +++++++++++++------ .../dsl/acir_format/serde/acir.hpp | 2 + .../circuit_builder/ultra_circuit_builder.hpp | 6 +- noir/Cargo.lock | 2 +- 4 files changed, 71 insertions(+), 28 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp index 9b6b36d7e06..437a6fde542 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp @@ -19,8 +19,22 @@ namespace acir_format { +/** + * @brief Construct a poly_tuple for a standard width-3 arithmetic gate from its acir representation + * + * @param arg acir representation of an 3-wire arithmetic operation + * @return poly_triple + * @note In principle Circuit::Expression can accomodate arbitrarily many quadratic and linear terms but in practice the + * ones processed here have a max of 1 and 3 respectively, in accordance with the standard width-3 arithmetic gate. + */ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Instead of zeros for a,b,c, what we really want + // is something like the bberg zero_idx. Hardcoding these to 0 has the same effect only if zero_idx == 0, as has + // historically been the case. If that changes, this might break since now "0" points to some non-zero witness in + // variables. (From some testing, it seems like it may not break but only because the selectors multiplying the + // erroneously non-zero value are zero. Still, it seems like a bad idea to have erroneous wire values even if they + // dont break the relation. They'll still add cost in commitments, for example). poly_triple pt{ .a = 0, .b = 0, @@ -31,32 +45,53 @@ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) .q_o = 0, .q_c = 0, }; - // Think this never longer than 1? - for (const auto& e : arg.mul_terms) { - uint256_t qm(std::get<0>(e)); - uint32_t a = std::get<1>(e).value; - uint32_t b = std::get<2>(e).value; - pt.q_m = qm; - pt.a = a; - pt.b = b; + + // Flags indicating whether each witness index for the present poly_tuple has been set + bool a_set = false; + bool b_set = false; + bool c_set = false; + + // If necessary, set values for quadratic term (q_m * w_l * w_r) + ASSERT(arg.mul_terms.size() <= 1); // We can only accomodate 1 quadratic term + // Note: mul_terms are tuples of the form {selector_value, witness_idx_1, witness_idx_2} + if (!arg.mul_terms.empty()) { + const auto& mul_term = arg.mul_terms[0]; + pt.q_m = uint256_t(std::get<0>(mul_term)); + pt.a = std::get<1>(mul_term).value; + pt.b = std::get<2>(mul_term).value; + a_set = true; + b_set = true; } - for (const auto& e : arg.linear_combinations) { - barretenberg::fr x(uint256_t(std::get<0>(e))); - uint32_t witness = std::get<1>(e).value; - if (pt.a == 0 || pt.a == witness) { - pt.a = witness; - pt.q_l = x; - } else if (pt.b == 0 || pt.b == witness) { - pt.b = witness; - pt.q_r = x; - } else if (pt.c == 0 || pt.c == witness) { - pt.c = witness; - pt.q_o = x; + // If necessary, set values for linears terms q_l * w_l, q_r * w_r and q_o * w_o + ASSERT(arg.linear_combinations.size() <= 3); // We can only accomodate 3 linear terms + for (const auto& linear_term : arg.linear_combinations) { + barretenberg::fr selector_value(uint256_t(std::get<0>(linear_term))); + uint32_t witness_idx = std::get<1>(linear_term).value; + + // If the witness index has not yet been set or if the corresponding linear term is active, set the witness + // index and the corresponding selector value. + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): May need to adjust the pt.a == witness_idx + // check (and the others like it) since we initialize a,b,c with 0 but 0 becomes a valid witness index if the +1 + // offset is removed from noir. + if (!a_set || pt.a == witness_idx) { // q_l * w_l + pt.a = witness_idx; + pt.q_l = selector_value; + a_set = true; + } else if (!b_set || pt.b == witness_idx) { // q_r * w_r + pt.b = witness_idx; + pt.q_r = selector_value; + b_set = true; + } else if (!c_set || pt.c == witness_idx) { // q_o * w_o + pt.c = witness_idx; + pt.q_o = selector_value; + c_set = true; } else { - throw_or_abort("Cannot assign linear term to a constrain of width 3"); + throw_or_abort("Cannot assign linear term to a constraint of width 3"); } } + + // Set constant value q_c pt.q_c = uint256_t(arg.q_c); return pt; } @@ -270,9 +305,7 @@ acir_format circuit_buf_to_acir_format(std::vector const& buf) auto circuit = Circuit::Circuit::bincodeDeserialize(buf); acir_format af; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): this +1 seems to be accounting for the const 0 at - // the first index in variables - af.varnum = circuit.current_witness_index + 1; + af.varnum = circuit.current_witness_index; af.public_inputs = join({ map(circuit.public_parameters.value, [](auto e) { return e.value; }), map(circuit.return_values.value, [](auto e) { return e.value; }) }); std::map block_id_to_block_constraint; @@ -310,10 +343,16 @@ WitnessVector witness_buf_to_witness_data(std::vector const& buf) { auto w = WitnessMap::WitnessMap::bincodeDeserialize(buf); WitnessVector wv; + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Does "index" need to be initialized to 0 once we + // get rid of the +1 offeet in noir? size_t index = 1; for (auto& e : w.value) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/824): Document what this mechanism is doing. It + // appears to be somewhat unrelated to the const 0 issue (but see discussion in issue for caveats). See 2_div + // for an example of when this gets used. Seems like a mechanism for adding 0s intermittently between known + // witness values. while (index < e.first.value) { - wv.push_back(barretenberg::fr(0)); // TODO(https://github.com/AztecProtocol/barretenberg/issues/816)? + wv.push_back(barretenberg::fr(0)); index++; } wv.push_back(barretenberg::fr(uint256_t(e.second))); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp index 4aa912073c8..9dc3142e28c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -217,6 +217,8 @@ struct BlockId { static BlockId bincodeDeserialize(std::vector); }; +// TODO(https://github.com/AztecProtocol/barretenberg/issues/825): This struct is more general than it needs / should be +// allowed to be. We can only accommodate 1 quadratic term and 3 linear terms. struct Expression { std::vector> mul_terms; std::vector> linear_combinations; 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 01b8e20e85c..f7f47d2ca29 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 @@ -681,11 +681,13 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasezero_idx = put_constant_variable(FF::zero()); this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this + 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. diff --git a/noir/Cargo.lock b/noir/Cargo.lock index 05bcfb2b582..0334ee5f552 100644 --- a/noir/Cargo.lock +++ b/noir/Cargo.lock @@ -564,7 +564,7 @@ dependencies = [ "arrayref", "arrayvec", "cc", - "cfg-if", + "cfg-if 1.0.0", "constant_time_eq", ] From 5a0de8f27b080c277ab3676c455deda2308e1395 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 10 Jan 2024 20:24:32 +0000 Subject: [PATCH 11/11] typos --- .../dsl/acir_format/acir_to_constraint_buf.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp index 437a6fde542..6cc6e28fb59 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp @@ -24,8 +24,8 @@ namespace acir_format { * * @param arg acir representation of an 3-wire arithmetic operation * @return poly_triple - * @note In principle Circuit::Expression can accomodate arbitrarily many quadratic and linear terms but in practice the - * ones processed here have a max of 1 and 3 respectively, in accordance with the standard width-3 arithmetic gate. + * @note In principle Circuit::Expression can accommodate arbitrarily many quadratic and linear terms but in practice + * the ones processed here have a max of 1 and 3 respectively, in accordance with the standard width-3 arithmetic gate. */ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) { @@ -52,7 +52,7 @@ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) bool c_set = false; // If necessary, set values for quadratic term (q_m * w_l * w_r) - ASSERT(arg.mul_terms.size() <= 1); // We can only accomodate 1 quadratic term + ASSERT(arg.mul_terms.size() <= 1); // We can only accommodate 1 quadratic term // Note: mul_terms are tuples of the form {selector_value, witness_idx_1, witness_idx_2} if (!arg.mul_terms.empty()) { const auto& mul_term = arg.mul_terms[0]; @@ -64,7 +64,7 @@ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) } // If necessary, set values for linears terms q_l * w_l, q_r * w_r and q_o * w_o - ASSERT(arg.linear_combinations.size() <= 3); // We can only accomodate 3 linear terms + ASSERT(arg.linear_combinations.size() <= 3); // We can only accommodate 3 linear terms for (const auto& linear_term : arg.linear_combinations) { barretenberg::fr selector_value(uint256_t(std::get<0>(linear_term))); uint32_t witness_idx = std::get<1>(linear_term).value;