chore: makes ec addition opcode unsafe

barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp

@@ -5,6 +5,7 @@
 // =====================
 
 #include "ec_operations.hpp"
+#include "barretenberg/dsl/acir_format/witness_constant.hpp"
 #include "barretenberg/ecc/curves/bn254/fr.hpp"
 #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
 #include "barretenberg/ecc/groups/affine_element.hpp"
@@ -13,19 +14,126 @@
 
 namespace acir_format {
 
+// This functions assumes that:
+// 1. the points are on the curve
+// 2a. the points have not the same abssissa, OR
+// 2b. the points are identical (same witness index or same value)
+// If the points at infinity are known and constant, the function will work properly
+// If not, and if the points are not identical, it is an error.
 template <typename Builder>
 void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_valid_witness_assignments)
 {
-    // Input to cycle_group points
+    // Cycle_group points are used by BB to lay out constraints on Grumpkin curve points
     using cycle_group_ct = bb::stdlib::cycle_group<Builder>;
 
-    auto input1_point = to_grumpkin_point(
-        input.input1_x, input.input1_y, input.input1_infinite, has_valid_witness_assignments, builder);
-    auto input2_point = to_grumpkin_point(
-        input.input2_x, input.input2_y, input.input2_infinite, has_valid_witness_assignments, builder);
+    // Check if operands are the 'same' (same witness or same constant value).
+    bool x_match = false;
+    if (!input.input1_x.is_constant && !input.input2_x.is_constant) {
+        x_match = (input.input1_x.index == input.input2_x.index);
+    } else {
+        if (input.input1_x.is_constant && input.input2_x.is_constant) {
+            x_match = (input.input1_x.value == input.input2_x.value);
+        }
+    }
+    bool y_match = false;
+    if (!input.input1_y.is_constant && !input.input2_y.is_constant) {
+        y_match = (input.input1_y.index == input.input2_y.index);
+    } else {
+        if (input.input1_y.is_constant && input.input2_y.is_constant) {
+            y_match = (input.input1_y.value == input.input2_y.value);
+        }
+    }
+
+    cycle_group_ct result;
+    // If operands are the same, we double.
+    // Note that the doubling function handles the infinity case
+    if (x_match && y_match) {
+        cycle_group_ct input1_point;
+
+        // When there are no valid witness assignements, we need to define dummy values that will
+        // satisfy the doubling constraints, which we can do easily when the inputs are witness.
+        // If the is_infinity is a witness, we can simply set it to 1
+        // Or, if the coordinates are witness, we can simply set them to a valid point on the curve (G1)
+        if (!input.input1_infinite.is_constant || (!input.input1_x.is_constant && !input.input1_y.is_constant)) {
+            input1_point = to_grumpkin_point(
+                input.input1_x, input.input1_y, input.input1_infinite, has_valid_witness_assignments, true, builder);
+        } else {
+            // If not, the coordinates are mixed constant/witness, and we generate witness so that the point is using
+            // only witnesses.
+            input1_point = to_witness_grumpkin_point(
+                input.input1_x, input.input1_y, input.input1_infinite, has_valid_witness_assignments, true, builder);
+        }
+        result = input1_point.dbl();
+    } else {
+        // Regular addition
+        // if one point is (constant) zero, we simply return the other point.
+        if (input.input2_infinite.is_constant && input.input1_infinite.is_constant) {
+            if (get_value(input.input1_infinite, builder) == 1) {
+                // input1 is infinity, so we can just return input2
+                result = to_grumpkin_point(input.input2_x,
+                                           input.input2_y,
+                                           input.input2_infinite,
+                                           has_valid_witness_assignments,
+                                           false,
+                                           builder);
+
+            } else if (get_value(input.input2_infinite, builder) == 1) {
+                // input2 is infinity, so we can just return input1
+                result = to_grumpkin_point(input.input1_x,
+                                           input.input1_y,
+                                           input.input1_infinite,
+                                           has_valid_witness_assignments,
+                                           true,
+                                           builder);
+            } else {
+
+                cycle_group_ct input1_point;
+                cycle_group_ct input2_point;
+                // all or nothing: the inputs must be all constant or all witness. Cf #1108 for more details.
+                if (!input.input1_x.is_constant || !input.input1_y.is_constant || !input.input1_infinite.is_constant ||
+                    !input.input2_x.is_constant || !input.input2_y.is_constant || !input.input2_infinite.is_constant) {
+                    // One of the input is a witness, so we ensure that all inputs are witness, by creating witness for
+                    // constant values.
+                    input1_point = to_witness_grumpkin_point(input.input1_x,
+                                                             input.input1_y,
+                                                             input.input1_infinite,
+                                                             has_valid_witness_assignments,
+                                                             true,
+                                                             builder);
+                    input2_point = to_witness_grumpkin_point(input.input2_x,
+                                                             input.input2_y,
+                                                             input.input2_infinite,
+                                                             has_valid_witness_assignments,
+                                                             false,
+                                                             builder);
+
+                } else {
+                    input1_point = to_grumpkin_point(input.input1_x,
+                                                     input.input1_y,
+                                                     input.input1_infinite,
+                                                     has_valid_witness_assignments,
+                                                     true,
+                                                     builder);
+                    input2_point = to_grumpkin_point(input.input2_x,
+                                                     input.input2_y,
+                                                     input.input2_infinite,
+                                                     has_valid_witness_assignments,
+                                                     false,
+                                                     builder);
+                }
+                // both points are not infinity, so we can use unconditional_add
+                if (has_valid_witness_assignments) {
+                    // Runtime check that the inputs have not the same x coordinate, as assumed by the function.
+                    ASSERT(input1_point.x.get_value() != input2_point.x.get_value());
+                }
+                result = input1_point.unconditional_add(input2_point);
+            }
+        } else {
+            // Some points could be at infinity, which is not supported by the function
+            ASSERT(false, "Unsupported EC ADDITION UNSAFE; is_infinite status must be known at compile time");
+        }
+    }
 
-    // Addition
-    cycle_group_ct result = input1_point + input2_point;
     cycle_group_ct standard_result = result.get_standard_form();
     auto x_normalized = standard_result.x.normalize();
     auto y_normalized = standard_result.y.normalize();
@@ -41,6 +149,7 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
     } else {
         builder.assert_equal(y_normalized.witness_index, input.result_y);
     }
+
     if (infinite.is_constant()) {
         builder.fix_witness(input.result_infinite, infinite.get_value());
     } else {

barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp

@@ -20,19 +20,18 @@ class EcOperations : public ::testing::Test {
 
 size_t generate_ec_add_constraint(EcAdd& ec_add_constraint, WitnessVector& witness_values)
 {
-    using cycle_group_ct = bb::stdlib::cycle_group<Builder>;
     witness_values.push_back(0);
     auto g1 = grumpkin::g1::affine_one;
-    cycle_group_ct input_point(g1);
-    // Doubling
-    cycle_group_ct result = input_point.dbl();
+    auto g2 = g1 + g1;
+    auto affine_result = g1 + g2;
+
     // add: x,y,x2,y2
     witness_values.push_back(g1.x);
     witness_values.push_back(g1.y);
-    witness_values.push_back(g1.x);
-    witness_values.push_back(g1.y);
-    witness_values.push_back(result.x.get_value());
-    witness_values.push_back(result.y.get_value());
+    witness_values.push_back(g2.x);
+    witness_values.push_back(g2.y);
+    witness_values.push_back(affine_result.x);
+    witness_values.push_back(affine_result.y);
     witness_values.push_back(fr(0));
     witness_values.push_back(fr(0));
     ec_add_constraint = EcAdd{

barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp

@@ -31,7 +31,7 @@ void create_multi_scalar_mul_constraint(Builder& builder,
     for (size_t i = 0; i < input.points.size(); i += 3) {
         // Instantiate the input point/variable base as `cycle_group_ct`
         cycle_group_ct input_point = to_grumpkin_point(
-            input.points[i], input.points[i + 1], input.points[i + 2], has_valid_witness_assignments, builder);
+            input.points[i], input.points[i + 1], input.points[i + 2], has_valid_witness_assignments, true, builder);
 
         //  Reconstruct the scalar from the low and high limbs
         field_ct scalar_low_as_field = to_field_ct(input.scalars[2 * (i / 3)], builder);

barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.cpp

@@ -5,6 +5,7 @@
 // =====================
 
 #include "witness_constant.hpp"
+#include "barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp"
 #include "barretenberg/ecc/curves/bn254/fr.hpp"
 
 namespace acir_format {
@@ -16,12 +17,20 @@ bb::stdlib::cycle_group<Builder> to_grumpkin_point(const WitnessOrConstant<FF>&
                                                    const WitnessOrConstant<FF>& input_y,
                                                    const WitnessOrConstant<FF>& input_infinite,
                                                    bool has_valid_witness_assignments,
+                                                   bool use_g1,
                                                    Builder& builder)
 {
-    using bool_ct = bb::stdlib::bool_t<Builder>;
     auto point_x = to_field_ct(input_x, builder);
     auto point_y = to_field_ct(input_y, builder);
-    auto infinite = bool_ct(to_field_ct(input_infinite, builder));
+    // We assume input_infinite is boolean, so we do not add a boolean gate
+    bool_t infinite(&builder);
+    if (!input_infinite.is_constant) {
+        infinite.witness_index = input_infinite.index;
+        infinite.witness_bool = get_value(input_infinite, builder) == FF::one();
+    } else {
+        infinite.witness_index = IS_CONSTANT;
+        infinite.witness_bool = input_infinite.value == FF::one();
+    }
 
     // When we do not have the witness assignments, we set is_infinite value to true if it is not constant
     // else default values would give a point which is not on the curve and this will fail verification
@@ -32,8 +41,15 @@ bb::stdlib::cycle_group<Builder> to_grumpkin_point(const WitnessOrConstant<FF>&
             // else, if is_infinite is false, but the coordinates (x, y) are witness (and not constant)
             // then we set their value to an arbitrary valid curve point (in our case G1).
             auto g1 = bb::grumpkin::g1::affine_one;
-            builder.set_variable(input_x.index, g1.x);
-            builder.set_variable(input_y.index, g1.y);
+
+            if (use_g1) {
+                builder.set_variable(input_x.index, g1.x);
+                builder.set_variable(input_y.index, g1.y);
+            } else {
+                auto g2 = g1 + g1;
+                builder.set_variable(input_x.index, g2.x);
+                builder.set_variable(input_y.index, g2.y);
+            }
         }
     }
     cycle_group<Builder> input_point(point_x, point_y, infinite);
@@ -44,11 +60,80 @@ template bb::stdlib::cycle_group<UltraCircuitBuilder> to_grumpkin_point(const Wi
                                                                         const WitnessOrConstant<fr>& input_y,
                                                                         const WitnessOrConstant<fr>& input_infinite,
                                                                         bool has_valid_witness_assignments,
+                                                                        bool use_g1,
                                                                         UltraCircuitBuilder& builder);
 template bb::stdlib::cycle_group<MegaCircuitBuilder> to_grumpkin_point(const WitnessOrConstant<fr>& input_x,
                                                                        const WitnessOrConstant<fr>& input_y,
                                                                        const WitnessOrConstant<fr>& input_infinite,
                                                                        bool has_valid_witness_assignments,
+                                                                       bool use_g1,
                                                                        MegaCircuitBuilder& builder);
 
+template <typename Builder, typename FF>
+bb::stdlib::cycle_group<Builder> to_witness_grumpkin_point(const WitnessOrConstant<FF>& input_x,
+                                                           const WitnessOrConstant<FF>& input_y,
+                                                           const WitnessOrConstant<FF>& input_infinite,
+                                                           bool has_valid_witness_assignments,
+                                                           bool use_g1,
+                                                           Builder& builder)
+{
+    // Creates a Grumpkin point(cycle_group) from WitnessOrConstant inputs by always using
+    // witnesses, even if the inputs are constant.
+    auto point_x = to_field_ct(input_x, builder);
+    if (point_x.is_constant()) {
+        point_x.convert_constant_to_fixed_witness(&builder);
+    }
+    auto point_y = to_field_ct(input_y, builder);
+    if (point_y.is_constant()) {
+        point_y.convert_constant_to_fixed_witness(&builder);
+    }
+    // We assume input_infinite is boolean, so we do not add a boolean gate
+    bool_t infinite(&builder);
+    if (!input_infinite.is_constant) {
+        infinite.witness_index = input_infinite.index;
+        infinite.witness_bool = get_value(input_infinite, builder) == FF::one();
+    } else {
+        infinite.witness_index = IS_CONSTANT;
+        infinite.witness_bool = input_infinite.value == FF::one();
+        infinite.convert_constant_to_fixed_witness(&builder);
+    }
+
+    // When we do not have the witness assignments, we set is_infinite value to true if it is not constant
+    // else default values would give a point which is not on the curve and this will fail verification
+    // If is_infinite is constant and false, and since the created point is using witnesses for the coordinates,
+    // we set their value to a valid curve point (in our case G1).
+    if (!has_valid_witness_assignments) {
+        if (!input_infinite.is_constant) {
+            builder.set_variable(input_infinite.index, fr(1));
+        } else if (input_infinite.value == fr::zero() && !point_x.is_constant() && !point_x.is_constant()) {
+
+            auto g1 = bb::grumpkin::g1::affine_one;
+            if (use_g1) {
+                builder.set_variable(point_x.witness_index, g1.x);
+                builder.set_variable(point_y.witness_index, g1.y);
+            } else {
+                auto g2 = g1 + g1;
+                builder.set_variable(point_x.witness_index, g2.x);
+                builder.set_variable(point_y.witness_index, g2.y);
+            }
+        }
+    }
+    cycle_group<Builder> input_point(point_x, point_y, infinite);
+    return input_point;
+}
+
+template bb::stdlib::cycle_group<UltraCircuitBuilder> to_witness_grumpkin_point(
+    const WitnessOrConstant<fr>& input_x,
+    const WitnessOrConstant<fr>& input_y,
+    const WitnessOrConstant<fr>& input_infinite,
+    bool has_valid_witness_assignments,
+    bool use_g1,
+    UltraCircuitBuilder& builder);
+template bb::stdlib::cycle_group<MegaCircuitBuilder> to_witness_grumpkin_point(
+    const WitnessOrConstant<fr>& input_x,
+    const WitnessOrConstant<fr>& input_y,
+    const WitnessOrConstant<fr>& input_infinite,
+    bool has_valid_witness_assignments,
+    bool use_g1,
+    MegaCircuitBuilder& builder);
 } // namespace acir_format

barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.hpp

@@ -43,6 +43,23 @@ bb::stdlib::cycle_group<Builder> to_grumpkin_point(const WitnessOrConstant<FF>&
                                                    const WitnessOrConstant<FF>& input_y,
                                                    const WitnessOrConstant<FF>& input_infinite,
                                                    bool has_valid_witness_assignments,
+                                                   bool use_g1,
                                                    Builder& builder);
 
-} // namespace acir_format
+template <typename Builder, typename FF>
+bb::stdlib::cycle_group<Builder> to_witness_grumpkin_point(const WitnessOrConstant<FF>& input_x,
+                                                           const WitnessOrConstant<FF>& input_y,
+                                                           const WitnessOrConstant<FF>& input_infinite,
+                                                           bool has_valid_witness_assignments,
+                                                           bool use_g1,
+                                                           Builder& builder);
+
+template <typename Builder, typename FF> FF get_value(const WitnessOrConstant<FF>& input, Builder& builder)
+{
+    if (input.is_constant) {
+        return input.value;
+    }
+    return builder.get_variable(input.index);
+}
+
+} // namespace acir_format

barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.hpp

@@ -82,6 +82,19 @@ template <typename Builder> class bool_t {
     mutable bool witness_inverted = false;
     mutable uint32_t witness_index = IS_CONSTANT;
     mutable OriginTag tag{};
+
+    /**
+     * Create a witness from a constant. This way the value of the witness is fixed and public (public, because the
+     * value becomes hard-coded as an element of the q_c selector vector).
+     */
+    void convert_constant_to_fixed_witness(Builder* ctx)
+    {
+        ASSERT(is_constant() && ctx);
+        context = ctx;
+        (*this) = bool_t<Builder>(witness_t<Builder>(context, get_value()));
+        context->fix_witness(witness_index, get_value());
+        unset_free_witness_tag();
+    }
 };
 
 template <typename T> inline std::ostream& operator<<(std::ostream& os, bool_t<T> const& v)