spec: port jolt-transcript to spongefish#1455
Conversation
|
Claude spec review session started: https://claude.ai/code/session_011hNRFc6sPWLP97RESoU3kV |
|
Spec Analysis: Port jolt-transcript to spongefish
Status: Questions remain — 5 ambiguities to resolve before one-shot implementation. The spec is in strong shape overall. Goal, abstractions, acceptance checklist, and alternatives are precise; file:line citations throughout make the scope easy to trace. Ambiguities cluster around (a) the proof wire format's non-NARG tail, (b) the challenge-width contract for Poseidon, and (c) pass/fail thresholds that the implementer would otherwise have to guess. Questions: 1. [Goal] 2. [Constraints] 3. [Constraints] The spec flags the CI treatment of 4. [Success Criteria] The performance bar reads "no observable regression beyond benchmark noise on
5. [Context] Minor — the Architecture section says
Generated by Claude Code |
shreyas-londhe
force-pushed
the
spec/jolt-transcript-spongefish
branch
from
3779d1e to
ea626dc
Compare
We don't have any Solidity integration tests. With regard to the transpiler tests, I think we should keep the jolt-core transcript implementation in place for now. We can deal with activating the new |
Thanks for the context, @moodlezoup. I've tailored the spec to a narrower goal — internal port of Ready for re-analysis when convenient. |
|
Claude spec review session started: https://claude.ai/code/session_01KnxCordRFfR2gGNymiRw3F |
Spec Analysis
Status: Approved — the spec is clear enough for a one-shot implementation. Summary of what will be built
Key invariants
Critical evaluation criteria
Validation of spec anchors against HEAD
Minor, non-blocking notes for the implementer (judgement calls, not ambiguities that would block a one-shot impl)
Next step: Run Generated by Claude Code |
shreyas-londhe
force-pushed
the
spec/jolt-transcript-spongefish
branch
from
ccb930b to
848ebb1
Compare
Concrete field type (ark_bn254::Fr) named for all decoders. Compat facade trait bound requirements (Default + Clone + Sync + Send + 'static) made explicit. Test command gains feature flags for all three sponges. Invariant struct shape clarified as three concrete structs (not generic) matching the JoltInvariants dispatch pattern. JoltInvariants enum update added to jolt-eval acceptance criteria.
Vishalkulkarni45
force-pushed
the
spec/jolt-transcript-spongefish
branch
from
beeff40 to
d90cda5
Compare
shreyas-londhe
force-pushed
the
spec/jolt-transcript-spongefish
branch
from
af59719 to
d90cda5
Compare
Reasons: - Establish spongefish-native ProverTranscript/VerifierTranscript split surface on top of spongefish::ProverState / VerifierState so future jolt-core, dory, and gnark transpiler migrations land on a single Fiat-Shamir construction owned by this crate. - Replace the hand-rolled digest-based transcript with a duplex sponge to get NARG byte strings, DomainSeparator-based protocol-id/session/ instance binding, and check_eof rejection of trailing proof bytes. - Add a Circom-compatible BN254 Poseidon sponge (rate-2 capacity-1 over light-poseidon::Poseidon::new_circom(3), byte-driven via 31-byte LE chunks) so the gnark verifier follow-up can recompute the same challenge stream. Layout: flat crates/jolt-transcript/src/ (lib, domain, codec, prover, verifier, poseidon, compat). The compat module preserves the legacy Transcript / AppendToTranscript / Label / LabelWithCount / U64Word surface for jolt-sumcheck, jolt-openings, and jolt-crypto and routes Transcript::new(label) through the same DomainSeparator builder as the split traits. Per-sponge challenge-width contract: Blake2b and Keccak expose both a 128-bit-truncating OptimizedChallenge::challenge_128 and a full-field FieldEl decoder; PoseidonSponge has no OptimizedChallenge impl, so the 128-bit method on a Poseidon-backed state is a compile error. Known-vector tests pin the absolute Fr challenge value per backend for new(b"Jolt") + append_bytes(12345u64.to_be_bytes()) so any accidental encoding change (PROTOCOL_ID, session encoding, append_bytes layout, challenge decoder) flips the pinned bytes.
Reasons: - Provide an in-tree mechanical correctness gauge for the new jolt-transcript spongefish wiring, since jolt-core does not yet exercise the new ProverTranscript / VerifierTranscript surface and the muldiv e2e only touches the compat facade. - The differential check (prover/verifier pair driven by the same operation sequence must round-trip every prover_message and agree on every verifier_message) directly mechanises the sponge-symmetry invariant declared in the spec, and any future encoding regression in BytesMsg, FieldEl, or the DomainSeparator routing will surface here without waiting for a full e2e run. Adds three concrete Invariant structs (one per sponge) sharing an Op enum that covers PublicBytes, PublicScalar, ProverBytes, ProverScalar, and Challenge. Pattern follows SplitEqBindLowHigh / SplitEqBindHighLow since the #[invariant] macro does not support generics. Seed corpus covers empty, single-message per variant, 10-message mixed, and 1000-message mixed. Fuzz targets compile against libfuzzer-sys. Also fixes sync_targets.sh to (a) skip the macro_tests module and (b) match struct names containing digits (e.g. TranscriptConsistency Blake2bInvariant).
Adapt jolt-transcript spongefish port to upstream's recent changes: - jolt-field trait split (PR a16z#1484): blanket AppendToTranscript bound switches from `F: Field` to `F: CanonicalBytes`; Transcript challenge bound switches from `F: Field` to `F: TranscriptChallenge`; challenge decoder switches from `F::from_u128` to `F::from_challenge_bytes`. - Transcript trait gained `state() -> &[u8; 32]` and test-only `compare_to`. SpongeTranscript implements state() via a 32-byte peek from a sponge clone, cached on each absorb/squeeze so reads return a reference. compare_to is a no-op on the duplex-sponge facade. - Internal `domain` module renamed to `setup` so the now-public `jolt_transcript::domain` path can re-export `Label`, `LabelWithCount`, and `U64Word` (jolt-dory imports from this path on upstream main). - `light-poseidon` made an optional dependency gated by `transcript-poseidon`; `PoseidonSponge` / `PoseidonTranscript` are cfg-gated to match.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
- rustfmt: post-merge files reformatted (line-wrapping diffs in lib.rs, prover.rs, setup.rs, transcript_symmetry.rs). - taplo: 2-space indentation on Cargo.toml feature lists. - Cargo.lock: pin `generic-array` to 0.14.7 (matches upstream main's lock). The fresh resolution after the upstream merge pulled in 0.14.9, whose new `as_slice` deprecation tripped clippy's `-D warnings` against jolt-inlines-p256 even though that crate is unchanged. - transcript_symmetry.rs: expand the 3-arg `transcript_invariant!` macro into three explicit impls. rustfmt cycles indentation inside `#[doc = concat!(...)]` macro arms on each run, so the macro form failed `cargo fmt --check` deterministically; hand-written impls avoid the unstable rustfmt path.
moodlezoup
left a comment
moodlezoup
left a comment
There was a problem hiding this comment.
great work! just a couple of comments
| /// Empty `instance` value so the spongefish builder reaches the `to_prover` | ||
| /// stage. Encodes to zero bytes. |
There was a problem hiding this comment.
I'm not sure I understand –– are to_prover and to_verifier exclusively used for testing (to dump the transcript state)? If so, let's rename them to dump_prover_state and dump_verifier_state
There was a problem hiding this comment.
Not test-only — mirrors spongefish's own DomainSeparator::to_prover / to_verifier (we delegate at setup.rs:48,60). Used by jolt-eval/src/invariant/transcript_symmetry.rs and the planned jolt-core migration.
| /// Length-prefixed byte string. 8-byte LE length keeps `BytesMsg(a) ; BytesMsg(b)` | ||
| /// distinguishable from `BytesMsg(a||b)`. | ||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub struct BytesMsg(pub Vec<u8>); |
There was a problem hiding this comment.
Can we implement Encoding and NargDeserialize for Vec<u8> upstream, so we can get rid of this wrapper?
There was a problem hiding this comment.
Upstream = external spongefish crate. Wrapper enforces an 8-byte LE u64 length prefix so BytesMsg(a); BytesMsg(b) ≠ BytesMsg(a‖b) — a jolt-side choice.
@mmaker — thoughts on adding Encoding<[u8]> + NargDeserialize for Vec<u8> upstream?
There was a problem hiding this comment.
I agree with you and @moodlezoup. This wrapper should be in spongefish and derive(Encoding) should work for Vec<u8>.
There was a problem hiding this comment.
Instead of implementing these traits for ark_bn254::Fr, can we do a blanket implementation over F: jolt_field::CanonicalBytes like we had in blanket.rs?
There was a problem hiding this comment.
Done — FieldEl<F> / FieldElOptimized<F> now generic with per-impl bounds (CanonicalBytes for Encoding, ReducingBytes/FromPrimitiveInt for Decoding). 64-byte uniform constant stays hardcoded with a doc note (BN254-tuned; sound for any ≤254-bit field).
|
Claude code review session started: https://claude.ai/code/session_01FAUUPvuGwgBtJv39uUDsav |
moodlezoup
left a comment
moodlezoup
left a comment
There was a problem hiding this comment.
Spongefish port looks solid overall — known-vector tests pin the wire format and the new symmetry invariant is a good defense-in-depth. A few items worth fixing before this lands.
Generated by Claude Code
| let mut len_bytes = [0u8; 8]; | ||
| len_bytes.copy_from_slice(&buf[..8]); | ||
| let len = u64::from_le_bytes(len_bytes) as usize; | ||
| if buf.len() < 8 + len { |
There was a problem hiding this comment.
8 + len overflows when an attacker-supplied NARG declares len = u64::MAX. On 64-bit release builds the sum wraps to 7, the length guard passes, and buf[8..8+len] then panics on slice OOB instead of returning VerificationError. Any service that verifies untrusted proofs gets DoSed via a single 8-byte length prefix.
| if buf.len() < 8 + len { | |
| let total = 8usize.checked_add(len).ok_or(VerificationError)?; | |
| if buf.len() < total { | |
| return Err(VerificationError); | |
| } | |
| let body = buf[8..total].to_vec(); | |
| *buf = &buf[total..]; |
Generated by Claude Code
There was a problem hiding this comment.
Fixed — 8usize.checked_add(len) per suggestion. Added regression test feeding u64::MAX length.
| buf.extend_from_slice(&(bytes.len() as u64).to_le_bytes()); | ||
| buf.extend_from_slice(bytes); | ||
| let _ = self.sponge.absorb(&buf); | ||
| self.state = peek_state(&self.sponge); |
There was a problem hiding this comment.
peek_state clones the entire sponge after every append_bytes and challenge call. For PoseidonTranscript the clone calls fresh_hasher() (poseidon.rs:95), which re-runs Poseidon::new_circom(3) and rebuilds the BN254 x5 round constants from scratch on every transcript op. No current consumer of jolt-transcript (jolt-sumcheck, jolt-openings, jolt-crypto) actually calls state(), so this work is pure overhead. Consider lazy refresh: store Cell<Option<[u8; 32]>>, invalidate on absorb/squeeze, populate on state() access.
Generated by Claude Code
There was a problem hiding this comment.
Fixed — dropped the per-op cache. state() now returns owned [u8; 32] and calls peek_state on demand. No production caller exists (debug-only path), so the trade is fine.
| let mut buf = [0u8; 16]; | ||
| let _ = self.sponge.squeeze(&mut buf); | ||
| self.state = peek_state(&self.sponge); | ||
| F::from_challenge_bytes(&buf) |
There was a problem hiding this comment.
The compat facade squeezes 16 bytes for every sponge including Poseidon, then maps via from_challenge_bytes. That contradicts the doc at prover.rs:53-55 which says calling a 128-bit decoder on a Poseidon-backed state is "a compile error" — Poseidon-via-compat does produce 128-bit-truncated challenges, just through a different entry point. Either add a WARNING: comment here noting the deliberate inconsistency pending the jolt-core migration, or specialize the squeeze width per-sponge so both surfaces agree.
Generated by Claude Code
There was a problem hiding this comment.
Added WARNING comment. Divergence is intentional (compat preserves legacy width; not unsound — 128-bit truncation of Poseidon output is fine, just leaves entropy on the table). No external crate consumes compat PoseidonTranscript; goes away once jolt-sumcheck/openings/crypto migrate.
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| pub struct FieldElOptimized(pub Fr); | ||
|
|
||
| impl Encoding<[u8]> for FieldElOptimized { |
There was a problem hiding this comment.
Spongefish's blanket impl<T: Encoding<[u8]>> NargSerialize for T (spongefish-0.7.0/src/io.rs:62) means prover.prover_message(&FieldElOptimized(f)) compiles silently, writes 32 bytes to the NARG, and the verifier has no way to read them back (no NargDeserialize impl). The data orphans in NARG and check_eof fails later, far from the misuse. The intended compile-error guard described in the doc comment doesn't fire on prover_message. Consider dropping the Encoding impl entirely (or wrapping FieldElOptimized in a pub(crate) newtype that only implements Decoding) so misuse is caught at the callsite.
Generated by Claude Code
There was a problem hiding this comment.
Fixed — dropped the Encoding impl entirely. FieldElOptimized is verifier-message-only (squeeze → Decoding), so no caller breaks. Misuse via prover_message now fails at the call site instead of orphaning bytes in the NARG.
Address PR a16z#1455 review: - Generalize FieldEl<F> / FieldElOptimized<F> over jolt-field traits (CanonicalBytes for Encoding, ReducingBytes/FromPrimitiveInt for Decoding) so the codec is not pinned to ark_bn254::Fr. - BytesMsg::deserialize_from_narg used `8 + len` directly, which wraps on attacker-supplied len = u64::MAX and lets buf[8..8+len] panic instead of returning VerificationError. Use checked_add and add a regression test feeding u64::MAX.
Address PR a16z#1455 review. SpongeTranscript cloned the entire sponge and squeezed 32 bytes on every append_bytes / challenge to keep state() returning a reference cheaply. For PoseidonSponge each clone re-ran Poseidon::new_circom(3), rebuilding BN254 x5 round constants per op. No production caller of state() exists in this branch -- the method is only used by debug-only cross-verifier comparisons. Change the Transcript trait to return owned [u8; 32] and peek on demand inside state(), removing the cache field and the per-op clone.
…divergence Address PR a16z#1455 review. compat::SpongeTranscript::challenge squeezes 16 bytes uniformly across all sponges, including Poseidon, while the split-trait OptimizedChallenge in prover.rs is deliberately not implemented for Poseidon (compile error). The divergence is intentional: compat preserves the legacy jolt-core challenge width for in-flight consumers and goes away once they migrate. Document the asymmetry inline so future readers don't treat compat as a bug.
… silent NARG misuse Address PR a16z#1455 review. Spongefish 0.7's blanket `impl<T: Encoding<[u8]>> NargSerialize for T` (io.rs:62) meant `prover_message(&FieldElOptimized(f))` compiled silently, wrote 32 bytes into the NARG, and the verifier had no `NargDeserialize` impl to read them back — leaving the data orphaned and surfacing as a far-away `check_eof` failure. Drop the `Encoding` impl entirely so misuse fails at the call site. `FieldElOptimized` is verifier-message-only (Decoding via squeeze), so no caller is affected.
There was a problem hiding this comment.
This PR is great progress. Thank you, @shreyas-londhe!
Here is some review/feedback, sorted by priority.
Constructing the sponge
The instance is not specified! It is always set to EmptyInstance, which is going to be a big footgun. I would suggest removing this struct, and requesting an instance from the caller.
Would it make sense for the constructor to be part of the prover and verifier transcripts, or perhaps to have a constructor specifically for the domain separation part?
Secondly, the session ID is not really used in this PR, and I’m assuming it will come from the application layer. It would be useful for me to have an example that I can check.
Third, Transcript requires Clone, and the legacy transcript structs derive it. This defeats the “prover state cannot be copied/restored” mitigation at the API level that we had in spongefish.
Top-level proof bytes are malleable. Are you checking the NARG string is fully read? valid_proof || garbage appears to deserialize as the same proof. This also affects generated verifier paths that call it.
Poseidon
[This is not necessarily related to this specific PR; it is a problem in main too.]
The Poseidon implementation is not using DSFS, so it does not benefit from the provable security results we have. If I understand correctly, it is doing something like this for absorb:
state_0 = 0
state_1 = P(0, block_0_a, block_0_b)[0]
state_2 = P(0, state_1, block_1_a, block_1_b)[0]
...And something like this for squeezing:
state_next = PoseidonPermutation([0, state_old, 0, 0])[0]
output state_nextFor absorb: you are throwing away 1/4 of the rate. That is, you can have 25% more hash throughput with a small change.
For squeeze: this is not a sponge. It seems closer to the hash-chain Fiat-Shamir variant. I am not able to see an attack, as long as the protocol ID does not create confusion there.
Also here you're wasting all the rate, and losing at least 50% of the throughput.
It is also not streaming-friendly, which I believe is one of Jolt’s targets.
I would suggest implementing a true permutation function, in the sense of spongefish::Permutation, and then simply using DuplexSponge<P: Permutation> instead of writing a custom absorb/squeeze implementation.
I have not checked the Poseidon implementation in detail, but it seems to go through another library. I didn't check yet if that library has test vectors to match the spec.
Redundant structures
Some structures can be removed. I will go through them in order, though I may be missing something for some of them.
-
FieldEl,FieldElOptimized,UniformFrBytesField elements mod
$p$ have historically been represented using the smallest representation in big-endian order [RFC8017].You can patch the
spongefishcodec forark-ffusing an appropriate[replace]block. This is already done for you in my fork here: mmaker@168ab74. -
OptimizedChallengeOnce item 1 is done, this can be a function with a default implementation in the trait. I would suggest moving the following into the trait, since it is such a simple function:
fn challenge_128(&mut self) -> Fr { Fr::from(VerifierState::verifier_message::<u128>(self)) }
-
BytesMsgI agree with Michael that we should handle this at the
spongefishlevel.
Minor
Some wording/miscellaneous advice after having spent a lot of time thinking about this:
- challenge: I would rather call this a verifier message.
- transcript generally refers to the list of prover and verifier messages. Here, it is a bit misused.
- compat: why not
legacy? - Add a
deprecatedannotation to all the structures that are not going to be used/needed? - Some consistency tests are listed as executables. Why not move them into unit tests? They do not seem to be testing the full API (all the different transcript traits you have), are they?
| /// Length-prefixed byte string. 8-byte LE length keeps `BytesMsg(a) ; BytesMsg(b)` | ||
| /// distinguishable from `BytesMsg(a||b)`. | ||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub struct BytesMsg(pub Vec<u8>); |
There was a problem hiding this comment.
I agree with you and @moodlezoup. This wrapper should be in spongefish and derive(Encoding) should work for Vec<u8>.
|
Thanks for the review, @mmaker. Status per item below. Sponge construction
Native API redesigned. prover_transcript(session: &[u8], instance: [u8; 32], sponge) -> ProverState<H, StdRng>
verifier_transcript(session: &[u8], instance: [u8; 32], sponge, narg) -> VerifierState<'_, H>
transcript_builder() -> DomainSeparator<WithoutInstance, WithoutSession> // escape hatch
Doc-test on
Dropped from both
Poseidon
Out of scope for this PR. @moodlezoup — three paths: (a) swap to Redundant structures
Deleted
Body collapsed to @moodlezoup — flag if Poseidon should be able to opt into truncated challenges eventually; would switch to a marker-trait design then.
Agreed. spongefish 0.7 has Minor
|
Spongefish deliberately makes prover state non-cloneable to prevent fork-and-resume of a Fiat-Shamir transcript. The compat facade trait required Clone, which defeated that mitigation at the API level. Drop the bound from compat::Transcript and remove the manual Clone impl on SpongeTranscript. Removes test_clone_independence from the shared transcript test macro (no longer applicable). Per mmaker's review on a16z#1455.
… codec `FieldElOptimized<F>` and `UniformFrBytes` wrapped spongefish's u128 absorb path with our own newtype. The wrapper added indirection without behavior — spongefish's built-in `u128` codec encodes identical bytes (16-byte LE) and decodes to the same truncated challenge. Delete both wrappers from `codec.rs`. Collapse every `OptimizedChallenge` body in `prover.rs` and `verifier.rs` to `Fr::from(verifier_message::<u128>(self))`. Wire format preserved exactly — `to_le_bytes` is spongefish's u128 Encoding too. `FieldEl<F>` (full-field LE absorb / 64-byte uniform decode) stays — spongefish 0.7's `ark-ff` feature would pull in `ark-ff ^0.6`, incompatible with the workspace's pinned `ark-ff 0.5` fork. Per mmaker's review on a16z#1455.
The source-compatible facade (`Transcript`, `SpongeTranscript`, `AppendToTranscript`, `Label`, `LabelWithCount`, `U64Word`) is slated for deletion once jolt-sumcheck / jolt-openings / jolt-crypto migrate to the native split-trait surface. Rename `mod compat` to `mod legacy` to advertise the lifecycle — "compat" implied a permanent compatibility shim; "legacy" names what it is. Internal rename only — public API surface unchanged. Per mmaker's review on a16z#1455.
…verifier_transcript
Two positional factory functions plus one escape hatch:
prover_transcript(session: &[u8], instance: [u8; 32], sponge)
-> ProverState<H, StdRng>
verifier_transcript(session: &[u8], instance: [u8; 32], sponge, narg)
-> VerifierState<'_, H>
transcript_builder() -> DomainSeparator<WithoutInstance, WithoutSession>
`instance: [u8; 32]` is positional and required. `transcript_builder()`
exposes spongefish's `DomainSeparator` with PROTOCOL_ID pre-bound for
composite verifiers (e.g. `BlindFold` sub-rounds) that need the full
type-state builder.
`EmptyInstance` is retained in setup.rs for the legacy facade, which
still absorbs an empty instance step in its own constructor. The new
factory uses `InstanceDigest` internally for the 32-byte digest.
Update jolt-eval transcript-symmetry invariant to the new shape with
a placeholder INSTANCE_DIGEST.
Per mmaker's review on a16z#1455.
Strengthen the `VerifierTranscript::check_eof` doc-comment to spell out the malleability consequence of skipping it and the contract for callers: invoke once at the end of the top-level verify function on the success path; error paths skip it. Add `tests/narg_eof_tests.rs` covering the regression: exact-NARG verifies, trailing 7 bytes / 1 byte rejects, unread prover messages reject. Add `tests/native_traits_tests.rs` covering the native split-trait surface (`ProverTranscript`, `VerifierTranscript`, `OptimizedChallenge`) — round-trip, 128-bit truncation, session and instance domain separation — for Blake2b and Keccak backends. Per mmaker's review on a16z#1455.
PROTOCOL_ID should name the protocol, not the cryptographic substrate it runs on. Spongefish is the duplex-sponge framework we're built on; it's an implementation choice, not the identity of this transcript. Change `a16z/jolt-transcript/spongefish/v1` → `a16z/jolt-transcript/v1`. Wire-format-breaking — PROTOCOL_ID is absorbed first, so every downstream challenge shifts. Repin all four known-vector tests (Blake2b, Keccak, Poseidon, narg_eof PROTOCOL_ID byte check). Spec doc updated to match. Acceptable here because the spongefish-based wire format isn't shipped yet.
Downgrade spongefish 0.7 → 0.6.1 and switch `[patch.crates-io]` → `[replace]`. Spongefish 0.6.1 requires `ark-ff ^0.5`, matching the workspace's pinned `a16z/arkworks-algebra@dev/twist-shout` fork (0.5.0); `[replace]` overrides by exact `name:version` so the spongefish-side transitive ark-ff dep resolves to our fork, eliminating the multi-version skew that forced the local `FieldEl<F>` wrapper. Enable `spongefish/ark-ff` in jolt-transcript and delete `FieldEl<F>` and `UniformFrBytes`. Field absorbs now go through spongefish's `Encoding<[u8]>` for `ark_ff::Fp<C, N>` directly — big-endian canonical encoding per RFC8017. `codec.rs` retains only `BytesMsg` (length-prefixed byte string framing) since spongefish lacks a length-prefixed Vec<u8>. Wire-format-breaking for scalar absorbs (LE → BE). Known-vector tests in this crate use `append_bytes` (byte-level) not field absorbs, so they are unaffected. `jolt-eval/transcript_symmetry` updated to use `ark_bn254::Fr` directly (spongefish's Encoding impl is on `Fp`, not on the `jolt_field::Fr` newtype — orphan rule blocks forwarding impls inside jolt-transcript). Per mmaker's review on a16z#1455.
…t-spongefish # Conflicts: # Cargo.lock # jolt-eval/src/invariant/mod.rs
iirc the reason we went with light-poseidon was that it seemed relatively mature and has received an audit from Veridise. What's the status of the arkworks implementation? cc @mmaker |
* docs(transcript): spec for transcript hardening (P0 + P2 + P3)
First-PR scope, off main:
- P0: AkitaInstanceDescriptor (four lifetime-tiered sections:
AlgebraSection / SetupSection / PlanSection / CallSection) absorbed
into spongefish DomainSeparator.instance.
- P2: AkitaTranscript<Sponge> over spongefish ProverState/VerifierState
plus a Label ZST + label!() macro that strips labels from the
production sponge (Invariant 5) and keeps {tag, file, line} in
test/logging builds. LoggingTranscript<Sponge> with five smell
checks, including wire_value_before_squeeze_coverage which carries
the PR #88 soundness invariant at test time.
- P3: differential proptest over (config, nv, num_polys, basis)
asserting verify-success and prover/verifier event-stream equality.
Deferred to follow-ups (named with crisp triggers in spec): the
prover/verifier trait split (P1), Bound<T> at module seams (P4), the
algorithm-as-bytes digest (bolt-lean direction), NARG-as-proof
migration, per-deployment setup PRG salt, and wider primes /
non-subfield extensions. Six TODO(transcript-hardening-v2) markers in
the descriptor flag the assumptions that may need future revision.
Adopts spongefish 0.7.x directly (does not wait on a16z/jolt#1455);
adds transcript-blake2b (default) and transcript-keccak features.
Goal of this first PR: get domain separation correct, not obviously
unsound, and ship the diagnostic substrate (LoggingTranscript +
schedule dumper) that future protocol changes will need.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs(transcript): drop local NEVER-COMMIT path leaks from spec
Bugbot flagged two absolute-path references to a local-only
synthesis-notes file in §Deferred Follow-Ups #3 body and the
§References list. Removed both. Replaced the only load-bearing
reference with a forward pointer to a tentative future spec file
(`specs/verifier-program-ir.md`) that will land alongside the
algorithm-as-bytes-digest follow-up work when activated.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs(transcript): PR #90 review revisions + three follow-up nits
Substantial design refinements from the review pass:
- Label cfg gating tightened to feature = "logging-transcript" only
(was cfg(any(test, feature = ...))); ordinary cargo test runs now
see the Label ZST, making size_of::<Label>() == 0 a real
default-feature unit test rather than a release-only property.
- PlanSection binds effective_schedule_digest (post-fallback) instead
of raw schedule_plan_digest; catches verifier/prover divergence on
the root-fold-to-root-direct fallback predicate.
- CallSection gains num_points + incidence_digest over the normalized
ClaimIncidenceSummary; distinguishes [2,1] from [1,2] batch
groupings, which have the same totals but different verifier
branching and row-batching challenges.
- SetupSection adds setup_seed_digest, shared_matrix_digest,
protocol_features, and clarifies level_params_digest to bind the
full LevelParams envelope; catches mis-expanded / custom-loaded
Ajtai matrices and silent ZK-mode flips.
- New smell check #5: tracked wire coverage is complete. Uses a
proof-field coverage manifest or TrackedAkitaBatchedProof view to
prevent smell check #4 from passing vacuously when a new proof field
is added without instrumentation.
- Smell check #4 reframed from "deserialize" to "semantic wire-use"
(the actual soundness concern); anchored to the terminal-fold
AkitaStage2Verifier::new_with_direct_witness handoff.
- label!() macro restricted to $tag:literal only; defends against
accidental runtime label captures in production callsites.
- Deferred-follow-up triggers tightened, especially #3 (algorithm digest
now requires versioned dialect registry, golden vectors, stability
tests) and #4 (NARG migration must explicitly preserve or replace
planner accounting / debug visibility / serialization stability).
Plus three follow-up nits folded in on the same pass:
1. Sponge family in the binding. PROTOCOL_TAG is a cfg-gated
pub const [u8; 64] carrying the family name
(b"akita-pcs/transcript/v1/{blake2b|keccak}\0..."); keeps sponge
identity inside spongefish's own discipline (no extra descriptor
field), and makes cross-family transcripts byte-distinguishable at
the very first absorb.
2. ClaimIncidenceSummary cited at its existing path
crates/akita-types/src/proof/incidence.rs:94. The type already
exists with the exact field set the spec references; no new type
needed in the implementation PR.
3. Ordering note in Pillar P0: verifier computes the effective
schedule (from setup + public claims + extension-packing predicate)
BEFORE descriptor construction. No chicken-and-egg with the
transcript; the schedule digest is computable from inputs known
before any transcript exists.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs(transcript): reconcile implementation spec wording
* feat(types): add transcript instance descriptor
* feat(transcript): add spongefish transcript substrate
* feat(transcript): bind descriptors before protocol replay
* feat(transcript): remove legacy transcript backend
* feat(transcript): add logging transcript checks
* test(transcript): add hardening event checks
* docs(transcript): add hardening guide and schedule example
* test(challenges): update sparse transcript vector
* feat(transcript): record verifier wire events
* fix(transcript): address bugbot and feature unification
* refactor(transcript): finish akita transcript cutover
* test(transcript): share public event filter
* perf(transcript): cache setup descriptor digests
* fix(ci): update benchmark transcript
* fix(ci): update transcript fuzz and deny policy
* fix(transcript): require instance binding
* docs(transcript): clarify positional labels
* ci(profile): disable native benchmark cache
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Omid Bodaghi <42227752+omibo@users.noreply.github.com>
There was a problem hiding this comment.
great work, this lgtm. fyi @markosg04 @0xAndoroid I think merging this would break compatibility between crates/ and jolt-core, but once we follow-up and integrate jolt-core with jolt-transcript we should be ok again
mmaker
left a comment
mmaker
left a comment
There was a problem hiding this comment.
there's some sloppy tests, perhaps we can make them a bit more clear and reduce the size of this PR?
| fn build_valid_narg(messages: &[&[u8]]) -> Vec<u8> { | ||
| let mut prover = prover_transcript(SESSION, INSTANCE, Blake2b512::default()); | ||
| for m in messages { | ||
| ProverTranscript::<Blake2b512>::prover_message(&mut prover, &BytesMsg(m.to_vec())); | ||
| } | ||
| ProverTranscript::<Blake2b512>::narg_string(&prover).to_vec() | ||
| } |
There was a problem hiding this comment.
| fn build_valid_narg(messages: &[&[u8]]) -> Vec<u8> { | |
| let mut prover = prover_transcript(SESSION, INSTANCE, Blake2b512::default()); | |
| for m in messages { | |
| ProverTranscript::<Blake2b512>::prover_message(&mut prover, &BytesMsg(m.to_vec())); | |
| } | |
| ProverTranscript::<Blake2b512>::narg_string(&prover).to_vec() | |
| } | |
| fn build_valid_narg(messages: &[&[u8]]) -> Vec<u8> { | |
| let mut prover = prover_transcript(SESSION, INSTANCE, Blake2b512::default()); | |
| for m in messages { | |
| prover.prover_message(&BytesMsg(m.to_vec())); | |
| } | |
| prover.narg_string().to_vec() | |
| } |
| .expect("valid prover message must deserialize"); | ||
| assert_eq!(got.as_slice(), *expected); | ||
| } | ||
| VerifierTranscript::<Blake2b512>::check_eof(verifier).expect("exact narg must pass check_eof"); |
| .expect("valid prefix must deserialize"); | ||
| assert_eq!(got.as_slice(), *expected); | ||
| } | ||
| let result = VerifierTranscript::<Blake2b512>::check_eof(verifier); |
| fn prover_verifier_round_trip() { | ||
| let mut prover = prover_transcript(SESSION, INSTANCE, Blake2b512::default()); | ||
| ProverTranscript::<Blake2b512>::public_message(&mut prover, &BytesMsg(b"pub".to_vec())); | ||
| ProverTranscript::<Blake2b512>::prover_message(&mut prover, &BytesMsg(b"private".to_vec())); | ||
| let _c1: Fr = | ||
| <spongefish::ProverState<Blake2b512> as OptimizedChallenge>::challenge_128(&mut prover); | ||
| let narg = ProverTranscript::<Blake2b512>::narg_string(&prover).to_vec(); | ||
|
|
||
| let mut verifier = verifier_transcript(SESSION, INSTANCE, Blake2b512::default(), &narg); | ||
| VerifierTranscript::<Blake2b512>::public_message(&mut verifier, &BytesMsg(b"pub".to_vec())); | ||
| let got: BytesMsg = VerifierTranscript::<Blake2b512>::prover_message(&mut verifier) | ||
| .expect("prover_message must deserialize"); | ||
| assert_eq!(got.as_slice(), b"private"); | ||
| let _c2: Fr = | ||
| <spongefish::VerifierState<'_, Blake2b512> as OptimizedChallenge>::challenge_128( | ||
| &mut verifier, | ||
| ); | ||
| VerifierTranscript::<Blake2b512>::check_eof(verifier).expect("eof"); | ||
| } |
There was a problem hiding this comment.
| fn prover_verifier_round_trip() { | |
| let mut prover = prover_transcript(SESSION, INSTANCE, Blake2b512::default()); | |
| ProverTranscript::<Blake2b512>::public_message(&mut prover, &BytesMsg(b"pub".to_vec())); | |
| ProverTranscript::<Blake2b512>::prover_message(&mut prover, &BytesMsg(b"private".to_vec())); | |
| let _c1: Fr = | |
| <spongefish::ProverState<Blake2b512> as OptimizedChallenge>::challenge_128(&mut prover); | |
| let narg = ProverTranscript::<Blake2b512>::narg_string(&prover).to_vec(); | |
| let mut verifier = verifier_transcript(SESSION, INSTANCE, Blake2b512::default(), &narg); | |
| VerifierTranscript::<Blake2b512>::public_message(&mut verifier, &BytesMsg(b"pub".to_vec())); | |
| let got: BytesMsg = VerifierTranscript::<Blake2b512>::prover_message(&mut verifier) | |
| .expect("prover_message must deserialize"); | |
| assert_eq!(got.as_slice(), b"private"); | |
| let _c2: Fr = | |
| <spongefish::VerifierState<'_, Blake2b512> as OptimizedChallenge>::challenge_128( | |
| &mut verifier, | |
| ); | |
| VerifierTranscript::<Blake2b512>::check_eof(verifier).expect("eof"); | |
| } | |
| #[test] | |
| fn prover_verifier_round_trip() { | |
| let mut prover = prover_transcript(SESSION, INSTANCE, Blake2b512::default()); | |
| prover.public_message(&BytesMsg(b"pub".to_vec())); | |
| prover.prover_message(&BytesMsg(b"private".to_vec())); | |
| let _c1: Fr = prover.challenge_128(); | |
| let narg = prover.narg_string(); | |
| let mut verifier = verifier_transcript(SESSION, INSTANCE, Blake2b512::default(), &narg); | |
| verifier.public_message(&BytesMsg(b"pub".to_vec())); | |
| let got: BytesMsg = verifier | |
| .prover_message() | |
| .expect("prover_message must deserialize"); | |
| assert_eq!(got.as_slice(), b"private"); | |
| let _c2: Fr = verifier.challenge_128(); | |
| verifier.check_eof().expect("eof"); | |
| } |
5 participants
Summary
Spec-only PR. Proposes porting
crates/jolt-transcriptfrom its hand-rolled Fiat-Shamir construction to spongefish, the audited duplex-sponge library.Key decisions captured in the spec:
DuplexSpongeInterface.spongefish::Blake2b512andspongefish::Keccakcome ready-made; Poseidon gets a newDuplexSpongeInterfaceimpl overlight-poseidon.spongefish::ProverState<Sponge>/VerifierState<Sponge>. Orphan rule allows it; no wrapper structs.JoltProofcollapses to a NARG byte string plus whatever public inputs the verifier doesn't already know. Today's cfg-gatedopening_claims/blindfold_prooffields become different prover-message sequences inside the NARG.transpilable_verifier.rs), gnark verifier (transpiler/), and Lean extractor (zklean-extractor/) are explicit non-goals here and become coordinated follow-ups.a16z/dory. This PR updates theJoltToDoryTranscriptbridge to wrap the new traits while dory keeps its currentDoryTranscript. The bridge is removed once dory consumesjolt-transcript.muldive2e matrix across 3 sponges × 2 modes.Open question flagged in the spec for maintainer input: how CI should treat
transpiler/go/e2e_test.goand any Solidity tests that pin current byte layouts, since those will fail as a direct consequence.Context: this is the redirect from a16z/dory PR #17 (closed by the maintainer) — per that direction, spongefish work lives in
jolt-transcript, and dory will consume it.Test plan
Closes none — this is a spec PR.