From 1de3e2b4e45971d7dc5663eee4705141ac6e9b4a Mon Sep 17 00:00:00 2001 From: fmoletta <99273364+fmoletta@users.noreply.github.com> Date: Mon, 6 Mar 2023 16:21:16 +0200 Subject: [PATCH] Add `MathError` for math operations (#855) * Use only option for Memory.get * Fix some tests + refactor range_check validation * use proper error for get_memory_holes * Move MaybeRelocatable methods get_int_ref & get_reloctable to Option * Fix tests * Clippy * Fix `CairoRunner::write_output` so that it prints missing and relocatable values (#853) * Print relocatables & missing members in write_output * Add test * Move errors outputed by math_utils to MathError * Start moving relocatable operations to MathError * Fix tests * Remove math-related errors from vm error * Move conversion errors to MathError * Move type conversions to MathError * Remove unused errors * Clippy * Clippy * Simplify addition * Simplify addition * Clippy * Add math_errors.rs * Check for overflows in relocatable operations (#859) * Catch possible overflows in Relocatable::add * Move sub implementations to trait impl * Swap sub_usize for - operator * Vheck possible overflows in Add * Fix should_panic test * remove referenced add * Replace Relocatable methods for trait implementations * Catch overflows in mayberelocatable operations * Fix keccak * Clippy --- .../builtin_hint_processor/blake2s_utils.rs | 22 +- .../cairo_keccak/keccak_hints.rs | 14 +- .../builtin_hint_processor/dict_hint_utils.rs | 2 +- .../find_element_hint.rs | 13 +- .../builtin_hint_processor/keccak_utils.rs | 2 +- .../builtin_hint_processor/math_utils.rs | 6 +- .../builtin_hint_processor/pow_utils.rs | 4 +- .../secp/bigint_utils.rs | 6 +- .../builtin_hint_processor/secp/ec_utils.rs | 4 +- .../builtin_hint_processor/secp/signature.rs | 8 +- .../builtin_hint_processor/set.rs | 31 +- .../builtin_hint_processor/sha256_utils.rs | 2 +- .../squash_dict_utils.rs | 4 +- .../builtin_hint_processor/uint256_utils.rs | 18 +- .../builtin_hint_processor/usort.rs | 6 +- src/hint_processor/hint_processor_utils.rs | 25 +- src/math_utils.rs | 40 +- src/types/errors/math_errors.rs | 54 +++ src/types/errors/mod.rs | 1 + src/types/relocatable.rs | 418 ++++++++---------- src/vm/context/run_context.rs | 38 +- src/vm/errors/hint_errors.rs | 7 +- src/vm/errors/memory_errors.rs | 9 +- src/vm/errors/runner_errors.rs | 11 +- src/vm/errors/trace_errors.rs | 2 +- src/vm/errors/vm_errors.rs | 42 +- src/vm/runners/builtin_runner/bitwise.rs | 7 +- src/vm/runners/builtin_runner/ec_op.rs | 9 +- src/vm/runners/builtin_runner/hash.rs | 5 +- src/vm/runners/builtin_runner/keccak.rs | 16 +- src/vm/runners/builtin_runner/output.rs | 5 +- src/vm/runners/builtin_runner/range_check.rs | 5 +- src/vm/runners/builtin_runner/signature.rs | 9 +- src/vm/runners/cairo_runner.rs | 13 +- src/vm/vm_core.rs | 42 +- src/vm/vm_memory/memory.rs | 8 +- src/vm/vm_memory/memory_segments.rs | 4 +- 37 files changed, 446 insertions(+), 466 deletions(-) create mode 100644 src/types/errors/math_errors.rs diff --git a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs index 44e847b3f7..7723b41d05 100644 --- a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs +++ b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs @@ -44,11 +44,10 @@ output_ptr should point to the middle of an instance, right after initial_state, which should all have a value at this point, and right before the output portion which will be written by this function.*/ fn compute_blake2s_func(vm: &mut VirtualMachine, output_ptr: Relocatable) -> Result<(), HintError> { - let h = get_fixed_size_u32_array::<8>(&vm.get_integer_range(output_ptr.sub_usize(26)?, 8)?)?; - let message = - get_fixed_size_u32_array::<16>(&vm.get_integer_range(output_ptr.sub_usize(18)?, 16)?)?; - let t = felt_to_u32(vm.get_integer(output_ptr.sub_usize(2)?)?.as_ref())?; - let f = felt_to_u32(vm.get_integer(output_ptr.sub_usize(1)?)?.as_ref())?; + let h = get_fixed_size_u32_array::<8>(&vm.get_integer_range((output_ptr - 26)?, 8)?)?; + let message = get_fixed_size_u32_array::<16>(&vm.get_integer_range((output_ptr - 18)?, 16)?)?; + let t = felt_to_u32(vm.get_integer((output_ptr - 2)?)?.as_ref())?; + let f = felt_to_u32(vm.get_integer((output_ptr - 1)?)?.as_ref())?; let new_state = get_maybe_relocatable_array_from_u32(&blake2s_compress(&h, &message, t, 0, f, 0)); vm.load_data(output_ptr, &new_state) @@ -155,7 +154,7 @@ pub fn blake2s_add_uint256( } //Insert second batch of data let data = get_maybe_relocatable_array_from_felt(&inner_data); - vm.load_data(data_ptr + 4, &data) + vm.load_data((data_ptr + 4)?, &data) .map_err(HintError::Memory)?; Ok(()) } @@ -198,7 +197,7 @@ pub fn blake2s_add_uint256_bigend( } //Insert second batch of data let data = get_maybe_relocatable_array_from_felt(&inner_data); - vm.load_data(data_ptr + 4, &data) + vm.load_data((data_ptr + 4)?, &data) .map_err(HintError::Memory)?; Ok(()) } @@ -206,7 +205,7 @@ pub fn blake2s_add_uint256_bigend( #[cfg(test)] mod tests { use super::*; - use crate::vm::errors::vm_errors::VirtualMachineError; + use crate::types::errors::math_errors::MathError; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ any_box, @@ -238,9 +237,10 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::CantSubOffset( - 5, 26 - ))) + Err(HintError::Math(MathError::RelocatableSubNegOffset( + x, + y + ))) if x == relocatable!(2,5) && y == 26 ); } diff --git a/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs b/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs index 7a97bad4f2..a6a0646f7e 100644 --- a/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs +++ b/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs @@ -6,7 +6,7 @@ use crate::{ hint_processor_definition::HintReference, }, serde::deserialize_program::ApTracking, - types::relocatable::MaybeRelocatable, + types::{errors::math_errors::MathError, relocatable::MaybeRelocatable}, vm::{ errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, vm_core::VirtualMachine, @@ -14,7 +14,7 @@ use crate::{ }; use felt::Felt; use num_traits::{ToPrimitive, Zero}; -use std::{borrow::Cow, collections::HashMap, ops::Add}; +use std::{borrow::Cow, collections::HashMap}; // Constants in package "starkware.cairo.common.cairo_keccak.keccak". const BYTES_IN_WORD: &str = "starkware.cairo.common.cairo_keccak.keccak.BYTES_IN_WORD"; @@ -53,7 +53,7 @@ pub fn keccak_write_args( .map_err(HintError::Memory)?; let high_args: Vec<_> = high_args.into_iter().map(MaybeRelocatable::from).collect(); - vm.write_arg(inputs_ptr.add(2_i32), &high_args) + vm.write_arg((inputs_ptr + 2_i32)?, &high_args) .map_err(HintError::Memory)?; Ok(()) @@ -144,7 +144,7 @@ pub fn block_permutation( let keccak_state_size_felts = keccak_state_size_felts.to_usize().unwrap(); let values = vm.get_range( - keccak_ptr.sub_usize(keccak_state_size_felts)?, + (keccak_ptr - keccak_state_size_felts)?, keccak_state_size_felts, ); @@ -233,9 +233,9 @@ pub(crate) fn maybe_reloc_vec_to_u64_array( .iter() .map(|n| match n { Some(Cow::Owned(MaybeRelocatable::Int(ref num))) - | Some(Cow::Borrowed(MaybeRelocatable::Int(ref num))) => { - num.to_u64().ok_or(VirtualMachineError::BigintToU64Fail) - } + | Some(Cow::Borrowed(MaybeRelocatable::Int(ref num))) => num + .to_u64() + .ok_or_else(|| MathError::FeltToU64Conversion(num.clone()).into()), _ => Err(VirtualMachineError::ExpectedIntAtRange( n.as_ref().map(|x| x.as_ref().to_owned()), )), diff --git a/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs b/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs index a8c576c1f6..ab222d1ecf 100644 --- a/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs @@ -138,7 +138,7 @@ pub fn dict_write( let tracker = dict.get_tracker_mut(dict_ptr)?; //dict_ptr is a pointer to a struct, with the ordered fields (key, prev_value, new_value), //dict_ptr.prev_value will be equal to dict_ptr + 1 - let dict_ptr_prev_value = dict_ptr + 1_i32; + let dict_ptr_prev_value = (dict_ptr + 1_i32)?; //Tracker set to track next dictionary entry tracker.current_ptr.offset += DICT_ACCESS_SIZE; //Get previous value diff --git a/src/hint_processor/builtin_hint_processor/find_element_hint.rs b/src/hint_processor/builtin_hint_processor/find_element_hint.rs index 7d9021742a..1e559fa252 100644 --- a/src/hint_processor/builtin_hint_processor/find_element_hint.rs +++ b/src/hint_processor/builtin_hint_processor/find_element_hint.rs @@ -8,11 +8,8 @@ use crate::{ hint_processor_utils::felt_to_usize, }, serde::deserialize_program::ApTracking, - types::exec_scope::ExecutionScopes, - vm::{ - errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, - vm_core::VirtualMachine, - }, + types::{errors::math_errors::MathError, exec_scope::ExecutionScopes}, + vm::{errors::hint_errors::HintError, vm_core::VirtualMachine}, }; use felt::Felt; use num_traits::{Signed, ToPrimitive}; @@ -39,7 +36,7 @@ pub fn find_element( if let Some(find_element_index_value) = find_element_index { let find_element_index_usize = felt_to_usize(&find_element_index_value)?; let found_key = vm - .get_integer(array_start + (elm_size * find_element_index_usize)) + .get_integer((array_start + (elm_size * find_element_index_usize))?) .map_err(|_| HintError::KeyNotFound)?; if found_key.as_ref() != key.as_ref() { @@ -67,11 +64,11 @@ pub fn find_element( } let n_elms_iter: i32 = n_elms .to_i32() - .ok_or_else(|| VirtualMachineError::OffsetExceeded(n_elms.into_owned()))?; + .ok_or_else(|| MathError::FeltToI32Conversion(n_elms.into_owned()))?; for i in 0..n_elms_iter { let iter_key = vm - .get_integer(array_start + (elm_size * i as usize)) + .get_integer((array_start + (elm_size * i as usize))?) .map_err(|_| HintError::KeyNotFound)?; if iter_key.as_ref() == key.as_ref() { diff --git a/src/hint_processor/builtin_hint_processor/keccak_utils.rs b/src/hint_processor/builtin_hint_processor/keccak_utils.rs index f6bbfee3a6..38a5bb348c 100644 --- a/src/hint_processor/builtin_hint_processor/keccak_utils.rs +++ b/src/hint_processor/builtin_hint_processor/keccak_utils.rs @@ -144,7 +144,7 @@ pub fn unsafe_keccak_finalize( offset: keccak_state_ptr.offset + 1, })?; - let n_elems = end_ptr.sub(&start_ptr)?; + let n_elems = (end_ptr - start_ptr)?; let mut keccak_input = Vec::new(); let range = vm.get_integer_range(start_ptr, n_elems)?; diff --git a/src/hint_processor/builtin_hint_processor/math_utils.rs b/src/hint_processor/builtin_hint_processor/math_utils.rs index e6a3e3fca2..f99246b10e 100644 --- a/src/hint_processor/builtin_hint_processor/math_utils.rs +++ b/src/hint_processor/builtin_hint_processor/math_utils.rs @@ -121,10 +121,10 @@ pub fn assert_le_felt( let (q_0, r_0) = (lengths_and_indices[0].0).div_mod_floor(prime_over_3_high); let (q_1, r_1) = (lengths_and_indices[1].0).div_mod_floor(prime_over_2_high); - vm.insert_value(range_check_ptr + 1_i32, q_0)?; + vm.insert_value((range_check_ptr + 1_i32)?, q_0)?; vm.insert_value(range_check_ptr, r_0)?; - vm.insert_value(range_check_ptr + 3_i32, q_1)?; - vm.insert_value(range_check_ptr + 2_i32, r_1)?; + vm.insert_value((range_check_ptr + 3_i32)?, q_1)?; + vm.insert_value((range_check_ptr + 2_i32)?, r_1)?; Ok(()) } diff --git a/src/hint_processor/builtin_hint_processor/pow_utils.rs b/src/hint_processor/builtin_hint_processor/pow_utils.rs index cb903b1d3e..6de8b9e877 100644 --- a/src/hint_processor/builtin_hint_processor/pow_utils.rs +++ b/src/hint_processor/builtin_hint_processor/pow_utils.rs @@ -22,7 +22,9 @@ pub fn pow( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let prev_locs_exp = vm - .get_integer(get_relocatable_from_var_name("prev_locs", vm, ids_data, ap_tracking)? + 4_i32) + .get_integer( + (get_relocatable_from_var_name("prev_locs", vm, ids_data, ap_tracking)? + 4_i32)?, + ) .map_err(|_| { HintError::IdentifierHasNoMember("prev_locs".to_string(), "exp".to_string()) })?; diff --git a/src/hint_processor/builtin_hint_processor/secp/bigint_utils.rs b/src/hint_processor/builtin_hint_processor/secp/bigint_utils.rs index e76f4e4dae..68670332bd 100644 --- a/src/hint_processor/builtin_hint_processor/secp/bigint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/bigint_utils.rs @@ -33,10 +33,10 @@ impl BigInt3<'_> { d0: vm.get_integer(addr).map_err(|_| { HintError::IdentifierHasNoMember(name.to_string(), "d0".to_string()) })?, - d1: vm.get_integer(addr + 1).map_err(|_| { + d1: vm.get_integer((addr + 1)?).map_err(|_| { HintError::IdentifierHasNoMember(name.to_string(), "d1".to_string()) })?, - d2: vm.get_integer(addr + 2).map_err(|_| { + d2: vm.get_integer((addr + 2)?).map_err(|_| { HintError::IdentifierHasNoMember(name.to_string(), "d2".to_string()) })?, }) @@ -91,7 +91,7 @@ pub fn bigint_to_uint256( ) -> Result<(), HintError> { let x_struct = get_relocatable_from_var_name("x", vm, ids_data, ap_tracking)?; let d0 = vm.get_integer(x_struct)?; - let d1 = vm.get_integer(x_struct + 1_i32)?; + let d1 = vm.get_integer((x_struct + 1_i32)?)?; let d0 = d0.as_ref(); let d1 = d1.as_ref(); let base_86 = constants diff --git a/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs b/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs index 6e1f3b3a17..cad7bd38a1 100644 --- a/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs @@ -40,7 +40,7 @@ impl EcPoint<'_> { let point_addr = get_relocatable_from_var_name(name, vm, ids_data, ap_tracking)?; Ok(EcPoint { x: BigInt3::from_base_addr(point_addr, &format!("{}.x", name), vm)?, - y: BigInt3::from_base_addr(point_addr + 3, &format!("{}.y", name), vm)?, + y: BigInt3::from_base_addr((point_addr + 3)?, &format!("{}.y", name), vm)?, }) } } @@ -71,7 +71,7 @@ pub fn ec_negate( .to_bigint(); //ids.point - let point_y = get_relocatable_from_var_name("point", vm, ids_data, ap_tracking)? + 3i32; + let point_y = (get_relocatable_from_var_name("point", vm, ids_data, ap_tracking)? + 3i32)?; let y_bigint3 = BigInt3::from_base_addr(point_y, "point.y", vm)?; let y = pack(y_bigint3); let value = (-y).mod_floor(&secp_p); diff --git a/src/hint_processor/builtin_hint_processor/secp/signature.rs b/src/hint_processor/builtin_hint_processor/secp/signature.rs index f3095dbe30..25d3375c70 100644 --- a/src/hint_processor/builtin_hint_processor/secp/signature.rs +++ b/src/hint_processor/builtin_hint_processor/secp/signature.rs @@ -148,6 +148,7 @@ pub fn get_point_from_x( #[cfg(test)] mod tests { use super::*; + use crate::types::errors::math_errors::MathError; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ any_box, @@ -160,10 +161,7 @@ mod tests { }, types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable}, utils::test_utils::*, - vm::{ - errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, - vm_memory::memory::Memory, - }, + vm::{errors::memory_errors::MemoryError, vm_memory::memory::Memory}, }; use assert_matches::assert_matches; use num_traits::Zero; @@ -222,7 +220,7 @@ mod tests { .collect() ), Err( - HintError::Internal(VirtualMachineError::SafeDivFailBigInt( + HintError::Math(MathError::SafeDivFailBigInt( x, y, ) diff --git a/src/hint_processor/builtin_hint_processor/set.rs b/src/hint_processor/builtin_hint_processor/set.rs index 34463b7ed6..330f1435d3 100644 --- a/src/hint_processor/builtin_hint_processor/set.rs +++ b/src/hint_processor/builtin_hint_processor/set.rs @@ -6,11 +6,8 @@ use crate::{ hint_processor_definition::HintReference, }, serde::deserialize_program::ApTracking, - types::relocatable::MaybeRelocatable, - vm::{ - errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, - vm_core::VirtualMachine, - }, + types::{errors::math_errors::MathError, relocatable::MaybeRelocatable}, + vm::{errors::hint_errors::HintError, vm_core::VirtualMachine}, }; use felt::Felt; use num_traits::{One, ToPrimitive, Zero}; @@ -22,14 +19,18 @@ pub fn set_add( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let set_ptr = get_ptr_from_var_name("set_ptr", vm, ids_data, ap_tracking)?; - let elm_size = get_integer_from_var_name("elm_size", vm, ids_data, ap_tracking)? - .to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail)?; + let elm_size = + get_integer_from_var_name("elm_size", vm, ids_data, ap_tracking).and_then(|x| { + x.to_usize() + .ok_or_else(|| MathError::FeltToUsizeConversion(x.into_owned()).into()) + })?; let elm_ptr = get_ptr_from_var_name("elm_ptr", vm, ids_data, ap_tracking)?; let set_end_ptr = get_ptr_from_var_name("set_end_ptr", vm, ids_data, ap_tracking)?; if elm_size.is_zero() { - Err(VirtualMachineError::ValueNotPositive(Felt::new(elm_size)))?; + Err(HintError::AssertionFailed(String::from( + "assert ids.elm_size > 0", + )))?; } let elm = vm.get_range(elm_ptr, elm_size); @@ -40,10 +41,10 @@ pub fn set_add( )); } - let range_limit = set_end_ptr.sub(&set_ptr)?; + let range_limit = (set_end_ptr - set_ptr)?; for i in (0..range_limit).step_by(elm_size) { - let set_iter = vm.get_range(set_ptr + i, elm_size); + let set_iter = vm.get_range((set_ptr + i)?, elm_size); if set_iter == elm { insert_value_from_var_name( @@ -154,7 +155,7 @@ mod tests { let (mut vm, ids_data) = init_vm_ids_data(None, Some(-2), None, None); assert_matches!( run_hint!(vm, ids_data, HINT_CODE), - Err(HintError::Internal(VirtualMachineError::BigintToUsizeFail)) + Err(HintError::Math(MathError::FeltToUsizeConversion(_))) ); } @@ -163,9 +164,9 @@ mod tests { let (mut vm, ids_data) = init_vm_ids_data(None, Some(0), None, None); assert_matches!( run_hint!(vm, ids_data, HINT_CODE), - Err(HintError::Internal(VirtualMachineError::ValueNotPositive( - int - ))) if int.is_zero() + Err(HintError::AssertionFailed( + m + )) if m == *"assert ids.elm_size > 0" ); } #[test] diff --git a/src/hint_processor/builtin_hint_processor/sha256_utils.rs b/src/hint_processor/builtin_hint_processor/sha256_utils.rs index df8672e035..ee7f92a31f 100644 --- a/src/hint_processor/builtin_hint_processor/sha256_utils.rs +++ b/src/hint_processor/builtin_hint_processor/sha256_utils.rs @@ -56,7 +56,7 @@ pub fn sha256_main( let mut message: Vec = Vec::with_capacity(4 * SHA256_INPUT_CHUNK_SIZE_FELTS); for i in 0..SHA256_INPUT_CHUNK_SIZE_FELTS { - let input_element = vm.get_integer(input_ptr + i)?; + let input_element = vm.get_integer((input_ptr + i)?)?; let bytes = felt_to_u32(input_element.as_ref())?.to_be_bytes(); message.extend(bytes); } diff --git a/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs b/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs index e24847c735..d12193e669 100644 --- a/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs +++ b/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs @@ -141,7 +141,7 @@ pub fn squash_dict_inner_continue_loop( }; //loop_temps.delta_minus1 = loop_temps + 3 as it is the fourth field of the struct //Insert loop_temps.delta_minus1 into memory - let should_continue_addr = loop_temps_addr + 3_i32; + let should_continue_addr = (loop_temps_addr + 3_i32)?; vm.insert_value(should_continue_addr, should_continue) .map_err(HintError::Memory) } @@ -265,7 +265,7 @@ pub fn squash_dict( //A map from key to the list of indices accessing it. let mut access_indices = HashMap::>::new(); for i in 0..n_accesses_usize { - let key_addr = address + DICT_ACCESS_SIZE * i; + let key_addr = (address + DICT_ACCESS_SIZE * i)?; let key = vm .get_integer(key_addr) .map_err(|_| MemoryError::ExpectedInteger(key_addr))?; diff --git a/src/hint_processor/builtin_hint_processor/uint256_utils.rs b/src/hint_processor/builtin_hint_processor/uint256_utils.rs index d7ba38457d..f358b78713 100644 --- a/src/hint_processor/builtin_hint_processor/uint256_utils.rs +++ b/src/hint_processor/builtin_hint_processor/uint256_utils.rs @@ -33,9 +33,9 @@ pub fn uint256_add( let a_relocatable = get_relocatable_from_var_name("a", vm, ids_data, ap_tracking)?; let b_relocatable = get_relocatable_from_var_name("b", vm, ids_data, ap_tracking)?; let a_low = vm.get_integer(a_relocatable)?; - let a_high = vm.get_integer(a_relocatable + 1_usize)?; + let a_high = vm.get_integer((a_relocatable + 1_usize)?)?; let b_low = vm.get_integer(b_relocatable)?; - let b_high = vm.get_integer(b_relocatable + 1_usize)?; + let b_high = vm.get_integer((b_relocatable + 1_usize)?)?; let a_low = a_low.as_ref(); let a_high = a_high.as_ref(); let b_low = b_low.as_ref(); @@ -105,7 +105,7 @@ pub fn uint256_sqrt( let n_addr = get_relocatable_from_var_name("n", vm, ids_data, ap_tracking)?; let root_addr = get_relocatable_from_var_name("root", vm, ids_data, ap_tracking)?; let n_low = vm.get_integer(n_addr)?; - let n_high = vm.get_integer(n_addr + 1_usize)?; + let n_high = vm.get_integer((n_addr + 1_usize)?)?; let n_low = n_low.as_ref(); let n_high = n_high.as_ref(); @@ -127,7 +127,7 @@ pub fn uint256_sqrt( ))); } vm.insert_value(root_addr, Felt::new(root))?; - vm.insert_value(root_addr + 1_i32, Felt::zero()) + vm.insert_value((root_addr + 1_i32)?, Felt::zero()) .map_err(HintError::Memory) } @@ -141,7 +141,7 @@ pub fn uint256_signed_nn( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let a_addr = get_relocatable_from_var_name("a", vm, ids_data, ap_tracking)?; - let a_high = vm.get_integer(a_addr + 1_usize)?; + let a_high = vm.get_integer((a_addr + 1_usize)?)?; //Main logic //memory[ap] = 1 if 0 <= (ids.a.high % PRIME) < 2 ** 127 else 0 let result: Felt = if !a_high.is_negative() && a_high.as_ref() <= &Felt::new(i128::MAX) { @@ -176,9 +176,9 @@ pub fn uint256_unsigned_div_rem( let remainder_addr = get_relocatable_from_var_name("remainder", vm, ids_data, ap_tracking)?; let a_low = vm.get_integer(a_addr)?; - let a_high = vm.get_integer(a_addr + 1_usize)?; + let a_high = vm.get_integer((a_addr + 1_usize)?)?; let div_low = vm.get_integer(div_addr)?; - let div_high = vm.get_integer(div_addr + 1_usize)?; + let div_high = vm.get_integer((div_addr + 1_usize)?)?; let a_low = a_low.as_ref(); let a_high = a_high.as_ref(); let div_low = div_low.as_ref(); @@ -208,11 +208,11 @@ pub fn uint256_unsigned_div_rem( //Insert ids.quotient.low vm.insert_value(quotient_addr, quotient_low)?; //Insert ids.quotient.high - vm.insert_value(quotient_addr + 1_i32, quotient_high)?; + vm.insert_value((quotient_addr + 1_i32)?, quotient_high)?; //Insert ids.remainder.low vm.insert_value(remainder_addr, remainder_low)?; //Insert ids.remainder.high - vm.insert_value(remainder_addr + 1_i32, remainder_high)?; + vm.insert_value((remainder_addr + 1_i32)?, remainder_high)?; Ok(()) } diff --git a/src/hint_processor/builtin_hint_processor/usort.rs b/src/hint_processor/builtin_hint_processor/usort.rs index 5df0fee8bc..72c8b8d6be 100644 --- a/src/hint_processor/builtin_hint_processor/usort.rs +++ b/src/hint_processor/builtin_hint_processor/usort.rs @@ -48,7 +48,7 @@ pub fn usort_body( let mut positions_dict: HashMap> = HashMap::new(); let mut output: Vec = Vec::new(); for i in 0..input_len_u64 { - let val = vm.get_integer(input_ptr + i as usize)?.into_owned(); + let val = vm.get_integer((input_ptr + i as usize)?)?.into_owned(); if let Err(output_index) = output.binary_search(&val) { output.insert(output_index, val.clone()); } @@ -65,11 +65,11 @@ pub fn usort_body( let output_len = output.len(); for (i, sorted_element) in output.into_iter().enumerate() { - vm.insert_value(output_base + i, sorted_element)?; + vm.insert_value((output_base + i)?, sorted_element)?; } for (i, repetition_amount) in multiplicities.into_iter().enumerate() { - vm.insert_value(multiplicities_base + i, Felt::new(repetition_amount))?; + vm.insert_value((multiplicities_base + i)?, Felt::new(repetition_amount))?; } insert_value_from_var_name( diff --git a/src/hint_processor/hint_processor_utils.rs b/src/hint_processor/hint_processor_utils.rs index 04a13ded80..0a846e8b4a 100644 --- a/src/hint_processor/hint_processor_utils.rs +++ b/src/hint_processor/hint_processor_utils.rs @@ -1,13 +1,11 @@ use crate::{ serde::deserialize_program::{ApTracking, OffsetValue}, types::{ + errors::math_errors::MathError, instruction::Register, relocatable::{MaybeRelocatable, Relocatable}, }, - vm::{ - errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, - vm_core::VirtualMachine, - }, + vm::{errors::hint_errors::HintError, vm_core::VirtualMachine}, }; use std::borrow::Cow; @@ -114,9 +112,9 @@ pub fn compute_addr_from_reference( &hint_reference.offset2, )?; - Some(offset1 + value.get_int_ref()?.to_usize()?) + Some((offset1 + value.get_int_ref()?.to_usize()?).ok()?) } - OffsetValue::Value(value) => Some(offset1 + *value), + OffsetValue::Value(value) => Some((offset1 + *value).ok()?), _ => None, } } @@ -131,18 +129,19 @@ fn apply_ap_tracking_correction( return None; } let ap_diff = hint_ap_tracking.offset - ref_ap_tracking.offset; - ap.sub_usize(ap_diff).ok() + (ap - ap_diff).ok() } //Tries to convert a Felt value to usize -pub fn felt_to_usize(felt: &Felt) -> Result { +pub fn felt_to_usize(felt: &Felt) -> Result { felt.to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail) + .ok_or_else(|| MathError::FeltToUsizeConversion(felt.clone())) } ///Tries to convert a Felt value to u32 -pub fn felt_to_u32(felt: &Felt) -> Result { - felt.to_u32().ok_or(VirtualMachineError::BigintToU32Fail) +pub fn felt_to_u32(felt: &Felt) -> Result { + felt.to_u32() + .ok_or_else(|| MathError::FeltToU32Conversion(felt.clone())) } fn get_offset_value_reference( @@ -169,9 +168,9 @@ fn get_offset_value_reference( } if *deref { - vm.get_maybe(&(base_addr + *offset)) + vm.get_maybe(&(base_addr + *offset).ok()?) } else { - Some((base_addr + *offset).into()) + Some((base_addr + *offset).ok()?.into()) } } diff --git a/src/math_utils.rs b/src/math_utils.rs index 8a03cb678c..70633c449a 100644 --- a/src/math_utils.rs +++ b/src/math_utils.rs @@ -1,4 +1,4 @@ -use crate::vm::errors::vm_errors::VirtualMachineError; +use crate::types::errors::math_errors::MathError; use felt::Felt; use num_bigint::{BigInt, BigUint}; use num_integer::Integer; @@ -8,7 +8,7 @@ use std::ops::Shr; ///Returns the integer square root of the nonnegative integer n. ///This is the floor of the exact square root of n. ///Unlike math.sqrt(), this function doesn't have rounding error issues. -pub fn isqrt(n: &BigUint) -> Result { +pub fn isqrt(n: &BigUint) -> Result { /* # The following algorithm was copied from # https://stackoverflow.com/questions/15390807/integer-square-root-in-python. x = n @@ -29,51 +29,51 @@ pub fn isqrt(n: &BigUint) -> Result { } if !(&x.pow(2) <= n && n < &(&x + 1_u32).pow(2_u32)) { - return Err(VirtualMachineError::FailedToGetSqrt(n.clone())); + return Err(MathError::FailedToGetSqrt(n.clone())); }; Ok(x) } /// Performs integer division between x and y; fails if x is not divisible by y. -pub fn safe_div(x: &Felt, y: &Felt) -> Result { +pub fn safe_div(x: &Felt, y: &Felt) -> Result { if y.is_zero() { - return Err(VirtualMachineError::DividedByZero); + return Err(MathError::DividedByZero); } let (q, r) = x.div_mod_floor(y); if !r.is_zero() { - return Err(VirtualMachineError::SafeDivFail(x.clone(), y.clone())); + return Err(MathError::SafeDivFail(x.clone(), y.clone())); } Ok(q) } /// Performs integer division between x and y; fails if x is not divisible by y. -pub fn safe_div_bigint(x: &BigInt, y: &BigInt) -> Result { +pub fn safe_div_bigint(x: &BigInt, y: &BigInt) -> Result { if y.is_zero() { - return Err(VirtualMachineError::DividedByZero); + return Err(MathError::DividedByZero); } let (q, r) = x.div_mod_floor(y); if !r.is_zero() { - return Err(VirtualMachineError::SafeDivFailBigInt(x.clone(), y.clone())); + return Err(MathError::SafeDivFailBigInt(x.clone(), y.clone())); } Ok(q) } /// Performs integer division between x and y; fails if x is not divisible by y. -pub fn safe_div_usize(x: usize, y: usize) -> Result { +pub fn safe_div_usize(x: usize, y: usize) -> Result { if y.is_zero() { - return Err(VirtualMachineError::DividedByZero); + return Err(MathError::DividedByZero); } let (q, r) = x.div_mod_floor(&y); if !r.is_zero() { - return Err(VirtualMachineError::SafeDivFailUsize(x, y)); + return Err(MathError::SafeDivFailUsize(x, y)); } Ok(q) @@ -239,7 +239,7 @@ mod tests { let result = safe_div(&x, &y); assert_matches!( result, - Err(VirtualMachineError::SafeDivFail( + Err(MathError::SafeDivFail( i, j )) if i == Felt::new(25) && j == Felt::new(4)); } @@ -249,7 +249,7 @@ mod tests { let x = Felt::new(25); let y = Felt::zero(); let result = safe_div(&x, &y); - assert_matches!(result, Err(VirtualMachineError::DividedByZero)); + assert_matches!(result, Err(MathError::DividedByZero)); } #[test] @@ -261,16 +261,13 @@ mod tests { fn compute_safe_div_usize_non_divisor() { assert_matches!( safe_div_usize(25, 4), - Err(VirtualMachineError::SafeDivFailUsize(25, 4)) + Err(MathError::SafeDivFailUsize(25, 4)) ); } #[test] fn compute_safe_div_usize_by_zero() { - assert_matches!( - safe_div_usize(25, 0), - Err(VirtualMachineError::DividedByZero) - ); + assert_matches!(safe_div_usize(25, 0), Err(MathError::DividedByZero)); } #[test] @@ -541,9 +538,6 @@ mod tests { fn safe_div_bigint_by_zero() { let x = BigInt::one(); let y = BigInt::zero(); - assert_matches!( - safe_div_bigint(&x, &y), - Err(VirtualMachineError::DividedByZero) - ) + assert_matches!(safe_div_bigint(&x, &y), Err(MathError::DividedByZero)) } } diff --git a/src/types/errors/math_errors.rs b/src/types/errors/math_errors.rs new file mode 100644 index 0000000000..bb7accb49a --- /dev/null +++ b/src/types/errors/math_errors.rs @@ -0,0 +1,54 @@ +use felt::Felt; +use num_bigint::{BigInt, BigUint}; +use thiserror::Error; + +use crate::types::relocatable::{MaybeRelocatable, Relocatable}; + +#[derive(Debug, Error, PartialEq)] +pub enum MathError { + // Math functions + #[error("Can't calculate the square root of negative number: {0})")] + SqrtNegative(Felt), + #[error("{0} is not divisible by {1}")] + SafeDivFail(Felt, Felt), + #[error("{0} is not divisible by {1}")] + SafeDivFailBigInt(BigInt, BigInt), + #[error("{0} is not divisible by {1}")] + SafeDivFailBigUint(BigUint, BigUint), + #[error("{0} is not divisible by {1}")] + SafeDivFailU32(u32, u32), + #[error("Attempted to divide by zero")] + SafeDivFailUsize(usize, usize), + #[error("Attempted to divide by zero")] + DividedByZero, + #[error("Failed to calculate the square root of: {0})")] + FailedToGetSqrt(BigUint), + // Relocatable Operations + #[error("Cant convert felt: {0} to Relocatable")] + FeltToRelocatable(Felt), + #[error("Operation failed: {0} - {1}, offsets cant be negative")] + RelocatableSubNegOffset(Relocatable, usize), + #[error("Operation failed: {0} + {1}, maximum offset value exceeded")] + RelocatableAddFeltOffsetExceeded(Relocatable, Felt), + #[error("Operation failed: {0} + {1}, maximum offset value exceeded")] + RelocatableAddUsizeOffsetExceeded(Relocatable, usize), + #[error("Operation failed: {0} + {1}, cant add two relocatable values")] + RelocatableAdd(Relocatable, Relocatable), + #[error("Operation failed: {0} - {1}, cant substract two relocatable values with different segment indexes")] + RelocatableSubDiffIndex(Relocatable, Relocatable), + #[error( + "Operation failed: {0}.divmod({1}, divmod can only be performed between two integer values" + )] + DivModWrongType(MaybeRelocatable, MaybeRelocatable), + #[error("Operation failed {0} - {1}, cant substract a relocatable value from an integer")] + SubRelocatableFromInt(Felt, Relocatable), + // Type conversions + #[error("Conversion to i32 failed for Felt {0}")] + FeltToI32Conversion(Felt), + #[error("Conversion to u32 failed for Felt {0}")] + FeltToU32Conversion(Felt), + #[error("Conversion to usize failed for Felt {0}")] + FeltToUsizeConversion(Felt), + #[error("Conversion to u64 failed for Felt {0}")] + FeltToU64Conversion(Felt), +} diff --git a/src/types/errors/mod.rs b/src/types/errors/mod.rs index 71caf778a3..fab63a949b 100644 --- a/src/types/errors/mod.rs +++ b/src/types/errors/mod.rs @@ -1 +1,2 @@ +pub mod math_errors; pub mod program_errors; diff --git a/src/types/relocatable.rs b/src/types/relocatable.rs index 0321f0d6a0..2b3bae84fc 100644 --- a/src/types/relocatable.rs +++ b/src/types/relocatable.rs @@ -1,13 +1,12 @@ use crate::{ - relocatable, - vm::errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, + relocatable, types::errors::math_errors::MathError, vm::errors::memory_errors::MemoryError, }; use felt::Felt; use num_traits::{ToPrimitive, Zero}; use serde::{Deserialize, Serialize}; use std::{ fmt::{self, Display}, - ops::{Add, AddAssign}, + ops::{Add, AddAssign, Sub}, }; #[derive(Eq, Hash, PartialEq, PartialOrd, Clone, Copy, Debug, Serialize, Deserialize)] @@ -89,12 +88,16 @@ impl Display for Relocatable { } impl Add for Relocatable { - type Output = Relocatable; - fn add(self, other: usize) -> Self { - relocatable!(self.segment_index, self.offset + other) + type Output = Result; + fn add(self, other: usize) -> Result { + self.offset + .checked_add(other) + .map(|x| Relocatable::from((self.segment_index, x))) + .ok_or(MathError::RelocatableAddUsizeOffsetExceeded(self, other)) } } +/// Warning: may panic if self.offset + rhs exceeds usize::MAX impl AddAssign for Relocatable { fn add_assign(&mut self, rhs: usize) { self.offset += rhs @@ -102,30 +105,65 @@ impl AddAssign for Relocatable { } impl Add for Relocatable { - type Output = Relocatable; - fn add(self, other: i32) -> Self { + type Output = Result; + fn add(self, other: i32) -> Result { if other >= 0 { - relocatable!(self.segment_index, self.offset + other as usize) + self + other as usize } else { - relocatable!( - self.segment_index, - self.offset - other.unsigned_abs() as usize - ) + self - other.unsigned_abs() as usize } } } +impl Add<&Felt> for Relocatable { + type Output = Result; + fn add(self, other: &Felt) -> Result { + let big_offset = other + self.offset; + let new_offset = big_offset + .to_usize() + .ok_or_else(|| MathError::RelocatableAddFeltOffsetExceeded(self, other.clone()))?; + Ok(Relocatable { + segment_index: self.segment_index, + offset: new_offset, + }) + } +} -impl Add for &Relocatable { - type Output = Relocatable; - fn add(self, other: i32) -> Relocatable { - if other >= 0 { - relocatable!(self.segment_index, self.offset + other as usize) - } else { - relocatable!( - self.segment_index, - self.offset - other.unsigned_abs() as usize - ) +/// Adds a MaybeRelocatable to self +/// Cant add two relocatable values +impl Add<&MaybeRelocatable> for Relocatable { + type Output = Result; + fn add(self, other: &MaybeRelocatable) -> Result { + let num_ref = match other { + MaybeRelocatable::RelocatableValue(rel) => { + return Err(MathError::RelocatableAdd(self, *rel)) + } + MaybeRelocatable::Int(num) => num, + }; + self + num_ref + } +} + +impl Sub for Relocatable { + type Output = Result; + fn sub(self, other: usize) -> Result { + if self.offset < other { + return Err(MathError::RelocatableSubNegOffset(self, other)); } + let new_offset = self.offset - other; + Ok(relocatable!(self.segment_index, new_offset)) + } +} +impl Sub for Relocatable { + type Output = Result; + fn sub(self, other: Self) -> Result { + if self.segment_index != other.segment_index { + return Err(MathError::RelocatableSubDiffIndex(self, other)); + } + if self.offset < other.offset { + return Err(MathError::RelocatableSubNegOffset(self, other.offset)); + } + let result = self.offset - other.offset; + Ok(result) } } @@ -146,78 +184,25 @@ impl From<&MaybeRelocatable> for MaybeRelocatable { } impl TryFrom<&MaybeRelocatable> for Relocatable { - type Error = MemoryError; - fn try_from(other: &MaybeRelocatable) -> Result { + type Error = MathError; + fn try_from(other: &MaybeRelocatable) -> Result { match other { MaybeRelocatable::RelocatableValue(rel) => Ok(*rel), - _ => Err(MemoryError::AddressNotRelocatable), + MaybeRelocatable::Int(num) => Err(MathError::FeltToRelocatable(num.clone())), } } } -impl Relocatable { - pub fn sub_usize(&self, other: usize) -> Result { - if self.offset < other { - return Err(VirtualMachineError::CantSubOffset(self.offset, other)); - } - let new_offset = self.offset - other; - Ok(relocatable!(self.segment_index, new_offset)) - } - - ///Adds a Felt to self - pub fn add_int(&self, other: &Felt) -> Result { - let big_offset = other + self.offset; - let new_offset = big_offset - .to_usize() - .ok_or(VirtualMachineError::OffsetExceeded(big_offset))?; - Ok(Relocatable { - segment_index: self.segment_index, - offset: new_offset, - }) - } - - /// Adds a MaybeRelocatable to self - /// Cant add two relocatable values - pub fn add_maybe(&self, other: &MaybeRelocatable) -> Result { - let num_ref = other - .get_int_ref() - .ok_or(VirtualMachineError::RelocatableAdd)?; - - let big_offset: Felt = num_ref + self.offset; - let new_offset = big_offset - .to_usize() - .ok_or(VirtualMachineError::OffsetExceeded(big_offset))?; - Ok(Relocatable { - segment_index: self.segment_index, - offset: new_offset, - }) - } - - pub fn sub(&self, other: &Self) -> Result { - if self.segment_index != other.segment_index { - return Err(VirtualMachineError::DiffIndexSub); - } - if self.offset < other.offset { - return Err(VirtualMachineError::CantSubOffset( - self.offset, - other.offset, - )); - } - let result = self.offset - other.offset; - Ok(result) - } -} - impl MaybeRelocatable { /// Adds a Felt to self - pub fn add_int(&self, other: &Felt) -> Result { + pub fn add_int(&self, other: &Felt) -> Result { match *self { MaybeRelocatable::Int(ref value) => Ok(MaybeRelocatable::Int(value + other)), MaybeRelocatable::RelocatableValue(ref rel) => { let big_offset = other + rel.offset; - let new_offset = big_offset - .to_usize() - .ok_or(VirtualMachineError::OffsetExceeded(big_offset))?; + let new_offset = big_offset.to_usize().ok_or_else(|| { + MathError::RelocatableAddFeltOffsetExceeded(*rel, other.clone()) + })?; Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: rel.segment_index, offset: new_offset, @@ -227,39 +212,27 @@ impl MaybeRelocatable { } /// Adds a usize to self - pub fn add_usize(&self, other: usize) -> MaybeRelocatable { - match *self { + pub fn add_usize(&self, other: usize) -> Result { + Ok(match *self { MaybeRelocatable::Int(ref value) => MaybeRelocatable::Int(value + other), - MaybeRelocatable::RelocatableValue(ref rel) => { - MaybeRelocatable::RelocatableValue(Relocatable { - segment_index: rel.segment_index, - offset: rel.offset + other, - }) - } - } + MaybeRelocatable::RelocatableValue(rel) => (rel + other)?.into(), + }) } /// Adds a MaybeRelocatable to self /// Cant add two relocatable values - pub fn add(&self, other: &MaybeRelocatable) -> Result { + pub fn add(&self, other: &MaybeRelocatable) -> Result { match (self, other) { (MaybeRelocatable::Int(num_a_ref), MaybeRelocatable::Int(num_b)) => { Ok(MaybeRelocatable::Int(num_a_ref + num_b)) } - (&MaybeRelocatable::RelocatableValue(_), &MaybeRelocatable::RelocatableValue(_)) => { - Err(VirtualMachineError::RelocatableAdd) - } - (&MaybeRelocatable::RelocatableValue(ref rel), &MaybeRelocatable::Int(ref num_ref)) - | (&MaybeRelocatable::Int(ref num_ref), &MaybeRelocatable::RelocatableValue(ref rel)) => - { - let big_offset: Felt = num_ref + rel.offset; - let new_offset = big_offset - .to_usize() - .ok_or(VirtualMachineError::OffsetExceeded(big_offset))?; - Ok(MaybeRelocatable::RelocatableValue(Relocatable { - segment_index: rel.segment_index, - offset: new_offset, - })) + ( + &MaybeRelocatable::RelocatableValue(rel_a), + &MaybeRelocatable::RelocatableValue(rel_b), + ) => Err(MathError::RelocatableAdd(rel_a, rel_b)), + (&MaybeRelocatable::RelocatableValue(rel), &MaybeRelocatable::Int(ref num_ref)) + | (&MaybeRelocatable::Int(ref num_ref), &MaybeRelocatable::RelocatableValue(rel)) => { + Ok((rel + num_ref)?.into()) } } } @@ -267,7 +240,7 @@ impl MaybeRelocatable { /// Substracts two MaybeRelocatable values and returns the result as a MaybeRelocatable value. /// Only values of the same type may be substracted. /// Relocatable values can only be substracted if they belong to the same segment. - pub fn sub(&self, other: &MaybeRelocatable) -> Result { + pub fn sub(&self, other: &MaybeRelocatable) -> Result { match (self, other) { (MaybeRelocatable::Int(num_a), MaybeRelocatable::Int(num_b)) => { Ok(MaybeRelocatable::Int(num_a - num_b)) @@ -277,21 +250,21 @@ impl MaybeRelocatable { MaybeRelocatable::RelocatableValue(rel_b), ) => { if rel_a.segment_index == rel_b.segment_index { - return Ok(MaybeRelocatable::from(Felt::new( - rel_a.offset - rel_b.offset, - ))); + return Ok(MaybeRelocatable::from(Felt::new((*rel_a - *rel_b)?))); } - Err(VirtualMachineError::DiffIndexSub) + Err(MathError::RelocatableSubDiffIndex(*rel_a, *rel_b)) } (MaybeRelocatable::RelocatableValue(rel_a), MaybeRelocatable::Int(ref num_b)) => { Ok(MaybeRelocatable::from(( rel_a.segment_index, - (rel_a.offset - num_b) - .to_usize() - .ok_or_else(|| VirtualMachineError::OffsetExceeded(rel_a.offset - num_b))?, + (rel_a.offset - num_b).to_usize().ok_or_else(|| { + MathError::RelocatableAddFeltOffsetExceeded(*rel_a, num_b.clone()) + })?, ))) } - _ => Err(VirtualMachineError::NotImplemented), + (MaybeRelocatable::Int(int), MaybeRelocatable::RelocatableValue(rel)) => { + Err(MathError::SubRelocatableFromInt(int.clone(), *rel)) + } } } @@ -300,14 +273,14 @@ impl MaybeRelocatable { pub fn divmod( &self, other: &MaybeRelocatable, - ) -> Result<(MaybeRelocatable, MaybeRelocatable), VirtualMachineError> { + ) -> Result<(MaybeRelocatable, MaybeRelocatable), MathError> { match (self, other) { (MaybeRelocatable::Int(val), MaybeRelocatable::Int(div)) => Ok(( MaybeRelocatable::from(val / div), // NOTE: elements on a field element always have multiplicative inverse MaybeRelocatable::from(Felt::zero()), )), - _ => Err(VirtualMachineError::NotImplemented), + _ => Err(MathError::DivModWrongType(self.clone(), other.clone())), } } @@ -378,7 +351,6 @@ pub fn relocate_address( mod tests { use super::*; use crate::{relocatable, utils::test_utils::mayberelocatable}; - use assert_matches::assert_matches; use felt::felt_str; use num_traits::{One, Zero}; @@ -386,16 +358,13 @@ mod tests { fn add_bigint_to_int() { let addr = MaybeRelocatable::from(Felt::new(7i32)); let added_addr = addr.add_int(&Felt::new(2i32)); - assert_matches!( - added_addr, - Ok(MaybeRelocatable::Int(num)) if num == Felt::new(9) - ); + assert_eq!(added_addr, Ok(MaybeRelocatable::Int(Felt::new(9)))); } #[test] fn add_usize_to_int() { let addr = MaybeRelocatable::from(Felt::new(7_i32)); - let added_addr = addr.add_usize(2); + let added_addr = addr.add_usize(2).unwrap(); assert_eq!(MaybeRelocatable::Int(Felt::new(9)), added_addr); } @@ -403,7 +372,7 @@ mod tests { fn add_bigint_to_relocatable() { let addr = MaybeRelocatable::RelocatableValue(relocatable!(7, 65)); let added_addr = addr.add_int(&Felt::new(2)); - assert_matches!( + assert_eq!( added_addr, Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: 7, @@ -416,11 +385,12 @@ mod tests { fn add_int_mod_offset_exceeded() { let addr = MaybeRelocatable::from((0, 0)); let error = addr.add_int(&felt_str!("18446744073709551616")); - assert_matches!( + assert_eq!( error, - Err(VirtualMachineError::OffsetExceeded(x)) if x == felt_str!( - "18446744073709551616" - ) + Err(MathError::RelocatableAddFeltOffsetExceeded( + relocatable!(0, 0), + felt_str!("18446744073709551616") + )) ); } @@ -428,12 +398,12 @@ mod tests { fn add_usize_to_relocatable() { let addr = MaybeRelocatable::RelocatableValue(relocatable!(7, 65)); let added_addr = addr.add_usize(2); - assert_matches!( + assert_eq!( added_addr, - MaybeRelocatable::RelocatableValue(Relocatable { + Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: 7, offset: 67 - }) + })) ); } @@ -444,10 +414,7 @@ mod tests { 16 )); let added_addr = addr.add_int(&Felt::one()); - assert_matches!( - added_addr, - Ok(MaybeRelocatable::Int(num)) if num == Felt::new(4) - ); + assert_eq!(added_addr, Ok(MaybeRelocatable::Int(Felt::new(4)))); } #[test] @@ -456,7 +423,7 @@ mod tests { let added_addr = addr.add_int(&felt_str!( "3618502788666131213697322783095070105623107215331596699973092056135872020481" )); - assert_matches!( + assert_eq!( added_addr, Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: 1, @@ -472,10 +439,7 @@ mod tests { )); let addr_b = &MaybeRelocatable::from(Felt::new(17_i32)); let added_addr = addr_a.add(addr_b); - assert_matches!( - added_addr, - Ok(MaybeRelocatable::Int(num)) if num == Felt::new(24) - ); + assert_eq!(added_addr, Ok(MaybeRelocatable::Int(Felt::new(24)))); } #[test] @@ -483,7 +447,13 @@ mod tests { let addr_a = &MaybeRelocatable::from((7, 5)); let addr_b = &MaybeRelocatable::RelocatableValue(relocatable!(7, 10)); let error = addr_a.add(addr_b); - assert_matches!(error, Err(VirtualMachineError::RelocatableAdd)); + assert_eq!( + error, + Err(MathError::RelocatableAdd( + relocatable!(7, 5), + relocatable!(7, 10) + )) + ); } #[test] @@ -491,7 +461,7 @@ mod tests { let addr_a = &MaybeRelocatable::from((7, 7)); let addr_b = &MaybeRelocatable::from(Felt::new(10)); let added_addr = addr_a.add(addr_b); - assert_matches!( + assert_eq!( added_addr, Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: 7, @@ -505,7 +475,7 @@ mod tests { let addr_a = &MaybeRelocatable::from(Felt::new(10_i32)); let addr_b = &MaybeRelocatable::RelocatableValue(relocatable!(7, 7)); let added_addr = addr_a.add(addr_b); - assert_matches!( + assert_eq!( added_addr, Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: 7, @@ -522,7 +492,7 @@ mod tests { 16 )); let added_addr = addr_a.add(addr_b); - assert_matches!( + assert_eq!( added_addr, Ok(MaybeRelocatable::RelocatableValue(Relocatable { segment_index: 7, @@ -535,11 +505,12 @@ mod tests { fn add_int_rel_int_offset_exceeded() { let addr = MaybeRelocatable::from((0, 0)); let error = addr.add(&MaybeRelocatable::from(felt_str!("18446744073709551616"))); - assert_matches!( + assert_eq!( error, - Err(VirtualMachineError::OffsetExceeded(x)) if x == felt_str!( - "18446744073709551616" - ) + Err(MathError::RelocatableAddFeltOffsetExceeded( + relocatable!(0, 0), + felt_str!("18446744073709551616") + )) ); } @@ -551,11 +522,12 @@ mod tests { segment_index: 0, }; let error = addr.add(&MaybeRelocatable::RelocatableValue(relocatable)); - assert_matches!( + assert_eq!( error, - Err(VirtualMachineError::OffsetExceeded(x)) if x == felt_str!( - "18446744073709551616" - ) + Err(MathError::RelocatableAddFeltOffsetExceeded( + relocatable!(0, 0), + felt_str!("18446744073709551616") + )) ); } @@ -564,10 +536,7 @@ mod tests { let addr_a = &MaybeRelocatable::from(Felt::new(7)); let addr_b = &MaybeRelocatable::from(Felt::new(5)); let sub_addr = addr_a.sub(addr_b); - assert_matches!( - sub_addr, - Ok(MaybeRelocatable::Int(num)) if num == Felt::new(2) - ); + assert_eq!(sub_addr, Ok(MaybeRelocatable::Int(Felt::new(2)))); } #[test] @@ -575,10 +544,7 @@ mod tests { let addr_a = &MaybeRelocatable::from((7, 17)); let addr_b = &MaybeRelocatable::from((7, 7)); let sub_addr = addr_a.sub(addr_b); - assert_matches!( - sub_addr, - Ok(MaybeRelocatable::Int(num)) if num == Felt::new(10) - ); + assert_eq!(sub_addr, Ok(MaybeRelocatable::Int(Felt::new(10)))); } #[test] @@ -586,10 +552,12 @@ mod tests { let addr_a = &MaybeRelocatable::from((7, 17)); let addr_b = &MaybeRelocatable::from((8, 7)); let error = addr_a.sub(addr_b); - assert_matches!(error, Err(VirtualMachineError::DiffIndexSub)); assert_eq!( - error.unwrap_err().to_string(), - "Can only subtract two relocatable values of the same segment" + error, + Err(MathError::RelocatableSubDiffIndex( + relocatable!(7, 17), + relocatable!(8, 7) + )) ); } @@ -598,14 +566,17 @@ mod tests { let addr_a = &MaybeRelocatable::from((7, 17)); let addr_b = &MaybeRelocatable::from(Felt::new(5_i32)); let addr_c = addr_a.sub(addr_b); - assert_matches!(addr_c, Ok(x) if x == MaybeRelocatable::from((7, 12))); + assert_eq!(addr_c, Ok(MaybeRelocatable::from((7, 12)))); } #[test] fn sub_rel_to_int_error() { - assert_matches!( - &MaybeRelocatable::from(Felt::new(7_i32)).sub(&MaybeRelocatable::from((7, 10))), - Err::(VirtualMachineError::NotImplemented) + assert_eq!( + MaybeRelocatable::from(Felt::new(7_i32)).sub(&MaybeRelocatable::from((7, 10))), + Err(MathError::SubRelocatableFromInt( + Felt::new(7_i32), + Relocatable::from((7, 10)) + )) ); } @@ -622,7 +593,13 @@ mod tests { fn divmod_bad_type() { let value = &MaybeRelocatable::from(Felt::new(10)); let div = &MaybeRelocatable::from((2, 7)); - assert_matches!(value.divmod(div), Err(VirtualMachineError::NotImplemented)); + assert_eq!( + value.divmod(div), + Err(MathError::DivModWrongType( + MaybeRelocatable::from(Felt::new(10)), + MaybeRelocatable::from((2, 7)) + )) + ); } #[test] @@ -681,23 +658,18 @@ mod tests { #[test] fn relocatable_add_int() { - assert_matches!( - relocatable!(1, 2).add_int(&Felt::new(4)), - Ok::(x) if x == relocatable!(1, 6) - ); - assert_matches!( - relocatable!(3, 2).add_int(&Felt::zero()), - Ok::(x) if x == relocatable!(3, 2) - ); + assert_eq!(relocatable!(1, 2) + &Felt::new(4), Ok(relocatable!(1, 6))); + assert_eq!(relocatable!(3, 2) + &Felt::zero(), Ok(relocatable!(3, 2))); } #[test] fn relocatable_add_int_mod_offset_exceeded_error() { - assert_matches!( - relocatable!(0, 0).add_int(&(Felt::new(usize::MAX) + 1_usize)), - Err::(VirtualMachineError::OffsetExceeded( - x - )) if x == Felt::new(usize::MAX) + 1_usize + assert_eq!( + relocatable!(0, 0) + &(Felt::new(usize::MAX) + 1_usize), + Err(MathError::RelocatableAddFeltOffsetExceeded( + relocatable!(0, 0), + Felt::new(usize::MAX) + 1_usize + )) ); } @@ -705,16 +677,18 @@ mod tests { fn relocatable_add_i32() { let reloc = relocatable!(1, 5); - assert_eq!(reloc + 3, relocatable!(1, 8)); - assert_eq!(reloc + (-3), relocatable!(1, 2)); + assert_eq!(reloc + 3, Ok(relocatable!(1, 8))); + assert_eq!(reloc + (-3), Ok(relocatable!(1, 2))); } #[test] - #[should_panic] fn relocatable_add_i32_with_overflow() { let reloc = relocatable!(1, 1); - let _panic = reloc + (-3); + assert_eq!( + reloc + (-3), + Err(MathError::RelocatableSubNegOffset(relocatable!(1, 1), 3)) + ); } #[test] @@ -730,13 +704,10 @@ mod tests { #[test] fn relocatable_sub_rel_test() { let reloc = relocatable!(7, 6); - assert_matches!( - reloc.sub(&relocatable!(7, 5)), - Ok::(1) - ); - assert_matches!( - reloc.sub(&relocatable!(7, 9)), - Err::(VirtualMachineError::CantSubOffset(6, 9)) + assert_eq!(reloc - relocatable!(7, 5), Ok(1)); + assert_eq!( + reloc - relocatable!(7, 9), + Err(MathError::RelocatableSubNegOffset(relocatable!(7, 6), 9)) ); } @@ -744,61 +715,62 @@ mod tests { fn sub_rel_different_indexes() { let a = relocatable!(7, 6); let b = relocatable!(8, 6); - assert_matches!( - a.sub(&b), - Err::(VirtualMachineError::DiffIndexSub) - ); + assert_eq!(a - b, Err(MathError::RelocatableSubDiffIndex(a, b))); } #[test] fn add_maybe_mod_ok() { - assert_matches!( - relocatable!(1, 0).add_maybe(&mayberelocatable!(2)), - Ok::(x) if x == relocatable!(1, 2) + assert_eq!( + relocatable!(1, 0) + &mayberelocatable!(2), + Ok(relocatable!(1, 2)) ); - assert_matches!( - relocatable!(0, 29).add_maybe(&mayberelocatable!(100)), - Ok::(x) if x == relocatable!(0, 129) + assert_eq!( + relocatable!(0, 29) + &mayberelocatable!(100), + Ok(relocatable!(0, 129)) ); - assert_matches!( - relocatable!(2, 12).add_maybe(&mayberelocatable!(104)), - Ok::(x) if x == relocatable!(2, 116) + assert_eq!( + relocatable!(2, 12) + &mayberelocatable!(104), + Ok(relocatable!(2, 116)) ); - assert_matches!( - relocatable!(1, 0).add_maybe(&mayberelocatable!(0)), - Ok::(x) if x == relocatable!(1, 0) + assert_eq!( + relocatable!(1, 0) + &mayberelocatable!(0), + Ok(relocatable!(1, 0)) ); - assert_matches!( - relocatable!(1, 2).add_maybe(&mayberelocatable!(71)), - Ok::(x) if x == relocatable!(1, 73) + assert_eq!( + relocatable!(1, 2) + &mayberelocatable!(71), + Ok(relocatable!(1, 73)) ); } #[test] fn add_maybe_mod_add_two_relocatable_error() { - assert_matches!( - relocatable!(1, 0).add_maybe(&mayberelocatable!(1, 2)), - Err::(VirtualMachineError::RelocatableAdd) + assert_eq!( + relocatable!(1, 0) + &mayberelocatable!(1, 2), + Err(MathError::RelocatableAdd( + relocatable!(1, 0), + relocatable!(1, 2) + )) ); } #[test] fn add_maybe_mod_offset_exceeded_error() { - assert_matches!( - relocatable!(1, 0).add_maybe(&mayberelocatable!(usize::MAX as i128 + 1)), - Err::(VirtualMachineError::OffsetExceeded( - x - )) if x == Felt::new(usize::MAX) + 1_usize + assert_eq!( + relocatable!(1, 0) + &mayberelocatable!(usize::MAX as i128 + 1), + Err(MathError::RelocatableAddFeltOffsetExceeded( + relocatable!(1, 0), + Felt::new(usize::MAX) + 1_usize + )) ); } #[test] fn get_relocatable_test() { - assert_matches!( + assert_eq!( mayberelocatable!(1, 2).get_relocatable(), - Some(x) if x == relocatable!(1, 2) + Some(relocatable!(1, 2)) ); - assert_matches!(mayberelocatable!(3).get_relocatable(), None) + assert_eq!(mayberelocatable!(3).get_relocatable(), None) } #[test] diff --git a/src/vm/context/run_context.rs b/src/vm/context/run_context.rs index 804184d944..7a5839d634 100644 --- a/src/vm/context/run_context.rs +++ b/src/vm/context/run_context.rs @@ -7,7 +7,7 @@ use crate::{ memory_errors::MemoryError::AddressNotRelocatable, vm_errors::VirtualMachineError, }, }; -use num_traits::ToPrimitive; +use num_traits::abs; pub struct RunContext { pub(crate) pc: Relocatable, @@ -34,13 +34,11 @@ impl RunContext { Register::AP => self.get_ap(), Register::FP => self.get_fp(), }; - let new_offset = instruction.off0 + base_addr.offset as isize; - Ok(Relocatable::from(( - base_addr.segment_index, - new_offset - .to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail)?, - ))) + if instruction.off0 < 0 { + Ok((base_addr - abs(instruction.off0) as usize)?) + } else { + Ok((base_addr + (instruction.off0 as usize))?) + } } pub fn compute_op0_addr( @@ -51,13 +49,11 @@ impl RunContext { Register::AP => self.get_ap(), Register::FP => self.get_fp(), }; - let new_offset = instruction.off1 + base_addr.offset as isize; - Ok(Relocatable::from(( - base_addr.segment_index, - new_offset - .to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail)?, - ))) + if instruction.off1 < 0 { + Ok((base_addr - abs(instruction.off1) as usize)?) + } else { + Ok((base_addr + (instruction.off1 as usize))?) + } } pub fn compute_op1_addr( @@ -78,13 +74,11 @@ impl RunContext { None => return Err(VirtualMachineError::UnknownOp0), }, }; - let new_offset = instruction.off2 + base_addr.offset as isize; - Ok(Relocatable::from(( - base_addr.segment_index, - new_offset - .to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail)?, - ))) + if instruction.off2 < 0 { + Ok((base_addr - abs(instruction.off2) as usize)?) + } else { + Ok((base_addr + (instruction.off2 as usize))?) + } } #[doc(hidden)] diff --git a/src/vm/errors/hint_errors.rs b/src/vm/errors/hint_errors.rs index 160d2535ce..5479a8944f 100644 --- a/src/vm/errors/hint_errors.rs +++ b/src/vm/errors/hint_errors.rs @@ -2,7 +2,10 @@ use felt::Felt; use num_bigint::{BigInt, BigUint}; use thiserror::Error; -use crate::types::relocatable::{MaybeRelocatable, Relocatable}; +use crate::types::{ + errors::math_errors::MathError, + relocatable::{MaybeRelocatable, Relocatable}, +}; use super::{ exec_scope_errors::ExecScopeError, memory_errors::MemoryError, vm_errors::VirtualMachineError, @@ -160,4 +163,6 @@ pub enum HintError { AddSignatureWrongEcdsaPtr(Relocatable), #[error("Signature hint must point to the public key cell, not {0}.")] AddSignatureNotAPublicKey(Relocatable), + #[error(transparent)] + Math(#[from] MathError), } diff --git a/src/vm/errors/memory_errors.rs b/src/vm/errors/memory_errors.rs index d1cb90f102..afe99d6867 100644 --- a/src/vm/errors/memory_errors.rs +++ b/src/vm/errors/memory_errors.rs @@ -1,9 +1,12 @@ use felt::Felt; use thiserror::Error; -use crate::types::relocatable::{MaybeRelocatable, Relocatable}; +use crate::types::{ + errors::math_errors::MathError, + relocatable::{MaybeRelocatable, Relocatable}, +}; -#[derive(Debug, PartialEq, Eq, Error)] +#[derive(Debug, PartialEq, Error)] pub enum MemoryError { #[error("Can't insert into segment #{0}; memory only has {1} segment")] UnallocatedSegment(usize, usize), @@ -73,6 +76,8 @@ pub enum MemoryError { SegmentHasMoreAccessedAddressesThanSize(usize, usize, usize), #[error("gen_arg: found argument of invalid type.")] GenArgInvalidType, + #[error(transparent)] + Math(#[from] MathError), // Memory.get() errors #[error("Expected integer at address {0}")] ExpectedInteger(Relocatable), diff --git a/src/vm/errors/runner_errors.rs b/src/vm/errors/runner_errors.rs index 730e30f464..2fe4bc0558 100644 --- a/src/vm/errors/runner_errors.rs +++ b/src/vm/errors/runner_errors.rs @@ -1,11 +1,14 @@ use std::collections::HashSet; use super::memory_errors::MemoryError; -use crate::types::relocatable::{MaybeRelocatable, Relocatable}; +use crate::types::{ + errors::math_errors::MathError, + relocatable::{MaybeRelocatable, Relocatable}, +}; use felt::Felt; use thiserror::Error; -#[derive(Debug, PartialEq, Eq, Error)] +#[derive(Debug, PartialEq, Error)] pub enum RunnerError { #[error("Initialization failure: No execution base")] NoExecBase, @@ -73,10 +76,10 @@ pub enum RunnerError { MaybeRelocVecToU64ArrayError, #[error("Expected Integer value, got Relocatable instead")] FoundNonInt, - #[error("{0} is not divisible by {1}")] - SafeDivFailUsize(usize, usize), #[error(transparent)] Memory(#[from] MemoryError), + #[error(transparent)] + Math(#[from] MathError), #[error("keccak_builtin: Failed to get first input address")] KeccakNoFirstInput, #[error("keccak_builtin: Failed to convert input cells to u64 values")] diff --git a/src/vm/errors/trace_errors.rs b/src/vm/errors/trace_errors.rs index 95ae2252b2..2f63144512 100644 --- a/src/vm/errors/trace_errors.rs +++ b/src/vm/errors/trace_errors.rs @@ -1,7 +1,7 @@ use crate::vm::errors::memory_errors::MemoryError; use thiserror::Error; -#[derive(Debug, PartialEq, Eq, Error)] +#[derive(Debug, PartialEq, Error)] pub enum TraceError { #[error("Trace is not enabled for this run")] TraceNotEnabled, diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs index 1a4d10addd..1f099b4f93 100644 --- a/src/vm/errors/vm_errors.rs +++ b/src/vm/errors/vm_errors.rs @@ -1,12 +1,14 @@ use crate::{ - types::relocatable::{MaybeRelocatable, Relocatable}, + types::{ + errors::math_errors::MathError, + relocatable::{MaybeRelocatable, Relocatable}, + }, vm::errors::{ exec_scope_errors::ExecScopeError, hint_errors::HintError, memory_errors::MemoryError, runner_errors::RunnerError, trace_errors::TraceError, }, }; use felt::Felt; -use num_bigint::{BigInt, BigUint}; use std::error::Error; use thiserror::Error; @@ -52,14 +54,8 @@ pub enum VirtualMachineError { InvalidRes(i64), #[error("Invalid opcode value: {0}")] InvalidOpcode(i64), - #[error("Cannot add two relocatable values")] - RelocatableAdd, - #[error("Offset {0} exceeds maximum offset value")] - OffsetExceeded(Felt), #[error("This is not implemented")] NotImplemented, - #[error("Can only subtract two relocatable values of the same segment")] - DiffIndexSub, #[error("Inconsistent auto-deduction for builtin {0}, expected {1}, got {2:?}")] InconsistentAutoDeduction(&'static str, MaybeRelocatable, Option), #[error(transparent)] @@ -72,40 +68,14 @@ pub enum VirtualMachineError { NoRangeCheckBuiltin, #[error("Expected ecdsa builtin to be present")] NoSignatureBuiltin, - #[error("Value: {0} should be positive")] - ValueNotPositive(Felt), #[error("Div out of range: 0 < {0} <= {1}")] OutOfValidRange(Felt, Felt), #[error("Failed to compare {0} and {1}, cant compare a relocatable to an integer value")] DiffTypeComparison(MaybeRelocatable, MaybeRelocatable), #[error("Failed to compare {0} and {1}, cant compare two relocatable values of different segment indexes")] DiffIndexComp(Relocatable, Relocatable), - #[error("Couldn't convert BigInt to usize")] - BigintToUsizeFail, - #[error("Couldn't convert BigInt to u64")] - BigintToU64Fail, - #[error("Couldn't convert BigInt to u32")] - BigintToU32Fail, #[error("Couldn't convert usize to u32")] NoneInMemoryRange, - #[error("Couldn't convert usize to u32")] - UsizeToU32Fail, - #[error("Can't calculate the square root of negative number: {0})")] - SqrtNegative(Felt), - #[error("{0} is not divisible by {1}")] - SafeDivFail(Felt, Felt), - #[error("{0} is not divisible by {1}")] - SafeDivFailBigInt(BigInt, BigInt), - #[error("{0} is not divisible by {1}")] - SafeDivFailBigUint(BigUint, BigUint), - #[error("{0} is not divisible by {1}")] - SafeDivFailU32(u32, u32), - #[error("Attempted to divide by zero")] - SafeDivFailUsize(usize, usize), - #[error("Attempted to divide by zero")] - DividedByZero, - #[error("Failed to calculate the square root of: {0})")] - FailedToGetSqrt(BigUint), #[error("Expected integer, found: {0:?}")] ExpectedIntAtRange(Option), #[error("Could not convert slice to array")] @@ -114,8 +84,6 @@ pub enum VirtualMachineError { CompileHintFail(String), #[error("op1_addr is Op1Addr.IMM, but no immediate was given")] NoImm, - #[error("Cant substract {0} from offset {1}, offsets cant be negative")] - CantSubOffset(usize, usize), #[error("Execution reached the end of the program. Requested remaining steps: {0}.")] EndOfProgram(usize), #[error(transparent)] @@ -141,5 +109,7 @@ pub enum VirtualMachineError { #[error("accessed_addresses is None.")] MissingAccessedAddresses, #[error(transparent)] + Math(#[from] MathError), + #[error(transparent)] Other(Box), } diff --git a/src/vm/runners/builtin_runner/bitwise.rs b/src/vm/runners/builtin_runner/bitwise.rs index 310d3babec..d4f5bc33ff 100644 --- a/src/vm/runners/builtin_runner/bitwise.rs +++ b/src/vm/runners/builtin_runner/bitwise.rs @@ -77,7 +77,7 @@ impl BitwiseBuiltinRunner { return Ok(None); } let x_addr = Relocatable::from((address.segment_index, address.offset - index)); - let y_addr = x_addr + 1_usize; + let y_addr = (x_addr + 1_usize)?; let num_x = memory.get(&x_addr); let num_y = memory.get(&y_addr); @@ -183,9 +183,8 @@ impl BitwiseBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(BITWISE_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(BITWISE_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/builtin_runner/ec_op.rs b/src/vm/runners/builtin_runner/ec_op.rs index 72b3ee303b..152df6dd8c 100644 --- a/src/vm/runners/builtin_runner/ec_op.rs +++ b/src/vm/runners/builtin_runner/ec_op.rs @@ -158,7 +158,7 @@ impl EcOpBuiltinRunner { //If an input cell is not filled, return None let mut input_cells = Vec::<&Felt>::with_capacity(self.n_input_cells as usize); for i in 0..self.n_input_cells as usize { - match memory.get(&(instance + i)) { + match memory.get(&(instance + i)?) { None => return Ok(None), Some(addr) => { input_cells.push(match addr { @@ -166,7 +166,7 @@ impl EcOpBuiltinRunner { Cow::Borrowed(MaybeRelocatable::Int(ref num)) => num, _ => { return Err(RunnerError::Memory(MemoryError::ExpectedInteger( - instance + i, + (instance + i)?, ))) } }); @@ -275,9 +275,8 @@ impl EcOpBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(EC_OP_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(EC_OP_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/builtin_runner/hash.rs b/src/vm/runners/builtin_runner/hash.rs index a30d8eca88..599f21363c 100644 --- a/src/vm/runners/builtin_runner/hash.rs +++ b/src/vm/runners/builtin_runner/hash.rs @@ -177,9 +177,8 @@ impl HashBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(EC_OP_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(EC_OP_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/builtin_runner/keccak.rs b/src/vm/runners/builtin_runner/keccak.rs index 6fb83920fc..39ee2ee19a 100644 --- a/src/vm/runners/builtin_runner/keccak.rs +++ b/src/vm/runners/builtin_runner/keccak.rs @@ -75,10 +75,7 @@ impl KeccakBuiltinRunner { return Ok(None); } - let first_input_addr = address - .sub_usize(index) - .map_err(|_| RunnerError::KeccakNoFirstInput)?; - + let first_input_addr = (address - index).map_err(|_| RunnerError::KeccakNoFirstInput)?; if self.verified_addresses.contains(&first_input_addr) { return Ok(None); } @@ -86,7 +83,7 @@ impl KeccakBuiltinRunner { let mut input_felts_u64 = vec![]; for i in 0..self.n_input_cells { - let val = match memory.get(&(first_input_addr + i as usize)) { + let val = match memory.get(&(first_input_addr + i as usize)?) { Some(val) => val .as_ref() .get_int_ref() @@ -99,10 +96,10 @@ impl KeccakBuiltinRunner { } if let Some((i, bits)) = self.state_rep.iter().enumerate().next() { - let val = memory.get_integer(first_input_addr + i)?; + let val = memory.get_integer((first_input_addr + i)?)?; if val.as_ref() >= &(Felt::one() << *bits) { return Err(RunnerError::IntegerBiggerThanPowerOfTwo( - (first_input_addr + i).into(), + (first_input_addr + i)?.into(), *bits, val.into_owned(), )); @@ -185,9 +182,8 @@ impl KeccakBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(KECCAK_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(KECCAK_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/builtin_runner/output.rs b/src/vm/runners/builtin_runner/output.rs index 820942c4d5..618faa0f1a 100644 --- a/src/vm/runners/builtin_runner/output.rs +++ b/src/vm/runners/builtin_runner/output.rs @@ -84,9 +84,8 @@ impl OutputBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(OUTPUT_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(OUTPUT_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/builtin_runner/range_check.rs b/src/vm/runners/builtin_runner/range_check.rs index cfa1f7b6fa..edb5e34f46 100644 --- a/src/vm/runners/builtin_runner/range_check.rs +++ b/src/vm/runners/builtin_runner/range_check.rs @@ -202,9 +202,8 @@ impl RangeCheckBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(RANGE_CHECK_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(RANGE_CHECK_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/builtin_runner/signature.rs b/src/vm/runners/builtin_runner/signature.rs index 8019ef63a4..1095de4a93 100644 --- a/src/vm/runners/builtin_runner/signature.rs +++ b/src/vm/runners/builtin_runner/signature.rs @@ -103,8 +103,8 @@ impl SignatureBuiltinRunner { let cell_index = addr.offset % cells_per_instance as usize; let (pubkey_addr, message_addr) = match cell_index { - 0 => (addr, addr + 1), - 1 => match addr.sub_usize(1) { + 0 => (addr, (addr + 1)?), + 1 => match addr - 1 { Ok(prev_addr) => (prev_addr, addr), Err(_) => return Ok(vec![]), }, @@ -222,9 +222,8 @@ impl SignatureBuiltinRunner { pointer: Relocatable, ) -> Result { if self.included { - let stop_pointer_addr = pointer - .sub_usize(1) - .map_err(|_| RunnerError::NoStopPointer(SIGNATURE_BUILTIN_NAME))?; + let stop_pointer_addr = + (pointer - 1).map_err(|_| RunnerError::NoStopPointer(SIGNATURE_BUILTIN_NAME))?; let stop_pointer = segments .memory .get_relocatable(stop_pointer_addr) diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 5c3f25dca4..8f210c33dd 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -3,7 +3,7 @@ use crate::{ math_utils::safe_div_usize, serde::deserialize_program::OffsetValue, types::{ - errors::program_errors::ProgramError, + errors::{math_errors::MathError, program_errors::ProgramError}, exec_scope::ExecutionScopes, instance_definitions::{ bitwise_instance_def::BitwiseInstanceDef, ec_op_instance_def::EcOpInstanceDef, @@ -335,7 +335,7 @@ impl CairoRunner { .program_base .unwrap_or_else(|| Relocatable::from((0, 0))); for i in 0..self.program.data.len() { - vm.segments.memory.mark_as_accessed(base + i); + vm.segments.memory.mark_as_accessed((base + i)?); } } if let Some(exec_base) = self.execution_base { @@ -997,10 +997,11 @@ impl CairoRunner { let (public_memory_units, rem) = div_rem(total_memory_units, instance._public_memory_fraction); if rem != 0 { - return Err(VirtualMachineError::SafeDivFailU32( + return Err(MathError::SafeDivFailU32( total_memory_units, instance._public_memory_fraction, - )); + ) + .into()); } let instruction_memory_units = 4 * vm_current_step_u32; @@ -4547,7 +4548,7 @@ mod tests { .unwrap(); vm.segments.compute_effective_sizes(); let initial_pointer = vm.get_ap(); - let expected_pointer = vm.get_ap().sub_usize(1).unwrap(); + let expected_pointer = (vm.get_ap() - 1).unwrap(); assert_eq!( runner.get_builtins_final_stack(&mut vm, initial_pointer), Ok(expected_pointer) @@ -4566,7 +4567,7 @@ mod tests { .unwrap(); vm.segments.compute_effective_sizes(); let initial_pointer = vm.get_ap(); - let expected_pointer = vm.get_ap().sub_usize(4).unwrap(); + let expected_pointer = (vm.get_ap() - 4).unwrap(); assert_eq!( runner.get_builtins_final_stack(&mut vm, initial_pointer), Ok(expected_pointer) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index c60a834faf..f580a70ae6 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -2,6 +2,7 @@ use crate::{ hint_processor::hint_processor_definition::HintProcessor, serde::deserialize_program::ApTracking, types::{ + errors::math_errors::MathError, exec_scope::ExecutionScopes, instruction::{ is_call_instruction, ApUpdate, FpUpdate, Instruction, Opcode, PcUpdate, Res, @@ -139,7 +140,7 @@ impl VirtualMachine { _ => return Err(VirtualMachineError::InvalidInstructionEncoding), }; - let imm_addr = &self.run_context.pc + 1_i32; + let imm_addr = (self.run_context.pc + 1_i32)?; Ok((encoding_ref, self.segments.memory.get(&imm_addr))) } @@ -154,7 +155,7 @@ impl VirtualMachine { MaybeRelocatable::RelocatableValue(ref rel) => rel.offset, MaybeRelocatable::Int(ref num) => num .to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail)?, + .ok_or_else(|| MathError::FeltToUsizeConversion(num.clone()))?, }, FpUpdate::Regular => return Ok(()), }; @@ -169,7 +170,7 @@ impl VirtualMachine { ) -> Result<(), VirtualMachineError> { let new_ap_offset: usize = match instruction.ap_update { ApUpdate::Add => match &operands.res { - Some(res) => self.run_context.get_ap().add_maybe(res)?.offset, + Some(res) => (self.run_context.get_ap() + res)?.offset, None => return Err(VirtualMachineError::UnconstrainedResAdd), }, ApUpdate::Add1 => self.run_context.ap + 1, @@ -186,21 +187,21 @@ impl VirtualMachine { operands: &Operands, ) -> Result<(), VirtualMachineError> { let new_pc: Relocatable = match instruction.pc_update { - PcUpdate::Regular => self.run_context.pc + instruction.size(), + PcUpdate::Regular => (self.run_context.pc + instruction.size())?, PcUpdate::Jump => match operands.res.as_ref().and_then(|x| x.get_relocatable()) { Some(ref res) => *res, None => return Err(VirtualMachineError::UnconstrainedResJump), }, PcUpdate::JumpRel => match operands.res.clone() { Some(res) => match res { - MaybeRelocatable::Int(num_res) => self.run_context.pc.add_int(&num_res)?, + MaybeRelocatable::Int(num_res) => (self.run_context.pc + &num_res)?, _ => return Err(VirtualMachineError::JumpRelNotInt), }, None => return Err(VirtualMachineError::UnconstrainedResJumpRel), }, PcUpdate::Jnz => match VirtualMachine::is_zero(&operands.dst) { - true => self.run_context.pc + instruction.size(), - false => (self.run_context.pc.add_maybe(&operands.op1))?, + true => (self.run_context.pc + instruction.size())?, + false => (self.run_context.pc + &operands.op1)?, }, }; self.run_context.pc = new_pc; @@ -239,7 +240,7 @@ impl VirtualMachine { match instruction.opcode { Opcode::Call => Ok(( Some(MaybeRelocatable::from( - self.run_context.pc + instruction.size(), + (self.run_context.pc + instruction.size())?, )), None, )), @@ -359,7 +360,7 @@ impl VirtualMachine { _ => Ok(()), }, Opcode::Call => { - let return_pc = MaybeRelocatable::from(self.run_context.pc + instruction.size()); + let return_pc = MaybeRelocatable::from((self.run_context.pc + instruction.size())?); if operands.op0 != return_pc { return Err(VirtualMachineError::CantWriteReturnPc( operands.op0.clone(), @@ -701,7 +702,7 @@ impl VirtualMachine { return Err(VirtualMachineError::RunNotFinished); } for i in 0..len { - self.segments.memory.mark_as_accessed(base + i); + self.segments.memory.mark_as_accessed((base + i)?); } Ok(()) } @@ -714,8 +715,7 @@ impl VirtualMachine { // Fetch the fp and pc traceback entries for _ in 0..MAX_TRACEBACK_ENTRIES { // Get return pc - let ret_pc = match fp - .sub_usize(1) + let ret_pc = match (fp - 1) .ok() .map(|r| self.segments.memory.get_relocatable(r)) { @@ -723,8 +723,7 @@ impl VirtualMachine { _ => break, }; // Get fp traceback - match fp - .sub_usize(2) + match (fp - 2) .ok() .map(|r| self.segments.memory.get_relocatable(r)) { @@ -733,23 +732,21 @@ impl VirtualMachine { } // Try to check if the call instruction is (instruction0, instruction1) or just // instruction1 (with no immediate). - let call_pc = match ret_pc - .sub_usize(1) + let call_pc = match (ret_pc - 1) .ok() .map(|r| self.segments.memory.get_integer(r)) { Some(Ok(instruction1)) => { match is_call_instruction(&instruction1, None) { - true => ret_pc.sub_usize(1).unwrap(), // This unwrap wont fail as it is checked before + true => (ret_pc - 1).unwrap(), // This unwrap wont fail as it is checked before false => { - match ret_pc - .sub_usize(2) + match (ret_pc - 2) .ok() .map(|r| self.segments.memory.get_integer(r)) { Some(Ok(instruction0)) => { match is_call_instruction(&instruction0, Some(&instruction1)) { - true => ret_pc.sub_usize(2).unwrap(), // This unwrap wont fail as it is checked before + true => (ret_pc - 2).unwrap(), // This unwrap wont fail as it is checked before false => break, } } @@ -841,10 +838,7 @@ impl VirtualMachine { ///Gets `n_ret` return values from memory pub fn get_return_values(&self, n_ret: usize) -> Result, MemoryError> { - let addr = self - .run_context - .get_ap() - .sub_usize(n_ret) + let addr = (self.run_context.get_ap() - n_ret) .map_err(|_| MemoryError::FailedToGetReturnValues(n_ret, self.get_ap()))?; self.segments.memory.get_continuous_range(addr, n_ret) } diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index cf44667303..6f6afe4892 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -173,7 +173,7 @@ impl Memory { // Rely on Memory::insert to catch memory inconsistencies self.insert(&addr, cell.get_value())?; } - addr = addr + 1; + addr = (addr + 1)?; } } } @@ -280,7 +280,7 @@ impl Memory { let mut values = Vec::new(); for i in 0..size { - values.push(self.get(&(addr + i))); + values.push((addr + i).ok().and_then(|x| self.get(&x))); } values @@ -296,7 +296,7 @@ impl Memory { let mut values = Vec::with_capacity(size); for i in 0..size { - values.push(match self.get(&(addr + i)) { + values.push(match self.get(&(addr + i)?) { Some(elem) => elem.into_owned(), None => return Err(MemoryError::GetRangeMemoryGap(addr, size)), }); @@ -316,7 +316,7 @@ impl Memory { let mut values = Vec::new(); for i in 0..size { - values.push(self.get_integer(addr + i)?); + values.push(self.get_integer((addr + i)?)?); } Ok(values) diff --git a/src/vm/vm_memory/memory_segments.rs b/src/vm/vm_memory/memory_segments.rs index b12434abb7..1e59bbfcd6 100644 --- a/src/vm/vm_memory/memory_segments.rs +++ b/src/vm/vm_memory/memory_segments.rs @@ -54,9 +54,9 @@ impl MemorySegmentManager { data: &Vec, ) -> Result { for (num, value) in data.iter().enumerate() { - self.memory.insert(&(ptr + num), value)?; + self.memory.insert(&(ptr + num)?, value)?; } - Ok(ptr + data.len()) + (ptr + data.len()).map_err(MemoryError::Math) } pub fn new() -> MemorySegmentManager {