Enhance Prover and Verifier logic: SNARK, FRI, and STARK updates by this-vishalsingh · Pull Request #14 · ZippelLabs/ZP1

this-vishalsingh · 2026-01-08T18:18:17Z

No description provided.

Copilot

Pull request overview

This PR primarily refactors and reformats the proving/verification codepaths while introducing a major change to the prover’s Fiat–Shamir transcript implementation (moving from SHA-256 to a Plonky3 Poseidon2-based challenger).

Changes:

Replace ProverChannel’s SHA-256 transcript with a Poseidon2 duplex challenger implementation.
Reformat and lightly restructure STARK prover/verifier, FRI, recursion, serialization, and auxiliary modules for readability/consistency.
Reorder/re-export items in crates/prover/src/lib.rs and apply similar style cleanups across the prover stack.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
crates/verifier/src/verify.rs	Formatting changes; verifier still assumes a particular transcript and verification math.
crates/prover/src/stark.rs	Formatting changes; DEEP quotient coefficients and trace Merkle proof structure are notable.
crates/prover/src/snark.rs	Formatting changes; Groth16 verification logic remains placeholder-like and includes a tautological condition.
crates/prover/src/serialize.rs	Formatting/attribute layout changes for serde helpers and proof structs.
crates/prover/src/recursion.rs	Formatting changes around recursion/aggregation helpers.
crates/prover/src/ram.rs	Formatting changes in RAM prover/constraints/tests.
crates/prover/src/parallel.rs	Formatting changes in parallel helpers/tests.
crates/prover/src/memory.rs	Formatting changes in constraints/tests.
crates/prover/src/logup.rs	Formatting changes in lookup/log-derivative utilities and tests.
crates/prover/src/lib.rs	Reorders module exports and public re-exports.
crates/prover/src/lde.rs	Formatting changes and assertion formatting.
crates/prover/src/fri.rs	Formatting changes around FRI commitment/query flow.
crates/prover/src/delegation.rs	Formatting changes in delegation argument code and tests.
crates/prover/src/commitment.rs	Formatting changes in Merkle tree/proof code and tests.
crates/prover/src/channel.rs	Major behavioral change: new Poseidon2-based transcript/challenger and byte absorption logic.
crates/prover/src/bitwise_tables.rs	Formatting changes and test formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-05-01T03:46:04Z

    pub fn absorb(&mut self, data: &[u8]) {
-        self.hasher.update(data);
-        self.transcript.extend_from_slice(data);
+        // Naive byte absorption - pack into field elements
+        // In a real implementation, we'd use a proper byte-to-field packing
+        for chunk in data.chunks(4) {
+            let mut bytes = [0u8; 4];
+            bytes[0..chunk.len()].copy_from_slice(chunk);
+            let val = u32::from_le_bytes(bytes) % 2147483647; // M31 modulus
+            self.challenger.observe(to_p3(M31::new(val)));
+        }


absorb(&[u8]) packs bytes into 4-byte little-endian chunks and reduces mod p. This mapping is not injective (e.g., trailing zero bytes and many distinct 32-byte commitments can collide after reduction), which weakens Fiat–Shamir soundness and may allow transcript collisions. Use a standard byte-to-field encoding with length/domain tags (or hash-to-field) that avoids lossy mod reduction, and keep it consistent with the verifier implementation.

Copilot · 2026-05-01T03:46:05Z

        // Get DEEP combination alphas
        let num_terms = trace_lde.num_columns() + 1; // trace columns + composition
        let deep_alphas: Vec<M31> = (0..num_terms)
            .map(|i| M31::new((i as u32 + 1) * 7919)) // Deterministic for testing
            .collect();


DEEP combination coefficients are hard-coded as deterministic constants ((i+1)*7919). The verifier derives deep_alphas from the Fiat–Shamir transcript, so proofs will not verify, and using non-transcript randomness undermines soundness. Sample these alphas from self.channel after absorbing the same transcript elements the verifier uses (or include them explicitly in the proof and bind them into the transcript).

Copilot · 2026-05-01T03:46:05Z

        let input_ok = proof.proof_data[..16] != [0u8; 16]; // Non-zero proof
-
-        Ok(pairing_ok && input_ok && input_commitment[0] == input_commitment[0]) // Always true if reached
+
+        Ok(pairing_ok && input_ok && input_commitment[0] == input_commitment[0])
+        // Always true if reached


The Groth16 verifier currently includes input_commitment[0] == input_commitment[0], which is a tautology and does not bind the proof to the public inputs. As written, verification reduces to pairing_ok && input_ok, which is not a meaningful check and can accept invalid proofs. Compare the computed input commitment against a value derived from the proof/vk (or remove the bogus condition and implement a real public-input consistency check).

Copilot · 2026-05-01T03:46:05Z

+        // Initialize Poseidon2 permutation
+        let mut rng = StdRng::seed_from_u64(42);
+        let permutation = Poseidon2::new_from_rng_128(&mut rng);
+        let mut challenger = DuplexChallenger::new(permutation);


ProverChannel::new constructs a fresh Poseidon2 permutation by seeding an RNG and generating constants every time a channel is created. This is likely unnecessary work and can become a bottleneck if many channels are instantiated. Consider using a fixed, precomputed Poseidon2 parameterization (or a static/lazy-initialized permutation) and reusing it across channels.

Copilot · 2026-05-01T03:46:06Z

+    pub fn new(_domain_separator: &[u8]) -> Self {
+        // Initialize Poseidon2 permutation
+        let mut rng = StdRng::seed_from_u64(42);
+        let permutation = Poseidon2::new_from_rng_128(&mut rng);
+        let mut challenger = DuplexChallenger::new(permutation);
+
+        // TODO: Absorb domain separator properly (convert to field elements)
+        // For now we just start fresh
+        Self { challenger }


ProverChannel::new ignores the provided domain separator and initializes the transcript identically for all protocols. This breaks domain separation (cross-protocol replay/collision risk) and also makes the prover transcript incompatible with crates/verifier’s VerifierChannel::new(domain_separator) behavior. Absorb the domain separator into the challenger state (in a collision-resistant, length-aware way) and ensure prover + verifier use the same transcript construction.

Copilot · 2026-05-01T03:46:06Z

                let trace_values = trace_lde.get_row(idx);
-                
+
                // For interleaved commitment, prove starting index of the row
                // Row idx contains indices [idx*num_cols, idx*num_cols + 1, ..., idx*num_cols + num_cols-1]
                let interleaved_idx = idx * num_cols;
                let trace_proof = trace_tree.prove(interleaved_idx);
-                
+
                let composition_value = composition_evals[idx];
                let composition_proof = composition_tree.prove(idx);
                let deep_quotient_value = deep_quotient[idx];


The trace Merkle proof is generated only for interleaved_idx = idx * num_cols (the first column in the row), while trace_values contains all columns. This means the commitment/proof binds only a single column value and the remaining columns can be forged, which is a soundness break. Commit per-row (hash the full row into one leaf) or provide/verify Merkle proofs for every committed leaf corresponding to all trace columns at the queried row.

Copilot · 2026-05-01T03:46:06Z

            let trace_value = query_proof.trace_values[0];
-            if !query_proof.trace_proof.verify(&proof.trace_commitment, trace_value) {
+            if !query_proof
+                .trace_proof
+                .verify(&proof.trace_commitment, trace_value)
+            {
                return Err(VerifyError::MerkleError {
                    index: query_proof.index,
                });
            }


Verifier checks the trace Merkle proof against only trace_values[0], so even if trace_values includes multiple columns, only the first value is authenticated. Given the prover’s interleaved commitment/proof flow, this leaves other columns unauthenticated (soundness issue). Either verify Merkle proofs for all committed trace leaves at this row, or switch to committing a per-row hash and verify that single leaf instead.

Copilot · 2026-05-01T03:46:06Z

        // Get evaluation domain point X at query index
        // For simplicity, use query.index as M31 (real impl would use circle domain point)
        let domain_point_m31 = M31::new(query.index as u32);
        let domain_point = QM31::from(domain_point_m31);


verify_deep_quotient uses QM31::from(M31::new(query.index as u32)) as the domain point X. The prover’s DEEP quotient construction uses the actual circle-domain x-coordinate (TraceLDE::get_domain_x(i)), so this verifier computation will not match and can also invalidate the intended soundness argument. Recompute X using the same circle domain construction as the prover (from log_trace_len/blowup_factor) and use that x-coordinate here.

Copilot · 2026-05-01T03:46:06Z

+use p3_challenger::{CanObserve, CanSample, DuplexChallenger};
+use p3_poseidon2::Poseidon2;
+use zp1_primitives::{M31, QM31, to_p3, from_p3};
+use p3_mersenne_31::{Poseidon2ExternalLayerMersenne31, Poseidon2InternalLayerMersenne31};
+use rand::SeedableRng;
+use rand::rngs::StdRng;


channel.rs now depends on p3_challenger, p3_poseidon2, and rand in non-test code, but crates/prover/Cargo.toml does not list these as regular dependencies (and rand is currently only a dev-dependency). As-is, this will fail to compile for normal builds. Add the missing crates to [dependencies] (prefer workspace deps if intended) or gate this implementation behind a feature.

this-vishalsingh · 2026-05-01T03:47:08Z

@copilot apply changes based on the comments in this thread

Agent-Logs-Url: https://github.com/ZippelLabs/ZP1/sessions/fea3d953-dede-478b-994a-643090632cc7 Co-authored-by: this-vishalsingh <93567955+this-vishalsingh@users.noreply.github.com>

Enhance Prover and Verifier logic: SNARK, FRI, and STARK updates

23753af

this-vishalsingh self-assigned this May 1, 2026

this-vishalsingh requested a review from Copilot May 1, 2026 03:39

Copilot started reviewing on behalf of this-vishalsingh May 1, 2026 03:40 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Copilot started work on behalf of this-vishalsingh May 1, 2026 03:47 View session

Changes before error encountered

1faffa5

Agent-Logs-Url: https://github.com/ZippelLabs/ZP1/sessions/fea3d953-dede-478b-994a-643090632cc7 Co-authored-by: this-vishalsingh <93567955+this-vishalsingh@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Prover and Verifier logic: SNARK, FRI, and STARK updates#14

Enhance Prover and Verifier logic: SNARK, FRI, and STARK updates#14
this-vishalsingh wants to merge 2 commits intomainfrom
feat/prover-verifier-logic

this-vishalsingh commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

this-vishalsingh commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

this-vishalsingh commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

this-vishalsingh commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants