diff --git a/.noir-sync-commit b/.noir-sync-commit index e7c73939ac6..bec736647ac 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -b541e793e20fa3c991e0328ec2ff7926bdcdfd45 +a5b7df12faf9d71ff24f8c5cde5e78da44558caf 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 3e77b60d689..d51797034dc 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 @@ -342,15 +342,19 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg, Aci .scalars = map(arg.scalars, [](auto& e) { return e.witness.value; }), .out_point_x = arg.outputs[0].value, .out_point_y = arg.outputs[1].value, + .out_point_is_infinite = arg.outputs[2].value, }); } else if constexpr (std::is_same_v) { af.ec_add_constraints.push_back(EcAdd{ - .input1_x = arg.input1_x.witness.value, - .input1_y = arg.input1_y.witness.value, - .input2_x = arg.input2_x.witness.value, - .input2_y = arg.input2_y.witness.value, + .input1_x = arg.input1[0].witness.value, + .input1_y = arg.input1[1].witness.value, + .input1_infinite = arg.input1[2].witness.value, + .input2_x = arg.input2[0].witness.value, + .input2_y = arg.input2[1].witness.value, + .input2_infinite = arg.input2[2].witness.value, .result_x = arg.outputs[0].value, .result_y = arg.outputs[1].value, + .result_infinite = arg.outputs[2].value, }); } else if constexpr (std::is_same_v) { af.keccak_constraints.push_back(KeccakConstraint{ diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp index 15494fa871a..4894537b659 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp @@ -13,30 +13,35 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val // Input to cycle_group points using cycle_group_ct = bb::stdlib::cycle_group; using field_ct = bb::stdlib::field_t; + using bool_ct = bb::stdlib::bool_t; auto x1 = field_ct::from_witness_index(&builder, input.input1_x); auto y1 = field_ct::from_witness_index(&builder, input.input1_y); auto x2 = field_ct::from_witness_index(&builder, input.input2_x); auto y2 = field_ct::from_witness_index(&builder, input.input2_y); + auto infinite1 = bool_ct(field_ct::from_witness_index(&builder, input.input1_infinite)); + auto infinite2 = bool_ct(field_ct::from_witness_index(&builder, input.input2_infinite)); if (!has_valid_witness_assignments) { auto g1 = grumpkin::g1::affine_one; // We need to have correct values representing points on the curve builder.variables[input.input1_x] = g1.x; builder.variables[input.input1_y] = g1.y; + builder.variables[input.input1_infinite] = fr(0); builder.variables[input.input2_x] = g1.x; builder.variables[input.input2_y] = g1.y; + builder.variables[input.input2_infinite] = fr(0); } - - cycle_group_ct input1_point(x1, y1, false); - cycle_group_ct input2_point(x2, y2, false); - + cycle_group_ct input1_point(x1, y1, infinite1); + cycle_group_ct input2_point(x2, y2, infinite2); // Addition cycle_group_ct result = input1_point + input2_point; auto x_normalized = result.x.normalize(); auto y_normalized = result.y.normalize(); + auto infinite = result.is_point_at_infinity().normalize(); builder.assert_equal(x_normalized.witness_index, input.result_x); builder.assert_equal(y_normalized.witness_index, input.result_y); + builder.assert_equal(infinite.witness_index, input.result_infinite); } template void create_ec_add_constraint(UltraCircuitBuilder& builder, diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.hpp index de28743d16a..5c16505bc90 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.hpp @@ -8,13 +8,17 @@ namespace acir_format { struct EcAdd { uint32_t input1_x; uint32_t input1_y; + uint32_t input1_infinite; uint32_t input2_x; uint32_t input2_y; + uint32_t input2_infinite; uint32_t result_x; uint32_t result_y; + uint32_t result_infinite; // for serialization, update with any new fields - MSGPACK_FIELDS(input1_x, input1_y, input2_x, input2_y, result_x, result_y); + MSGPACK_FIELDS( + input1_x, input1_y, input1_infinite, input2_x, input2_y, input2_infinite, result_x, result_y, result_infinite); friend bool operator==(EcAdd const& lhs, EcAdd const& rhs) = default; }; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp index 65be4aaae55..59333ddc0d4 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp @@ -30,13 +30,18 @@ size_t generate_ec_add_constraint(EcAdd& ec_add_constraint, WitnessVector& witne witness_values.push_back(g1.y); witness_values.push_back(result.x.get_value()); witness_values.push_back(result.y.get_value()); + witness_values.push_back(fr(0)); + witness_values.push_back(fr(0)); ec_add_constraint = EcAdd{ .input1_x = 1, .input1_y = 2, + .input1_infinite = 7, .input2_x = 3, .input2_y = 4, + .input2_infinite = 7, .result_x = 5, .result_y = 6, + .result_infinite = 8, }; return witness_values.size(); } @@ -85,6 +90,92 @@ TEST_F(EcOperations, TestECOperations) auto prover = composer.create_prover(builder); auto proof = prover.construct_proof(); + + EXPECT_TRUE(CircuitChecker::check(builder)); + auto verifier = composer.create_verifier(builder); + EXPECT_EQ(verifier.verify_proof(proof), true); +} + +TEST_F(EcOperations, TestECMultiScalarMul) +{ + MultiScalarMul msm_constrain; + + WitnessVector witness_values; + witness_values.emplace_back(fr(0)); + + witness_values = { + // dummy + fr(0), + // g1: x,y,infinite + fr(1), + fr("0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c"), + fr(0), + // low, high scalars + fr(1), + fr(0), + // result + fr("0x06ce1b0827aafa85ddeb49cdaa36306d19a74caa311e13d46d8bc688cdbffffe"), + fr("0x1c122f81a3a14964909ede0ba2a6855fc93faf6fa1a788bf467be7e7a43f80ac"), + fr(0), + }; + msm_constrain = MultiScalarMul{ + .points = { 1, 2, 3, 1, 2, 3 }, + .scalars = { 4, 5, 4, 5 }, + .out_point_x = 6, + .out_point_y = 7, + .out_point_is_infinite = 0, + }; + auto res_x = fr("0x06ce1b0827aafa85ddeb49cdaa36306d19a74caa311e13d46d8bc688cdbffffe"); + auto assert_equal = poly_triple{ + .a = 6, + .b = 0, + .c = 0, + .q_m = 0, + .q_l = fr::neg_one(), + .q_r = 0, + .q_o = 0, + .q_c = res_x, + }; + + size_t num_variables = witness_values.size(); + AcirFormat constraint_system{ + .varnum = static_cast(num_variables + 1), + .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 = {}, + .bigint_from_le_bytes_constraints = {}, + .bigint_to_le_bytes_constraints = {}, + .bigint_operations = {}, + .poly_triple_constraints = { assert_equal }, + .quad_constraints = {}, + .block_constraints = {}, + }; + + auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness_values); + + auto composer = Composer(); + auto prover = composer.create_prover(builder); + + auto proof = prover.construct_proof(); + EXPECT_TRUE(CircuitChecker::check(builder)); auto verifier = composer.create_verifier(builder); EXPECT_EQ(verifier.verify_proof(proof), true); 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 83354d97c76..83d7395bafa 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 @@ -12,19 +12,20 @@ template void create_multi_scalar_mul_constraint(Builder& bui using cycle_group_ct = bb::stdlib::cycle_group; using cycle_scalar_ct = typename bb::stdlib::cycle_group::cycle_scalar; using field_ct = bb::stdlib::field_t; + using bool_ct = bb::stdlib::bool_t; std::vector points; std::vector scalars; - for (size_t i = 0; i < input.points.size(); i += 2) { + for (size_t i = 0; i < input.points.size(); i += 3) { // Instantiate the input point/variable base as `cycle_group_ct` auto point_x = field_ct::from_witness_index(&builder, input.points[i]); auto point_y = field_ct::from_witness_index(&builder, input.points[i + 1]); - cycle_group_ct input_point(point_x, point_y, false); - + auto infinite = bool_ct(field_ct::from_witness_index(&builder, input.points[i + 2])); + 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 = field_ct::from_witness_index(&builder, input.scalars[i]); - field_ct scalar_high_as_field = field_ct::from_witness_index(&builder, input.scalars[i + 1]); + field_ct scalar_low_as_field = field_ct::from_witness_index(&builder, input.scalars[2 * (i / 3)]); + field_ct scalar_high_as_field = field_ct::from_witness_index(&builder, input.scalars[2 * (i / 3) + 1]); cycle_scalar_ct scalar(scalar_low_as_field, scalar_high_as_field); // Add the point and scalar to the vectors @@ -38,6 +39,7 @@ template void create_multi_scalar_mul_constraint(Builder& bui // Add the constraints builder.assert_equal(output_point.x.get_witness_index(), input.out_point_x); builder.assert_equal(output_point.y.get_witness_index(), input.out_point_y); + builder.assert_equal(output_point.is_point_at_infinity().witness_index, input.out_point_is_infinite); } template void create_multi_scalar_mul_constraint(UltraCircuitBuilder& builder, 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 12b070076f9..57c143226d3 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 @@ -10,9 +10,10 @@ struct MultiScalarMul { std::vector scalars; uint32_t out_point_x; uint32_t out_point_y; + uint32_t out_point_is_infinite; // for serialization, update with any new fields - MSGPACK_FIELDS(points, scalars, out_point_x, out_point_y); + MSGPACK_FIELDS(points, scalars, out_point_x, out_point_y, out_point_is_infinite); friend bool operator==(MultiScalarMul const& lhs, MultiScalarMul const& rhs) = default; }; 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 683e4c62407..2c8ad3cc5f8 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -149,7 +149,7 @@ struct BlackBoxFuncCall { struct MultiScalarMul { std::vector points; std::vector scalars; - std::array outputs; + std::array outputs; friend bool operator==(const MultiScalarMul&, const MultiScalarMul&); std::vector bincodeSerialize() const; @@ -157,11 +157,9 @@ struct BlackBoxFuncCall { }; struct EmbeddedCurveAdd { - Program::FunctionInput input1_x; - Program::FunctionInput input1_y; - Program::FunctionInput input2_x; - Program::FunctionInput input2_y; - std::array outputs; + std::array input1; + std::array input2; + std::array outputs; friend bool operator==(const EmbeddedCurveAdd&, const EmbeddedCurveAdd&); std::vector bincodeSerialize() const; @@ -807,8 +805,10 @@ struct BlackBoxOp { struct EmbeddedCurveAdd { Program::MemoryAddress input1_x; Program::MemoryAddress input1_y; + Program::MemoryAddress input1_infinite; Program::MemoryAddress input2_x; Program::MemoryAddress input2_y; + Program::MemoryAddress input2_infinite; Program::HeapArray result; friend bool operator==(const EmbeddedCurveAdd&, const EmbeddedCurveAdd&); @@ -3194,16 +3194,10 @@ namespace Program { inline bool operator==(const BlackBoxFuncCall::EmbeddedCurveAdd& lhs, const BlackBoxFuncCall::EmbeddedCurveAdd& rhs) { - if (!(lhs.input1_x == rhs.input1_x)) { + if (!(lhs.input1 == rhs.input1)) { return false; } - if (!(lhs.input1_y == rhs.input1_y)) { - return false; - } - if (!(lhs.input2_x == rhs.input2_x)) { - return false; - } - if (!(lhs.input2_y == rhs.input2_y)) { + if (!(lhs.input2 == rhs.input2)) { return false; } if (!(lhs.outputs == rhs.outputs)) { @@ -3237,10 +3231,8 @@ template void serde::Serializable::serialize( const Program::BlackBoxFuncCall::EmbeddedCurveAdd& obj, Serializer& serializer) { - serde::Serializable::serialize(obj.input1_x, serializer); - serde::Serializable::serialize(obj.input1_y, serializer); - serde::Serializable::serialize(obj.input2_x, serializer); - serde::Serializable::serialize(obj.input2_y, serializer); + serde::Serializable::serialize(obj.input1, serializer); + serde::Serializable::serialize(obj.input2, serializer); serde::Serializable::serialize(obj.outputs, serializer); } @@ -3250,10 +3242,8 @@ Program::BlackBoxFuncCall::EmbeddedCurveAdd serde::Deserializable< Program::BlackBoxFuncCall::EmbeddedCurveAdd>::deserialize(Deserializer& deserializer) { Program::BlackBoxFuncCall::EmbeddedCurveAdd obj; - obj.input1_x = serde::Deserializable::deserialize(deserializer); - obj.input1_y = serde::Deserializable::deserialize(deserializer); - obj.input2_x = serde::Deserializable::deserialize(deserializer); - obj.input2_y = serde::Deserializable::deserialize(deserializer); + obj.input1 = serde::Deserializable::deserialize(deserializer); + obj.input2 = serde::Deserializable::deserialize(deserializer); obj.outputs = serde::Deserializable::deserialize(deserializer); return obj; } @@ -4638,12 +4628,18 @@ inline bool operator==(const BlackBoxOp::EmbeddedCurveAdd& lhs, const BlackBoxOp if (!(lhs.input1_y == rhs.input1_y)) { return false; } + if (!(lhs.input1_infinite == rhs.input1_infinite)) { + return false; + } if (!(lhs.input2_x == rhs.input2_x)) { return false; } if (!(lhs.input2_y == rhs.input2_y)) { return false; } + if (!(lhs.input2_infinite == rhs.input2_infinite)) { + return false; + } if (!(lhs.result == rhs.result)) { return false; } @@ -4676,8 +4672,10 @@ void serde::Serializable::serialize( { serde::Serializable::serialize(obj.input1_x, serializer); serde::Serializable::serialize(obj.input1_y, serializer); + serde::Serializable::serialize(obj.input1_infinite, serializer); serde::Serializable::serialize(obj.input2_x, serializer); serde::Serializable::serialize(obj.input2_y, serializer); + serde::Serializable::serialize(obj.input2_infinite, serializer); serde::Serializable::serialize(obj.result, serializer); } @@ -4689,8 +4687,10 @@ Program::BlackBoxOp::EmbeddedCurveAdd serde::Deserializable::deserialize(deserializer); obj.input1_y = serde::Deserializable::deserialize(deserializer); + obj.input1_infinite = serde::Deserializable::deserialize(deserializer); obj.input2_x = serde::Deserializable::deserialize(deserializer); obj.input2_y = serde::Deserializable::deserialize(deserializer); + obj.input2_infinite = serde::Deserializable::deserialize(deserializer); obj.result = serde::Deserializable::deserialize(deserializer); return obj; } diff --git a/noir-projects/aztec-nr/aztec/src/keys/point_to_symmetric_key.nr b/noir-projects/aztec-nr/aztec/src/keys/point_to_symmetric_key.nr index e2f0edfcd70..934306e32ab 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/point_to_symmetric_key.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/point_to_symmetric_key.nr @@ -2,12 +2,15 @@ use dep::protocol_types::{ constants::GENERATOR_INDEX__SYMMETRIC_KEY, grumpkin_private_key::GrumpkinPrivateKey, grumpkin_point::GrumpkinPoint, utils::arr_copy_slice }; -use dep::std::{hash::sha256, embedded_curve_ops::multi_scalar_mul}; +use dep::std::{hash::sha256, embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar, multi_scalar_mul}}; // TODO(#5726): This function is called deriveAESSecret in TS. I don't like point_to_symmetric_key name much since // point is not the only input of the function. Unify naming with TS once we have a better name. pub fn point_to_symmetric_key(secret: GrumpkinPrivateKey, point: GrumpkinPoint) -> [u8; 32] { - let shared_secret_fields = multi_scalar_mul([point.x, point.y], [secret.low, secret.high]); + let shared_secret_fields = multi_scalar_mul( + [EmbeddedCurvePoint { x: point.x, y: point.y, is_infinite: false }], + [EmbeddedCurveScalar { lo: secret.low, hi: secret.high }] + ); // TODO(https://github.com/AztecProtocol/aztec-packages/issues/6061): make the func return Point struct directly let shared_secret = GrumpkinPoint::new(shared_secret_fields[0], shared_secret_fields[1]); let mut shared_secret_bytes_with_separator = [0 as u8; 65]; diff --git a/noir-projects/aztec-nr/value-note/src/utils.nr b/noir-projects/aztec-nr/value-note/src/utils.nr index 4c4ba85021c..466cf660d3f 100644 --- a/noir-projects/aztec-nr/value-note/src/utils.nr +++ b/noir-projects/aztec-nr/value-note/src/utils.nr @@ -47,10 +47,23 @@ pub fn decrement_by_at_most( let options = create_note_getter_options_for_decreasing_balance(max_amount); let opt_notes = balance.get_notes(options); + let owner_npk_m_hash = get_npk_m_hash(balance.context.private.unwrap(), owner); + let mut decremented = 0; for i in 0..opt_notes.len() { if opt_notes[i].is_some() { - decremented += destroy_note(balance, owner, opt_notes[i].unwrap_unchecked()); + let note = opt_notes[i].unwrap_unchecked(); + + // This is similar to destroy_note, except we only compute the owner_npk_m_hash once instead of doing it in + // each loop iteration. + + // Ensure the note is actually owned by the owner (to prevent user from generating a valid proof while + // spending someone else's notes). + // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this after rotating keys. + assert(note.npk_m_hash.eq(owner_npk_m_hash)); + decremented += note.value; + + balance.remove(note); } } diff --git a/noir/noir-repo/.tokeignore b/noir/noir-repo/.tokeignore new file mode 100644 index 00000000000..55f24e41dbd --- /dev/null +++ b/noir/noir-repo/.tokeignore @@ -0,0 +1,12 @@ +docs +scripts + +# aztec_macros is explicitly considered OOS for Noir audit +aztec_macros + +# config files +*.toml +*.md +*.json +*.txt +*.config.mjs diff --git a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp index 222a7da6399..47e184a6332 100644 --- a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp +++ b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp @@ -149,7 +149,7 @@ namespace Program { struct MultiScalarMul { std::vector points; std::vector scalars; - std::array outputs; + std::array outputs; friend bool operator==(const MultiScalarMul&, const MultiScalarMul&); std::vector bincodeSerialize() const; @@ -157,11 +157,9 @@ namespace Program { }; struct EmbeddedCurveAdd { - Program::FunctionInput input1_x; - Program::FunctionInput input1_y; - Program::FunctionInput input2_x; - Program::FunctionInput input2_y; - std::array outputs; + std::array input1; + std::array input2; + std::array outputs; friend bool operator==(const EmbeddedCurveAdd&, const EmbeddedCurveAdd&); std::vector bincodeSerialize() const; @@ -782,8 +780,10 @@ namespace Program { struct EmbeddedCurveAdd { Program::MemoryAddress input1_x; Program::MemoryAddress input1_y; + Program::MemoryAddress input1_infinite; Program::MemoryAddress input2_x; Program::MemoryAddress input2_y; + Program::MemoryAddress input2_infinite; Program::HeapArray result; friend bool operator==(const EmbeddedCurveAdd&, const EmbeddedCurveAdd&); @@ -2800,10 +2800,8 @@ Program::BlackBoxFuncCall::MultiScalarMul serde::Deserializable template void serde::Serializable::serialize(const Program::BlackBoxFuncCall::EmbeddedCurveAdd &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.input1_x, serializer); - serde::Serializable::serialize(obj.input1_y, serializer); - serde::Serializable::serialize(obj.input2_x, serializer); - serde::Serializable::serialize(obj.input2_y, serializer); + serde::Serializable::serialize(obj.input1, serializer); + serde::Serializable::serialize(obj.input2, serializer); serde::Serializable::serialize(obj.outputs, serializer); } @@ -2839,10 +2835,8 @@ template <> template Program::BlackBoxFuncCall::EmbeddedCurveAdd serde::Deserializable::deserialize(Deserializer &deserializer) { Program::BlackBoxFuncCall::EmbeddedCurveAdd obj; - obj.input1_x = serde::Deserializable::deserialize(deserializer); - obj.input1_y = serde::Deserializable::deserialize(deserializer); - obj.input2_x = serde::Deserializable::deserialize(deserializer); - obj.input2_y = serde::Deserializable::deserialize(deserializer); + obj.input1 = serde::Deserializable::deserialize(deserializer); + obj.input2 = serde::Deserializable::deserialize(deserializer); obj.outputs = serde::Deserializable::deserialize(deserializer); return obj; } @@ -3909,8 +3903,10 @@ namespace Program { inline bool operator==(const BlackBoxOp::EmbeddedCurveAdd &lhs, const BlackBoxOp::EmbeddedCurveAdd &rhs) { if (!(lhs.input1_x == rhs.input1_x)) { return false; } if (!(lhs.input1_y == rhs.input1_y)) { return false; } + if (!(lhs.input1_infinite == rhs.input1_infinite)) { return false; } if (!(lhs.input2_x == rhs.input2_x)) { return false; } if (!(lhs.input2_y == rhs.input2_y)) { return false; } + if (!(lhs.input2_infinite == rhs.input2_infinite)) { return false; } if (!(lhs.result == rhs.result)) { return false; } return true; } @@ -3937,8 +3933,10 @@ template void serde::Serializable::serialize(const Program::BlackBoxOp::EmbeddedCurveAdd &obj, Serializer &serializer) { serde::Serializable::serialize(obj.input1_x, serializer); serde::Serializable::serialize(obj.input1_y, serializer); + serde::Serializable::serialize(obj.input1_infinite, serializer); serde::Serializable::serialize(obj.input2_x, serializer); serde::Serializable::serialize(obj.input2_y, serializer); + serde::Serializable::serialize(obj.input2_infinite, serializer); serde::Serializable::serialize(obj.result, serializer); } @@ -3948,8 +3946,10 @@ Program::BlackBoxOp::EmbeddedCurveAdd serde::Deserializable::deserialize(deserializer); obj.input1_y = serde::Deserializable::deserialize(deserializer); + obj.input1_infinite = serde::Deserializable::deserialize(deserializer); obj.input2_x = serde::Deserializable::deserialize(deserializer); obj.input2_y = serde::Deserializable::deserialize(deserializer); + obj.input2_infinite = serde::Deserializable::deserialize(deserializer); obj.result = serde::Deserializable::deserialize(deserializer); return obj; } diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 115a33c1c9d..b0e77b15c2c 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -89,14 +89,12 @@ pub enum BlackBoxFuncCall { MultiScalarMul { points: Vec, scalars: Vec, - outputs: (Witness, Witness), + outputs: (Witness, Witness, Witness), }, EmbeddedCurveAdd { - input1_x: FunctionInput, - input1_y: FunctionInput, - input2_x: FunctionInput, - input2_y: FunctionInput, - outputs: (Witness, Witness), + input1: Box<[FunctionInput; 3]>, + input2: Box<[FunctionInput; 3]>, + outputs: (Witness, Witness, Witness), }, Keccak256 { inputs: Vec, @@ -245,9 +243,9 @@ impl BlackBoxFuncCall { inputs.extend(scalars.iter().copied()); inputs } - BlackBoxFuncCall::EmbeddedCurveAdd { - input1_x, input1_y, input2_x, input2_y, .. - } => vec![*input1_x, *input1_y, *input2_x, *input2_y], + BlackBoxFuncCall::EmbeddedCurveAdd { input1, input2, .. } => { + vec![input1[0], input1[1], input2[0], input2[1]] + } BlackBoxFuncCall::RANGE { input } => vec![*input], BlackBoxFuncCall::SchnorrVerify { public_key_x, @@ -343,9 +341,11 @@ impl BlackBoxFuncCall { | BlackBoxFuncCall::EcdsaSecp256k1 { output, .. } | BlackBoxFuncCall::PedersenHash { output, .. } | BlackBoxFuncCall::EcdsaSecp256r1 { output, .. } => vec![*output], + BlackBoxFuncCall::PedersenCommitment { outputs, .. } => vec![outputs.0, outputs.1], BlackBoxFuncCall::MultiScalarMul { outputs, .. } - | BlackBoxFuncCall::PedersenCommitment { outputs, .. } - | BlackBoxFuncCall::EmbeddedCurveAdd { outputs, .. } => vec![outputs.0, outputs.1], + | BlackBoxFuncCall::EmbeddedCurveAdd { outputs, .. } => { + vec![outputs.0, outputs.1, outputs.2] + } BlackBoxFuncCall::RANGE { .. } | BlackBoxFuncCall::RecursiveAggregation { .. } | BlackBoxFuncCall::BigIntFromLeBytes { .. } diff --git a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs index ecc1a26e3a4..19e4beb6158 100644 --- a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs +++ b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs @@ -63,19 +63,26 @@ fn multi_scalar_mul_circuit() { points: vec![ FunctionInput { witness: Witness(1), num_bits: 128 }, FunctionInput { witness: Witness(2), num_bits: 128 }, + FunctionInput { witness: Witness(3), num_bits: 1 }, ], scalars: vec![ - FunctionInput { witness: Witness(3), num_bits: 128 }, FunctionInput { witness: Witness(4), num_bits: 128 }, + FunctionInput { witness: Witness(5), num_bits: 128 }, ], - outputs: (Witness(5), Witness(6)), + outputs: (Witness(6), Witness(7), Witness(8)), }); let circuit = Circuit { - current_witness_index: 7, + current_witness_index: 9, opcodes: vec![multi_scalar_mul], - private_parameters: BTreeSet::from([Witness(1), Witness(2), Witness(3), Witness(4)]), - return_values: PublicInputs(BTreeSet::from_iter(vec![Witness(5), Witness(6)])), + private_parameters: BTreeSet::from([ + Witness(1), + Witness(2), + Witness(3), + Witness(4), + Witness(5), + ]), + return_values: PublicInputs(BTreeSet::from_iter(vec![Witness(6), Witness(7), Witness(8)])), ..Circuit::default() }; let program = Program { functions: vec![circuit], unconstrained_functions: vec![] }; @@ -83,10 +90,10 @@ fn multi_scalar_mul_circuit() { let bytes = Program::serialize_program(&program); let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 85, 76, 65, 14, 0, 32, 8, 82, 179, 186, 244, 104, 159, - 30, 45, 218, 136, 141, 33, 40, 186, 93, 76, 208, 57, 31, 93, 96, 136, 47, 250, 146, 188, - 209, 39, 181, 131, 131, 187, 148, 110, 240, 246, 101, 38, 63, 180, 243, 97, 3, 125, 173, - 118, 131, 153, 0, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 219, 10, 0, 32, 8, 67, 243, 214, 5, 250, 232, + 62, 189, 69, 123, 176, 132, 195, 116, 50, 149, 114, 107, 0, 97, 127, 116, 2, 75, 243, 2, + 74, 53, 122, 202, 189, 211, 15, 106, 5, 13, 116, 238, 35, 221, 81, 230, 61, 249, 37, 253, + 250, 179, 79, 109, 218, 22, 67, 227, 173, 0, 0, 0, ]; assert_eq!(bytes, expected_serialization) diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs index ee35385fa81..0b52ae295a2 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs @@ -11,7 +11,7 @@ pub(super) fn multi_scalar_mul( initial_witness: &mut WitnessMap, points: &[FunctionInput], scalars: &[FunctionInput], - outputs: (Witness, Witness), + outputs: (Witness, Witness, Witness), ) -> Result<(), OpcodeResolutionError> { let points: Result, _> = points.iter().map(|input| witness_to_value(initial_witness, input.witness)).collect(); @@ -19,35 +19,44 @@ pub(super) fn multi_scalar_mul( let scalars: Result, _> = scalars.iter().map(|input| witness_to_value(initial_witness, input.witness)).collect(); - let scalars: Vec<_> = scalars?.into_iter().cloned().collect(); - + let mut scalars_lo = Vec::new(); + let mut scalars_hi = Vec::new(); + for (i, scalar) in scalars?.into_iter().enumerate() { + if i % 2 == 0 { + scalars_lo.push(*scalar); + } else { + scalars_hi.push(*scalar); + } + } // Call the backend's multi-scalar multiplication function - let (res_x, res_y) = backend.multi_scalar_mul(&points, &scalars)?; + let (res_x, res_y, is_infinite) = + backend.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; // Insert the resulting point into the witness map insert_value(&outputs.0, res_x, initial_witness)?; insert_value(&outputs.1, res_y, initial_witness)?; - + insert_value(&outputs.2, is_infinite, initial_witness)?; Ok(()) } pub(super) fn embedded_curve_add( backend: &impl BlackBoxFunctionSolver, initial_witness: &mut WitnessMap, - input1_x: FunctionInput, - input1_y: FunctionInput, - input2_x: FunctionInput, - input2_y: FunctionInput, - outputs: (Witness, Witness), + input1: [FunctionInput; 3], + input2: [FunctionInput; 3], + outputs: (Witness, Witness, Witness), ) -> Result<(), OpcodeResolutionError> { - let input1_x = witness_to_value(initial_witness, input1_x.witness)?; - let input1_y = witness_to_value(initial_witness, input1_y.witness)?; - let input2_x = witness_to_value(initial_witness, input2_x.witness)?; - let input2_y = witness_to_value(initial_witness, input2_y.witness)?; - let (res_x, res_y) = backend.ec_add(input1_x, input1_y, input2_x, input2_y)?; + let input1_x = witness_to_value(initial_witness, input1[0].witness)?; + let input1_y = witness_to_value(initial_witness, input1[1].witness)?; + let input1_infinite = witness_to_value(initial_witness, input1[2].witness)?; + let input2_x = witness_to_value(initial_witness, input2[0].witness)?; + let input2_y = witness_to_value(initial_witness, input2[1].witness)?; + let input2_infinite = witness_to_value(initial_witness, input2[2].witness)?; + let (res_x, res_y, res_infinite) = + backend.ec_add(input1_x, input1_y, input1_infinite, input2_x, input2_y, input2_infinite)?; insert_value(&outputs.0, res_x, initial_witness)?; insert_value(&outputs.1, res_y, initial_witness)?; - + insert_value(&outputs.2, res_infinite, initial_witness)?; Ok(()) } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs index a74f44b79dc..99ed09a52e4 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -164,16 +164,8 @@ pub(crate) fn solve( BlackBoxFuncCall::MultiScalarMul { points, scalars, outputs } => { multi_scalar_mul(backend, initial_witness, points, scalars, *outputs) } - BlackBoxFuncCall::EmbeddedCurveAdd { input1_x, input1_y, input2_x, input2_y, outputs } => { - embedded_curve_add( - backend, - initial_witness, - *input1_x, - *input1_y, - *input2_x, - *input2_y, - *outputs, - ) + BlackBoxFuncCall::EmbeddedCurveAdd { input1, input2, outputs } => { + embedded_curve_add(backend, initial_witness, **input1, **input2, *outputs) } // Recursive aggregation will be entirely handled by the backend and is not solved by the ACVM BlackBoxFuncCall::RecursiveAggregation { .. } => Ok(()), diff --git a/noir/noir-repo/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts b/noir/noir-repo/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts index 8ee0a067a3a..5401da76974 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts +++ b/noir/noir-repo/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts @@ -1,21 +1,24 @@ // See `multi_scalar_mul_circuit` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 85, 76, 65, 14, 0, 32, 8, 82, 179, 186, 244, 104, 159, 30, 45, 218, 136, 141, 33, - 40, 186, 93, 76, 208, 57, 31, 93, 96, 136, 47, 250, 146, 188, 209, 39, 181, 131, 131, 187, 148, 110, 240, 246, 101, - 38, 63, 180, 243, 97, 3, 125, 173, 118, 131, 153, 0, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 219, 10, 0, 32, 8, 67, 243, 214, 5, 250, 232, 62, 189, 69, 123, 176, 132, + 195, 116, 50, 149, 114, 107, 0, 97, 127, 116, 2, 75, 243, 2, 74, 53, 122, 202, 189, 211, 15, 106, 5, 13, 116, 238, 35, + 221, 81, 230, 61, 249, 37, 253, 250, 179, 79, 109, 218, 22, 67, 227, 173, 0, 0, 0, ]); export const initialWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], [2, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], - [3, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [4, '0x0000000000000000000000000000000000000000000000000000000000000000'], + [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], + [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], + [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], ]); export const expectedWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], [2, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], - [3, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [4, '0x0000000000000000000000000000000000000000000000000000000000000000'], - [5, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [6, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], + [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], + [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], + [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], + [6, '0x0000000000000000000000000000000000000000000000000000000000000001'], + [7, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], + [8, '0x0000000000000000000000000000000000000000000000000000000000000000'], ]); diff --git a/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index 3403b0fe232..73f64d3d9d1 100644 --- a/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs @@ -27,15 +27,18 @@ pub trait BlackBoxFunctionSolver { fn multi_scalar_mul( &self, points: &[FieldElement], - scalars: &[FieldElement], - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError>; + scalars_lo: &[FieldElement], + scalars_hi: &[FieldElement], + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError>; fn ec_add( &self, input1_x: &FieldElement, input1_y: &FieldElement, + input1_infinite: &FieldElement, input2_x: &FieldElement, input2_y: &FieldElement, - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError>; + input2_infinite: &FieldElement, + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError>; fn poseidon2_permutation( &self, _inputs: &[FieldElement], @@ -81,17 +84,20 @@ impl BlackBoxFunctionSolver for StubbedBlackBoxSolver { fn multi_scalar_mul( &self, _points: &[FieldElement], - _scalars: &[FieldElement], - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { + _scalars_lo: &[FieldElement], + _scalars_hi: &[FieldElement], + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { Err(Self::fail(BlackBoxFunc::MultiScalarMul)) } fn ec_add( &self, _input1_x: &FieldElement, _input1_y: &FieldElement, + _input1_infinite: &FieldElement, _input2_x: &FieldElement, _input2_y: &FieldElement, - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { + _input2_infinite: &FieldElement, + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { Err(Self::fail(BlackBoxFunc::EmbeddedCurveAdd)) } fn poseidon2_permutation( diff --git a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index 3f6d2ac86c1..901eb9d5a0f 100644 --- a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -10,9 +10,11 @@ use crate::BlackBoxResolutionError; /// Performs multi scalar multiplication of points with scalars. pub fn multi_scalar_mul( points: &[FieldElement], - scalars: &[FieldElement], -) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { - if points.len() != scalars.len() { + scalars_lo: &[FieldElement], + scalars_hi: &[FieldElement], +) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { + if points.len() != 3 * scalars_lo.len() || scalars_lo.len() != scalars_hi.len() { + dbg!(&points.len(), &scalars_lo.len(), &scalars_hi.len()); return Err(BlackBoxResolutionError::Failed( BlackBoxFunc::MultiScalarMul, "Points and scalars must have the same length".to_string(), @@ -21,21 +23,22 @@ pub fn multi_scalar_mul( let mut output_point = grumpkin::SWAffine::zero(); - for i in (0..points.len()).step_by(2) { - let point = create_point(points[i], points[i + 1]) - .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::MultiScalarMul, e))?; + for i in (0..points.len()).step_by(3) { + let point = + create_point(points[i], points[i + 1], points[i + 2] == FieldElement::from(1_u128)) + .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::MultiScalarMul, e))?; - let scalar_low: u128 = scalars[i].try_into_u128().ok_or_else(|| { + let scalar_low: u128 = scalars_lo[i / 3].try_into_u128().ok_or_else(|| { BlackBoxResolutionError::Failed( BlackBoxFunc::MultiScalarMul, - format!("Limb {} is not less than 2^128", scalars[i].to_hex()), + format!("Limb {} is not less than 2^128", scalars_lo[i].to_hex()), ) })?; - let scalar_high: u128 = scalars[i + 1].try_into_u128().ok_or_else(|| { + let scalar_high: u128 = scalars_hi[i / 3].try_into_u128().ok_or_else(|| { BlackBoxResolutionError::Failed( BlackBoxFunc::MultiScalarMul, - format!("Limb {} is not less than 2^128", scalars[i + 1].to_hex()), + format!("Limb {} is not less than 2^128", scalars_hi[i].to_hex()), ) })?; @@ -59,25 +62,33 @@ pub fn multi_scalar_mul( } if let Some((out_x, out_y)) = output_point.xy() { - Ok((FieldElement::from_repr(*out_x), FieldElement::from_repr(*out_y))) + Ok(( + FieldElement::from_repr(*out_x), + FieldElement::from_repr(*out_y), + FieldElement::from(output_point.is_zero() as u128), + )) } else { - Ok((FieldElement::zero(), FieldElement::zero())) + Ok((FieldElement::from(0_u128), FieldElement::from(0_u128), FieldElement::from(1_u128))) } } pub fn embedded_curve_add( - input1_x: FieldElement, - input1_y: FieldElement, - input2_x: FieldElement, - input2_y: FieldElement, -) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { - let point1 = create_point(input1_x, input1_y) + input1: [FieldElement; 3], + input2: [FieldElement; 3], +) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { + let point1 = create_point(input1[0], input1[1], input1[2] == FieldElement::one()) .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::EmbeddedCurveAdd, e))?; - let point2 = create_point(input2_x, input2_y) + let point2 = create_point(input2[0], input2[1], input2[2] == FieldElement::one()) .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::EmbeddedCurveAdd, e))?; let res = grumpkin::SWAffine::from(point1 + point2); if let Some((res_x, res_y)) = res.xy() { - Ok((FieldElement::from_repr(*res_x), FieldElement::from_repr(*res_y))) + Ok(( + FieldElement::from_repr(*res_x), + FieldElement::from_repr(*res_y), + FieldElement::from(res.is_zero() as u128), + )) + } else if res.is_zero() { + Ok((FieldElement::from(0_u128), FieldElement::from(0_u128), FieldElement::from(1_u128))) } else { Err(BlackBoxResolutionError::Failed( BlackBoxFunc::EmbeddedCurveAdd, @@ -86,7 +97,14 @@ pub fn embedded_curve_add( } } -fn create_point(x: FieldElement, y: FieldElement) -> Result { +fn create_point( + x: FieldElement, + y: FieldElement, + is_infinite: bool, +) -> Result { + if is_infinite { + return Ok(grumpkin::SWAffine::zero()); + } let point = grumpkin::SWAffine::new_unchecked(x.into_repr(), y.into_repr()); if !point.is_on_curve() { return Err(format!("Point ({}, {}) is not on curve", x.to_hex(), y.to_hex())); @@ -103,11 +121,11 @@ mod tests { use super::*; - fn get_generator() -> [FieldElement; 2] { + fn get_generator() -> [FieldElement; 3] { let generator = grumpkin::SWAffine::generator(); let generator_x = FieldElement::from_repr(*generator.x().unwrap()); let generator_y = FieldElement::from_repr(*generator.y().unwrap()); - [generator_x, generator_y] + [generator_x, generator_y, FieldElement::zero()] } #[test] @@ -115,7 +133,7 @@ mod tests { // We check that multiplying 1 by generator results in the generator let generator = get_generator(); - let res = multi_scalar_mul(&generator, &[FieldElement::one(), FieldElement::zero()])?; + let res = multi_scalar_mul(&generator, &[FieldElement::one()], &[FieldElement::zero()])?; assert_eq!(generator[0], res.0); assert_eq!(generator[1], res.1); @@ -125,9 +143,10 @@ mod tests { #[test] fn low_high_smoke_test() -> Result<(), BlackBoxResolutionError> { let points = get_generator(); - let scalars = [FieldElement::one(), FieldElement::from(2u128)]; + let scalars_lo = [FieldElement::one()]; + let scalars_hi = [FieldElement::from(2u128)]; - let res = multi_scalar_mul(&points, &scalars)?; + let res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; let x = "0702ab9c7038eeecc179b4f209991bcb68c7cb05bf4c532d804ccac36199c9a9"; let y = "23f10e9e43a3ae8d75d24154e796aae12ae7af546716e8f81a2564f1b5814130"; @@ -148,10 +167,10 @@ mod tests { "Limb 0000000000000000000000000000000100000000000000000000000000000000 is not less than 2^128".into(), )); - let res = multi_scalar_mul(&points, &[FieldElement::one(), invalid_limb]); + let res = multi_scalar_mul(&points, &[FieldElement::one()], &[invalid_limb]); assert_eq!(res, expected_error); - let res = multi_scalar_mul(&points, &[invalid_limb, FieldElement::one()]); + let res = multi_scalar_mul(&points, &[invalid_limb], &[FieldElement::one()]); assert_eq!(res, expected_error); } @@ -162,7 +181,7 @@ mod tests { let low = FieldElement::from_be_bytes_reduce(&x[16..32]); let high = FieldElement::from_be_bytes_reduce(&x[0..16]); - let res = multi_scalar_mul(&get_generator(), &[low, high]); + let res = multi_scalar_mul(&get_generator(), &[low], &[high]); assert_eq!( res, @@ -181,8 +200,9 @@ mod tests { let valid_scalar_high = FieldElement::zero(); let res = multi_scalar_mul( - &[invalid_point_x, invalid_point_y], - &[valid_scalar_low, valid_scalar_high], + &[invalid_point_x, invalid_point_y, FieldElement::zero()], + &[valid_scalar_low], + &[valid_scalar_high], ); assert_eq!( @@ -197,9 +217,10 @@ mod tests { #[test] fn throws_on_args_length_mismatch() { let points = get_generator(); - let scalars = [FieldElement::from(2u128)]; + let scalars_lo = [FieldElement::from(2u128)]; + let scalars_hi = []; - let res = multi_scalar_mul(&points, &scalars); + let res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi); assert_eq!( res, @@ -215,7 +236,10 @@ mod tests { let x = FieldElement::from(1u128); let y = FieldElement::from(2u128); - let res = embedded_curve_add(x, y, x, y); + let res = embedded_curve_add( + [x, y, FieldElement::from(0u128)], + [x, y, FieldElement::from(0u128)], + ); assert_eq!( res, @@ -229,10 +253,14 @@ mod tests { #[test] fn output_of_msm_matches_add() -> Result<(), BlackBoxResolutionError> { let points = get_generator(); - let scalars = [FieldElement::from(2u128), FieldElement::zero()]; - - let msm_res = multi_scalar_mul(&points, &scalars)?; - let add_res = embedded_curve_add(points[0], points[1], points[0], points[1])?; + let scalars_lo = [FieldElement::from(2u128)]; + let scalars_hi = [FieldElement::zero()]; + + let msm_res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + let add_res = embedded_curve_add( + [points[0], points[1], FieldElement::from(0u128)], + [points[0], points[1], FieldElement::from(0u128)], + )?; assert_eq!(msm_res.0, add_res.0); assert_eq!(msm_res.1, add_res.1); diff --git a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs index 4cb51b59755..a85ddcd894e 100644 --- a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -92,19 +92,25 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { fn multi_scalar_mul( &self, points: &[FieldElement], - scalars: &[FieldElement], - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { - multi_scalar_mul(points, scalars) + scalars_lo: &[FieldElement], + scalars_hi: &[FieldElement], + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { + multi_scalar_mul(points, scalars_lo, scalars_hi) } fn ec_add( &self, input1_x: &FieldElement, input1_y: &FieldElement, + input1_infinite: &FieldElement, input2_x: &FieldElement, input2_y: &FieldElement, - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { - embedded_curve_add(*input1_x, *input1_y, *input2_x, *input2_y) + input2_infinite: &FieldElement, + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { + embedded_curve_add( + [*input1_x, *input1_y, *input1_infinite], + [*input2_x, *input2_y, *input2_infinite], + ) } fn poseidon2_permutation( diff --git a/noir/noir-repo/acvm-repo/brillig/src/black_box.rs b/noir/noir-repo/acvm-repo/brillig/src/black_box.rs index 9a66b428dc3..3887092a8c2 100644 --- a/noir/noir-repo/acvm-repo/brillig/src/black_box.rs +++ b/noir/noir-repo/acvm-repo/brillig/src/black_box.rs @@ -83,8 +83,10 @@ pub enum BlackBoxOp { EmbeddedCurveAdd { input1_x: MemoryAddress, input1_y: MemoryAddress, + input1_infinite: MemoryAddress, input2_x: MemoryAddress, input2_y: MemoryAddress, + input2_infinite: MemoryAddress, result: HeapArray, }, BigIntAdd { diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs index d6ecd25f454..ebaa6976283 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs @@ -157,22 +157,63 @@ pub(crate) fn evaluate_black_box( Ok(()) } BlackBoxOp::MultiScalarMul { points, scalars, outputs: result } => { - let points: Vec = - read_heap_vector(memory, points).iter().map(|x| x.try_into().unwrap()).collect(); + let points: Vec = read_heap_vector(memory, points) + .iter() + .enumerate() + .map(|(i, x)| { + if i % 3 == 2 { + let is_infinite: bool = x.try_into().unwrap(); + FieldElement::from(is_infinite as u128) + } else { + x.try_into().unwrap() + } + }) + .collect(); let scalars: Vec = read_heap_vector(memory, scalars).iter().map(|x| x.try_into().unwrap()).collect(); - - let (x, y) = solver.multi_scalar_mul(&points, &scalars)?; - memory.write_slice(memory.read_ref(result.pointer), &[x.into(), y.into()]); + let mut scalars_lo = Vec::with_capacity(scalars.len() / 2); + let mut scalars_hi = Vec::with_capacity(scalars.len() / 2); + for (i, scalar) in scalars.iter().enumerate() { + if i % 2 == 0 { + scalars_lo.push(*scalar); + } else { + scalars_hi.push(*scalar); + } + } + let (x, y, is_infinite) = solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + memory.write_slice( + memory.read_ref(result.pointer), + &[x.into(), y.into(), is_infinite.into()], + ); Ok(()) } - BlackBoxOp::EmbeddedCurveAdd { input1_x, input1_y, input2_x, input2_y, result } => { + BlackBoxOp::EmbeddedCurveAdd { + input1_x, + input1_y, + input2_x, + input2_y, + result, + input1_infinite, + input2_infinite, + } => { let input1_x = memory.read(*input1_x).try_into().unwrap(); let input1_y = memory.read(*input1_y).try_into().unwrap(); + let input1_infinite: bool = memory.read(*input1_infinite).try_into().unwrap(); let input2_x = memory.read(*input2_x).try_into().unwrap(); let input2_y = memory.read(*input2_y).try_into().unwrap(); - let (x, y) = solver.ec_add(&input1_x, &input1_y, &input2_x, &input2_y)?; - memory.write_slice(memory.read_ref(result.pointer), &[x.into(), y.into()]); + let input2_infinite: bool = memory.read(*input2_infinite).try_into().unwrap(); + let (x, y, infinite) = solver.ec_add( + &input1_x, + &input1_y, + &input1_infinite.into(), + &input2_x, + &input2_y, + &input2_infinite.into(), + )?; + memory.write_slice( + memory.read_ref(result.pointer), + &[x.into(), y.into(), infinite.into()], + ); Ok(()) } BlackBoxOp::PedersenCommitment { inputs, domain_separator, output } => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs index d587abc9463..f56c5daf315 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs @@ -207,15 +207,17 @@ pub(crate) fn convert_black_box_call( } BlackBoxFunc::EmbeddedCurveAdd => { if let ( - [BrilligVariable::SingleAddr(input1_x), BrilligVariable::SingleAddr(input1_y), BrilligVariable::SingleAddr(input2_x), BrilligVariable::SingleAddr(input2_y)], + [BrilligVariable::SingleAddr(input1_x), BrilligVariable::SingleAddr(input1_y), BrilligVariable::SingleAddr(input1_infinite), BrilligVariable::SingleAddr(input2_x), BrilligVariable::SingleAddr(input2_y), BrilligVariable::SingleAddr(input2_infinite)], [BrilligVariable::BrilligArray(result_array)], ) = (function_arguments, function_results) { brillig_context.black_box_op_instruction(BlackBoxOp::EmbeddedCurveAdd { input1_x: input1_x.address, input1_y: input1_y.address, + input1_infinite: input1_infinite.address, input2_x: input2_x.address, input2_y: input2_y.address, + input2_infinite: input2_infinite.address, result: result_array.to_heap_array(), }); } else { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index fadcdb22c15..2bd57dc9486 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -170,18 +170,21 @@ pub(crate) mod tests { fn multi_scalar_mul( &self, _points: &[FieldElement], - _scalars: &[FieldElement], - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { - Ok((4_u128.into(), 5_u128.into())) + _scalars_lo: &[FieldElement], + _scalars_hi: &[FieldElement], + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { + Ok((4_u128.into(), 5_u128.into(), 0_u128.into())) } fn ec_add( &self, _input1_x: &FieldElement, _input1_y: &FieldElement, + _input1_infinite: &FieldElement, _input2_x: &FieldElement, _input2_y: &FieldElement, - ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { + _input2_infinite: &FieldElement, + ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { panic!("Path not trodden by this test") } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs index f02f6059e7c..def91f82bfd 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -334,7 +334,9 @@ impl DebugShow { outputs ); } - BlackBoxOp::EmbeddedCurveAdd { input1_x, input1_y, input2_x, input2_y, result } => { + BlackBoxOp::EmbeddedCurveAdd { + input1_x, input1_y, input2_x, input2_y, result, .. + } => { debug_println!( self.enable_debug_trace, " EMBEDDED_CURVE_ADD ({} {}) ({} {}) -> {}", diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index c1249ae41c8..d23f4abe5f5 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -293,14 +293,13 @@ impl GeneratedAcir { BlackBoxFunc::MultiScalarMul => BlackBoxFuncCall::MultiScalarMul { points: inputs[0].clone(), scalars: inputs[1].clone(), - outputs: (outputs[0], outputs[1]), + outputs: (outputs[0], outputs[1], outputs[2]), }, + BlackBoxFunc::EmbeddedCurveAdd => BlackBoxFuncCall::EmbeddedCurveAdd { - input1_x: inputs[0][0], - input1_y: inputs[1][0], - input2_x: inputs[2][0], - input2_y: inputs[3][0], - outputs: (outputs[0], outputs[1]), + input1: Box::new([inputs[0][0], inputs[1][0], inputs[2][0]]), + input2: Box::new([inputs[3][0], inputs[4][0], inputs[5][0]]), + outputs: (outputs[0], outputs[1], outputs[2]), }, BlackBoxFunc::Keccak256 => { let var_message_size = match inputs.to_vec().pop() { @@ -684,8 +683,8 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Option { // Recursive aggregation has a variable number of inputs BlackBoxFunc::RecursiveAggregation => None, - // Addition over the embedded curve: input are coordinates (x1,y1) and (x2,y2) of the Grumpkin points - BlackBoxFunc::EmbeddedCurveAdd => Some(4), + // Addition over the embedded curve: input are coordinates (x1,y1,infinite1) and (x2,y2,infinite2) of the Grumpkin points + BlackBoxFunc::EmbeddedCurveAdd => Some(6), // Big integer operations take in 0 inputs. They use constants for their inputs. BlackBoxFunc::BigIntAdd @@ -735,7 +734,7 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Option { // Output of operations over the embedded curve // will be 2 field elements representing the point. - BlackBoxFunc::MultiScalarMul | BlackBoxFunc::EmbeddedCurveAdd => Some(2), + BlackBoxFunc::MultiScalarMul | BlackBoxFunc::EmbeddedCurveAdd => Some(3), // Big integer operations return a big integer BlackBoxFunc::BigIntAdd diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 0de0c28be75..117804b370a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1741,13 +1741,16 @@ impl<'a> Context<'a> { // will expand the array if there is one. let return_acir_vars = self.flatten_value_list(return_values, dfg)?; let mut warnings = Vec::new(); - for acir_var in return_acir_vars { + for (acir_var, is_databus) in return_acir_vars { if self.acir_context.is_constant(&acir_var) { warnings.push(SsaReport::Warning(InternalWarning::ReturnConstant { call_stack: call_stack.clone(), })); } - self.acir_context.return_var(acir_var)?; + if !is_databus { + // We do not return value for the data bus. + self.acir_context.return_var(acir_var)?; + } } Ok(warnings) } @@ -2671,12 +2674,22 @@ impl<'a> Context<'a> { &mut self, arguments: &[ValueId], dfg: &DataFlowGraph, - ) -> Result, InternalError> { + ) -> Result, InternalError> { let mut acir_vars = Vec::with_capacity(arguments.len()); for value_id in arguments { + let is_databus = if let Some(return_databus) = self.data_bus.return_data { + dfg[*value_id] == dfg[return_databus] + } else { + false + }; let value = self.convert_value(*value_id, dfg); acir_vars.append( - &mut self.acir_context.flatten(value)?.iter().map(|(var, _)| *var).collect(), + &mut self + .acir_context + .flatten(value)? + .iter() + .map(|(var, _)| (*var, is_databus)) + .collect(), ); } Ok(acir_vars) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 77b9e545e03..73dc3888184 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -393,10 +393,11 @@ impl<'function> PerFunctionContext<'function> { let function = &ssa.functions[&func_id]; // If we have not already finished the flattening pass, functions marked // to not have predicates should be marked as entry points unless we are inlining into brillig. + let entry_point = &ssa.functions[&self.context.entry_point]; let no_predicates_is_entry_point = self.context.no_predicates_is_entry_point && function.is_no_predicates() - && !matches!(self.source_function.runtime(), RuntimeType::Brillig); + && !matches!(entry_point.runtime(), RuntimeType::Brillig); if function.runtime().is_entry_point() || no_predicates_is_entry_point { self.push_instruction(*id); } else { diff --git a/noir/noir-repo/noir_stdlib/src/embedded_curve_ops.nr b/noir/noir-repo/noir_stdlib/src/embedded_curve_ops.nr index 21d658db615..2aec90e0a55 100644 --- a/noir/noir-repo/noir_stdlib/src/embedded_curve_ops.nr +++ b/noir/noir-repo/noir_stdlib/src/embedded_curve_ops.nr @@ -1,15 +1,20 @@ use crate::ops::arith::{Add, Sub, Neg}; - +use crate::cmp::Eq; // TODO(https://github.com/noir-lang/noir/issues/4931) struct EmbeddedCurvePoint { x: Field, y: Field, + is_infinite: bool } impl EmbeddedCurvePoint { fn double(self) -> EmbeddedCurvePoint { embedded_curve_add(self, self) } + + fn point_at_infinity() -> EmbeddedCurvePoint { + EmbeddedCurvePoint { x: 0, y: 0, is_infinite: true } + } } impl Add for EmbeddedCurvePoint { @@ -28,11 +33,24 @@ impl Neg for EmbeddedCurvePoint { fn neg(self) -> EmbeddedCurvePoint { EmbeddedCurvePoint { x: self.x, - y: -self.y + y: -self.y, + is_infinite: self.is_infinite } } } +impl Eq for EmbeddedCurvePoint { + fn eq(self: Self, b: EmbeddedCurvePoint) -> bool { + (self.is_infinite & b.is_infinite) | ((self.is_infinite == b.is_infinite) & (self.x == b.x) & (self.y == b.y)) + } +} + +// Scalar represented as low and high limbs +struct EmbeddedCurveScalar { + lo: Field, + hi: Field, +} + // Computes a multi scalar multiplication over the embedded curve. // For bn254, We have Grumpkin and Baby JubJub. // For bls12-381, we have JubJub and Bandersnatch. @@ -42,9 +60,9 @@ impl Neg for EmbeddedCurvePoint { #[foreign(multi_scalar_mul)] // docs:start:multi_scalar_mul pub fn multi_scalar_mul( - points: [Field; N], // points represented as x and y coordinates [x1, y1, x2, y2, ...] - scalars: [Field; N] // scalars represented as low and high limbs [low1, high1, low2, high2, ...] -) -> [Field; 2] + points: [EmbeddedCurvePoint; N], + scalars: [EmbeddedCurveScalar; N] +) -> [Field; 3] // docs:end:multi_scalar_mul {} @@ -52,12 +70,12 @@ pub fn multi_scalar_mul( pub fn fixed_base_scalar_mul( scalar_low: Field, scalar_high: Field -) -> [Field; 2] +) -> [Field; 3] // docs:end:fixed_base_scalar_mul { - let g1_x = 1; - let g1_y = 17631683881184975370165255887551781615748388533673675138860; - multi_scalar_mul([g1_x, g1_y], [scalar_low, scalar_high]) + let g1 = EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; + let scalar = EmbeddedCurveScalar { lo: scalar_low, hi: scalar_high }; + multi_scalar_mul([g1], [scalar]) } // This is a hack as returning an `EmbeddedCurvePoint` from a foreign function in brillig returns a [BrilligVariable::SingleAddr; 2] rather than BrilligVariable::BrilligArray @@ -72,8 +90,8 @@ fn embedded_curve_add( let point_array = embedded_curve_add_array_return(point1, point2); let x = point_array[0]; let y = point_array[1]; - EmbeddedCurvePoint { x, y } + EmbeddedCurvePoint { x, y, is_infinite: point_array[2] == 1 } } #[foreign(embedded_curve_add)] -fn embedded_curve_add_array_return(_point1: EmbeddedCurvePoint, _point2: EmbeddedCurvePoint) -> [Field; 2] {} +fn embedded_curve_add_array_return(_point1: EmbeddedCurvePoint, _point2: EmbeddedCurvePoint) -> [Field; 3] {} diff --git a/noir/noir-repo/noir_stdlib/src/uint128.nr b/noir/noir-repo/noir_stdlib/src/uint128.nr index 9c61fc801f3..0332c8ac865 100644 --- a/noir/noir-repo/noir_stdlib/src/uint128.nr +++ b/noir/noir-repo/noir_stdlib/src/uint128.nr @@ -1,8 +1,9 @@ use crate::ops::{Add, Sub, Mul, Div, Rem, Not, BitOr, BitAnd, BitXor, Shl, Shr}; use crate::cmp::{Eq, Ord, Ordering}; +use crate::println; global pow64 : Field = 18446744073709551616; //2^64; - +global pow63 : Field = 9223372036854775808; // 2^63; struct U128 { lo: Field, hi: Field, @@ -20,6 +21,13 @@ impl U128 { U128::from_u64s_le(lo, hi) } + pub fn zero() -> U128 { + U128 { lo: 0, hi: 0 } + } + + pub fn one() -> U128 { + U128 { lo: 1, hi: 0 } + } pub fn from_le_bytes(bytes: [u8; 16]) -> U128 { let mut lo = 0; let mut base = 1; @@ -87,27 +95,44 @@ impl U128 { U128 { lo: lo as Field, hi: hi as Field } } + unconstrained fn uconstrained_check_is_upper_ascii(ascii: u8) -> bool { + ((ascii >= 65) & (ascii <= 90)) // Between 'A' and 'Z' + } + fn decode_ascii(ascii: u8) -> Field { if ascii < 58 { ascii - 48 - } else if ascii < 71 { - ascii - 55 } else { + let ascii = ascii + 32 * (U128::uconstrained_check_is_upper_ascii(ascii) as u8); + assert(ascii >= 97); // enforce >= 'a' + assert(ascii <= 102); // enforce <= 'f' ascii - 87 } as Field } + // TODO: Replace with a faster version. + // A circuit that uses this function can be slow to compute + // (we're doing up to 127 calls to compute the quotient) unconstrained fn unconstrained_div(self: Self, b: U128) -> (U128, U128) { - if self < b { - (U128::from_u64s_le(0, 0), self) + if b == U128::zero() { + // Return 0,0 to avoid eternal loop + (U128::zero(), U128::zero()) + } else if self < b { + (U128::zero(), self) + } else if self == b { + (U128::one(), U128::zero()) } else { - //TODO check if this can overflow? - let (q,r) = self.unconstrained_div(b * U128::from_u64s_le(2, 0)); + let (q,r) = if b.hi as u64 >= pow63 as u64 { + // The result of multiplication by 2 would overflow + (U128::zero(), self) + } else { + self.unconstrained_div(b * U128::from_u64s_le(2, 0)) + }; let q_mul_2 = q * U128::from_u64s_le(2, 0); if r < b { (q_mul_2, r) } else { - (q_mul_2 + U128::from_u64s_le(1, 0), r - b) + (q_mul_2 + U128::one(), r - b) } } } @@ -129,11 +154,7 @@ impl U128 { let low = self.lo * b.lo; let lo = low as u64 as Field; let carry = (low - lo) / pow64; - let high = if crate::field::modulus_num_bits() as u32 > 196 { - (self.lo + self.hi) * (b.lo + b.hi) - low + carry - } else { - self.lo * b.hi + self.hi * b.lo + carry - }; + let high = self.lo * b.hi + self.hi * b.lo + carry; let hi = high as u64 as Field; U128 { lo, hi } } @@ -294,8 +315,8 @@ impl Shr for U128 { } } -mod test { - use crate::uint128::{U128, pow64}; +mod tests { + use crate::uint128::{U128, pow64, pow63}; #[test] fn test_not() { @@ -309,4 +330,205 @@ mod test { let not_not_num = not_num.not(); assert_eq(num, not_not_num); } + + #[test] + fn test_construction() { + // Check little-endian u64 is inversed with big-endian u64 construction + let a = U128::from_u64s_le(2, 1); + let b = U128::from_u64s_be(1, 2); + assert_eq(a, b); + // Check byte construction is equivalent + let c = U128::from_le_bytes([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]); + let d = U128::from_u64s_le(0x0706050403020100, 0x0f0e0d0c0b0a0908); + assert_eq(c, d); + } + + #[test] + fn test_byte_decomposition() { + let a = U128::from_u64s_le(0x0706050403020100, 0x0f0e0d0c0b0a0908); + // Get big-endian and little-endian byte decompostions + let le_bytes_a= a.to_le_bytes(); + let be_bytes_a= a.to_be_bytes(); + + // Check equivalence + for i in 0..16 { + assert_eq(le_bytes_a[i], be_bytes_a[15 - i]); + } + // Reconstruct U128 from byte decomposition + let b= U128::from_le_bytes(le_bytes_a); + // Check that it's the same element + assert_eq(a, b); + } + + #[test] + fn test_hex_constuction() { + let a = U128::from_u64s_le(0x1, 0x2); + let b = U128::from_hex("0x20000000000000001"); + assert_eq(a, b); + + let c= U128::from_hex("0xffffffffffffffffffffffffffffffff"); + let d= U128::from_u64s_le(0xffffffffffffffff, 0xffffffffffffffff); + assert_eq(c, d); + + let e= U128::from_hex("0x00000000000000000000000000000000"); + let f= U128::from_u64s_le(0, 0); + assert_eq(e, f); + } + + // Ascii decode tests + + #[test] + fn test_ascii_decode_correct_range() { + // '0'..'9' range + for i in 0..10 { + let decoded= U128::decode_ascii(48 + i); + assert_eq(decoded, i as Field); + } + // 'A'..'F' range + for i in 0..6 { + let decoded = U128::decode_ascii(65 + i); + assert_eq(decoded, (i + 10) as Field); + } + // 'a'..'f' range + for i in 0..6 { + let decoded = U128::decode_ascii(97 + i); + assert_eq(decoded, (i + 10) as Field); + } + } + + #[test(should_fail)] + fn test_ascii_decode_range_less_than_48_fails_0() { + crate::println(U128::decode_ascii(0)); + } + + #[test(should_fail)] + fn test_ascii_decode_range_less_than_48_fails_1() { + crate::println(U128::decode_ascii(47)); + } + + #[test(should_fail)] + fn test_ascii_decode_range_58_64_fails_0() { + let _ = U128::decode_ascii(58); + } + + #[test(should_fail)] + fn test_ascii_decode_range_58_64_fails_1() { + let _ = U128::decode_ascii(64); + } + + #[test(should_fail)] + fn test_ascii_decode_range_71_96_fails_0() { + let _ = U128::decode_ascii(71); + } + + #[test(should_fail)] + fn test_ascii_decode_range_71_96_fails_1() { + let _ = U128::decode_ascii(96); + } + + #[test(should_fail)] + fn test_ascii_decode_range_greater_than_102_fails() { + let _ = U128::decode_ascii(103); + } + + #[test(should_fail)] + fn test_ascii_decode_regression() { + // This code will actually fail because of ascii_decode, + // but in the past it was possible to create a value > (1<<128) + let a = U128::from_hex("0x~fffffffffffffffffffffffffffffff"); + let b:Field= a.to_integer(); + let c= b.to_le_bytes(17); + assert(c[16] != 0); + } + + #[test] + fn test_unconstrained_div() { + // Test the potential overflow case + let a= U128::from_u64s_le(0x0, 0xffffffffffffffff); + let b= U128::from_u64s_le(0x0, 0xfffffffffffffffe); + let c= U128::one(); + let d= U128::from_u64s_le(0x0, 0x1); + let (q,r) = a.unconstrained_div(b); + assert_eq(q, c); + assert_eq(r, d); + + let a = U128::from_u64s_le(2, 0); + let b = U128::one(); + // Check the case where a is a multiple of b + let (c,d ) = a.unconstrained_div(b); + assert_eq((c, d), (a, U128::zero())); + + // Check where b is a multiple of a + let (c,d) = b.unconstrained_div(a); + assert_eq((c, d), (U128::zero(), b)); + + // Dividing by zero returns 0,0 + let a = U128::from_u64s_le(0x1, 0x0); + let b = U128::zero(); + let (c,d)= a.unconstrained_div(b); + assert_eq((c, d), (U128::zero(), U128::zero())); + + // Dividing 1<<127 by 1<<127 (special case) + let a = U128::from_u64s_le(0x0, pow63 as u64); + let b = U128::from_u64s_le(0x0, pow63 as u64); + let (c,d )= a.unconstrained_div(b); + assert_eq((c, d), (U128::one(), U128::zero())); + } + + #[test] + fn integer_conversions() { + // Maximum + let start:Field = 0xffffffffffffffffffffffffffffffff; + let a = U128::from_integer(start); + let end = a.to_integer(); + assert_eq(start, end); + + // Minimum + let start:Field = 0x0; + let a = U128::from_integer(start); + let end = a.to_integer(); + assert_eq(start, end); + + // Low limb + let start:Field = 0xffffffffffffffff; + let a = U128::from_integer(start); + let end = a.to_integer(); + assert_eq(start, end); + + // High limb + let start:Field = 0xffffffffffffffff0000000000000000; + let a = U128::from_integer(start); + let end = a.to_integer(); + assert_eq(start, end); + } + #[test] + fn test_wrapping_mul() { + // 1*0==0 + assert_eq(U128::zero(), U128::zero().wrapping_mul(U128::one())); + + // 0*1==0 + assert_eq(U128::zero(), U128::one().wrapping_mul(U128::zero())); + + // 1*1==1 + assert_eq(U128::one(), U128::one().wrapping_mul(U128::one())); + + // 0 * ( 1 << 64 ) == 0 + assert_eq(U128::zero(), U128::zero().wrapping_mul(U128::from_u64s_le(0, 1))); + + // ( 1 << 64 ) * 0 == 0 + assert_eq(U128::zero(), U128::from_u64s_le(0, 1).wrapping_mul(U128::zero())); + + // 1 * ( 1 << 64 ) == 1 << 64 + assert_eq(U128::from_u64s_le(0, 1), U128::from_u64s_le(0, 1).wrapping_mul(U128::one())); + + // ( 1 << 64 ) * 1 == 1 << 64 + assert_eq(U128::from_u64s_le(0, 1), U128::one().wrapping_mul(U128::from_u64s_le(0, 1))); + + // ( 1 << 64 ) * ( 1 << 64 ) == 1 << 64 + assert_eq(U128::zero(), U128::from_u64s_le(0, 1).wrapping_mul(U128::from_u64s_le(0, 1))); + // -1 * -1 == 1 + assert_eq( + U128::one(), U128::from_u64s_le(0xffffffffffffffff, 0xffffffffffffffff).wrapping_mul(U128::from_u64s_le(0xffffffffffffffff, 0xffffffffffffffff)) + ); + } } diff --git a/noir/noir-repo/scripts/count_loc.sh b/noir/noir-repo/scripts/count_loc.sh new file mode 100755 index 00000000000..91565aa6c4a --- /dev/null +++ b/noir/noir-repo/scripts/count_loc.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +set -eu + +# Run relative to repo root +cd $(dirname "$0")/../ + +if ! command -v "tokei" >/dev/null 2>&1; then + echo "Error: tokei is required but not installed." >&2 + echo "Error: Run \`cargo install --git https://github.com/TomAFrench/tokei --branch tf/add-noir-support tokei\`" >&2 + + exit 1 +fi + +echo "" +echo "Total:" + +tokei ./ --sort code + +echo "" +echo "ACIR/ACVM:" +tokei ./acvm-repo --sort code + +echo "" +echo "Compiler:" +tokei ./compiler --sort code + +echo "" +echo "Tooling:" +tokei ./tooling --sort code + +echo "" +echo "Standard Library:" +tokei ./noir_stdlib --sort code diff --git a/noir/noir-repo/security/insectarium/noir_stdlib.md b/noir/noir-repo/security/insectarium/noir_stdlib.md new file mode 100644 index 00000000000..5ec4eb5f6cd --- /dev/null +++ b/noir/noir-repo/security/insectarium/noir_stdlib.md @@ -0,0 +1,61 @@ +# Bugs found in Noir stdlib + +## U128 + +### decode_ascii +Old **decode_ascii** function didn't check that the values of individual bytes in the string were just in the range of [0-9a-f-A-F]. +```rust +fn decode_ascii(ascii: u8) -> Field { + if ascii < 58 { + ascii - 48 + } else if ascii < 71 { + ascii - 55 + } else { + ascii - 87 + } as Field +} +``` +Since the function used the assumption that decode_ascii returns values in range [0,15] to construct **lo** and **hi** it was possible to overflow these 64-bit limbs. + +### unconstrained_div +```rust + unconstrained fn unconstrained_div(self: Self, b: U128) -> (U128, U128) { + if self < b { + (U128::from_u64s_le(0, 0), self) + } else { + //TODO check if this can overflow? + let (q,r) = self.unconstrained_div(b * U128::from_u64s_le(2, 0)); + let q_mul_2 = q * U128::from_u64s_le(2, 0); + if r < b { + (q_mul_2, r) + } else { + (q_mul_2 + U128::from_u64s_le(1, 0), r - b) + } + } + } +``` +There were 2 issues in unconstrained_div: +1) Attempting to divide by zero resulted in an infinite loop, because there was no check. +2) $a >= 2^{127}$ cause the function to multiply b to such power of 2 that the result would be more than $2^{128}$ and lead to assertion failure even though it was a legitimate input + +N.B. initial fix by Rumata888 also had an edgecase missing for when a==b and b >= (1<<127). + +### wrapping_mul +```rust +fn wrapping_mul(self: Self, b: U128) -> U128 { + let low = self.lo * b.lo; + let lo = low as u64 as Field; + let carry = (low - lo) / pow64; + let high = if crate::field::modulus_num_bits() as u32 > 196 { + (self.lo + self.hi) * (b.lo + b.hi) - low + carry // Bug + } else { + self.lo * b.hi + self.hi * b.lo + carry + }; + let hi = high as u64 as Field; + U128 { lo, hi } + } +``` +Wrapping mul had the code copied from regular mul barring the assertion that the product of high limbs is zero. Because that check was removed, the optimized path for moduli > 196 bits was incorrect, since it included their product (as at least one of them was supposed to be zero originally, but not for wrapping multiplication) + + + diff --git a/noir/noir-repo/test_programs/compile_success_empty/intrinsic_die/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/intrinsic_die/src/main.nr index 9ce17f72c0d..a6c6d3df9a1 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/intrinsic_die/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/intrinsic_die/src/main.nr @@ -4,5 +4,7 @@ fn main(x: Field) { let hash = std::hash::pedersen_commitment([x]); let g1_x = 0x0000000000000000000000000000000000000000000000000000000000000001; let g1_y = 0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c; - let _p1 = std::embedded_curve_ops::multi_scalar_mul([g1_x, g1_y], [x, 0]); + let g1 = std::embedded_curve_ops::EmbeddedCurvePoint { x: g1_x, y: g1_y, is_infinite: false }; + let scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: x, hi: 0 }; + let _p1 = std::embedded_curve_ops::multi_scalar_mul([g1], [scalar]); } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_embedded_curve/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_embedded_curve/src/main.nr index 8a1a7f08975..89a699448dc 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_embedded_curve/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_embedded_curve/src/main.nr @@ -2,22 +2,22 @@ use dep::std; unconstrained fn main(priv_key: Field, pub_x: pub Field, pub_y: pub Field) { let g1_y = 17631683881184975370165255887551781615748388533673675138860; - let g1 = std::embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: g1_y }; - + let g1 = std::embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: g1_y, is_infinite: false }; + let scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: priv_key, hi: 0 }; // Test that multi_scalar_mul correctly derives the public key - let res = std::embedded_curve_ops::multi_scalar_mul([g1.x, g1.y], [priv_key, 0]); + let res = std::embedded_curve_ops::multi_scalar_mul([g1], [scalar]); assert(res[0] == pub_x); assert(res[1] == pub_y); // Test that double function calling embedded_curve_add works as expected - let pub_point = std::embedded_curve_ops::EmbeddedCurvePoint { x: pub_x, y: pub_y }; + let pub_point = std::embedded_curve_ops::EmbeddedCurvePoint { x: pub_x, y: pub_y, is_infinite: false }; let res = pub_point.double(); let double = g1.add(g1); assert(double.x == res.x); // Test calling multi_scalar_mul with multiple points and scalars - let res = std::embedded_curve_ops::multi_scalar_mul([g1.x, g1.y, g1.x, g1.y], [priv_key, 0, priv_key, 0]); + let res = std::embedded_curve_ops::multi_scalar_mul([g1, g1], [scalar, scalar]); // The results should be double the g1 point because the scalars are 1 and we pass in g1 twice assert(double.x == res[0]); diff --git a/noir/noir-repo/test_programs/execution_success/embedded_curve_ops/src/main.nr b/noir/noir-repo/test_programs/execution_success/embedded_curve_ops/src/main.nr index 3cb27d8c181..46f919e947a 100644 --- a/noir/noir-repo/test_programs/execution_success/embedded_curve_ops/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/embedded_curve_ops/src/main.nr @@ -2,22 +2,22 @@ use dep::std; fn main(priv_key: Field, pub_x: pub Field, pub_y: pub Field) { let g1_y = 17631683881184975370165255887551781615748388533673675138860; - let g1 = std::embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: g1_y }; - + let g1 = std::embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: g1_y, is_infinite: false }; + let scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: priv_key, hi: 0 }; // Test that multi_scalar_mul correctly derives the public key - let res = std::embedded_curve_ops::multi_scalar_mul([g1.x, g1.y], [priv_key, 0]); + let res = std::embedded_curve_ops::multi_scalar_mul([g1], [scalar]); assert(res[0] == pub_x); assert(res[1] == pub_y); // Test that double function calling embedded_curve_add works as expected - let pub_point = std::embedded_curve_ops::EmbeddedCurvePoint { x: pub_x, y: pub_y }; + let pub_point = std::embedded_curve_ops::EmbeddedCurvePoint { x: pub_x, y: pub_y, is_infinite: false }; let res = pub_point.double(); let double = g1.add(g1); assert(double.x == res.x); // Test calling multi_scalar_mul with multiple points and scalars - let res = std::embedded_curve_ops::multi_scalar_mul([g1.x, g1.y, g1.x, g1.y], [priv_key, 0, priv_key, 0]); + let res = std::embedded_curve_ops::multi_scalar_mul([g1, g1], [scalar, scalar]); // The results should be double the g1 point because the scalars are 1 and we pass in g1 twice assert(double.x == res[0]); diff --git a/noir/noir-repo/test_programs/execution_success/no_predicates_brillig/src/main.nr b/noir/noir-repo/test_programs/execution_success/no_predicates_brillig/src/main.nr index 1d088473aa7..65e2e5d61fe 100644 --- a/noir/noir-repo/test_programs/execution_success/no_predicates_brillig/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/no_predicates_brillig/src/main.nr @@ -1,4 +1,8 @@ unconstrained fn main(x: u32, y: pub u32) { + intermediate_function(x, y); +} + +fn intermediate_function(x: u32, y: u32) { basic_checks(x, y); } diff --git a/noir/noir-repo/test_programs/noir_test_success/embedded_curve_ops/Nargo.toml b/noir/noir-repo/test_programs/noir_test_success/embedded_curve_ops/Nargo.toml new file mode 100644 index 00000000000..65e6efea538 --- /dev/null +++ b/noir/noir-repo/test_programs/noir_test_success/embedded_curve_ops/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "embedded_curve_ops" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/noir_test_success/embedded_curve_ops/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/embedded_curve_ops/src/main.nr new file mode 100644 index 00000000000..9e3c5d87874 --- /dev/null +++ b/noir/noir-repo/test_programs/noir_test_success/embedded_curve_ops/src/main.nr @@ -0,0 +1,37 @@ +use dep::std::embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar, multi_scalar_mul}; + +#[test] + + fn test_infinite_point() { + let zero = EmbeddedCurvePoint::point_at_infinity(); + let zero = EmbeddedCurvePoint { x: 0, y: 0, is_infinite: true }; + let g1 = EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; + let g2 = g1 + g1; + + let s1 = EmbeddedCurveScalar { lo: 1, hi: 0 }; + let a = multi_scalar_mul([g1], [s1]); + assert(a[2] == 0); + assert(g1 + zero == g1); + assert(g1 - g1 == zero); + assert(g1 - zero == g1); + assert(zero + zero == zero); + assert( + multi_scalar_mul([g1], [s1]) + == [1, 17631683881184975370165255887551781615748388533673675138860, 0] + ); + assert(multi_scalar_mul([g1, g1], [s1, s1]) == [g2.x, g2.y, 0]); + assert( + multi_scalar_mul( + [g1, zero], + [EmbeddedCurveScalar { lo: 2, hi: 0 }, EmbeddedCurveScalar { lo: 42, hi: 25 }] + ) + == [g2.x, g2.y, 0] + ); + assert( + multi_scalar_mul( + [g1, g1, zero], + [s1, s1, EmbeddedCurveScalar { lo: 42, hi: 25 }] + ) + == [g2.x, g2.y, 0] + ); +} diff --git a/noir/noir-repo/tooling/lsp/src/solver.rs b/noir/noir-repo/tooling/lsp/src/solver.rs index 249406effaf..87327b01e36 100644 --- a/noir/noir-repo/tooling/lsp/src/solver.rs +++ b/noir/noir-repo/tooling/lsp/src/solver.rs @@ -27,9 +27,13 @@ impl BlackBoxFunctionSolver for WrapperSolver { fn multi_scalar_mul( &self, points: &[acvm::FieldElement], - scalars: &[acvm::FieldElement], - ) -> Result<(acvm::FieldElement, acvm::FieldElement), acvm::BlackBoxResolutionError> { - self.0.multi_scalar_mul(points, scalars) + scalars_lo: &[acvm::FieldElement], + scalars_hi: &[acvm::FieldElement], + ) -> Result< + (acvm::FieldElement, acvm::FieldElement, acvm::FieldElement), + acvm::BlackBoxResolutionError, + > { + self.0.multi_scalar_mul(points, scalars_lo, scalars_hi) } fn pedersen_hash( @@ -44,10 +48,15 @@ impl BlackBoxFunctionSolver for WrapperSolver { &self, input1_x: &acvm::FieldElement, input1_y: &acvm::FieldElement, + input1_infinite: &acvm::FieldElement, input2_x: &acvm::FieldElement, input2_y: &acvm::FieldElement, - ) -> Result<(acvm::FieldElement, acvm::FieldElement), acvm::BlackBoxResolutionError> { - self.0.ec_add(input1_x, input1_y, input2_x, input2_y) + input2_infinite: &acvm::FieldElement, + ) -> Result< + (acvm::FieldElement, acvm::FieldElement, acvm::FieldElement), + acvm::BlackBoxResolutionError, + > { + self.0.ec_add(input1_x, input1_y, input1_infinite, input2_x, input2_y, input2_infinite) } fn poseidon2_permutation( diff --git a/noir/noir-repo/tooling/noirc_abi/src/lib.rs b/noir/noir-repo/tooling/noirc_abi/src/lib.rs index 7e89a102a98..7a1d1787ca5 100644 --- a/noir/noir-repo/tooling/noirc_abi/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_abi/src/lib.rs @@ -471,7 +471,15 @@ impl Abi { .copied() }) { - Some(decode_value(&mut return_witness_values.into_iter(), &return_type.abi_type)?) + // We do not return value for the data bus. + if return_type.visibility == AbiVisibility::DataBus { + None + } else { + Some(decode_value( + &mut return_witness_values.into_iter(), + &return_type.abi_type, + )?) + } } else { // Unlike for the circuit inputs, we tolerate not being able to find the witness values for the return value. // This is because the user may be decoding a partial witness map for which is hasn't been calculated yet.