feat(rln): extend error handling for rln module #358
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… error types and propagate errors in calc_witness function
This was
linked to
issues
Dec 9, 2025
Benchmark for 4d261f4Click to view benchmark
|
Benchmark for 4d261f4Click to view benchmark
|
…it, and hasher modules, add PoseidonError type
Benchmark for 1f7375dClick to view benchmark
|
Benchmark for 1f7375dClick to view benchmark
|
Benchmark for d9ce2d9Click to view benchmark
|
Benchmark for d9ce2d9Click to view benchmark
|
…oduce HashError type
vinhtc27
requested review from
Copilot and
seemenkina
and removed request for
seemenkina
Benchmark for 38bb2d9Click to view benchmark
|
There was a problem hiding this comment.
Pull request overview
This PR extends error handling throughout the RLN module by converting hash and witness calculation functions from panic-based error handling to proper Result types. The key changes include introducing structured error types (PoseidonError, HashError, WitnessCalcError), updating all hash function signatures to return Result, and propagating errors through the call chain.
Key Changes:
- Introduced structured error types for Poseidon hashing, general hashing, and witness calculation operations
- Converted
poseidon_hash,hash_to_field_le,hash_to_field_be, and keygen functions to returnResultinstead of panicking - Updated the
Hashertrait to include an associatedErrortype and modified thehashmethod to returnResult - Propagated error handling changes through Merkle tree operations, proof generation/verification, and FFI interfaces
- Restructured module exports in
utilscrate, removing wildcard re-exports (breaking API change) - Updated error messages for consistency and capitalization
- Made several internal modules public (
rln/src/protocolsubmodules,rln-wasm/srcmodules)
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
utils/src/error.rs |
Introduced new HashError enum wrapping Poseidon and generic hash errors |
utils/src/poseidon/error.rs |
Added PoseidonError enum for input validation errors |
utils/src/poseidon/poseidon_hash.rs |
Updated hash() to return Result<F, PoseidonError> with improved error handling |
utils/src/merkle_tree/merkle_tree.rs |
Extended Hasher trait with Error associated type and fallible hash() method |
utils/src/merkle_tree/error.rs |
Added HashError variant to ZerokitMerkleTreeError |
utils/src/merkle_tree/optimal_merkle_tree.rs |
Propagated hash errors through tree operations and proof computation |
utils/src/merkle_tree/full_merkle_tree.rs |
Updated hash operations to handle errors with parallel processing support |
rln/src/hashers.rs |
Converted hash functions to return Result types with proper error handling |
rln/src/protocol/keygen.rs |
Updated all keygen functions to return Result types |
rln/src/protocol/witness.rs |
Propagated errors through witness and tree root computation |
rln/src/protocol/proof.rs |
Added error handling for witness calculation |
rln/src/circuit/error.rs |
Added WitnessCalcError enum for witness calculation failures |
rln/src/circuit/iden3calc.rs |
Converted witness calculation to proper error handling |
rln/src/error.rs |
Enhanced error types with better documentation and error message formatting |
rln/src/ffi/ffi_utils.rs |
Updated FFI functions to return CResult for proper error propagation to C/foreign interfaces |
rln/src/pm_tree_adapter.rs |
Added TODOs for pmtree error handling limitations |
utils/src/lib.rs |
Removed wildcard re-exports (breaking change) |
rln/src/protocol/mod.rs |
Made submodules public (API surface expansion) |
rln/src/prelude.rs |
Changed to wildcard import (potential naming conflicts) |
| Tests and examples | Updated all call sites to handle new Result types with .unwrap() or proper error handling |
| FFI examples (C/Nim) | Updated to handle new CResult return types |
| WASM bindings | Updated functions to return Result<T, String> |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Benchmark for 38bb2d9Click to view benchmark
|
Benchmark for 99963c3Click to view benchmark
|
Benchmark for 99963c3Click to view benchmark
|
Benchmark for 6e359ecClick to view benchmark
|
Benchmark for 6e359ecClick to view benchmark
|
Collaborator
|
@vinhtc27 could you please also change expect to proper error handle? Because expect return panic if something going wrong |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.