From e0439c1e421e8e087315caf5fe99b6270dfdcb87 Mon Sep 17 00:00:00 2001 From: Joey Beauvais-Feisthauer Date: Tue, 7 Apr 2026 18:47:28 -0400 Subject: [PATCH 1/3] fp: round-trippable serde with golden format tests Derive Serialize/Deserialize on Fp, FqVector, Matrix, Subspace, and QuasiInverse, and rework FpVector's manual serde impl to route through FqVector> so the prime is encoded in the wire format. This gives a uniform representation across all primes that round-trips without out-of-band context, which the zarr save system relies on. The sseq_gui web frontend reads permanents / classes / decompositions as flat JS arrays of u32 (interface/panels.js: `d[0].reduce(...)`, `d[0].indexOf(1)`, `permanentClasses.map(rowToKaTeX)`), so SetClass now carries those fields as Vec and converts from FpVector at the message boundary. That keeps the JS format stable independently of FpVector's own serde impl. Add `tests/serde_format.rs`: * round-trip proptests for FpVector, Matrix, Subspace, and QuasiInverse over a broad spread of primes, dimensions, and limb counts (F_2 vectors up to 5 limbs); * golden tests pinned via expect-test for p = 2, 3, 5, including F_2 vectors that span two and three limbs, so any future format drift fails loudly and is only updatable via UPDATE_EXPECT=1. --- ext/crates/fp/Cargo.toml | 2 +- ext/crates/fp/src/field/fp.rs | 4 +- ext/crates/fp/src/matrix/matrix_inner.rs | 3 +- ext/crates/fp/src/matrix/quasi_inverse.rs | 3 +- ext/crates/fp/src/matrix/subspace.rs | 3 +- ext/crates/fp/src/vector/fp_wrapper/mod.rs | 25 ++- ext/crates/fp/src/vector/inner.rs | 4 +- ext/crates/fp/tests/serde_format.rs | 177 +++++++++++++++++++++ web_ext/sseq_gui/src/actions.rs | 13 +- web_ext/sseq_gui/src/sseq.rs | 13 +- 10 files changed, 229 insertions(+), 18 deletions(-) create mode 100644 ext/crates/fp/tests/serde_format.rs diff --git a/ext/crates/fp/Cargo.toml b/ext/crates/fp/Cargo.toml index d032f12892..b0123aba05 100644 --- a/ext/crates/fp/Cargo.toml +++ b/ext/crates/fp/Cargo.toml @@ -6,7 +6,7 @@ edition = "2024" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -aligned-vec = "0.6.4" +aligned-vec = { version = "0.6.4", features = ["serde"] } build_const = "0.2.2" byteorder = "1.5.0" cfg-if = "1.0.1" diff --git a/ext/crates/fp/src/field/fp.rs b/ext/crates/fp/src/field/fp.rs index 08a17876ce..18a9d0f9d5 100644 --- a/ext/crates/fp/src/field/fp.rs +++ b/ext/crates/fp/src/field/fp.rs @@ -1,3 +1,5 @@ +use serde::{Deserialize, Serialize}; + use super::{ Field, element::{FieldElement, FieldElementContainer}, @@ -8,7 +10,7 @@ pub use crate::prime::fp::*; use crate::{constants::BITS_PER_LIMB, limb::Limb, prime::Prime}; /// A prime field. This is just a wrapper around a prime. -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct Fp

{ p: P, } diff --git a/ext/crates/fp/src/matrix/matrix_inner.rs b/ext/crates/fp/src/matrix/matrix_inner.rs index a2b82c0a0c..61916058e3 100644 --- a/ext/crates/fp/src/matrix/matrix_inner.rs +++ b/ext/crates/fp/src/matrix/matrix_inner.rs @@ -4,6 +4,7 @@ use aligned_vec::AVec; use either::Either; use itertools::Itertools; use maybe_rayon::prelude::*; +use serde::{Deserialize, Serialize}; use super::{QuasiInverse, Subspace}; use crate::{ @@ -19,7 +20,7 @@ use crate::{ /// The way we store matrices means it is easier to perform row operations than column operations, /// and the way we use matrices means we want our matrices to act on the right. Hence we think of /// vectors as row vectors. -#[derive(Clone)] +#[derive(Clone, Serialize, Deserialize)] pub struct Matrix { fp: Fp, rows: usize, diff --git a/ext/crates/fp/src/matrix/quasi_inverse.rs b/ext/crates/fp/src/matrix/quasi_inverse.rs index c109fc2e71..833f3ef9f8 100644 --- a/ext/crates/fp/src/matrix/quasi_inverse.rs +++ b/ext/crates/fp/src/matrix/quasi_inverse.rs @@ -2,6 +2,7 @@ use std::io; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use itertools::Itertools; +use serde::{Deserialize, Serialize}; use super::Matrix; use crate::{ @@ -17,7 +18,7 @@ use crate::{ /// everything (with the standard basis). /// * `preimage` - The actual quasi-inverse, where the basis of the image is that given by /// `image`. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct QuasiInverse { image: Option>, preimage: Matrix, diff --git a/ext/crates/fp/src/matrix/subspace.rs b/ext/crates/fp/src/matrix/subspace.rs index 8e8db13c51..6e96db9669 100644 --- a/ext/crates/fp/src/matrix/subspace.rs +++ b/ext/crates/fp/src/matrix/subspace.rs @@ -2,6 +2,7 @@ use std::{io, ops::Deref}; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use itertools::Itertools; +use serde::{Deserialize, Serialize}; use super::Matrix; use crate::{ @@ -18,7 +19,7 @@ use crate::{ /// # Fields /// * `matrix` - A matrix in reduced row echelon, whose number of columns is the dimension of the /// ambient space and each row is a basis vector of the subspace. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[repr(transparent)] pub struct Subspace { matrix: Matrix, diff --git a/ext/crates/fp/src/vector/fp_wrapper/mod.rs b/ext/crates/fp/src/vector/fp_wrapper/mod.rs index 9ba2eca5cc..112760002a 100644 --- a/ext/crates/fp/src/vector/fp_wrapper/mod.rs +++ b/ext/crates/fp/src/vector/fp_wrapper/mod.rs @@ -239,22 +239,39 @@ impl<'a> IntoIterator for &'a FpVector { impl_from!(); impl_try_into!(); +// `FpVector`'s serde format routes through `FqVector>`, giving a uniform +// representation across all primes: the prime is encoded in the data, so round-tripping works +// without out-of-band context. This is what the zarr save system relies on. +// +// Callers whose wire format must stay a flat `Vec` (e.g. the sseq_gui web frontend, which +// reads vectors as plain JS arrays) should declare their fields as `Vec` and convert at the +// boundary rather than relying on `FpVector`'s own serde impl. impl Serialize for FpVector { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - Vec::::from(self).serialize(serializer) + use crate::field::Fp; + let p = self.prime(); + let fq = Fp::new(p); + let v = FqVector::from_raw_parts(fq, self.len(), self.limbs().to_vec()); + v.serialize(serializer) } } impl<'de> Deserialize<'de> for FpVector { - fn deserialize(_deserializer: D) -> Result + fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { - panic!("Deserializing FpVector not supported"); - // This is needed for ext-websocket/actions to be happy + use crate::field::{Field, Fp}; + let v: FqVector> = FqVector::deserialize(deserializer)?; + let p = v.fq().characteristic(); + // Reconstruct an `FpVector` by round-tripping through the binary limb format. The + // intermediate byte buffer is small and only allocated on deserialize. + let mut bytes = Vec::new(); + v.to_bytes(&mut bytes).map_err(serde::de::Error::custom)?; + Self::from_bytes(p, v.len(), &mut &bytes[..]).map_err(serde::de::Error::custom) } } diff --git a/ext/crates/fp/src/vector/inner.rs b/ext/crates/fp/src/vector/inner.rs index 430e0b83e4..9eb0260384 100644 --- a/ext/crates/fp/src/vector/inner.rs +++ b/ext/crates/fp/src/vector/inner.rs @@ -1,13 +1,15 @@ // This generates better llvm optimization #![allow(clippy::int_plus_one)] +use serde::{Deserialize, Serialize}; + use crate::{field::Field, limb::Limb}; /// A vector over a finite field. /// /// Interally, it packs entries of the vectors into limbs. However, this is an abstraction that must /// not leave the `fp` library. -#[derive(Debug, Hash, Eq, PartialEq, Clone)] +#[derive(Debug, Hash, Eq, PartialEq, Clone, Serialize, Deserialize)] pub struct FqVector { fq: F, len: usize, diff --git a/ext/crates/fp/tests/serde_format.rs b/ext/crates/fp/tests/serde_format.rs new file mode 100644 index 0000000000..4dd889e9ec --- /dev/null +++ b/ext/crates/fp/tests/serde_format.rs @@ -0,0 +1,177 @@ +//! Tests for the serde impls on `FpVector`, `Matrix`, `Subspace`, and `QuasiInverse`. +//! +//! Two kinds of tests: +//! +//! 1. **Round-trip** (`*_roundtrip`). Serialize a value to JSON, deserialize it back, and assert +//! equality. Driven by `proptest` using the existing `Arbitrary` impls on `FqVector`, `Matrix`, +//! and `Subspace`. These verify that the serde impls are mutually consistent across a broad +//! spread of primes, dimensions, and limb counts. +//! +//! 2. **Golden format** (`*_json_format`). Serialize a known small value to JSON and compare +//! against an expected string pinned via `expect-test`. These verify that the on-the-wire format +//! doesn't drift silently — any future change to the serde representation must be explicitly +//! acknowledged by running `UPDATE_EXPECT=1 cargo test -p fp --test serde_format`. Includes F_2 +//! vectors spanning multiple limbs to catch mistakes in the multi-limb encoding used by +//! `FqVector::limbs` / `Matrix::data`. + +use expect_test::expect; +use fp::{ + field::Fp, + matrix::{ + Matrix, QuasiInverse, Subspace, + arbitrary::{MatrixArbParams, SubspaceArbParams}, + }, + prime::ValidPrime, + vector::{FpVector, FqVector, arbitrary::FqVectorArbParams}, +}; +use proptest::prelude::*; + +fn p(n: u32) -> ValidPrime { + ValidPrime::new(n) +} + +// ---------- Proptest strategies ---------- + +/// Arbitrary `FpVector` with length up to 300 (covers 0–5 limbs at F_2). +fn arb_fpvector() -> impl Strategy { + any_with::>>(FqVectorArbParams { + fq: None, + len: (0..=300usize).boxed(), + }) + .prop_map(|v| v.into()) +} + +/// Arbitrary small `Matrix` (dimensions capped so the generated JSON stays manageable). +fn arb_matrix() -> impl Strategy { + any_with::(MatrixArbParams { + p: None, + rows: (0..=20usize).boxed(), + columns: (0..=20usize).boxed(), + }) +} + +/// Arbitrary small `Subspace`. +fn arb_subspace() -> impl Strategy { + any_with::(SubspaceArbParams { + p: None, + dim: (0..=20usize).boxed(), + }) +} + +/// Arbitrary `QuasiInverse` whose `image` field is either `None` or an arbitrary `Vec`. +/// +/// We don't enforce semantic consistency between `image` and `preimage` because this test only +/// exercises serde, and the serialization treats both fields as opaque data. +fn arb_quasi_inverse() -> impl Strategy { + let image = proptest::option::of(proptest::collection::vec(-1isize..100, 0..20usize)); + (arb_matrix(), image).prop_map(|(m, image)| QuasiInverse::new(image, m)) +} + +// ---------- Round-trip tests ---------- + +proptest! { + #![proptest_config(ProptestConfig { cases: 128, ..ProptestConfig::default() })] + + #[test] + fn fpvector_roundtrip(v in arb_fpvector()) { + let s = serde_json::to_string(&v).unwrap(); + let v2: FpVector = serde_json::from_str(&s).unwrap(); + prop_assert_eq!(v, v2); + } + + #[test] + fn matrix_roundtrip(m in arb_matrix()) { + let s = serde_json::to_string(&m).unwrap(); + let m2: Matrix = serde_json::from_str(&s).unwrap(); + prop_assert_eq!(m, m2); + } + + #[test] + fn subspace_roundtrip(s in arb_subspace()) { + let json = serde_json::to_string(&s).unwrap(); + let s2: Subspace = serde_json::from_str(&json).unwrap(); + prop_assert_eq!(s, s2); + } + + #[test] + fn quasi_inverse_roundtrip(qi in arb_quasi_inverse()) { + let s = serde_json::to_string(&qi).unwrap(); + let qi2: QuasiInverse = serde_json::from_str(&s).unwrap(); + prop_assert_eq!(qi, qi2); + } +} + +// ---------- Golden format tests ---------- +// +// To update after an intentional format change: +// +// UPDATE_EXPECT=1 cargo test -p fp --test serde_format + +#[test] +fn fpvector_p2_single_limb_json_format() { + let v = FpVector::from_slice(p(2), &[1, 0, 1, 1, 0]); + let s = serde_json::to_string(&v).unwrap(); + expect![[r#"{"fq":{"p":2},"len":5,"limbs":[13]}"#]].assert_eq(&s); +} + +/// F_2 vector spanning exactly two limbs (entries 0..64 in limb 0, entries 64..128 in limb 1). +#[test] +fn fpvector_p2_two_limbs_json_format() { + let mut entries = vec![0u32; 128]; + entries[0] = 1; + entries[1] = 1; + entries[63] = 1; + entries[64] = 1; + entries[127] = 1; + let v = FpVector::from_slice(p(2), &entries); + let s = serde_json::to_string(&v).unwrap(); + expect![[r#"{"fq":{"p":2},"len":128,"limbs":[9223372036854775811,9223372036854775809]}"#]] + .assert_eq(&s); +} + +/// F_2 vector straddling three limbs (130 entries, with a bit set in every limb). +#[test] +fn fpvector_p2_three_limbs_json_format() { + let mut entries = vec![0u32; 130]; + entries[5] = 1; + entries[70] = 1; + entries[129] = 1; + let v = FpVector::from_slice(p(2), &entries); + let s = serde_json::to_string(&v).unwrap(); + expect![[r#"{"fq":{"p":2},"len":130,"limbs":[32,64,2]}"#]].assert_eq(&s); +} + +#[test] +fn fpvector_p3_json_format() { + let v = FpVector::from_slice(p(3), &[1, 2, 0, 2, 1]); + let s = serde_json::to_string(&v).unwrap(); + expect![[r#"{"fq":{"p":3},"len":5,"limbs":[5137]}"#]].assert_eq(&s); +} + +#[test] +fn fpvector_p5_json_format() { + let v = FpVector::from_slice(p(5), &[4, 2, 0, 3]); + let s = serde_json::to_string(&v).unwrap(); + expect![[r#"{"fq":{"p":5},"len":4,"limbs":[98372]}"#]].assert_eq(&s); +} + +#[test] +fn matrix_p2_json_format() { + let m = Matrix::from_vec(p(2), &[vec![1, 0, 1], vec![0, 1, 1]]); + let s = serde_json::to_string(&m).unwrap(); + expect![[ + r#"{"fp":{"p":2},"rows":2,"physical_rows":2,"columns":3,"data":[5,6],"stride":1,"pivots":[]}"# + ]] + .assert_eq(&s); +} + +#[test] +fn quasi_inverse_p2_json_format() { + let preimage = Matrix::from_vec(p(2), &[vec![1, 0, 1], vec![0, 1, 1]]); + let qi = QuasiInverse::new(Some(vec![0, 1, -1]), preimage); + let s = serde_json::to_string(&qi).unwrap(); + expect![[ + r#"{"image":[0,1,-1],"preimage":{"fp":{"p":2},"rows":2,"physical_rows":2,"columns":3,"data":[5,6],"stride":1,"pivots":[]}}"# + ]] + .assert_eq(&s); +} diff --git a/web_ext/sseq_gui/src/actions.rs b/web_ext/sseq_gui/src/actions.rs index 7ac7173eec..e32d92af7d 100644 --- a/web_ext/sseq_gui/src/actions.rs +++ b/web_ext/sseq_gui/src/actions.rs @@ -321,13 +321,20 @@ pub struct SetDifferential { } impl ActionT for SetDifferential {} +/// The `FpVector` fields are serialized as bare `Vec` (entries in the basis of the ambient F_p +/// vector space) rather than going through `FpVector`'s `Serialize` impl. +/// +/// The sseq_gui web frontend reads these as flat JS arrays (see `interface/panels.js`: +/// `d[0].reduce(...)`, `d[0].indexOf(1)`, `permanentClasses.map(rowToKaTeX)`), and that frontend is +/// the only consumer, so we do the `FpVector → Vec` conversion at the `SetClass` boundary and +/// keep `FpVector`'s own serde format free to carry prime metadata for the zarr save system. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SetClass { pub b: Bidegree, pub state: ClassState, - pub permanents: Vec, - pub classes: Vec>, - pub decompositions: Vec<(FpVector, String, Bidegree)>, + pub permanents: Vec>, + pub classes: Vec>>, + pub decompositions: Vec<(Vec, String, Bidegree)>, pub class_names: Vec, } impl ActionT for SetClass {} diff --git a/web_ext/sseq_gui/src/sseq.rs b/web_ext/sseq_gui/src/sseq.rs index 113947b922..d03c9393f1 100644 --- a/web_ext/sseq_gui/src/sseq.rs +++ b/web_ext/sseq_gui/src/sseq.rs @@ -244,7 +244,10 @@ impl> SseqWrapper

{ ClassState::InProgress }; - let mut decompositions: Vec<(FpVector, String, Bidegree)> = Vec::new(); + // `SetClass` carries vectors as `Vec` so the JS frontend can read them as plain arrays + // (see the comment on `SetClass`). Convert here rather than relying on `FpVector`'s + // `Serialize` impl. + let mut decompositions: Vec<(Vec, String, Bidegree)> = Vec::new(); for (name, prod) in &self.products { let prod_b = prod.inner.b; let prod_origin_b = b - prod_b; @@ -255,7 +258,7 @@ impl> SseqWrapper

{ continue; } decompositions.push(( - matrix.row(i).to_owned(), + matrix.row(i).iter().collect(), format!("{name} {}", self.class_names[prod_origin_b][i]), prod_b, )); @@ -273,7 +276,7 @@ impl> SseqWrapper

{ .inner .permanent_classes(b) .basis() - .map(FpSlice::to_owned) + .map(|row| row.iter().collect()) .collect(), class_names: self.class_names[b].clone(), decompositions, @@ -281,8 +284,8 @@ impl> SseqWrapper

{ .inner .page_data(b) .iter() - .map(|x| x.gens().map(FpSlice::to_owned).collect()) - .collect::>>(), + .map(|x| x.gens().map(|row| row.iter().collect()).collect()) + .collect(), }), }); } From f49fdce0c775924dab6d859b9c042d65f4d2fbb7 Mon Sep 17 00:00:00 2001 From: Joey Beauvais-Feisthauer Date: Tue, 7 Apr 2026 19:01:32 -0400 Subject: [PATCH 2/3] Fix lint --- web_ext/sseq_gui/src/actions.rs | 2 +- web_ext/sseq_gui/src/sseq.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web_ext/sseq_gui/src/actions.rs b/web_ext/sseq_gui/src/actions.rs index e32d92af7d..e1f58774d9 100644 --- a/web_ext/sseq_gui/src/actions.rs +++ b/web_ext/sseq_gui/src/actions.rs @@ -323,7 +323,7 @@ impl ActionT for SetDifferential {} /// The `FpVector` fields are serialized as bare `Vec` (entries in the basis of the ambient F_p /// vector space) rather than going through `FpVector`'s `Serialize` impl. -/// +/// /// The sseq_gui web frontend reads these as flat JS arrays (see `interface/panels.js`: /// `d[0].reduce(...)`, `d[0].indexOf(1)`, `permanentClasses.map(rowToKaTeX)`), and that frontend is /// the only consumer, so we do the `FpVector → Vec` conversion at the `SetClass` boundary and diff --git a/web_ext/sseq_gui/src/sseq.rs b/web_ext/sseq_gui/src/sseq.rs index d03c9393f1..663d1f7c37 100644 --- a/web_ext/sseq_gui/src/sseq.rs +++ b/web_ext/sseq_gui/src/sseq.rs @@ -4,7 +4,7 @@ use bivec::BiVec; use fp::{ matrix::{Matrix, Subquotient}, prime::ValidPrime, - vector::{FpSlice, FpVector}, + vector::FpSlice, }; use once::MultiIndexed; use serde::{Deserialize, Serialize}; From 8fd3664b540dba7af55aa81e9c9e58e57a7dfab0 Mon Sep 17 00:00:00 2001 From: Joey Beauvais-Feisthauer Date: Tue, 7 Apr 2026 22:30:19 -0400 Subject: [PATCH 3/3] fp: validate FqVector/Matrix invariants in Deserialize The derived `Deserialize` impls for `FqVector` and `Matrix` accepted any combination of dimensions and limb data, so malformed input could build instances whose internal invariants (`limbs.len() == fq.number(len)`, `data.len() == physical_rows * stride`, etc.) didn't hold. Downstream accessors like `FqVector::entry`, `FqVector::to_bytes`, `Matrix::row`, and `Matrix::to_bytes` then panicked on bounds-checked slice indexing, which escapes the deserialize boundary instead of surfacing as a normal serde error. Replace both derived impls with manual ones that: * accept an inert `Raw` struct via the derive, * validate the invariants, and * return a `serde::de::Error::custom` on mismatch. The same validation protects `FpVector::deserialize`, which routes through `FqVector>`, so its caller-visible behavior is now "recoverable serde error on malformed input" instead of "panic on the first accessor that touches the bad limb data". Add regression tests covering both the FqVector limb-count check and the Matrix stride / data-length checks. Also switch the golden format tests to `serde_json::to_string_pretty` so the pinned JSON is readable in the test source. This only affects test source formatting, not the wire format. --- ext/crates/fp/src/matrix/matrix_inner.rs | 69 ++++++++- ext/crates/fp/src/vector/inner.rs | 42 ++++- ext/crates/fp/tests/serde_format.rs | 186 ++++++++++++++++++++--- 3 files changed, 274 insertions(+), 23 deletions(-) diff --git a/ext/crates/fp/src/matrix/matrix_inner.rs b/ext/crates/fp/src/matrix/matrix_inner.rs index 61916058e3..40d00545e4 100644 --- a/ext/crates/fp/src/matrix/matrix_inner.rs +++ b/ext/crates/fp/src/matrix/matrix_inner.rs @@ -4,7 +4,7 @@ use aligned_vec::AVec; use either::Either; use itertools::Itertools; use maybe_rayon::prelude::*; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use super::{QuasiInverse, Subspace}; use crate::{ @@ -20,7 +20,7 @@ use crate::{ /// The way we store matrices means it is easier to perform row operations than column operations, /// and the way we use matrices means we want our matrices to act on the right. Hence we think of /// vectors as row vectors. -#[derive(Clone, Serialize, Deserialize)] +#[derive(Clone, Serialize)] pub struct Matrix { fp: Fp, rows: usize, @@ -34,6 +34,71 @@ pub struct Matrix { pub(crate) pivots: Vec, } +// `Deserialize` is implemented manually rather than derived so that we can validate `Matrix`'s +// internal invariants. Without these checks, malformed input could build a `Matrix` whose +// accessors (`row`, `to_bytes`, ...) later panic on bounds-checked slice indexing into `data`, +// escaping the `Deserialize` boundary instead of surfacing as a normal serde error. +impl<'de> Deserialize<'de> for Matrix { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + use serde::de::Error; + + #[derive(Deserialize)] + struct Raw { + fp: Fp, + rows: usize, + physical_rows: usize, + columns: usize, + data: AVec, + stride: usize, + pivots: Vec, + } + + let raw = Raw::deserialize(deserializer)?; + let expected_stride = raw.fp.number(raw.columns); + if raw.stride != expected_stride { + return Err(D::Error::custom(format!( + "Matrix stride {} does not match expected {} for columns={}", + raw.stride, expected_stride, raw.columns, + ))); + } + if raw.physical_rows < raw.rows { + return Err(D::Error::custom(format!( + "Matrix physical_rows {} less than rows {}", + raw.physical_rows, raw.rows, + ))); + } + if raw.data.len() != raw.physical_rows * raw.stride { + return Err(D::Error::custom(format!( + "Matrix data length {} does not match physical_rows*stride = {}*{} = {}", + raw.data.len(), + raw.physical_rows, + raw.stride, + raw.physical_rows * raw.stride, + ))); + } + // `pivots` is either empty (matrix not yet row-reduced) or has one entry per column. + if !raw.pivots.is_empty() && raw.pivots.len() != raw.columns { + return Err(D::Error::custom(format!( + "Matrix pivots length {} must be 0 or columns = {}", + raw.pivots.len(), + raw.columns, + ))); + } + Ok(Self { + fp: raw.fp, + rows: raw.rows, + physical_rows: raw.physical_rows, + columns: raw.columns, + data: raw.data, + stride: raw.stride, + pivots: raw.pivots, + }) + } +} + impl PartialEq for Matrix { fn eq(&self, other: &Self) -> bool { self.data == other.data diff --git a/ext/crates/fp/src/vector/inner.rs b/ext/crates/fp/src/vector/inner.rs index 9eb0260384..38015714a9 100644 --- a/ext/crates/fp/src/vector/inner.rs +++ b/ext/crates/fp/src/vector/inner.rs @@ -1,7 +1,7 @@ // This generates better llvm optimization #![allow(clippy::int_plus_one)] -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use crate::{field::Field, limb::Limb}; @@ -9,13 +9,51 @@ use crate::{field::Field, limb::Limb}; /// /// Interally, it packs entries of the vectors into limbs. However, this is an abstraction that must /// not leave the `fp` library. -#[derive(Debug, Hash, Eq, PartialEq, Clone, Serialize, Deserialize)] +#[derive(Debug, Hash, Eq, PartialEq, Clone, Serialize)] pub struct FqVector { fq: F, len: usize, limbs: Vec, } +// `Deserialize` is implemented manually rather than derived so that we can validate the +// invariant `limbs.len() == fq.number(len)`. Without this check, malformed input that supplies +// too few limbs would build an `FqVector` whose internal accessors (`entry`, `to_bytes`, etc.) +// later panic on bounds-checked slice indexing. With it, malformed input surfaces as a normal +// serde error from the `Deserialize` impl, which is the contract callers expect. +impl<'de, F: Field + Deserialize<'de>> Deserialize<'de> for FqVector { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + use serde::de::Error; + + #[derive(Deserialize)] + #[serde(bound(deserialize = "F: Deserialize<'de>"))] + struct Raw { + fq: F, + len: usize, + limbs: Vec, + } + + let raw = Raw::::deserialize(deserializer)?; + let expected = raw.fq.number(raw.len); + if raw.limbs.len() != expected { + return Err(D::Error::custom(format!( + "FqVector limbs length {} does not match expected {} for len={}", + raw.limbs.len(), + expected, + raw.len, + ))); + } + Ok(Self { + fq: raw.fq, + len: raw.len, + limbs: raw.limbs, + }) + } +} + /// A slice of an `FqVector`. /// /// This immutably borrows the vector and implements `Copy`. diff --git a/ext/crates/fp/tests/serde_format.rs b/ext/crates/fp/tests/serde_format.rs index 4dd889e9ec..04fa2db115 100644 --- a/ext/crates/fp/tests/serde_format.rs +++ b/ext/crates/fp/tests/serde_format.rs @@ -101,8 +101,73 @@ proptest! { } } +// ---------- Malformed input handling ---------- +// +// FqVector's manual `Deserialize` validates that `limbs.len() == fq.number(len)`. Without +// this check, downstream accessors (e.g. `entry()`, `to_bytes()`) panic on bounds-checked +// slice indexing — which would escape the deserialize boundary instead of being a normal +// serde error. + +#[test] +fn fpvector_malformed_too_few_limbs_errors() { + // F_2, len=200 needs ceil(200/64) = 4 limbs, but the input supplies 0. + let json = r#"{"fq":{"p":2},"len":200,"limbs":[]}"#; + let result: Result = serde_json::from_str(json); + assert!( + result.is_err(), + "FpVector::deserialize should error on too few limbs, got {result:?}" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("limbs length") && err.contains("does not match"), + "expected limb-mismatch error, got: {err}" + ); +} + +#[test] +fn fpvector_malformed_too_many_limbs_errors() { + // F_2, len=5 needs 1 limb, but the input supplies 4. + let json = r#"{"fq":{"p":2},"len":5,"limbs":[1,2,3,4]}"#; + let result: Result = serde_json::from_str(json); + assert!( + result.is_err(), + "FpVector::deserialize should error on too many limbs, got {result:?}" + ); +} + +#[test] +fn matrix_malformed_short_data_errors() { + // Claims 10 rows × 3 columns at p=2 (stride=1, so data must be physical_rows*stride + // bytes), but supplies a `data` array that's far too short to back the row indexing. + let json = r#"{"fp":{"p":2},"rows":10,"physical_rows":10,"columns":3,"data":[],"stride":1,"pivots":[]}"#; + let result: Result = serde_json::from_str(json); + assert!( + result.is_err(), + "Matrix::deserialize should error on short data, got {result:?}" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("data length"), + "expected data-length error, got: {err}" + ); +} + +#[test] +fn matrix_malformed_inconsistent_stride_errors() { + // At p=2 with 100 columns we need stride = ceil(100/64) = 2, not 1. + let json = r#"{"fp":{"p":2},"rows":1,"physical_rows":1,"columns":100,"data":[0],"stride":1,"pivots":[]}"#; + let result: Result = serde_json::from_str(json); + assert!( + result.is_err(), + "Matrix::deserialize should error on stride mismatch, got {result:?}" + ); +} + // ---------- Golden format tests ---------- // +// These use `to_string_pretty` so the expected strings in source are readable. The serde +// format itself is the same; pretty-printing only affects whitespace in the test source. +// // To update after an intentional format change: // // UPDATE_EXPECT=1 cargo test -p fp --test serde_format @@ -110,8 +175,18 @@ proptest! { #[test] fn fpvector_p2_single_limb_json_format() { let v = FpVector::from_slice(p(2), &[1, 0, 1, 1, 0]); - let s = serde_json::to_string(&v).unwrap(); - expect![[r#"{"fq":{"p":2},"len":5,"limbs":[13]}"#]].assert_eq(&s); + let s = serde_json::to_string_pretty(&v).unwrap(); + expect![[r#" + { + "fq": { + "p": 2 + }, + "len": 5, + "limbs": [ + 13 + ] + }"#]] + .assert_eq(&s); } /// F_2 vector spanning exactly two limbs (entries 0..64 in limb 0, entries 64..128 in limb 1). @@ -124,9 +199,19 @@ fn fpvector_p2_two_limbs_json_format() { entries[64] = 1; entries[127] = 1; let v = FpVector::from_slice(p(2), &entries); - let s = serde_json::to_string(&v).unwrap(); - expect![[r#"{"fq":{"p":2},"len":128,"limbs":[9223372036854775811,9223372036854775809]}"#]] - .assert_eq(&s); + let s = serde_json::to_string_pretty(&v).unwrap(); + expect![[r#" + { + "fq": { + "p": 2 + }, + "len": 128, + "limbs": [ + 9223372036854775811, + 9223372036854775809 + ] + }"#]] + .assert_eq(&s); } /// F_2 vector straddling three limbs (130 entries, with a bit set in every limb). @@ -137,31 +222,75 @@ fn fpvector_p2_three_limbs_json_format() { entries[70] = 1; entries[129] = 1; let v = FpVector::from_slice(p(2), &entries); - let s = serde_json::to_string(&v).unwrap(); - expect![[r#"{"fq":{"p":2},"len":130,"limbs":[32,64,2]}"#]].assert_eq(&s); + let s = serde_json::to_string_pretty(&v).unwrap(); + expect![[r#" + { + "fq": { + "p": 2 + }, + "len": 130, + "limbs": [ + 32, + 64, + 2 + ] + }"#]] + .assert_eq(&s); } #[test] fn fpvector_p3_json_format() { let v = FpVector::from_slice(p(3), &[1, 2, 0, 2, 1]); - let s = serde_json::to_string(&v).unwrap(); - expect![[r#"{"fq":{"p":3},"len":5,"limbs":[5137]}"#]].assert_eq(&s); + let s = serde_json::to_string_pretty(&v).unwrap(); + expect![[r#" + { + "fq": { + "p": 3 + }, + "len": 5, + "limbs": [ + 5137 + ] + }"#]] + .assert_eq(&s); } #[test] fn fpvector_p5_json_format() { let v = FpVector::from_slice(p(5), &[4, 2, 0, 3]); - let s = serde_json::to_string(&v).unwrap(); - expect![[r#"{"fq":{"p":5},"len":4,"limbs":[98372]}"#]].assert_eq(&s); + let s = serde_json::to_string_pretty(&v).unwrap(); + expect![[r#" + { + "fq": { + "p": 5 + }, + "len": 4, + "limbs": [ + 98372 + ] + }"#]] + .assert_eq(&s); } #[test] fn matrix_p2_json_format() { let m = Matrix::from_vec(p(2), &[vec![1, 0, 1], vec![0, 1, 1]]); - let s = serde_json::to_string(&m).unwrap(); - expect![[ - r#"{"fp":{"p":2},"rows":2,"physical_rows":2,"columns":3,"data":[5,6],"stride":1,"pivots":[]}"# - ]] + let s = serde_json::to_string_pretty(&m).unwrap(); + expect![[r#" + { + "fp": { + "p": 2 + }, + "rows": 2, + "physical_rows": 2, + "columns": 3, + "data": [ + 5, + 6 + ], + "stride": 1, + "pivots": [] + }"#]] .assert_eq(&s); } @@ -169,9 +298,28 @@ fn matrix_p2_json_format() { fn quasi_inverse_p2_json_format() { let preimage = Matrix::from_vec(p(2), &[vec![1, 0, 1], vec![0, 1, 1]]); let qi = QuasiInverse::new(Some(vec![0, 1, -1]), preimage); - let s = serde_json::to_string(&qi).unwrap(); - expect![[ - r#"{"image":[0,1,-1],"preimage":{"fp":{"p":2},"rows":2,"physical_rows":2,"columns":3,"data":[5,6],"stride":1,"pivots":[]}}"# - ]] + let s = serde_json::to_string_pretty(&qi).unwrap(); + expect![[r#" + { + "image": [ + 0, + 1, + -1 + ], + "preimage": { + "fp": { + "p": 2 + }, + "rows": 2, + "physical_rows": 2, + "columns": 3, + "data": [ + 5, + 6 + ], + "stride": 1, + "pivots": [] + } + }"#]] .assert_eq(&s); }