Merge pull request #136 from LLFourn/implement-ord-for-point-and-scalar

Implement Ord and Partial Ord on scalar and point

Author: LLFourn

Diff for: CHANGELOG.md

@@ -8,11 +8,12 @@
 - Allow `Zero` points to serialize
 - Remove requirement of `CryptoRng` everywhere
 - Rename `from_scalar_mul` to `even_y_from_scalar_mul` to be more explicit
-- Remove `XOnly` in favor of `Point<EvenY>`
+- Remove `XOnly` in favour of `Point<EvenY>`
 - Replace `.mark` system with methods for changing each marker type.
 - Make `From<u32>` for `Scalar` regardless of secrecy
 - Merge `AddTag` and `Tagged` into one trait `Tag`
 - Add `NonceRng` impls for `RefCell` and `Mutex`
+- Add `Ord` and `PartialOrd` implementations for Scalar and Point
 
 ## 0.7.1
 

Diff for: schnorr_fun/src/frost.rs

@@ -722,7 +722,7 @@ impl<H: Digest<OutputSize = U32> + Clone, NG> Frost<H, NG> {
                 .non_zero()
                 .unwrap_or_else(|| {
                     // Use the same trick as the MuSig spec
-                    G.clone().normalize()
+                    G.normalize()
                 })
                 .into_point_with_even_y();
 

Diff for: schnorr_fun/src/musig.rs

@@ -479,7 +479,7 @@ impl<H: Digest<OutputSize = U32> + Clone, NG> MuSig<H, Schnorr<H, NG>> {
             .unwrap_or_else(|| {
                 // if final nonce is zero we set it to generator as in MuSig spec
                 debug_assert!(G.is_y_even());
-                G.clone().normalize()
+                G.normalize()
             })
             .into_point_with_even_y();
 

Diff for: secp256kfun/src/backend/k256_impl.rs

@@ -6,9 +6,9 @@ use crate::{
 use core::ops::Neg;
 use subtle::{Choice, ConditionallyNegatable, ConditionallySelectable, ConstantTimeEq};
 
-pub static G_TABLE: ProjectivePoint = ProjectivePoint::GENERATOR;
 pub static G_POINT: ProjectivePoint = ProjectivePoint::GENERATOR;
 pub type Point = ProjectivePoint;
+// We don't implement multiplication tables yet
 pub type BasePoint = ProjectivePoint;
 
 impl BackendScalar for Scalar {

Diff for: secp256kfun/src/lib.rs

@@ -68,7 +68,7 @@ pub extern crate proptest;
 ///[_SEC 2: Recommended Elliptic Curve Domain Parameters_]: https://www.secg.org/sec2-v2.pdf
 ///[`BasePoint`]: crate::marker::BasePoint
 pub static G: &'static Point<marker::BasePoint, marker::Public, marker::NonZero> =
-    &Point::from_inner(backend::G_POINT, marker::BasePoint(backend::G_TABLE));
+    &Point::from_inner(backend::G_POINT, marker::BasePoint);
 
 // it is applied to nonce generators too so export at root
 pub use hash::Tag;

Diff for: secp256kfun/src/marker/point_type.rs

@@ -8,7 +8,9 @@
 ///
 /// [`Point<T,S,Z>`]: crate::Point
 /// [`G`]: crate::G
-pub trait PointType: Sized + Clone + Copy + 'static {
+pub trait PointType:
+    Sized + Clone + Copy + PartialEq + Eq + core::hash::Hash + Ord + PartialOrd
+{
     /// The point type returned from the negation of a point of this type.
     type NegationType: Default;
 
@@ -18,7 +20,7 @@ pub trait PointType: Sized + Clone + Copy + 'static {
 
 /// A Fully Normalized Point. Internally `Normal` points are represented using
 /// _affine_ coordinates with fully normalized `x` and `y` field elements.
-#[derive(Default, Debug, Clone, Copy)]
+#[derive(Default, Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[cfg_attr(feautre = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Normal;
 /// A Non-normalized Point. Usually, represented as three field elements three field elements:
@@ -37,15 +39,15 @@ pub struct Normal;
 ///
 /// [`normalize`]: crate::Point::normalize
 
-#[derive(Default, Debug, Clone, Copy)]
+#[derive(Default, Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct NonNormal;
 
 /// Backwards compatibility type alias.
 #[deprecated(note = "use NonNormal instead")]
 pub type Jacobian = NonNormal;
 
 /// A [`Normal`] point whose `y` coordinate is known to be even.
-#[derive(Default, Debug, Clone, Copy)]
+#[derive(Default, Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[cfg_attr(feautre = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct EvenY;
 
@@ -56,22 +58,16 @@ pub struct EvenY;
 /// At the time of writing no pre-computation is done.
 ///
 /// [`G`]: crate::G
-#[derive(Clone, Copy)]
-pub struct BasePoint(pub(crate) crate::backend::BasePoint);
+#[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)]
+pub struct BasePoint;
 
 /// A marker trait that indicates a `PointType` uses a affine internal representation.
 pub trait Normalized: PointType {}
 
-pub(crate) trait NotBasePoint: Default {}
-
 impl Normalized for EvenY {}
 impl Normalized for Normal {}
 impl Normalized for BasePoint {}
 
-impl NotBasePoint for NonNormal {}
-impl NotBasePoint for EvenY {}
-impl NotBasePoint for Normal {}
-
 impl<N: Normalized> PointType for N {
     type NegationType = Normal;
 

Diff for: secp256kfun/src/marker/secrecy.rs

@@ -31,16 +31,16 @@
 /// [`Point`]: crate::marker::Public
 /// [`Scalar`s]: crate::Scalar
 /// [`Point`s]: crate::Point
-pub trait Secrecy: Default + Clone + PartialEq + Copy + 'static {}
+pub trait Secrecy: Default + Clone + PartialEq + Eq + Copy + 'static + Ord + PartialOrd {}
 
 /// Indicates that the value is secret and therefore makes core operations
 /// executed on it to use  _constant time_ versions of the operations.
-#[derive(Debug, Clone, Default, PartialEq, Copy)]
+#[derive(Debug, Clone, Default, PartialEq, Eq, Copy, Ord, PartialOrd)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct Secret;
 
 /// Indicates that variable time operations may be used on the value.
-#[derive(Debug, Clone, Default, PartialEq, Copy, Eq, Hash)]
+#[derive(Debug, Clone, Default, PartialEq, Eq, Copy, Hash, Ord, PartialOrd)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct Public;
 

Diff for: secp256kfun/src/marker/zero_choice.rs

@@ -1,10 +1,10 @@
 /// Something marked with Zero might be `0` i.e. the additive identity
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Ord, PartialOrd)]
 #[cfg_attr(feautre = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Zero;
 
 /// Something marked with `NonZero` is guaranteed not to be 0.
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Ord, PartialOrd)]
 #[cfg_attr(feautre = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct NonZero;
 
@@ -16,11 +16,13 @@ pub trait ZeroChoice:
     Default
     + Clone
     + PartialEq
+    + Eq
     + Copy
     + DecideZero<NonZero>
     + DecideZero<Zero>
-    + Eq
     + core::hash::Hash
+    + Ord
+    + PartialOrd
     + 'static
 {
     /// Returns whether the type is `Zero`

Diff for: secp256kfun/src/op.rs

@@ -15,7 +15,7 @@
 //! use secp256kfun::{marker::*, op, Scalar, G};
 //! let x = Scalar::random(&mut rand::thread_rng());
 //! let X1 = op::scalar_mul_point(&x, G); // fast
-//! let H = &G.clone().normalize(); // scrub `BasePoint` marker
+//! let H = &G.normalize(); // scrub `BasePoint` marker
 //! let X2 = op::scalar_mul_point(&x, &H); // slow
 //! assert_eq!(X1, X2);
 //! ```

Diff for: secp256kfun/src/point.rs

@@ -327,6 +327,20 @@ impl core::hash::Hash for Point<EvenY, Public, NonZero> {
     }
 }
 
+impl<T1: Normalized, S1, Z1, T2: Normalized, S2, Z2> PartialOrd<Point<T2, S2, Z2>>
+    for Point<T1, S1, Z1>
+{
+    fn partial_cmp(&self, other: &Point<T2, S2, Z2>) -> Option<core::cmp::Ordering> {
+        Some(self.to_bytes().cmp(&other.to_bytes()))
+    }
+}
+
+impl<T1: Normalized, S1, Z1> Ord for Point<T1, S1, Z1> {
+    fn cmp(&self, other: &Point<T1, S1, Z1>) -> core::cmp::Ordering {
+        self.to_bytes().cmp(&other.to_bytes())
+    }
+}
+
 impl<S, Z, T: Normalized> Point<T, S, Z> {
     /// Converts the point to its compressed encoding as specified by [_Standards for Efficient Cryptography_].
     ///

Diff for: secp256kfun/src/scalar.rs

@@ -49,7 +49,6 @@ use rand_core::RngCore;
 /// [`Secrecy`]: crate::marker::Secrecy
 /// [`Secret`]: crate::marker::Secret
 /// [`ZeroChoice]: crate::marker::ZeroChoice
-#[derive(Eq)]
 pub struct Scalar<S = Secret, Z = NonZero>(pub(crate) backend::Scalar, PhantomData<(Z, S)>);
 
 impl<Z> Copy for Scalar<Public, Z> {}
@@ -291,6 +290,8 @@ impl<Z1, Z2, S1, S2> PartialEq<Scalar<S2, Z2>> for Scalar<S1, Z1> {
     }
 }
 
+impl<Z, S> Eq for Scalar<Z, S> {}
+
 impl<S> From<u32> for Scalar<S, Zero> {
     fn from(int: u32) -> Self {
         Self::from_inner(backend::BackendScalar::from_u32(int))
@@ -405,6 +406,18 @@ impl<SL, SR, ZR: ZeroChoice> MulAssign<&Scalar<SR, ZR>> for Scalar<SL, Zero> {
     }
 }
 
+impl<S1, Z1, S2, Z2> PartialOrd<Scalar<S2, Z2>> for Scalar<S1, Z1> {
+    fn partial_cmp(&self, other: &Scalar<S2, Z2>) -> Option<core::cmp::Ordering> {
+        Some(self.to_bytes().cmp(&other.to_bytes()))
+    }
+}
+
+impl<S, Z> Ord for Scalar<S, Z> {
+    fn cmp(&self, other: &Self) -> core::cmp::Ordering {
+        self.to_bytes().cmp(&other.to_bytes())
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

Diff for: secp256kfun/tests/against_c_lib.rs

@@ -1,9 +1,10 @@
 #![allow(non_snake_case)]
+#![cfg(feature = "proptest")]
 #[cfg(not(target_arch = "wasm32"))]
 mod against_c_lib {
     use proptest::prelude::*;
     use secp256k1::{PublicKey, SecretKey, SECP256K1 as SECP};
-    use secp256kfun::{g, op::double_mul, s, Scalar, G};
+    use secp256kfun::{g, op::double_mul, s, Point, Scalar, G};
 
     proptest! {
         #[test]
@@ -121,5 +122,21 @@ mod against_c_lib {
             let scalar = Scalar::from_bytes_mod_order(bytes.clone());
             prop_assert_eq!(&(-scalar).to_bytes()[..], &sk[..]);
         }
+
+        #[test]
+        fn point_ord(point1 in any::<Point>(), point2 in any::<Point>()) {
+            prop_assert_eq!(
+                point1.cmp(&point2),
+                PublicKey::from(point1).cmp(&PublicKey::from(point2))
+            );
+        }
+
+        #[test]
+        fn scalar_ord(scalar1 in any::<Scalar>(), scalar2 in any::<Scalar>()) {
+            prop_assert_eq!(
+                scalar1.cmp(&scalar2),
+                SecretKey::from(scalar1).cmp(&SecretKey::from(scalar2))
+            );
+        }
     }
 }