From 2fc7f28fabb59a85e7bb665021baf734a563064c Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 00:41:53 +0000 Subject: [PATCH 01/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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 da2c889063dc58d277e3cc2e05ed29082617f484 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 9 Jan 2024 18:15:38 +0000 Subject: [PATCH 10/28] gitignore WiP --- barretenberg/acir_tests/.gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/barretenberg/acir_tests/.gitignore b/barretenberg/acir_tests/.gitignore index 47b24c40c7e..76ef433ebec 100644 --- a/barretenberg/acir_tests/.gitignore +++ b/barretenberg/acir_tests/.gitignore @@ -1 +1,3 @@ -acir_tests \ No newline at end of file +acir_tests +acir_tests_classic +acir_tests_new \ No newline at end of file From 6a0a68a66651040c83b8df31eb0b12fa36a0c1f4 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 9 Jan 2024 00:46:20 +0000 Subject: [PATCH 11/28] have the start witness of ACIR generated by noir start at zero --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 8 +++---- .../ssa/acir_gen/acir_ir/generated_acir.rs | 21 ++++++++++--------- .../src/ssa/acir_gen/acir_ir/sort.rs | 4 ++-- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index ddafc0bb570..d6fad629c2e 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -138,7 +138,7 @@ impl AcirContext { /// Adds a Variable to the context, whose exact value is resolved at /// runtime. pub(crate) fn add_variable(&mut self) -> AcirVar { - let var_index = self.acir_ir.next_witness_index(); + let var_index = self.acir_ir.get_current_witness_and_update(); let var_data = AcirVarData::Witness(var_index); @@ -1347,7 +1347,7 @@ impl AcirContext { let mut b_outputs = Vec::new(); let outputs_var = vecmap(outputs, |output| match output { AcirType::NumericType(_) => { - let witness_index = self.acir_ir.next_witness_index(); + let witness_index = self.acir_ir.get_current_witness_and_update(); b_outputs.push(BrilligOutputs::Simple(witness_index)); let var = self.add_data(AcirVarData::Witness(witness_index)); AcirValue::Var(var, output.clone()) @@ -1412,7 +1412,7 @@ impl AcirContext { array_values.push_back(nested_acir_value); } AcirType::NumericType(_) => { - let witness_index = self.acir_ir.next_witness_index(); + let witness_index = self.acir_ir.get_current_witness_and_update(); witnesses.push(witness_index); let var = self.add_data(AcirVarData::Witness(witness_index)); array_values.push_back(AcirValue::Var(var, element_type.clone())); @@ -1497,7 +1497,7 @@ impl AcirContext { // Convert the inputs into expressions let inputs_expr = try_vecmap(inputs, |input| self.var_to_expression(input))?; // Generate output witnesses - let outputs_witness = vecmap(0..len, |_| self.acir_ir.next_witness_index()); + let outputs_witness = vecmap(0..len, |_| self.acir_ir.get_current_witness_and_update()); let output_expr = vecmap(&outputs_witness, |witness_index| Expression::from(*witness_index)); let outputs_var = vecmap(&outputs_witness, |witness_index| { diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index bea0a5e3158..f3c78bb811a 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -75,11 +75,12 @@ impl GeneratedAcir { std::mem::take(&mut self.opcodes) } - /// Updates the witness index counter and returns - /// the next witness index. - pub(crate) fn next_witness_index(&mut self) -> Witness { + /// Returns the current witness index and updates it + /// so that we have a fresh witness the next time this method is called + pub(crate) fn get_current_witness_and_update(&mut self) -> Witness { + let next_witness_index = Witness(self.current_witness_index); self.current_witness_index += 1; - Witness(self.current_witness_index) + next_witness_index } /// Converts [`Expression`] `expr` into a [`Witness`]. @@ -100,7 +101,7 @@ impl GeneratedAcir { /// Once the `Expression` goes over degree-2, then it needs to be reduced to a `Witness` /// which has degree-1 in order to be able to continue the multiplication chain. pub(crate) fn create_witness_for_expression(&mut self, expression: &Expression) -> Witness { - let fresh_witness = self.next_witness_index(); + let fresh_witness = self.get_current_witness_and_update(); // Create a constraint that sets them to be equal to each other // Then return the witness as this can now be used in places @@ -138,7 +139,7 @@ impl GeneratedAcir { intrinsics_check_inputs(func_name, input_count); intrinsics_check_outputs(func_name, output_count); - let outputs = vecmap(0..output_count, |_| self.next_witness_index()); + let outputs = vecmap(0..output_count, |_| self.get_current_witness_and_update()); // clone is needed since outputs is moved when used in blackbox function. let outputs_clone = outputs.clone(); @@ -271,7 +272,7 @@ impl GeneratedAcir { "ICE: Radix must be a power of 2" ); - let limb_witnesses = vecmap(0..limb_count, |_| self.next_witness_index()); + let limb_witnesses = vecmap(0..limb_count, |_| self.get_current_witness_and_update()); self.push_opcode(AcirOpcode::Directive(Directive::ToLeRadix { a: input_expr.clone(), b: limb_witnesses.clone(), @@ -356,7 +357,7 @@ impl GeneratedAcir { /// resulting `Witness` is constrained to be the inverse. pub(crate) fn brillig_inverse(&mut self, expr: Expression) -> Witness { // Create the witness for the result - let inverted_witness = self.next_witness_index(); + let inverted_witness = self.get_current_witness_and_update(); // Compute the inverse with brillig code let inverse_code = brillig_directive::directive_invert(); @@ -450,7 +451,7 @@ impl GeneratedAcir { // the prover can choose anything here. let z = self.brillig_inverse(t_witness.into()); - let y = self.next_witness_index(); + let y = self.get_current_witness_and_update(); // Add constraint y == 1 - tz => y + tz - 1 == 0 let y_is_boolean_constraint = Expression { @@ -540,7 +541,7 @@ impl GeneratedAcir { bits_len += ((i + 1) as f32).log2().ceil() as u32; } - let bits = vecmap(0..bits_len, |_| self.next_witness_index()); + let bits = vecmap(0..bits_len, |_| self.get_current_witness_and_update()); let inputs = in_expr.iter().map(|a| vec![a.clone()]).collect(); self.push_opcode(AcirOpcode::Directive(Directive::PermutationSort { inputs, diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs index 52640d32337..570d2d11ff6 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs @@ -25,7 +25,7 @@ impl GeneratedAcir { // witness for the input switches let mut conf = iter_extended::vecmap(0..n1, |i| { if generate_witness { - self.next_witness_index() + self.get_current_witness_and_update() } else { bits[i] } @@ -63,7 +63,7 @@ impl GeneratedAcir { let (w2, b2) = self.permutation_layer(&in_sub2, bits2, generate_witness)?; // apply the output switches for i in 0..(n - 1) / 2 { - let c = if generate_witness { self.next_witness_index() } else { bits[n1 + i] }; + let c = if generate_witness { self.get_current_witness_and_update() } else { bits[n1 + i] }; conf.push(c); let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i])); out_expr.push(&intermediate + &b1[i]); diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index edf0461430f..430eef2435d 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -294,7 +294,7 @@ impl Context { dfg: &DataFlowGraph, ) -> Result, RuntimeError> { // The first witness (if any) is the next one - let start_witness = self.acir_context.current_witness_index().0 + 1; + let start_witness = self.acir_context.current_witness_index().0; for param_id in params { let typ = dfg.type_of_value(*param_id); let value = self.convert_ssa_block_param(&typ)?; From a5f0558b784ebf8ac64a1eb507a7a1a708d64051 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 9 Jan 2024 14:20:40 +0000 Subject: [PATCH 12/28] rename variable --- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index f3c78bb811a..aff56c52473 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -78,9 +78,9 @@ impl GeneratedAcir { /// Returns the current witness index and updates it /// so that we have a fresh witness the next time this method is called pub(crate) fn get_current_witness_and_update(&mut self) -> Witness { - let next_witness_index = Witness(self.current_witness_index); + let current_witness_index = Witness(self.current_witness_index); self.current_witness_index += 1; - next_witness_index + current_witness_index } /// Converts [`Expression`] `expr` into a [`Witness`]. From 299e82846e62fe336eb3659851339249561df6ce Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 10 Jan 2024 18:31:47 +0000 Subject: [PATCH 13/28] decrement cw index (Maxim) --- .../noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index d6fad629c2e..19f156e1b7a 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1290,6 +1290,7 @@ impl AcirContext { inputs: Vec, warnings: Vec, ) -> GeneratedAcir { + self.acir_ir.current_witness_index -= 1; self.acir_ir.input_witnesses = inputs; self.acir_ir.warnings = warnings; self.acir_ir From b52fd30faea270989eb6ca95f64e996345b292dd Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 10 Jan 2024 18:51:39 +0000 Subject: [PATCH 14/28] debug w Maxim WiP --- barretenberg/acir_tests/.gitignore | 3 +- barretenberg/acir_tests/run_acir_tests.sh | 6 +- .../acir_format/acir_to_constraint_buf.hpp | 101 ++++++++++++++---- .../dsl/acir_format/serde/acir.hpp | 3 + .../dsl/acir_proofs/acir_composer.cpp | 32 ++++++ .../circuit_builder/ultra_circuit_builder.cpp | 10 ++ .../circuit_builder/ultra_circuit_builder.hpp | 18 +++- noir/Cargo.lock | 2 +- 8 files changed, 143 insertions(+), 32 deletions(-) diff --git a/barretenberg/acir_tests/.gitignore b/barretenberg/acir_tests/.gitignore index 76ef433ebec..1bf1d213922 100644 --- a/barretenberg/acir_tests/.gitignore +++ b/barretenberg/acir_tests/.gitignore @@ -1,3 +1,4 @@ acir_tests acir_tests_classic -acir_tests_new \ No newline at end of file +acir_tests_new +acir_tests_new_new \ No newline at end of file diff --git a/barretenberg/acir_tests/run_acir_tests.sh b/barretenberg/acir_tests/run_acir_tests.sh index ee28c975113..fc5c88771c4 100755 --- a/barretenberg/acir_tests/run_acir_tests.sh +++ b/barretenberg/acir_tests/run_acir_tests.sh @@ -31,10 +31,12 @@ export BIN CRS_PATH VERBOSE BRANCH ./clone_test_vectors.sh -cd acir_tests +cd acir_tests_new_new +# cd acir_tests_new +# cd acir_tests_classic # Convert them to array -SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member) +SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member sha2_byte) function test() { cd $1 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..7717a8603da 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 @@ -21,6 +21,11 @@ namespace acir_format { poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) { + // WORKTODO: Deal with this now? + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): The zeros for a,b,c are making an implicit + // assumption that the 0th variable will be FF(0). Once this is not the case, this might break. Instead it should be + // something like zero_idx - but thats a builder member that's not known at this time. Could we do something like + // define a pointer to a uint32_t that will eventually hold the right zero_idx and set it later? poly_triple pt{ .a = 0, .b = 0, @@ -31,32 +36,75 @@ 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; + + bool a_set = false; + bool b_set = false; + bool c_set = false; + + // Handle quadratic term if it exists + // info(); + 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; + // info("MUL!"); } - 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; + // WORKTODO: Look at simple_radix. Nearly everything looks correct + // except for the w_3 witness index in the poly gates. Its 5 in classic and stays 5 in new. + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): May need to adjust the pt.a == witness_idx check + // since we initialize with 0 but 0 is now also a valid witness index. + 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; + + // info("witness_idx = ", witness_idx); + + if (!a_set || pt.a == witness_idx) { + pt.a = witness_idx; + pt.q_l = selector_value; + a_set = true; + // info("IF 0"); + } else if (!b_set || pt.b == witness_idx) { + pt.b = witness_idx; + pt.q_r = selector_value; + b_set = true; + // info("IF 1"); + } else if (!c_set || pt.c == witness_idx) { + pt.c = witness_idx; + pt.q_o = selector_value; + c_set = true; + // info("IF 2"); } 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"); } } + + // 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; + + // // WORKTODO: this is where the problem is + // if (pt.a == 0 || pt.a == witness_idx) { + // pt.a = witness_idx; + // pt.q_l = selector_value; + // info("IF 0"); + // } else if (pt.b == 0 || pt.b == witness_idx) { + // pt.b = witness_idx; + // pt.q_r = selector_value; + // info("IF 1"); + // } else if (pt.c == 0 || pt.c == witness_idx) { + // pt.c = witness_idx; + // pt.q_o = selector_value; + // info("IF 2"); + // } else { + // throw_or_abort("Cannot assign linear term to a constrain of width 3"); + // } + // } + pt.q_c = uint256_t(arg.q_c); return pt; } @@ -272,7 +320,7 @@ acir_format circuit_buf_to_acir_format(std::vector const& 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 +358,17 @@ WitnessVector witness_buf_to_witness_data(std::vector const& buf) { auto w = WitnessMap::WitnessMap::bincodeDeserialize(buf); WitnessVector wv; - size_t index = 1; + size_t index = 1; // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Does this need to become 0 once + // we get rid of the +1 offeet in noir? for (auto& e : w.value) { + // info("MAIN!"); while (index < e.first.value) { + // WORKTODO: this actually appears to be unrelated to the const 0 issue. See 2_div for an example of when + // this gets used. It only triggers once (for the last witness) and e.first.value is equal to 16. Ask Kev? + // Seems like a mechanism for adding 0s intermittently between known witness values. wv.push_back(barretenberg::fr(0)); // TODO(https://github.com/AztecProtocol/barretenberg/issues/816)? + // info("IN HERE!"); + // info("e.first.value = ", e.first.value); 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..46af8139f01 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -218,6 +218,9 @@ struct BlockId { }; struct Expression { + // WORKTODO: should be this: + // std::tuple mul_term; + // WORKTODO: also, why is this string instead of uint256 or field or something std::vector> mul_terms; std::vector> linear_combinations; std::string q_c; 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 5eb044f1491..171e2ac036e 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -52,6 +52,38 @@ std::vector AcirComposer::create_proof(acir_format::acir_format& constr vinfo("building circuit with witness..."); builder_ = acir_format::create_circuit(constraint_system, size_hint_, witness); + // info("\nVARIABLES:"); + // for (auto& var : builder_.variables) { + // info("variable = ", var); + // } + + auto num_gates = builder_.num_gates; + info("num_gates = ", num_gates); + info("witness.size() = ", witness.size()); + + // info("\nPUBLIC INPUT:"); + // for (uint32_t public_input : builder_.public_inputs) { + // info("PI = ", builder_.get_variable(public_input)); + // } + + // info("\nWIRES:"); + // for (size_t i = 0; i < num_gates; ++i) { + // info("w_1 = ", builder_.get_variable(builder_.w_l()[i])); + // info("w_2 = ", builder_.get_variable(builder_.w_r()[i])); + // info("w_3 = ", builder_.get_variable(builder_.w_o()[i])); + // info("w_4 = ", builder_.get_variable(builder_.w_4()[i])); + // } + // info("\nSELECTORS:"); + // for (size_t i = 0; i < num_gates; ++i) { + // info("q_1 = ", builder_.q_1()[i]); + // info("q_2 = ", builder_.q_2()[i]); + // info("q_3 = ", builder_.q_3()[i]); + // info("q_4 = ", builder_.q_4()[i]); + // info("q_c = ", builder_.q_c()[i]); + // info("q_m = ", builder_.q_m()[i]); + // info("q_arith = ", builder_.q_arith()[i]); + // } + vinfo("gates: ", builder_.get_total_circuit_size()); auto composer = [&]() { diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index d3d9767bb74..f958546d7ec 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -384,6 +384,16 @@ void UltraCircuitBuilder_::create_poly_gate(const poly_triple_< { this->assert_valid_variables({ in.a, in.b, in.c }); + // info("\nCreating poly gate:"); + // info("w_1 = ", in.a); + // info("w_2 = ", in.b); + // info("w_3 = ", in.c); + // info("q_l = ", in.q_l); + // info("q_r = ", in.q_r); + // info("q_o = ", in.q_o); + // info("q_c = ", in.q_c); + // info("q_m = ", in.q_m); + w_l().emplace_back(in.a); w_r().emplace_back(in.b); w_o().emplace_back(in.c); 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..762c13b7420 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,11 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasezero_idx = put_constant_variable(FF::zero()); - this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this + + // // WORKTODO + // 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. @@ -695,6 +695,14 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasepublic_inputs = public_inputs; + + // WORKTODO + this->zero_idx = put_constant_variable(FF::zero()); + this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this + + // info("varnum = ", varnum); + // info("witness_values.size() = ", witness_values.size()); + // info("ZERO IDX = ", this->zero_idx); }; UltraCircuitBuilder_(const UltraCircuitBuilder_& other) = default; UltraCircuitBuilder_(UltraCircuitBuilder_&& other) 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 b9c40290cf0eadc802b175d3ea2fd6f0973a27cf Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 20:40:09 +0000 Subject: [PATCH 15/28] passing acir tests --- barretenberg/acir_tests/run_acir_tests.sh | 5 +++-- .../dsl/acir_format/acir_to_constraint_buf.hpp | 8 ++++---- .../circuit_builder/ultra_circuit_builder.hpp | 6 +++--- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/barretenberg/acir_tests/run_acir_tests.sh b/barretenberg/acir_tests/run_acir_tests.sh index fc5c88771c4..19db8ec2ec3 100755 --- a/barretenberg/acir_tests/run_acir_tests.sh +++ b/barretenberg/acir_tests/run_acir_tests.sh @@ -31,7 +31,8 @@ export BIN CRS_PATH VERBOSE BRANCH ./clone_test_vectors.sh -cd acir_tests_new_new +cd acir_tests +# cd acir_tests_new_new # cd acir_tests_new # cd acir_tests_classic @@ -54,7 +55,7 @@ function test() { else echo -e "\033[31mFAILED\033[0m" touch "$error_file" - exit 1 + # exit 1 fi cd .. 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 7717a8603da..5335b1ff1c7 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 @@ -42,7 +42,7 @@ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) bool c_set = false; // Handle quadratic term if it exists - // info(); + // info("!arg.mul_terms.empty() = ", !arg.mul_terms.empty()); if (!arg.mul_terms.empty()) { const auto& mul_term = arg.mul_terms[0]; pt.q_m = uint256_t(std::get<0>(mul_term)); @@ -86,7 +86,6 @@ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) // 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; - // // WORKTODO: this is where the problem is // if (pt.a == 0 || pt.a == witness_idx) { // pt.a = witness_idx; @@ -320,7 +319,8 @@ acir_format circuit_buf_to_acir_format(std::vector const& 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; + // `varnum` is the true number of variables, thus we add one to the index which starts at zero + af.varnum = circuit.current_witness_index + 1; 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; @@ -358,7 +358,7 @@ WitnessVector witness_buf_to_witness_data(std::vector const& buf) { auto w = WitnessMap::WitnessMap::bincodeDeserialize(buf); WitnessVector wv; - size_t index = 1; // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Does this need to become 0 once + size_t index = 0; // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Does this need to become 0 once // we get rid of the +1 offeet in noir? for (auto& e : w.value) { // info("MAIN!"); 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 762c13b7420..496ba2800c7 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 @@ -700,9 +700,9 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasezero_idx = put_constant_variable(FF::zero()); this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this - // info("varnum = ", varnum); - // info("witness_values.size() = ", witness_values.size()); - // info("ZERO IDX = ", this->zero_idx); + info("varnum = ", varnum); + info("witness_values.size() = ", witness_values.size()); + info("ZERO IDX = ", this->zero_idx); }; UltraCircuitBuilder_(const UltraCircuitBuilder_& other) = default; UltraCircuitBuilder_(UltraCircuitBuilder_&& other) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index aff56c52473..57030cf29fc 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -30,7 +30,7 @@ pub(crate) struct GeneratedAcir { /// The next witness index that may be declared. /// /// Equivalent to acvm::acir::circuit::Circuit's field of the same name. - pub(crate) current_witness_index: u32, + pub(crate) current_witness_index: Option, /// The opcodes of which the compiled ACIR will comprise. opcodes: Vec, From 9435618f42ba41b515487df211b783d7ef32e7a9 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 20:54:07 +0000 Subject: [PATCH 16/28] remove option --- .../noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 57030cf29fc..aff56c52473 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -30,7 +30,7 @@ pub(crate) struct GeneratedAcir { /// The next witness index that may be declared. /// /// Equivalent to acvm::acir::circuit::Circuit's field of the same name. - pub(crate) current_witness_index: Option, + pub(crate) current_witness_index: u32, /// The opcodes of which the compiled ACIR will comprise. opcodes: Vec, From 03d8f491ee66dc827207339a9e677e712a993e22 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 21:46:21 +0000 Subject: [PATCH 17/28] cleanup --- barretenberg/acir_tests/.gitignore | 5 +---- barretenberg/acir_tests/run_acir_tests.sh | 7 ++----- .../circuit_builder/ultra_circuit_builder.cpp | 10 ---------- .../circuit_builder/ultra_circuit_builder.hpp | 11 ++--------- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/barretenberg/acir_tests/.gitignore b/barretenberg/acir_tests/.gitignore index 1bf1d213922..47b24c40c7e 100644 --- a/barretenberg/acir_tests/.gitignore +++ b/barretenberg/acir_tests/.gitignore @@ -1,4 +1 @@ -acir_tests -acir_tests_classic -acir_tests_new -acir_tests_new_new \ No newline at end of file +acir_tests \ No newline at end of file diff --git a/barretenberg/acir_tests/run_acir_tests.sh b/barretenberg/acir_tests/run_acir_tests.sh index 19db8ec2ec3..ee28c975113 100755 --- a/barretenberg/acir_tests/run_acir_tests.sh +++ b/barretenberg/acir_tests/run_acir_tests.sh @@ -32,12 +32,9 @@ export BIN CRS_PATH VERBOSE BRANCH ./clone_test_vectors.sh cd acir_tests -# cd acir_tests_new_new -# cd acir_tests_new -# cd acir_tests_classic # Convert them to array -SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member sha2_byte) +SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member) function test() { cd $1 @@ -55,7 +52,7 @@ function test() { else echo -e "\033[31mFAILED\033[0m" touch "$error_file" - # exit 1 + exit 1 fi cd .. diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index f958546d7ec..d3d9767bb74 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -384,16 +384,6 @@ void UltraCircuitBuilder_::create_poly_gate(const poly_triple_< { this->assert_valid_variables({ in.a, in.b, in.c }); - // info("\nCreating poly gate:"); - // info("w_1 = ", in.a); - // info("w_2 = ", in.b); - // info("w_3 = ", in.c); - // info("q_l = ", in.q_l); - // info("q_r = ", in.q_r); - // info("q_o = ", in.q_o); - // info("q_c = ", in.q_c); - // info("q_m = ", in.q_m); - w_l().emplace_back(in.a); w_r().emplace_back(in.b); w_o().emplace_back(in.c); 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 762c13b7420..9ecc81bb829 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 @@ -682,10 +682,6 @@ 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. @@ -696,13 +692,10 @@ class UltraCircuitBuilder_ : public CircuitBuilderBasepublic_inputs = public_inputs; - // WORKTODO + // Add the const zero variable after the acir witness has been + // incorporated into variables. this->zero_idx = put_constant_variable(FF::zero()); this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this - - // info("varnum = ", varnum); - // info("witness_values.size() = ", witness_values.size()); - // info("ZERO IDX = ", this->zero_idx); }; UltraCircuitBuilder_(const UltraCircuitBuilder_& other) = default; UltraCircuitBuilder_(UltraCircuitBuilder_&& other) From 37e8207a4f8421c20170054003170c095abb5849 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 21:50:08 +0000 Subject: [PATCH 18/28] restrict access to current_witness_index --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 2 +- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index add80ea07e3..d066f607251 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1290,7 +1290,7 @@ impl AcirContext { inputs: Vec, warnings: Vec, ) -> GeneratedAcir { - self.acir_ir.current_witness_index -= 1; + self.acir_ir.decrement_witness_index(); self.acir_ir.input_witnesses = inputs; self.acir_ir.warnings = warnings; self.acir_ir diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index f91799e814c..c6e3de5d125 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -30,7 +30,7 @@ pub(crate) struct GeneratedAcir { /// The next witness index that may be declared. /// /// Equivalent to acvm::acir::circuit::Circuit's field of the same name. - pub(crate) current_witness_index: u32, + current_witness_index: u32, /// The opcodes of which the compiled ACIR will comprise. opcodes: Vec, @@ -83,6 +83,10 @@ impl GeneratedAcir { current_witness_index } + pub(crate) fn decrement_witness_index(&mut self) { + self.current_witness_index -= 1; + } + /// Converts [`Expression`] `expr` into a [`Witness`]. /// /// If `expr` can be represented as a `Witness` then this function will return it, From 3f74422ba55dcc085ae959dbe1550c29b1376dc8 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 21:57:33 +0000 Subject: [PATCH 19/28] use current_witness_index method --- noir/compiler/noirc_evaluator/src/ssa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa.rs b/noir/compiler/noirc_evaluator/src/ssa.rs index deffe84baea..20c860ac9ad 100644 --- a/noir/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/compiler/noirc_evaluator/src/ssa.rs @@ -93,8 +93,8 @@ pub fn create_circuit( let mut generated_acir = optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?; let opcodes = generated_acir.take_opcodes(); + let current_witness_index = generated_acir.current_witness_index(); let GeneratedAcir { - current_witness_index, return_witnesses, locations, input_witnesses, From 8026ccc46cb4c7e09a986268baed3bcccc4123c3 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 22:05:28 +0000 Subject: [PATCH 20/28] messed up types --- noir/compiler/noirc_evaluator/src/ssa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa.rs b/noir/compiler/noirc_evaluator/src/ssa.rs index 20c860ac9ad..f11c077d49a 100644 --- a/noir/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/compiler/noirc_evaluator/src/ssa.rs @@ -93,7 +93,7 @@ pub fn create_circuit( let mut generated_acir = optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?; let opcodes = generated_acir.take_opcodes(); - let current_witness_index = generated_acir.current_witness_index(); + let current_witness_index = generated_acir.current_witness_index().0; let GeneratedAcir { return_witnesses, locations, From d450b40cbf85c4d410750e3990f3e4226ac73b05 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 22:44:53 +0000 Subject: [PATCH 21/28] switch current_witness_index to be an option --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 9 ++--- .../ssa/acir_gen/acir_ir/generated_acir.rs | 40 ++++++++++--------- .../src/ssa/acir_gen/acir_ir/sort.rs | 4 +- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index d066f607251..f8b8579eb66 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -138,7 +138,7 @@ impl AcirContext { /// Adds a Variable to the context, whose exact value is resolved at /// runtime. pub(crate) fn add_variable(&mut self) -> AcirVar { - let var_index = self.acir_ir.get_current_witness_and_update(); + let var_index = self.acir_ir.next_witness_index(); let var_data = AcirVarData::Witness(var_index); @@ -1290,7 +1290,6 @@ impl AcirContext { inputs: Vec, warnings: Vec, ) -> GeneratedAcir { - self.acir_ir.decrement_witness_index(); self.acir_ir.input_witnesses = inputs; self.acir_ir.warnings = warnings; self.acir_ir @@ -1348,7 +1347,7 @@ impl AcirContext { let mut b_outputs = Vec::new(); let outputs_var = vecmap(outputs, |output| match output { AcirType::NumericType(_) => { - let witness_index = self.acir_ir.get_current_witness_and_update(); + let witness_index = self.acir_ir.next_witness_index(); b_outputs.push(BrilligOutputs::Simple(witness_index)); let var = self.add_data(AcirVarData::Witness(witness_index)); AcirValue::Var(var, output.clone()) @@ -1413,7 +1412,7 @@ impl AcirContext { array_values.push_back(nested_acir_value); } AcirType::NumericType(_) => { - let witness_index = self.acir_ir.get_current_witness_and_update(); + let witness_index = self.acir_ir.next_witness_index(); witnesses.push(witness_index); let var = self.add_data(AcirVarData::Witness(witness_index)); array_values.push_back(AcirValue::Var(var, element_type.clone())); @@ -1498,7 +1497,7 @@ impl AcirContext { // Convert the inputs into expressions let inputs_expr = try_vecmap(inputs, |input| self.var_to_expression(input))?; // Generate output witnesses - let outputs_witness = vecmap(0..len, |_| self.acir_ir.get_current_witness_and_update()); + let outputs_witness = vecmap(0..len, |_| self.acir_ir.next_witness_index()); let output_expr = vecmap(&outputs_witness, |witness_index| Expression::from(*witness_index)); let outputs_var = vecmap(&outputs_witness, |witness_index| { diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index c6e3de5d125..7855515255c 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -28,9 +28,12 @@ use num_bigint::BigUint; /// The output of the Acir-gen pass pub(crate) struct GeneratedAcir { /// The next witness index that may be declared. - /// + /// If witness index is `None` then we have not yet created a witness + /// and thus next witness index that be declared is zero. + /// This field is private should only ever be accessed through its getter and setter. + /// /// Equivalent to acvm::acir::circuit::Circuit's field of the same name. - current_witness_index: u32, + current_witness_index: Option, /// The opcodes of which the compiled ACIR will comprise. opcodes: Vec, @@ -60,7 +63,7 @@ pub(crate) struct GeneratedAcir { impl GeneratedAcir { /// Returns the current witness index. pub(crate) fn current_witness_index(&self) -> Witness { - Witness(self.current_witness_index) + Witness(self.current_witness_index.unwrap_or(0)) } /// Adds a new opcode into ACIR. @@ -75,16 +78,15 @@ impl GeneratedAcir { std::mem::take(&mut self.opcodes) } - /// Returns the current witness index and updates it - /// so that we have a fresh witness the next time this method is called - pub(crate) fn get_current_witness_and_update(&mut self) -> Witness { - let current_witness_index = Witness(self.current_witness_index); - self.current_witness_index += 1; - current_witness_index - } - - pub(crate) fn decrement_witness_index(&mut self) { - self.current_witness_index -= 1; + /// Updates the witness index counter and returns + /// the next witness index. + pub(crate) fn next_witness_index(&mut self) -> Witness { + if let Some(current_index) = self.current_witness_index { + self.current_witness_index.replace(current_index + 1); + } else { + self.current_witness_index = Some(0); + } + Witness(self.current_witness_index.expect("ICE: current_witness_index should exist")) } /// Converts [`Expression`] `expr` into a [`Witness`]. @@ -105,7 +107,7 @@ impl GeneratedAcir { /// Once the `Expression` goes over degree-2, then it needs to be reduced to a `Witness` /// which has degree-1 in order to be able to continue the multiplication chain. pub(crate) fn create_witness_for_expression(&mut self, expression: &Expression) -> Witness { - let fresh_witness = self.get_current_witness_and_update(); + let fresh_witness = self.next_witness_index(); // Create a constraint that sets them to be equal to each other // Then return the witness as this can now be used in places @@ -143,7 +145,7 @@ impl GeneratedAcir { intrinsics_check_inputs(func_name, input_count); intrinsics_check_outputs(func_name, output_count); - let outputs = vecmap(0..output_count, |_| self.get_current_witness_and_update()); + let outputs = vecmap(0..output_count, |_| self.next_witness_index()); // clone is needed since outputs is moved when used in blackbox function. let outputs_clone = outputs.clone(); @@ -274,7 +276,7 @@ impl GeneratedAcir { "ICE: Radix must be a power of 2" ); - let limb_witnesses = vecmap(0..limb_count, |_| self.get_current_witness_and_update()); + let limb_witnesses = vecmap(0..limb_count, |_| self.next_witness_index()); self.push_opcode(AcirOpcode::Directive(Directive::ToLeRadix { a: input_expr.clone(), b: limb_witnesses.clone(), @@ -359,7 +361,7 @@ impl GeneratedAcir { /// resulting `Witness` is constrained to be the inverse. pub(crate) fn brillig_inverse(&mut self, expr: Expression) -> Witness { // Create the witness for the result - let inverted_witness = self.get_current_witness_and_update(); + let inverted_witness = self.next_witness_index(); // Compute the inverse with brillig code let inverse_code = brillig_directive::directive_invert(); @@ -453,7 +455,7 @@ impl GeneratedAcir { // the prover can choose anything here. let z = self.brillig_inverse(t_witness.into()); - let y = self.get_current_witness_and_update(); + let y = self.next_witness_index(); // Add constraint y == 1 - tz => y + tz - 1 == 0 let y_is_boolean_constraint = Expression { @@ -543,7 +545,7 @@ impl GeneratedAcir { bits_len += ((i + 1) as f32).log2().ceil() as u32; } - let bits = vecmap(0..bits_len, |_| self.get_current_witness_and_update()); + let bits = vecmap(0..bits_len, |_| self.next_witness_index()); let inputs = in_expr.iter().map(|a| vec![a.clone()]).collect(); self.push_opcode(AcirOpcode::Directive(Directive::PermutationSort { inputs, diff --git a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs index 570d2d11ff6..52640d32337 100644 --- a/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs +++ b/noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs @@ -25,7 +25,7 @@ impl GeneratedAcir { // witness for the input switches let mut conf = iter_extended::vecmap(0..n1, |i| { if generate_witness { - self.get_current_witness_and_update() + self.next_witness_index() } else { bits[i] } @@ -63,7 +63,7 @@ impl GeneratedAcir { let (w2, b2) = self.permutation_layer(&in_sub2, bits2, generate_witness)?; // apply the output switches for i in 0..(n - 1) / 2 { - let c = if generate_witness { self.get_current_witness_and_update() } else { bits[n1 + i] }; + let c = if generate_witness { self.next_witness_index() } else { bits[n1 + i] }; conf.push(c); let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i])); out_expr.push(&intermediate + &b1[i]); From d8ea433d0944ee55eeda29a566d6e4f4e23c8bd7 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 11 Jan 2024 23:16:11 +0000 Subject: [PATCH 22/28] passing dsl tests except for TestVarKeccak --- .../dsl/acir_format/acir_format.test.cpp | 82 +++++++++---------- .../dsl/acir_format/block_constraint.test.cpp | 12 +-- .../dsl/acir_format/ecdsa_secp256k1.test.cpp | 2 +- .../dsl/acir_format/ecdsa_secp256r1.test.cpp | 2 +- .../acir_format/recursion_constraint.test.cpp | 39 +++++---- 5 files changed, 68 insertions(+), 69 deletions(-) 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 eff4e6928ae..99c25d3e8d6 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 @@ -79,24 +79,24 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) * } **/ RangeConstraint range_a{ - .witness = 1, + .witness = 0, .num_bits = 32, }; RangeConstraint range_b{ - .witness = 2, + .witness = 1, .num_bits = 32, }; LogicConstraint logic_constraint{ - .a = 1, - .b = 2, - .result = 3, + .a = 0, + .b = 1, + .result = 2, .num_bits = 32, .is_xor_gate = 1, }; poly_triple expr_a{ - .a = 3, - .b = 4, + .a = 2, + .b = 3, .c = 0, .q_m = 0, .q_l = 1, @@ -105,9 +105,9 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) .q_c = -10, }; poly_triple expr_b{ - .a = 4, - .b = 5, - .c = 6, + .a = 3, + .b = 4, + .c = 5, .q_m = 1, .q_l = 0, .q_r = 0, @@ -115,9 +115,9 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) .q_c = 0, }; poly_triple expr_c{ - .a = 4, - .b = 6, - .c = 4, + .a = 3, + .b = 5, + .c = 3, .q_m = 1, .q_l = 0, .q_r = 0, @@ -126,7 +126,7 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) }; poly_triple expr_d{ - .a = 6, + .a = 5, .b = 0, .c = 0, .q_m = 0, @@ -139,8 +139,8 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit) // EXPR [ (1, _4, _6) (-1, _4) 0 ] // EXPR [ (-1, _6) 1 ] - acir_format constraint_system{ .varnum = 7, - .public_inputs = { 2 }, + acir_format constraint_system{ .varnum = 6, + .public_inputs = { 1 }, .logic_constraints = { logic_constraint }, .range_constraints = { range_a, range_b }, .sha256_constraints = {}, @@ -181,13 +181,13 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass) std::vector range_constraints; for (uint32_t i = 0; i < 10; i++) { range_constraints.push_back(RangeConstraint{ - .witness = i + 1, + .witness = i, .num_bits = 15, }); } std::vector signature(64); - for (uint32_t i = 0, value = 13; i < 64; i++, value++) { + for (uint32_t i = 0, value = 12; i < 64; i++, value++) { signature[i] = value; range_constraints.push_back(RangeConstraint{ .witness = value, @@ -196,13 +196,13 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass) } SchnorrConstraint schnorr_constraint{ - .message = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }, - .public_key_x = 11, - .public_key_y = 12, - .result = 77, + .message = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }, + .public_key_x = 10, + .public_key_y = 11, + .result = 76, .signature = signature, }; - acir_format constraint_system{ .varnum = 82, + acir_format constraint_system{ .varnum = 81, .public_inputs = {}, .logic_constraints = {}, .range_constraints = range_constraints, @@ -271,13 +271,13 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange) std::vector range_constraints; for (uint32_t i = 0; i < 10; i++) { range_constraints.push_back(RangeConstraint{ - .witness = i + 1, + .witness = i, .num_bits = 8, }); } std::vector signature(64); - for (uint32_t i = 0, value = 13; i < 64; i++, value++) { + for (uint32_t i = 0, value = 12; i < 64; i++, value++) { signature[i] = value; range_constraints.push_back(RangeConstraint{ .witness = value, @@ -286,14 +286,14 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange) } SchnorrConstraint schnorr_constraint{ - .message = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }, - .public_key_x = 11, - .public_key_y = 12, - .result = 77, + .message = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }, + .public_key_x = 10, + .public_key_y = 11, + .result = 76, .signature = signature, }; acir_format constraint_system{ - .varnum = 82, + .varnum = 81, .public_inputs = {}, .logic_constraints = {}, .range_constraints = range_constraints, @@ -360,39 +360,39 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange) TEST_F(AcirFormatTests, TestVarKeccak) { HashInput input1; - input1.witness = 1; + input1.witness = 0; input1.num_bits = 8; HashInput input2; - input2.witness = 2; + input2.witness = 1; input2.num_bits = 8; HashInput input3; - input3.witness = 3; + input3.witness = 2; input3.num_bits = 8; KeccakVarConstraint keccak; keccak.inputs = { input1, input2, input3 }; keccak.var_message_size = 4; - keccak.result = { 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, - 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36 }; + keccak.result = { 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35 }; RangeConstraint range_a{ - .witness = 1, + .witness = 0, .num_bits = 8, }; RangeConstraint range_b{ - .witness = 2, + .witness = 1, .num_bits = 8, }; RangeConstraint range_c{ - .witness = 3, + .witness = 2, .num_bits = 8, }; RangeConstraint range_d{ - .witness = 4, + .witness = 3, .num_bits = 8, }; auto dummy = poly_triple{ - .a = 1, + .a = 0, .b = 0, .c = 0, .q_m = 0, @@ -403,7 +403,7 @@ TEST_F(AcirFormatTests, TestVarKeccak) }; acir_format constraint_system{ - .varnum = 37, + .varnum = 36, .public_inputs = {}, .logic_constraints = {}, .range_constraints = { range_a, range_b, range_c, range_d }, 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 4451cfb7956..526eabed481 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 @@ -14,13 +14,13 @@ class UltraPlonkRAM : public ::testing::Test { }; size_t generate_block_constraint(BlockConstraint& constraint, WitnessVector& witness_values) { - size_t witness_len = 1; + size_t witness_len = 0; witness_values.emplace_back(1); witness_len++; fr two = fr::one() + fr::one(); poly_triple a0 = poly_triple{ - .a = 1, + .a = 0, .b = 0, .c = 0, .q_m = 0, @@ -41,7 +41,7 @@ size_t generate_block_constraint(BlockConstraint& constraint, WitnessVector& wit .q_c = three, }; poly_triple r1 = poly_triple{ - .a = 1, + .a = 0, .b = 0, .c = 0, .q_m = 0, @@ -51,7 +51,7 @@ size_t generate_block_constraint(BlockConstraint& constraint, WitnessVector& wit .q_c = fr::neg_one(), }; poly_triple r2 = poly_triple{ - .a = 1, + .a = 0, .b = 0, .c = 0, .q_m = 0, @@ -61,7 +61,7 @@ size_t generate_block_constraint(BlockConstraint& constraint, WitnessVector& wit .q_c = fr::neg_one(), }; poly_triple y = poly_triple{ - .a = 2, + .a = 1, .b = 0, .c = 0, .q_m = 0, @@ -73,7 +73,7 @@ size_t generate_block_constraint(BlockConstraint& constraint, WitnessVector& wit witness_values.emplace_back(2); witness_len++; poly_triple z = poly_triple{ - .a = 3, + .a = 2, .b = 0, .c = 0, .q_m = 0, 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 c5ce1392780..be404915e22 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 @@ -40,7 +40,7 @@ size_t generate_ecdsa_constraint(EcdsaSecp256k1Constraint& ecdsa_constraint, Wit std::vector pub_x_indices_in; std::vector pub_y_indices_in; std::vector signature_in; - size_t offset = 1; + size_t offset = 0; for (size_t i = 0; i < hashed_message.size(); ++i) { message_in.emplace_back(i + offset); const auto byte = static_cast(hashed_message[i]); 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 0c9b56e37fe..a87876e8789 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 @@ -23,7 +23,7 @@ size_t generate_r1_constraints(EcdsaSecp256r1Constraint& ecdsa_r1_constraint, std::vector pub_x_indices_in; std::vector pub_y_indices_in; std::vector signature_in; - size_t offset = 1; + size_t offset = 0; for (size_t i = 0; i < hashed_message.size(); ++i) { message_in.emplace_back(i + offset); const auto byte = static_cast(hashed_message[i]); 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 17073459779..4e8e53ff327 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 @@ -24,24 +24,24 @@ Builder create_inner_circuit() * } **/ RangeConstraint range_a{ - .witness = 1, + .witness = 0, .num_bits = 32, }; RangeConstraint range_b{ - .witness = 2, + .witness = 1, .num_bits = 32, }; LogicConstraint logic_constraint{ - .a = 1, - .b = 2, - .result = 3, + .a = 0, + .b = 1, + .result = 2, .num_bits = 32, .is_xor_gate = 1, }; poly_triple expr_a{ - .a = 3, - .b = 4, + .a = 2, + .b = 3, .c = 0, .q_m = 0, .q_l = 1, @@ -50,9 +50,9 @@ Builder create_inner_circuit() .q_c = -10, }; poly_triple expr_b{ - .a = 4, - .b = 5, - .c = 6, + .a = 3, + .b = 4, + .c = 5, .q_m = 1, .q_l = 0, .q_r = 0, @@ -60,9 +60,9 @@ Builder create_inner_circuit() .q_c = 0, }; poly_triple expr_c{ - .a = 4, - .b = 6, - .c = 4, + .a = 3, + .b = 5, + .c = 3, .q_m = 1, .q_l = 0, .q_r = 0, @@ -71,7 +71,7 @@ Builder create_inner_circuit() }; poly_triple expr_d{ - .a = 6, + .a = 5, .b = 0, .c = 0, .q_m = 0, @@ -81,8 +81,8 @@ Builder create_inner_circuit() .q_c = 1, }; - acir_format constraint_system{ .varnum = 7, - .public_inputs = { 2, 3 }, + acir_format constraint_system{ .varnum = 6, + .public_inputs = { 1, 2 }, .logic_constraints = { logic_constraint }, .range_constraints = { range_a, range_b }, .sha256_constraints = {}, @@ -122,8 +122,7 @@ Builder create_outer_circuit(std::vector& inner_circuits) { std::vector recursion_constraints; - // witness count starts at 1 (Composer reserves 1st witness to be the zero-valued zero_idx) - size_t witness_offset = 1; + size_t witness_offset = 0; std::array output_aggregation_object; std::vector> witness; @@ -226,7 +225,7 @@ Builder create_outer_circuit(std::vector& inner_circuits) // then we could get a segmentation fault as `inner_public_inputs` was never filled with values. if (!has_nested_proof) { for (size_t i = 0; i < num_inner_public_inputs; ++i) { - witness[inner_public_inputs[i] - 1] = inner_public_input_values[i]; + witness[inner_public_inputs[i]] = inner_public_input_values[i]; } } @@ -234,7 +233,7 @@ Builder create_outer_circuit(std::vector& inner_circuits) circuit_idx++; } - acir_format constraint_system{ .varnum = static_cast(witness.size() + 1), + acir_format constraint_system{ .varnum = static_cast(witness.size()), .public_inputs = {}, .logic_constraints = {}, .range_constraints = {}, From 1f41adabf4960bb3166cb6eedb0c6d79613f69c7 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 12 Jan 2024 01:21:48 +0000 Subject: [PATCH 23/28] fix goblin acir tests --- .../dsl/acir_proofs/acir_composer.cpp | 16 +++------------- .../goblin_ultra_circuit_builder.hpp | 16 +--------------- 2 files changed, 4 insertions(+), 28 deletions(-) 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 5eb044f1491..d1a83f29a6b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp @@ -82,20 +82,10 @@ 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) { - // 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) { - corrected_public_inputs.emplace_back(index - pre_applied_noir_offset); - } - // 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 }; + goblin_builder_ = acir_format::GoblinBuilder{ + goblin.op_queue, witness, constraint_system.public_inputs, constraint_system.varnum + }; // Populate constraints in the builder via the data in constraint_system acir_format::build_constraints(goblin_builder_, constraint_system, true); 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 ac6d25c2603..0a5d9a2f610 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,23 +95,9 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui auto& witness_values, std::vector& public_inputs, size_t varnum) - : UltraCircuitBuilder_>() + : UltraCircuitBuilder_>(/*size_hint=*/0, witness_values, public_inputs, varnum) , op_queue(op_queue_in) { - // 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. - 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; - // Set indices to constants corresponding to Goblin ECC op codes set_goblin_ecc_op_code_constant_variables(); }; From be90b2de62b38161b20c7b93852e420a427147f4 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Fri, 12 Jan 2024 15:48:06 +0000 Subject: [PATCH 24/28] reduce var_message_size in TestVarKeccak --- .../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 99c25d3e8d6..2aa6557b03f 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 @@ -370,7 +370,7 @@ TEST_F(AcirFormatTests, TestVarKeccak) input3.num_bits = 8; KeccakVarConstraint keccak; keccak.inputs = { input1, input2, input3 }; - keccak.var_message_size = 4; + keccak.var_message_size = 3; keccak.result = { 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35 }; From 30bdf0ef28cbc0ba708eb930d3e67b5c675a1bb6 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 12 Jan 2024 16:39:48 +0000 Subject: [PATCH 25/28] comments update --- .../dsl/acir_format/acir_to_constraint_buf.hpp | 16 ++++++++-------- .../barretenberg/dsl/acir_format/serde/acir.hpp | 3 --- 2 files changed, 8 insertions(+), 11 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 a1c87796979..d1ebbb6b89f 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 @@ -29,12 +29,12 @@ namespace acir_format { */ 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). + // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): The initialization of the witness indices a,b,c + // to 0 is implicitly assuming that (builder.zero_idx == 0) which is no longer the case. Now, witness idx 0 in + // general will correspond to some non-zero value and some witnesses which are not explicitly set below will be + // erroneously populated with this value. This does not cause failures however because the corresponding selector + // will indeed be 0 so the gate will be satisfied. Still, its 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, @@ -72,8 +72,8 @@ poly_triple serialize_arithmetic_gate(Circuit::Expression const& arg) // 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. + // check (and the others like it) since we initialize a,b,c with 0 but 0 is a valid witness index once 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; 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 c1273c30b90..52e4d5a0b55 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -242,9 +242,6 @@ struct BlockId { // 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 { - // WORKTODO: should be this: - // std::tuple mul_term; - // WORKTODO: also, why is this string instead of uint256 or field or something std::vector> mul_terms; std::vector> linear_combinations; std::string q_c; From 12c6ba8662e5c54d19bea3ae438a8bcac8e9967a Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 13 Jan 2024 22:23:46 +0000 Subject: [PATCH 26/28] fix: remove offset from `acir-simulator` --- .../acir-simulator/src/client/unconstrained_execution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts index d197ec386fb..c59fe74c405 100644 --- a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts @@ -26,7 +26,7 @@ export async function executeUnconstrainedFunction( log(`Executing unconstrained function ${contractAddress}:${functionSelector}`); const acir = Buffer.from(artifact.bytecode, 'base64'); - const initialWitness = toACVMWitness(1, args); + const initialWitness = toACVMWitness(0, args); const { partialWitness } = await acvm( await AcirSimulator.getSolver(), acir, From da8eab0be29032e6098e2a307e457e3f65403eef Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 13 Jan 2024 22:24:41 +0000 Subject: [PATCH 27/28] fix: remove other offset --- .../acir-simulator/src/client/client_execution_context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/acir-simulator/src/client/client_execution_context.ts b/yarn-project/acir-simulator/src/client/client_execution_context.ts index 5df69a7037e..062cfdf4def 100644 --- a/yarn-project/acir-simulator/src/client/client_execution_context.ts +++ b/yarn-project/acir-simulator/src/client/client_execution_context.ts @@ -106,7 +106,7 @@ export class ClientExecutionContext extends ViewDataOracle { ...args, ]; - return toACVMWitness(1, fields); + return toACVMWitness(0, fields); } /** From 10b03a5033eba3d8067dd3f4e0a11df934e7c654 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 13 Jan 2024 22:40:47 +0000 Subject: [PATCH 28/28] fix: fix final offset --- .../acir-simulator/src/public/public_execution_context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/acir-simulator/src/public/public_execution_context.ts b/yarn-project/acir-simulator/src/public/public_execution_context.ts index 75942e7d93c..886c0c40434 100644 --- a/yarn-project/acir-simulator/src/public/public_execution_context.ts +++ b/yarn-project/acir-simulator/src/public/public_execution_context.ts @@ -53,7 +53,7 @@ export class PublicExecutionContext extends TypedOracle { * @param witnessStartIndex - The index where to start inserting the parameters. * @returns The initial witness. */ - public getInitialWitness(witnessStartIndex = 1) { + public getInitialWitness(witnessStartIndex = 0) { const { callContext, args } = this.execution; const fields = [ ...toACVMCallContext(callContext),