Adds SPARK#345
Conversation
CSP benchmarks
Prover time, peak RSS, peak heap, and verifier time are arithmetic means across the iterations. Peak heap comes from the largest Each metric cell shows the current value followed by the percentage delta against the latest successful Results
|
| let request = if requests.len() == 1 { | ||
| requests[0].clone() | ||
| } else { | ||
| let alphas = alphas_from_spark( |
There was a problem hiding this comment.
For multi-query batching we compute alphas from requests[0].row, but never ensure the rest of the requests use the same row. Please reject mixed-row batches before proving.
|
|
||
| R1CSSparkQuery { | ||
| point_to_evaluate: Point { | ||
| row: requests[0].point_to_evaluate.row.clone(), |
There was a problem hiding this comment.
Same-row batching needs to be enforced verifier-side too. Otherwise verification can fold claims using requests[0].row even when later requests target different rows.
| let scheme = SPARKProverScheme::new(num_rows, num_cols, nonzero_terms, hash_config); | ||
|
|
||
| let ds = DomainSeparator::protocol(&scheme.whir_configs).instance(&Empty); | ||
| let mut merlin = ProverState::new(&ds, TranscriptSponge::default()); |
There was a problem hiding this comment.
This uses TranscriptSponge::default() even though setup receives hash_config. SPARK should use the configured Fiat-Shamir hash consistently, or store the config in SPARKSetup.
| .session(&setup.transcript.narg_string) | ||
| .instance(&hash_query_set(requests)), | ||
| &whir_proof, | ||
| TranscriptSponge::default(), |
There was a problem hiding this comment.
Verifier also uses the default transcript sponge. Please derive it from trusted setup/hash config so --hash does not affect commitments but not Fiat-Shamir.
| let setup = setup?; | ||
| let queries = queries?; | ||
|
|
||
| SPARKVerifierScheme |
There was a problem hiding this comment.
verify and verify-spark share no cryptographic state, a prover can pair an honest NoirProof for C₁ with a self-consistent .sp+.spc+query JSON set for a different circuit C₂ with matching dimensions, and both verifiers will accept. Derive the expected query from the Noir transcript and require verify-spark to consume it, ship a combined verifier.
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct COOMatrix { |
There was a problem hiding this comment.
COOMatrix stores row: Vec and row_field: Vec (same data twice). Doubles memory for big circuits and the invariant row_field[i] == FieldElement::from(row[i] as u64) is unenforced. Drop one and derive on demand, or wrap construction in a constructor that enforces it.
| } | ||
| } | ||
|
|
||
| fn collect_queries(dir: &Path) -> Result<Vec<R1CSSparkQuery>> { |
There was a problem hiding this comment.
collect_queries walks spark_query_.json until first miss, if _3.json is missing but _4.json exists, _4 is silently skipped and the SPARK proof binds the wrong set. Use read_dir+sort, or an explicit manifest, and ensure! the discovered set is contiguous.
| public_inputs: &PublicInputs, | ||
| ) -> Result<WhirR1CSProof>; | ||
| produce_spark_query: bool, | ||
| ) -> Result<(WhirR1CSProof, Vec<R1CSSparkQuery>)>; |
There was a problem hiding this comment.
Here and :159, :269, #[allow(clippy::too_many_arguments)] mutes a workspace-level lint. Group into a ProveFromAlphasCtx, SparkProverContext already sets the precedent.
| }, | ||
| }; | ||
|
|
||
| pub trait SPARKProver { |
There was a problem hiding this comment.
SPARK* acronym casing violates Rust API Guidelines C-CASE and trips clippy::upper_case_acronyms. The codebase already uses Spark* for SparkMatrix/SparkWitnesses/SparkProverContext, internally inconsistent. Rename to Spark*.
| .try_into() | ||
| .expect("4 polys"); | ||
|
|
||
| merlin.prover_hint_ark(&row_address_eval); |
There was a problem hiding this comment.
One-line comment per hint call naming which WHIR check seals it would prevent a future refactor from breaking soundness.
3 participants
SPARK
Reference for this implementation
Proposed protototype workflow
Serve step
Provekit prove step
Provekit and SPARK verify step
Design decisions
Pack$A$ , $B$ , $C$ into one block matrix Z:
This is a result from Marcin (https://gist.github.com/kustosz/14b62de666f721ab855536e575891bd1)
The trick:
Same total non-zeros, double the dimensions. Then for any$\beta$ , $p$ , and $q$ :
One matrix, one commitment, one opening.
Batching GPA and WHIR proofs
Combining GPA
WHIR Batching
|
num_terms_2batchede-values are committed and opened together. Opened once in sumcheck and once in rs_ws GPA|
num_terms_4batched| Address/timestamp values for row-wise and col-wise memory checks are committed and opened togetherTemporary Sumcheck for split witness
The current ZK WHIR doesn't support batching which would enable easier handling of split witness commitment. The repo currently uses an additional sumcheck proposed by Marcin until batch ZK commitment is supported https://gist.github.com/kustosz/c7c3f756aaae77f37e035c30c4961ea3.
The trick:
Collapsing two claims into one: With claims on$A(r,0,q_1)$ and $A(r,1,q_2)$ . Draw random $\beta$ and note their RLC is just another sum over $A(r,\cdot)$ , so run a Sumcheck #2 to reduce to a single claim $A(r, \gamma)$ .
Full workflow for a Noir passport circuit:
What is not included
Benchmark
[TODO: Update with new benchmark]