chore: reproduce AVM ecadd bug

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -145,6 +145,11 @@ contract AvmTest {
         a % 2
     }
 
+    #[public]
+    fn elliptic_curve_add(lhs: Point, rhs: Point) -> Point {
+        lhs + rhs
+    }
+
     #[public]
     fn elliptic_curve_add_and_double() -> Point {
         let g = Point { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false };

yarn-project/foundation/package.json

@@ -160,4 +160,4 @@
   "engines": {
     "node": ">=18"
   }
-}
+}

yarn-project/foundation/src/fields/point.ts

@@ -258,7 +258,7 @@ export class Point {
    * Check this is consistent with how bb is encoding the point at infinity
    */
   public get inf() {
-    return this.x.isZero() && this.y.isZero() && this.isInfinite;
+    return this.isInfinite;
   }
 
   isOnGrumpkin() {

yarn-project/simulator/src/avm/avm_simulator.test.ts

@@ -176,6 +176,25 @@ describe('AVM simulator: transpiled Noir contracts', () => {
     expect(results.output).toEqual([new Fr(1), new Fr(2), new Fr(3)]);
   });
 
+  it('ec_add should not revert', async () => {
+    // This test performs the same doubling as in elliptic_curve_add_and_double
+    // But the optimizer is not able to optimize out the addition
+    const calldata: Fr[] = [
+      new Fr(1), // P1x
+      new Fr(17631683881184975370165255887551781615748388533673675138860n), // P1y
+      new Fr(0), // P1inf
+      new Fr(1), // P2x
+      new Fr(17631683881184975370165255887551781615748388533673675138860n), // P2y
+      new Fr(0), // P2inf
+    ];
+    const context = initContext({ env: initExecutionEnvironment({ calldata }) });
+
+    const bytecode = getAvmTestContractBytecode('elliptic_curve_add');
+    const results = await new AvmSimulator(context).executeBytecode(bytecode);
+
+    expect(results.reverted).toBe(false);
+  });
+
   it('elliptic curve operations', async () => {
     const context = initContext();
 

yarn-project/simulator/src/avm/opcodes/ec_add.ts

@@ -72,7 +72,14 @@ export class EcAdd extends Instruction {
     }
 
     const grumpkin = new Grumpkin();
-    let dest = grumpkin.add(p1, p2);
+    let dest;
+    if (p1IsInfinite) {
+      dest = p2;
+    } else if (p2IsInfinite) {
+      dest = p1;
+    } else {
+      dest = grumpkin.add(p1, p2);
+    }
     // Temporary,
     if (p1IsInfinite) {
       dest = p2;

yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts

@@ -69,10 +69,8 @@ export class MultiScalarMul extends Instruction {
     // Now we need to reconstruct the points and scalars into something we can operate on.
     const grumpkinPoints: Point[] = [];
     for (let i = 0; i < numPoints; i++) {
-      const p: Point = new Point(pointsVector[3 * i].toFr(), pointsVector[3 * i + 1].toFr(), false);
-      // Include this later when we have a standard for representing infinity
-      // const isInf = pointsVector[i + 2].toBoolean();
-
+      const isInf = pointsVector[3 * i + 2].toNumber() === 1;
+      const p: Point = new Point(pointsVector[3 * i].toFr(), pointsVector[3 * i + 1].toFr(), isInf);
       if (!p.isOnGrumpkin()) {
         throw new InstructionExecutionError(`Point ${p.toString()} is not on the curve.`);
       }
@@ -93,10 +91,20 @@ export class MultiScalarMul extends Instruction {
     const [firstBaseScalarPair, ...rest]: Array<[Point, Fq]> = grumpkinPoints.map((p, idx) => [p, scalarFqVector[idx]]);
     // Fold the points and scalars into a single point
     // We have to ensure get the first point, since the identity element (point at infinity) isn't quite working in ts
-    const outputPoint = rest.reduce(
-      (acc, curr) => grumpkin.add(acc, grumpkin.mul(curr[0], curr[1])),
-      grumpkin.mul(firstBaseScalarPair[0], firstBaseScalarPair[1]),
-    );
+    const outputPoint = rest.reduce((acc, curr) => {
+      if (curr[1] === Fq.ZERO) {
+        // If we multiply by 0, the result will the point at infinity - so we ignore it
+        return acc;
+      } else if (curr[0].inf) {
+        // If we multiply the point at infinity by a scalar, it's still the point at infinity
+        return acc;
+      } else if (acc.inf) {
+        // If we accumulator is the point at infinity, we can just return the current point
+        return curr[0];
+      } else {
+        return grumpkin.add(acc, grumpkin.mul(curr[0], curr[1]));
+      }
+    }, grumpkin.mul(firstBaseScalarPair[0], firstBaseScalarPair[1]));
     const output = outputPoint.toFields().map(f => new Field(f));
 
     memory.setSlice(outputOffset, output);