Repository: dusk-network/bls12_381 (language: rust)

Issue:
Implement is_zero & is_one fn for `Scalar`
>I'm wondering, if we have `Scalar::one()` and `Scalar::zero` as shortcut, and we do a lot of >comparison with both (at least `Scalar::zero`), maybe we should add a utility method such as >`.is_zero` ? So we would have `if !u.is_zero() {` and `let y = if u.is_zero() {`.
>
>What do you think?

_Originally posted by @ZER0 in https://github.com/dusk-network/dusk-blindbid/pull/10_


Discussion / hints:
For my understanding we have to possible way to approach the API:

**1. A single `is_zero()` / `is_one()` method that returns a `Choice`**
_Pro_: Can be easily used together with the rest of `subtle`, such as `ConditionallySelectable` trait.
_Cons_: For `boolean` check `into()` must be used (e.g. `let y = if u.is_zero().into() {`)

**2. Implements methods for both `boolean` and `Choice`: `is_zero()` / `is_one()` and `ct_is_zero()` / `ct_is_one()`**
_Pro_: We can use `ct_is_zero()` when we need to deal with `subtle` and `ConditionallySelectable` and `is_zero` for `boolean` checks.
_Cons_: The API is verbose, two method that virtually are doing the same thing just to save a `.into()` inside an `if` (the `boolean` methods are a wrapper to the `Choice` methods, plus`into()`) and possibly confusing.





Fix patch:
diff --git a/src/g1.rs b/src/g1.rs
index 16280b4a..479e9b79 100644
--- a/src/g1.rs
+++ b/src/g1.rs
@@ -496,7 +496,8 @@ impl ConstantTimeEq for G1Projective {
         let other_is_zero = other.z.is_zero();
 
         (self_is_zero & other_is_zero) // Both point at infinity
-            | ((!self_is_zero) & (!other_is_zero) & x1.ct_eq(&x2) & y1.ct_eq(&y2)) // Neither point at infinity, coordinates are the same
+            | ((!self_is_zero) & (!other_is_zero) & x1.ct_eq(&x2) & y1.ct_eq(&y2))
+        // Neither point at infinity, coordinates are the same
     }
 }
 
diff --git a/src/g2.rs b/src/g2.rs
index ac20a655..93cb98ca 100644
--- a/src/g2.rs
+++ b/src/g2.rs
@@ -570,7 +570,8 @@ impl ConstantTimeEq for G2Projective {
         let other_is_zero = other.z.is_zero();
 
         (self_is_zero & other_is_zero) // Both point at infinity
-            | ((!self_is_zero) & (!other_is_zero) & x1.ct_eq(&x2) & y1.ct_eq(&y2)) // Neither point at infinity, coordinates are the same
+            | ((!self_is_zero) & (!other_is_zero) & x1.ct_eq(&x2) & y1.ct_eq(&y2))
+        // Neither point at infinity, coordinates are the same
     }
 }
 
diff --git a/src/notes/design.rs b/src/notes/design.rs
index 0f8c7faf..6dac9ddc 100644
--- a/src/notes/design.rs
+++ b/src/notes/design.rs
@@ -18,7 +18,7 @@
 //!
 //! This can be derived using the following sage script:
 //!
-//! ```norun
+//! ```no_run
 //! param = -0xd201000000010000
 //! def r(x):
 //!     return (x**4) - (x**2) + 1
diff --git a/src/scalar.rs b/src/scalar.rs
index 18132a03..40b16448 100644
--- a/src/scalar.rs
+++ b/src/scalar.rs
@@ -346,6 +346,16 @@ impl Scalar {
         R
     }
 
+    /// Checks in ct_time whether a Scalar is equal to zero.
+    pub fn is_zero(&self) -> Choice {
+        self.ct_eq(&Scalar::zero())
+    }
+
+    /// Checks in ct_time whether a Scalar is equal to one.   
+    pub fn is_one(&self) -> Choice {
+        self.ct_eq(&Scalar::one())
+    }
+
     /// Returns the internal representation of the Scalar.
     pub const fn internal_repr(&self) -> &[u64; 4] {
         &self.0
@@ -405,12 +415,12 @@ impl Scalar {
     /// Returns the bit representation of the given `Scalar` as
     /// an array of 256 bits represented as `u8`.
     pub fn to_bits(&self) -> [u8; 256] {
-        let mut res = [0u8;256];
+        let mut res = [0u8; 256];
         let bytes = self.to_bytes();
         for (byte, bits) in bytes.iter().zip(res.chunks_mut(8)) {
             bits.iter_mut()
-                  .enumerate()
-                  .for_each(|(i, bit)| *bit = (byte >> i) & 1)
+                .enumerate()
+                .for_each(|(i, bit)| *bit = (byte >> i) & 1)
         }
         res
     }
@@ -1368,12 +1378,30 @@ fn random_scalar_generation() {
 
 #[test]
 fn bit_repr() {
-    let two_pow_128 = Scalar::from(2u64).pow(&[128,0,0,0]);
-    let two_pow_128_bits  = [0u8,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0u8,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0];
+    let two_pow_128 = Scalar::from(2u64).pow(&[128, 0, 0, 0]);
+    let two_pow_128_bits = [
+        0u8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 1, 0u8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    ];
     assert_eq!(&two_pow_128.to_bits()[..], &two_pow_128_bits[..]);
 
-
-    let two_pow_128_minus_rand = Scalar::from(2u64).pow(&[128,0,0,0]) - Scalar::from(7568589u64);
-    let two_pow_128_bits  = [1u8, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1];
-    assert_eq!(&two_pow_128_minus_rand.to_bits()[..128], &two_pow_128_bits[..])
+    let two_pow_128_minus_rand = Scalar::from(2u64).pow(&[128, 0, 0, 0]) - Scalar::from(7568589u64);
+    let two_pow_128_bits = [
+        1u8, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1,
+        1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+        1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+        1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+        1, 1, 1, 1, 1, 1, 1, 1,
+    ];
+    assert_eq!(
+        &two_pow_128_minus_rand.to_bits()[..128],
+        &two_pow_128_bits[..]
+    )
 }


Test patch: