Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions jolt-core/src/field/challenge/mont_ark_u128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ pub struct MontU128Challenge<F: JoltField> {
// Custom serialization: serialize as [u64; 4] for compatibility with field element format
impl<F: JoltField> Valid for MontU128Challenge<F> {
fn check(&self) -> Result<(), SerializationError> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Score: 25] >> 61 check and >> 3 mask express the same constraint differently

Validation: (self.high >> 61) != 0
Construction: u64::MAX >> 3 (masks top 3 bits)

Both enforce the top 3 bits of high are zero (61 usable bits), but expressed two different ways. A shared constant would make the equivalence explicit:

const CHALLENGE_HIGH_MASK: u64 = u64::MAX >> 3;
// in check(): self.high > CHALLENGE_HIGH_MASK

Nit — the validation is correct for the 125-bit challenge width (64-bit low + 61-bit high).

if (self.high >> 61) != 0 {
return Err(SerializationError::InvalidData);
}
Ok(())
}
}
Expand Down Expand Up @@ -63,12 +66,18 @@ impl<F: JoltField> CanonicalDeserialize for MontU128Challenge<F> {
validate: Validate,
) -> Result<Self, SerializationError> {
let arr = <[u64; 4]>::deserialize_with_mode(reader, compress, validate)?;
// arr[0] and arr[1] should be 0, arr[2] is low, arr[3] is high
Ok(Self {
if arr[0] != 0 || arr[1] != 0 {
return Err(SerializationError::InvalidData);
}
let value = Self {
low: arr[2],
high: arr[3],
_marker: PhantomData,
})
};
if validate == Validate::Yes {
value.check()?;
}
Ok(value)
}
}

Expand Down Expand Up @@ -230,11 +239,28 @@ impl OptimizedMul<TrackedFr, TrackedFr> for MontU128Challenge<TrackedFr> {
#[cfg(test)]
mod tests {
use super::*;
use ark_serialize::CanonicalSerialize;

#[test]
fn masks_high_three_bits_in_default_challenge_width() {
let challenge = MontU128Challenge::<ark_bn254::Fr>::new(u128::MAX);
assert_eq!(challenge.low, u64::MAX);
assert_eq!(challenge.high, (u64::MAX >> 3));
}

#[test]
fn rejects_nonzero_lower_limbs_on_deserialize() {
let mut bytes = Vec::new();
[1u64, 0, 0, 0].serialize_compressed(&mut bytes).unwrap();
assert!(MontU128Challenge::<ark_bn254::Fr>::deserialize_compressed(&bytes[..]).is_err());
}

#[test]
fn rejects_high_bits_outside_challenge_width() {
let mut bytes = Vec::new();
[0u64, 0, 0, 1u64 << 61]
.serialize_compressed(&mut bytes)
.unwrap();
assert!(MontU128Challenge::<ark_bn254::Fr>::deserialize_compressed(&bytes[..]).is_err());
}
}
54 changes: 52 additions & 2 deletions jolt-core/src/poly/commitment/hyrax.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, Valid};

use crate::field::JoltField;
use crate::poly::eq_poly::EqPolynomial;
use crate::utils::serialization::{
deserialize_bounded_vec, serialize_vec_with_len, serialized_vec_with_len_size,
MAX_BLINDFOLD_VECTOR_LEN,
};

/// combined[k] = Σ_i eq(ry_row, i) · flat[i*cols + k]
pub fn combined_row<F: JoltField>(flat: &[F], cols: usize, ry_row: &[F]) -> Vec<F> {
Expand Down Expand Up @@ -44,12 +48,58 @@ pub fn combined_blinding<F: JoltField>(row_blindings: &[F], ry_row: &[F]) -> F {
.sum()
}

#[derive(Clone, Debug, CanonicalSerialize, CanonicalDeserialize)]
#[derive(Clone, Debug)]
pub struct HyraxOpeningProof<F: JoltField> {
pub combined_row: Vec<F>,
pub combined_blinding: F,
}

impl<F: JoltField> CanonicalSerialize for HyraxOpeningProof<F> {
fn serialize_with_mode<W: std::io::Write>(
&self,
mut writer: W,
compress: ark_serialize::Compress,
) -> Result<(), ark_serialize::SerializationError> {
serialize_vec_with_len(&self.combined_row, &mut writer, compress)?;
self.combined_blinding
.serialize_with_mode(&mut writer, compress)
}

fn serialized_size(&self, compress: ark_serialize::Compress) -> usize {
serialized_vec_with_len_size(&self.combined_row, compress)
+ self.combined_blinding.serialized_size(compress)
}
}

impl<F: JoltField> ark_serialize::Valid for HyraxOpeningProof<F> {
fn check(&self) -> Result<(), ark_serialize::SerializationError> {
self.combined_row.check()?;
self.combined_blinding.check()
}
}

impl<F: JoltField> CanonicalDeserialize for HyraxOpeningProof<F> {
fn deserialize_with_mode<R: std::io::Read>(
mut reader: R,
compress: ark_serialize::Compress,
validate: ark_serialize::Validate,
) -> Result<Self, ark_serialize::SerializationError> {
let proof = Self {
combined_row: deserialize_bounded_vec(
&mut reader,
compress,
validate,
MAX_BLINDFOLD_VECTOR_LEN,
)?,
combined_blinding: F::deserialize_with_mode(&mut reader, compress, validate)?,
};
if validate == ark_serialize::Validate::Yes {
proof.check()?;
}
Ok(proof)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
168 changes: 151 additions & 17 deletions jolt-core/src/poly/unipoly.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use crate::field::{ChallengeFieldOps, FieldChallengeOps, JoltField};
use crate::utils::serialization::{
deserialize_bounded_vec, serialize_vec_with_len, serialized_vec_with_len_size,
MAX_UNIPOLY_COEFFS,
};
use std::cmp::Ordering;
use std::iter::zip;
use std::ops::{Add, AddAssign, Index, IndexMut, Mul, MulAssign, Sub};
Expand All @@ -14,21 +18,106 @@ use crate::utils::small_scalar::SmallScalar;

// ax^2 + bx + c stored as vec![c,b,a]
// ax^3 + bx^2 + cx + d stored as vec![d,c,b,a]
#[derive(CanonicalSerialize, CanonicalDeserialize, Debug, Clone, PartialEq, Allocative)]
#[derive(Debug, Clone, PartialEq, Allocative)]
pub struct UniPoly<F: CanonicalSerialize + CanonicalDeserialize> {
pub coeffs: Vec<F>,
}

// ax^2 + bx + c stored as vec![c,a]
// ax^3 + bx^2 + cx + d stored as vec![d,b,a]
#[derive(CanonicalSerialize, CanonicalDeserialize, Debug, Clone)]
#[derive(Debug, Clone)]
pub struct CompressedUniPoly<F: JoltField> {
pub coeffs_except_linear_term: Vec<F>,
}

impl<F: CanonicalSerialize + CanonicalDeserialize> CanonicalSerialize for UniPoly<F> {
fn serialize_with_mode<W: std::io::Write>(
&self,
writer: W,
compress: Compress,
) -> Result<(), SerializationError> {
serialize_vec_with_len(&self.coeffs, writer, compress)
}

fn serialized_size(&self, compress: Compress) -> usize {
serialized_vec_with_len_size(&self.coeffs, compress)
}
}

impl<F: CanonicalSerialize + CanonicalDeserialize> Valid for UniPoly<F> {
fn check(&self) -> Result<(), SerializationError> {
if self.coeffs.is_empty() {
return Err(SerializationError::InvalidData);
}
self.coeffs.check()
}
}

impl<F: CanonicalSerialize + CanonicalDeserialize> CanonicalDeserialize for UniPoly<F> {
fn deserialize_with_mode<R: std::io::Read>(
reader: R,
compress: Compress,
validate: Validate,
) -> Result<Self, SerializationError> {
let coeffs = deserialize_bounded_vec(reader, compress, validate, MAX_UNIPOLY_COEFFS)?;
let poly = Self { coeffs };
if validate == Validate::Yes {
poly.check()?;
}
Ok(poly)
}
}

impl<F: JoltField> CanonicalSerialize for CompressedUniPoly<F> {
fn serialize_with_mode<W: std::io::Write>(
&self,
writer: W,
compress: Compress,
) -> Result<(), SerializationError> {
serialize_vec_with_len(&self.coeffs_except_linear_term, writer, compress)
}

fn serialized_size(&self, compress: Compress) -> usize {
serialized_vec_with_len_size(&self.coeffs_except_linear_term, compress)
}
}

impl<F: JoltField> Valid for CompressedUniPoly<F> {
fn check(&self) -> Result<(), SerializationError> {
if self.coeffs_except_linear_term.is_empty() {
return Err(SerializationError::InvalidData);
}
self.coeffs_except_linear_term.check()
}
}

impl<F: JoltField> CanonicalDeserialize for CompressedUniPoly<F> {
fn deserialize_with_mode<R: std::io::Read>(
reader: R,
compress: Compress,
validate: Validate,
) -> Result<Self, SerializationError> {
let coeffs_except_linear_term =
deserialize_bounded_vec(reader, compress, validate, MAX_UNIPOLY_COEFFS)?;
let poly = Self {
coeffs_except_linear_term,
};
if validate == Validate::Yes {
poly.check()?;
}
Ok(poly)
}
}

impl<F: JoltField> UniPoly<F> {
pub fn from_coeff(coeffs: Vec<F>) -> Self {
UniPoly { coeffs }
if coeffs.is_empty() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Score: 85] divide_with_remainder remainder bypasses the new non-empty invariant

This from_coeff change establishes the invariant that coeffs is never empty. However, divide_with_remainder (line 267) returns the remainder directly without going through from_coeff:

Some((Self::from_coeff(quotient), remainder))

The pop loop at line 263 can empty remainder.coeffs when division is exact. quotient is correctly wrapped via from_coeff, but remainder is not. A subsequent call to remainder.degree() hits the new debug_assert! in debug and underflows (0usize - 1 = usize::MAX) in release.

Also: is_zero() (line 271) still checks self.coeffs.is_empty() — dead branch under the new invariant but load-bearing until this is fixed.

Fix at line 267:

Some((Self::from_coeff(quotient), Self::from_coeff(remainder.coeffs)))

UniPoly {
coeffs: vec![F::zero()],
}
} else {
UniPoly { coeffs }
}
}

/// Interpolate a polynomial from its evaluations at the points 0, 1, 2, ..., n-1.
Expand Down Expand Up @@ -188,10 +277,11 @@ impl<F: JoltField> UniPoly<F> {
}

pub fn zero() -> Self {
Self::from_coeff(Vec::new())
Self::from_coeff(vec![F::zero()])
}

pub fn degree(&self) -> usize {
debug_assert!(!self.coeffs.is_empty());
self.coeffs.len() - 1
}

Expand Down Expand Up @@ -297,10 +387,14 @@ impl<F: JoltField> UniPoly<F> {
}

pub fn compress(&self) -> CompressedUniPoly<F> {
let mut coeffs_except_linear_term = Vec::with_capacity(self.coeffs.len() - 1);
debug_assert!(!self.coeffs.is_empty());
let mut coeffs_except_linear_term =
Vec::with_capacity(self.coeffs.len().saturating_sub(1).max(1));
coeffs_except_linear_term.push(self.coeffs[0]);
coeffs_except_linear_term.extend_from_slice(&self.coeffs[2..]);
debug_assert_eq!(coeffs_except_linear_term.len() + 1, self.coeffs.len());
if self.coeffs.len() > 2 {
coeffs_except_linear_term.extend_from_slice(&self.coeffs[2..]);
debug_assert_eq!(coeffs_except_linear_term.len() + 1, self.coeffs.len());
}
CompressedUniPoly {
coeffs_except_linear_term,
}
Expand Down Expand Up @@ -481,13 +575,22 @@ impl<F: JoltField> MulAssign<&F> for UniPoly<F> {
}

impl<F: JoltField> CompressedUniPoly<F> {
fn recover_linear_term(&self, hint: &F) -> F {
let constant_term = self.coeffs_except_linear_term[0];
let mut linear_term = *hint - constant_term - constant_term;
for coeff in &self.coeffs_except_linear_term[1..] {
linear_term -= *coeff;
}
linear_term
}

// we require eval(0) + eval(1) = hint, so we can solve for the linear term as:
// linear_term = hint - 2 * constant_term - deg2 term - deg3 term
pub fn decompress(&self, hint: &F) -> UniPoly<F> {
let mut linear_term =
*hint - self.coeffs_except_linear_term[0] - self.coeffs_except_linear_term[0];
for i in 1..self.coeffs_except_linear_term.len() {
linear_term -= self.coeffs_except_linear_term[i];
debug_assert!(!self.coeffs_except_linear_term.is_empty());
let linear_term = self.recover_linear_term(hint);
if self.coeffs_except_linear_term.len() == 1 && linear_term.is_zero() {
return UniPoly::from_coeff(vec![self.coeffs_except_linear_term[0]]);
}

let mut coeffs = vec![self.coeffs_except_linear_term[0], linear_term];
Expand All @@ -499,11 +602,8 @@ impl<F: JoltField> CompressedUniPoly<F> {
// In the verifier we do not have to check that f(0) + f(1) = hint as we can just
// recover the linear term assuming the prover did it right, then eval the poly
pub fn eval_from_hint(&self, hint: &F, x: &F::Challenge) -> F {
let mut linear_term =
*hint - self.coeffs_except_linear_term[0] - self.coeffs_except_linear_term[0];
for i in 1..self.coeffs_except_linear_term.len() {
linear_term -= self.coeffs_except_linear_term[i];
}
debug_assert!(!self.coeffs_except_linear_term.is_empty());
let linear_term = self.recover_linear_term(hint);

let mut running_point: F = (*x).into();
let mut running_sum = self.coeffs_except_linear_term[0] + *x * linear_term;
Expand All @@ -515,14 +615,20 @@ impl<F: JoltField> CompressedUniPoly<F> {
}

pub fn degree(&self) -> usize {
self.coeffs_except_linear_term.len()
debug_assert!(!self.coeffs_except_linear_term.is_empty());
if self.coeffs_except_linear_term.len() == 1 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Score: 25] CompressedUniPoly::degree() returns 0 for len==1, underreporting degree-1 polynomials

When a degree-1 poly [c, b] is compressed, result is [c] (len=1). The new degree() returns 0, but the original had degree 1. A malicious compressed poly with 1 coeff trivially passes degree() > degree_bound checks.

In practice sumcheck rounds always have degree >= 2, so this can't trigger from an honest prover. But it weakens the defense-in-depth this PR is adding. The old behavior (coeffs_except_linear_term.len(), returning 1) was more conservative. Consider documenting this limitation.

0
} else {
self.coeffs_except_linear_term.len()
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use ark_bn254::Fr;
use ark_serialize::CanonicalSerialize;
use rand_chacha::ChaCha20Rng;
use rand_core::SeedableRng;

Expand Down Expand Up @@ -577,6 +683,20 @@ mod tests {
fn test_from_evals_cubic() {
test_from_evals_cubic_helper::<Fr>()
}

#[test]
fn test_compressed_linear_round_trip() {
let poly = UniPoly::<Fr>::from_coeff(vec![Fr::from_u64(5), Fr::from_u64(7)]);
let hint = poly.eval_at_zero() + poly.eval_at_one();
let compressed_poly = poly.compress();
let decompressed_poly = compressed_poly.decompress(&hint);

assert_eq!(decompressed_poly.coeffs, poly.coeffs);

let x = <Fr as JoltField>::Challenge::from(9u128);
assert_eq!(compressed_poly.eval_from_hint(&hint, &x), poly.evaluate(&x));
}

fn test_from_evals_cubic_helper<F: JoltField>() {
// polynomial is x^3 + 2x^2 + 3x + 1
let e0 = F::one();
Expand Down Expand Up @@ -661,4 +781,18 @@ mod tests {
);
assert_eq!(poly.coeffs, true_poly.coeffs);
}

#[test]
fn rejects_empty_unipoly_deserialization() {
let mut bytes = Vec::new();
0usize.serialize_compressed(&mut bytes).unwrap();
assert!(UniPoly::<Fr>::deserialize_compressed(&bytes[..]).is_err());
}

#[test]
fn rejects_empty_compressed_unipoly_deserialization() {
let mut bytes = Vec::new();
0usize.serialize_compressed(&mut bytes).unwrap();
assert!(CompressedUniPoly::<Fr>::deserialize_compressed(&bytes[..]).is_err());
}
}
Loading