Skip to content

refactor: replace rolling-hash transcript with spongefish NARG protocol#17

Closed
shreyas-londhe wants to merge 3 commits intoa16z:mainfrom
shreyas-londhe:feat/spongefish-1-core
Closed

refactor: replace rolling-hash transcript with spongefish NARG protocol#17
shreyas-londhe wants to merge 3 commits intoa16z:mainfrom
shreyas-londhe:feat/spongefish-1-core

Conversation

@shreyas-londhe
Copy link
Copy Markdown

@shreyas-londhe shreyas-londhe commented Mar 6, 2026

Summary

Replace Blake2b rolling-hash Fiat-Shamir transcript with upstream spongefish v0.5.1 duplex-sponge construction. Proofs become opaque NARG byte strings — no separate Proof struct.

Breaking change: Proof format is incompatible with previous versions.

Core changes

  • Add upstream spongefish dependency (arkworks-rs/spongefish tag v0.5.1, features: ark-ec, sha3)
  • New ProverTranscript/VerifierTranscript traits abstracting the sponge API
  • domain.rs — domain separator with instance encoding (nu, sigma, zk flag)
  • spongefish_codecs.rs — trait impls mapping ProverState/VerifierState to Dory's transcript traits
  • Commitment (GT), evaluation (Fr), and point coordinates bound as public_message into the sponge state for Fiat-Shamir domain separation

Removed

  • blake2b_transcript.rs, ark_proof.rs, proof.rs, VMVMessage
  • Runtime pattern enforcement (CheckedProverState, CheckedVerifierState, PatternBuilder)

Tests & examples

  • All tests, benchmarks, and examples migrated to upstream ProverState/VerifierState
  • Soundness tests cover byte-level NARG tampering
  • 71 tests pass, clippy clean

Replace the custom Blake2b rolling-hash transcript with spongefish's
duplex-sponge construction. Proofs are now opaque NARG byte strings
instead of serialized Proof structs.

Key changes:
- Add CheckedProverState/CheckedVerifierState with InteractionPattern
  enforcement for compile-time protocol structure validation
- Declare reusable sub-patterns (sigma1, sigma2, reduce round, scalar
  product) composed via scoped nesting for diagnostic paths
- Domain separator binds (sigma, zk) into sponge instance
- Remove Proof struct, blake2b transcript, ark_proof, VMVMessage
- Add check_eof() to all tests, benches, and examples
- Use macros to reduce spongefish codec boilerplate
- Pin spongefish to commit 45df37a7 on fork

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Blake2b rolling-hash Fiat-Shamir with spongefish duplex-sponge
(upstream arkworks-rs/spongefish v0.5.1). Proofs become opaque NARG byte
strings. Bind commitment, evaluation, and point as public messages into
the sponge state for defense-in-depth Fiat-Shamir domain separation.
Migrate all call sites to upstream ProverState/VerifierState. Remove
check_complete() calls (pattern enforcement dropped). Proof extraction
is now prover.narg_string().to_vec() directly.
@shreyas-londhe shreyas-londhe force-pushed the feat/spongefish-1-core branch from cf0490f to 16364e8 Compare April 6, 2026 06:30
@shreyas-londhe shreyas-londhe changed the title refactor: replace transcript with spongefish NARG protocol refactor: replace rolling-hash transcript with spongefish NARG protocol Apr 6, 2026
Copy link
Copy Markdown

@mmaker mmaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only skimmed it for now but here are some preliminary comments.
I think this pr can be substantially shrinked before reviewing from close

Comment thread benches/arkworks_proof.rs
Comment thread src/backends/arkworks/domain.rs
Comment thread src/backends/arkworks/spongefish_codecs.rs
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file can be removed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this for now. The traits decouple the protocol logic from spongefish so that Dory is generic over the transcript backend. The goal is for Jolt (which uses Dory as its PCS) to pass its own transcript into prove()/verify() — any type that implements ProverTranscript<BN254> / VerifierTranscript<BN254> works without changes in Dory. Without the traits, Dory's API would hardcode spongefish::ProverState and force every consumer to use that specific type.

That said, open to discussion — @markosg04 what's your preference here?

let fe = F::from_le_bytes_mod_order(&repr);

if fe.is_zero() {
panic!("Challenge scalar cannot be zero")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it seems that you care about challenges not being zero -- I'm not sure we are preserving this in the current changes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The protocol already handles this — reduce_and_fold.rs calls .inv().expect("beta must be invertible") on all three challenges (beta, alpha, gamma), so a zero challenge panics regardless of whether the transcript checks. The probability of a cryptographic sponge outputting zero is 1/p ≈ 2^{-254} — if that happens, the sponge is broken and we have bigger problems. No additional check needed.

Comment thread src/evaluation_proof.rs
Comment on lines +214 to +219
transcript.absorb_gt(&first_msg.d1_left);
transcript.absorb_gt(&first_msg.d1_right);
transcript.absorb_gt(&first_msg.d2_left);
transcript.absorb_gt(&first_msg.d2_right);
transcript.absorb_g1(&first_msg.e1_beta);
transcript.absorb_g2(&first_msg.e2_beta);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not have first_msg derive Codec and just add it here for clarity

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deriving spongefish's Codec on the message types would couple the protocol code to spongefish::ProverState::prover_message() directly, which conflicts with keeping the protocol generic over transcript backends (comment #4). The explicit per-field absorb calls are the cost of that genericity — and they make the absorption order explicit, which is useful for auditing the Fiat-Shamir transcript.

@markosg04
Copy link
Copy Markdown
Collaborator

Hey @shreyas-londhe, thanks for putting this together.

Before we go deeper on review, I want to flag some upcoming structural changes that affect how we should approach this. We're consolidating dory into the modular jolt crates (jolt-transcript, jolt-field, jolt-crypto, etc.) to unify abstractions and avoid proliferating new types across repos. Context:

Given that, rather than trying the spongefish integration directly in dory, it'd be more useful to port jolt-transcript itself to spongefish: A PR against jolt-transcript (a16z/jolt) that reworks it to use spongefish construction. Dory would then import the updated jolt-transcript alongside jolt-field and jolt-crypto.

Note that in Jolt, we recently switched to a spec-driven development, you can find more here: https://github.com/a16z/jolt/blob/main/CONTRIBUTING.md

Happy to chat through the design before you start. I'll close this PR for now. Let's pick it up on the jolt-transcript side.

Thanks again!

@markosg04 markosg04 closed this Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants