Skip to content

Commit

Permalink
Add MathError for math operations (#855)
Browse files Browse the repository at this point in the history
* 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<i32>

* Fix should_panic test

* remove referenced add

* Replace Relocatable methods for trait implementations

* Catch overflows in mayberelocatable operations

* Fix keccak

* Clippy
fmoletta authored Mar 6, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent d5a7bea commit 1de3e2b
Showing 37 changed files with 446 additions and 466 deletions.
22 changes: 11 additions & 11 deletions src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
@@ -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,15 +197,15 @@ 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(())
}

#[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
);
}

Original file line number Diff line number Diff line change
@@ -6,15 +6,15 @@ 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,
},
};
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()),
)),
Original file line number Diff line number Diff line change
@@ -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
13 changes: 5 additions & 8 deletions src/hint_processor/builtin_hint_processor/find_element_hint.rs
Original file line number Diff line number Diff line change
@@ -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() {
2 changes: 1 addition & 1 deletion src/hint_processor/builtin_hint_processor/keccak_utils.rs
Original file line number Diff line number Diff line change
@@ -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)?;
6 changes: 3 additions & 3 deletions src/hint_processor/builtin_hint_processor/math_utils.rs
Original file line number Diff line number Diff line change
@@ -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(())
}

4 changes: 3 additions & 1 deletion src/hint_processor/builtin_hint_processor/pow_utils.rs
Original file line number Diff line number Diff line change
@@ -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())
})?;
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions src/hint_processor/builtin_hint_processor/secp/ec_utils.rs
Original file line number Diff line number Diff line change
@@ -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);
8 changes: 3 additions & 5 deletions src/hint_processor/builtin_hint_processor/secp/signature.rs
Original file line number Diff line number Diff line change
@@ -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,
)
31 changes: 16 additions & 15 deletions src/hint_processor/builtin_hint_processor/set.rs
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 1 addition & 1 deletion src/hint_processor/builtin_hint_processor/sha256_utils.rs
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ pub fn sha256_main(
let mut message: Vec<u8> = 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);
}
Original file line number Diff line number Diff line change
@@ -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::<Felt, Vec<Felt>>::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))?;
18 changes: 9 additions & 9 deletions src/hint_processor/builtin_hint_processor/uint256_utils.rs
Original file line number Diff line number Diff line change
@@ -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(())
}

6 changes: 3 additions & 3 deletions src/hint_processor/builtin_hint_processor/usort.rs
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ pub fn usort_body(
let mut positions_dict: HashMap<Felt, Vec<u64>> = HashMap::new();
let mut output: Vec<Felt> = 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(
25 changes: 12 additions & 13 deletions src/hint_processor/hint_processor_utils.rs
Original file line number Diff line number Diff line change
@@ -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<usize, VirtualMachineError> {
pub fn felt_to_usize(felt: &Felt) -> Result<usize, MathError> {
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<u32, VirtualMachineError> {
felt.to_u32().ok_or(VirtualMachineError::BigintToU32Fail)
pub fn felt_to_u32(felt: &Felt) -> Result<u32, MathError> {
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())
}
}

40 changes: 17 additions & 23 deletions src/math_utils.rs
Original file line number Diff line number Diff line change
@@ -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<BigUint, VirtualMachineError> {
pub fn isqrt(n: &BigUint) -> Result<BigUint, MathError> {
/* # 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<BigUint, VirtualMachineError> {
}

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<Felt, VirtualMachineError> {
pub fn safe_div(x: &Felt, y: &Felt) -> Result<Felt, MathError> {
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<BigInt, VirtualMachineError> {
pub fn safe_div_bigint(x: &BigInt, y: &BigInt) -> Result<BigInt, MathError> {
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<usize, VirtualMachineError> {
pub fn safe_div_usize(x: usize, y: usize) -> Result<usize, MathError> {
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))
}
}
54 changes: 54 additions & 0 deletions src/types/errors/math_errors.rs
Original file line number Diff line number Diff line change
@@ -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),
}
1 change: 1 addition & 0 deletions src/types/errors/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
pub mod math_errors;
pub mod program_errors;
418 changes: 195 additions & 223 deletions src/types/relocatable.rs

Large diffs are not rendered by default.

38 changes: 16 additions & 22 deletions src/vm/context/run_context.rs
Original file line number Diff line number Diff line change
@@ -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)]
7 changes: 6 additions & 1 deletion src/vm/errors/hint_errors.rs
Original file line number Diff line number Diff line change
@@ -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),
}
9 changes: 7 additions & 2 deletions src/vm/errors/memory_errors.rs
Original file line number Diff line number Diff line change
@@ -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),
11 changes: 7 additions & 4 deletions src/vm/errors/runner_errors.rs
Original file line number Diff line number Diff line change
@@ -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")]
2 changes: 1 addition & 1 deletion src/vm/errors/trace_errors.rs
Original file line number Diff line number Diff line change
@@ -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,
42 changes: 6 additions & 36 deletions src/vm/errors/vm_errors.rs
Original file line number Diff line number Diff line change
@@ -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<MaybeRelocatable>),
#[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<MaybeRelocatable>),
#[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<dyn Error>),
}
7 changes: 3 additions & 4 deletions src/vm/runners/builtin_runner/bitwise.rs
Original file line number Diff line number Diff line change
@@ -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<Relocatable, RunnerError> {
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)
9 changes: 4 additions & 5 deletions src/vm/runners/builtin_runner/ec_op.rs
Original file line number Diff line number Diff line change
@@ -158,15 +158,15 @@ 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 {
// Only relocatable values can be owned
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<Relocatable, RunnerError> {
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)
5 changes: 2 additions & 3 deletions src/vm/runners/builtin_runner/hash.rs
Original file line number Diff line number Diff line change
@@ -177,9 +177,8 @@ impl HashBuiltinRunner {
pointer: Relocatable,
) -> Result<Relocatable, RunnerError> {
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)
16 changes: 6 additions & 10 deletions src/vm/runners/builtin_runner/keccak.rs
Original file line number Diff line number Diff line change
@@ -75,18 +75,15 @@ 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);
}

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<Relocatable, RunnerError> {
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)
5 changes: 2 additions & 3 deletions src/vm/runners/builtin_runner/output.rs
Original file line number Diff line number Diff line change
@@ -84,9 +84,8 @@ impl OutputBuiltinRunner {
pointer: Relocatable,
) -> Result<Relocatable, RunnerError> {
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)
5 changes: 2 additions & 3 deletions src/vm/runners/builtin_runner/range_check.rs
Original file line number Diff line number Diff line change
@@ -202,9 +202,8 @@ impl RangeCheckBuiltinRunner {
pointer: Relocatable,
) -> Result<Relocatable, RunnerError> {
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)
9 changes: 4 additions & 5 deletions src/vm/runners/builtin_runner/signature.rs
Original file line number Diff line number Diff line change
@@ -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<Relocatable, RunnerError> {
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)
13 changes: 7 additions & 6 deletions src/vm/runners/cairo_runner.rs
Original file line number Diff line number Diff line change
@@ -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)
42 changes: 18 additions & 24 deletions src/vm/vm_core.rs
Original file line number Diff line number Diff line change
@@ -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,17 +715,15 @@ 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))
{
Some(Ok(opt_pc)) => opt_pc,
_ => 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<Vec<MaybeRelocatable>, 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)
}
8 changes: 4 additions & 4 deletions src/vm/vm_memory/memory.rs
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 2 additions & 2 deletions src/vm/vm_memory/memory_segments.rs
Original file line number Diff line number Diff line change
@@ -54,9 +54,9 @@ impl MemorySegmentManager {
data: &Vec<MaybeRelocatable>,
) -> Result<Relocatable, MemoryError> {
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 {

1 comment on commit 1de3e2b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.

Benchmark suite Current: 1de3e2b Previous: d5a7bea Ratio
cairo_run(cairo_programs/benchmarks/compare_arrays_200000.json) 884503786 ns/iter (± 28972997) 637231489 ns/iter (± 1969874) 1.39
cairo_run(cairo_programs/benchmarks/factorial_multirun.json) 329271401 ns/iter (± 12461198) 240630877 ns/iter (± 664836) 1.37
cairo_run(cairo_programs/benchmarks/fibonacci_1000_multirun.json) 139728837 ns/iter (± 7714931) 101045574 ns/iter (± 351582) 1.38
cairo_run(cairo_programs/benchmarks/integration_builtins.json) 537004106 ns/iter (± 23605098) 386937505 ns/iter (± 1718909) 1.39
cairo_run(cairo_programs/benchmarks/linear_search.json) 103559057 ns/iter (± 6181470) 79573059 ns/iter (± 5279566) 1.30
cairo_run(cairo_programs/benchmarks/keccak_integration_benchmark.json) 1682060789 ns/iter (± 48924206) 1240800087 ns/iter (± 8607617) 1.36
cairo_run(cairo_programs/benchmarks/secp_integration_benchmark.json) 1869022503 ns/iter (± 80441664) 1348564409 ns/iter (± 2660517) 1.39
cairo_run(cairo_programs/benchmarks/blake2s_integration_benchmark.json) 1534652558 ns/iter (± 71555462) 1082080349 ns/iter (± 8505744) 1.42
cairo_run(cairo_programs/benchmarks/dict_integration_benchmark.json) 1000563302 ns/iter (± 32632253) 757723182 ns/iter (± 8971950) 1.32
cairo_run(cairo_programs/benchmarks/math_integration_benchmark.json) 493033960 ns/iter (± 24944368) 377129902 ns/iter (± 871064) 1.31
cairo_run(cairo_programs/benchmarks/math_cmp_and_pow_integration_benchmark.json) 23903260 ns/iter (± 1455740) 18089601 ns/iter (± 107357) 1.32
cairo_run(cairo_programs/benchmarks/operations_with_data_structures_benchmarks.json) 2213835247 ns/iter (± 67634907) 1610375764 ns/iter (± 30312668) 1.37
cairo_run(cairo_programs/benchmarks/uint256_integration_benchmark.json) 1502525606 ns/iter (± 61358205) 1081656715 ns/iter (± 3755716) 1.39
cairo_run(cairo_programs/benchmarks/set_integration_benchmark.json) 169453151 ns/iter (± 10920855) 121522873 ns/iter (± 418889) 1.39

This comment was automatically generated by workflow using github-action-benchmark.

CC: @unbalancedparentheses

Please sign in to comment.