-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
impl (From<u32>, Hash, Ord, Product) for Scalar #52
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ use crate::util::{adc, mac, sbb}; | |
// The internal representation of this type is four 64-bit unsigned | ||
// integers in little-endian order. `Scalar` values are always in | ||
// Montgomery form; i.e., Scalar(a) = aR mod q, with R = 2^256. | ||
#[derive(Clone, Copy, Eq)] | ||
#[derive(Clone, Copy, Eq, Hash)] | ||
pub struct Scalar(pub(crate) [u64; 4]); | ||
|
||
impl fmt::Debug for Scalar { | ||
|
@@ -37,6 +37,12 @@ impl fmt::Display for Scalar { | |
} | ||
} | ||
|
||
impl From<u32> for Scalar { | ||
fn from(val: u32) -> Scalar { | ||
Scalar([val as u64, 0, 0, 0]) * R2 | ||
} | ||
} | ||
|
||
impl From<u64> for Scalar { | ||
fn from(val: u64) -> Scalar { | ||
Scalar([val, 0, 0, 0]) * R2 | ||
|
@@ -59,6 +65,22 @@ impl PartialEq for Scalar { | |
} | ||
} | ||
|
||
impl Ord for Scalar { | ||
fn cmp(&self, other: &Self) -> core::cmp::Ordering { | ||
let mut self_bytes = self.to_bytes(); | ||
let mut other_bytes = other.to_bytes(); | ||
&self_bytes.reverse(); | ||
&other_bytes.reverse(); | ||
self_bytes.cmp(&other_bytes) | ||
} | ||
} | ||
|
||
impl PartialOrd for Scalar { | ||
fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { | ||
Some(self.cmp(other)) | ||
} | ||
} | ||
Comment on lines
+68
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cryptographic structures, in particular finite fields, do not have a total ordering (in the "well acktually" mathematical sense) so I disagree with exposing If you need |
||
|
||
impl ConditionallySelectable for Scalar { | ||
fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self { | ||
Scalar([ | ||
|
@@ -786,6 +808,18 @@ where | |
} | ||
} | ||
|
||
impl<T> core::iter::Product<T> for Scalar | ||
where | ||
T: core::borrow::Borrow<Scalar>, | ||
{ | ||
fn product<I>(iter: I) -> Self | ||
where | ||
I: Iterator<Item = T>, | ||
{ | ||
iter.fold(Self::one(), |acc, item| acc * item.borrow()) | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_inv() { | ||
// Compute -(q^{-1} mod 2^64) mod 2^64 by exponentiating | ||
|
@@ -1231,3 +1265,31 @@ fn test_double() { | |
|
||
assert_eq!(a.double(), a + a); | ||
} | ||
|
||
#[test] | ||
fn test_ord() { | ||
assert!(Scalar::one() > Scalar::zero()); | ||
A-Manning marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let x = Scalar::from_raw([ | ||
0x0000_0000_0000_0000, | ||
0x0000_0000_0000_0000, | ||
0x1111_1111_1111_1111, | ||
0x1111_1111_1111_1111, | ||
]); | ||
let y = Scalar::from_raw([ | ||
0x1111_1111_1111_1111, | ||
0x0000_0000_0000_0000, | ||
0x1111_1111_1111_1111, | ||
0x0000_0000_0000_0000, | ||
]); | ||
assert!(y < x); | ||
} | ||
|
||
// Check that Ord is not comparing the Montgomery representations | ||
#[test] | ||
fn test_ord_montgomery() { | ||
/* 4R mod q = | ||
0x6092c566b31415be66313fbfb2f13fd56212dfe8000d200800000007fffffff8 | ||
5R mod q = | ||
0x04c9cf6d363b9de5cc83b7a7960bb7c566d9f3df00120c0b0000000afffffff5 */ | ||
assert!(Scalar::from(5u32) > Scalar::from(4u32)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As is visible from other changes in the PR, this decreases usability by requiring the caller to specify the source type (e.g.
5u64.into()
instead of5.into()
). I suspect this would be more usable ifimpl From<u64> for Scalar
was replaced withimpl From<T: Into<u64>> for Scalar
(and it would then also work for anyu*
type).