chore(avm)!: Remove is_infinite flag from point wrapper constructor (#22921)

This branch solely contains the changes needed to remove the
`is_infinite` flag from our `StandardAffinePoint` C++ wrapper. Now, we
check whether `x` and `y` are zero to assign an `inf` underlying point.

Will close [Foundation AVM Issue
17](https://linear.app/aztec-foundation/issue/AVM-17/remove-is-inf-flag-from-avms-standardaffinepoint)

---

Stack:

- #22745
- #22564
- `mw/avm-rem-inf-point-wrapper` <-- here
- #22795
- #22945
- #23031

Author: MirandaWood

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;

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 <typename AffinePoint> class StandardAffinePoint {
   public:
@@ -32,12 +29,10 @@ template <typename AffinePoint> 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 +82,7 @@ template <typename AffinePoint> class StandardAffinePoint {
 
     static const StandardAffinePoint& infinity()
     {
-        static auto infinity = StandardAffinePoint(zero, zero, true);
+        static auto infinity = StandardAffinePoint(zero, zero);
         return infinity;
     }
 

barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.test.cpp

@@ -10,24 +10,17 @@ using EmbeddedCurvePoint = StandardAffinePoint<grumpkin::g1::affine_element>;
 using Fr = grumpkin::fr;
 using Fq = grumpkin::fq;
 
-TEST(StandardAffinePointTest, InfinityDiscardsRawCoordinates)
+TEST(StandardAffinePointTest, ConstructingInfinityNormalized)
 {
-    // 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());
+    // 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());
@@ -88,10 +81,8 @@ TEST(StandardAffinePointTest, StaticInfinityHasZeroCoordinates)
 
 TEST(StandardAffinePointTest, NegatingInfinity)
 {
-    // Negating an infinity point should return (0,0,true)
-    EmbeddedCurvePoint inf(0, 0, /*is_infinity=*/true);
-
-    auto neg_inf = -inf;
+    // Negating an infinity point should return (0,0)
+    auto neg_inf = -EmbeddedCurvePoint::infinity();
 
     EXPECT_TRUE(neg_inf.is_infinity());
     EXPECT_TRUE(neg_inf.x().is_zero());

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<EccAddEvent> 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 },
@@ -772,40 +772,6 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinity)
     EventEmitter<ScalarMulEvent> scalar_mul_event_emitter;
     NoopEventEmitter<EccAddMemoryEvent> ecc_add_memory_event_emitter;
 
-    StrictMock<MockExecutionIdManager> execution_id_manager;
-    StrictMock<MockGreaterThan> 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<EccTraceBuilder, lookup_scalar_mul_double_settings, lookup_scalar_mul_add_settings>(trace);
-
-    check_relation<scalar_mul>(trace);
-    check_relation<ecc>(trace);
-}
-
-TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep)
-{
-    EccTraceBuilder builder;
-
-    EventEmitter<EccAddEvent> ecc_add_event_emitter;
-    EventEmitter<ScalarMulEvent> scalar_mul_event_emitter;
-    NoopEventEmitter<EccAddMemoryEvent> ecc_add_memory_event_emitter;
-
     StrictMock<MockExecutionIdManager> execution_id_manager;
     StrictMock<MockGreaterThan> gt;
     PureToRadix to_radix_simulator = PureToRadix();
@@ -817,16 +783,13 @@ 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());
+    EXPECT_EQ(result.x(), inf.x());
+    EXPECT_EQ(result.y(), inf.y());
 
     TestTraceContainer trace({
         { { C::precomputed_first_row, 1 } },
@@ -1309,7 +1272,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;
 
@@ -1369,16 +1332,12 @@ 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);
+    EXPECT_EQ(inf_bb, inf);
     TestTraceContainer trace;
 
     // The circuit correctly assigns double_op = true when doubling inf:
@@ -1419,25 +1378,6 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations)
     trace.set(C::ecc_add_mem_q_y, 0, 2);
     EXPECT_THROW_WITH_MESSAGE(check_relation<mem_aware_ecc>(trace, mem_aware_ecc::SR_Q_INF_X_CHECK), "Q_INF_X_CHECK");
     EXPECT_THROW_WITH_MESSAGE(check_relation<mem_aware_ecc>(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<mem_aware_ecc>(trace);
-    // check_relation<ecc>(trace);
-    // check_all_interactions<EccTraceBuilder>(trace);
-    // check_interaction<tracegen::ExecutionTraceBuilder,
-    // bb::avm2::perm_execution_dispatch_to_ecc_add_settings>(trace);
 }
 
 } // namespace

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,