feat: add Jolt witness crate#1604
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. |
9df1b17 to
aab0fc9
Compare
f3b8924 to
57390c6
Compare
|
Claude code review session started: https://claude.ai/code/session_01CAbeAZB8bnAxaepDkaBMKN |
|
Stack maintenance — rebased onto #1597 ( 📚 Stack A: #1596 → #1597 → #1598 → #1599 → #1600 → #1601 → #1602 → #1603 #1596 and #1597 are shared by both stacks; each PR targets the one before it. Conflict resolution
Local validation
|
moodlezoup
left a comment
There was a problem hiding this comment.
Reviewed the jolt-witness crate that this PR actually introduces. To get a tractable diff I reviewed the incremental range 81a7c62..57390c6 (the 6 witness commits) rather than the full GitHub diff — heads up that the PR base is set to prover-stack/02 while the description says 08, which inflates the displayed diff to 251 files / 64k lines. Re-targeting the base to 08 would make this reviewable.
No soundness issues. The booleanity address/cycle phase split is constraint-preserving (the combined output is reused verbatim, the degree-3 bound and round counts partition correctly, and the intermediate claim is bound across phases via the shared BooleanityAddrClaim opening). The crate is prover-side only, with no unsafe, no attacker-reachable panics, and dimension/overflow guards (checked_pow2, power_of_two_log_rows) used consistently.
Two latent correctness divergences flagged inline — both currently dormant (their stageN_rows have no consumer yet) but will produce wrong/malformed witness once wired into the prover.
Lower-priority notes (non-blocking):
- Missing-PC handling diverges across paths.
stage6.rs:39(pc_for_row_optional(...).unwrap_or(0)) andstreams.rs:493(get_pc(...)→ silent non-hot row) silently swallow a missing bytecode PC for a real instruction, whereas the fast pathra.rs::bytecode_pc_for_rowerrors viamissing_pc_mapping. Unreachable for valid traces (every executed instruction is mapped; NoOp→Some(0)), so defensive consistency rather than a live bug — but worth unifying on the erroring path. - Speculative surface.
MaterializationPolicy::Streamingis never constructed, and neitherViewRequirement.materializationnor.retentionis read by any non-test logic (PolynomialView::Deferredis only constructed in tests). Per the repo's "abstractions must pay for themselves within the PR" guidance, consider trimming until a consumer exists — though this is forward-looking API for the stack, so reasonable to keep if a near-term consumer is planned. - Duplication of jolt-core.
RaChunkSelector(ra.rs),JoltVmIncrementStreamKind::value_from_row(streams.rs:452), and the one-hot/compact expansion instreams.rsre-deriveOneHotParamschunk math andwitness.rs::stream_witness_and_commit_rowsincrement logic. Presumably intentional for the modular-crate migration; worth a note in the PR that the jolt-core versions are slated for deletion so this doesn't become permanent duplication. - Perf (when wired up).
materialize_ram_val/materialize_ram_ra/RegistersVal are sequential O(K·T) loops; jolt-core parallelizes the equivalent generation.field_inlinealready uses rayon, so the gap is confined to the dense-materialization fallbacks.
Generated by Claude Code
57390c6 to
236ec22
Compare
91dbcf2 to
62aab06
Compare
236ec22 to
bdf3b33
Compare
… remap Addresses review feedback on #1604: - stage2: next_is_noop used is_none_or (-> true at the final cycle), inverting the boundary relative to the canonical is_some_and used by every other path (jolt-core r1cs/inputs.rs, spartan_outer.rs, trace.rs). It feeds ShouldJump = Jump*(1 - NextIsNoop), so the boundary value was inverted. Switch to is_some_and. - stage6: remapped_ram_address was computed inline, skipping the >= ram_k bound that the shared remapped_ram_address() helper (ram.rs) enforces and that stage2 and every ram.rs path route through. An out-of-range remap would index past the ram_k one-hot domain. Route through the helper, keeping the ram_address binding (still needed for ram_access_nonzero). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce the jolt-witness crate: the witness-generation layer producing committed and virtual polynomials for the Jolt VM, dory-assist, wrapper, and blindfold protocols. Consolidates follow-up refactors: - Split the jolt_vm witness module by stage and polynomial family (spartan_outer, stage2/3/6, registers, ra, ram, trace, streams, provider). - Share require_unique_ids / power_of_two_log_rows helpers across the wrapper and dory_assist providers via protocols/util.rs. - Tighten the core witness data-access API: log_rows-only WitnessDimensions, ViewRequirement-based oracle_view, and removal of dead encodings, policies, and builder indirection. - Document the core witness vocabulary types. - Restore the 1604 workspace build.
… remap Addresses review feedback on #1604: - stage2: next_is_noop used is_none_or (-> true at the final cycle), inverting the boundary relative to the canonical is_some_and used by every other path (jolt-core r1cs/inputs.rs, spartan_outer.rs, trace.rs). It feeds ShouldJump = Jump*(1 - NextIsNoop), so the boundary value was inverted. Switch to is_some_and. - stage6: remapped_ram_address was computed inline, skipping the >= ram_k bound that the shared remapped_ram_address() helper (ram.rs) enforces and that stage2 and every ram.rs path route through. An out-of-range remap would index past the ram_k one-hot domain. Route through the helper, keeping the ram_address binding (still needed for ram_access_nonzero). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
98b6a4c to
70f872d
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Part of the draft Jolt prover stack.
Base:
prover-stack/08-wrapper-verifierHead:
prover-stack/09-jolt-witnessAdds the standalone Jolt witness crate and protocol-specific witness providers.
Validation before submission:
cargo fmt -q -- --checkcargo metadata --no-deps --format-version 1gh stack pushsimulation against a temporary bare remotehandoffs/, oldSTACK.md,stack/, and old stack workflow