Complete add for AffineGagdet + fix serialization to field elements

algebra/src/to_field_vec.rs

@@ -1,15 +1,12 @@
-use crate::{
-    fields::{
-        Field, FpParameters, PrimeField,
-    },
-    curves::{
-        Curve,
-        models::{SWModelParameters, TEModelParameters},
-        short_weierstrass_jacobian::Jacobian,
-        short_weierstrass_projective::Projective,
-        twisted_edwards_extended::TEExtended,
-    }, ToBits,
-};
+use crate::{fields::{
+    Field, FpParameters, PrimeField,
+}, curves::{
+    Curve,
+    models::{SWModelParameters, TEModelParameters},
+    short_weierstrass_jacobian::Jacobian,
+    short_weierstrass_projective::Projective,
+    twisted_edwards_extended::TEExtended,
+}, ToBits, Group};
 
 type Error = Box<dyn std::error::Error>;
 
@@ -72,12 +69,13 @@ impl<M: SWModelParameters, ConstraintF: Field> ToConstraintField<ConstraintF> fo
             x_fe.extend_from_slice(&y_fe);
             Ok(x_fe)
         } else {
-            // Otherwise, return the coordinates of the infinity representation in Jacobian
-            let mut x_fe = self.x.to_field_elements()?;
-            let y_fe = self.y.to_field_elements()?;
+            // Otherwise, serialize the point as the affine point x=0, y=0,
+            // which is for sure not on the curve unless b=0 (which should never happen in our case
+            // as if b=0 then the curve has non-prime order).
+            debug_assert!(!M::COEFF_B.is_zero());
+            let mut x_fe = M::BaseField::zero().to_field_elements()?;
+            let y_fe = M::BaseField::zero().to_field_elements()?;
             x_fe.extend_from_slice(&y_fe);
-            let z_fe = self.z.to_field_elements()?;
-            x_fe.extend_from_slice(&z_fe);
             Ok(x_fe)
         }
     }
@@ -96,12 +94,13 @@ impl<M: SWModelParameters, ConstraintF: Field> ToConstraintField<ConstraintF> fo
             x_fe.extend_from_slice(&y_fe);
             Ok(x_fe)
         } else {
-            // Otherwise, return the coordinates of the infinity representation in Projective
-            let mut x_fe = self.x.to_field_elements()?;
-            let y_fe = self.y.to_field_elements()?;
+            // Otherwise, serialize the point as the affine point x=0, y=0,
+            // which is for sure not on the curve unless b=0 (which should never happen in our case
+            // as if b=0 then the curve has non-prime order).
+            debug_assert!(!M::COEFF_B.is_zero());
+            let mut x_fe = M::BaseField::zero().to_field_elements()?;
+            let y_fe = M::BaseField::zero().to_field_elements()?;
             x_fe.extend_from_slice(&y_fe);
-            let z_fe = self.z.to_field_elements()?;
-            x_fe.extend_from_slice(&z_fe);
             Ok(x_fe)
         }
     }

r1cs/gadgets/std/src/fields/fp.rs

@@ -6,8 +6,9 @@ use r1cs_core::{
 };
 
 use std::borrow::Borrow;
+use std::convert::TryInto;
 
-use crate::{boolean::AllocatedBit, prelude::*, Assignment};
+use crate::{boolean::AllocatedBit, prelude::*, Assignment, FromGadget};
 
 #[derive(Debug)]
 pub struct FpGadget<F: PrimeField> {
@@ -125,6 +126,60 @@ impl<F: PrimeField> FpGadget<F> {
     }
 }
 
+impl<F: PrimeField> TryInto<Boolean> for FpGadget<F> {
+    type Error = SynthesisError;
+
+    fn try_into(self) -> Result<Boolean, Self::Error> {
+
+        let variable = match self.get_variable() {
+            Var(var) =>  var,
+            LC(_) => Err(SynthesisError::Other(String::from("cannot convert linear combination of variables to Boolean variable")))?
+        };
+        let value = self.get_value().map(|val| if val.is_zero() {
+            Ok(Some(false))
+        } else if val.is_one() {
+            Ok(Some(true))
+        } else {
+            Err(SynthesisError::Other(String::from("converting field element other than 0 or 1 to Boolean")))
+        }).unwrap_or(Ok(None))?;
+        let alloc_bit = AllocatedBit{
+            variable,
+            value,
+        };
+        Ok(Boolean::from(alloc_bit))
+    }
+}
+
+impl<F: PrimeField> FromGadget<Boolean, F> for FpGadget<F> {
+    fn from<CS: ConstraintSystemAbstract<F>>(bit: Boolean, mut cs: CS) -> Result<Self, SynthesisError> {
+        if bit.is_constant() {
+            return if bit.get_value().unwrap() {
+                Self::one(cs.ns(|| "alloc constant 1"))
+            } else {
+                Self::zero(cs.ns(|| "alloc constant 0"))
+            }
+        }
+
+        let alloc_bit = match bit {
+            Boolean::Is(alloc_bit) | Boolean::Not(alloc_bit) => alloc_bit,
+            _ => unreachable!(),
+        };
+
+        let variable = ConstraintVar::<F>::from(alloc_bit.get_variable());
+        let value = bit.get_value().map(|bit_val| if bit_val {
+            F::one()
+        } else {
+            F::zero()
+        });
+
+        Ok(
+            Self{
+                value,
+                variable,
+        })
+    }
+}
+
 impl<F: PrimeField> FieldGadget<F, F> for FpGadget<F> {
     type Variable = ConstraintVar<F>;
 

r1cs/gadgets/std/src/fields/mod.rs

@@ -1,9 +1,10 @@
+use std::convert::TryInto;
 // use std::ops::{Mul, MulAssign};
 use algebra::{BitIterator, Field};
 use r1cs_core::{ConstraintSystemAbstract, SynthesisError};
 use std::fmt::Debug;
 
-use crate::{prelude::*, Assignment};
+use crate::{prelude::*, Assignment, FromGadget};
 
 pub mod cmp;
 pub mod fp;
@@ -23,6 +24,8 @@ pub trait FieldGadget<F: Field, ConstraintF: Field>:
     + TwoBitLookupGadget<ConstraintF, TableConstant = F>
     + ThreeBitCondNegLookupGadget<ConstraintF, TableConstant = F>
     + Debug
+    + TryInto<Boolean, Error=SynthesisError>
+    + FromGadget<Boolean, ConstraintF>
 {
     type Variable: Clone + Debug;
 

r1cs/gadgets/std/src/fields/nonnative/nonnative_field_gadget.rs

@@ -12,23 +12,16 @@ use num_bigint::BigUint;
 use num_traits::{One, Zero};
 
 use crate::fields::nonnative::NonNativeFieldParams;
-use crate::{
-    fields::fp::FpGadget,
-    fields::nonnative::{
-        nonnative_field_mul_result_gadget::NonNativeFieldMulResultGadget,
-        params::get_params,
-        reduce::Reducer,
-    },
-    fields::FieldGadget,
-    ceil_log_2,
-    prelude::*,
-    to_field_gadget_vec::ToConstraintFieldGadget,
-    Assignment,
-};
+use crate::{fields::fp::FpGadget, fields::nonnative::{
+    nonnative_field_mul_result_gadget::NonNativeFieldMulResultGadget,
+    params::get_params,
+    reduce::Reducer,
+}, fields::FieldGadget, ceil_log_2, prelude::*, to_field_gadget_vec::ToConstraintFieldGadget, Assignment, FromGadget};
 use r1cs_core::{ConstraintSystemAbstract, SynthesisError};
 use std::cmp::max;
 use std::marker::PhantomData;
 use std::{borrow::Borrow, vec, vec::Vec};
+use std::convert::TryInto;
 
 #[derive(Debug, Eq, PartialEq)]
 #[must_use]
@@ -1008,6 +1001,20 @@ impl<SimulationF: PrimeField, ConstraintF: PrimeField>
  *  The high-level functions for arithmetic mod p: Implementation of FieldGadget
  * 
  * *****************************************************************************/
+impl<SimulationF: PrimeField, ConstraintF: PrimeField> TryInto<Boolean> for NonNativeFieldGadget<SimulationF, ConstraintF> {
+    type Error = SynthesisError;
+
+    fn try_into(self) -> Result<Boolean, Self::Error> {
+        unimplemented!()
+    }
+}
+
+impl<SimulationF: PrimeField, ConstraintF: PrimeField> FromGadget<Boolean, ConstraintF> for NonNativeFieldGadget<SimulationF, ConstraintF> {
+    fn from<CS: ConstraintSystemAbstract<ConstraintF>>(other: Boolean, cs: CS) -> Result<Self, SynthesisError> {
+        unimplemented!()
+    }
+}
+
 impl<SimulationF: PrimeField, ConstraintF: PrimeField> FieldGadget<SimulationF, ConstraintF>
     for NonNativeFieldGadget<SimulationF, ConstraintF>
 {

r1cs/gadgets/std/src/groups/curves/short_weierstrass/short_weierstrass_jacobian.rs

@@ -259,6 +259,7 @@ where
     /// Compute 2 * self + other.
     /// Incomplete, safe, addition: neither `self` nor `other` can be the neutral
     /// element, and other != ±self.
+    // // ToDo: evaluate if it may be worthy to make it complete
     pub fn double_and_add<CS: ConstraintSystemAbstract<ConstraintF>>(
         &self,
         cs: CS,
@@ -333,8 +334,8 @@ where
     #[inline]
     fn zero<CS: ConstraintSystemAbstract<ConstraintF>>(mut cs: CS) -> Result<Self, SynthesisError> {
         Ok(Self::new(
-            F::zero(cs.ns(|| "zero"))?,
-            F::one(cs.ns(|| "one"))?,
+            F::zero(cs.ns(|| "x=zero"))?,
+            F::zero(cs.ns(|| "y=zero"))?,
             Boolean::constant(true),
         ))
     }
@@ -348,18 +349,120 @@ where
     }
 
     #[inline]
-    /// Incomplete, safe, addition: neither `self` nor `other` can be the neutral
-    /// element.
+    // Algorithm for complete addition ported in R1CS from zcash: https://github.com/ebfull/halo/blob/master/src/gadgets/ecc.rs#L357.
+    // The algorithm from zcash works only with Weierstrass curves y^2 = x^3+b, the algorithm here
+    // is modified to work with generic curves y^2 = x^3 +ax + b
     fn add<CS: ConstraintSystemAbstract<ConstraintF>>(
         &self,
-        cs: CS,
+        mut cs: CS,
         other: &Self,
     ) -> Result<Self, SynthesisError> {
-        self.add_internal(cs, other, true)
+        let zero = F::zero(cs.ns(|| "alloc constant 0"))?;
+        let is_same_x = F::alloc(cs.ns(|| "alloc x1 == x2"),
+                                     || Ok(
+                                         if self.x.get_value().get()? == other.x.get_value().get()? {
+                                             P::BaseField::one()
+                                         } else {
+                                             P::BaseField::zero()
+                                         })
+        )?;
+        let not_is_same_x = is_same_x.negate(cs.ns(|| "-is_same_x"))?.add_constant(cs.ns(|| "1 - is_same_x"), &P::BaseField::one())?;
+
+        let x2_minus_x1 = other.x.sub(cs.ns(|| "x2 - x1"), &self.x)?;
+        let inv = F::alloc(cs.ns(|| "alloc (x2-x1)^-1"), ||
+            Ok(
+                x2_minus_x1.get_value().get()?.inverse().unwrap_or(P::BaseField::one())
+            )
+        )?;
+
+        // enforce that is_same_x = 0 if x2 != x1
+        x2_minus_x1.mul_equals(cs.ns(|| "enforce (x2-x1)*is_same_x = 0"), &is_same_x, &zero)?;
+        // enforce that is_same_x = 1 if x2 == x1
+        x2_minus_x1.mul_equals(cs.ns(|| "enforce (x2-x1)*(x2-x1)^-1 = 1-is_same_x"), &inv, &not_is_same_x)?;
+
+        // compute lambda_diff = (y2-y1)/(x2-x1+is_same_x), the value for lambda to be used in case x1 != x2
+        let y2_minus_y1 = other.y.sub(cs.ns(|| "y2 - y1"), &self.y)?;
+        let x2_minus_x1_plus_x_same = x2_minus_x1.add(cs.ns(|| "x2 - x1 + x_is_same"), &is_same_x)?;
+        let lambda_diff = F::alloc(cs.ns(|| "alloc lambda_diff"), || Ok(
+            y2_minus_y1.get_value().get()?*x2_minus_x1_plus_x_same.get_value().get()?.inverse().unwrap()
+            // safe to unwrap inverse as x2_minus_x1_plus_x_same cannot be 0 by construction
+            )
+        )?;
+        lambda_diff.mul_equals(cs.ns(|| "enforce lambda_diff = (y2-y1) div (x2-x1+is_same_x"), &x2_minus_x1_plus_x_same, &y2_minus_y1)?;
+
+        // compute lambda_same = (3x1^2+a)/(2y1), the value for lambda to be used in case x1 = x2.
+        // in case y1 = 0, which happens only when self is the identity, the constraint
+        // 2y1*lambda_same = 3x1^2+a cannot be satisfied, unless a == 0.
+        // Therefore, we modify the constraint as (2y1+self.infinity)*lambda_same = 3x1^2+a, which
+        // allows to satisfy the constraint when y1=0 for an arbitrary value of a
+        let infinity_flag_to_field = F::from(self.infinity, cs.ns(|| "self infinity to field element"))?;
+        let two_y1_plus_infinity_flag = self.y.double(cs.ns(|| "2y1"))?.add(cs.ns(|| "2y1+self.infinity"), &infinity_flag_to_field)?;
+        let three_times_x1_square_plus_a = self.x.square(cs.ns(|| "x1^2"))?.mul_by_constant(cs.ns(|| "3x1^2"), &P::BaseField::from(3u128))?.add_constant(cs.ns(|| "3x1^2+a"), &P::COEFF_A)?;
+        let lambda_same = F::alloc(cs.ns(|| "alloc lambda_same"), || Ok(
+            three_times_x1_square_plus_a.get_value().get()?*two_y1_plus_infinity_flag.get_value().get()?.inverse().unwrap_or(P::BaseField::one())
+        ))?;
+        lambda_same.mul_equals(cs.ns(|| "enforce lambda_same = (3x1^2+a) div 2y1"), &two_y1_plus_infinity_flag, &three_times_x1_square_plus_a)?;
+
+        // set lambda to either lambda_same or lambda_diff depending on is_same_x flag
+        let lambda = F::alloc(cs.ns(|| "alloc lambda"), || Ok(
+            if is_same_x.get_value().get()?.is_one() {
+                lambda_same.get_value().get()?
+            } else {
+                lambda_diff.get_value().get()?
+            }
+        ))?;
+        let lambda_minus_diff = lambda.sub(cs.ns(|| "lambda - lambda_diff"), &lambda_diff)?;
+        let lambda_same_minus_diff = lambda_same.sub(cs.ns(|| "lambda_same - lambda_diff"), &lambda_diff)?;
+        lambda_same_minus_diff.mul_equals(cs.ns(|| "enforce (lambda-lambda_diff) = is_same_x*(lambda_same-lambda_diff)"), &is_same_x, &lambda_minus_diff)?;
+
+        // x3 = lambda^2-x1-x2
+        let x1_plus_x2 = self.x.add(cs.ns(|| "x1+x2"), &other.x)?;
+        let x3 = lambda.square(cs.ns(|| "lambda^2"))?.sub(cs.ns(|| "x3=lambda^2-x1-x2"), &x1_plus_x2)?;
+        // y3 = lambda*(x1-x3)-y1
+        let x1_minus_x3 = self.x.sub(cs.ns(|| "x1-x3"), &x3)?;
+        let y3 = lambda.mul(cs.ns(|| "lambda*(x1-x3)"), &x1_minus_x3)?.sub(cs.ns(|| "y3=lambda*(x1-x3)-y1"), &self.y)?;
+
+        // compute a flag is_sum_zero which is true iff self+other == identity
+        let y1_plus_y2 = self.y.add(cs.ns(|| "y1+y2"), &other.y)?;
+        let is_sum_zero = F::alloc(cs.ns(|| "alloc self+other == 0"), || Ok(
+            if y1_plus_y2.get_value().get()?.is_zero() {
+                is_same_x.get_value().get()?
+            } else {
+                P::BaseField::zero()
+            }
+        ))?;
+        // enforce sum_is_zero == 0 if y1+y2 != 0, that is if the sum is not the identity
+        y1_plus_y2.mul_equals(cs.ns(|| "enforce (y1+y2)*sum_is_zero=0"), &is_sum_zero, &zero)?;
+        let same_x_minus_sum_zero = is_same_x.sub(cs.ns(|| "is_same_x-sum_is_zero"), &is_sum_zero)?;
+        let inv = F::alloc(cs.ns(|| "alloc (y1+y2)^-1"), || Ok(
+            // set inv to is_same_x if y1+y2 !=0, otherwise we can set it to an arbitrary as it has no impact on the constraint
+            is_same_x.get_value().get()?*y1_plus_y2.get_value().get()?.inverse().unwrap_or(P::BaseField::one())
+        ))?;
+
+        // enforce sum_is_zero == is_same_x if y1+y2 == 0, as in this case the sum is the identity iff the x-coordinates are the same
+        y1_plus_y2.mul_equals(cs.ns(|| "enforce (y1+y2)*(y1+y2)^-1=is_same_x-sum_is_zero"), &inv, &same_x_minus_sum_zero)?;
+
+        // enforce that x4 = y4 = 0 if sum is zero
+        let is_sum_not_zero = is_sum_zero.negate(cs.ns(|| "-is_sum_zero"))?.add_constant(cs.ns(|| "1-is_sum_zero"), &P::BaseField::one())?;
+        let x4 = x3.mul(cs.ns(|| "x4 = x3*(1-is_sum_zero)"), &is_sum_not_zero)?;
+        let y4 = y3.mul(cs.ns(|| "y4 = y3*(1-is_sum_zero)"), &is_sum_not_zero)?;
+
+        // deal with the case when self is the identity
+        let x5 = F::conditionally_select(cs.ns(|| "x5=x2 if self == 0, x4 otherwise"), &self.infinity, &other.x, &x4)?;
+        let y5 = F::conditionally_select(cs.ns(|| "y5=y2 if self == 0, y4 otherwise"), &self.infinity, &other.y, &y4)?;
+
+        // deal with the case when other is the identity
+        let x_out = F::conditionally_select(cs.ns(|| "x_out=x1 if other == 0, x5 otherwise"), &other.infinity, &self.x, &x5)?;
+        let y_out = F::conditionally_select(cs.ns(|| "y_out=y1 if other == 0, y5 otherwise"), &other.infinity, &self.y, &y5)?;
+
+        let infinity_out = F::try_into(is_sum_zero)?;
+
+        Ok(Self::new(x_out, y_out, infinity_out))
     }
 
     /// Incomplete addition: neither `self` nor `other` can be the neutral
     /// element.
+    // ToDo: make addition complete
     fn add_constant<CS: ConstraintSystemAbstract<ConstraintF>>(
         &self,
         mut cs: CS,
@@ -804,7 +907,8 @@ where
                 // Last chunk we must use safe add
                 _ => {
                     let adder: Self = Self::new(x, y, Boolean::constant(false));
-                    acc = acc.add(cs.ns(|| format!("Add_{}", i)), &adder)?;
+                    // acc = acc.add(cs.ns(|| format!("Add_{}", i)), &adder)?;
+                    acc = acc.add_internal(cs.ns(|| format!("Add_{}", i)), &adder, true)?;
                 }
             }
 
@@ -1117,7 +1221,7 @@ where
             Ok(ge) => {
                 let ge = ge.borrow();
                 if ge.is_zero() {
-                    (Ok(P::BaseField::zero()), Ok(P::BaseField::one()), Ok(true))
+                    (Ok(P::BaseField::zero()), Ok(P::BaseField::zero()), Ok(true))
                 } else {
                     let ge = ge.into_affine().unwrap();
                     (Ok(ge.x), Ok(ge.y), Ok(false))
@@ -1170,7 +1274,7 @@ where
             Ok(ge) => {
                 let ge = ge.borrow();
                 if ge.is_zero() {
-                    (Ok(P::BaseField::zero()), Ok(P::BaseField::one()), Ok(true))
+                    (Ok(P::BaseField::zero()), Ok(P::BaseField::zero()), Ok(true))
                 } else {
                     let ge = ge.into_affine().unwrap();
                     (Ok(ge.x), Ok(ge.y), Ok(false))
@@ -1279,7 +1383,7 @@ where
             Ok(ge) => {
                 let ge = ge.borrow();
                 if ge.is_zero() {
-                    (Ok(P::BaseField::zero()), Ok(P::BaseField::one()), Ok(true))
+                    (Ok(P::BaseField::zero()), Ok(P::BaseField::zero()), Ok(true))
                 } else {
                     let ge = ge.into_affine().unwrap();
                     (Ok(ge.x), Ok(ge.y), Ok(false))