From c2ccaea3b510719a038b2d3fb49e401f51468661 Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 29 Jul 2024 17:02:22 +0000 Subject: [PATCH 1/6] ensure dummy values are on the curve for MSM --- .../dsl/acir_format/acir_format.cpp | 2 +- .../dsl/acir_format/multi_scalar_mul.cpp | 43 ++++++++++++------- .../dsl/acir_format/multi_scalar_mul.hpp | 5 ++- 3 files changed, 32 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 c3c1b0c60db..8fb66b1b522 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -176,7 +176,7 @@ void build_constraints(Builder& builder, // Add multi scalar mul constraints for (size_t i = 0; i < constraint_system.multi_scalar_mul_constraints.size(); ++i) { const auto& constraint = constraint_system.multi_scalar_mul_constraints.at(i); - create_multi_scalar_mul_constraint(builder, constraint); + create_multi_scalar_mul_constraint(builder, constraint, has_valid_witness_assignments); track_gate_diff(constraint_system.gates_per_opcode, constraint_system.original_opcode_indices.multi_scalar_mul_constraints.at(i)); } diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp index 398c794dea1..e6e943f793a 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp @@ -10,7 +10,10 @@ namespace acir_format { using namespace bb; -template void create_multi_scalar_mul_constraint(Builder& builder, const MultiScalarMul& input) +template +void create_multi_scalar_mul_constraint(Builder& builder, + const MultiScalarMul& input, + bool has_valid_witness_assignments) { using cycle_group_ct = stdlib::cycle_group; using cycle_scalar_ct = typename stdlib::cycle_group::cycle_scalar; @@ -25,21 +28,27 @@ template void create_multi_scalar_mul_constraint(Builder& bui field_ct point_x; field_ct point_y; bool_ct infinite; - if (input.points[i].is_constant) { - point_x = field_ct(input.points[i].value); - } else { - point_x = field_ct::from_witness_index(&builder, input.points[i].index); - } - if (input.points[i + 1].is_constant) { - point_y = field_ct(input.points[i + 1].value); - } else { - point_y = field_ct::from_witness_index(&builder, input.points[i + 1].index); - } - if (input.points[i + 2].is_constant) { - infinite = bool_ct(field_ct(input.points[i + 2].value)); + + if (!has_valid_witness_assignments) { + infinite = bool_ct(true); } else { - infinite = bool_ct(field_ct::from_witness_index(&builder, input.points[i + 2].index)); + if (input.points[i].is_constant) { + point_x = field_ct(input.points[i].value); + } else { + point_x = field_ct::from_witness_index(&builder, input.points[i].index); + } + if (input.points[i + 1].is_constant) { + point_y = field_ct(input.points[i + 1].value); + } else { + point_y = field_ct::from_witness_index(&builder, input.points[i + 1].index); + } + if (input.points[i + 2].is_constant) { + infinite = bool_ct(field_ct(input.points[i + 2].value)); + } else { + infinite = bool_ct(field_ct::from_witness_index(&builder, input.points[i + 2].index)); + } } + cycle_group_ct input_point(point_x, point_y, infinite); // Reconstruct the scalar from the low and high limbs field_ct scalar_low_as_field; @@ -81,8 +90,10 @@ template void create_multi_scalar_mul_constraint(Builder& bui } template void create_multi_scalar_mul_constraint(UltraCircuitBuilder& builder, - const MultiScalarMul& input); + const MultiScalarMul& input, + bool has_valid_witness_assignments); template void create_multi_scalar_mul_constraint(MegaCircuitBuilder& builder, - const MultiScalarMul& input); + const MultiScalarMul& input, + bool has_valid_witness_assignments); } // namespace acir_format diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.hpp index ef980a99ab6..849f9df9528 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.hpp @@ -28,6 +28,9 @@ struct MultiScalarMul { friend bool operator==(MultiScalarMul const& lhs, MultiScalarMul const& rhs) = default; }; -template void create_multi_scalar_mul_constraint(Builder& builder, const MultiScalarMul& input); +template +void create_multi_scalar_mul_constraint(Builder& builder, + const MultiScalarMul& input, + bool has_valid_witness_assignments); } // namespace acir_format From 2d1eade8a7820433e434d3488034ca04c552c2b5 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 30 Jul 2024 13:16:03 +0000 Subject: [PATCH 2/6] use same constant/witness nature when no witness assignement --- .../dsl/acir_format/multi_scalar_mul.cpp | 34 ++--- .../dsl/acir_format/multi_scalar_mul.test.cpp | 116 ++++++++++++++++++ 2 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp index e6e943f793a..6e3e3487bca 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp @@ -29,24 +29,24 @@ void create_multi_scalar_mul_constraint(Builder& builder, field_ct point_y; bool_ct infinite; - if (!has_valid_witness_assignments) { - infinite = bool_ct(true); + if (input.points[i].is_constant) { + point_x = field_ct(input.points[i].value); } else { - if (input.points[i].is_constant) { - point_x = field_ct(input.points[i].value); - } else { - point_x = field_ct::from_witness_index(&builder, input.points[i].index); - } - if (input.points[i + 1].is_constant) { - point_y = field_ct(input.points[i + 1].value); - } else { - point_y = field_ct::from_witness_index(&builder, input.points[i + 1].index); - } - if (input.points[i + 2].is_constant) { - infinite = bool_ct(field_ct(input.points[i + 2].value)); - } else { - infinite = bool_ct(field_ct::from_witness_index(&builder, input.points[i + 2].index)); - } + point_x = field_ct::from_witness_index(&builder, input.points[i].index); + } + if (input.points[i + 1].is_constant) { + point_y = field_ct(input.points[i + 1].value); + } else { + point_y = field_ct::from_witness_index(&builder, input.points[i + 1].index); + } + if (input.points[i + 2].is_constant) { + infinite = bool_ct(field_ct(input.points[i + 2].value)); + } else { + infinite = bool_ct(field_ct::from_witness_index(&builder, input.points[i + 2].index)); + } + + if (!has_valid_witness_assignments && !input.points[i + 2].is_constant) { + builder.variables[input.points[i + 2].index] = fr(1); } cycle_group_ct input_point(point_x, point_y, infinite); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp new file mode 100644 index 00000000000..4235226282d --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp @@ -0,0 +1,116 @@ +#include "acir_format.hpp" +#include "acir_format_mocks.hpp" +#include "acir_to_constraint_buf.hpp" +#include "barretenberg/numeric/uint256/uint256.hpp" +#include "barretenberg/plonk/composer/ultra_composer.hpp" +#include "barretenberg/plonk/proof_system/types/proof.hpp" +#include "barretenberg/plonk/proof_system/verification_key/verification_key.hpp" +#include "poseidon2_constraint.hpp" + +#include +#include +#include + +namespace acir_format::tests { + +using namespace bb; +using Composer = plonk::UltraComposer; + +class MSMTests : public ::testing::Test { + protected: + static void SetUpTestSuite() { srs::init_crs_factory("../srs_db/ignition"); } +}; +using fr = field; + +/** + * @brief Create a circuit testing the a simple scalar mul with a constant generator + * + */ +TEST_F(MSMTests, TestMSM) +{ + MultiScalarMul msm_constrain{ + .points = { WitnessConstant{ + .index = 0, + .value = fr(1), + .is_constant = true, + }, + WitnessConstant{ + .index = 0, + .value = fr("0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c"), + .is_constant = true, + }, + WitnessConstant{ + .index = 0, + .value = fr(0), + .is_constant = true, + } }, + .scalars = { WitnessConstant{ + .index = 0, + .value = fr(std::string("0x000000000000000000000000000000000000000000000000000000616c696365")), + .is_constant = false, + }, + WitnessConstant{ + .index = 0, + .value = fr(0), + .is_constant = true, + } }, + + .out_point_x = 1, + .out_point_y = 2, + .out_point_is_infinite = 3, + }; + + AcirFormat constraint_system{ + .varnum = 9, + .recursive = false, + .num_acir_opcodes = 1, + .public_inputs = {}, + .logic_constraints = {}, + .range_constraints = {}, + .aes128_constraints = {}, + .sha256_constraints = {}, + .sha256_compression = {}, + .schnorr_constraints = {}, + .ecdsa_k1_constraints = {}, + .ecdsa_r1_constraints = {}, + .blake2s_constraints = {}, + .blake3_constraints = {}, + .keccak_constraints = {}, + .keccak_permutations = {}, + .pedersen_constraints = {}, + .pedersen_hash_constraints = {}, + .poseidon2_constraints = {}, + .multi_scalar_mul_constraints = { msm_constrain }, + .ec_add_constraints = {}, + .recursion_constraints = {}, + .honk_recursion_constraints = {}, + .bigint_from_le_bytes_constraints = {}, + .bigint_to_le_bytes_constraints = {}, + .bigint_operations = {}, + .poly_triple_constraints = {}, + .quad_constraints = {}, + .block_constraints = {}, + .original_opcode_indices = create_empty_original_opcode_indices(), + }; + mock_opcode_indices(constraint_system); + + WitnessVector witness{ + fr("0x000000000000000000000000000000000000000000000000000000616c696365"), + fr("0x0bff8247aa94b08d1c680d7a3e10831bd8c8cf2ea2c756b0d1d89acdcad877ad"), + fr("0x2a5d7253a6ed48462fedb2d350cc768d13956310f54e73a8a47914f34a34c5c4"), + fr(0), + }; + + auto builder = create_circuit(constraint_system, /*size_hint=*/0, witness); + auto composer = Composer(); + auto prover = composer.create_ultra_with_keccak_prover(builder); + auto proof = prover.construct_proof(); + + auto builder2 = create_circuit(constraint_system, /*size_hint=*/0, {}); + auto composer2 = Composer(); + auto verifier = composer2.create_ultra_with_keccak_verifier(builder2); + + EXPECT_EQ(verifier.verify_proof(proof), true); +} + +} // namespace acir_format::tests From 1e842832d663c18a8652d687183913b0b8d551b9 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 30 Jul 2024 13:23:18 +0000 Subject: [PATCH 3/6] fix merge from master --- .../dsl/acir_format/multi_scalar_mul.test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp index 4235226282d..7f59d142d9b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp @@ -1,3 +1,4 @@ +#include "multi_scalar_mul.hpp" #include "acir_format.hpp" #include "acir_format_mocks.hpp" #include "acir_to_constraint_buf.hpp" @@ -5,7 +6,6 @@ #include "barretenberg/plonk/composer/ultra_composer.hpp" #include "barretenberg/plonk/proof_system/types/proof.hpp" #include "barretenberg/plonk/proof_system/verification_key/verification_key.hpp" -#include "poseidon2_constraint.hpp" #include #include @@ -29,27 +29,27 @@ using fr = field; TEST_F(MSMTests, TestMSM) { MultiScalarMul msm_constrain{ - .points = { WitnessConstant{ + .points = { WitnessOrConstant{ .index = 0, .value = fr(1), .is_constant = true, }, - WitnessConstant{ + WitnessOrConstant{ .index = 0, .value = fr("0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c"), .is_constant = true, }, - WitnessConstant{ + WitnessOrConstant{ .index = 0, .value = fr(0), .is_constant = true, } }, - .scalars = { WitnessConstant{ + .scalars = { WitnessOrConstant{ .index = 0, .value = fr(std::string("0x000000000000000000000000000000000000000000000000000000616c696365")), .is_constant = false, }, - WitnessConstant{ + WitnessOrConstant{ .index = 0, .value = fr(0), .is_constant = true, From e68d47c82ded4284c4ce11077af63fef07c07124 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 30 Jul 2024 13:27:49 +0000 Subject: [PATCH 4/6] handle also the constant case --- .../src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp index b388c3c1b38..205643a8a2d 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp @@ -34,7 +34,11 @@ void create_multi_scalar_mul_constraint(Builder& builder, infinite = bool_ct(to_field_ct(input.points[i + 2], builder)); if (!has_valid_witness_assignments && !input.points[i + 2].is_constant) { - builder.variables[input.points[i + 2].index] = fr(1); + if (input.points[i + 2].is_constant) { + infinite = bool_ct(field_ct(fr(1))); + } else { + builder.variables[input.points[i + 2].index] = fr(1); + } } cycle_group_ct input_point(point_x, point_y, infinite); From 2e362e1eb473f1b5efdad49d517e8cf32b363be8 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 30 Jul 2024 14:10:44 +0000 Subject: [PATCH 5/6] revert previous change, as we should not change a constant value. --- .../src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp index 205643a8a2d..b388c3c1b38 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp @@ -34,11 +34,7 @@ void create_multi_scalar_mul_constraint(Builder& builder, infinite = bool_ct(to_field_ct(input.points[i + 2], builder)); if (!has_valid_witness_assignments && !input.points[i + 2].is_constant) { - if (input.points[i + 2].is_constant) { - infinite = bool_ct(field_ct(fr(1))); - } else { - builder.variables[input.points[i + 2].index] = fr(1); - } + builder.variables[input.points[i + 2].index] = fr(1); } cycle_group_ct input_point(point_x, point_y, infinite); From 0c0671b47ec1a777b91c1c040ba976a1f025c1a0 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 30 Jul 2024 15:55:07 +0000 Subject: [PATCH 6/6] handle one more case: infinite is constant but coordinates are witness --- .../dsl/acir_format/multi_scalar_mul.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp index b388c3c1b38..776baf67227 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp @@ -33,8 +33,19 @@ void create_multi_scalar_mul_constraint(Builder& builder, point_y = to_field_ct(input.points[i + 1], builder); infinite = bool_ct(to_field_ct(input.points[i + 2], builder)); - if (!has_valid_witness_assignments && !input.points[i + 2].is_constant) { - builder.variables[input.points[i + 2].index] = fr(1); + // When we do not have the witness assignments, we set is_infinite value to true if it is not constant + // else default values would give a point which is not on the curve and this will fail verification + if (!has_valid_witness_assignments) { + if (!input.points[i + 2].is_constant) { + builder.variables[input.points[i + 2].index] = fr(1); + } else if (input.points[i + 2].value == fr::zero() && + !(input.points[i].is_constant || input.points[i + 1].is_constant)) { + // else, if is_infinite is false, but the coordinates (x, y) are witness + // then we set their value so to a curve point. + auto g1 = bb::grumpkin::g1::affine_one; + builder.variables[input.points[i].index] = g1.x; + builder.variables[input.points[i + 1].index] = g1.y; + } } cycle_group_ct input_point(point_x, point_y, infinite);