Fix circuit for cast lossy field to group.

This resolves the underconstraining bug that was found in that circuit.

The fix involves the creation of two new circuits, which are more general and
may have other uses:
- A circuit for square roots flagged nondeterministic, which return both square
  roots, in no specified order (hence 'nondeterministic'), if the input is a
  non-zero square, and returns an error flag set if the input is not a square.
- A circuit for turning an x coordinate into the (unique, if it exists) point
  in the subgroup with that x coordinate, with an error flag set if there is
  no such point in the subgroup. This circuit makes use of the previous one
  to solve the curve equation, i.e. to find whether and where x intersects
  the elliptic curve.

The new circuit for cast lossy field to group is realized by using the latter
circuit to attempt to find the point in the subgroup with the given value as x
coordinate. If none exists, we return the generator if the input is 0, otherwise
the result of Elligator-2 is returned, which is always a subgroup point.

The use of the new circuits for flagged operations eliminate the
underconstraining without eliminating desired solutions (which would happen by
enforcing constraints without taking flags into account).

circuit/program/src/data/literal/cast_lossy/field.rs

@@ -61,9 +61,12 @@ impl<E: Environment> CastLossy<Group<E>> for Field<E> {
     #[inline]
     fn cast_lossy(&self) -> Group<E> {
         // This method requires that an `x-coordinate` of 1 is an invalid group element.
-        // This is used by the ternary below, which uses 'is_one' to determine whether to return the generator.
+        // This is used by the ternary below, which uses 'is_x_one' to determine whether to return the generator.
         debug_assert!(console::Group::from_x_coordinate(<console::Field<E::Network> as console::One>::one()).is_err());
 
+        // Attempt to find a group element with self as the x-coordinate.
+        let (x_is_not_in_group, point_with_x) = Group::from_x_coordinate_flagged(self.clone());
+
         // Determine if the field element is zero.
         let is_x_zero = self.is_zero();
         // Determine if the field element is one.
@@ -73,35 +76,20 @@ impl<E: Environment> CastLossy<Group<E>> for Field<E> {
         let generator = Group::generator();
 
         // Determine the input to Elligator-2, based on the x-coordinate.
+        // If self is 0, we pass 1 to Elligator-2 instead.
+        // Note that, in this case, we won't use the result of Elligator-2,
+        // because the point (0, 1) is in the subgroup, and that is what we return.
         let elligator_input = Field::ternary(&is_x_zero, &Field::one(), self);
         // Perform Elligator-2 on the field element, to recover a group element.
-        let elligator = Elligator2::encode(&elligator_input);
-
-        // Determine the initial x-coordinate, if the given field element is one.
-        let initial_x = Field::ternary(&is_x_one, &generator.to_x_coordinate(), &elligator.to_x_coordinate());
-        // Determine the initial y-coordinate, if the given field element is one.
-        let initial_y = Field::ternary(&is_x_one, &generator.to_y_coordinate(), &elligator.to_y_coordinate());
-
-        // Determine the y-coordinate, if the x-coordinate is valid.
-        let possible_y: Field<E> = {
-            // Set the x-coordinate.
-            let x = self.clone();
-            // Derive the y-coordinate.
-            witness!(|x| match console::Group::from_x_coordinate(x) {
-                Ok(point) => point.to_y_coordinate(),
-                Err(_) => console::Zero::zero(),
-            })
-        };
-        // Determine if the recovered y-coordinate is zero.
-        let is_y_zero = possible_y.is_zero();
+        let elligator_point = Elligator2::encode(&elligator_input);
 
-        // Determine the final x-coordinate, based on whether the possible y-coordinate is zero.
-        let final_x = Field::ternary(&is_y_zero, &initial_x, self);
-        // Determine the final y-coordinate, based on whether the possible y-coordinate is zero.
-        let final_y = Field::ternary(&is_y_zero, &initial_y, &possible_y);
+        // Select either the generator or the result of Elligator-2, depending on whether x is 1 or not.
+        // This is only used when x is not in the group, see below.
+        let generator_or_elligator_point = Group::ternary(&is_x_one, &generator, &elligator_point);
 
-        // Return the result.
-        Group::from_xy_coordinates(final_x, final_y)
+        // Select either the group point with x or the generator or the result of Elligator-2,
+        // depending on whether x is in the group or not, and, if it is not, based on whether it is 1 or not.
+        Group::ternary(&x_is_not_in_group, &generator_or_elligator_point, &point_with_x)
     }
 }
 

circuit/types/field/src/square_root.rs

@@ -63,6 +63,69 @@ impl<E: Environment> Field<E> {
     }
 }
 
+impl<E: Environment> Field<E> {
+    /// Returns both square roots of `self` (hence the plural 'roots' in the name of the function),
+    /// along with a boolean error flag, which is set iff `self` is not a square.
+    ///
+    /// In the console computation:
+    /// if `self` is a non-zero square,
+    /// the first field result is the positive root (i.e. closer to 0)
+    /// and the second field result is the negative root (i.e. closer to the prime);
+    /// if `self` is 0, both field results are 0;
+    /// if `self` is not a square, both field results are 0, but immaterial.
+    ///
+    /// The 'nondeterministic' part of the function name refers to the synthesized circuit,
+    /// whose represented computation, unlike the console computation just described,
+    /// returns the two roots (if `self` is a non-zero square) in no specified order.
+    /// This nondeterminism saves constraints, but generally this circuit should be only used
+    /// as part of larger circuits for which the nondeterminism in the order of the two roots does not matter,
+    /// and where the larger circuits represent deterministic computations despite this internal nondeterminism.
+    pub fn square_roots_flagged_nondeterministic(&self) -> (Boolean<E>, Self, Self) {
+        // Obtain (p-1)/2, as a constant field element.
+        let modulus_minus_one_div_two = match E::BaseField::from_bigint(E::BaseField::modulus_minus_one_div_two()) {
+            Some(modulus_minus_one_div_two) => Field::constant(console::Field::new(modulus_minus_one_div_two)),
+            None => E::halt("Failed to initialize (modulus - 1) / 2"),
+        };
+
+        // Use Euler's criterion: self is a non-zero square iff self^((p-1)/2) is 1.
+        let euler = self.pow(modulus_minus_one_div_two);
+        let is_nonzero_square = euler.is_equal(&Field::one());
+
+        // Calculate the witness for the first square result.
+        // The called function square_root returns the square root closer to 0.
+        let root_witness = match self.eject_value().square_root() {
+            Ok(root) => root,
+            Err(_) => E::halt("Failed to calculate square root witness"),
+        };
+
+        // In order to avoid actually calculating the square root in the circuit,
+        // we would like to generate a constraint saying that squaring the root yields self.
+        // But this constraint would have no solutions if self is not a square.
+        // So we introduce a new variable that is either self (if square) or 0 (otherwise):
+        // either way, this new variable is a square.
+        let square = Self::ternary(&is_nonzero_square, self, &Field::zero());
+
+        // We introduce a variable for the first root we return,
+        // and constrain it to yield, when squared, the square introduced just above.
+        // Thus, if self is a square this is a square root of self; otherwise it is 0, because only 0 yields 0 when squared.
+        // The variable is actually a constant if self is constant, otherwise it is private (even if self is public).
+        let mode = if self.eject_mode() == Mode::Constant { Mode::Constant } else { Mode::Private };
+        let first_root = Field::new(mode, root_witness);
+        E::enforce(|| (&first_root, &first_root, &square));
+
+        // The second root returned by this function is the negation of the first one.
+        // So if self is a non-zero square, this is always different from the first root,
+        // but in the circuit it can be either positive (and the other negative) or vice versa.
+        let second_root = first_root.clone().neg();
+
+        // The error flag is set iff self is a non-square, i.e. it is neither zero nor a non-zero square.
+        let is_nonzero = self.is_not_equal(&Field::zero());
+        let error_flag = is_nonzero.bitand(is_nonzero_square.not());
+
+        (error_flag, first_root, second_root)
+    }
+}
+
 impl<E: Environment> Metrics<dyn SquareRoot<Output = Field<E>>> for Field<E> {
     type Case = Mode;
 

circuit/types/group/src/helpers/from_x_coordinate.rs

@@ -26,6 +26,76 @@ impl<E: Environment> Group<E> {
 
         Self::from_xy_coordinates(x, y)
     }
+
+    /// Initializes an affine group element from a given x-coordinate field element.
+    /// Also returns an error flag, set if there is no group element with the given x-coordinate;
+    /// in that case, the returned point is `(0, 0)`, but immaterial.
+    pub fn from_x_coordinate_flagged(x: Field<E>) -> (Boolean<E>, Self) {
+        // Obtain the A and D coefficients of the elliptic curve.
+        let a = Field::constant(console::Field::new(E::EDWARDS_A));
+        let d = Field::constant(console::Field::new(E::EDWARDS_D));
+
+        // Compute x^2.
+        let xx = &x * &x;
+
+        // Compute a * x^2 - 1.
+        let a_xx_minus_1 = a * &xx - Field::one();
+
+        // Compute d * x^2 - 1.
+        let d_xx_minus_1 = d * &xx - Field::one();
+
+        // Compute y^2 = (a * x^2 - 1) / (d * x^2 - 1), i.e. solve the curve equation for y^2.
+        let yy: Field<E> = witness!(|a_xx_minus_1, d_xx_minus_1| { a_xx_minus_1 / d_xx_minus_1 });
+        E::enforce(|| (&yy, &d_xx_minus_1, &a_xx_minus_1));
+
+        // Compute both square roots of y^2, in no specified order, with a flag saying whether y^2 is a square or not.
+        // That is, finish solving the curve equation for y.
+        // If the x-coordinate line does not intersect the elliptic curve, this returns (1, 0, 0).
+        let (yy_is_not_square, y1, y2) = yy.square_roots_flagged_nondeterministic();
+
+        // Form the two points, which are on the curve if yy_is_not_square is false.
+        // Note that the Group<E> type is not restricted to the points in the subgroup or even on the curve;
+        // it includes all possible points, i.e. all possible pairs of field elements.
+        let point1 = Self { x: x.clone(), y: y1.clone() };
+        let point2 = Self { x: x.clone(), y: y2.clone() };
+
+        // We need to check whether either of the two points is in the subgroup.
+        // There may be at most one, but in a circuit we need to always represent both computation paths.
+        // In fact, we represent this computation also when yy_is_not_square is true,
+        // in which case the results of checking whether either point is in the subgroup are meaningless,
+        // but ignored in the final selection of the results returned below.
+        // The criterion for membership in the subgroup is that
+        // multiplying the point by the subgroup order yields the zero point (0, 1).
+        // The group operation that we use here is for the type `Group<E>` of the subgroup,
+        // which as mentioned above it can be performed on points outside the subgroup as well.
+        // We turn the subgroup order into big endian bits,
+        // to get around the issue that the subgroup order is not of Scalar<E> type.
+        let order = E::ScalarField::modulus();
+        let order_bits_be = order.to_bits_be();
+        let mut order_bits_be_constants = Vec::new();
+        for bit in order_bits_be.iter() {
+            order_bits_be_constants.push(Boolean::constant(bit.clone()));
+        }
+        let point1_times_order = order_bits_be_constants.mul(point1);
+        let point2_times_order = order_bits_be_constants.mul(point2);
+        let point1_is_in_subgroup = point1_times_order.is_equal(&Self::zero());
+        let point2_is_in_subgroup = point2_times_order.is_equal(&Self::zero());
+
+        // We select y1 if (x, y1) is in the subgroup (which implies that (x, y2) is not in the subgroup),
+        // or y2 if (x, y2) is in the subgroup (which implies that (x, y1) is not in the subgroup),
+        // or 0 if neither is in the subgroup, or x does not even intersect the elliptic curve.
+        // Since at most one of the two points can be in the subgroup, the order of y1 and y2 returned by square root is immaterial:
+        // that nondeterminism (in the circuit) is resolved, and the circuit for from_x_coordinate_flagged is deterministic.
+        let y2_or_zero = Field::ternary(&point2_is_in_subgroup, &y2, &Field::zero());
+        let y1_or_y2_or_zero = Field::ternary(&point1_is_in_subgroup, &y1, &y2_or_zero);
+        let y = Field::ternary(&yy_is_not_square, &Field::zero(), &y1_or_y2_or_zero);
+
+        // The error flag is set iff x does not intersect the elliptic curve or neither intersection point is in the subgroup.
+        let neither_in_subgroup = point1_is_in_subgroup.not().bitand(point2_is_in_subgroup.not());
+        let error_flag = yy_is_not_square.bitor(&neither_in_subgroup);
+
+        (error_flag, Self { x, y })
+    }
 }
 
 #[cfg(test)]