feat(jolt-hyperkzg): add HyperKZG multilinear commitment scheme#1488
Conversation
|
Warning This PR has more than 500 changed lines and does not include a spec. Large features and architectural changes benefit from a spec-driven workflow. If this PR is a bug fix, refactor, or doesn't warrant a spec, feel free to ignore this message. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| assert_eq!( | ||
| wit.len(), | ||
| 3, | ||
| "HyperKZG requires exactly 3 evaluation points" | ||
| ); |
There was a problem hiding this comment.
Critical DoS vulnerability: This assertion in the verifier code path will panic if a malicious prover sends a proof with incorrect number of witness commitments. A panic in verification creates a denial-of-service vector.
// Replace assertion with error return:
if wit.len() != 3 {
return false;
}The verifier should gracefully reject invalid proofs rather than panic.
| assert_eq!( | |
| wit.len(), | |
| 3, | |
| "HyperKZG requires exactly 3 evaluation points" | |
| ); | |
| if wit.len() != 3 { | |
| return false; | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
0xAndoroid
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
from
cae9bc9 to
9cdc512
Compare
Benchmark comparison (crates) |
0xAndoroid
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
from
9cdc512 to
093cf0e
Compare
Adds a `fuzz/` sub-workspace to `jolt-sumcheck` with two libFuzzer targets, matching the pattern already established for `jolt-crypto`, `jolt-field`, `jolt-poly`, and `jolt-transcript`. ## Targets * `sumcheck_verifier` — drives `SumcheckVerifier::verify` with attacker- controlled `SumcheckClaim` (num_vars 0..=8, degree 1..=6, arbitrary claimed_sum) and a fuzzer-chosen sequence of `UnivariatePoly<Fr>` round polynomials whose lengths are independently controlled per round (0..=degree+1). Asserts the verifier returns `Ok(_)` or a typed `SumcheckError` and never panics. * `batched_sumcheck_verifier` — same panic-guard for `BatchedSumcheckVerifier::verify`, exercising the front-loaded batching path: variable `num_claims` (including 0, which must surface as `EmptyClaims`), per-claim `num_vars` mismatches, and `mul_pow_2` scaling. ## CI * Adds `jolt-sumcheck` to the matrix in `.github/workflows/fuzz-crates.yml`. ## Why `jolt-sumcheck` is verifier-only and protocol-critical: a panic on a malformed proof would let a malicious prover crash the verifier rather than receive a clean rejection. This is the same threat model that PR a16z#1408 hardened in `jolt-core` (`catch_unwind` around the verifier) and that `verify_tampered` in `jolt-dory` and the `tampered_proof` target in `jolt-hyperkzg` (a16z#1488) cover for the PCS layer. The existing unit tests in `crates/jolt-sumcheck/src/tests.rs` cover honest-prover soundness; this crate covers the dual property — adversarial-input robustness — with coverage-guided exploration. ## Local validation * `cargo +nightly fuzz build` (apple-darwin) — clean * `cargo +nightly fuzz run sumcheck_verifier -- -max_total_time=15` — 2.66M runs, no crashes * `cargo +nightly fuzz run batched_sumcheck_verifier -- -max_total_time=15` — 3.06M runs, no crashes * `cargo nextest run -p jolt-sumcheck` — 40/40 pass The fuzz sub-workspace re-applies the root workspace's `[patch.crates-io]` for the arkworks fork, mirroring the existing `jolt-transcript/fuzz` and `jolt-crypto/fuzz` setup, since fuzz sub-workspaces do not inherit the root workspace patches.
| pub(crate) fn compute_witness_polynomial<F: Field>(f: &[F], u: F) -> Vec<F> { | ||
| let d = f.len(); | ||
| let mut h = vec![F::zero(); d]; | ||
| for i in (1..d).rev() { | ||
| h[i - 1] = f[i] + h[i] * u; | ||
| } | ||
| h | ||
| } |
There was a problem hiding this comment.
The witness polynomial returns a vector of length d but the quotient polynomial h(x) = f(x)/(x-u) should have degree d-2 (i.e., d-1 coefficients). The returned vector includes a trailing zero at h[d-1] that is never set in the loop. While mathematically correct, this causes the MSM at line 111 to include an unnecessary zero coefficient, wasting computation. More critically, if the SRS has exactly d-1 powers, the MSM at line 111 will attempt to access setup.g1_powers[d-1] which would be out of bounds.
pub(crate) fn compute_witness_polynomial<F: Field>(f: &[F], u: F) -> Vec<F> {
let d = f.len();
let mut h = vec![F::zero(); d - 1]; // Should be d-1, not d
for i in (1..d).rev() {
h[i - 1] = f[i] + h[i] * u;
}
h
}| pub(crate) fn compute_witness_polynomial<F: Field>(f: &[F], u: F) -> Vec<F> { | |
| let d = f.len(); | |
| let mut h = vec![F::zero(); d]; | |
| for i in (1..d).rev() { | |
| h[i - 1] = f[i] + h[i] * u; | |
| } | |
| h | |
| } | |
| pub(crate) fn compute_witness_polynomial<F: Field>(f: &[F], u: F) -> Vec<F> { | |
| let d = f.len(); | |
| let mut h = vec![F::zero(); d - 1]; | |
| for i in (1..d).rev() { | |
| let h_i = if i < d - 1 { h[i] } else { F::zero() }; | |
| h[i - 1] = f[i] + h_i * u; | |
| } | |
| h | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
* feat(jolt-sumcheck): add fuzz crate for verifier panic coverage Adds a `fuzz/` sub-workspace to `jolt-sumcheck` with two libFuzzer targets, matching the pattern already established for `jolt-crypto`, `jolt-field`, `jolt-poly`, and `jolt-transcript`. ## Targets * `sumcheck_verifier` — drives `SumcheckVerifier::verify` with attacker- controlled `SumcheckClaim` (num_vars 0..=8, degree 1..=6, arbitrary claimed_sum) and a fuzzer-chosen sequence of `UnivariatePoly<Fr>` round polynomials whose lengths are independently controlled per round (0..=degree+1). Asserts the verifier returns `Ok(_)` or a typed `SumcheckError` and never panics. * `batched_sumcheck_verifier` — same panic-guard for `BatchedSumcheckVerifier::verify`, exercising the front-loaded batching path: variable `num_claims` (including 0, which must surface as `EmptyClaims`), per-claim `num_vars` mismatches, and `mul_pow_2` scaling. ## CI * Adds `jolt-sumcheck` to the matrix in `.github/workflows/fuzz-crates.yml`. ## Why `jolt-sumcheck` is verifier-only and protocol-critical: a panic on a malformed proof would let a malicious prover crash the verifier rather than receive a clean rejection. This is the same threat model that PR #1408 hardened in `jolt-core` (`catch_unwind` around the verifier) and that `verify_tampered` in `jolt-dory` and the `tampered_proof` target in `jolt-hyperkzg` (#1488) cover for the PCS layer. The existing unit tests in `crates/jolt-sumcheck/src/tests.rs` cover honest-prover soundness; this crate covers the dual property — adversarial-input robustness — with coverage-guided exploration. ## Local validation * `cargo +nightly fuzz build` (apple-darwin) — clean * `cargo +nightly fuzz run sumcheck_verifier -- -max_total_time=15` — 2.66M runs, no crashes * `cargo +nightly fuzz run batched_sumcheck_verifier -- -max_total_time=15` — 3.06M runs, no crashes * `cargo nextest run -p jolt-sumcheck` — 40/40 pass The fuzz sub-workspace re-applies the root workspace's `[patch.crates-io]` for the arkworks fork, mirroring the existing `jolt-transcript/fuzz` and `jolt-crypto/fuzz` setup, since fuzz sub-workspaces do not inherit the root workspace patches. * fuzz(jolt-sumcheck): add valid_prefix_proof target Address review feedback on PR #1493 from @0xAndoroid: extend the verifier panic-guard fuzzing to cover proofs whose first `K` round polynomials are constructed to satisfy the sum-check invariant, so the verifier proceeds past round 0 and exercises the Fiat-Shamir transcript chain end-to-end. The existing `sumcheck_verifier` and `batched_sumcheck_verifier` targets feed raw fuzz bytes; most iterations fail the verifier's round-0 sum check and return `Err` early, leaving every later `append_to_transcript` / `challenge` / `evaluate` call uncovered. `valid_prefix_proof` lets the fuzzer pick: - `num_vars` (0..=8), `degree` (1..=6), `claimed_sum` (any field elt) - `valid_rounds` ∈ [0, num_vars]: how many leading rounds are valid by construction For valid rounds, the target reads `c_0` and `c_2 .. c_d` from the fuzz stream and derives `c_1` from the sum-check invariant `s(0) + s(1) = running_sum`, mirroring the standard compressed-unipoly format. It then mirrors the verifier's `append_to_transcript / challenge / evaluate` pipeline prover-side so the running sum the verifier will check against is known. Tail rounds (after `valid_rounds`) use raw random bytes as before. Invariants: - `verify` must never panic on any fuzzer input. - When `valid_rounds == num_vars` the proof is fully valid by construction, so `verify` MUST return `Ok(EvaluationClaim)` whose `value` equals the prover-side running sum and whose `point` length equals `num_vars`. Local validation: - `cargo +nightly fuzz build` clean - `cargo +nightly fuzz run valid_prefix_proof -- -max_total_time=15` — 742k runs, no crash - `cargo +nightly fuzz run sumcheck_verifier -- -max_total_time=8` — 1.46M runs, no crash (regression-checked) - `cargo +nightly fuzz run batched_sumcheck_verifier -- -max_total_time=8` — 1.22M runs, no crash (regression-checked) - `cargo nextest run -p jolt-sumcheck` — 42/42 pass - `cargo clippy -p jolt-poly --all-targets -- -D warnings` clean --------- Co-authored-by: wstran <wstran@Wilson-Tran.local>
0xAndoroid
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
2 times, most recently
from
43a2e35 to
bcb566e
Compare
Introduces jolt-hyperkzg, a pairing-based multilinear polynomial commitment scheme implementing the jolt-openings trait surface. Generic over `P: PairingGroup`; BN254 is the production instantiation. Contents: - scheme.rs: HyperKZGScheme implementing CommitmentScheme and AdditivelyHomomorphic; prove/verify built on KZG univariate openings - kzg.rs: univariate KZG primitives (commit, evaluate, witness polynomial division) used internally by the multilinear protocol - types.rs: HyperKZGCommitment, HyperKZGProof, HyperKZGProverSetup, HyperKZGVerifierSetup - error.rs: HyperKzgError enum
markosg04
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
from
bcb566e to
65f7570
Compare
| let mut polys = Vec::with_capacity(ell); | ||
| polys.push(evals.to_vec()); | ||
|
|
||
| for i in 0..ell - 1 { |
There was a problem hiding this comment.
Integer underflow when ell = 0. If a 0-variable polynomial is passed (where point.len() = 0), the expression 0..ell - 1 becomes 0..usize::MAX due to wraparound, causing either a hang or memory exhaustion.
for i in 0..ell.saturating_sub(1) {
// ...
}Alternatively, add an assertion at the start of the function:
assert!(ell > 0, "polynomial must have at least 1 variable");| for i in 0..ell - 1 { | |
| for i in 0..ell.saturating_sub(1) { |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
|
Claude code review session started: https://claude.ai/code/session_01MF57kXRHWCn4a7oeMifTUc |
moodlezoup
left a comment
moodlezoup
left a comment
There was a problem hiding this comment.
Protocol math (folding consistency + 3-point batched KZG pairing) checks out, and the dimension validation before transcript mutation is good. A few items below — mostly perf in hot paths plus one trapdoor-leakage hardening for setup_from_secret.
Generated by Claude Code
| h[i - 1] = f[i] + h[i] * u; | ||
| } | ||
| h | ||
| } |
There was a problem hiding this comment.
h[d-1] is always 0 (the loop writes h[0..=d-2]), but the returned vec has length d and the caller at line 109 MSMs &setup.g1_powers[..h.len()] over the trailing zero. The MSM backend (arkworks msm_bigint) still pays the into_affine/into_bigint conversion for that term. Return a length-d-1 vec and slice the SRS accordingly — three witness commits per open in a hot path.
Generated by Claude Code
| fn combine(commitments: &[Self::Output], scalars: &[Self::Field]) -> Self::Output { | ||
| assert_eq!(commitments.len(), scalars.len()); | ||
| let combined = commitments | ||
| .iter() | ||
| .zip(scalars.iter()) | ||
| .map(|(c, s)| c.point.scalar_mul(s)) | ||
| .fold(P::G1::identity(), |acc, x| acc + x); | ||
| HyperKZGCommitment { point: combined } | ||
| } |
There was a problem hiding this comment.
Naive scalar_mul + fold here loses to P::G1::msm(points, scalars) for any non-trivial commitments.len(). The rest of the crate (kzg.rs:26, :109, :196) uses msm directly.
| fn combine(commitments: &[Self::Output], scalars: &[Self::Field]) -> Self::Output { | |
| assert_eq!(commitments.len(), scalars.len()); | |
| let combined = commitments | |
| .iter() | |
| .zip(scalars.iter()) | |
| .map(|(c, s)| c.point.scalar_mul(s)) | |
| .fold(P::G1::identity(), |acc, x| acc + x); | |
| HyperKZGCommitment { point: combined } | |
| } | |
| fn combine(commitments: &[Self::Output], scalars: &[Self::Field]) -> Self::Output { | |
| assert_eq!(commitments.len(), scalars.len()); | |
| let bases: Vec<P::G1> = commitments.iter().map(|c| c.point).collect(); | |
| HyperKZGCommitment { | |
| point: P::G1::msm(&bases, scalars), | |
| } | |
| } |
Generated by Claude Code
| let mut polys = Vec::with_capacity(ell); | ||
| polys.push(evals.to_vec()); | ||
|
|
||
| for i in 0..ell - 1 { |
There was a problem hiding this comment.
for i in 0..ell - 1 underflows usize when ell == 0. The trait-level commit/open API makes this hard to reach in normal use (a 0-var poly has 1 eval), but the public HyperKZGScheme::open(setup, evals, point, ...) is also pub and would crash on an empty point. Either return HyperKZGError::InvalidProof up front (this function's caller already returns Result) or guard with an early if ell == 0 return so a malformed caller doesn't panic.
Generated by Claude Code
| impl<P: PairingGroup> DeriveSetup<HyperKZGProverSetup<P>> for PedersenSetup<P::G1> { | ||
| fn derive(source: &HyperKZGProverSetup<P>, capacity: usize) -> Self { | ||
| assert!( | ||
| source.g1_powers.len() > capacity, | ||
| "SRS has {} G1 powers, need at least {} (capacity + 1 for blinding)", | ||
| source.g1_powers.len(), | ||
| capacity + 1, | ||
| ); | ||
| let message_generators = source.g1_powers[..capacity].to_vec(); | ||
| let blinding_generator = source.g1_powers[capacity]; | ||
| PedersenSetup::new(message_generators, blinding_generator) | ||
| } | ||
| } |
There was a problem hiding this comment.
Using KZG SRS powers as Pedersen generators couples Pedersen binding to the same trapdoor beta that breaks KZG binding — once beta is destroyed both are sound, but you lose defense-in-depth (a typical Pedersen setup uses independent hash-to-curve generators). Worth a sentence in the doc here so an integrator doesn't assume the two schemes have independent security.
Generated by Claude Code
| VerificationFailed, | ||
|
|
||
| #[error("invalid proof structure: {0}")] | ||
| InvalidProof(&'static str), |
There was a problem hiding this comment.
InvalidProof(&'static str) and the context-free VerificationFailed lose information that would help debugging. The three call sites in scheme.rs:159-178 are semantically distinct (wrong com length, wrong v shape, wrong w length), and VerificationFailed covers both "folding consistency failed at level i" and "pairing check failed" — those have very different implications when a downstream regression triggers them. Consider splitting into typed variants.
Generated by Claude Code
| // Absorb witness commitments and derive one more challenge to keep | ||
| // prover/verifier transcripts in sync | ||
| for wi in &w { | ||
| transcript.append(wi); | ||
| } | ||
| let _: P::ScalarField = transcript.challenge(); |
There was a problem hiding this comment.
Naming this _ (and the "keep transcripts in sync" framing) obscures that this is the verifier's d_0. The prover doesn't use it, but it must advance the transcript identically to the verifier (kzg.rs:161).
| // Absorb witness commitments and derive one more challenge to keep | |
| // prover/verifier transcripts in sync | |
| for wi in &w { | |
| transcript.append(wi); | |
| } | |
| let _: P::ScalarField = transcript.challenge(); | |
| // Absorb witness commitments and mirror the verifier's `d_0` challenge | |
| // to keep prover/verifier transcripts in sync. | |
| for wi in &w { | |
| transcript.append(wi); | |
| } | |
| let _d_0: P::ScalarField = transcript.challenge(); |
Generated by Claude Code
| } | ||
|
|
||
| let mut tampered = proof.clone(); | ||
| tampered.v[tamper_row][tamper_col] = tamper_val; |
There was a problem hiding this comment.
Fuzz only perturbs proof.v. A target that tampers proof.com[i] or proof.w[i] (e.g., scalar-multiplying by a fuzzer-supplied value) would exercise the pairing-check side of the verifier, which is currently uncovered.
Generated by Claude Code
moodlezoup
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
from
cc3a810 to
e5ea5bb
Compare
- Return d-1 elements from compute_witness_polynomial to avoid wasted zero-coefficient MSM work in the hot path - Use MSM in AdditivelyHomomorphic::combine instead of naive scalar_mul+fold - Guard against ell==0 underflow in HyperKZGScheme::open - Split HyperKZGError into typed variants (FoldingConsistencyFailed, PairingCheckFailed, DegenerateChallenge, dimension-specific errors) for better debuggability - Rename prover transcript challenge to _d_0 to clarify it mirrors the verifier's d_0 - Document shared KZG/Pedersen trapdoor in DeriveSetup impl - Expand fuzz target to tamper proof.com and proof.w fields, exercising the pairing-check side of the verifier Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
moodlezoup
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
from
e5ea5bb to
76c5b22
Compare
…fields
Replace Vec with [T; 3] for the three evaluation points {r, -r, r²}
throughout the HyperKZG crate. The proof struct now uses [P::G1; 3]
for witness commitments and [Vec<P::ScalarField>; 3] for evaluation
rows, enforcing the length invariant at the type level.
- Serde rejects wrong-length proofs at deserialization
- Removed WrongEvaluationRowCount and WrongWitnessCount error variants
- Removed malformed_witness_commitments test (type system enforces it)
- Switched kzg_open_batch from par_iter to [T;3]::map (3 elements)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
moodlezoup
force-pushed
the
jolt-v2/jolt-hyperkzg
branch
from
fba5e43 to
73a1f47
Compare
3 participants
Summary
jolt-hyperkzg, a pairing-based multilinear polynomial commitment scheme implementing thejolt-openingstrait surfaceP: PairingGroup; BN254 is the production instantiationChanges
src/scheme.rs—HyperKZGSchemeimplementingCommitmentSchemeandAdditivelyHomomorphic; prove/verify built on KZG univariate openingssrc/kzg.rs— univariate KZG primitives used internallysrc/types.rs—HyperKZGCommitment,HyperKZGProof,HyperKZGProverSetup,HyperKZGVerifierSetupsrc/error.rs—HyperKzgErrorenumtests/commit_open_verify.rs— end-to-end commit/open/verify coveragebenches/hyperkzg.rs— perf benchmarksfuzz/— three fuzz targets:commit_open_verify,tampered_proof,wrong_evalTesting
cargo nextest run -p jolt-hyperkzg