Merge pull request #33 from ebfull/general-improvements

General improvements

src/circuit/boolean.rs

@@ -234,6 +234,77 @@ impl AllocatedBit {
     }
 }
 
+pub fn u64_into_allocated_bits_be<E: Engine, CS: ConstraintSystem<E>>(
+    mut cs: CS,
+    value: Option<u64>
+) -> Result<Vec<AllocatedBit>, SynthesisError>
+{
+    let values = match value {
+        Some(ref value) => {
+            let mut tmp = Vec::with_capacity(64);
+
+            for i in (0..64).rev() {
+                tmp.push(Some(*value >> i & 1 == 1));
+            }
+
+            tmp
+        },
+        None => {
+            vec![None; 64]
+        }
+    };
+
+    let bits = values.into_iter().enumerate().map(|(i, b)| {
+        AllocatedBit::alloc(
+            cs.namespace(|| format!("bit {}", i)),
+            b
+        )
+    }).collect::<Result<Vec<_>, SynthesisError>>()?;
+
+    Ok(bits)
+}
+
+pub fn field_into_allocated_bits_be<E: Engine, CS: ConstraintSystem<E>, F: PrimeField>(
+    mut cs: CS,
+    value: Option<F>
+) -> Result<Vec<AllocatedBit>, SynthesisError>
+{
+    let values = match value {
+        Some(ref value) => {
+            let mut field_char = BitIterator::new(F::char());
+
+            let mut tmp = Vec::with_capacity(F::NUM_BITS as usize);
+
+            let mut found_one = false;
+            for b in BitIterator::new(value.into_repr()) {
+                // Skip leading bits
+                found_one |= field_char.next().unwrap();
+                if !found_one {
+                    continue;
+                }
+
+                tmp.push(Some(b));
+            }
+
+            assert_eq!(tmp.len(), F::NUM_BITS as usize);
+
+            tmp
+        },
+        None => {
+            vec![None; F::NUM_BITS as usize]
+        }
+    };
+
+    let bits = values.into_iter().enumerate().map(|(i, b)| {
+        AllocatedBit::alloc(
+            cs.namespace(|| format!("bit {}", i)),
+            b
+        )
+    }).collect::<Result<Vec<_>, SynthesisError>>()?;
+
+    Ok(bits)
+}
+
 /// This is a boolean value which may be either a constant or
 /// an interpretation of an `AllocatedBit`.
 #[derive(Clone)]
@@ -509,7 +580,12 @@ mod test {
     use pairing::bls12_381::{Bls12, Fr};
     use pairing::{Field, PrimeField, PrimeFieldRepr, BitIterator};
     use ::circuit::test::*;
-    use super::{AllocatedBit, Boolean};
+    use super::{
+        AllocatedBit,
+        Boolean,
+        field_into_allocated_bits_be,
+        u64_into_allocated_bits_be
+    };
 
     #[test]
     fn test_allocated_bit() {
@@ -1129,4 +1205,47 @@ mod test {
             }
         }
     }
+
+    #[test]
+    fn test_u64_into_allocated_bits_be() {
+        let mut cs = TestConstraintSystem::<Bls12>::new();
+
+        let bits = u64_into_allocated_bits_be(&mut cs, Some(17234652694787248421)).unwrap();
+
+        assert!(cs.is_satisfied());
+
+        assert_eq!(bits.len(), 64);
+
+        assert_eq!(bits[0].value.unwrap(), true);
+        assert_eq!(bits[1].value.unwrap(), true);
+        assert_eq!(bits[2].value.unwrap(), true);
+        assert_eq!(bits[3].value.unwrap(), false);
+        assert_eq!(bits[4].value.unwrap(), true);
+        assert_eq!(bits[5].value.unwrap(), true);
+        assert_eq!(bits[20].value.unwrap(), true);
+        assert_eq!(bits[21].value.unwrap(), false);
+        assert_eq!(bits[22].value.unwrap(), false);
+    }
+
+    #[test]
+    fn test_field_into_allocated_bits_be() {
+        let mut cs = TestConstraintSystem::<Bls12>::new();
+
+        let r = Fr::from_str("9147677615426976802526883532204139322118074541891858454835346926874644257775").unwrap();
+
+        let bits = field_into_allocated_bits_be(&mut cs, Some(r)).unwrap();
+
+        assert!(cs.is_satisfied());
+
+        assert_eq!(bits.len(), 255);
+
+        assert_eq!(bits[0].value.unwrap(), false);
+        assert_eq!(bits[1].value.unwrap(), false);
+        assert_eq!(bits[2].value.unwrap(), true);
+        assert_eq!(bits[3].value.unwrap(), false);
+        assert_eq!(bits[4].value.unwrap(), true);
+        assert_eq!(bits[5].value.unwrap(), false);
+        assert_eq!(bits[20].value.unwrap(), true);
+        assert_eq!(bits[23].value.unwrap(), true);
+    }
 }

src/circuit/ecc.rs

@@ -12,9 +12,13 @@ use super::{
     Assignment
 };
 
-use super::num::AllocatedNum;
+use super::num::{
+    AllocatedNum,
+    Num
+};
 
 use ::jubjub::{
+    edwards,
     JubjubEngine,
     JubjubParams,
     FixedGenerators
@@ -41,8 +45,7 @@ impl<E: Engine> Clone for EdwardsPoint<E> {
 }
 
 /// Perform a fixed-base scalar multiplication with
-/// `by` being in little-endian bit order. `by` must
+/// `by` being in little-endian bit order.
-/// be a multiple of 3.
 pub fn fixed_base_multiplication<E, CS>(
     mut cs: CS,
     base: FixedGenerators,
@@ -52,11 +55,6 @@ pub fn fixed_base_multiplication<E, CS>(
     where CS: ConstraintSystem<E>,
           E: JubjubEngine
 {
-    // We're going to chunk the scalar into 3-bit windows,
-    // so let's force the caller to supply the right number
-    // of bits for our lookups.
-    assert!(by.len() % 3 == 0);
-
     // Represents the result of the multiplication
     let mut result = None;
 
@@ -64,9 +62,13 @@ pub fn fixed_base_multiplication<E, CS>(
                                   .zip(params.circuit_generators(base).iter())
                                   .enumerate()
     {
+        let chunk_a = chunk.get(0).map(|e| e.clone()).unwrap_or(Boolean::constant(false));
+        let chunk_b = chunk.get(1).map(|e| e.clone()).unwrap_or(Boolean::constant(false));
+        let chunk_c = chunk.get(2).map(|e| e.clone()).unwrap_or(Boolean::constant(false));
+
         let (x, y) = lookup3_xy(
             cs.namespace(|| format!("window table lookup {}", i)),
-            chunk,
+            &[chunk_a, chunk_b, chunk_c],
             window
         )?;
 
@@ -90,6 +92,41 @@ pub fn fixed_base_multiplication<E, CS>(
 }
 
 impl<E: JubjubEngine> EdwardsPoint<E> {
+    /// This 'witnesses' a point inside the constraint system.
+    /// It guarantees the point is on the curve.
+    pub fn witness<Order, CS>(
+        mut cs: CS,
+        p: Option<edwards::Point<E, Order>>,
+        params: &E::Params
+    ) -> Result<Self, SynthesisError>
+        where CS: ConstraintSystem<E>
+    {
+        let p = p.map(|p| p.into_xy());
+
+        // Allocate x
+        let x = AllocatedNum::alloc(
+            cs.namespace(|| "x"),
+            || {
+                Ok(p.get()?.0)
+            }
+        )?;
+
+        // Allocate y
+        let y = AllocatedNum::alloc(
+            cs.namespace(|| "y"),
+            || {
+                Ok(p.get()?.1)
+            }
+        )?;
+
+        Self::interpret(
+            cs.namespace(|| "point interpretation"),
+            &x,
+            &y,
+            params
+        )
+    }
+
     /// This extracts the x-coordinate, which is an injective
     /// encoding for elements of the prime order subgroup.
     pub fn into_num(&self) -> AllocatedNum<E> {
@@ -238,12 +275,116 @@ impl<E: JubjubEngine> EdwardsPoint<E> {
 
     pub fn double<CS>(
         &self,
-        cs: CS,
+        mut cs: CS,
         params: &E::Params
     ) -> Result<Self, SynthesisError>
         where CS: ConstraintSystem<E>
     {
-        self.add(cs, self, params)
+        // Compute T = (x1 + y1) * (x1 + y1)
+        let t = AllocatedNum::alloc(cs.namespace(|| "T"), || {
+            let mut t0 = *self.x.get_value().get()?;
+            t0.add_assign(self.y.get_value().get()?);
+
+            let mut t1 = *self.x.get_value().get()?;
+            t1.add_assign(self.y.get_value().get()?);
+
+            t0.mul_assign(&t1);
+
+            Ok(t0)
+        })?;
+
+        cs.enforce(
+            || "T computation",
+            |lc| lc + self.x.get_variable()
+                    + self.y.get_variable(),
+            |lc| lc + self.x.get_variable()
+                    + self.y.get_variable(),
+            |lc| lc + t.get_variable()
+        );
+
+        // Compute A = x1 * y1
+        let a = self.x.mul(cs.namespace(|| "A computation"), &self.y)?;
+
+        // Compute C = d*A*A
+        let c = AllocatedNum::alloc(cs.namespace(|| "C"), || {
+            let mut t0 = *a.get_value().get()?;
+            t0.square();
+            t0.mul_assign(params.edwards_d());
+
+            Ok(t0)
+        })?;
+
+        cs.enforce(
+            || "C computation",
+            |lc| lc + (*params.edwards_d(), a.get_variable()),
+            |lc| lc + a.get_variable(),
+            |lc| lc + c.get_variable()
+        );
+
+        // Compute x3 = (2.A) / (1 + C)
+        let x3 = AllocatedNum::alloc(cs.namespace(|| "x3"), || {
+            let mut t0 = *a.get_value().get()?;
+            t0.double();
+
+            let mut t1 = E::Fr::one();
+            t1.add_assign(c.get_value().get()?);
+
+            match t1.inverse() {
+                Some(t1) => {
+                    t0.mul_assign(&t1);
+
+                    Ok(t0)
+                },
+                None => {
+                    Err(SynthesisError::DivisionByZero)
+                }
+            }
+        })?;
+
+        let one = CS::one();
+        cs.enforce(
+            || "x3 computation",
+            |lc| lc + one + c.get_variable(),
+            |lc| lc + x3.get_variable(),
+            |lc| lc + a.get_variable()
+                    + a.get_variable()
+        );
+
+        // Compute y3 = (U - 2.A) / (1 - C)
+        let y3 = AllocatedNum::alloc(cs.namespace(|| "y3"), || {
+            let mut t0 = *a.get_value().get()?;
+            t0.double();
+            t0.negate();
+            t0.add_assign(t.get_value().get()?);
+
+            let mut t1 = E::Fr::one();
+            t1.sub_assign(c.get_value().get()?);
+
+            match t1.inverse() {
+                Some(t1) => {
+                    t0.mul_assign(&t1);
+
+                    Ok(t0)
+                },
+                None => {
+                    Err(SynthesisError::DivisionByZero)
+                }
+            }
+        })?;
+
+        cs.enforce(
+            || "y3 computation",
+            |lc| lc + one - c.get_variable(),
+            |lc| lc + y3.get_variable(),
+            |lc| lc + t.get_variable()
+                    - a.get_variable()
+                    - a.get_variable()
+        );
+
+        Ok(EdwardsPoint {
+            x: x3,
+            y: y3
+        })
     }
 
     /// Perform addition between any two points
@@ -366,8 +507,8 @@ impl<E: JubjubEngine> EdwardsPoint<E> {
 }
 
 pub struct MontgomeryPoint<E: Engine> {
-    x: AllocatedNum<E>,
+    x: Num<E>,
-    y: AllocatedNum<E>
+    y: Num<E>
 }
 
 impl<E: JubjubEngine> MontgomeryPoint<E> {
@@ -400,9 +541,9 @@ impl<E: JubjubEngine> MontgomeryPoint<E> {
 
         cs.enforce(
             || "u computation",
-            |lc| lc + self.y.get_variable(),
+            |lc| lc + &self.y.lc(E::Fr::one()),
             |lc| lc + u.get_variable(),
-            |lc| lc + (*params.scale(), self.x.get_variable())
+            |lc| lc + &self.x.lc(*params.scale())
         );
 
         // Compute v = (x - 1) / (x + 1)
@@ -427,10 +568,10 @@ impl<E: JubjubEngine> MontgomeryPoint<E> {
         let one = CS::one();
         cs.enforce(
             || "v computation",
-            |lc| lc + self.x.get_variable()
+            |lc| lc + &self.x.lc(E::Fr::one())
                     + one,
             |lc| lc + v.get_variable(),
-            |lc| lc + self.x.get_variable()
+            |lc| lc + &self.x.lc(E::Fr::one())
                     - one,
         );
 
@@ -445,8 +586,8 @@ impl<E: JubjubEngine> MontgomeryPoint<E> {
     /// on the curve. Useful for constants and
     /// window table lookups.
     pub fn interpret_unchecked(
-        x: AllocatedNum<E>,
+        x: Num<E>,
-        y: AllocatedNum<E>
+        y: Num<E>
     ) -> Self
     {
         MontgomeryPoint {
@@ -486,13 +627,13 @@ impl<E: JubjubEngine> MontgomeryPoint<E> {
 
         cs.enforce(
             || "evaluate lambda",
-            |lc| lc + other.x.get_variable()
+            |lc| lc + &other.x.lc(E::Fr::one())
-                    - self.x.get_variable(),
+                    - &self.x.lc(E::Fr::one()),
 
             |lc| lc + lambda.get_variable(),
 
-            |lc| lc + other.y.get_variable()
+            |lc| lc + &other.y.lc(E::Fr::one())
-                    - self.y.get_variable()
+                    - &self.y.lc(E::Fr::one())
         );
 
         // Compute x'' = lambda^2 - A - x - x'
@@ -513,8 +654,8 @@ impl<E: JubjubEngine> MontgomeryPoint<E> {
             |lc| lc + lambda.get_variable(),
             |lc| lc + lambda.get_variable(),
             |lc| lc + (*params.montgomery_a(), one)
-                    + self.x.get_variable()
+                    + &self.x.lc(E::Fr::one())
-                    + other.x.get_variable()
+                    + &other.x.lc(E::Fr::one())
                     + xprime.get_variable()
         );
 
@@ -532,121 +673,18 @@ impl<E: JubjubEngine> MontgomeryPoint<E> {
         // y' + y = lambda(x - x')
         cs.enforce(
             || "evaluate yprime",
-            |lc| lc + self.x.get_variable()
+            |lc| lc + &self.x.lc(E::Fr::one())
                     - xprime.get_variable(),
 
             |lc| lc + lambda.get_variable(),
 
             |lc| lc + yprime.get_variable()
-                    + self.y.get_variable()
+                    + &self.y.lc(E::Fr::one())
         );
 
         Ok(MontgomeryPoint {
-            x: xprime,
+            x: xprime.into(),
-            y: yprime
+            y: yprime.into()
-        })
-    }
-
-    /// Performs an affine point doubling, not defined for
-    /// the point of order two (0, 0).
-    pub fn double<CS>(
-        &self,
-        mut cs: CS,
-        params: &E::Params
-    ) -> Result<Self, SynthesisError>
-        where CS: ConstraintSystem<E>
-    {
-        // Square x
-        let xx = self.x.square(&mut cs)?;
-
-        // Compute lambda = (3.xx + 2.A.x + 1) / 2.y
-        let lambda = AllocatedNum::alloc(cs.namespace(|| "lambda"), || {
-            let mut t0 = *xx.get_value().get()?;
-            let mut t1 = t0;
-            t0.double(); // t0 = 2.xx
-            t0.add_assign(&t1); // t0 = 3.xx
-            t1 = *self.x.get_value().get()?; // t1 = x
-            t1.mul_assign(params.montgomery_2a()); // t1 = 2.A.x
-            t0.add_assign(&t1);
-            t0.add_assign(&E::Fr::one());
-            t1 = *self.y.get_value().get()?; // t1 = y
-            t1.double(); // t1 = 2.y
-            match t1.inverse() {
-                Some(t1) => {
-                    t0.mul_assign(&t1);
-
-                    Ok(t0)
-                },
-                None => {
-                    Err(SynthesisError::DivisionByZero)
-                }
-            }
-        })?;
-
-        // (2.y) * (lambda) = (3.xx + 2.A.x + 1)
-        let one = CS::one();
-        cs.enforce(
-            || "evaluate lambda",
-            |lc| lc + self.y.get_variable()
-                    + self.y.get_variable(),
-
-            |lc| lc + lambda.get_variable(),
-
-            |lc| lc + xx.get_variable()
-                    + xx.get_variable()
-                    + xx.get_variable()
-                    + (*params.montgomery_2a(), self.x.get_variable())
-                    + one
-        );
-
-        // Compute x' = (lambda^2) - A - 2.x
-        let xprime = AllocatedNum::alloc(cs.namespace(|| "xprime"), || {
-            let mut t0 = *lambda.get_value().get()?;
-            t0.square();
-            t0.sub_assign(params.montgomery_a());
-            t0.sub_assign(self.x.get_value().get()?);
-            t0.sub_assign(self.x.get_value().get()?);
-
-            Ok(t0)
-        })?;
-
-        // (lambda) * (lambda) = (A + 2.x + x')
-        cs.enforce(
-            || "evaluate xprime",
-            |lc| lc + lambda.get_variable(),
-            |lc| lc + lambda.get_variable(),
-            |lc| lc + (*params.montgomery_a(), one)
-                    + self.x.get_variable()
-                    + self.x.get_variable()
-                    + xprime.get_variable()
-        );
-
-        // Compute y' = -(y + lambda(x' - x))
-        let yprime = AllocatedNum::alloc(cs.namespace(|| "yprime"), || {
-            let mut t0 = *xprime.get_value().get()?;
-            t0.sub_assign(self.x.get_value().get()?);
-            t0.mul_assign(lambda.get_value().get()?);
-            t0.add_assign(self.y.get_value().get()?);
-            t0.negate();
-
-            Ok(t0)
-        })?;
-
-        // y' + y = lambda(x - x')
-        cs.enforce(
-            || "evaluate yprime",
-            |lc| lc + self.x.get_variable()
-                    - xprime.get_variable(),
-
-            |lc| lc + lambda.get_variable(),
-
-            |lc| lc + yprime.get_variable()
-                    + self.y.get_variable()
-        );
-
-        Ok(MontgomeryPoint {
-            x: xprime,
-            y: yprime
         })
     }
 }
@@ -696,7 +734,7 @@ mod test {
                 Ok(y)
             }).unwrap();
 
-            let p = MontgomeryPoint::interpret_unchecked(numx, numy);
+            let p = MontgomeryPoint::interpret_unchecked(numx.into(), numy.into());
 
             let q = p.into_edwards(&mut cs, params).unwrap();
 
@@ -721,6 +759,23 @@ mod test {
         let params = &JubjubBls12::new();
         let rng = &mut XorShiftRng::from_seed([0x5dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
 
+        for _ in 0..100 {
+            let p = edwards::Point::<Bls12, _>::rand(rng, &params);
+
+            let mut cs = TestConstraintSystem::<Bls12>::new();
+            let q = EdwardsPoint::witness(
+                &mut cs,
+                Some(p.clone()),
+                &params
+            ).unwrap();
+
+            let p = p.into_xy();
+
+            assert!(cs.is_satisfied());
+            assert_eq!(q.x.get_value().unwrap(), p.0);
+            assert_eq!(q.y.get_value().unwrap(), p.1);
+        }
+
         for _ in 0..100 {
             let p = edwards::Point::<Bls12, _>::rand(rng, &params);
             let (x, y) = p.into_xy();
@@ -759,27 +814,6 @@ mod test {
         }
     }
 
-    #[test]
-    fn test_doubling_order_2() {
-        let params = &JubjubBls12::new();
-
-        let mut cs = TestConstraintSystem::<Bls12>::new();
-
-        let x = AllocatedNum::alloc(cs.namespace(|| "x"), || {
-            Ok(Fr::zero())
-        }).unwrap();
-        let y = AllocatedNum::alloc(cs.namespace(|| "y"), || {
-            Ok(Fr::zero())
-        }).unwrap();
-
-        let p = MontgomeryPoint {
-            x: x,
-            y: y
-        };
-
-        assert!(p.double(&mut cs, params).is_err());
-    }
-
     #[test]
     fn test_edwards_fixed_base_multiplication()  {
         let params = &JubjubBls12::new();
@@ -788,7 +822,7 @@ mod test {
         for _ in 0..100 {
             let mut cs = TestConstraintSystem::<Bls12>::new();
 
-            let p = params.generator(FixedGenerators::NoteCommitmentRandomization);
+            let p = params.generator(FixedGenerators::NoteCommitmentRandomness);
             let s = Fs::rand(rng);
             let q = p.mul(s, params);
             let (x1, y1) = q.into_xy();
@@ -805,7 +839,7 @@ mod test {
 
             let q = fixed_base_multiplication(
                 cs.namespace(|| "multiplication"),
-                FixedGenerators::NoteCommitmentRandomization,
+                FixedGenerators::NoteCommitmentRandomness,
                 &s_bits,
                 params
             ).unwrap();
@@ -1088,13 +1122,13 @@ mod test {
             }).unwrap();
 
             let p1 = MontgomeryPoint {
-                x: num_x0,
+                x: num_x0.into(),
-                y: num_y0
+                y: num_y0.into()
             };
 
             let p2 = MontgomeryPoint {
-                x: num_x1,
+                x: num_x1.into(),
-                y: num_y1
+                y: num_y1.into()
             };
 
             let p3 = p1.add(cs.namespace(|| "addition"), &p2, params).unwrap();
@@ -1118,60 +1152,4 @@ mod test {
             assert_eq!(cs.which_is_unsatisfied(), Some("addition/evaluate lambda"));
         }
     }
-
-    #[test]
-    fn test_montgomery_doubling() {
-        let params = &JubjubBls12::new();
-        let rng = &mut XorShiftRng::from_seed([0x5dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
-
-        for _ in 0..100 {
-            let p = loop {
-                let x: Fr = rng.gen();
-                let s: bool = rng.gen();
-
-                if let Some(p) = montgomery::Point::<Bls12, _>::get_for_x(x, s, params) {
-                    break p;
-                }
-            };
-
-            let p2 = p.double(params);
-
-            let (x0, y0) = p.into_xy().unwrap();
-            let (x1, y1) = p2.into_xy().unwrap();
-
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let x = AllocatedNum::alloc(cs.namespace(|| "x"), || {
-                Ok(x0)
-            }).unwrap();
-            let y = AllocatedNum::alloc(cs.namespace(|| "y"), || {
-                Ok(y0)
-            }).unwrap();
-
-            let p = MontgomeryPoint {
-                x: x,
-                y: y
-            };
-
-            let p2 = p.double(cs.namespace(|| "doubling"), params).unwrap();
-
-            assert!(cs.is_satisfied());
-
-            assert!(p2.x.get_value().unwrap() == x1);
-            assert!(p2.y.get_value().unwrap() == y1);
-
-            cs.set("doubling/yprime/num", rng.gen());
-            assert_eq!(cs.which_is_unsatisfied(), Some("doubling/evaluate yprime"));
-            cs.set("doubling/yprime/num", y1);
-            assert!(cs.is_satisfied());
-
-            cs.set("doubling/xprime/num", rng.gen());
-            assert_eq!(cs.which_is_unsatisfied(), Some("doubling/evaluate xprime"));
-            cs.set("doubling/xprime/num", x1);
-            assert!(cs.is_satisfied());
-
-            cs.set("doubling/lambda/num", rng.gen());
-            assert_eq!(cs.which_is_unsatisfied(), Some("doubling/evaluate lambda"));
-        }
-    }
 }

src/circuit/lookup.rs

@@ -1,6 +1,9 @@
 use pairing::{Engine, Field};
 use super::*;
-use super::num::AllocatedNum;
+use super::num::{
+    AllocatedNum,
+    Num
+};
 use super::boolean::Boolean;
 use bellman::{
     ConstraintSystem
@@ -123,7 +126,7 @@ pub fn lookup3_xy_with_conditional_negation<E: Engine, CS>(
     mut cs: CS,
     bits: &[Boolean],
     coords: &[(E::Fr, E::Fr)]
-) -> Result<(AllocatedNum<E>, AllocatedNum<E>), SynthesisError>
+) -> Result<(Num<E>, Num<E>), SynthesisError>
     where CS: ConstraintSystem<E>
 {
     assert_eq!(bits.len(), 3);
@@ -145,19 +148,16 @@ pub fn lookup3_xy_with_conditional_negation<E: Engine, CS>(
         _ => None
     };
 
-    // Allocate the x-coordinate resulting from the lookup
-    let res_x = AllocatedNum::alloc(
-        cs.namespace(|| "x"),
-        || {
-            Ok(coords[*i.get()?].0)
-        }
-    )?;
-
     // Allocate the y-coordinate resulting from the lookup
-    let res_y = AllocatedNum::alloc(
+    // and conditional negation
+    let y = AllocatedNum::alloc(
         cs.namespace(|| "y"),
         || {
-            Ok(coords[*i.get()?].1)
+            let mut tmp = coords[*i.get()?].1;
+            if *bits[2].get_value().get()? {
+                tmp.negate();
+            }
+            Ok(tmp)
         }
     )?;
 
@@ -169,29 +169,27 @@ pub fn lookup3_xy_with_conditional_negation<E: Engine, CS>(
     synth::<E, _>(2, coords.iter().map(|c| &c.0), &mut x_coeffs);
     synth::<E, _>(2, coords.iter().map(|c| &c.1), &mut y_coeffs);
 
-    cs.enforce(
+    let precomp = Boolean::and(cs.namespace(|| "precomp"), &bits[0], &bits[1])?;
-        || "x-coordinate lookup",
+
-        |lc| lc + (x_coeffs[0b01], one)
+    let x = Num::zero()
-                + &bits[1].lc::<E>(one, x_coeffs[0b11]),
+            .add_bool_with_coeff(one, &Boolean::constant(true), x_coeffs[0b00])
-        |lc| lc + &bits[0].lc::<E>(one, E::Fr::one()),
+            .add_bool_with_coeff(one, &bits[0], x_coeffs[0b01])
-        |lc| lc + res_x.get_variable()
+            .add_bool_with_coeff(one, &bits[1], x_coeffs[0b10])
-                - (x_coeffs[0b00], one)
+            .add_bool_with_coeff(one, &precomp, x_coeffs[0b11]);
-                - &bits[1].lc::<E>(one, x_coeffs[0b10])
+
-    );
+    let y_lc = precomp.lc::<E>(one, y_coeffs[0b11]) +
+               &bits[1].lc::<E>(one, y_coeffs[0b10]) +
+               &bits[0].lc::<E>(one, y_coeffs[0b01]) +
+               (y_coeffs[0b00], one);
 
     cs.enforce(
         || "y-coordinate lookup",
-        |lc| lc + (y_coeffs[0b01], one)
+        |lc| lc + &y_lc + &y_lc,
-                + &bits[1].lc::<E>(one, y_coeffs[0b11]),
+        |lc| lc + &bits[2].lc::<E>(one, E::Fr::one()),
-        |lc| lc + &bits[0].lc::<E>(one, E::Fr::one()),
+        |lc| lc + &y_lc - y.get_variable()
-        |lc| lc + res_y.get_variable()
-                - (y_coeffs[0b00], one)
-                - &bits[1].lc::<E>(one, y_coeffs[0b10])
     );
 
-    let final_y = res_y.conditionally_negate(&mut cs, &bits[2])?;
+    Ok((x, y.into()))
-
-    Ok((res_x, final_y))
 }
 
 #[cfg(test)]

src/circuit/mod.rs

@@ -6,7 +6,7 @@ pub mod uint32;
 pub mod blake2s;
 pub mod num;
 pub mod lookup;
-pub mod mont;
+pub mod ecc;
 pub mod pedersen_hash;
 
 use bellman::SynthesisError;

src/circuit/num.rs

@@ -2,7 +2,6 @@ use pairing::{
     Engine,
     Field,
     PrimeField,
-    BitIterator
 };
 
 use bellman::{
@@ -17,8 +16,8 @@ use super::{
 };
 
 use super::boolean::{
-   Boolean,
+    self,
-   AllocatedBit
+    Boolean,
 };
 
 pub struct AllocatedNum<E: Engine> {
@@ -76,39 +75,10 @@ impl<E: Engine> AllocatedNum<E> {
     ) -> Result<Vec<Boolean>, SynthesisError>
         where CS: ConstraintSystem<E>
     {
-        let bit_values = match self.value {
+        let bits = boolean::field_into_allocated_bits_be(
-            Some(value) => {
+            &mut cs,
-                let mut field_char = BitIterator::new(E::Fr::char());
+            self.value
-
+        )?;
-                let mut tmp = Vec::with_capacity(E::Fr::NUM_BITS as usize);
-
-                let mut found_one = false;
-                for b in BitIterator::new(value.into_repr()) {
-                    // Skip leading bits
-                    found_one |= field_char.next().unwrap();
-                    if !found_one {
-                        continue;
-                    }
-
-                    tmp.push(Some(b));
-                }
-
-                assert_eq!(tmp.len(), E::Fr::NUM_BITS as usize);
-
-                tmp
-            },
-            None => {
-                vec![None; E::Fr::NUM_BITS as usize]
-            }
-        };
-
-        let mut bits = vec![];
-        for (i, b) in bit_values.into_iter().enumerate() {
-            bits.push(AllocatedBit::alloc(
-                cs.namespace(|| format!("bit {}", i)),
-                b
-            )?);
-        }
 
         let mut lc = LinearCombination::zero();
         let mut coeff = E::Fr::one();
@@ -342,38 +312,6 @@ impl<E: Engine> AllocatedNum<E> {
         Ok((c, d))
     }
 
-    pub fn conditionally_negate<CS>(
-        &self,
-        mut cs: CS,
-        condition: &Boolean
-    ) -> Result<Self, SynthesisError>
-        where CS: ConstraintSystem<E>
-    {
-        let r = Self::alloc(
-            cs.namespace(|| "conditional negation result"),
-            || {
-                let mut tmp = *self.value.get()?;
-                if *condition.get_value().get()? {
-                    tmp.negate();
-                }
-                Ok(tmp)
-            }
-        )?;
-
-        // (1-c)(x) + (c)(-x) = r
-        // x - 2cx = r
-        // (2x) * (c) = x - r
-
-        cs.enforce(
-            || "conditional negation",
-            |lc| lc + self.variable + self.variable,
-            |_| condition.lc(CS::one(), E::Fr::one()),
-            |lc| lc + self.variable - r.variable
-        );
-
-        Ok(r)
-    }
-
     pub fn get_value(&self) -> Option<E::Fr> {
         self.value
     }
@@ -383,6 +321,61 @@ impl<E: Engine> AllocatedNum<E> {
     }
 }
 
+pub struct Num<E: Engine> {
+    value: Option<E::Fr>,
+    lc: LinearCombination<E>
+}
+
+impl<E: Engine> From<AllocatedNum<E>> for Num<E> {
+    fn from(num: AllocatedNum<E>) -> Num<E> {
+        Num {
+            value: num.value,
+            lc: LinearCombination::<E>::zero() + num.variable
+        }
+    }
+}
+
+impl<E: Engine> Num<E> {
+    pub fn zero() -> Self {
+        Num {
+            value: Some(E::Fr::zero()),
+            lc: LinearCombination::zero()
+        }
+    }
+
+    pub fn get_value(&self) -> Option<E::Fr> {
+        self.value
+    }
+
+    pub fn lc(&self, coeff: E::Fr) -> LinearCombination<E> {
+        LinearCombination::zero() + (coeff, &self.lc)
+    }
+
+    pub fn add_bool_with_coeff(
+        self,
+        one: Variable,
+        bit: &Boolean,
+        coeff: E::Fr
+    ) -> Self
+    {
+        let newval = match (self.value, bit.get_value()) {
+            (Some(mut curval), Some(mut bval)) => {
+                if bval {
+                    curval.add_assign(&coeff);
+                }
+
+                Some(curval)
+            },
+            _ => None
+        };
+
+        Num {
+            value: newval,
+            lc: self.lc + &bit.lc(one, coeff)
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use rand::{SeedableRng, Rand, Rng, XorShiftRng};
@@ -463,107 +456,6 @@ mod test {
         }
     }
 
-    #[test]
-    fn test_num_conditional_negation() {
-        {
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let n = AllocatedNum::alloc(cs.namespace(|| "a"), || Ok(Fr::one())).unwrap();
-            let b = Boolean::constant(true);
-            let n2 = n.conditionally_negate(&mut cs, &b).unwrap();
-
-            let mut negone = Fr::one();
-            negone.negate();
-
-            assert!(cs.is_satisfied());
-            assert!(cs.get("conditional negation result/num") == negone);
-            assert!(n2.value.unwrap() == negone);
-            cs.set("conditional negation result/num", Fr::from_str("1").unwrap());
-            assert!(!cs.is_satisfied());
-        }
-        {
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let n = AllocatedNum::alloc(cs.namespace(|| "a"), || Ok(Fr::one())).unwrap();
-            let b = Boolean::constant(false);
-            let n2 = n.conditionally_negate(&mut cs, &b).unwrap();
-
-            assert!(cs.is_satisfied());
-            assert!(cs.get("conditional negation result/num") == Fr::one());
-            assert!(n2.value.unwrap() == Fr::one());
-            cs.set("conditional negation result/num", Fr::from_str("2").unwrap());
-            assert!(!cs.is_satisfied());
-        }
-
-        {
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let n = AllocatedNum::alloc(cs.namespace(|| "a"), || Ok(Fr::one())).unwrap();
-            let b = Boolean::from(
-                AllocatedBit::alloc(cs.namespace(|| "condition"), Some(true)).unwrap()
-            );
-            let n2 = n.conditionally_negate(&mut cs, &b).unwrap();
-
-            let mut negone = Fr::one();
-            negone.negate();
-
-            assert!(cs.is_satisfied());
-            assert!(cs.get("conditional negation result/num") == negone);
-            assert!(n2.value.unwrap() == negone);
-            cs.set("conditional negation result/num", Fr::from_str("1").unwrap());
-            assert!(!cs.is_satisfied());
-        }
-        {
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let n = AllocatedNum::alloc(cs.namespace(|| "a"), || Ok(Fr::one())).unwrap();
-            let b = Boolean::from(
-                AllocatedBit::alloc(cs.namespace(|| "condition"), Some(false)).unwrap()
-            );
-            let n2 = n.conditionally_negate(&mut cs, &b).unwrap();
-
-            assert!(cs.is_satisfied());
-            assert!(cs.get("conditional negation result/num") == Fr::one());
-            assert!(n2.value.unwrap() == Fr::one());
-            cs.set("conditional negation result/num", Fr::from_str("2").unwrap());
-            assert!(!cs.is_satisfied());
-        }
-
-        {
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let n = AllocatedNum::alloc(cs.namespace(|| "a"), || Ok(Fr::one())).unwrap();
-            let b = Boolean::from(
-                AllocatedBit::alloc(cs.namespace(|| "condition"), Some(false)).unwrap()
-            ).not();
-            let n2 = n.conditionally_negate(&mut cs, &b).unwrap();
-
-            let mut negone = Fr::one();
-            negone.negate();
-
-            assert!(cs.is_satisfied());
-            assert!(cs.get("conditional negation result/num") == negone);
-            assert!(n2.value.unwrap() == negone);
-            cs.set("conditional negation result/num", Fr::from_str("1").unwrap());
-            assert!(!cs.is_satisfied());
-        }
-        {
-            let mut cs = TestConstraintSystem::<Bls12>::new();
-
-            let n = AllocatedNum::alloc(cs.namespace(|| "a"), || Ok(Fr::one())).unwrap();
-            let b = Boolean::from(
-                AllocatedBit::alloc(cs.namespace(|| "condition"), Some(true)).unwrap()
-            ).not();
-            let n2 = n.conditionally_negate(&mut cs, &b).unwrap();
-
-            assert!(cs.is_satisfied());
-            assert!(cs.get("conditional negation result/num") == Fr::one());
-            assert!(n2.value.unwrap() == Fr::one());
-            cs.set("conditional negation result/num", Fr::from_str("2").unwrap());
-            assert!(!cs.is_satisfied());
-        }
-    }
-
     #[test]
     fn test_num_nonzero() {
         {

src/circuit/pedersen_hash.rs

@@ -1,5 +1,5 @@
 use super::*;
-use super::mont::{
+use super::ecc::{
     MontgomeryPoint,
     EdwardsPoint
 };
@@ -10,18 +10,43 @@ use bellman::{
 };
 use super::lookup::*;
 
+// TODO: ensure these match the spec
+pub enum Personalization {
+    NoteCommitment,
+    AnotherPersonalization
+}
+
+impl Personalization {
+    fn get_constant_bools(&self) -> Vec<Boolean> {
+        self.get_bits()
+        .into_iter()
+        .map(|e| Boolean::constant(e))
+        .collect()
+    }
+
+    pub fn get_bits(&self) -> Vec<bool> {
+        match *self {
+            Personalization::NoteCommitment =>
+                vec![false, false, false, false, false, false],
+            Personalization::AnotherPersonalization =>
+                vec![false, false, false, false, false, true],
+        }
+    }
+}
+
 pub fn pedersen_hash<E: JubjubEngine, CS>(
     mut cs: CS,
+    personalization: Personalization,
     bits: &[Boolean],
     params: &E::Params
 ) -> Result<EdwardsPoint<E>, SynthesisError>
     where CS: ConstraintSystem<E>
 {
-    // Unnecessary if forced personalization is introduced
+    let personalization = personalization.get_constant_bools();
-    assert!(bits.len() > 0);
+    assert_eq!(personalization.len(), 6);
 
     let mut edwards_result = None;
-    let mut bits = bits.iter();
+    let mut bits = personalization.iter().chain(bits.iter());
     let mut segment_generators = params.pedersen_circuit_generators().iter();
     let boolean_false = Boolean::constant(false);
 
@@ -124,12 +149,13 @@ mod test {
 
         pedersen_hash(
             cs.namespace(|| "pedersen hash"),
+            Personalization::NoteCommitment,
             &input_bools,
             params
         ).unwrap();
 
         assert!(cs.is_satisfied());
-        assert_eq!(cs.num_constraints(), 1539);
+        assert_eq!(cs.num_constraints(), 1377);
     }
 
     #[test]
@@ -151,6 +177,7 @@ mod test {
 
                 let res = pedersen_hash(
                     cs.namespace(|| "pedersen hash"),
+                    Personalization::NoteCommitment,
                     &input_bools,
                     params
                 ).unwrap();
@@ -158,12 +185,23 @@ mod test {
                 assert!(cs.is_satisfied());
 
                 let expected = ::pedersen_hash::pedersen_hash::<Bls12, _>(
-                    input.into_iter(),
+                    Personalization::NoteCommitment,
+                    input.clone().into_iter(),
                     params
                 ).into_xy();
 
                 assert_eq!(res.x.get_value().unwrap(), expected.0);
                 assert_eq!(res.y.get_value().unwrap(), expected.1);
+
+                // Test against the output of a different personalization
+                let unexpected = ::pedersen_hash::pedersen_hash::<Bls12, _>(
+                    Personalization::AnotherPersonalization,
+                    input.into_iter(),
+                    params
+                ).into_xy();
+
+                assert!(res.x.get_value().unwrap() != unexpected.0);
+                assert!(res.y.get_value().unwrap() != unexpected.1);
             }
         }
     }

src/jubjub/mod.rs

@@ -67,8 +67,11 @@ impl JubjubEngine for Bls12 {
 /// exponent.
 #[derive(Copy, Clone)]
 pub enum FixedGenerators {
-    NoteCommitmentRandomization = 0,
+    NoteCommitmentRandomness = 0,
-    Max = 1
+    ProvingPublicKey = 1,
+    ValueCommitmentValue = 2,
+    ValueCommitmentRandomness = 3,
+    Max = 4
 }
 
 pub struct JubjubBls12 {

src/pedersen_hash.rs

@@ -1,14 +1,17 @@
 use jubjub::*;
 use pairing::*;
 
+use circuit::pedersen_hash::Personalization;
+
 pub fn pedersen_hash<E, I>(
+    personalization: Personalization,
     bits: I,
     params: &E::Params
 ) -> edwards::Point<E, PrimeOrder>
     where I: IntoIterator<Item=bool>,
           E: JubjubEngine
 {
-    let mut bits = bits.into_iter();
+    let mut bits = personalization.get_bits().into_iter().chain(bits.into_iter());
 
     let mut result = edwards::Point::zero();
     let mut generators = params.pedersen_hash_generators().iter();