From 6122dccb6e76affed060d9d644bdabfac878ce6b Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Mon, 12 Jun 2023 10:08:30 +0800 Subject: [PATCH 1/3] Move operations to Field Implement the TODOs and move `from_noncanonical_u64` and `from_noncanonical_i64` from `Field64` to `Field`. --- field/src/goldilocks_field.rs | 32 ++++++++++++++++---------------- field/src/types.rs | 28 ++++++++++++++++++++-------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/field/src/goldilocks_field.rs b/field/src/goldilocks_field.rs index e10d0c74..4432d9c4 100644 --- a/field/src/goldilocks_field.rs +++ b/field/src/goldilocks_field.rs @@ -118,22 +118,6 @@ impl Field for GoldilocksField { reduce128(n) } - #[inline] - fn multiply_accumulate(&self, x: Self, y: Self) -> Self { - // u64 + u64 * u64 cannot overflow. - reduce128((self.0 as u128) + (x.0 as u128) * (y.0 as u128)) - } -} - -impl PrimeField for GoldilocksField { - fn to_canonical_biguint(&self) -> BigUint { - self.to_canonical_u64().into() - } -} - -impl Field64 for GoldilocksField { - const ORDER: u64 = 0xFFFFFFFF00000001; - #[inline] fn from_noncanonical_u64(n: u64) -> Self { Self(n) @@ -151,6 +135,22 @@ impl Field64 for GoldilocksField { }) } + #[inline] + fn multiply_accumulate(&self, x: Self, y: Self) -> Self { + // u64 + u64 * u64 cannot overflow. + reduce128((self.0 as u128) + (x.0 as u128) * (y.0 as u128)) + } +} + +impl PrimeField for GoldilocksField { + fn to_canonical_biguint(&self) -> BigUint { + self.to_canonical_u64().into() + } +} + +impl Field64 for GoldilocksField { + const ORDER: u64 = 0xFFFFFFFF00000001; + #[inline] unsafe fn add_canonical_u64(&self, rhs: u64) -> Self { let (res_wrapped, carry) = self.0.overflowing_add(rhs); diff --git a/field/src/types.rs b/field/src/types.rs index 0ae31847..2c0d15c7 100644 --- a/field/src/types.rs +++ b/field/src/types.rs @@ -341,6 +341,26 @@ pub trait Field: /// Returns `n % Self::characteristic()`. fn from_noncanonical_u128(n: u128) -> Self; + /// Returns `x % Self::CHARACTERISTIC`. + /// + /// Implemented by default via `from_noncanonical_u128` + /// Override, if you have a faster implementation for your field. + fn from_noncanonical_u64(n: u64) -> Self { + Self::from_noncanonical_u128(n as u128) + } + + /// Returns `n` as an element of this field. + /// + /// Implemented by default via `from_noncanonical_u128` and case analysis. + /// Override, if you have a faster implementation for your field. + fn from_noncanonical_i64(n: i64) -> Self { + if n < 0 { + -Self::from_noncanonical_u128((n as i128).unsigned_abs()) + } else { + Self::from_noncanonical_u128(n as u128) + } + } + /// Returns `n % Self::characteristic()`. May be cheaper than from_noncanonical_u128 when we know /// that `n < 2 ** 96`. #[inline] @@ -501,14 +521,6 @@ pub trait PrimeField: Field { pub trait Field64: Field { const ORDER: u64; - /// Returns `x % Self::CHARACTERISTIC`. - // TODO: Move to `Field`. - fn from_noncanonical_u64(n: u64) -> Self; - - /// Returns `n` as an element of this field. - // TODO: Move to `Field`. - fn from_noncanonical_i64(n: i64) -> Self; - /// Returns `n` as an element of this field. Assumes that `0 <= n < Self::ORDER`. // TODO: Move to `Field`. // TODO: Should probably be unsafe. From 4df4d865556ed4e17e84defeb0a55dbd8706fb73 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Tue, 4 Jul 2023 17:21:27 +0800 Subject: [PATCH 2/3] No default implementation --- field/src/extension/quadratic.rs | 8 ++++++++ field/src/extension/quartic.rs | 8 ++++++++ field/src/extension/quintic.rs | 8 ++++++++ field/src/secp256k1_base.rs | 9 +++++++++ field/src/secp256k1_scalar.rs | 9 +++++++++ field/src/types.rs | 18 ++---------------- 6 files changed, 44 insertions(+), 16 deletions(-) diff --git a/field/src/extension/quadratic.rs b/field/src/extension/quadratic.rs index c0c9758b..281369d2 100644 --- a/field/src/extension/quadratic.rs +++ b/field/src/extension/quadratic.rs @@ -109,6 +109,14 @@ impl> Field for QuadraticExtension { fn from_noncanonical_u128(n: u128) -> Self { F::from_noncanonical_u128(n).into() } + + fn from_noncanonical_i64(n: i64) -> Self { + F::from_noncanonical_i64(n).into() + } + + fn from_noncanonical_u64(n: u64) -> Self { + F::from_noncanonical_u64(n).into() + } } impl> Display for QuadraticExtension { diff --git a/field/src/extension/quartic.rs b/field/src/extension/quartic.rs index e7aba63b..8c8a9e7e 100644 --- a/field/src/extension/quartic.rs +++ b/field/src/extension/quartic.rs @@ -119,6 +119,14 @@ impl> Field for QuarticExtension { fn from_noncanonical_u128(n: u128) -> Self { F::from_noncanonical_u128(n).into() } + + fn from_noncanonical_i64(n: i64) -> Self { + F::from_noncanonical_i64(n).into() + } + + fn from_noncanonical_u64(n: u64) -> Self { + F::from_noncanonical_u64(n).into() + } } impl> Display for QuarticExtension { diff --git a/field/src/extension/quintic.rs b/field/src/extension/quintic.rs index d4b605eb..28ec9226 100644 --- a/field/src/extension/quintic.rs +++ b/field/src/extension/quintic.rs @@ -126,6 +126,14 @@ impl> Field for QuinticExtension { fn from_noncanonical_u128(n: u128) -> Self { F::from_noncanonical_u128(n).into() } + + fn from_noncanonical_i64(n: i64) -> Self { + F::from_noncanonical_i64(n).into() + } + + fn from_noncanonical_u64(n: u64) -> Self { + F::from_noncanonical_u64(n).into() + } } impl> Display for QuinticExtension { diff --git a/field/src/secp256k1_base.rs b/field/src/secp256k1_base.rs index eaa964f8..bf4cbe61 100644 --- a/field/src/secp256k1_base.rs +++ b/field/src/secp256k1_base.rs @@ -142,6 +142,15 @@ impl Field for Secp256K1Base { fn from_noncanonical_u96(n: (u64, u32)) -> Self { Self([n.0, n.1 as u64, 0, 0]) } + + fn from_noncanonical_i64(n: i64) -> Self { + let f = Self::from_canonical_u64(n.unsigned_abs()); + if n < 0 { -f } else { f } + } + + fn from_noncanonical_u64(n: u64) -> Self { + Self::from_canonical_u64(n) + } } impl PrimeField for Secp256K1Base { diff --git a/field/src/secp256k1_scalar.rs b/field/src/secp256k1_scalar.rs index 1f1de697..b4494681 100644 --- a/field/src/secp256k1_scalar.rs +++ b/field/src/secp256k1_scalar.rs @@ -150,6 +150,15 @@ impl Field for Secp256K1Scalar { fn from_noncanonical_u96(n: (u64, u32)) -> Self { Self([n.0, n.1 as u64, 0, 0]) } + + fn from_noncanonical_i64(n: i64) -> Self { + let f = Self::from_canonical_u64(n.unsigned_abs()); + if n < 0 { -f } else { f } + } + + fn from_noncanonical_u64(n: u64) -> Self { + Self::from_canonical_u64(n) + } } impl PrimeField for Secp256K1Scalar { diff --git a/field/src/types.rs b/field/src/types.rs index 2c0d15c7..646dff73 100644 --- a/field/src/types.rs +++ b/field/src/types.rs @@ -342,24 +342,10 @@ pub trait Field: fn from_noncanonical_u128(n: u128) -> Self; /// Returns `x % Self::CHARACTERISTIC`. - /// - /// Implemented by default via `from_noncanonical_u128` - /// Override, if you have a faster implementation for your field. - fn from_noncanonical_u64(n: u64) -> Self { - Self::from_noncanonical_u128(n as u128) - } + fn from_noncanonical_u64(n: u64) -> Self; /// Returns `n` as an element of this field. - /// - /// Implemented by default via `from_noncanonical_u128` and case analysis. - /// Override, if you have a faster implementation for your field. - fn from_noncanonical_i64(n: i64) -> Self { - if n < 0 { - -Self::from_noncanonical_u128((n as i128).unsigned_abs()) - } else { - Self::from_noncanonical_u128(n as u128) - } - } + fn from_noncanonical_i64(n: i64) -> Self; /// Returns `n % Self::characteristic()`. May be cheaper than from_noncanonical_u128 when we know /// that `n < 2 ** 96`. From 2d7a94de6adc3accd06ed02b1e0a40481cbe1ed4 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Tue, 11 Jul 2023 12:05:22 -0700 Subject: [PATCH 3/3] formatting --- field/src/secp256k1_base.rs | 6 +++++- field/src/secp256k1_scalar.rs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/field/src/secp256k1_base.rs b/field/src/secp256k1_base.rs index bf4cbe61..6632a7f8 100644 --- a/field/src/secp256k1_base.rs +++ b/field/src/secp256k1_base.rs @@ -145,7 +145,11 @@ impl Field for Secp256K1Base { fn from_noncanonical_i64(n: i64) -> Self { let f = Self::from_canonical_u64(n.unsigned_abs()); - if n < 0 { -f } else { f } + if n < 0 { + -f + } else { + f + } } fn from_noncanonical_u64(n: u64) -> Self { diff --git a/field/src/secp256k1_scalar.rs b/field/src/secp256k1_scalar.rs index b4494681..3ca5b6ba 100644 --- a/field/src/secp256k1_scalar.rs +++ b/field/src/secp256k1_scalar.rs @@ -153,7 +153,11 @@ impl Field for Secp256K1Scalar { fn from_noncanonical_i64(n: i64) -> Self { let f = Self::from_canonical_u64(n.unsigned_abs()); - if n < 0 { -f } else { f } + if n < 0 { + -f + } else { + f + } } fn from_noncanonical_u64(n: u64) -> Self {