From 6e363ec51164af8118d8676cdf153a3699909314 Mon Sep 17 00:00:00 2001 From: maramihali Date: Fri, 31 May 2024 20:02:54 +0100 Subject: [PATCH] feat: cycle scalar <> bigfield interactions (#6744) Adds missing methods in cycle_group and cycle_scalar required for the PCS in the ECCVM recursive verifier. As a summary, we add missing cycle_group constructors, a cycle group representation of `one`, a naive method to convert bigfield to cycle_scalar (but that adds constraints) and unify the `batch_mul` interface with the others present in our codebase (i.e. swap points and scalars). --------- Co-authored-by: codygunton --- barretenberg/barretenberg.code-workspace | 42 +++---- .../dsl/acir_format/multi_scalar_mul.cpp | 2 +- .../stdlib/commitment/pedersen/pedersen.cpp | 4 +- .../stdlib/encryption/schnorr/schnorr.cpp | 2 +- .../stdlib/hash/pedersen/pedersen.cpp | 4 +- .../stdlib/primitives/group/cycle_group.cpp | 81 +++++++++++-- .../stdlib/primitives/group/cycle_group.hpp | 21 +++- .../primitives/group/cycle_group.test.cpp | 108 ++++++++++++++++-- 8 files changed, 208 insertions(+), 56 deletions(-) diff --git a/barretenberg/barretenberg.code-workspace b/barretenberg/barretenberg.code-workspace index ac2b6bb11f5..cd08c7a3166 100644 --- a/barretenberg/barretenberg.code-workspace +++ b/barretenberg/barretenberg.code-workspace @@ -58,26 +58,10 @@ "ms-vscode.cpptools" ] }, - "launch": { - // Configure LLDB - "configurations": [ - { - "name": "(lldb) Launch Target", - "type": "lldb", - "request": "launch", - "program": "${command:cmake.launchTargetPath}", - "args": [], - "cwd": "${workspaceFolder}/cpp/build", - "internalConsoleOptions": "openOnSessionStart", - "console": "internalConsole", - } - ], - }, // Global settings which will apply to all subprojects. // Each subproject may have their own `.vscode/settings.json` // for configuring extensions which are specific to a certain project. // Some settings can only be configured here. - // The following are just provided as example. "settings": { "files.associations": { "*.tcc": "cpp", @@ -96,7 +80,6 @@ // // Location of base CMakeLists file "cmake.sourceDirectory": "${workspaceFolder}/cpp/", - "cmake.buildDirectory": "${workspaceFolder}/cpp/build", // // C/C++ (should be disabled) // @@ -116,9 +99,9 @@ // // Ensures tests are run from the `build` directory // which ensures SRS can be read - "testMate.cpp.test.workingDirectory": "${workspaceFolder}/cpp/build", + "testMate.cpp.test.workingDirectory": "${command:cmake.buildDirectory}", // Filter all binaries that are not tests or benchmarks - "testMate.cpp.test.executables": "${workspaceFolder}/cpp/{build}/bin/*{test,Test,TEST,bench}*", + "testMate.cpp.test.executables": "${command:cmake.buildDirectory}/bin/*{test,Test,TEST,bench}*", // // Other // @@ -144,21 +127,24 @@ } ] }, - // - // GTest adapter (not currently used) - // - "gtest-adapter.debugConfig": [ - "(lldb) Launch Target" - ], "[cpp]": { "editor.defaultFormatter": "llvm-vs-code-extensions.vscode-clangd" }, "cmake.configureArgs": [ - "--preset clang16", "-G Ninja" ], - "cmake.useCMakePresets": "auto", + "cmake.useCMakePresets": "always", "editor.inlayHints.enabled": "offUnlessPressed", - "git.detectSubmodules": false + "git.detectSubmodules": false, + "testMate.cpp.discovery.loadOnStartup": false, + "testMate.cpp.debug.configTemplate": { + "type": "lldb", + "MIMode": "lldb", + "program": "${exec}", + "args": "${argsArray}", + "cwd": "${command:cmake.buildDirectory}", + "internalConsoleOptions": "openOnSessionStart", + "console": "internalConsole", + } }, } \ No newline at end of file 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 94f2def1d62..5141ddc69c7 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,7 @@ template void create_multi_scalar_mul_constraint(Builder& bui } // Call batch_mul to multiply the points and scalars and sum the results - auto output_point = cycle_group_ct::batch_mul(scalars, points); + auto output_point = cycle_group_ct::batch_mul(points, scalars); // Add the constraints builder.assert_equal(output_point.x.get_witness_index(), input.out_point_x); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/commitment/pedersen/pedersen.cpp b/barretenberg/cpp/src/barretenberg/stdlib/commitment/pedersen/pedersen.cpp index 7e7b68f2e6c..6ee9f970ef7 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/commitment/pedersen/pedersen.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/commitment/pedersen/pedersen.cpp @@ -22,7 +22,7 @@ cycle_group pedersen_commitment::commit(const std::vector& inputs points.emplace_back(base_points[i]); } - return cycle_group::batch_mul(scalars, points); + return cycle_group::batch_mul(points, scalars); } template @@ -37,7 +37,7 @@ cycle_group pedersen_commitment::commit(const std::vectorget(1, context.offset, context.domain_separator)[0]); } - return cycle_group::batch_mul(scalars, points); + return cycle_group::batch_mul(points, scalars); } template class pedersen_commitment; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/encryption/schnorr/schnorr.cpp b/barretenberg/cpp/src/barretenberg/stdlib/encryption/schnorr/schnorr.cpp index d2e265ba8f0..1a0b8a7bc02 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/encryption/schnorr/schnorr.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/encryption/schnorr/schnorr.cpp @@ -45,7 +45,7 @@ std::array, 2> schnorr_verify_signature_internal(const byte_array& cycle_group g1(grumpkin::g1::one); // compute g1 * sig.s + key * sig,e - auto x_3 = cycle_group::batch_mul({ sig.s, sig.e }, { g1, pub_key }).x; + auto x_3 = cycle_group::batch_mul({ g1, pub_key }, { sig.s, sig.e }).x; // build input (pedersen(([s]g + [e]pub).x | pub.x | pub.y) | message) to hash function // pedersen hash ([r].x | pub.x) to make sure the size of `hash_input` is <= 64 bytes for a 32 byte message byte_array hash_input(pedersen_hash::hash({ x_3, pub_key.x, pub_key.y })); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/pedersen/pedersen.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/pedersen/pedersen.cpp index c525068e354..57005db205b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/pedersen/pedersen.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/pedersen/pedersen.cpp @@ -23,7 +23,7 @@ field_t pedersen_hash::hash(const std::vector& inputs, const Gen points.emplace_back(base_points[i]); } - auto result = cycle_group::batch_mul(scalars, points); + auto result = cycle_group::batch_mul(points, scalars); return result.x; } @@ -47,7 +47,7 @@ field_t pedersen_hash::hash_skip_field_validation(const std::vector::cycle_group(const AffineElement& _in) , context(nullptr) {} +/** + * @brief Construct a cycle_group representation of Group::one. + * + * @tparam Builder + * @param _context + * @return cycle_group + */ +template cycle_group cycle_group::one(Builder* _context) +{ + field_t x(_context, Group::one.x); + field_t y(_context, Group::one.y); + return cycle_group(x, y, false); +} + /** * @brief Converts an AffineElement into a circuit witness. * @@ -540,25 +554,28 @@ cycle_group::cycle_scalar::cycle_scalar(const field_t& _lo, const field , hi(_hi) {} -template cycle_group::cycle_scalar::cycle_scalar(const field_t& _in) +template cycle_group::cycle_scalar::cycle_scalar(const field_t& in) { - const uint256_t value(_in.get_value()); + const uint256_t value(in.get_value()); const uint256_t lo_v = value.slice(0, LO_BITS); const uint256_t hi_v = value.slice(LO_BITS, HI_BITS); constexpr uint256_t shift = uint256_t(1) << LO_BITS; - if (_in.is_constant()) { + if (in.is_constant()) { lo = lo_v; hi = hi_v; } else { - lo = witness_t(_in.get_context(), lo_v); - hi = witness_t(_in.get_context(), hi_v); - (lo + hi * shift).assert_equal(_in); + lo = witness_t(in.get_context(), lo_v); + hi = witness_t(in.get_context(), hi_v); + (lo + hi * shift).assert_equal(in); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1022): ensure lo and hi are in bb::fr modulus not + // bb::fq modulus otherwise we could have two representations for in + validate_scalar_is_in_field(); } } -template cycle_group::cycle_scalar::cycle_scalar(const ScalarField& _in) +template cycle_group::cycle_scalar::cycle_scalar(const ScalarField& in) { - const uint256_t value(_in); + const uint256_t value(in); const uint256_t lo_v = value.slice(0, LO_BITS); const uint256_t hi_v = value.slice(LO_BITS, HI_BITS); lo = lo_v; @@ -627,6 +644,37 @@ typename cycle_group::cycle_scalar cycle_group::cycle_scalar:: cycle_scalar result{ lo, hi, NUM_BITS, skip_primality_test, true }; return result; } +/** + * @brief Construct a new cycle scalar from a bigfield _value, over the same ScalarField Field. If _value is a witness, + * we add constraints to ensure the conversion is correct by reconstructing a bigfield from the limbs of the + * cycle_scalar and checking equality with the initial _value. + * + * @tparam Builder + * @param _value + * @todo (https://github.com/AztecProtocol/barretenberg/issues/1016): Optimise this method + */ +template cycle_group::cycle_scalar::cycle_scalar(BigScalarField& scalar) +{ + auto* ctx = get_context() ? get_context() : scalar.get_context(); + const uint256_t value((scalar.get_value() % uint512_t(ScalarField::modulus)).lo); + const uint256_t value_lo = value.slice(0, LO_BITS); + const uint256_t value_hi = value.slice(LO_BITS, HI_BITS); + if (scalar.is_constant()) { + lo = value_lo; + hi = value_hi; + // N.B. to be able to call assert equal, these cannot be constants + } else { + lo = witness_t(ctx, value_lo); + hi = witness_t(ctx, value_hi); + field_t zero = field_t(0); + zero.convert_constant_to_fixed_witness(ctx); + BigScalarField lo_big(lo, zero); + BigScalarField hi_big(hi, zero); + BigScalarField res = lo_big + hi_big * BigScalarField((uint256_t(1) << LO_BITS)); + scalar.assert_equal(res); + validate_scalar_is_in_field(); + } +}; template bool cycle_group::cycle_scalar::is_constant() const { @@ -1186,8 +1234,8 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ * @return cycle_group */ template -cycle_group cycle_group::batch_mul(const std::vector& scalars, - const std::vector& base_points, +cycle_group cycle_group::batch_mul(const std::vector& base_points, + const std::vector& scalars, const GeneratorContext context) { ASSERT(scalars.size() == base_points.size()); @@ -1323,7 +1371,7 @@ cycle_group cycle_group::batch_mul(const std::vector cycle_group cycle_group::operator*(const cycle_scalar& scalar) const { - return batch_mul({ scalar }, { *this }); + return batch_mul({ *this }, { scalar }); } template cycle_group& cycle_group::operator*=(const cycle_scalar& scalar) @@ -1332,6 +1380,17 @@ template cycle_group& cycle_group::operator return *this; } +template cycle_group cycle_group::operator*(const BigScalarField& scalar) const +{ + return batch_mul({ *this }, { scalar }); +} + +template cycle_group& cycle_group::operator*=(const BigScalarField& scalar) +{ + *this = operator*(scalar); + return *this; +} + template bool_t cycle_group::operator==(const cycle_group& other) const { const auto equal_and_not_infinity = diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp index 70de06b7372..b54333bb1cf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -2,6 +2,7 @@ #include "barretenberg/crypto/pedersen_commitment/pedersen.hpp" #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp" +#include "barretenberg/stdlib/primitives/bigfield/bigfield.hpp" #include "barretenberg/stdlib/primitives/bool/bool.hpp" #include "barretenberg/stdlib/primitives/circuit_builders/circuit_builders.hpp" #include "barretenberg/stdlib/primitives/field/field.hpp" @@ -36,6 +37,7 @@ template class cycle_group { using AffineElement = typename Curve::AffineElement; using GeneratorContext = crypto::GeneratorContext; using ScalarField = typename Curve::ScalarField; + using BigScalarField = stdlib::bigfield; static constexpr size_t STANDARD_NUM_TABLE_BITS = 1; static constexpr size_t ULTRA_NUM_TABLE_BITS = 4; @@ -103,6 +105,8 @@ template class cycle_group { return _use_bn254_scalar_field_for_primality_test; } void validate_scalar_is_in_field() const; + + explicit cycle_scalar(BigScalarField&); }; /** @@ -171,6 +175,7 @@ template class cycle_group { cycle_group(field_t _x, field_t _y, bool_t _is_infinity); cycle_group(const FF& _x, const FF& _y, bool _is_infinity); cycle_group(const AffineElement& _in); + static cycle_group one(Builder* _context); static cycle_group from_witness(Builder* _context, const AffineElement& _in); static cycle_group from_constant_witness(Builder* _context, const AffineElement& _in); Builder* get_context(const cycle_group& other) const; @@ -196,11 +201,23 @@ template class cycle_group { cycle_group operator-() const; cycle_group& operator+=(const cycle_group& other); cycle_group& operator-=(const cycle_group& other); - static cycle_group batch_mul(const std::vector& scalars, - const std::vector& base_points, + static cycle_group batch_mul(const std::vector& base_points, + const std::vector& scalars, + GeneratorContext context = {}) + { + std::vector cycle_scalars; + for (auto scalar : scalars) { + cycle_scalars.emplace_back(scalar); + } + return batch_mul(base_points, cycle_scalars, context); + } + static cycle_group batch_mul(const std::vector& base_points, + const std::vector& scalars, GeneratorContext context = {}); cycle_group operator*(const cycle_scalar& scalar) const; cycle_group& operator*=(const cycle_scalar& scalar); + cycle_group operator*(const BigScalarField& scalar) const; + cycle_group& operator*=(const BigScalarField& scalar); bool_t operator==(const cycle_group& other) const; void assert_equal(const cycle_group& other, std::string const& msg = "cycle_group::assert_equal") const; static cycle_group conditional_assign(const bool_t& predicate, const cycle_group& lhs, const cycle_group& rhs); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index adb74054925..b531a0b2a4a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -3,6 +3,7 @@ #include "barretenberg/crypto/pedersen_commitment/pedersen.hpp" #include "barretenberg/crypto/pedersen_hash/pedersen.hpp" #include "barretenberg/numeric/random/engine.hpp" +#include "barretenberg/stdlib/primitives/bigfield/bigfield.hpp" #include "barretenberg/stdlib/primitives/field/field.hpp" #include "barretenberg/stdlib/primitives/witness/witness.hpp" #include "barretenberg/stdlib_circuit_builders/plookup_tables/fixed_base/fixed_base.hpp" @@ -16,7 +17,8 @@ using AffineElement = typename Curve::AffineElement; \ using Group = typename Curve::Group; \ using bool_ct = stdlib::bool_t; \ - using witness_ct = stdlib::witness_t; + using witness_ct = stdlib::witness_t; \ + using cycle_scalar_ct = cycle_group_ct::cycle_scalar; using namespace bb; @@ -46,7 +48,7 @@ template class CycleGroupTest : public ::testing::Test { }; }; -using CircuitTypes = ::testing::Types; +using CircuitTypes = ::testing::Types; TYPED_TEST_SUITE(CycleGroupTest, CircuitTypes); /** @@ -431,7 +433,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) points.emplace_back(cycle_group_ct(element)); scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); } - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_EQ(result.get_value(), AffineElement(expected)); } @@ -448,7 +450,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) points.emplace_back(cycle_group_ct::from_witness(&builder, element)); scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, -scalar)); - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_TRUE(result.is_point_at_infinity().get_value()); } @@ -461,7 +463,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) typename Group::subgroup_field scalar = 0; points.emplace_back(cycle_group_ct::from_witness(&builder, element)); scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_TRUE(result.is_point_at_infinity().get_value()); } @@ -487,7 +489,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) points.emplace_back(point); scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); } - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_TRUE(result.is_point_at_infinity().get_value()); } @@ -515,7 +517,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); scalars_native.emplace_back(uint256_t(scalar)); } - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_EQ(result.get_value(), AffineElement(expected)); EXPECT_EQ(result.get_value(), crypto::pedersen_commitment::commit_native(scalars_native)); } @@ -552,7 +554,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); scalars_native.emplace_back(scalar); } - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_EQ(result.get_value(), AffineElement(expected)); } @@ -573,7 +575,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) points.emplace_back((element)); scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); } - auto result = cycle_group_ct::batch_mul(scalars, points); + auto result = cycle_group_ct::batch_mul(points, scalars); EXPECT_EQ(result.is_point_at_infinity().get_value(), true); } @@ -620,4 +622,92 @@ TYPED_TEST(CycleGroupTest, TestMul) bool proof_result = CircuitChecker::check(builder); EXPECT_EQ(proof_result, true); } + +TYPED_TEST(CycleGroupTest, TestOne) +{ + STDLIB_TYPE_ALIASES + Builder builder; + cycle_group_ct one = cycle_group_ct::one(&builder); + auto expected_one_native = Group::one; + auto one_native = one.get_value(); + EXPECT_EQ(one_native.x, expected_one_native.x); + EXPECT_EQ(one_native.y, expected_one_native.y); +} + +/** + * @brief Ensures naive conversion from a bigfield representation of bb::fq (Grumpkin::ScalarField) to cycle_scalar + * preserves the same value until we implement a smarter function. + * + */ +TYPED_TEST(CycleGroupTest, TestConversionFromBigfield) +{ + STDLIB_TYPE_ALIASES + using FF = typename Curve::ScalarField; + using FF_ct = stdlib::bigfield; + + const auto run_test = [](bool construct_witnesses) { + Builder builder; + auto elt = FF::random_element(&engine); + FF_ct big_elt; + if (construct_witnesses) { + big_elt = FF_ct::from_witness(&builder, elt); + } else { + big_elt = FF_ct(elt); + } + cycle_scalar_ct scalar_from_big(big_elt); + EXPECT_EQ(elt, scalar_from_big.get_value()); + cycle_scalar_ct scalar_from_elt(big_elt); + EXPECT_EQ(elt, scalar_from_elt.get_value()); + if (construct_witnesses) { + EXPECT_FALSE(big_elt.is_constant()); + EXPECT_FALSE(scalar_from_big.is_constant()); + EXPECT_FALSE(scalar_from_elt.is_constant()); + EXPECT_TRUE(CircuitChecker::check(builder)); + } + }; + run_test(/*construct_witnesses=*/true); + run_test(/*construct_witnesses=*/false); +} + +TYPED_TEST(CycleGroupTest, TestBatchMulIsConsistent) +{ + STDLIB_TYPE_ALIASES + using FF = typename Curve::ScalarField; + using FF_ct = stdlib::bigfield; + + const auto run_test = [](bool construct_witnesses) { + Builder builder; + auto scalar1 = FF::random_element(&engine); + auto scalar2 = FF::random_element(&engine); + + FF_ct big_scalar1; + FF_ct big_scalar2; + if (construct_witnesses) { + big_scalar1 = FF_ct::from_witness(&builder, scalar1); + big_scalar2 = FF_ct::from_witness(&builder, scalar2); + } else { + big_scalar1 = FF_ct(scalar1); + big_scalar2 = FF_ct(scalar2); + } + cycle_group_ct result1 = cycle_group_ct::batch_mul({ TestFixture::generators[0], TestFixture::generators[1] }, + { big_scalar1, big_scalar2 }); + + cycle_group_ct result2 = + cycle_group_ct::batch_mul({ TestFixture::generators[0], TestFixture::generators[1] }, + { cycle_scalar_ct(big_scalar1), cycle_scalar_ct(big_scalar2) }); + + AffineElement result1_native = result1.get_value(); + AffineElement result2_native = result2.get_value(); + EXPECT_EQ(result1_native.x, result2_native.x); + EXPECT_EQ(result1_native.y, result2_native.y); + if (construct_witnesses) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1020): Re-enable these. + // EXPECT_FALSE(result1.is_constant()); + // EXPECT_FALSE(result2.is_constant()); + EXPECT_TRUE(CircuitChecker::check(builder)); + } + }; + run_test(/*construct_witnesses=*/true); + run_test(/*construct_witnesses=*/false); +} #pragma GCC diagnostic pop