Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces dynamic prove/verify functionality to support multiple circuit versions dynamically at runtime, rather than requiring separate static instances for each circuit variant.
Key Changes:
- Adds
DynamicProveandDynamicVerifymethods that accept circuit-specific parameters at runtime - Refactors
ProvingMethodGroth16AuthV3to support multiple circuit sub-versions (e.g., authV3-8-32) through a single instance - Removes the separate
ProvingMethodGroth16AuthV3_8_32Instancein favor of dynamic circuit selection
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| jwz.go | Adds new types (ProvingParam, VerificationKeyParam, DynamicProofInputsPreparerHandlerFunc) and methods (DynamicProve, DynamicVerify, circuitIDFromProofOrHeader) to support dynamic circuit selection |
| authV3Groth16.go | Consolidates AuthV3 circuit support by adding circuitSubVersions field and SupportedCircuits() method; removes separate AuthV3_8_32 instance; moves Groth16 constant definition from deleted authGroth16.go |
| authV2Groth16.go | Adds circuitSubVersions field (empty) and SupportedCircuits() method to maintain consistency with authV3 structure |
| authGroth16.go | Removes entire file (AuthV1 circuit support) |
| jwz_test.go | Refactors test to use new DynamicProve and DynamicVerify methods with authV3-8-32 circuit |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (m *ProvingMethodGroth16AuthV3) SupportedCircuits() []string { | ||
| seen := make(map[string]struct{}, 1+len(m.circuitSubVersions)) | ||
| out := make([]string, 0, 1+len(m.circuitSubVersions)) | ||
|
|
||
| add := func(s string) { | ||
| if _, ok := seen[s]; ok { | ||
| return | ||
| } | ||
| seen[s] = struct{}{} | ||
| out = append(out, s) | ||
| } | ||
|
|
||
| add(m.ProvingMethodAlg.CircuitID) | ||
| for _, s := range m.circuitSubVersions { | ||
| add(s) | ||
| } | ||
| return out | ||
| } |
There was a problem hiding this comment.
The SupportedCircuits() method is duplicated identically between ProvingMethodGroth16AuthV3 and ProvingMethodGroth16AuthV2. Consider extracting this logic into a shared helper function or embedding a common type to reduce code duplication and improve maintainability.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential nil pointer dereference: The function checks if token != nil at line 394, but then directly accesses token.CircuitID at line 406 without verifying token is not nil in the fallback case. If token is nil, this will panic. Add a nil check before accessing token.CircuitID.
| } | |
| } | |
| if token == nil { | |
| return "" | |
| } |
jwz.go
Outdated
| } | ||
|
|
||
| // ProofInputsPreparerHandlerFunc is a function type for preparing proof inputs | ||
| type DynamicProofInputsPreparerHandlerFunc func( |
There was a problem hiding this comment.
The type name DynamicProofInputsPreparerHandlerFunc is inconsistent with the existing ProofInputsPreparerHandlerFunc naming convention. Consider renaming to ProofInputsPreparerDynamicHandlerFunc or simply DynamicProofInputsPreparer to maintain consistency with the existing naming pattern where the "HandlerFunc" suffix follows a simpler name.
| type DynamicProofInputsPreparerHandlerFunc func( | |
| type ProofInputsPreparerHandlerFunc func( |
| func (m *ProvingMethodGroth16AuthV2) SupportedCircuits() []string { | ||
| seen := make(map[string]struct{}, 1+len(m.circuitSubVersions)) | ||
| out := make([]string, 0, 1+len(m.circuitSubVersions)) | ||
|
|
||
| add := func(s string) { | ||
| if _, ok := seen[s]; ok { | ||
| return | ||
| } | ||
| seen[s] = struct{}{} | ||
| out = append(out, s) | ||
| } | ||
|
|
||
| add(m.ProvingMethodAlg.CircuitID) | ||
| for _, s := range m.circuitSubVersions { | ||
| add(s) | ||
| } | ||
| return out | ||
| } |
There was a problem hiding this comment.
The SupportedCircuits() method is duplicated identically between ProvingMethodGroth16AuthV2 and ProvingMethodGroth16AuthV3. Consider extracting this logic into a shared helper function or embedding a common type to reduce code duplication and improve maintainability.
No description provided.