Skip to content

Commit

Permalink
fix: ensure dummy values are on the curve for MSM (#7653)
Browse files Browse the repository at this point in the history
This PR fix an issue with MSM when trying to verify a circuit having MSM
blackbox, because dummy values are not on the curve.

I also added a BB unit test which ensure non-constant inputs are
properly handled in the 'no witness assignment' case
  • Loading branch information
guipublic authored Jul 30, 2024
1 parent dd6176b commit 11f3885
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ namespace acir_format {

using namespace bb;

template <typename Builder> void create_multi_scalar_mul_constraint(Builder& builder, const MultiScalarMul& input)
template <typename Builder>
void create_multi_scalar_mul_constraint(Builder& builder,
const MultiScalarMul& input,
bool has_valid_witness_assignments)
{
using cycle_group_ct = stdlib::cycle_group<Builder>;
using cycle_scalar_ct = typename stdlib::cycle_group<Builder>::cycle_scalar;
Expand All @@ -25,9 +28,26 @@ template <typename Builder> void create_multi_scalar_mul_constraint(Builder& bui
field_ct point_x;
field_ct point_y;
bool_ct infinite;

point_x = to_field_ct(input.points[i], builder);
point_y = to_field_ct(input.points[i + 1], builder);
infinite = bool_ct(to_field_ct(input.points[i + 2], builder));

// 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);
// Reconstruct the scalar from the low and high limbs
field_ct scalar_low_as_field;
Expand Down Expand Up @@ -69,8 +89,10 @@ template <typename Builder> void create_multi_scalar_mul_constraint(Builder& bui
}

template void create_multi_scalar_mul_constraint<UltraCircuitBuilder>(UltraCircuitBuilder& builder,
const MultiScalarMul& input);
const MultiScalarMul& input,
bool has_valid_witness_assignments);
template void create_multi_scalar_mul_constraint<MegaCircuitBuilder>(MegaCircuitBuilder& builder,
const MultiScalarMul& input);
const MultiScalarMul& input,
bool has_valid_witness_assignments);

} // namespace acir_format
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ struct MultiScalarMul {
friend bool operator==(MultiScalarMul const& lhs, MultiScalarMul const& rhs) = default;
};

template <typename Builder> void create_multi_scalar_mul_constraint(Builder& builder, const MultiScalarMul& input);
template <typename Builder>
void create_multi_scalar_mul_constraint(Builder& builder,
const MultiScalarMul& input,
bool has_valid_witness_assignments);

} // namespace acir_format
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
#include "multi_scalar_mul.hpp"
#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 <cstdint>
#include <gtest/gtest.h>
#include <vector>

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<Bn254FrParams>;

/**
* @brief Create a circuit testing the a simple scalar mul with a constant generator
*
*/
TEST_F(MSMTests, TestMSM)
{
MultiScalarMul msm_constrain{
.points = { WitnessOrConstant<fr>{
.index = 0,
.value = fr(1),
.is_constant = true,
},
WitnessOrConstant<fr>{
.index = 0,
.value = fr("0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c"),
.is_constant = true,
},
WitnessOrConstant<fr>{
.index = 0,
.value = fr(0),
.is_constant = true,
} },
.scalars = { WitnessOrConstant<fr>{
.index = 0,
.value = fr(std::string("0x000000000000000000000000000000000000000000000000000000616c696365")),
.is_constant = false,
},
WitnessOrConstant<fr>{
.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

0 comments on commit 11f3885

Please sign in to comment.