From 0a16e60014aa5f8665e45dd987a8120a2abbfe82 Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 1 Aug 2024 06:36:42 +0000 Subject: [PATCH 1/8] handle point at infinity consistently --- .../ecc/fields/field_conversion.hpp | 16 ++++- .../ecc/groups/affine_element.test.cpp | 22 ++++++- .../ecc/groups/affine_element_impl.hpp | 4 +- .../barretenberg/ecc/groups/element_impl.hpp | 2 + .../eccvm/eccvm_transcript.test.cpp | 3 +- .../eccvm_recursive_verifier.cpp | 6 -- .../transcript/transcript.test.cpp | 63 +++++++++++++++++++ .../primitives/field/field_conversion.hpp | 16 +++-- 8 files changed, 113 insertions(+), 19 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp index e19c535da318..b727ddb7bb2f 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp @@ -61,6 +61,9 @@ template T convert_from_bn254_frs(std::span fr_vec) T val; val.x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); val.y = convert_from_bn254_frs(fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); + if (val.x == BaseField::zero() && val.y == BaseField::zero()) { + val.self_set_infinity(); + } return val; } else { // Array or Univariate @@ -95,8 +98,17 @@ template std::vector convert_to_bn254_frs(const T& val) } else if constexpr (IsAnyOf) { return convert_grumpkin_fr_to_bn254_frs(val); } else if constexpr (IsAnyOf) { - auto fr_vec_x = convert_to_bn254_frs(val.x); - auto fr_vec_y = convert_to_bn254_frs(val.y); + using BaseField = typename T::Fq; + + std::vector fr_vec_x; + std::vector fr_vec_y; + if (val.is_point_at_infinity()) { + fr_vec_x = convert_to_bn254_frs(BaseField::zero()); + fr_vec_y = convert_to_bn254_frs(BaseField::zero()); + } else { + fr_vec_x = convert_to_bn254_frs(val.x); + fr_vec_y = convert_to_bn254_frs(val.y); + } std::vector fr_vec(fr_vec_x.begin(), fr_vec_x.end()); fr_vec.insert(fr_vec.end(), fr_vec_y.begin(), fr_vec_y.end()); return fr_vec; diff --git a/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp b/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp index f1c29250aaf0..9e60c134f917 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp @@ -149,10 +149,19 @@ template class TestAffineElement : public testing::Test { EXPECT_EQ(affine_points[i], -result[i]); } } + + static void test_fixed_point_at_infinity() + { + using Fq = affine_element::Fq; + affine_element P = affine_element::infinity(); + affine_element Q(Fq::zero(), Fq::zero()); + Q.x.self_set_msb(); + EXPECT_EQ(P, Q); + } }; -using TestTypes = testing::Types; -// using TestTypes = testing::Types; +// using TestTypes = testing::Types; +using TestTypes = testing::Types; } // namespace TYPED_TEST_SUITE(TestAffineElement, TestTypes); @@ -176,6 +185,15 @@ TYPED_TEST(TestAffineElement, PointCompression) } } +TYPED_TEST(TestAffineElement, FixedInfinityPoint) +{ + if constexpr (TypeParam::Fq::modulus.data[3] >= 0x4000000000000000ULL) { + GTEST_SKIP(); + } else { + TestFixture::test_fixed_point_at_infinity(); + } +} + TYPED_TEST(TestAffineElement, PointCompressionUnsafe) { if constexpr (TypeParam::Fq::modulus.data[3] >= 0x4000000000000000ULL) { diff --git a/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp b/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp index c1906e403749..f58e6f5b7bd8 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp @@ -82,7 +82,7 @@ constexpr uint256_t affine_element::compress() const noexcept template affine_element affine_element::infinity() { - affine_element e; + affine_element e{}; e.self_set_infinity(); return e; } @@ -105,6 +105,8 @@ template constexpr void affine_element: x.data[3] = Fq::modulus.data[3]; } else { + (*this).x = Fq::zero(); + (*this).y = Fq::zero(); x.self_set_msb(); } } diff --git a/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp b/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp index 5cc22a57dba6..1a4f6a4f5678 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp @@ -515,6 +515,8 @@ template constexpr void element::self_s x.data[2] = Fq::modulus.data[2]; x.data[3] = Fq::modulus.data[3]; } else { + (*this).x = Fq::zero(); + (*this).y = Fq::zero(); x.self_set_msb(); } } diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp index 2f8999ada5da..889418a3cb2d 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp @@ -26,11 +26,10 @@ class ECCVMTranscriptTests : public ::testing::Test { * * @return TranscriptManifest */ - TranscriptManifest construct_eccvm_honk_manifest(size_t circuit_size, size_t log_ipa_poly_degree) + TranscriptManifest construct_eccvm_honk_manifest(size_t circuit_size, [[maybe_unused]] size_t log_ipa_poly_degree) { TranscriptManifest manifest_expected; auto log_n = numeric::get_msb(circuit_size); - ASSERT(log_n == log_ipa_poly_degree); size_t MAX_PARTIAL_RELATION_LENGTH = Flavor::BATCHED_RELATION_PARTIAL_LENGTH; // Size of types is number of bb::frs needed to represent the type diff --git a/barretenberg/cpp/src/barretenberg/eccvm_recursion/eccvm_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/eccvm_recursion/eccvm_recursive_verifier.cpp index e2821b7789b1..fe47fd9469d0 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm_recursion/eccvm_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm_recursion/eccvm_recursive_verifier.cpp @@ -38,12 +38,6 @@ template void ECCVMRecursiveVerifier_::verify_proof(co for (auto [comm, label] : zip_view(commitments.get_wires(), commitment_labels.get_wires())) { comm = transcript->template receive_from_prover(label); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1017): This is a hack to ensure zero commitments - // are still on curve as the transcript doesn't currently support a point at infinity representation for - // cycle_group - if (!comm.get_value().on_curve()) { - comm.set_point_at_infinity(true); - } } // Get challenge for sorted list batching and wire four memory records diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp index 3f4b4d68886c..f04edb5a8c9a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp @@ -179,4 +179,67 @@ TEST(RecursiveHonkTranscript, ReturnValuesMatch) EXPECT_EQ(static_cast(native_alpha), stdlib_alpha.get_value()); EXPECT_EQ(static_cast(native_beta), stdlib_beta.get_value()); } + +TEST(RecursiveTranscript, InfinityConsistencyGrumpkin) +{ + using NativeCurve = curve::Grumpkin; + using NativeCommitment = typename NativeCurve::AffineElement; + using NativeFF = NativeCurve::ScalarField; + + using FF = bigfield; + using Commitment = cycle_group; + + Builder builder; + + NativeCommitment infinity = NativeCommitment::infinity(); + + NativeTranscript prover_transcript; + prover_transcript.send_to_verifier("infinity", infinity); + NativeFF challenge = prover_transcript.get_challenge("challenge"); + auto proof_data = prover_transcript.proof_data; + + NativeTranscript verifier_transcript(proof_data); + verifier_transcript.receive_from_prover("infinity"); + auto verifier_challenge = verifier_transcript.get_challenge("challenge"); + + StdlibProof stdlib_proof = bb::convert_proof_to_witness(&builder, proof_data); + StdlibTranscript stdlib_transcript{ stdlib_proof }; + stdlib_transcript.receive_from_prover("infinity"); + auto stdlib_challenge = stdlib_transcript.get_challenge("challenge"); + + EXPECT_EQ(challenge, verifier_challenge); + EXPECT_EQ(verifier_challenge, NativeFF(stdlib_challenge.get_value() % FF::modulus)); +} + +TEST(RecursiveTranscript, InfinityConsistencyBN254) +{ + using NativeCurve = curve::BN254; + using NativeCommitment = typename NativeCurve::AffineElement; + using NativeFF = NativeCurve::ScalarField; + + using FF = field_t; + using BF = bigfield; + using Commitment = element; + + Builder builder; + + NativeCommitment infinity = NativeCommitment::infinity(); + + NativeTranscript prover_transcript; + prover_transcript.send_to_verifier("infinity", infinity); + NativeFF challenge = prover_transcript.get_challenge("challenge"); + auto proof_data = prover_transcript.proof_data; + + NativeTranscript verifier_transcript(proof_data); + verifier_transcript.receive_from_prover("infinity"); + auto verifier_challenge = verifier_transcript.get_challenge("challenge"); + + StdlibProof stdlib_proof = bb::convert_proof_to_witness(&builder, proof_data); + StdlibTranscript stdlib_transcript{ stdlib_proof }; + stdlib_transcript.receive_from_prover("infinity"); + auto stdlib_challenge = stdlib_transcript.get_challenge("challenge"); + + EXPECT_EQ(challenge, verifier_challenge); + EXPECT_EQ(verifier_challenge, stdlib_challenge.get_value()); +} } // namespace bb::stdlib::recursion::honk \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 0cc9f98ba581..aa9ffe26fc19 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -78,7 +78,7 @@ template T convert_from_bn254_frs(Builder& builde return fr_vec[0]; } else if constexpr (IsAnyOf>) { ASSERT(fr_vec.size() == 2); - fq result(fr_vec[0], fr_vec[1], 0, 0); + fq result(fr_vec[0], fr_vec[1]); return result; } else if constexpr (IsAnyOf>) { using BaseField = fq; @@ -88,16 +88,20 @@ template T convert_from_bn254_frs(Builder& builde result.x = convert_from_bn254_frs(builder, fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); result.y = convert_from_bn254_frs( builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); + + result.set_point_at_infinity(fr_vec[0].is_zero() && fr_vec[1].is_zero() && fr_vec[2].is_zero() && + fr_vec[3].is_zero()); + return result; } else if constexpr (IsAnyOf>) { using BaseField = fr; constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); ASSERT(fr_vec.size() == 2 * BASE_FIELD_SCALAR_SIZE); - grumpkin_element result( - convert_from_bn254_frs>(builder, fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)), - convert_from_bn254_frs>( - builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)), - false); + fr x = + convert_from_bn254_frs>(builder, fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); + fr y = convert_from_bn254_frs>( + builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); + grumpkin_element result(x, y, x.is_zero() && y.is_zero()); return result; } else { // Array or Univariate From 714455297e41e362ab6b4803a6bbf9d73f5608bf Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 1 Aug 2024 07:40:51 +0000 Subject: [PATCH 2/8] handle biggroup and add test for bn254 --- .../transcript/transcript.test.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp index f04edb5a8c9a..9b532748a7e0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_recursion/transcript/transcript.test.cpp @@ -180,6 +180,11 @@ TEST(RecursiveHonkTranscript, ReturnValuesMatch) EXPECT_EQ(static_cast(native_beta), stdlib_beta.get_value()); } +/** + * @brief Ensure that when encountering an infinity commitment results stay consistent in the recursive and native case + * for Grumpkin and the native and stdlib transcripts produce the same challenge. + * @todo(https://github.com/AztecProtocol/barretenberg/issues/1064) Add more transcript tests for both curves + */ TEST(RecursiveTranscript, InfinityConsistencyGrumpkin) { using NativeCurve = curve::Grumpkin; @@ -204,13 +209,19 @@ TEST(RecursiveTranscript, InfinityConsistencyGrumpkin) StdlibProof stdlib_proof = bb::convert_proof_to_witness(&builder, proof_data); StdlibTranscript stdlib_transcript{ stdlib_proof }; - stdlib_transcript.receive_from_prover("infinity"); + auto stdlib_infinity = stdlib_transcript.receive_from_prover("infinity"); + EXPECT_TRUE(stdlib_infinity.is_point_at_infinity().get_value()); auto stdlib_challenge = stdlib_transcript.get_challenge("challenge"); EXPECT_EQ(challenge, verifier_challenge); EXPECT_EQ(verifier_challenge, NativeFF(stdlib_challenge.get_value() % FF::modulus)); } +/** + * @brief Ensure that when encountering an infinity commitment results stay consistent in the recursive and native case + * for BN254 and the native and stdlib transcripts produce the same challenge. + * @todo(https://github.com/AztecProtocol/barretenberg/issues/1064) Add more transcript tests for both curves + */ TEST(RecursiveTranscript, InfinityConsistencyBN254) { using NativeCurve = curve::BN254; @@ -236,7 +247,8 @@ TEST(RecursiveTranscript, InfinityConsistencyBN254) StdlibProof stdlib_proof = bb::convert_proof_to_witness(&builder, proof_data); StdlibTranscript stdlib_transcript{ stdlib_proof }; - stdlib_transcript.receive_from_prover("infinity"); + auto stdlib_commitment = stdlib_transcript.receive_from_prover("infinity"); + EXPECT_TRUE(stdlib_commitment.is_point_at_infinity().get_value()); auto stdlib_challenge = stdlib_transcript.get_challenge("challenge"); EXPECT_EQ(challenge, verifier_challenge); From 65e96133d49fb92798a6b5db4599065e6063eef5 Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 1 Aug 2024 07:50:42 +0000 Subject: [PATCH 3/8] cleanup --- .../cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp index 889418a3cb2d..3f06d27aed11 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp @@ -26,11 +26,10 @@ class ECCVMTranscriptTests : public ::testing::Test { * * @return TranscriptManifest */ - TranscriptManifest construct_eccvm_honk_manifest(size_t circuit_size, [[maybe_unused]] size_t log_ipa_poly_degree) + TranscriptManifest construct_eccvm_honk_manifest(size_t circuit_size) { TranscriptManifest manifest_expected; auto log_n = numeric::get_msb(circuit_size); - size_t MAX_PARTIAL_RELATION_LENGTH = Flavor::BATCHED_RELATION_PARTIAL_LENGTH; // Size of types is number of bb::frs needed to represent the type size_t frs_per_Fr = bb::field_conversion::calc_num_bn254_frs(); @@ -251,8 +250,7 @@ TEST_F(ECCVMTranscriptTests, ProverManifestConsistency) auto proof = prover.construct_proof(); // Check that the prover generated manifest agrees with the manifest hard coded in this suite - auto manifest_expected = - this->construct_eccvm_honk_manifest(prover.key->circuit_size, prover.sumcheck_output.challenge.size()); + auto manifest_expected = this->construct_eccvm_honk_manifest(prover.key->circuit_size); auto prover_manifest = prover.transcript->get_manifest(); // Note: a manifest can be printed using manifest.print() for (size_t round = 0; round < manifest_expected.size(); ++round) { From b6e27b98c11170e4ca3c868ca20afd0b1edaac90 Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 1 Aug 2024 11:11:42 +0000 Subject: [PATCH 4/8] resolve review comments --- .../cpp/src/barretenberg/ecc/fields/field_conversion.hpp | 3 +++ .../cpp/src/barretenberg/ecc/groups/affine_element.test.cpp | 6 ++++++ .../cpp/src/barretenberg/ecc/groups/element_impl.hpp | 1 + .../stdlib/primitives/field/field_conversion.hpp | 3 ++- .../barretenberg/stdlib/primitives/group/cycle_group.cpp | 1 + 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp index b727ddb7bb2f..fa3dfe2864a1 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp @@ -64,6 +64,7 @@ template T convert_from_bn254_frs(std::span fr_vec) if (val.x == BaseField::zero() && val.y == BaseField::zero()) { val.self_set_infinity(); } + ASSERT(val.on_curve()); return val; } else { // Array or Univariate @@ -102,6 +103,8 @@ template std::vector convert_to_bn254_frs(const T& val) std::vector fr_vec_x; std::vector fr_vec_y; + // When encountering a point at infinity we pass a zero point in the proof to ensure that on the receiving size + // there are no inconsistencies whenre constructing and hashing. if (val.is_point_at_infinity()) { fr_vec_x = convert_to_bn254_frs(BaseField::zero()); fr_vec_y = convert_to_bn254_frs(BaseField::zero()); diff --git a/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp b/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp index 9e60c134f917..4a0b2f7bf74b 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp @@ -150,13 +150,19 @@ template class TestAffineElement : public testing::Test { } } + /** + * @brief Ensure that the point at inifinity has a fixed value. + * + */ static void test_fixed_point_at_infinity() { using Fq = affine_element::Fq; affine_element P = affine_element::infinity(); affine_element Q(Fq::zero(), Fq::zero()); Q.x.self_set_msb(); + affine_element R = affine_element(element::random_element()); EXPECT_EQ(P, Q); + EXPECT_NE(P, R); } }; diff --git a/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp b/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp index 1a4f6a4f5678..19bc945e4762 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp @@ -517,6 +517,7 @@ template constexpr void element::self_s } else { (*this).x = Fq::zero(); (*this).y = Fq::zero(); + (*this).z = Fq::zero(); x.self_set_msb(); } } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index aa9ffe26fc19..014851a611f9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -91,7 +91,7 @@ template T convert_from_bn254_frs(Builder& builde result.set_point_at_infinity(fr_vec[0].is_zero() && fr_vec[1].is_zero() && fr_vec[2].is_zero() && fr_vec[3].is_zero()); - + result.validate_on_curve(); return result; } else if constexpr (IsAnyOf>) { using BaseField = fr; @@ -102,6 +102,7 @@ template T convert_from_bn254_frs(Builder& builde fr y = convert_from_bn254_frs>( builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); grumpkin_element result(x, y, x.is_zero() && y.is_zero()); + result.validate_is_on_curve(); return result; } else { // Array or Univariate diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index db0cca7c0db5..56428340f734 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -38,6 +38,7 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity) } else { context = is_infinity.get_context(); } + ASSERT(get_value().on_curve()); } /** From dc486a06ae58e1e54a52f9267874b83a7c1fe9dc Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 1 Aug 2024 12:06:28 +0000 Subject: [PATCH 5/8] bump circuit size --- .../cpp/src/barretenberg/client_ivc/client_ivc.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp index 5a977dd75bfe..ae656e6f6995 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp @@ -50,7 +50,7 @@ class ClientIVCTests : public ::testing::Test { * polynomials will bump size to next power of 2) * */ - static Builder create_mock_circuit(ClientIVC& ivc, size_t log2_num_gates = 15) + static Builder create_mock_circuit(ClientIVC& ivc, size_t log2_num_gates = 16) { Builder circuit{ ivc.goblin.op_queue }; MockCircuits::construct_arithmetic_circuit(circuit, log2_num_gates); From b56cb858feb61ea4a138126ad204928d0610e6ba Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 1 Aug 2024 12:36:05 +0000 Subject: [PATCH 6/8] add issue to optimise validate on curve --- .../cpp/src/barretenberg/client_ivc/client_ivc.test.cpp | 2 +- .../barretenberg/stdlib/primitives/field/field_conversion.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp index ae656e6f6995..5a977dd75bfe 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp @@ -50,7 +50,7 @@ class ClientIVCTests : public ::testing::Test { * polynomials will bump size to next power of 2) * */ - static Builder create_mock_circuit(ClientIVC& ivc, size_t log2_num_gates = 16) + static Builder create_mock_circuit(ClientIVC& ivc, size_t log2_num_gates = 15) { Builder circuit{ ivc.goblin.op_queue }; MockCircuits::construct_arithmetic_circuit(circuit, log2_num_gates); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 014851a611f9..a1ca8703c6ba 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -70,6 +70,8 @@ template constexpr size_t calc_num_bn254_frs() * @param builder * @param fr_vec * @return T + * @todo https://github.com/AztecProtocol/barretenberg/issues/1065 optimise validate_on_curve and check points + * reconstructed from the transcript */ template T convert_from_bn254_frs(Builder& builder, std::span> fr_vec) { @@ -91,7 +93,6 @@ template T convert_from_bn254_frs(Builder& builde result.set_point_at_infinity(fr_vec[0].is_zero() && fr_vec[1].is_zero() && fr_vec[2].is_zero() && fr_vec[3].is_zero()); - result.validate_on_curve(); return result; } else if constexpr (IsAnyOf>) { using BaseField = fr; @@ -102,7 +103,6 @@ template T convert_from_bn254_frs(Builder& builde fr y = convert_from_bn254_frs>( builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); grumpkin_element result(x, y, x.is_zero() && y.is_zero()); - result.validate_is_on_curve(); return result; } else { // Array or Univariate From 11592c9b0592feac83edf4b1b42e791abb54faf7 Mon Sep 17 00:00:00 2001 From: maramihali Date: Fri, 2 Aug 2024 07:00:32 +0000 Subject: [PATCH 7/8] fix issue in schnorr --- .../cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp index e65224b429c5..351c1835e13f 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp @@ -92,7 +92,9 @@ template void create_schnorr_verify_constraints(Builder& buil fr pubkey_value_x = builder.get_variable(input.public_key_x); fr pubkey_value_y = builder.get_variable(input.public_key_y); - cycle_group_ct pub_key{ witness_ct(&builder, pubkey_value_x), witness_ct(&builder, pubkey_value_y), false }; + cycle_group_ct pub_key{ witness_ct(&builder, pubkey_value_x), + witness_ct(&builder, pubkey_value_y), + pubkey_value_x.is_zero() && pubkey_value_y.is_zero() }; schnorr_signature_bits_ct sig = schnorr_convert_signature(&builder, new_sig); From c60ac06d8d0267aff4e06f8e8bee2c77e705c75a Mon Sep 17 00:00:00 2001 From: maramihali Date: Fri, 2 Aug 2024 08:13:17 +0000 Subject: [PATCH 8/8] add issue to investigate bad schnorr input and enable assert --- .../cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp | 4 +--- .../barretenberg/stdlib/primitives/group/cycle_group.cpp | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp index 351c1835e13f..e65224b429c5 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/schnorr_verify.cpp @@ -92,9 +92,7 @@ template void create_schnorr_verify_constraints(Builder& buil fr pubkey_value_x = builder.get_variable(input.public_key_x); fr pubkey_value_y = builder.get_variable(input.public_key_y); - cycle_group_ct pub_key{ witness_ct(&builder, pubkey_value_x), - witness_ct(&builder, pubkey_value_y), - pubkey_value_x.is_zero() && pubkey_value_y.is_zero() }; + cycle_group_ct pub_key{ witness_ct(&builder, pubkey_value_x), witness_ct(&builder, pubkey_value_y), false }; schnorr_signature_bits_ct sig = schnorr_convert_signature(&builder, new_sig); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 56428340f734..993d67b31ea0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -38,7 +38,11 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity) } else { context = is_infinity.get_context(); } - ASSERT(get_value().on_curve()); + + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1067): This ASSERT is missing in the constructor but + // causes schnorr acir test to fail due to a bad input (a public key that has x and y coordinate set to 0). + // Investigate this to be able to enable the test. + // ASSERT(get_value().on_curve()); } /**