From 898f85bdc0f0e818c7abf8714732aa51498708b0 Mon Sep 17 00:00:00 2001 From: MirandaWood Date: Thu, 30 Apr 2026 15:49:55 +0000 Subject: [PATCH 1/2] feat: first sweep removing inf from point wrapper constructor --- .../avm_fuzzer/harness/ecc.fuzzer.cpp | 4 +- .../vm2/common/standard_affine_point.hpp | 12 ++-- .../vm2/common/standard_affine_point.test.cpp | 42 +++++++------- .../vm2/constraining/relations/ecc.test.cpp | 56 +++++++++---------- .../vm2/simulation/gadgets/ecc.cpp | 4 +- .../vm2/simulation/gadgets/ecc.test.cpp | 32 +++++------ .../vm2/simulation/gadgets/execution.cpp | 4 +- .../vm2/simulation/gadgets/execution.test.cpp | 4 +- .../vm2/tracegen/ecc_trace.test.cpp | 16 +++--- 9 files changed, 82 insertions(+), 92 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/avm_fuzzer/harness/ecc.fuzzer.cpp b/barretenberg/cpp/src/barretenberg/avm_fuzzer/harness/ecc.fuzzer.cpp index 19e79a8078ed..7298fc3f3e85 100644 --- a/barretenberg/cpp/src/barretenberg/avm_fuzzer/harness/ecc.fuzzer.cpp +++ b/barretenberg/cpp/src/barretenberg/avm_fuzzer/harness/ecc.fuzzer.cpp @@ -288,13 +288,11 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) // Verify output in memory MemoryValue res_x = mem->get(input.addresses[6]); MemoryValue res_y = mem->get(input.addresses[6] + 1); - MemoryValue res_inf = mem->get(input.addresses[6] + 2); - EmbeddedCurvePoint result_point = EmbeddedCurvePoint(res_x.as_ff(), res_y.as_ff(), res_inf.as_ff() == FF(1)); + EmbeddedCurvePoint result_point = EmbeddedCurvePoint(res_x.as_ff(), res_y.as_ff()); BB_ASSERT(result_point.x() == expected_result.x(), "Result x-coordinate mismatch"); BB_ASSERT(result_point.y() == expected_result.y(), "Result y-coordinate mismatch"); - BB_ASSERT(result_point.is_infinity() == expected_result.is_infinity(), "Result infinity flag mismatch"); // Non mem-aware ecmul result: expected_result = point_p * input.scalar; diff --git a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp index c469a822000a..0c0e840881bd 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp @@ -32,12 +32,10 @@ template class StandardAffinePoint { , y_coord(val.is_point_at_infinity() ? zero : val.y) {} - // TODO(MW): Do we want to silently discard input x, y if is_infinity = true? - // Or, discard is_infinity and set point to AffinePoint::infinity() iff x = y = 0? - constexpr StandardAffinePoint(BaseField x, BaseField y, bool is_infinity) noexcept - : point(is_infinity ? AffinePoint::infinity() : AffinePoint(x, y)) - , x_coord(is_infinity ? zero : x) - , y_coord(is_infinity ? zero : y) + constexpr StandardAffinePoint(BaseField x, BaseField y) noexcept + : point((x.is_zero() && y.is_zero()) ? AffinePoint::infinity() : AffinePoint(x, y)) + , x_coord(x) + , y_coord(y) {} constexpr StandardAffinePoint operator+(const StandardAffinePoint& other) const noexcept @@ -87,7 +85,7 @@ template class StandardAffinePoint { static const StandardAffinePoint& infinity() { - static auto infinity = StandardAffinePoint(zero, zero, true); + static auto infinity = StandardAffinePoint(zero, zero); return infinity; } diff --git a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp index 4a8ae5a65e1f..7a5b26ed3434 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp @@ -10,25 +10,25 @@ using EmbeddedCurvePoint = StandardAffinePoint; using Fr = grumpkin::fr; using Fq = grumpkin::fq; -TEST(StandardAffinePointTest, InfinityDiscardsRawCoordinates) -{ - // NOTE: As of #AVM-248, we moved from preserving raw coordinates in - // infinity points to our (0,0) representation when using x() and y(). - // The underlying AffinePoint is set to AffinePoint::infinity() for - // bb operations. - - // When constructing an infinity point with non-zero coordinates, - // x() and y() should return our standard representation. - Fq raw_x = 1; - Fq raw_y = 2; - - // Note that raw x and y are silently discarded. - EmbeddedCurvePoint point(raw_x, raw_y, /*is_infinity=*/true); - - EXPECT_TRUE(point.is_infinity()); - EXPECT_TRUE(point.x().is_zero()); - EXPECT_TRUE(point.y().is_zero()); -} +// TEST(StandardAffinePointTest, InfinityDiscardsRawCoordinates) +// { +// // NOTE: As of #AVM-248, we moved from preserving raw coordinates in +// // infinity points to our (0,0) representation when using x() and y(). +// // The underlying AffinePoint is set to AffinePoint::infinity() for +// // bb operations. + +// // When constructing an infinity point with non-zero coordinates, +// // x() and y() should return our standard representation. +// Fq raw_x = 1; +// Fq raw_y = 2; + +// // Note that raw x and y are silently discarded. +// EmbeddedCurvePoint point(raw_x, raw_y); + +// EXPECT_TRUE(point.is_infinity()); +// EXPECT_TRUE(point.x().is_zero()); +// EXPECT_TRUE(point.y().is_zero()); +// } TEST(StandardAffinePointTest, NormalPointCoordinates) { @@ -88,8 +88,8 @@ TEST(StandardAffinePointTest, StaticInfinityHasZeroCoordinates) TEST(StandardAffinePointTest, NegatingInfinity) { - // Negating an infinity point should return (0,0,true) - EmbeddedCurvePoint inf(0, 0, /*is_infinity=*/true); + // Negating an infinity point should return (0,0) + EmbeddedCurvePoint inf(0, 0); auto neg_inf = -inf; diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp index 7be0decf60d2..eab2a3887afc 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp @@ -64,11 +64,11 @@ using simulation::ToRadixMemoryEvent; // Known good points for P and Q FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); -EmbeddedCurvePoint p(p_x, p_y, false); +EmbeddedCurvePoint p(p_x, p_y); FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3"); -EmbeddedCurvePoint q(q_x, q_y, false); +EmbeddedCurvePoint q(q_x, q_y); TEST(EccAddConstrainingTest, EccEmptyRow) { @@ -80,7 +80,7 @@ TEST(EccAddConstrainingTest, EccAdd) // R = P + Q; FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6"); FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09"); - EmbeddedCurvePoint r(r_x, r_y, false); + EmbeddedCurvePoint r(r_x, r_y); auto trace = TestTraceContainer({ { { C::ecc_add_op, 1 }, @@ -124,7 +124,7 @@ TEST(EccAddConstrainingTest, EccDouble) // R = P + P; FF r_x("0x088b996194bb5e6e8e5e49733bb671c3e660cf77254f743f366cc8e33534ee3b"); FF r_y("0x2807ffa01c0f522d0be1e1acfb6914ac8eabf1acf420c0629d37beee992e9a0e"); - EmbeddedCurvePoint r(r_x, r_y, false); + EmbeddedCurvePoint r(r_x, r_y); auto trace = TestTraceContainer({ { { C::ecc_add_op, 0 }, @@ -174,13 +174,13 @@ TEST(EccAddConstrainingTest, EccAddSameYDifferentX) // Point P - known valid point on Grumpkin FF local_p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF local_p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint local_p(local_p_x, local_p_y, false); + EmbeddedCurvePoint local_p(local_p_x, local_p_y); // Point Q - p_x * omega (cube root of unity), same y-coordinate! // omega = 0x0000000000000000b3c4d79d41a917585bfc41088d8daaa78b17ea66b99c90dd FF local_q_x("0x14dd39aa19e1c8b29e0c530a28106a7d64d2213486baba3c86dce51bdddf75bb"); FF local_q_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint local_q(local_q_x, local_q_y, false); + EmbeddedCurvePoint local_q(local_q_x, local_q_y); // Verify preconditions: same y, different x ASSERT_NE(local_p.x(), local_q.x()); @@ -189,7 +189,7 @@ TEST(EccAddConstrainingTest, EccAddSameYDifferentX) // Expected result R = P + Q (lambda = 0 since y's are equal) FF local_r_x("0x16bdb7ada0799a3088b9dd3faade12c3f79dbfe9cb1234783a1a7add546398dc"); FF local_r_y("0x2d08e098faf58cb97223d13f2a1b87dd6614173f3cefe87ca6a74e3034c244a1"); - EmbeddedCurvePoint local_r(local_r_x, local_r_y, false); + EmbeddedCurvePoint local_r(local_r_x, local_r_y); // Use simulation to generate events EventEmitter ecc_add_event_emitter; @@ -222,8 +222,8 @@ TEST(EccAddConstrainingTest, EccAddSameYDifferentX) TEST(EccAddConstrainingTest, EccAddResultingInInfinity) { // R = P + (-P) = O; , where O is the point at infinity - EmbeddedCurvePoint q(p.x(), -p.y(), false); - EmbeddedCurvePoint r(0, 0, true); + EmbeddedCurvePoint q(p.x(), -p.y()); + EmbeddedCurvePoint r(0, 0); auto trace = TestTraceContainer({ { { C::ecc_add_op, 0 }, @@ -262,11 +262,11 @@ TEST(EccAddConstrainingTest, EccAddResultingInInfinity) TEST(EccAddConstrainingTest, EccAddingToInfinity) { - EmbeddedCurvePoint p(0, 0, true); + EmbeddedCurvePoint p(0, 0); // R = O + Q = Q; , where O is the point at infinity - EmbeddedCurvePoint r(q.x(), q.y(), false); + EmbeddedCurvePoint r(q.x(), q.y()); auto trace = TestTraceContainer({ { { C::ecc_add_op, 1 }, @@ -305,10 +305,10 @@ TEST(EccAddConstrainingTest, EccAddingToInfinity) TEST(EccAddConstrainingTest, EccAddingInfinity) { - EmbeddedCurvePoint q(0, 0, true); + EmbeddedCurvePoint q(0, 0); // R = P + O = P; , where O is the point at infinity - EmbeddedCurvePoint r(p.x(), p.y(), false); + EmbeddedCurvePoint r(p.x(), p.y()); auto trace = TestTraceContainer({ { { C::ecc_add_op, 1 }, @@ -348,10 +348,10 @@ TEST(EccAddConstrainingTest, EccAddingInfinity) TEST(EccAddConstrainingTest, EccDoublingInf) { - EmbeddedCurvePoint p(0, 0, true); + EmbeddedCurvePoint p(0, 0); // r = O + O = O; , where O is the point at infinity - EmbeddedCurvePoint r(0, 0, true); + EmbeddedCurvePoint r(0, 0); auto trace = TestTraceContainer({ { { C::ecc_add_op, 0 }, @@ -470,7 +470,7 @@ TEST(EccAddConstrainingTest, EccNegativeBadAdd) FF r_x("0x20f096ae3de9aea007e0b94a0274b2443d6682d1901f6909f284ec967bc169be"); FF r_y("0x27948713833bb314e828f2b6f45f408da6564a3ac03b9e430a9c6634bb849ef2"); - EmbeddedCurvePoint r(r_x, r_y, false); + EmbeddedCurvePoint r(r_x, r_y); auto trace = TestTraceContainer({ { { C::ecc_add_op, 1 }, @@ -514,7 +514,7 @@ TEST(EccAddConstrainingTest, EccNegativeBadDouble) FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6"); FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09"); - EmbeddedCurvePoint r(r_x, r_y, false); + EmbeddedCurvePoint r(r_x, r_y); auto trace = TestTraceContainer({ { { C::ecc_add_op, 0 }, @@ -797,7 +797,9 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinity) check_relation(trace); check_relation(trace); } - +// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. +// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. +// EmbeddedCurvePoint preserves raw coordinates (see StandardAffinePointTest) TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep) { EccTraceBuilder builder; @@ -817,16 +819,11 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep) ecc_add_memory_event_emitter); EmbeddedCurvePoint inf = EmbeddedCurvePoint::infinity(); - // TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. - // We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. - // EmbeddedCurvePoint preserves raw coordinates (see StandardAffinePointTest) + EmbeddedCurvePoint inf_bb = EmbeddedCurvePoint(avm2::AffinePoint::infinity()); - EmbeddedCurvePoint inf_alt = EmbeddedCurvePoint(1, 2, true); EmbeddedCurvePoint result = ecc_simulator.scalar_mul(inf_bb, FF(10)); ASSERT_TRUE(result.is_infinity()); - result = ecc_simulator.scalar_mul(inf_alt, FF(10)); - ASSERT_TRUE(result.is_infinity()); TestTraceContainer trace({ { { C::precomputed_first_row, 1 } }, @@ -1309,7 +1306,7 @@ TEST(EccAddMemoryConstrainingTest, EccAddMemoryPointError) // Point P is not on the curve FF p_x("0x0000000000063d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x00000000000c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); uint32_t dst_address = 0x1000; @@ -1347,6 +1344,8 @@ TEST(EccAddMemoryConstrainingTest, EccAddMemoryPointError) check_relation(trace); } +// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. +// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. TEST(EccAddMemoryConstrainingTest, InfinityRepresentations) { EccTraceBuilder builder; @@ -1369,16 +1368,13 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations) ecc_add_memory_event_emitter); MemoryAddress dst_address = 5; - // TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. - // We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. - // Point P is infinity EmbeddedCurvePoint inf = EmbeddedCurvePoint::infinity(); // EmbeddedCurvePoint always sets extractable coordinates as (0,0) and the underlying point as // AffinePoint::infinity() for input infinity points. EmbeddedCurvePoint inf_bb = EmbeddedCurvePoint(avm2::AffinePoint::infinity()); - EmbeddedCurvePoint inf_alt = EmbeddedCurvePoint(0, 7, true); - EXPECT_EQ(inf_bb, inf_alt); + // EmbeddedCurvePoint inf_alt = EmbeddedCurvePoint(0, 7, true); + // EXPECT_EQ(inf_bb, inf_alt); TestTraceContainer trace; // The circuit correctly assigns double_op = true when doubling inf: diff --git a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.cpp b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.cpp index dee062405404..30106a38fdf6 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.cpp @@ -146,7 +146,9 @@ void Ecc::add(MemoryInterface& memory, } catch (const InternalEccException& e) { // Note this point is not on the curve, but corresponds // to default values the circuit will assign. - EmbeddedCurvePoint res = EmbeddedCurvePoint(0, 0, false); + // TODO(MW): This is now a point on the curve technically (inf) - check this doesnt cause issues now res.is_inf + // is true: + EmbeddedCurvePoint res = EmbeddedCurvePoint(0, 0); add_memory_events.emit({ .execution_clk = execution_clk, .space_id = space_id, .p = p, diff --git a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.test.cpp index c47cdef79b2e..b71e12fec604 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.test.cpp @@ -37,11 +37,11 @@ TEST(AvmSimulationEccTest, Add) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3"); - EmbeddedCurvePoint q(q_x, q_y, false); + EmbeddedCurvePoint q(q_x, q_y); EmbeddedCurvePoint result = ecc.add(p, q); @@ -74,7 +74,7 @@ TEST(AvmSimulationEccTest, ScalarMul) FF scalar("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); uint256_t scalar_num = scalar; - std::vector bits(254, false); + std::vector bits(254); for (size_t i = 0; i < 254; ++i) { bits[i] = scalar_num.get_bit(i); } @@ -83,7 +83,7 @@ TEST(AvmSimulationEccTest, ScalarMul) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); EmbeddedCurvePoint result = ecc.scalar_mul(p, scalar); @@ -128,7 +128,7 @@ TEST(AvmSimulationEccDeathTest, ScalarMulNotOnCurve) // Point P is not on the curve FF p_x("0x0000000000063d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x00000000000c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); FF scalar("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); @@ -155,22 +155,20 @@ TEST(AvmSimulationEccTest, AddWithMemory) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3"); - EmbeddedCurvePoint q(q_x, q_y, false); + EmbeddedCurvePoint q(q_x, q_y); FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6"); FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09"); - EmbeddedCurvePoint expected_result(r_x, r_y, false); + EmbeddedCurvePoint expected_result(r_x, r_y); uint32_t dst_address = 0x1000; ecc.add(memory, p, q, dst_address); - EmbeddedCurvePoint result = { memory.get(dst_address).as_ff(), - memory.get(dst_address + 1).as_ff(), - static_cast(memory.get(dst_address + 2).as_ff()) }; + EmbeddedCurvePoint result = { memory.get(dst_address).as_ff(), memory.get(dst_address + 1).as_ff() }; EXPECT_EQ(result, expected_result); } @@ -194,12 +192,12 @@ TEST(AvmSimulationEccTest, AddNotOnCurve) // Point P is not on the curve FF p_x("0x0000000000063d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x00000000000c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); // Point Q is on the curve FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3"); - EmbeddedCurvePoint q(q_x, q_y, false); + EmbeddedCurvePoint q(q_x, q_y); uint32_t dst_address = 0x1000; EXPECT_THROW(ecc.add(memory, p, q, dst_address), EccException); @@ -228,14 +226,12 @@ TEST(AvmSimulationEccTest, InfinityOnCurve) // Point Q is on the curve FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3"); - EmbeddedCurvePoint q(q_x, q_y, false); + EmbeddedCurvePoint q(q_x, q_y); uint32_t dst_address = 0x1000; ecc.add(memory, p, q, dst_address); - EmbeddedCurvePoint result = { memory.get(dst_address).as_ff(), - memory.get(dst_address + 1).as_ff(), - static_cast(memory.get(dst_address + 2).as_ff()) }; + EmbeddedCurvePoint result = { memory.get(dst_address).as_ff(), memory.get(dst_address + 1).as_ff() }; // INF + Q = Q EXPECT_EQ(result, q); } @@ -260,7 +256,7 @@ TEST(AvmSimulationEccTest, AddsUpToInfinity) // Both points on the curve, q = -p FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); EmbeddedCurvePoint q = -p; diff --git a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.cpp b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.cpp index 33c1b09dd2e6..6c4088fb3f29 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.cpp @@ -1521,10 +1521,10 @@ void Execution::ecc_add(ContextInterface& context, // is_infinity. The flag will be removed in future. const FF p_x_ff = p_x.as_ff(); const FF p_y_ff = p_y.as_ff(); - EmbeddedCurvePoint p = EmbeddedCurvePoint(p_x_ff, p_y_ff, (p_x_ff == FF::zero()) && (p_y_ff == FF::zero())); + EmbeddedCurvePoint p = EmbeddedCurvePoint(p_x_ff, p_y_ff); const FF q_x_ff = q_x.as_ff(); const FF q_y_ff = q_y.as_ff(); - EmbeddedCurvePoint q = EmbeddedCurvePoint(q_x_ff, q_y_ff, (q_x_ff == FF::zero()) && (q_y_ff == FF::zero())); + EmbeddedCurvePoint q = EmbeddedCurvePoint(q_x_ff, q_y_ff); try { embedded_curve.add(memory, p, q, dst_addr); diff --git a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.test.cpp index 8824c8fe91a0..06752ba433b4 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.test.cpp @@ -1042,11 +1042,11 @@ TEST_F(ExecutionSimulationTest, EccAdd) MemoryValue p_x = MemoryValue::from(FF("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a")); MemoryValue p_y = MemoryValue::from(FF("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60")); - EmbeddedCurvePoint p(p_x.as_ff(), p_y, false); + EmbeddedCurvePoint p(p_x.as_ff(), p_y); MemoryValue q_x = MemoryValue::from(FF("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7")); MemoryValue q_y = MemoryValue::from(FF("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3")); - EmbeddedCurvePoint q(q_x.as_ff(), q_y.as_ff(), false); + EmbeddedCurvePoint q(q_x.as_ff(), q_y.as_ff()); // Mock the context and memory interactions MemoryValue zero = MemoryValue::from(0); diff --git a/barretenberg/cpp/src/barretenberg/vm2/tracegen/ecc_trace.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/tracegen/ecc_trace.test.cpp index fd639b225c27..9589d06b8468 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/tracegen/ecc_trace.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/tracegen/ecc_trace.test.cpp @@ -21,13 +21,13 @@ TEST(EccTraceGenTest, TraceGenerationAdd) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7"); FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3"); - EmbeddedCurvePoint q(q_x, q_y, false); + EmbeddedCurvePoint q(q_x, q_y); FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6"); FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09"); - EmbeddedCurvePoint r(r_x, r_y, false); + EmbeddedCurvePoint r(r_x, r_y); builder.process_add({ { .p = p, .q = q, .result = r } }, trace); EXPECT_THAT(trace.as_rows(), @@ -61,11 +61,11 @@ TEST(EccTraceGenTest, TraceGenerationDouble) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); EmbeddedCurvePoint q = p; FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6"); FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09"); - EmbeddedCurvePoint r(r_x, r_y, false); + EmbeddedCurvePoint r(r_x, r_y); builder.process_add({ { .p = p, .q = q, .result = r } }, trace); @@ -100,9 +100,9 @@ TEST(EccTraceGenTest, TraceGenerationInfResult) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); - EmbeddedCurvePoint q(p.x(), -p.y(), false); + EmbeddedCurvePoint q(p.x(), -p.y()); EmbeddedCurvePoint r = p + q; builder.process_add({ { .p = p, .q = q, .result = r } }, trace); @@ -138,7 +138,7 @@ TEST(EccTraceGenTest, TraceGenerationInfAdd) FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a"); FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60"); - EmbeddedCurvePoint p(p_x, p_y, false); + EmbeddedCurvePoint p(p_x, p_y); // We always assume infinity coordinates have been normalized to (0,0) before reaching tracegen EmbeddedCurvePoint q = EmbeddedCurvePoint::infinity(); From 0e73c1663b7be9e293ce0feea808e63a80c989f3 Mon Sep 17 00:00:00 2001 From: MirandaWood Date: Thu, 7 May 2026 11:36:59 +0000 Subject: [PATCH 2/2] chore: cleanup, docs --- .../vm2/common/standard_affine_point.hpp | 15 ++--- .../vm2/common/standard_affine_point.test.cpp | 37 +++++------ .../vm2/constraining/relations/ecc.test.cpp | 62 +------------------ 3 files changed, 23 insertions(+), 91 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp index 0c0e840881bd..680292e2d53c 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp @@ -6,18 +6,15 @@ namespace bb::avm2 { /** - * AVM bytecode expects the representation of points to be triplets, the two coordinates and an is_infinity boolean. - * Furthermore, its representation of infinity is inherited from noir's and is expected to be 0,0,true. - * BB, however, uses only the two coordinates to represent points. Infinity in barretenberg is represented as (P+1)/2,0. - * This class is a wrapper of the BB representation, needed to operate with points, that allows us to extract the - * standard representation that AVM bytecode expects. + * The AVM's representation of infinity is inherited from noir's and is expected to be 0,0. + * BB, however, uses only the two coordinates to represent points. Infinity in barretenberg is represented as + * (P+1)/2,0,true. This class is a wrapper of the BB representation, needed to operate with points, that allows us to + * extract the standard representation that AVM bytecode expects. * * NOTE: When constructing infinity from BB's two element representation, we keep the original AffinePoint so operations * can use it in the background, but set extractable coordinates to be our represention of (0,0). - * NOTE: When constructing infinity via BaseFields, input coordinates are overwritten to our representation of (0,0) - * if the input is_infinity is true, so will mismatch the underlying AffinePoint's coordinates. - * - * TODO(#AVM-266): Remove is_infinity flag from point representation. + * NOTE: When constructing infinity via BaseFields (equiv. inputting (0, 0), the underlying AffinePoint is set to BB's + * representation so operations can use it in the background. */ template class StandardAffinePoint { public: diff --git a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp index 7a5b26ed3434..b8e79e2a1f50 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp @@ -10,25 +10,18 @@ using EmbeddedCurvePoint = StandardAffinePoint; using Fr = grumpkin::fr; using Fq = grumpkin::fq; -// TEST(StandardAffinePointTest, InfinityDiscardsRawCoordinates) -// { -// // NOTE: As of #AVM-248, we moved from preserving raw coordinates in -// // infinity points to our (0,0) representation when using x() and y(). -// // The underlying AffinePoint is set to AffinePoint::infinity() for -// // bb operations. - -// // When constructing an infinity point with non-zero coordinates, -// // x() and y() should return our standard representation. -// Fq raw_x = 1; -// Fq raw_y = 2; - -// // Note that raw x and y are silently discarded. -// EmbeddedCurvePoint point(raw_x, raw_y); - -// EXPECT_TRUE(point.is_infinity()); -// EXPECT_TRUE(point.x().is_zero()); -// EXPECT_TRUE(point.y().is_zero()); -// } +TEST(StandardAffinePointTest, ConstructingInfinityNormalized) +{ + // Constructing a point with (0,0) coordinates should result in infinity + EmbeddedCurvePoint inf(0, 0); + EXPECT_TRUE(inf.is_infinity()); + // Constructing a point with BB's inf should result in infinity with (0,0) coordinates + EmbeddedCurvePoint inf_bb(grumpkin::g1::affine_element::infinity()); + EXPECT_TRUE(inf_bb.is_infinity()); + EXPECT_TRUE(inf_bb.x().is_zero()); + EXPECT_TRUE(inf_bb.y().is_zero()); + EXPECT_EQ(inf, inf_bb); +} TEST(StandardAffinePointTest, NormalPointCoordinates) { @@ -78,7 +71,7 @@ TEST(StandardAffinePointTest, ScalarMultiplicationResultingInInfinityNormalized) TEST(StandardAffinePointTest, StaticInfinityHasZeroCoordinates) { - // The static infinity() method should return (0,0,true) + // The static infinity() method should return (0,0) auto& inf = EmbeddedCurvePoint::infinity(); EXPECT_TRUE(inf.is_infinity()); @@ -89,9 +82,7 @@ TEST(StandardAffinePointTest, StaticInfinityHasZeroCoordinates) TEST(StandardAffinePointTest, NegatingInfinity) { // Negating an infinity point should return (0,0) - EmbeddedCurvePoint inf(0, 0); - - auto neg_inf = -inf; + auto neg_inf = -EmbeddedCurvePoint::infinity(); EXPECT_TRUE(neg_inf.is_infinity()); EXPECT_TRUE(neg_inf.x().is_zero()); diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp index eab2a3887afc..e25773a8323c 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp @@ -772,42 +772,6 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinity) EventEmitter scalar_mul_event_emitter; NoopEventEmitter ecc_add_memory_event_emitter; - StrictMock execution_id_manager; - StrictMock gt; - PureToRadix to_radix_simulator = PureToRadix(); - EccSimulator ecc_simulator(execution_id_manager, - gt, - to_radix_simulator, - ecc_add_event_emitter, - scalar_mul_event_emitter, - ecc_add_memory_event_emitter); - - EmbeddedCurvePoint result = ecc_simulator.scalar_mul(EmbeddedCurvePoint::infinity(), FF(10)); - ASSERT_TRUE(result.is_infinity()); - - TestTraceContainer trace({ - { { C::precomputed_first_row, 1 } }, - }); - - builder.process_scalar_mul(scalar_mul_event_emitter.dump_events(), trace); - builder.process_add(ecc_add_event_emitter.dump_events(), trace); - - check_interaction(trace); - - check_relation(trace); - check_relation(trace); -} -// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. -// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. -// EmbeddedCurvePoint preserves raw coordinates (see StandardAffinePointTest) -TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep) -{ - EccTraceBuilder builder; - - EventEmitter ecc_add_event_emitter; - EventEmitter scalar_mul_event_emitter; - NoopEventEmitter ecc_add_memory_event_emitter; - StrictMock execution_id_manager; StrictMock gt; PureToRadix to_radix_simulator = PureToRadix(); @@ -824,6 +788,8 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep) EmbeddedCurvePoint result = ecc_simulator.scalar_mul(inf_bb, FF(10)); ASSERT_TRUE(result.is_infinity()); + EXPECT_EQ(result.x(), inf.x()); + EXPECT_EQ(result.y(), inf.y()); TestTraceContainer trace({ { { C::precomputed_first_row, 1 } }, @@ -1344,8 +1310,6 @@ TEST(EccAddMemoryConstrainingTest, EccAddMemoryPointError) check_relation(trace); } -// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. -// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. TEST(EccAddMemoryConstrainingTest, InfinityRepresentations) { EccTraceBuilder builder; @@ -1373,8 +1337,7 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations) // EmbeddedCurvePoint always sets extractable coordinates as (0,0) and the underlying point as // AffinePoint::infinity() for input infinity points. EmbeddedCurvePoint inf_bb = EmbeddedCurvePoint(avm2::AffinePoint::infinity()); - // EmbeddedCurvePoint inf_alt = EmbeddedCurvePoint(0, 7, true); - // EXPECT_EQ(inf_bb, inf_alt); + EXPECT_EQ(inf_bb, inf); TestTraceContainer trace; // The circuit correctly assigns double_op = true when doubling inf: @@ -1415,25 +1378,6 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations) trace.set(C::ecc_add_mem_q_y, 0, 2); EXPECT_THROW_WITH_MESSAGE(check_relation(trace, mem_aware_ecc::SR_Q_INF_X_CHECK), "Q_INF_X_CHECK"); EXPECT_THROW_WITH_MESSAGE(check_relation(trace, mem_aware_ecc::SR_Q_INF_Y_CHECK), "Q_INF_Y_CHECK"); - - // TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations. - // We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf. - // The below test no longer makes sense since it checks we store non-(0,0) coordinates for an inf - // point, which we do not allow. - - // builder.process_add_with_memory(ecc_add_memory_event_emitter.dump_events(), trace); - - // // The original coordinates are stored in memory for the read... - // EXPECT_EQ(trace.get(C::ecc_add_mem_q_x, 1), inf_bb.x()); - // EXPECT_EQ(trace.get(C::ecc_add_mem_q_y, 1), inf_bb.y()); - // // ...but normalised coordinates are sent to the ecc subtrace: - // EXPECT_EQ(trace.get(C::ecc_add_mem_q_x_n, 1), 0); - // EXPECT_EQ(trace.get(C::ecc_add_mem_q_y_n, 1), 0); - // check_relation(trace); - // check_relation(trace); - // check_all_interactions(trace); - // check_interaction(trace); } } // namespace