Replace Scalar::from_u64 with From impls

Unfortunately, Rust selects `i32` as the type for an integer literal
when the literal has no other type constraints.  This means that someone
cannot write `Scalar::from(1)`, as Rust will choose `i32` as the type for
`1`, and we don't `impl From<i32> for Scalar`.

We could implement `From` conversions for signed integers, but since
`Scalar` operations should be constant-time by default, this would
require us to extract the sign bit of the integer and use it to
conditionally select between the positive and negative of Scalar
constructed from the value bits.  This is more expensive than the
unsigned operation, and I don't think it's what anyone really wants.

Making API consumers specify that their literals are unsigned is
slightly annoying, but better than the above alternative.

It would also be nice to change `Scalar::from_hash` to be
`impl<D: Digest<OutputSize = U64>> From<D> for Scalar`,
but this isn't currently allowed by Rust (since that `impl` "could"
conflict with the `impl From<u8>` if someone decided that `u8` should
`impl Digest`).
This commit is contained in:
Henry de Valence 2018-07-17 10:53:02 -07:00
parent e0b7af8957
commit 1e74cb3e56
7 changed files with 94 additions and 59 deletions

View file

@ -42,7 +42,7 @@ harness = false
[dependencies]
rand = { version = "0.5", default-features = false }
byteorder = { version = "1", default-features = false }
byteorder = { version = "1", default-features = false, features = ["i128"] }
digest = "0.7"
generic-array = "0.9"
clear_on_drop = "=0.2.3"
@ -51,7 +51,7 @@ serde = { version = "1.0", optional = true }
[build-dependencies]
rand = { version = "0.5", default-features = false }
byteorder = { version = "1", default-features = false }
byteorder = { version = "1", default-features = false, features = ["i128"] }
digest = "0.7"
generic-array = "0.9"
clear_on_drop = "=0.2.3"

View file

@ -39,7 +39,7 @@ mod edwards_benches {
fn consttime_fixed_base_scalar_mul(c: &mut Criterion) {
let B = &constants::ED25519_BASEPOINT_TABLE;
let s = Scalar::from_u64(897987897).invert();
let s = Scalar::from(897987897u64).invert();
c.bench_function("Constant-time fixed-base scalar mul", move |b| {
b.iter(|| B * &s)
});
@ -47,7 +47,7 @@ mod edwards_benches {
fn consttime_variable_base_scalar_mul(c: &mut Criterion) {
let B = &constants::ED25519_BASEPOINT_POINT;
let s = Scalar::from_u64(897987897).invert();
let s = Scalar::from(897987897u64).invert();
c.bench_function("Constant-time variable-base scalar mul", move |b| {
b.iter(|| B * &s)
});
@ -56,8 +56,8 @@ mod edwards_benches {
fn vartime_double_base_scalar_mul(c: &mut Criterion) {
c.bench_function("Variable-time aA+bB, A variable, B fixed", |bench| {
let B = &constants::ED25519_BASEPOINT_POINT;
let a = Scalar::from_u64(298374928).invert();
let b = Scalar::from_u64(897987897).invert();
let a = Scalar::from(298374928u64).invert();
let b = Scalar::from(897987897u64).invert();
let A = B * (b * a);
bench.iter(|| EdwardsPoint::vartime_double_scalar_mul_basepoint(&a, &A, &b));
});
@ -157,7 +157,7 @@ mod montgomery_benches {
fn montgomery_ladder(c: &mut Criterion) {
c.bench_function("Montgomery pseudomultiplication", |b| {
let B = constants::X25519_BASEPOINT;
let s = Scalar::from_u64(897987897).invert();
let s = Scalar::from(897987897u64).invert();
b.iter(|| B * s);
});
}
@ -174,7 +174,7 @@ mod scalar_benches {
fn scalar_inversion(c: &mut Criterion) {
c.bench_function("Scalar inversion", |b| {
let s = Scalar::from_u64(897987897).invert();
let s = Scalar::from(897987897u64).invert();
b.iter(|| s.invert());
});
}

View file

@ -431,7 +431,7 @@ mod test {
println!("Testing B +- kB");
let P = constants::ED25519_BASEPOINT_POINT;
let Q = &constants::ED25519_BASEPOINT_TABLE * &Scalar::from_u64(8475983829);
let Q = &constants::ED25519_BASEPOINT_TABLE * &Scalar::from(8475983829u64);
addition_test_helper(P, Q);
}
@ -510,7 +510,7 @@ mod test {
doubling_test_helper(P);
println!("Testing [2]([k]B)");
let P = &constants::ED25519_BASEPOINT_TABLE * &Scalar::from_u64(8475983829);
let P = &constants::ED25519_BASEPOINT_TABLE * &Scalar::from(8475983829u64);
doubling_test_helper(P);
}
}

View file

@ -1004,7 +1004,7 @@ mod test {
/// Test that computing 2*basepoint is the same as basepoint.double()
#[test]
fn basepoint_mult_two_vs_basepoint2() {
let two = Scalar::from_u64(2);
let two = Scalar::from(2u64);
let bp2 = &constants::ED25519_BASEPOINT_TABLE * &two;
assert_eq!(bp2.compress(), BASE2_CMPRSSD);
}
@ -1030,10 +1030,10 @@ mod test {
// Test that sum works for non-empty iterators
let BASE = constants::ED25519_BASEPOINT_POINT;
let s1 = Scalar::from_u64(999);
let s1 = Scalar::from(999u64);
let P1 = &BASE * &s1;
let s2 = Scalar::from_u64(333);
let s2 = Scalar::from(333u64);
let P2 = &BASE * &s2;
let vec = vec![P1.clone(), P2.clone()];
@ -1048,7 +1048,7 @@ mod test {
assert_eq!(sum, EdwardsPoint::identity());
// Test that sum works on owning iterators
let s = Scalar::from_u64(2);
let s = Scalar::from(2u64);
let mapped = vec.iter().map(|x| x * &s);
let sum: EdwardsPoint = mapped.sum();

View file

@ -839,7 +839,7 @@ impl VartimeMultiscalarMul for RistrettoPoint {
/// use curve25519_dalek::constants;
/// use curve25519_dalek::scalar::Scalar;
///
/// let a = Scalar::from_u64(87329482);
/// let a = Scalar::from(87329482u64);
/// let P = &a * &constants::RISTRETTO_BASEPOINT_TABLE;
/// ```
#[derive(Clone)]
@ -959,7 +959,7 @@ mod test {
#[test]
fn scalarmult_ristrettopoint_works_both_ways() {
let P = constants::RISTRETTO_BASEPOINT_POINT;
let s = Scalar::from_u64(999);
let s = Scalar::from(999u64);
let P1 = &P * &s;
let P2 = &s * &P;
@ -973,10 +973,10 @@ mod test {
// Test that sum works for non-empty iterators
let BASE = constants::RISTRETTO_BASEPOINT_POINT;
let s1 = Scalar::from_u64(999);
let s1 = Scalar::from(999u64);
let P1 = &BASE * &s1;
let s2 = Scalar::from_u64(333);
let s2 = Scalar::from(333u64);
let P2 = &BASE * &s2;
let vec = vec![P1.clone(), P2.clone()];
@ -991,7 +991,7 @@ mod test {
assert_eq!(sum, RistrettoPoint::identity());
// Test that sum works on owning iterators
let s = Scalar::from_u64(2);
let s = Scalar::from(2u64);
let mapped = vec.iter().map(|x| x * &s);
let sum: RistrettoPoint = mapped.sum();

View file

@ -426,6 +426,50 @@ where
}
}
impl From<u8> for Scalar {
fn from(x: u8) -> Scalar {
let mut s_bytes = [0u8; 32];
s_bytes[0] = x;
Scalar{ bytes: s_bytes }
}
}
impl From<u16> for Scalar {
fn from(x: u16) -> Scalar {
use byteorder::{ByteOrder, LittleEndian};
let mut s_bytes = [0u8; 32];
LittleEndian::write_u16(&mut s_bytes, x);
Scalar{ bytes: s_bytes }
}
}
impl From<u32> for Scalar {
fn from(x: u32) -> Scalar {
use byteorder::{ByteOrder, LittleEndian};
let mut s_bytes = [0u8; 32];
LittleEndian::write_u32(&mut s_bytes, x);
Scalar{ bytes: s_bytes }
}
}
impl From<u64> for Scalar {
fn from(x: u64) -> Scalar {
use byteorder::{ByteOrder, LittleEndian};
let mut s_bytes = [0u8; 32];
LittleEndian::write_u64(&mut s_bytes, x);
Scalar{ bytes: s_bytes }
}
}
impl From<u128> for Scalar {
fn from(x: u128) -> Scalar {
use byteorder::{ByteOrder, LittleEndian};
let mut s_bytes = [0u8; 32];
LittleEndian::write_u128(&mut s_bytes, x);
Scalar{ bytes: s_bytes }
}
}
impl Scalar {
/// Return a `Scalar` chosen uniformly at random using a user-provided RNG.
///
@ -513,15 +557,6 @@ impl Scalar {
}
}
/// Construct a scalar from the given `u64`.
pub fn from_u64(x: u64) -> Scalar {
let mut s_bytes = [0u8; 32];
for i in 0..8 {
s_bytes[i] = (x >> (i*8)) as u8;
}
Scalar{ bytes: s_bytes }
}
/// Given a nonzero `Scalar`, compute its multiplicative inverse.
///
/// # Warning
@ -585,19 +620,19 @@ impl Scalar {
/// # use curve25519_dalek::scalar::Scalar;
/// # fn main() {
/// let mut scalars = [
/// Scalar::from_u64(3),
/// Scalar::from_u64(5),
/// Scalar::from_u64(7),
/// Scalar::from_u64(11),
/// Scalar::from(3u64),
/// Scalar::from(5u64),
/// Scalar::from(7u64),
/// Scalar::from(11u64),
/// ];
///
/// let allinv = Scalar::batch_invert(&mut scalars);
///
/// assert_eq!(allinv, Scalar::from_u64(3*5*7*11).invert());
/// assert_eq!(scalars[0], Scalar::from_u64(3).invert());
/// assert_eq!(scalars[1], Scalar::from_u64(5).invert());
/// assert_eq!(scalars[2], Scalar::from_u64(7).invert());
/// assert_eq!(scalars[3], Scalar::from_u64(11).invert());
/// assert_eq!(allinv, Scalar::from(3*5*7*11u64).invert());
/// assert_eq!(scalars[0], Scalar::from(3u64).invert());
/// assert_eq!(scalars[1], Scalar::from(5u64).invert());
/// assert_eq!(scalars[2], Scalar::from(7u64).invert());
/// assert_eq!(scalars[3], Scalar::from(11u64).invert());
/// # }
/// ```
#[cfg(any(feature = "alloc", feature = "std"))]
@ -1050,9 +1085,9 @@ mod test {
}
#[test]
fn from_unsigned() {
let val = 0xdeadbeefdeadbeef;
let s = Scalar::from_u64(val);
fn from_u64() {
let val: u64 = 0xdeadbeefdeadbeef;
let s = Scalar::from(val);
assert_eq!(s[7], 0xde);
assert_eq!(s[6], 0xad);
assert_eq!(s[5], 0xbe);
@ -1073,7 +1108,7 @@ mod test {
#[test]
fn impl_add() {
let two = Scalar::from_u64(2);
let two = Scalar::from(2u64);
let one = Scalar::one();
let should_be_two = &one + &one;
assert_eq!(should_be_two, two);
@ -1101,8 +1136,8 @@ mod test {
assert_eq!(should_be_one, one);
// Test that product works for iterators where Item = Scalar
let xs = [Scalar::from_u64(2); 10];
let ys = [Scalar::from_u64(3); 10];
let xs = [Scalar::from(2u64); 10];
let ys = [Scalar::from(3u64); 10];
// now zs is an iterator with Item = Scalar
let zs = xs.iter().zip(ys.iter()).map(|(x,y)| x * y);
@ -1110,9 +1145,9 @@ mod test {
let y_prod: Scalar = ys.iter().product();
let z_prod: Scalar = zs.product();
assert_eq!(x_prod, Scalar::from_u64(1024));
assert_eq!(y_prod, Scalar::from_u64(59049));
assert_eq!(z_prod, Scalar::from_u64(60466176));
assert_eq!(x_prod, Scalar::from(1024u64));
assert_eq!(y_prod, Scalar::from(59049u64));
assert_eq!(z_prod, Scalar::from(60466176u64));
assert_eq!(x_prod * y_prod, z_prod);
}
@ -1121,7 +1156,7 @@ mod test {
fn impl_sum() {
// Test that sum works for non-empty iterators
let two = Scalar::from_u64(2);
let two = Scalar::from(2u64);
let one_vector = vec![Scalar::one(), Scalar::one()];
let should_be_two: Scalar = one_vector.iter().sum();
assert_eq!(should_be_two, two);
@ -1133,8 +1168,8 @@ mod test {
assert_eq!(should_be_zero, zero);
// Test that sum works for owned types
let xs = [Scalar::from_u64(1); 10];
let ys = [Scalar::from_u64(2); 10];
let xs = [Scalar::from(1u64); 10];
let ys = [Scalar::from(2u64); 10];
// now zs is an iterator with Item = Scalar
let zs = xs.iter().zip(ys.iter()).map(|(x,y)| x + y);
@ -1142,9 +1177,9 @@ mod test {
let y_sum: Scalar = ys.iter().sum();
let z_sum: Scalar = zs.sum();
assert_eq!(x_sum, Scalar::from_u64(10));
assert_eq!(y_sum, Scalar::from_u64(20));
assert_eq!(z_sum, Scalar::from_u64(30));
assert_eq!(x_sum, Scalar::from(10u64));
assert_eq!(y_sum, Scalar::from(20u64));
assert_eq!(z_sum, Scalar::from(30u64));
assert_eq!(x_sum + y_sum, z_sum);
}
@ -1296,7 +1331,7 @@ mod test {
#[test]
fn batch_invert_consistency() {
let mut x = Scalar::from_u64(1);
let mut x = Scalar::from(1u64);
let mut v1: Vec<_> = (0..16).map(|_| {let tmp = x; x = x + x; tmp}).collect();
let v2 = v1.clone();

View file

@ -72,9 +72,9 @@ pub trait MultiscalarMul {
/// use curve25519_dalek::scalar::Scalar;
///
/// // Some scalars
/// let a = Scalar::from_u64(87329482);
/// let b = Scalar::from_u64(37264829);
/// let c = Scalar::from_u64(98098098);
/// let a = Scalar::from(87329482u64);
/// let b = Scalar::from(37264829u64);
/// let c = Scalar::from(98098098u64);
///
/// // Some points
/// let P = constants::RISTRETTO_BASEPOINT_POINT;
@ -128,9 +128,9 @@ pub trait VartimeMultiscalarMul {
/// use curve25519_dalek::scalar::Scalar;
///
/// // Some scalars
/// let a = Scalar::from_u64(87329482);
/// let b = Scalar::from_u64(37264829);
/// let c = Scalar::from_u64(98098098);
/// let a = Scalar::from(87329482u64);
/// let b = Scalar::from(37264829u64);
/// let c = Scalar::from(98098098u64);
///
/// // Some points
/// let P = constants::RISTRETTO_BASEPOINT_POINT;