fix: Improve serialization for prime fields (#85)

* fix: Improve serialization for prime fields Summary: 256-bit field serialization is currently 4x u64, ie. the native format. This implements the standard of byte-serialization (corresponding to the PrimeField::{to,from}_repr), and an hex-encoded variant of that for (de)serializers that are human-readable (concretely, json). - Added a new macro `serialize_deserialize_32_byte_primefield!` for custom serialization and deserialization of 32-byte prime field in different struct (Fq, Fp, Fr) across the secp256r, bn256, and derive libraries. - Implemented the new macro for serialization and deserialization in various structs, replacing the previous `serde::{Deserialize, Serialize}` direct use. - Enhanced error checking in the custom serialization methods to ensure valid field elements. - Updated the test function in the tests/field.rs file to include JSON serialization and deserialization tests for object integrity checking. * fixup! fix: Improve serialization for prime fields --------- Co-authored-by: Carlos Pérez <[email protected]>
privacy-scaling-explorations · Sep 18, 2023 · 8e3a33a · 8e3a33a
1 parent 6e2ff38
commit 8e3a33a
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 25 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -14,6 +14,7 @@ criterion = { version = "0.3", features = ["html_reports"] }
 rand_xorshift = "0.3"
 ark-std = { version = "0.3" }
 bincode = "1.3.3"
+serde_json = "1.0.105"
 
 [dependencies]
 subtle = "2.4"
@@ -30,14 +31,15 @@ num-traits = "0.2"
 paste = "1.0.11"
 serde = { version = "1.0", default-features = false, optional = true }
 serde_arrays = { version = "0.1.0", optional = true }
+hex = { version = "0.4", optional = true, default-features = false, features = ["alloc", "serde"] }
 blake2b_simd = "1"
 
 [features]
 default = ["reexport", "bits"]
 asm = []
 bits = ["ff/bits"]
 bn256-table = []
-derive_serde = ["serde/derive", "serde_arrays"]
+derive_serde = ["serde/derive", "serde_arrays", "hex"]
 prefetch = []
 print-trace = ["ark-std/print-trace"]
 reexport = []

diff --git a/src/bn256/fq.rs b/src/bn256/fq.rs
@@ -16,9 +16,6 @@ use core::ops::{Add, Mul, Neg, Sub};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
-#[cfg(feature = "derive_serde")]
-use serde::{Deserialize, Serialize};
-
 /// This represents an element of $\mathbb{F}_q$ where
 ///
 /// `p = 0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47`
@@ -28,9 +25,11 @@ use serde::{Deserialize, Serialize};
 // integers in little-endian order. `Fq` values are always in
 // Montgomery form; i.e., Fq(a) = aR mod q, with R = 2^256.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
 pub struct Fq(pub(crate) [u64; 4]);
 
+#[cfg(feature = "derive_serde")]
+crate::serialize_deserialize_32_byte_primefield!(Fq);
+
 /// Constant representing the modulus
 /// q = 0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47
 const MODULUS: Fq = Fq([

diff --git a/src/bn256/fr.rs b/src/bn256/fr.rs
@@ -31,9 +31,6 @@ use core::ops::{Add, Mul, Neg, Sub};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
-#[cfg(feature = "derive_serde")]
-use serde::{Deserialize, Serialize};
-
 /// This represents an element of $\mathbb{F}_r$ where
 ///
 /// `r = 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001`
@@ -43,9 +40,11 @@ use serde::{Deserialize, Serialize};
 // integers in little-endian order. `Fr` values are always in
 // Montgomery form; i.e., Fr(a) = aR mod r, with R = 2^256.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
 pub struct Fr(pub(crate) [u64; 4]);
 
+#[cfg(feature = "derive_serde")]
+crate::serialize_deserialize_32_byte_primefield!(Fr);
+
 /// Constant representing the modulus
 /// r = 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001
 const MODULUS: Fr = Fr([

diff --git a/src/derive/field.rs b/src/derive/field.rs
@@ -686,3 +686,38 @@ macro_rules! field_bits {
         }
     };
 }
+
+/// A macro to help define serialization and deserialization for prime field implementations
+/// that use 32-byte representations. This assumes the concerned type implements PrimeField
+/// (for from_repr, to_repr).
+#[macro_export]
+macro_rules! serialize_deserialize_32_byte_primefield {
+    ($type:ty) => {
+        impl ::serde::Serialize for $type {
+            fn serialize<S: ::serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+                let bytes = &self.to_repr();
+                if serializer.is_human_readable() {
+                    hex::serde::serialize(bytes, serializer)
+                } else {
+                    bytes.serialize(serializer)
+                }
+            }
+        }
+
+        use ::serde::de::Error as _;
+        impl<'de> ::serde::Deserialize<'de> for $type {
+            fn deserialize<D: ::serde::Deserializer<'de>>(
+                deserializer: D,
+            ) -> Result<Self, D::Error> {
+                let bytes = if deserializer.is_human_readable() {
+                    ::hex::serde::deserialize(deserializer)?
+                } else {
+                    <[u8; 32]>::deserialize(deserializer)?
+                };
+                Option::from(Self::from_repr(bytes)).ok_or_else(|| {
+                    D::Error::custom("deserialized bytes don't encode a valid field element")
+                })
+            }
+        }
+    };
+}
diff --git a/src/secp256k1/fp.rs b/src/secp256k1/fp.rs
@@ -11,9 +11,6 @@ use core::ops::{Add, Mul, Neg, Sub};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
-#[cfg(feature = "derive_serde")]
-use serde::{Deserialize, Serialize};
-
 /// This represents an element of $\mathbb{F}_p$ where
 ///
 /// `p = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f`
@@ -23,9 +20,11 @@ use serde::{Deserialize, Serialize};
 // integers in little-endian order. `Fp` values are always in
 // Montgomery form; i.e., Fp(a) = aR mod p, with R = 2^256.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
 pub struct Fp(pub(crate) [u64; 4]);
 
+#[cfg(feature = "derive_serde")]
+crate::serialize_deserialize_32_byte_primefield!(Fp);
+
 /// Constant representing the modulus
 /// p = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f
 const MODULUS: Fp = Fp([

diff --git a/src/secp256k1/fq.rs b/src/secp256k1/fq.rs
@@ -11,9 +11,6 @@ use core::ops::{Add, Mul, Neg, Sub};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
-#[cfg(feature = "derive_serde")]
-use serde::{Deserialize, Serialize};
-
 /// This represents an element of $\mathbb{F}_q$ where
 ///
 /// `q = 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141`
@@ -23,9 +20,11 @@ use serde::{Deserialize, Serialize};
 // integers in little-endian order. `Fq` values are always in
 // Montgomery form; i.e., Fq(a) = aR mod q, with R = 2^256.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
 pub struct Fq(pub(crate) [u64; 4]);
 
+#[cfg(feature = "derive_serde")]
+crate::serialize_deserialize_32_byte_primefield!(Fq);
+
 /// Constant representing the modulus
 /// q = 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141
 const MODULUS: Fq = Fq([

diff --git a/src/secp256r1/fp.rs b/src/secp256r1/fp.rs
@@ -11,9 +11,6 @@ use core::ops::{Add, Mul, Neg, Sub};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
-#[cfg(feature = "derive_serde")]
-use serde::{Deserialize, Serialize};
-
 /// This represents an element of $\mathbb{F}_p$ where
 ///
 /// `p = 0xffffffff00000001000000000000000000000000ffffffffffffffffffffffff
@@ -23,9 +20,11 @@ use serde::{Deserialize, Serialize};
 // integers in little-endian order. `Fp` values are always in
 // Montgomery form; i.e., Fp(a) = aR mod p, with R = 2^256.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
 pub struct Fp(pub(crate) [u64; 4]);
 
+#[cfg(feature = "derive_serde")]
+crate::serialize_deserialize_32_byte_primefield!(Fp);
+
 /// Constant representing the modulus
 /// p = 0xffffffff00000001000000000000000000000000ffffffffffffffffffffffff
 const MODULUS: Fp = Fp([

diff --git a/src/secp256r1/fq.rs b/src/secp256r1/fq.rs
@@ -5,9 +5,6 @@ use core::ops::{Add, Mul, Neg, Sub};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
-#[cfg(feature = "derive_serde")]
-use serde::{Deserialize, Serialize};
-
 /// This represents an element of $\mathbb{F}_q$ where
 ///
 /// `q = 0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551`
@@ -17,9 +14,11 @@ use serde::{Deserialize, Serialize};
 // integers in little-endian order. `Fq` values are always in
 // Montgomery form; i.e., Fq(a) = aR mod q, with R = 2^256.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
 pub struct Fq(pub(crate) [u64; 4]);
 
+#[cfg(feature = "derive_serde")]
+crate::serialize_deserialize_32_byte_primefield!(Fq);
+
 /// Constant representing the modulus
 /// q = 0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551
 const MODULUS: Fq = Fq([

diff --git a/src/tests/field.rs b/src/tests/field.rs
@@ -280,11 +280,18 @@ where
     let _message = format!("serialization with serde {type_name}");
     let start = start_timer!(|| _message);
     for _ in 0..1000000 {
+        // byte serialization
         let a = F::random(&mut rng);
         let bytes = bincode::serialize(&a).unwrap();
         let reader = std::io::Cursor::new(bytes);
         let b: F = bincode::deserialize_from(reader).unwrap();
         assert_eq!(a, b);
+
+        // json serialization
+        let json = serde_json::to_string(&a).unwrap();
+        let reader = std::io::Cursor::new(json);
+        let b: F = serde_json::from_reader(reader).unwrap();
+        assert_eq!(a, b);
     }
     end_timer!(start);
 }