-
Notifications
You must be signed in to change notification settings - Fork 29
pub_inputs support #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
pub_inputs support #215
Conversation
a35db76 to
67cae81
Compare
13a50e4 to
d6817ee
Compare
69ba9b3 to
857db9b
Compare
da0ce79 to
52b6528
Compare
576f283 to
54c49f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for public witness opening in the WHIR R1CS proof system. The implementation introduces a new PublicInputs struct to handle public values, updates both the prover and verifier to compute and verify public input hashes, and modifies the witness scheduling to ensure public inputs are correctly ordered in the witness commitment.
Key Changes:
- Introduced
PublicInputsstruct with SHA-256 hashing for commitment - Modified prover and verifier to handle public weights and verify public input consistency
- Updated witness scheduling to place public inputs in w1 (pre-challenge) layer with proper ordering
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| provekit/common/src/witness/mod.rs | Adds PublicInputs struct with hashing and serialization support |
| provekit/common/src/utils/serde_ark_vec.rs | New module for serializing vectors of field elements |
| provekit/common/src/utils/mod.rs | Exports new serde_ark_vec module |
| provekit/common/src/utils/sumcheck.rs | Adds add_public_inputs() method to IO pattern for public input hash and randomness |
| provekit/common/src/whir_r1cs.rs | Updates constraint counts and IO pattern to include public weights verification |
| provekit/common/src/noir_proof_scheme.rs | Adds public_inputs field to NoirProof struct |
| provekit/common/src/lib.rs | Exports PublicInputs type |
| provekit/common/Cargo.toml | Adds sha2 dependency for hashing |
| provekit/common/src/witness/witness_builder.rs | Passes public input indices to witness splitting logic |
| provekit/common/src/witness/scheduling/splitter.rs | Ensures public inputs are placed in w1 and properly ordered after constant builder |
| provekit/r1cs-compiler/src/noir_proof_scheme.rs | Extracts ACIR public input indices for witness splitting |
| provekit/prover/src/lib.rs | Extracts public inputs from witness and passes to proof generation |
| provekit/prover/src/whir_r1cs.rs | Implements public weights computation and statement updates for both single and batch commitment cases |
| provekit/verifier/src/lib.rs | Passes public inputs to verification |
| provekit/verifier/src/whir_r1cs.rs | Verifies public input hash and updates statements with public weights constraints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
37d7c42 to
15bddd8
Compare
15bddd8 to
f9776b5
Compare
| } | ||
|
|
||
| // hashPublicInputs computes the hash of public inputs as field elements sequentially | ||
| func hashPublicInputs(sc *skyscraper.Skyscraper, publicInputs PublicInputs) (frontend.Variable, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following must produce identical results:
- Rust: PublicInputs::hash() using SkyscraperCRH::evaluate()
- Go: hashPublicInputs() using sc.CompressV2()
Add a cross-language test with known test vectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ref in : #257
| let num_witnesses = 2; | ||
| let num_ood_constraints = num_witnesses * self.whir_witness.committment_ood_samples; | ||
| let num_statement_constraints = 6; // 2 statements × 3 constraints | ||
| let num_statement_constraints = 7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is hardcoded and assumes:
- Batch mode: 3 constraints for statement1 + 1 public weights + 3 for statement2 = 7
- But what if public inputs are empty in batch mode?
The Go verifier calculates this dynamically:
if !cfg.PublicInputs.IsEmpty() {
witnessLinearStatementEvalsSize = 7
} else {
witnessLinearStatementEvalsSize = 6
}| .test_witness_satisfaction(&witness.iter().map(|w| w.unwrap()).collect::<Vec<_>>()) | ||
| .context("While verifying R1CS instance")?; | ||
|
|
||
| // Gather public inputs from witness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No validation that the public inputs extracted from witness match what the circuit expects. If acir_public_inputs indices are wrong, proof will be valid but verify wrong values.
| whir_public_weights_query_answer: (FieldElement, FieldElement), | ||
| ) { | ||
| let (public_f_sum, public_g_sum) = whir_public_weights_query_answer; | ||
| let public_weight = Weights::linear(EvaluationsList::new(vec![FieldElement::zero(); 1 << m])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verifier creates dummy zero-weight vectors. This is correct because the verifier doesn't know the actual weights (only the prover does), but:
- This should be commented explaining why zeros are OK
- Verify that the WHIR protocol handles this correctly (the actual weighted sum comes from the hint)
| .hint() | ||
| .context("failed to read WHIR public weights query answer")?; | ||
|
|
||
| if !public_inputs.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When public inputs are empty, the hint whir_public_weights_query_answer is still read from the transcript but never validated. A malicious prover could put garbage here and it would be ignored.
| merlin.hint::<(Vec<FieldElement>, Vec<FieldElement>)>(&(f_sums, g_sums))?; | ||
|
|
||
| // VERIFY the size given by self.m | ||
| let public_weight = get_public_weights(public_inputs, &mut merlin, self.m); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate Logic Between Single/Batch Paths. Same pattern in the verifier, the public input hash verification and statement extension logic is copy-pasted between the two branches.
|
Please add unit tests for testing all cases of public inputs. |
Support for public_witness opening