feat: derive inf from coordinates in sim/tracegen

Author: MirandaWood

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp

@@ -1382,7 +1382,11 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations)
     EXPECT_THROW_WITH_MESSAGE(ecc_simulator.add(inf, inf_alt), "normalized");
 
     // The circuit correctly assigns double_op = true when doubling inf:
-    ecc_simulator.add(memory, inf, inf_bb, dst_address);
+    ecc_simulator.add(memory, inf, inf, dst_address);
+
+    // TODO(#AVM-266): As part of Noir's removal of is_infinite (#AVM-248) we assume any input infinities
+    // follow (0, 0) <==> is_inf.
+    EXPECT_THROW_WITH_MESSAGE(ecc_simulator.add(inf, inf_bb), "normalized");
 
     builder.process_add(ecc_add_event_emitter.dump_events(), trace);
     check_relation<ecc>(trace);

barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.cpp

@@ -136,21 +136,15 @@ void Ecc::add(MemoryInterface& memory,
             throw InternalEccException("dst address out of range");
         }
 
+        // TODO(#AVM-266): Note: bb infinity points (is_inf=true with x=(P+1)/2, y=0) pass on_curve() without
+        // throwing here, but would throw in the circuit. We assume here input points follow the Noir convention
+        // of (x=0, y=0) <==> is_infinity.
         if (!p.on_curve() || !q.on_curve()) {
-            // TODO(#AVM-266): Note: We now use this to enforce (X, Y) == (0, 0) ==> is_inf
-            // until is_inf is removed. This means a bb inf point (!= (0, 0)) will
-            // not throw here but would throw in the circuit. Since we remap such points to
-            // (0, 0) below, it shouldn't reach the circuit at all.
             throw InternalEccException("One of the points is not on the curve");
         }
 
-        // Normalize input infinity points.
-        EmbeddedCurvePoint p_input = p.is_infinity() ? EmbeddedCurvePoint::infinity() : p;
-        EmbeddedCurvePoint q_input = q.is_infinity() ? EmbeddedCurvePoint::infinity() : q;
-
         // Emits EccAddEvent, see #[INPUT_OUTPUT_ECC_ADD] in ecc_mem.pil.
-        EmbeddedCurvePoint result =
-            add(p_input, q_input); // Cannot throw since we have checked on_curve() and normalized.
+        EmbeddedCurvePoint result = add(p, q); // Cannot throw since we have checked on_curve().
 
         // Emits MemoryEvents, see #[WRITE_MEM_i] for i = 0, 1, 2,  in ecc_mem.pil.
         memory.set(dst_address, MemoryValue::from<FF>(result.x()));

barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/execution.cpp

@@ -1504,19 +1504,26 @@ void Execution::ecc_add(ContextInterface& context,
     // Read the points from memory.
     const auto& p_x = memory.get(p_x_addr);
     const auto& p_y = memory.get(p_y_addr);
+    // TODO(#AVM-266): Remove infinity flags from point representation, the below is currently ignored in-circuit.
     const auto& p_inf = memory.get(p_inf_addr);
 
     const auto& q_x = memory.get(q_x_addr);
     const auto& q_y = memory.get(q_y_addr);
+    // TODO(#AVM-266): Remove infinity flags from point representation, the below is currently ignored in-circuit.
     const auto& q_inf = memory.get(q_inf_addr);
 
     set_and_validate_inputs(opcode, { p_x, p_y, p_inf, q_x, q_y, q_inf });
     get_gas_tracker().consume_gas();
 
     // Once inputs are tag checked the conversion to EmbeddedCurvePoint is safe, on curve checks are done in the add
-    // method.
-    EmbeddedCurvePoint p = EmbeddedCurvePoint(p_x.as_ff(), p_y.as_ff(), p_inf == MemoryValue::from<uint1_t>(1));
-    EmbeddedCurvePoint q = EmbeddedCurvePoint(q_x.as_ff(), q_y.as_ff(), q_inf == MemoryValue::from<uint1_t>(1));
+    // method. TODO(#AVM-266): We derive is_infinity from coordinates using the Noir convention of (x=0, y=0) <==>
+    // 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()));
+    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()));
 
     try {
         embedded_curve.add(memory, p, q, dst_addr);

barretenberg/cpp/src/barretenberg/vm2/tracegen/ecc_trace.cpp

@@ -299,13 +299,10 @@ void EccTraceBuilder::process_add_with_memory(
 
         bool error = dst_out_of_range_err || !p_is_on_curve || !q_is_on_curve;
 
-        // TODO(#AVM-266): Remove is_infinity flag from point representation. For now, we derive is_inf by
-        // checking coordinates in-circuit and ignoring the flag read from memory. Below, we do the 'reverse'
-        // and set derive coordinates as (0, 0) if is_inf is true. This allows us to handle bb inf points until
-        // the flag is removed.
-        // Normalized points, ensures that input infinity points are represented by (0, 0) in the ecc subtrace.
-        EmbeddedCurvePoint p_n = event.p.is_infinity() ? EmbeddedCurvePoint::infinity() : event.p;
-        EmbeddedCurvePoint q_n = event.q.is_infinity() ? EmbeddedCurvePoint::infinity() : event.q;
+        // TODO(#AVM-266): is_inf is (for now) derived from coordinates in-circuit (#[P/Q_INF_X/Y_CHECK]), but will be
+        // removed.
+        bool p_is_inf = (event.p.x() == FF::zero()) && (event.p.y() == FF::zero());
+        bool q_is_inf = (event.q.x() == FF::zero()) && (event.q.y() == FF::zero());
 
         trace.set(
             row,
@@ -331,16 +328,14 @@ void EccTraceBuilder::process_add_with_memory(
                 { C::ecc_add_mem_dst_addr_1_, dst_addr + 1 },
                 { C::ecc_add_mem_dst_addr_2_, dst_addr + 2 },
                 // Input - Point P
-                { C::ecc_add_mem_p_x, p_n.x() },
-                { C::ecc_add_mem_p_y, p_n.y() },
-                { C::ecc_add_mem_p_is_inf,
-                  event.p.is_infinity() ? 1 : 0 }, // TODO(#AVM-266): If committed, Will be p.x() == 0 && p.y() == 0
+                { C::ecc_add_mem_p_x, event.p.x() },
+                { C::ecc_add_mem_p_y, event.p.y() },
+                { C::ecc_add_mem_p_is_inf, p_is_inf ? 1 : 0 },
                 { C::ecc_add_mem_p_is_inf_, event.p.is_infinity() ? 1 : 0 }, // TODO(#AVM-266): Remove is_infinity flag
                 // Input - Point Q
-                { C::ecc_add_mem_q_x, q_n.x() },
-                { C::ecc_add_mem_q_y, q_n.y() },
-                { C::ecc_add_mem_q_is_inf,
-                  event.q.is_infinity() ? 1 : 0 }, // TODO(#AVM-266): If committed, Will be q.x() == 0 && q.y() == 0
+                { C::ecc_add_mem_q_x, event.q.x() },
+                { C::ecc_add_mem_q_y, event.q.y() },
+                { C::ecc_add_mem_q_is_inf, q_is_inf ? 1 : 0 },
                 { C::ecc_add_mem_q_is_inf_, event.q.is_infinity() ? 1 : 0 }, // TODO(#AVM-266): Remove is_infinity flag
                 // Output
                 { C::ecc_add_mem_sel_should_exec, error ? 0 : 1 },