feat(avm)!: Remove is_infinite flag from AVM ECC & update noir submodule with serialization changes

avm-transpiler/src/procedures/compiler.rs

@@ -233,10 +233,8 @@ fn compile_opcode(
         Mnemonic::ECADD => {
             collector.memory_address_operand()?; // p1 x
             collector.memory_address_operand()?; // p1 y
-            collector.memory_address_operand()?; // p1 is_infinite
             collector.memory_address_operand()?; // p2 x
             collector.memory_address_operand()?; // p2 y
-            collector.memory_address_operand()?; // p2 is_infinite
             collector.memory_address_operand()?; // result
             let collection = collector.finish()?;
             result.add_instruction(

avm-transpiler/src/procedures/msm.rs

@@ -1,15 +1,13 @@
 pub(crate) const MSM_ASSEMBLY: &str = "
                 ; We are passed three pointers and one usize.
-                ; d0 points to the points. Points are represented by (x: Field, y: Field, is_infinite: bool)
+                ; d0 points to the points. Points are represented by (x: Field, y: Field).
                 ; d1 points to the scalars. Scalars are represented by (lo: Field, hi: Field) both range checked to 128 bits.
                 ; d2 contains the number of points.
                 ; d3 points to the result. The result is a point.
                 ADD d3, /*the reserved register 'one_usize'*/ $2, d4; Compute the pointer to the result y.
-                ADD d4, $2, d5; Compute the pointer to the result is_infinite
                 ; Initialize the msm result: point at infinity
                 SET i3, 0 ff
                 SET i4, 0 ff
-                SET i5, 1 u1
                 ; Loop globals
                 SET d6, 0 u32; Initialize the outer loop variable, ranging from 0 to the number of points
                 SET d8, 0 ff; Initialize a 0 FF
@@ -18,13 +16,12 @@ pub(crate) const MSM_ASSEMBLY: &str = "
                 SET d10, 128 u32; Initialize a constant 128
                 SET d11, 1 u1; Initialize a constant true
                 SET d12, 0 u1; Initialize a constant false
-                SET d13, 2 u32; Initialize a constant 2
-                SET d14, 3 u32; Initialize a constant 3 for computing pointers to the point components
+                SET d13, 2 u32; Initialize a constant 2 for computing pointers to point and scalar components
                 ; Main loop: iterate over the points/scalars
 OUTER_HEAD:     LT d6, d2, d15 ; Check if we are done with the outer loop
                 JUMPI d15, OUTER_BODY
                 JUMP OUTER_END
-OUTER_BODY:     MUL d6, d14, d16; Compute the pointer to the point
+OUTER_BODY:     MUL d6, d13, d16; Compute the pointer to the point
                 ADD d16, d0, d16;
                 MUL d6, d13, d17; Compute the pointer to the scalar lo
                 ADD d17, d1, d17
@@ -51,35 +48,32 @@ FIND_MSB_BODY:  JUMPI i19, FIND_MSB_END; Check if the current bit is one
                 JUMP FIND_MSB_BODY
                 ; Now we have the pointer of the MSB in d19
 
-                ; Now store the result of the scalar multiplication in d22, d23, d24
+                ; Now store the result of the scalar multiplication in d22, d23
 FIND_MSB_END:   MOV i16, d22; x
                 ADD d16, $2, d25; pointer to y
                 MOV i25, d23; y
-                ADD d25, $2, d25; pointer to is_infinite
-                MOV i25, d24; is_infinite
-                ; Also store the original point in d25, d26, d27
+                ; Also store the original point in d25, d26
                 MOV d22, d25; x
                 MOV d23, d26; y
-                MOV d24, d27; is_infinite
 
                 ; Now we need to do the inner loop, that will do double then add
                 ; We need to iterate from the pointer of the MSB + 1 to the end pointer (d21)
                 ADD d19, $2, d19; We start from the pointer of the MSB + 1
 INNER_HEAD:     LT d19, d21, d28; Check if we are done with the loop
                 JUMPI d28, INNER_BODY
                 JUMP INNER_END
-INNER_BODY:     ECADD d22, d23, d24, d22, d23, d24, /*not indirect, so the result is stored in d22, d23, d24*/ d22; Double the current result.
+INNER_BODY:     ECADD d22, d23, d22, d23, /*not indirect, so the result is stored in d22, d23*/ d22; Double the current result.
                 EQ i19, d12, d28; Check if the current bit is zero
                 JUMPI d28, INNER_INC; If the current bit is zero, continue
-                ECADD d25, d26, d27, d22, d23, d24, /*not indirect, so the result is stored in d22, d23, d24*/ d22; Add the original point to the result
+                ECADD d25, d26, d22, d23, /*not indirect, so the result is stored in d22, d23*/ d22; Add the original point to the result
 INNER_INC:      ADD d19, $2, d19; Increment the pointer
                 JUMP INNER_HEAD
 
                 ; After the inner loop we have computed the scalar multiplication. Add it to the msm result
-INNER_END:      ECADD i3, i4, i5, d22, d23, d24, i3; Add the result to the msm result
+INNER_END:      ECADD i3, i4, d22, d23, i3; Add the result to the msm result
 OUTER_INC:      ADD d6, $2, d6; Increment the outer loop variable
                 JUMP OUTER_HEAD
-                ; After the outer loop we have computed the msm. We can return since we wrote the result in i3, i4, i5
+                ; After the outer loop we have computed the msm. We can return since we wrote the result in i3, i4
 OUTER_END:      INTERNALRETURN
 ";
 

avm-transpiler/src/transpile.rs

@@ -1280,53 +1280,46 @@ fn handle_black_box_function(
         BlackBoxOp::EmbeddedCurveAdd {
             input1_x: p1_x_offset,
             input1_y: p1_y_offset,
-            input1_infinite: p1_infinite_offset,
             input2_x: p2_x_offset,
             input2_y: p2_y_offset,
-            input2_infinite: p2_infinite_offset,
             result,
         } => avm_instrs.push(AvmInstruction {
             opcode: AvmOpcode::ECADD,
-            // The result (SIXTH operand) is indirect (addressing mode).
+            // The result (FOURTH operand) is indirect (addressing mode).
             addressing_mode: Some(
                 AddressingModeBuilder::default()
                     .direct_operand(p1_x_offset)
                     .direct_operand(p1_y_offset)
-                    .direct_operand(p1_infinite_offset)
                     .direct_operand(p2_x_offset)
                     .direct_operand(p2_y_offset)
-                    .direct_operand(p2_infinite_offset)
                     .indirect_operand(&result.pointer)
                     .build(),
             ),
             operands: vec![
                 AvmOperand::U16 { value: p1_x_offset.to_u32() as u16 },
                 AvmOperand::U16 { value: p1_y_offset.to_u32() as u16 },
-                AvmOperand::U16 { value: p1_infinite_offset.to_u32() as u16 },
                 AvmOperand::U16 { value: p2_x_offset.to_u32() as u16 },
                 AvmOperand::U16 { value: p2_y_offset.to_u32() as u16 },
-                AvmOperand::U16 { value: p2_infinite_offset.to_u32() as u16 },
                 AvmOperand::U16 { value: result.pointer.to_u32() as u16 },
             ],
             ..Default::default()
         }),
 
         BlackBoxOp::MultiScalarMul { points, scalars, outputs } => {
             // The length of the scalars vector is 2x the length of the points vector due to limb
-            // decomposition
-            // Output array is fixed to 3
+            // decomposition. Points are (x, y); the point at infinity is encoded as (0, 0).
             assert_eq!(
                 outputs.size,
-                SemiFlattenedLength(3),
-                "Output array size must be equal to 3"
+                SemiFlattenedLength(2),
+                "Output array size must be equal to 2"
             );
-            assert_eq!(points.size.0 % 3, 0, "Points array size must be divisible by 3");
+            assert_eq!(points.size.0 % 2, 0, "Points array size must be divisible by 2");
 
             avm_instrs.push(generate_mov_to_procedure(&points.pointer, 0));
             avm_instrs.push(generate_mov_to_procedure(&scalars.pointer, 1));
             avm_instrs.push(generate_set_to_procedure(
                 AvmTypeTag::UINT32,
-                &FieldElement::from(points.size.0 / 3),
+                &FieldElement::from(points.size.0 / 2),
                 2,
             ));
             avm_instrs.push(generate_mov_to_procedure(&outputs.pointer, 3));

barretenberg/cpp/pil/vm2/bytecode/address_derivation.pil

@@ -30,7 +30,7 @@ include "../scalar_mul.pil";
  *               as an independent 'destination' trace which is purely responsible for address
  *               computation.
  *               This means we assume all public keys are not the point at infinity, and so use
- *               precomputed.zero to represent each key's is_infinity flag (see TODO(#7529)).
+ *               precomputed.zero to represent each key's is_infinity flag (see TODO(#7529) and PR #22462).
  *
  * USAGE: To enforce that an address is correctly derived from all preimage members
  *        (adapted from #[ADDRESS_DERIVATION] in contract_instance_retrieval.pil):
@@ -184,7 +184,9 @@ namespace address_derivation;
     // 3. Computation of public keys hash
     pol commit public_keys_hash;
 
-    // TODO(#7529): Remove all the 0s for is_infinity when removed from public_keys.nr
+    // TODO(#AVM-266): Remove infinity flags from point representation. Note that we may still need to use
+    // precomputed.zero in the hash preimages until address derivation removes them:
+    // TODO(#7529)/TODO(F-553): Remove all the 0s for is_infinity when removed from public_keys.nr
     // https://github.com/AztecProtocol/aztec-packages/issues/7529
     // TODO(#14031): Compress keys in public_keys_hash
     // https://github.com/AztecProtocol/aztec-packages/issues/14031
@@ -312,11 +314,11 @@ namespace address_derivation;
     sel {
         preaddress_public_key_x, preaddress_public_key_y, precomputed.zero,
         incoming_viewing_key_x, incoming_viewing_key_y, precomputed.zero,
-        address, address_y, precomputed.zero
+        address, address_y
     } in ecc.sel {
         ecc.p_x, ecc.p_y, ecc.p_is_inf,
         ecc.q_x, ecc.q_y, ecc.q_is_inf,
-        ecc.r_x, ecc.r_y, ecc.r_is_inf
+        ecc.r_x, ecc.r_y
     };
 
     // Note: We can safely assume the address point is not infinity since that would imply either

barretenberg/cpp/pil/vm2/ecc.pil

@@ -2,23 +2,32 @@
 /**
  * This subtrace supports point addition over the Grumpkin curve.
  * Given two points, P & Q, this trace computes R = P + Q.
- * PRECONDITIONS: The only assumption here is that the inputs P & Q are points on the Grumpkin curve (note that the Point at Infinity = (0, 0) is considered on the curve):
- * Grumpkin Curve Eqn in SW form:  Y^2 = X^3 − 17.
+ * PRECONDITIONS: This trace assumes that the inputs P & Q are points on the Grumpkin curve and infinity points are correctly
+ * flagged with p_is_inf and/or q_is_inf (note that the Point at Infinity = (0, 0) is considered on the curve):
+ *      Grumpkin Curve Eqn in SW form:  Y^2 = X^3 − 17.
  * Note: Grumpkin forms a 2-cycle with BN254, i.e the base field of one is the scalar field of the other and vice-versa.
  *
  * USAGE: This is a non-memory aware subtrace used to constrain point addition as defined above. Each point can be looked up
  *        by coordinates (lookup as defined in ecc_mem.pil):
  *        #[INPUT_OUTPUT_ECC_ADD]
  *        sel_should_exec {
- *          p_x_n, p_y_n, p_is_inf, // Point P
- *          q_x_n, q_y_n, q_is_inf, // Point Q
- *          res_x, res_y, res_is_inf // Point R
+ *          p_x_n, p_y_n, // Point P
+ *          p_is_inf, // P == O
+ *          q_x_n, q_y_n, // Point Q
+ *          q_is_inf, // Q == O
+ *          res_x, res_y // Point R
  *        } in ecc.sel {
- *          ecc.p_x, ecc.p_y, ecc.p_is_inf, // Point P
- *          ecc.q_x, ecc.q_y, ecc.q_is_inf, // Point Q
- *          ecc.r_x, ecc.r_y, ecc.r_is_inf // Point R
+ *          ecc.p_x, ecc.p_y, // Point P
+ *          ecc.p_is_inf, // P == O
+ *          ecc.q_x, ecc.q_y,, // Point Q
+ *          ecc.q_is_inf, // Q == O
+ *          ecc.r_x, ecc.r_y // Point R
  *        };
  *
+ * NOTE: For now, the calling trace MUST constrain that p_is_inf, q_is_inf above are correct. This is so if we have a calling
+ *       trace in which we know inf would never be an input we can simply use precomputed.zero and avoid wasting gates on deriving is_inf.
+ *       This follows the same logic for points being on the curve.
+ *
  * TRACE SHAPE: 1 single row per computation (P + Q = R).
  *
  * INTERACTIONS: This subtrace is looked up by:
@@ -37,11 +46,11 @@ namespace ecc;
     // We perform point addition over our Short Weierstrass (SW) curve with 3 cases outlined in the last section ('Assign Result').
     // The notation will be as follows:
     //    P + Q = R where:
-    //      P = (p_x, p_y, p_is_inf), Q = (q_x, q_y, q_is_inf), R = (r_x, r_y, r_is_inf),
+    //      P = (p_x, p_y), Q = (q_x, q_y), R = (r_x, r_y),
     //    where the coordinates satisfy:
     //      y^2 = x^3 - 17 (unless is_inf is true).
     // The point at infinity, O, does not have valid coordinates (a property of SW curves). We represent it as:
-    //    O = (0, 0, true).
+    //    O = (0, 0).
     //    Note: this is NOT enforced here for inputs, see ecc_mem.pil for example of constraining.
     //
 
@@ -70,20 +79,20 @@ namespace ecc;
     // Point P in affine form
     pol commit p_x;
     pol commit p_y;
+    // Must be constrained by the calling trace:
     pol commit p_is_inf; // @boolean
     p_is_inf * (1 - p_is_inf) = 0;
 
     // Point Q in affine form
     pol commit q_x;
     pol commit q_y;
+    // Must be constrained by the calling trace:
     pol commit q_is_inf; // @boolean
     q_is_inf * (1 - q_is_inf) = 0;
 
     // Resulting Point R in affine form
     pol commit r_x;
     pol commit r_y;
-    pol commit r_is_inf; // @boolean
-    r_is_inf * (1 - r_is_inf) = 0;
 
     // Check x coordinates, i.e. p_x == q_x
     pol commit x_match; // @boolean
@@ -147,9 +156,9 @@ namespace ecc;
     //    If P != Q where x_match, this implies p_y == -q_y <==> P == -Q (INVERSE_PRED == true):
     //      R := O
     //    If P == O:
-    //      R := Q (r_x := q_x, r_y := q_y, r_is_inf = q_is_inf)
+    //      R := Q (r_x := q_x, r_y := q_y)
     //    Vice versa, if Q == O:
-    //      R := P (r_x := p_x, r_y := p_y, r_is_inf = p_is_inf)
+    //      R := P (r_x := p_x, r_y := p_y)
     //
 
     pol INVERSE_PRED = x_match * (1 - y_match);
@@ -182,6 +191,4 @@ namespace ecc;
     sel * (r_x - (EITHER_INF * (p_is_inf * q_x + q_is_inf * p_x)) - result_infinity * INFINITY_X - use_computed_result * COMPUTED_R_X) = 0;
     #[OUTPUT_Y_COORD]
     sel * (r_y - (EITHER_INF * (p_is_inf * q_y + q_is_inf * p_y)) - result_infinity * INFINITY_Y - use_computed_result * COMPUTED_R_Y) = 0;
-    #[OUTPUT_INF_FLAG]
-    sel * (r_is_inf - result_infinity) = 0;