fix: missing validations and automatic PK/VK generation #177
Conversation
Bisht13
requested review from
ashpect and
Copilot
and removed request for
ashpect
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the CLI validation and key generation capabilities of the recursive verifier by adding input validation and automatic key generation. The changes prevent common user errors and improve the tool's usability by handling missing keys gracefully.
- Added validation to ensure required configuration and R1CS inputs are provided
- Implemented automatic PK/VK generation when keys are not explicitly provided
- Enhanced error handling with descriptive error messages
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pk, vk, err = circuit.GenerateUnsafeKeys() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to generate unsafe keys: %w", err) | ||
| } |
There was a problem hiding this comment.
The automatic generation of 'unsafe' keys in production code poses a security risk. Consider adding a warning log message or requiring explicit user consent before generating unsafe keys, as these should typically only be used for testing or development purposes.
There was a problem hiding this comment.
The call pk, vk, err = circuit.GenerateUnsafeKeys() does not exist in the circuit packag, this function isn’t defined and the code would not compile.
Regarding the generation of pk/vk keys, if they are not explicitly provided, they are generated at this location in the codebase
ashpect
left a comment
ashpect
left a comment
There was a problem hiding this comment.
In general, the CLI will exit with an error message, but the two improved logs do make life easier.
However, the pk/vk generation code is redundant and should be removed. Also, please rename the PR to fix: missing validation.
| pk, vk, err = circuit.GenerateUnsafeKeys() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to generate unsafe keys: %w", err) | ||
| } |
There was a problem hiding this comment.
The call pk, vk, err = circuit.GenerateUnsafeKeys() does not exist in the circuit packag, this function isn’t defined and the code would not compile.
Regarding the generation of pk/vk keys, if they are not explicitly provided, they are generated at this location in the codebase
2 participants
added a few things to make life easier and avoid crashes:
--configisn’t empty.--r1csor--r1cs_url).should save some headaches and prevent silly errors.