feat: add Dory assist verifier#1602
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. |
0xAndoroid
force-pushed
the
prover-stack/06-pcs-assist-dory-openings
branch
from
aa8e52f to
fd5609e
Compare
0xAndoroid
force-pushed
the
prover-stack/07-dory-assist-verifier
branch
from
6b1d13b to
9775090
Compare
|
Stack maintenance — rebased onto #1601 and retargeted this PR's base to 📚 Stack A: #1596 → #1597 → #1598 → #1599 → #1600 → #1601 → #1602 (this PR) → #1603 #1596 and #1597 are shared by both stacks; each PR targets the one before it. Conflict resolution
|
|
Claude code review session started: https://claude.ai/code/session_01LxnYiZdLGeHZwV8QSH7NT8 |
moodlezoup
left a comment
moodlezoup
left a comment
There was a problem hiding this comment.
Reviewed the new jolt-dory-assist-verifier crate (4 parallel passes: soundness, bugs, semantic consistency, simplification). Implementation hygiene is strong — DoS bounds (point_len <= 64 enforced before any sized allocation), Fiat-Shamir ordering (forked_fq_challenge correctly used only for non-advancing peeks; the binding digest is squeezed from the real transcript), inject_fr_to_fq/decode_native_final_fr injectivity, artifact layout offsets vs. consumers, identity-fallback element counts, and reduce-round indexing all check out. No reachable panics/unwraps on the deserialization/validation path.
Main item is a binding question (inline on native_final.rs): the live verifier trusts prover-supplied native_final.inputs / dense_commitment / packed_point, while the replay-based ground-truth functions that would anchor them to the real Dory opening exist but are test-only. Likely fine if the surrounding verifier anchors these later in the stack, but worth confirming since soundness rests on it.
One process note that isn't tied to a diff line: crates/jolt-dory-assist-verifier is not in the root Cargo.toml members list, and no member crate depends on it. cargo build -p jolt-dory-assist-verifier fails with "did not match any packages", so workspace CI (cargo clippy --all, cargo nextest) never compiles or tests this crate — which lines up with the PR's validation list (fmt + metadata only, no build/clippy/test). If membership is intentionally deferred to a later stack PR, fine; otherwise add it so CI exercises the crate.
Remaining inline comments are simplification/consistency cleanups (claim-struct hierarchy, duplicated artifact helpers, dead error-stage variants).
Generated by Claude Code
| let setup = input.setup.artifacts(); | ||
| let proof = input.pcs_proof; | ||
| let final_artifacts = proof.final_artifacts(); | ||
| let state = native_final_input_state(native_final_inputs)?; |
There was a problem hiding this comment.
The live native-final check consumes prover-supplied state (native_final.inputs from the proof) rather than recomputing it. transparent_replayed_final_pairing_check / transparent_native_final_input_claims (and the zk twins) right below compute the ground-truth state by replaying the real Dory reduce rounds against input.setup/input.pcs_proof/input.commitment — but those are only ever called from #[cfg(test)] code, never from verify_clear/verify_zk.
So the only thing tying native_final.inputs to the actual Dory opening is the final pairing equation plus the stage sumcheck chain, and those in turn reduce to opening-claim values that are themselves prover-chosen (stage 3's dense_commitment and packed_point are read verbatim from the proof and only checked for self-consistency against the claim fold, not anchored to any verifier-fixed commitment or to the per-relation sumcheck points). That leaves room for a prover to pick a set of claim values satisfying the algebraic constraints without corresponding to the real witness.
Is the external anchoring meant to come from the surrounding Jolt verifier later in the stack, or should the live path call the *_replayed_* functions and assert native_final.inputs == native_final_input_claims(replayed_state)? If the latter, this is the binding that makes the sumcheck reduction sound. Worth confirming the intent given this is WIP.
Generated by Claude Code
| } | ||
|
|
||
| #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct DoryAssistGtExponentiationClaims { |
There was a problem hiding this comment.
This file defines ~35 DoryAssist*Claims structs, each with named Fq fields plus a hand-written opening_claim(&DoryAssistOpeningId) -> Option<Fq> match arm. Production code never reads the named fields — everything is resolved by ID through the recursive opening_claim(id)/public_claim(id) dispatch. The crate already has the simpler form a few hundred lines down: DoryAssistDoryReduceClaims (line 63 / ~1209) stores claims as Vec<DoryAssistOpeningClaim> ({id, value}) and resolves with .find(|c| c.id == *id). Since lookup is always by DoryAssistOpeningId/DoryAssistPublicId, this whole hierarchy could collapse to an ID-keyed map (or the existing Vec form), deleting ~1000 lines of structs + match arms + the *_polynomial ID-projection helpers and removing the "add a field, add a match arm, keep them in sync" coupling. The named form does buy compile-time exhaustiveness, but given the repo's lean-code stance and the in-module precedent it doesn't seem to earn its size.
Generated by Claude Code
| fn gt_artifact_coefficients(value: &Bn254GT) -> [Fq; GT_ARTIFACT_COEFFS] { | ||
| let mut coefficients = [Fq::default(); GT_ARTIFACT_COEFFS]; | ||
| coefficients[..Bn254GT::FQ12_COEFFICIENTS].copy_from_slice(&value.fq12_coefficients()); | ||
| coefficients | ||
| } | ||
|
|
||
| fn g1_artifact_coordinates(value: Bn254G1) -> [Fq; G1_ARTIFACT_COORDS] { | ||
| value.affine_coordinates_with_infinity() | ||
| } | ||
|
|
||
| fn g2_artifact_coordinates(value: Bn254G2) -> [Fq; G2_ARTIFACT_COORDS] { | ||
| value.affine_coordinates_with_infinity() | ||
| } |
There was a problem hiding this comment.
gt_artifact_coefficients, g1_artifact_coordinates, and g2_artifact_coordinates are byte-for-byte identical to the copies in verifier.rs:604-616. Both modules already import the *_ARTIFACT_* width constants from artifacts.rs — hoist these three helpers there and reuse, rather than keeping two copies.
Generated by Claude Code
| pub enum DoryAssistStage { | ||
| CheckedInputs, | ||
| Stage1, | ||
| Stage2, | ||
| Stage3, | ||
| HyraxOpening, | ||
| NativeOutput, | ||
| } |
There was a problem hiding this comment.
Only Stage1/Stage2/Stage3 are ever attached to an error. CheckedInputs, HyraxOpening, and NativeOutput are never used to tag a variant — checked-input failures use CheckedInputMismatch (no stage), Hyrax failures use HyraxOpeningFailed (no stage), native-output failures use PublicOutputMismatch (no stage). These three only appear in the Display stability test, so a consumer switching on stage gets nothing for those failures. Either wire them into the corresponding errors or drop them.
Generated by Claude Code
0xAndoroid
force-pushed
the
prover-stack/06-pcs-assist-dory-openings
branch
from
dfaf326 to
8af0b23
Compare
0xAndoroid
force-pushed
the
prover-stack/07-dory-assist-verifier
branch
from
9775090 to
bc2d345
Compare
3 participants
Part of the draft Jolt prover stack.
Base:
prover-stack/06-pcs-assist-dory-openingsHead:
prover-stack/07-dory-assist-verifierAdds the concrete Dory assist verifier crate and its completeness/soundness fixtures.
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