Fix comments in jubjub code.

src/jubjub/edwards.rs

@@ -28,6 +28,9 @@ use std::io::{
 
 // Represents the affine point (X/Z, Y/Z) via the extended
 // twisted Edwards coordinates.
+//
+// See "Twisted Edwards Curves Revisited"
+//     Huseyin Hisil, Kenneth Koon-Ho Wong, Gary Carter, and Ed Dawson
 pub struct Point<E: JubjubEngine, Subgroup> {
     x: E::Fr,
     y: E::Fr,
@@ -120,7 +123,14 @@ impl<E: JubjubEngine> Point<E, Unknown> {
         params: &E::Params
     ) -> io::Result<Self>
     {
+        // Jubjub points are encoded least significant bit first.
+        // The most significant bit (bit 254) encodes the parity
+        // of the x-coordinate.
+
         let mut y_repr = <E::Fr as PrimeField>::Repr::default();
+
+        // This reads in big-endian, so we perform a swap of the
+        // limbs in the representation and swap the bit order.
         y_repr.read_be(reader)?;
 
         y_repr.as_mut().reverse();
@@ -393,11 +403,19 @@ impl<E: JubjubEngine, Subgroup> Point<E, Subgroup> {
     }
 
     pub fn double(&self, params: &E::Params) -> Self {
+        // Point addition is unified and complete.
+        // There are dedicated formulae, but we do
+        // not implement these now.
+        
         self.add(self, params)
     }
 
     pub fn add(&self, other: &Self, params: &E::Params) -> Self
     {
+        // See "Twisted Edwards Curves Revisited"
+        //     Huseyin Hisil, Kenneth Koon-Ho Wong, Gary Carter, and Ed Dawson
+        //     3.1 Unified Addition in E^e
+
         // A = x1 * x2
         let mut a = self.x;
         a.mul_assign(&other.x);
@@ -470,6 +488,8 @@ impl<E: JubjubEngine, Subgroup> Point<E, Subgroup> {
         params: &E::Params
     ) -> Self
     {
+        // Standard double-and-add scalar multiplication
+
         let mut res = Self::zero();
 
         for b in BitIterator::new(scalar.into()) {

src/jubjub/mod.rs

@@ -1,18 +1,21 @@
-//! Jubjub is an elliptic curve defined over the BLS12-381 scalar field, Fr.
+//! Jubjub is a twisted Edwards curve defined over the BLS12-381 scalar
-//! It is a Montgomery curve that takes the form `y^2 = x^3 + Ax^2 + x` where
+//! field, Fr. It takes the form `-x^2 + y^2 = 1 + dx^2y^2` with
-//! `A = 40962`. This is the smallest integer choice of A such that:
+//! `d = -(10240/10241)`. It is birationally equivalent to a Montgomery
+//! curve of the form `y^2 = x^3 + Ax^2 + x` with `A = 40962`. This
+//! value `A` is the smallest integer choice such that:
 //!
 //! * `(A - 2) / 4` is a small integer (`10240`).
 //! * `A^2 - 4` is quadratic residue.
-//! * The group order of the curve and its quadratic twist has a large prime factor.
+//! * The group order of the curve and its quadratic twist has a large
+//!   prime factor.
 //!
 //! Jubjub has `s = 0x0e7db4ea6533afa906673b0101343b00a6682093ccc81082d0970e5ed6f72cb7`
-//! as the prime subgroup order, with cofactor 8. (The twist has cofactor 4.)
+//! as the prime subgroup order, with cofactor 8. (The twist has
+//! cofactor 4.)
 //!
-//! This curve is birationally equivalent to a twisted Edwards curve of the
+//! It is a complete twisted Edwards curve, so the equivalence with
-//! form `-x^2 + y^2 = 1 + dx^2y^2` with `d = -(10240/10241)`. In fact, this equivalence
+//! the Montgomery curve forms a group isomorphism, allowing points
-//! forms a group isomorphism, so points can be freely converted between the Montgomery
+//! to be freely converted between the two forms.
-//! and twisted Edwards forms.
 
 use pairing::{
     Engine,
@@ -30,10 +33,17 @@ use pairing::bls12_381::{
 
 pub mod edwards;
 pub mod montgomery;
+pub mod fs;
 
 #[cfg(test)]
 pub mod tests;
 
+/// Point of unknown order.
+pub enum Unknown { }
+
+/// Point of prime order.
+pub enum PrimeOrder { }
+
 /// Fixed generators of the Jubjub curve of unknown
 /// exponent.
 #[derive(Copy, Clone)]
@@ -104,14 +114,6 @@ pub trait JubjubParams<E: JubjubEngine>: Sized {
     fn circuit_generators(&self, FixedGenerators) -> &[Vec<(E::Fr, E::Fr)>];
 }
 
-/// Point of unknown order.
-pub enum Unknown { }
-
-/// Point of prime order.
-pub enum PrimeOrder { }
-
-pub mod fs;
-
 impl JubjubEngine for Bls12 {
     type Fs = self::fs::Fs;
     type Params = JubjubBls12;

src/jubjub/montgomery.rs

@@ -20,8 +20,7 @@ use rand::{
 
 use std::marker::PhantomData;
 
-// Represents the affine point (X/Z, Y/Z) via the extended
+// Represents the affine point (X, Y)
-// twisted Edwards coordinates.
 pub struct Point<E: JubjubEngine, Subgroup> {
     x: E::Fr,
     y: E::Fr,
@@ -69,7 +68,7 @@ impl<E: JubjubEngine, Subgroup> PartialEq for Point<E, Subgroup> {
 impl<E: JubjubEngine> Point<E, Unknown> {
     pub fn get_for_x(x: E::Fr, sign: bool, params: &E::Params) -> Option<Self>
     {
-        // given an x on the curve, y^2 = x^3 + A*x^2 + x
+        // Given an x on the curve, y = sqrt(x^3 + A*x^2 + x)
 
         let mut x2 = x;
         x2.square();
@@ -230,10 +229,17 @@ impl<E: JubjubEngine, Subgroup> Point<E, Subgroup> {
             return Point::zero();
         }
 
+        // (0, 0) is the point of order 2. Doubling
+        // produces the point at infinity.
         if self.y == E::Fr::zero() {
             return Point::zero();
         }
 
+        // This is a standard affine point doubling formula
+        // See 4.3.2 The group law for Weierstrass curves
+        //     Montgomery curves and the Montgomery Ladder
+        //     Daniel J. Bernstein and Tanja Lange
+
         let mut delta = E::Fr::one();
         {
             let mut tmp = params.montgomery_a().clone();
@@ -276,6 +282,11 @@ impl<E: JubjubEngine, Subgroup> Point<E, Subgroup> {
 
     pub fn add(&self, other: &Self, params: &E::Params) -> Self
     {
+        // This is a standard affine point addition formula
+        // See 4.3.2 The group law for Weierstrass curves
+        //     Montgomery curves and the Montgomery Ladder
+        //     Daniel J. Bernstein and Tanja Lange
+
         match (self.infinity, other.infinity) {
             (true, true) => Point::zero(),
             (true, false) => other.clone(),
@@ -325,6 +336,8 @@ impl<E: JubjubEngine, Subgroup> Point<E, Subgroup> {
         params: &E::Params
     ) -> Self
     {
+        // Standard double-and-add scalar multiplication
+
         let mut res = Self::zero();
 
         for b in BitIterator::new(scalar.into()) {