-
Notifications
You must be signed in to change notification settings - Fork 52
Change error type for hash_utils module #773
Description
Currently, the functions defined in the hash_utils module return an error of type SyscallHandlerError
.
For example this one
starknet_in_rust/src/hash_utils.rs
Lines 112 to 131 in a8650ac
pub fn compute_hash_on_elements(vec: &[Felt252]) -> Result<Felt252, SyscallHandlerError> { | |
let mut felt_vec = vec | |
.iter() | |
.map(|num| { | |
FieldElement::from_dec_str(&num.to_str_radix(10)) | |
.map_err(|_| SyscallHandlerError::FailToComputeHash) | |
}) | |
.collect::<Result<Vec<FieldElement>, SyscallHandlerError>>()?; | |
felt_vec.push(FieldElement::from(felt_vec.len())); | |
felt_vec.insert(0, FieldElement::from(0_u16)); | |
let felt_result = felt_vec | |
.into_iter() | |
.reduce(|x, y| pedersen_hash(&x, &y)) | |
.ok_or(SyscallHandlerError::FailToComputeHash)?; | |
let result = Felt252::from_bytes_be(&felt_result.to_bytes_be()); | |
Ok(result) | |
} |
This is not good since the SyscallHandlerError
type is meant to be used inside the SyscallHandler
and the hash_utils
functions only deal with hashes. So we should improve this by modifying the return error types of these functions to be something more general.
The code above uses the map_err
method to override the error returned by the previous reduce
method and replace it with a SyscallHandleError::FailToComputeHash
.
We should fix this by not replacing the error with a SyscallHandlerError one but returning something meaningful.
Consider creating a new Error type (just a Rust Enum where each variant of the enum is a failure reason for these hash functions) for this.
After that, you need to fix the places where these functions are used (because they are still expecting a SyscallHandlerError).
To fix that you should consider extending SyscallHandlerError enum to add a new case that contains the new error type defined previously for hash functions.