feat: commit the R1CS witness with zook - #485
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CSP benchmarks
Prover time, peak RSS, peak heap, and verifier time are arithmetic means across the iterations. Peak heap comes from the largest No baseline available yet — deltas will appear once this workflow has produced at least one successful Results
|
Collects a slice of concrete covectors into the `Vec<&dyn LinearForm<F>>` shape whir's opening APIs take. The prover and verifier both build this vector by hand at several call sites; a shared helper removes the repetition and keeps the trait-object cast in one place.
Moves the `whir` pin from 33fbecf to 8804e80 (worldfnd/whir, PR #270). That rev replaces the `Messages` + separate-mask Reed-Solomon interface with segmented `Polynomials`, and adds the `zook` protocol module the following commits switch the witness commitment over to. Also enables serde_json's `float_roundtrip` feature: zook's `KneeWeight` and `ListSize` wrap `OrderedFloat<f64>`, and the default fast float parser is off by one ULP on read-back, which breaks scheme JSON roundtrips. Cargo.lock additionally picks up a `zeroize` edge for whir and a batch of incidental `windows-sys` version moves from re-resolving the graph; none of those affect the build on our targets.
`WhirR1CSScheme::whir_witness` becomes a zook `ProtocolConfig` derived from an explicit `SecuritySpec`/`TuningSpec` pair rather than a plain `whir::Config`. The tuning knobs move to named constants: `Mode::Standard` for the commitment mode, and `RateSchedule::Adaptive` at the default 0.5 knee, which plans per-round inverse rates against the prover-time / proof-size pareto knee instead of stepping the rate unboundedly. `ProtocolConfig::derive` is fallible, so `new_for_r1cs`, `new_from_dimensions` and `new_witness_config_for_size` now return `anyhow::Result`, and the two dimension invariants become `ensure!` instead of `assert!` — library code should not panic on caller error. The blinding commitment stays a plain `whir::Config`: the Spartan mask `g` is extension-valued and does not go through the base-field zook path.
`WhirR1CSCommitment::witness` holds a zook `CommittedWitness`, and the padded witness length now comes from the config's `tuning().vector_size` rather than `initial_num_variables()`. zook's `commit` consumes the buffer but the prove stage still needs the message for the covector evaluations, so the padded witness is cloned; an upstream `commit(&Buffer)` or a message accessor would remove that. Both the single- and dual-commitment opening paths hand zook borrowed linear forms via `linear_form_refs` instead of boxing each covector, and the SPARK covector snapshots go away with them: zook's `prove` returns `()`, so there is no final evaluation point to re-evaluate the alpha weights at.
zook's `verify` takes the linear forms and their claimed evaluations up front and returns a `FinalClaim` whose `verify` now takes a slice rather than an iterator. Its sumcheck also draws extension-field randomness, so the impl picks up a `Standard: Distribution<Ext<P>>` bound. All three weight-reference sites switch to `linear_form_refs`.
A SPARK query needs the final WHIR evaluation point to re-evaluate the A, B, C covectors at. zook's `prove` returns `()` and keeps that point internal, so the query cannot be built. Rather than silently emitting nothing, both the generic and the Mavros prove paths now fail fast with an explanatory error, and `ProveFromAlphasCtx::spark_row` is documented as reserved. Restoring this needs a whir-side change that exposes the prover's final claim; the call sites are marked with a TODO pointing at it.
Scheme construction can now fail while deriving the zook config, so the Noir and Mavros compilers propagate the error and `MavrosSchemeBuilder` returns `anyhow::Result<Self>`. `assert_configs_secure` no longer reads a security level off the witness config — zook owns that calculation. It runs zook's own `validate` and asserts the target stays at 128 bits. The blinding half still checks `security_level` directly, since it remains a plain whir config.
`prove_setup` returns `Result<ProveInputs<P>>` and the harness builders propagate the scheme-construction error.
The gnark exporter serialises a plain whir `Config`; it has no way to describe zook's `ProtocolConfig`, and the Go recursive verifier would have to be updated in lockstep to consume one. Until that paired change lands, both entry points — the `generate-gnark-inputs` CLI command and the verifier server's parameter preparation — fail with an error that says so, and `provekit-gnark` drops out of the two crates' dependencies so `cargo machete` stays quiet. Also drops two `return` statements from the verifier server's cancellation branches; they are the last statement in their `select!` arm and clippy flags them.
zook's `prove` now returns a `ProverClaim` carrying the final evaluation point and RLC coefficients (whir #274), which is what SPARK query production needs.
whir #274 makes zook's `prove` return a `ProverClaim` with the final evaluation point, so the A, B, C covectors can again be re-evaluated at the final WHIR point to form the SPARK queries. Restores the same query assembly as the pre-zook prover (selector-prefixed columns in the dual- commitment path) and drops the fail-fast guards from the generic and Mavros prove paths.
BornPsych
force-pushed
the
ys/zook-witness-commitment
branch
from
August 27, 2026 15:53
cefd4c8 to
4b58922
Compare
BornPsych
marked this pull request as ready for review
August 27, 2026 15:53
zkfriendly
requested changes
Aug 31, 2026
zkfriendly
left a comment
zkfriendly
left a comment
Collaborator
There was a problem hiding this comment.
Changes look good and correct. One thing I'd like to discuss briefly before merging is the required Zook configs. Right now, this PR hardcodes the config as consts, but Zook takes it as user-provided input; should we also take it as input? Maybe via the cli arguments, and we could have the default return the currently hardcoded values?
|
|
||
| /// Zook mode for the witness commitment; [`Mode::ZeroKnowledge`] makes it | ||
| /// hiding. | ||
| const ZOOK_MODE: Mode = Mode::Standard; |
Collaborator
There was a problem hiding this comment.
Should we have ZOOK_MODE as a const or a dynamic as a cli flag or prover config? my question is do we need any other protocol change if we set ZOOK_MODE = ZeroKnowledge?
Collaborator
There was a problem hiding this comment.
Same question for the rate and security bits and other protocol params that whir takes as config
Collaborator
Author
There was a problem hiding this comment.
good suggestion, I’ve made ZK the default, with --witness-mode standard as the opt-out for smaller non-hiding proofs. The config is already serialized into the scheme/domain separator, so the verifier follows whatever mode prepare used.
The witness commitment mode was pinned to `Mode::Standard` by a private `ZOOK_MODE` const, so a hiding commitment meant editing the source and rebuilding. It is now a `WitnessCommitmentMode` argument carried alongside `hash_config` from the frontends down to the zook `SecuritySpec`, selected by `prepare --witness-mode standard|zk`. The mode lands in the derived `ZookConfig`, which is already part of the scheme and therefore of both key files and the Fiat-Shamir domain separator, so `prove` and `verify` follow whatever `prepare` chose without a flag of their own. The rate schedule, security level and proof-of-work budget stay as consts: they are the security floor rather than a per-circuit choice. No behaviour change - the default is still `Mode::Standard`.
`Mode::ZeroKnowledge` round-trips on the pinned whir rev, so the witness commitment is now hiding unless `prepare --witness-mode standard` asks for the smaller non-hiding proofs. Proofs grow. On the example circuits the hiding proof is roughly 5x larger for `basic-2`, 3.3x for `basic` and 2.1x for `rangechecks`, where the fixed mask overhead dominates. Larger circuits are not measured yet. A hiding witness commitment is still not full proof ZK - the alpha, public-input and challenge evaluations are sent in the clear.
zkfriendly
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switches the R1CS witness commitment from plain WHIR to zook (
RateSchedule::Adaptive,Mode::Standard), and bumps whir to78e9460.On the passport circuit this is ~37% off the R1CS proving core and ~17% off total prove time, for ~26% larger proofs.
SPARK query production works under zook: worldfnd/whir#274 makes zook's
provereturn its final evaluation point and RLC coefficients (ProverClaim), and the SPARK queries are assembled from it exactly as before the switch. Verified end-to-end (prepare --spark→prove --produce-spark-query→prove-spark→verify-spark) on a single-commitment circuit and a dual-commitment (range-check) circuit.Notes:
provewhir#274 (still open); swap to a whirmainrev once it merges.