diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp index e19c535da31..fa3dfe2864a 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp @@ -61,6 +61,10 @@ 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(); + } + ASSERT(val.on_curve()); return val; } else { // Array or Univariate @@ -95,8 +99,19 @@ 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; + // 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()); + } 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 f1c29250aaf..4a0b2f7bf74 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,25 @@ template class TestAffineElement : public testing::Test { EXPECT_EQ(affine_points[i], -result[i]); } } + + /** + * @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); + } }; -using TestTypes = testing::Types; -// using TestTypes = testing::Types; +// using TestTypes = testing::Types; +using TestTypes = testing::Types; } // namespace TYPED_TEST_SUITE(TestAffineElement, TestTypes); @@ -176,6 +191,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 c1906e40374..f58e6f5b7bd 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 5cc22a57dba..19bc945e476 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp @@ -515,6 +515,9 @@ 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(); + (*this).z = 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 2f8999ada5d..3f06d27aed1 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp @@ -26,12 +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) { 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 size_t frs_per_Fr = bb::field_conversion::calc_num_bn254_frs(); @@ -252,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) { 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 e2821b7789b..fe47fd9469d 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 3f4b4d68886..9b532748a7e 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,79 @@ TEST(RecursiveHonkTranscript, ReturnValuesMatch) EXPECT_EQ(static_cast(native_alpha), stdlib_alpha.get_value()); 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; + 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 }; + 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; + 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 }; + 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); + 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 0cc9f98ba58..a1ca8703c6b 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) { @@ -78,7 +80,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 +90,19 @@ 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 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 db0cca7c0db..993d67b31ea 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,11 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity) } else { context = is_infinity.get_context(); } + + // 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()); } /**