From 42fe546f5745d45503e353d35cd039cfbbdcc099 Mon Sep 17 00:00:00 2001 From: Jonathan Wang Date: Thu, 18 May 2023 20:06:51 -0700 Subject: [PATCH] fix: guard `ScalarField` to be little-endian --- halo2-base/src/gates/flex_gate.rs | 8 +- halo2-base/src/gates/tests/flex_gate_tests.rs | 2 +- halo2-base/src/gates/tests/pos_prop_tests.rs | 15 +++ halo2-base/src/lib.rs | 1 + halo2-base/src/utils.rs | 116 +++++++++++++----- .../zkevm-keccak/src/keccak_packed_multi.rs | 2 +- hashes/zkevm-keccak/src/util.rs | 8 +- hashes/zkevm-keccak/src/util/eth_types.rs | 4 +- 8 files changed, 112 insertions(+), 44 deletions(-) diff --git a/halo2-base/src/gates/flex_gate.rs b/halo2-base/src/gates/flex_gate.rs index 95bc841e..cc24c76a 100644 --- a/halo2-base/src/gates/flex_gate.rs +++ b/halo2-base/src/gates/flex_gate.rs @@ -1107,13 +1107,7 @@ impl GateInstructions for GateChip { a: AssignedValue, range_bits: usize, ) -> Vec> { - let a_bytes = a.value().to_repr(); - let bits = a_bytes - .as_ref() - .iter() - .flat_map(|byte| (0..8u32).map(|i| (*byte as u64 >> i) & 1)) - .map(|x| Witness(F::from(x))) - .take(range_bits); + let bits = a.value().to_u64_limbs(range_bits, 1).into_iter().map(|x| Witness(F::from(x))); let mut bit_cells = Vec::with_capacity(range_bits); let row_offset = ctx.advice.len(); diff --git a/halo2-base/src/gates/tests/flex_gate_tests.rs b/halo2-base/src/gates/tests/flex_gate_tests.rs index eb64efcd..b6d3e5ec 100644 --- a/halo2-base/src/gates/tests/flex_gate_tests.rs +++ b/halo2-base/src/gates/tests/flex_gate_tests.rs @@ -239,7 +239,7 @@ pub fn test_is_equal(inputs: &[QuantumCell]) -> F { *a.value() } -#[test_case((Fr::one(), 2) => vec![Fr::one(), Fr::zero()] ; "num_to_bits(): 1 -> [1, 0]")] +#[test_case((Fr::from(6u64), 3) => vec![Fr::zero(), Fr::one(), Fr::one()] ; "num_to_bits(): 6")] pub fn test_num_to_bits(input: (F, usize)) -> Vec { let mut builder = GateThreadBuilder::mock(); let ctx = builder.main(0); diff --git a/halo2-base/src/gates/tests/pos_prop_tests.rs b/halo2-base/src/gates/tests/pos_prop_tests.rs index 695ec890..d23d019e 100644 --- a/halo2-base/src/gates/tests/pos_prop_tests.rs +++ b/halo2-base/src/gates/tests/pos_prop_tests.rs @@ -230,6 +230,21 @@ proptest! { prop_assert_eq!(result, ground_truth); } + #[test] + fn prop_test_num_to_bits(num in any::()) { + let mut tmp = num; + let mut bits = vec![]; + if num == 0 { + bits.push(0); + } + while tmp > 0 { + bits.push(tmp & 1); + tmp /= 2; + } + let result = flex_gate_tests::test_num_to_bits((Fr::from(num), bits.len())); + prop_assert_eq!(bits.into_iter().map(Fr::from).collect::>(), result); + } + /* #[test] fn prop_test_lagrange_eval(inputs in vec(rand_fr(), 3)) { diff --git a/halo2-base/src/lib.rs b/halo2-base/src/lib.rs index 45224578..289d4057 100644 --- a/halo2-base/src/lib.rs +++ b/halo2-base/src/lib.rs @@ -44,6 +44,7 @@ pub mod utils; /// Constant representing whether the Layouter calls `synthesize` once just to get region shape. #[cfg(feature = "halo2-axiom")] pub const SKIP_FIRST_PASS: bool = false; +/// Constant representing whether the Layouter calls `synthesize` once just to get region shape. #[cfg(feature = "halo2-pse")] pub const SKIP_FIRST_PASS: bool = true; diff --git a/halo2-base/src/utils.rs b/halo2-base/src/utils.rs index 39af3c6c..979fc099 100644 --- a/halo2-base/src/utils.rs +++ b/halo2-base/src/utils.rs @@ -19,7 +19,7 @@ pub trait BigPrimeField: ScalarField { #[cfg(feature = "halo2-axiom")] impl BigPrimeField for F where - F: FieldExt + Hash + Into<[u64; 4]> + From<[u64; 4]>, + F: ScalarField + From<[u64; 4]>, // Assume [u64; 4] is little-endian. We only implement ScalarField when this is true. { #[inline(always)] fn from_u64_digits(val: &[u64]) -> Self { @@ -30,10 +30,9 @@ where } } -/// Helper trait to convert to and from a [ScalarField] by decomposing its an field element into [u64] limbs. +/// Helper trait to represent a field element that can be converted into [u64] limbs. /// -/// Note: Since the number of bits necessary to represent a field element is larger than the number of bits in a u64, we decompose the bit representation of the field element into multiple [u64] values e.g. `limbs`. -#[cfg(feature = "halo2-axiom")] +/// Note: Since the number of bits necessary to represent a field element is larger than the number of bits in a u64, we decompose the integer representation of the field element into multiple [u64] values e.g. `limbs`. pub trait ScalarField: FieldExt + Hash { /// Returns the base `2bit_len` little endian representation of the [ScalarField] element up to `num_limbs` number of limbs (truncates any extra limbs). /// @@ -41,27 +40,26 @@ pub trait ScalarField: FieldExt + Hash { /// * `num_limbs`: number of limbs to return /// * `bit_len`: number of bits in each limb fn to_u64_limbs(self, num_limbs: usize, bit_len: usize) -> Vec; -} -#[cfg(feature = "halo2-axiom")] -impl ScalarField for F -where - F: FieldExt + Hash + Into<[u64; 4]>, -{ - #[inline(always)] - fn to_u64_limbs(self, num_limbs: usize, bit_len: usize) -> Vec { - // Basically same as `to_repr` but does not go further into bytes - let tmp: [u64; 4] = self.into(); - decompose_u64_digits_to_limbs(tmp, num_limbs, bit_len) + + /// Returns the little endian byte representation of the element. + fn to_bytes_le(&self) -> Vec; + + /// Creates a field element from a little endian byte representation. + /// + /// The default implementation assumes that `PrimeField::from_repr` is implemented for little-endian. + /// It should be overriden if this is not the case. + fn from_bytes_le(bytes: &[u8]) -> Self { + let mut repr = Self::Repr::default(); + repr.as_mut()[..bytes.len()].copy_from_slice(bytes); + Self::from_repr(repr).unwrap() } } +// See below for implementations // Later: will need to separate BigPrimeField from ScalarField when Goldilocks is introduced #[cfg(feature = "halo2-pse")] -pub trait BigPrimeField = FieldExt + Hash; - -#[cfg(feature = "halo2-pse")] -pub trait ScalarField = FieldExt + Hash; +pub trait BigPrimeField = FieldExt + ScalarField; /// Converts an [Iterator] of u64 digits into `number_of_limbs` limbs of `bit_len` bits returned as a [Vec]. /// @@ -149,10 +147,8 @@ pub fn biguint_to_fe(e: &BigUint) -> F { #[cfg(feature = "halo2-pse")] { - let mut repr = F::Repr::default(); let bytes = e.to_bytes_le(); - repr.as_mut()[..bytes.len()].copy_from_slice(&bytes); - F::from_repr(repr).unwrap() + F::from_bytes_le(&bytes) } } @@ -171,9 +167,7 @@ pub fn bigint_to_fe(e: &BigInt) -> F { #[cfg(feature = "halo2-pse")] { let (sign, bytes) = e.to_bytes_le(); - let mut repr = F::Repr::default(); - repr.as_mut()[..bytes.len()].copy_from_slice(&bytes); - let f_abs = F::from_repr(repr).unwrap(); + let f_abs = F::from_bytes_le(&bytes); if sign == Sign::Minus { -f_abs } else { @@ -184,12 +178,14 @@ pub fn bigint_to_fe(e: &BigInt) -> F { /// Converts an immutable reference to an PrimeField element into a [BigUint] element. /// * `fe`: immutable reference to PrimeField element to convert -pub fn fe_to_biguint(fe: &F) -> BigUint { - BigUint::from_bytes_le(fe.to_repr().as_ref()) +pub fn fe_to_biguint(fe: &F) -> BigUint { + BigUint::from_bytes_le(fe.to_bytes_le().as_ref()) } -/// Converts an immutable reference to a [BigPrimeField] element into a [BigInt] element. -/// * `fe`: immutable reference to [BigPrimeField] element to convert +/// Converts a [BigPrimeField] element into a [BigInt] element by sending `fe` in `[0, F::modulus())` to +/// ``` +/// fe < F::modulus() / 2 ? fe : fe - F::modulus() +/// ``` pub fn fe_to_bigint(fe: &F) -> BigInt { // TODO: `F` should just have modulus as lazy_static or something let modulus = modulus::(); @@ -332,6 +328,7 @@ pub fn compose(input: Vec, bit_len: usize) -> BigUint { #[cfg(feature = "halo2-axiom")] pub use halo2_proofs_axiom::halo2curves::CurveAffineExt; +/// Helper trait #[cfg(feature = "halo2-pse")] pub trait CurveAffineExt: CurveAffine { /// Unlike the `Coordinates` trait, this just returns the raw affine (X, Y) coordinantes without checking `is_on_curve` @@ -343,6 +340,67 @@ pub trait CurveAffineExt: CurveAffine { #[cfg(feature = "halo2-pse")] impl CurveAffineExt for C {} +mod scalar_field_impls { + use super::{decompose_u64_digits_to_limbs, ScalarField}; + use crate::halo2_proofs::halo2curves::{ + bn256::{Fq as bn254Fq, Fr as bn254Fr}, + secp256k1::{Fp as secpFp, Fq as secpFq}, + }; + #[cfg(feature = "halo2-pse")] + use ff::PrimeField; + + /// To ensure `ScalarField` is only implemented for `ff:Field` where `Repr` is little endian, we use the following macro + /// to implement the trait for each field. + #[cfg(feature = "halo2-axiom")] + #[macro_export] + macro_rules! impl_scalar_field { + ($field:ident) => { + impl ScalarField for $field { + #[inline(always)] + fn to_u64_limbs(self, num_limbs: usize, bit_len: usize) -> Vec { + // Basically same as `to_repr` but does not go further into bytes + let tmp: [u64; 4] = self.into(); + decompose_u64_digits_to_limbs(tmp, num_limbs, bit_len) + } + + #[inline(always)] + fn to_bytes_le(&self) -> Vec { + let tmp: [u64; 4] = (*self).into(); + tmp.iter().flat_map(|x| x.to_le_bytes()).collect() + } + } + }; + } + + /// To ensure `ScalarField` is only implemented for `ff:Field` where `Repr` is little endian, we use the following macro + /// to implement the trait for each field. + #[cfg(feature = "halo2-pse")] + #[macro_export] + macro_rules! impl_scalar_field { + ($field:ident) => { + impl ScalarField for $field { + #[inline(always)] + fn to_u64_limbs(self, num_limbs: usize, bit_len: usize) -> Vec { + let bytes = self.to_repr(); + let digits = (0..4) + .map(|i| u64::from_le_bytes(bytes[i * 8..(i + 1) * 8].try_into().unwrap())); + decompose_u64_digits_to_limbs(digits, num_limbs, bit_len) + } + + #[inline(always)] + fn to_bytes_le(&self) -> Vec { + self.to_repr().to_vec() + } + } + }; + } + + impl_scalar_field!(bn254Fr); + impl_scalar_field!(bn254Fq); + impl_scalar_field!(secpFp); + impl_scalar_field!(secpFq); +} + /// Module for reading parameters for Halo2 proving system from the file system. pub mod fs { use std::{ diff --git a/hashes/zkevm-keccak/src/keccak_packed_multi.rs b/hashes/zkevm-keccak/src/keccak_packed_multi.rs index 3edc2e1a..37b3d367 100644 --- a/hashes/zkevm-keccak/src/keccak_packed_multi.rs +++ b/hashes/zkevm-keccak/src/keccak_packed_multi.rs @@ -1941,7 +1941,7 @@ pub fn keccak_phase0( .take(4) .map(|a| { pack_with_base::(&unpack(a[0]), 2) - .to_repr() + .to_bytes_le() .into_iter() .take(8) .collect::>() diff --git a/hashes/zkevm-keccak/src/util.rs b/hashes/zkevm-keccak/src/util.rs index 868c366c..b3e2e2b5 100644 --- a/hashes/zkevm-keccak/src/util.rs +++ b/hashes/zkevm-keccak/src/util.rs @@ -183,7 +183,7 @@ pub fn pack_part(bits: &[u8], info: &PartInfo) -> u64 { /// Unpack a sparse keccak word into bits in the range [0,BIT_SIZE[ pub fn unpack(packed: F) -> [u8; NUM_BITS_PER_WORD] { let mut bits = [0; NUM_BITS_PER_WORD]; - let packed = Word::from_little_endian(packed.to_repr().as_ref()); + let packed = Word::from_little_endian(packed.to_bytes_le().as_ref()); let mask = Word::from(BIT_SIZE - 1); for (idx, bit) in bits.iter_mut().enumerate() { *bit = ((packed >> (idx * BIT_COUNT)) & mask).as_u32() as u8; @@ -200,10 +200,10 @@ pub fn pack_u64(value: u64) -> F { /// Calculates a ^ b with a and b field elements pub fn field_xor(a: F, b: F) -> F { let mut bytes = [0u8; 32]; - for (idx, (a, b)) in a.to_repr().as_ref().iter().zip(b.to_repr().as_ref().iter()).enumerate() { - bytes[idx] = *a ^ *b; + for (idx, (a, b)) in a.to_bytes_le().into_iter().zip(b.to_bytes_le()).enumerate() { + bytes[idx] = a ^ b; } - F::from_repr(bytes).unwrap() + F::from_bytes_le(&bytes) } /// Returns the size (in bits) of each part size when splitting up a keccak word diff --git a/hashes/zkevm-keccak/src/util/eth_types.rs b/hashes/zkevm-keccak/src/util/eth_types.rs index 3217f810..6fed74a5 100644 --- a/hashes/zkevm-keccak/src/util/eth_types.rs +++ b/hashes/zkevm-keccak/src/util/eth_types.rs @@ -71,7 +71,7 @@ impl ToScalar for U256 { fn to_scalar(&self) -> Option { let mut bytes = [0u8; 32]; self.to_little_endian(&mut bytes); - F::from_repr(bytes).into() + Some(F::from_bytes_le(&bytes)) } } @@ -113,7 +113,7 @@ impl ToScalar for Address { let mut bytes = [0u8; 32]; bytes[32 - Self::len_bytes()..].copy_from_slice(self.as_bytes()); bytes.reverse(); - F::from_repr(bytes).into() + Some(F::from_bytes_le(&bytes)) } }