From 6dccf3d06e159b0f994e5d670192e3091a8eb900 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 25 Feb 2024 18:58:27 +0000 Subject: [PATCH 1/3] Add assert to detect PI in transcript and fix Goblin Translator Composer tests --- .../src/barretenberg/ecc/fields/field_conversion.hpp | 1 + .../translator_vm/goblin_translator_composer.test.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp index 8e4962dd525b..06980212c971 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp @@ -95,6 +95,7 @@ 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) { + ASSERT(!val.is_point_at_infinity()); auto fr_vec_x = convert_to_bn254_frs(val.x); auto fr_vec_y = convert_to_bn254_frs(val.y); std::vector fr_vec(fr_vec_x.begin(), fr_vec_x.end()); diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp index 0bd890ccd34a..31d3edef4e18 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp @@ -49,12 +49,23 @@ TEST_F(GoblinTranslatorComposerTests, Basic) using Fr = fr; using Fq = fq; + // And an element and scalar the accumulation of which leaves no Point-at-Infinity commitments + const auto x = uint256_t(0xd3c208c16d87cfd3, 0xd97816a916871ca8, 0x9b85045b68181585, 0x30644e72e131a02); + const auto y = uint256_t(0x3ce1cc9c7e645a83, 0x2edac647851e3ac5, 0xd0cbe61fced2bc53, 0x1a76dae6d3272396); + auto padding_element = G1(x, y); + auto padding_scalar = -Fr::one(); + auto P1 = G1::random_element(); auto P2 = G1::random_element(); auto z = Fr::random_element(); // Add the same operations to the ECC op queue; the native computation is performed under the hood. auto op_queue = std::make_shared(); + + // Accumulate padding for non-PI commitments + op_queue->mul_accumulate(padding_element, padding_scalar); + + // Push everything else for (size_t i = 0; i < 500; i++) { op_queue->add_accumulate(P1); op_queue->mul_accumulate(P2, z); From b35644dc15019b0b8779875c31829a7cafc2db34 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 25 Feb 2024 19:19:17 +0000 Subject: [PATCH 2/3] Remove point at infinity detection --- .../cpp/src/barretenberg/ecc/fields/field_conversion.hpp | 1 - 1 file changed, 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 06980212c971..8e4962dd525b 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp @@ -95,7 +95,6 @@ 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) { - ASSERT(!val.is_point_at_infinity()); auto fr_vec_x = convert_to_bn254_frs(val.x); auto fr_vec_y = convert_to_bn254_frs(val.y); std::vector fr_vec(fr_vec_x.begin(), fr_vec_x.end()); From c58445ed962b35f8916b2d320bb28bee65f19b72 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 25 Feb 2024 22:05:59 +0000 Subject: [PATCH 3/3] Address Mara's comments --- .../translator_vm/goblin_translator_composer.test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp index 31d3edef4e18..fb1c92167944 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.test.cpp @@ -49,7 +49,7 @@ TEST_F(GoblinTranslatorComposerTests, Basic) using Fr = fr; using Fq = fq; - // And an element and scalar the accumulation of which leaves no Point-at-Infinity commitments + // Add an element and scalar the accumulation of which leaves no Point-at-Infinity commitments const auto x = uint256_t(0xd3c208c16d87cfd3, 0xd97816a916871ca8, 0x9b85045b68181585, 0x30644e72e131a02); const auto y = uint256_t(0x3ce1cc9c7e645a83, 0x2edac647851e3ac5, 0xd0cbe61fced2bc53, 0x1a76dae6d3272396); auto padding_element = G1(x, y); @@ -62,7 +62,8 @@ TEST_F(GoblinTranslatorComposerTests, Basic) // Add the same operations to the ECC op queue; the native computation is performed under the hood. auto op_queue = std::make_shared(); - // Accumulate padding for non-PI commitments + // Accumulate padding so that we don't produce Point-at-Infinity commitments. Currently our transcript can't handle + // them op_queue->mul_accumulate(padding_element, padding_scalar); // Push everything else