-
Notifications
You must be signed in to change notification settings - Fork 138
Description
Summary
This issue tracks technical debt and code quality improvements identified in the kimchi and signer crates. Addressing these items would improve maintainability, error handling, and debuggability.
1. Error Handling Improvements
Replace panics with proper error types
Several locations use panic!() where returning Result would be more appropriate for library code:
| File | Line | Current | Suggested |
|---|---|---|---|
kimchi/src/prover.rs |
834 | panic!("Bad evaluation") |
Return ProverError::InvalidEvaluationDomain |
kimchi/src/snarky/checked_runner.rs |
526 | panic!("woot") |
Return Err(CompilationError::NoConstraintSystem) |
kimchi/src/snarky/constraint_system.rs |
345, 352 | Panics on double-set | Return Result<(), ConfigurationError> |
Reduce unwrap() usage
The kimchi crate contains approximately 243 unwrap() calls. High-priority locations to address:
constraint_system.rs:1401-1405- Five consecutive unwraps on iterator chunkslagrange_basis_evaluations.rs:278,304,331,355- Domain creationexpr.rs:2035-2036,2101-2102- Cache lookups
2. Code Duplication in Signer
signer/src/schnorr.rs contains two similar functions:
derive_nonce()(lines 275-298)derive_nonce_compatible()(lines 167-237)
Both share ~60% identical code for BLAKE2b hashing and scalar field conversion. Consider extracting shared logic into a private helper function.
3. Function Decomposition
kimchi/src/prover.rs:create_recursive() spans approximately 1,291 lines. Consider decomposing into smaller functions:
- Witness padding and validation (~lines 240-370)
- Lookup constraint setup (~lines 371-594)
- Quotient polynomial computation (~lines 761-887)
- Linearization and evaluation (~lines 1114-1180)
4. Outstanding TODOs
The following TODOs in prover.rs may warrant tracking issues:
| Line | Description |
|---|---|
| 535 | Avoidable interpolation |
| 539 | Switch to lagrange commitments |
| 580 | Avoid storing coefficients |
| 633 | Domain expansion optimization |
| 1116 | Compute linearization polynomial in evaluation form |
Suggested Approach
- Create a custom error enum for prover/verifier errors
- Incrementally replace panics with
?operator - Add
#[must_use]annotations where appropriate - Extract helper functions from
create_recursive() - Consolidate
derive_noncevariants in signer