Merge 430980b into 86d41b3

acvm-repo/acir/src/circuit/black_box_functions.rs

@@ -164,7 +164,15 @@ pub enum BlackBoxFunc {
     /// ultimately fail.
     RecursiveAggregation,
 
-    /// Addition over the embedded curve on which the witness is defined.
+    /// Addition over the embedded curve on which the witness is defined
+    /// The opcode makes the following assumptions but does not enforce them because
+    /// it is more efficient to do it only when required. For instance, adding two
+    /// points that are on the curve it guarantee to give a point on the curve.
+    ///
+    /// It assumes that the points are on the curve.
+    /// If the inputs are the same witnesses index, it will perform a doubling,
+    /// If not, it assumes that the points' x-coordinates are not equal.
+    /// It also assumes neither point is the infinity point.
     EmbeddedCurveAdd,
 
     /// BigInt addition

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -1471,6 +1471,7 @@ impl<F: AcirField> AcirContext<F> {
                 | BlackBoxFunc::AND
                 | BlackBoxFunc::XOR
                 | BlackBoxFunc::AES128Encrypt
+                | BlackBoxFunc::EmbeddedCurveAdd
         );
         // Convert `AcirVar` to `FunctionInput`
         let inputs = self.prepare_inputs_for_black_box_func_call(inputs, allow_constant_inputs)?;

noir_stdlib/src/embedded_curve_ops.nr

@@ -125,20 +125,63 @@ pub fn fixed_base_scalar_mul(scalar: EmbeddedCurveScalar) -> EmbeddedCurvePoint
     multi_scalar_mul([g1], [scalar])
 }
 
-// This is a hack as returning an `EmbeddedCurvePoint` from a foreign function in brillig returns a [BrilligVariable::SingleAddr; 2] rather than BrilligVariable::BrilligArray
+/// This function only assumes that the points are on the curve
+/// It handles corner cases around the infinity point causing some overhead compared to embedded_curve_add_not_nul and embedded_curve_add_unsafe
+// This is a hack because returning an `EmbeddedCurvePoint` from a foreign function in brillig returns a [BrilligVariable::SingleAddr; 2] rather than BrilligVariable::BrilligArray
 // as is defined in the brillig bytecode format. This is a workaround which allows us to fix this without modifying the serialization format.
 // docs:start:embedded_curve_add
-fn embedded_curve_add(
-    point1: EmbeddedCurvePoint,
-    point2: EmbeddedCurvePoint
-) -> EmbeddedCurvePoint
-// docs:end:embedded_curve_add
-{
-    let point_array = embedded_curve_add_array_return(point1, point2);
-    let x = point_array[0];
-    let y = point_array[1];
-    EmbeddedCurvePoint { x, y, is_infinite: point_array[2] == 1 }
+pub fn embedded_curve_add(point1: EmbeddedCurvePoint, point2: EmbeddedCurvePoint) -> EmbeddedCurvePoint {
+    // docs:end:embedded_curve_add
+    let x_coordinates_match = point1.x == point2.x;
+    let y_coordinates_match = point1.y == point2.y;
+    let double_predicate = (x_coordinates_match & y_coordinates_match);
+    let infinity_predicate = (x_coordinates_match & !y_coordinates_match);
+    let point1_1 = EmbeddedCurvePoint { x: point1.x + (x_coordinates_match as Field), y: point1.y, is_infinite: x_coordinates_match };
+    // point1_1 is guaranteed to have a different abscissa than point2
+    let mut result = embedded_curve_add_unsafe(point1_1, point2);
+    result.is_infinite = x_coordinates_match;
+
+    // dbl if x_match, y_match
+    let double = embedded_curve_add_unsafe(point1, point1);
+    result = if double_predicate { double } else { result };
+
+    // infinity if x_match, !y_match
+    if point1.is_infinite {
+        result= point2;
+    }
+    if point2.is_infinite {
+        result = point1;
+    }
+    let mut result_is_infinity = infinity_predicate & (!point1.is_infinite & !point2.is_infinite);
+    result.is_infinite = result_is_infinity | (point1.is_infinite & point2.is_infinite);
+    result
 }
 
 #[foreign(embedded_curve_add)]
 fn embedded_curve_add_array_return(_point1: EmbeddedCurvePoint, _point2: EmbeddedCurvePoint) -> [Field; 3] {}
+
+/// This function assumes that:
+/// The points are on the curve, and
+/// The points don't share an x-coordinate, and
+/// Neither point is the infinity point.
+/// If it is used with correct input, the function ensures the correct non-zero result is returned.
+/// Except for points on the curve, the other assumptions are checked by the function. It will cause assertion failure if they are not respected.
+pub fn embedded_curve_add_not_nul(point1: EmbeddedCurvePoint, point2: EmbeddedCurvePoint) -> EmbeddedCurvePoint {
+    assert(point1.x != point2.x);
+    assert(!point1.is_infinite);
+    assert(!point2.is_infinite);
+    embedded_curve_add_unsafe(point1, point2)
+}
+
+/// Unsafe ec addition
+/// If the inputs are the same, it will perform a doubling, but only if point1 and point2 are the same variable.
+/// If they have the same value but are different variables, the result will be incorrect because in this case
+/// it assumes (but does not check) that the points' x-coordinates are not equal.
+/// It also assumes neither point is the infinity point.
+pub fn embedded_curve_add_unsafe(point1: EmbeddedCurvePoint, point2: EmbeddedCurvePoint) -> EmbeddedCurvePoint {
+    let point_array = embedded_curve_add_array_return(point1, point2);
+    let x = point_array[0];
+    let y = point_array[1];
+
+    EmbeddedCurvePoint { x, y, is_infinite: false }
+}