Skip to content

Commit

Permalink
Move memory-related errors to MemoryError (#854)
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

* Print relocatables & missing members in write_output

* Add test

* Remove unused memory errors from RunnerError

* Remove NegativeBuiltinBase error

* Move memory specific errors to MemoryError

* Remove errors

* Change error mapping

* Remove ExpectedInteger from RunnerError

* Use MemoryError for memory-related errors

* Fix test

* Use memory errors in memory segments methods

* Clippy
  • Loading branch information
fmoletta committed Feb 24, 2023
1 parent 71e7140 commit e0da0be
Show file tree
Hide file tree
Showing 33 changed files with 178 additions and 233 deletions.
28 changes: 14 additions & 14 deletions src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ use crate::{
},
serde::deserialize_program::ApTracking,
types::relocatable::{MaybeRelocatable, Relocatable},
vm::{
errors::{hint_errors::HintError, vm_errors::VirtualMachineError},
vm_core::VirtualMachine,
},
vm::{errors::hint_errors::HintError, vm_core::VirtualMachine},
};
use felt::Felt;
use num_traits::ToPrimitive;
Expand Down Expand Up @@ -56,7 +53,7 @@ fn compute_blake2s_func(vm: &mut VirtualMachine, output_rel: Relocatable) -> Res
get_maybe_relocatable_array_from_u32(&blake2s_compress(&h, &message, t, 0, f, 0));
let output_ptr = MaybeRelocatable::RelocatableValue(output_rel);
vm.load_data(&output_ptr, &new_state)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;
Ok(())
}

Expand Down Expand Up @@ -117,7 +114,7 @@ pub fn finalize_blake2s(
}
let data = get_maybe_relocatable_array_from_u32(&full_padding);
vm.load_data(&MaybeRelocatable::RelocatableValue(blake2s_ptr_end), &data)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;
Ok(())
}

Expand Down Expand Up @@ -152,7 +149,7 @@ pub fn blake2s_add_uint256(
//Insert first batch of data
let data = get_maybe_relocatable_array_from_felt(&inner_data);
vm.load_data(&MaybeRelocatable::RelocatableValue(data_ptr), &data)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;
//Build second batch of data
let mut inner_data = Vec::<Felt>::new();
for i in 0..4 {
Expand All @@ -164,7 +161,7 @@ pub fn blake2s_add_uint256(
&MaybeRelocatable::RelocatableValue(data_ptr).add_usize(4),
&data,
)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;
Ok(())
}

Expand Down Expand Up @@ -199,7 +196,7 @@ pub fn blake2s_add_uint256_bigend(
//Insert first batch of data
let data = get_maybe_relocatable_array_from_felt(&inner_data);
vm.load_data(&MaybeRelocatable::RelocatableValue(data_ptr), &data)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;
//Build second batch of data
let mut inner_data = Vec::<Felt>::new();
for i in 0..4 {
Expand All @@ -211,13 +208,14 @@ pub fn blake2s_add_uint256_bigend(
&MaybeRelocatable::RelocatableValue(data_ptr).add_usize(4),
&data,
)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
use crate::vm::errors::vm_errors::VirtualMachineError;
use crate::vm::vm_memory::memory_segments::MemorySegmentManager;
use crate::{
any_box,
Expand Down Expand Up @@ -270,7 +268,7 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
Err(HintError::Memory(MemoryError::UnknownMemoryCell(
x
))) if x == Relocatable::from((2, 0))
);
Expand Down Expand Up @@ -337,7 +335,9 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(x))) if x == Relocatable::from((2,0))
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((2, 0))
);
}

Expand Down Expand Up @@ -400,13 +400,13 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::MemoryError(
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
))) if x == MaybeRelocatable::from((2, 0)) &&
)) if x == MaybeRelocatable::from((2, 0)) &&
y == MaybeRelocatable::from((2, 0)) &&
z == MaybeRelocatable::from(Felt::new(1795745351))
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,7 @@ mod tests {
types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable},
utils::test_utils::*,
vm::{
errors::{
exec_scope_errors::ExecScopeError, memory_errors::MemoryError,
vm_errors::VirtualMachineError,
},
errors::{exec_scope_errors::ExecScopeError, memory_errors::MemoryError},
vm_core::VirtualMachine,
vm_memory::memory::Memory,
},
Expand Down Expand Up @@ -506,13 +503,13 @@ mod tests {
//ids and references are not needed for this test
assert_matches!(
run_hint!(vm, HashMap::new(), hint_code),
Err(HintError::Internal(VirtualMachineError::MemoryError(
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
))) if x == MaybeRelocatable::from((1, 6)) &&
)) if x == MaybeRelocatable::from((1, 6)) &&
y == MaybeRelocatable::from((1, 6)) &&
z == MaybeRelocatable::from((3, 0))
);
Expand Down Expand Up @@ -614,13 +611,13 @@ mod tests {
let ids_data = ids_data!["continue_copying"];
assert_matches!(
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
Err(HintError::Internal(VirtualMachineError::MemoryError(
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
))) if x == MaybeRelocatable::from((1, 1)) &&
)) if x == MaybeRelocatable::from((1, 1)) &&
y == MaybeRelocatable::from(Felt::new(5)) &&
z == MaybeRelocatable::from(Felt::zero())
);
Expand Down Expand Up @@ -801,9 +798,7 @@ mod tests {
let ids_data = non_continuous_ids_data![("keccak_state", -7), ("high", -3), ("low", -2)];
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
_
)))
Err(HintError::Memory(MemoryError::UnknownMemoryCell(_)))
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ pub fn keccak_write_args(

let low_args: Vec<_> = low_args.into_iter().map(MaybeRelocatable::from).collect();
vm.write_arg(inputs_ptr, &low_args)
.map_err(VirtualMachineError::MemoryError)?;
.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)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;

Ok(())
}
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn block_permutation(
&MaybeRelocatable::RelocatableValue(keccak_ptr.sub_usize(keccak_state_size_felts)?),
keccak_state_size_felts,
)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;

let mut u64_values = maybe_reloc_vec_to_u64_array(&values)?
.try_into()
Expand All @@ -161,7 +161,7 @@ pub fn block_permutation(
let bigint_values = u64_array_to_mayberelocatable_vec(&u64_values);

vm.write_arg(keccak_ptr, &bigint_values)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;

Ok(())
}
Expand Down Expand Up @@ -221,7 +221,7 @@ pub fn cairo_keccak_finalize(
let keccak_ptr_end = get_ptr_from_var_name("keccak_ptr_end", vm, ids_data, ap_tracking)?;

vm.write_arg(keccak_ptr_end, &padding)
.map_err(VirtualMachineError::MemoryError)?;
.map_err(HintError::Memory)?;

Ok(())
}
Expand Down Expand Up @@ -300,10 +300,7 @@ mod tests {
//Create ids
let ids_data = ids_data!["low", "high", "inputs"];
let error = run_hint!(vm, ids_data, hint_code);
assert_matches!(
error,
Err(HintError::Internal(VirtualMachineError::MemoryError(_)))
);
assert_matches!(error, Err(HintError::Memory(_)));
}

#[test]
Expand Down
5 changes: 2 additions & 3 deletions src/hint_processor/builtin_hint_processor/dict_hint_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@ mod tests {
use crate::hint_processor::builtin_hint_processor::hint_code;
use crate::hint_processor::hint_processor_definition::HintProcessor;
use crate::types::exec_scope::ExecutionScopes;
use crate::vm::errors::vm_errors::VirtualMachineError;
use crate::vm::vm_memory::memory::Memory;
use crate::vm::vm_memory::memory_segments::MemorySegmentManager;
use crate::{
Expand Down Expand Up @@ -325,13 +324,13 @@ mod tests {
//ids and references are not needed for this test
assert_matches!(
run_hint!(vm, HashMap::new(), hint_code, &mut exec_scopes),
Err(HintError::Internal(VirtualMachineError::MemoryError(
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
))) if x == MaybeRelocatable::from((1, 0)) &&
)) if x == MaybeRelocatable::from((1, 0)) &&
y == MaybeRelocatable::from(1) &&
z == MaybeRelocatable::from((2, 0))
);
Expand Down
13 changes: 1 addition & 12 deletions src/hint_processor/builtin_hint_processor/dict_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@ use std::collections::HashMap;

use crate::{
types::relocatable::{MaybeRelocatable, Relocatable},
vm::{
errors::{
hint_errors::HintError, memory_errors::MemoryError, vm_errors::VirtualMachineError,
},
vm_core::VirtualMachine,
},
vm::{errors::hint_errors::HintError, vm_core::VirtualMachine},
};

#[derive(PartialEq, Eq, Debug, Clone)]
Expand Down Expand Up @@ -80,12 +75,6 @@ impl DictManager {
return Err(HintError::CantCreateDictionaryOnTakenSegment(
base.segment_index,
));
}

if base.segment_index < 0 {
Err(VirtualMachineError::MemoryError(
MemoryError::AddressInTemporarySegment(base.segment_index),
))?;
};

self.trackers.insert(
Expand Down
4 changes: 2 additions & 2 deletions src/hint_processor/builtin_hint_processor/hint_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub fn insert_value_from_var_name(
) -> Result<(), HintError> {
let var_address = get_relocatable_from_var_name(var_name, vm, ids_data, ap_tracking)?;
vm.insert_value(var_address, value)
.map_err(HintError::Internal)
.map_err(HintError::Memory)
}

//Inserts value into ap
Expand All @@ -34,7 +34,7 @@ pub fn insert_value_into_ap(
value: impl Into<MaybeRelocatable>,
) -> Result<(), HintError> {
vm.insert_value(vm.get_ap(), value)
.map_err(HintError::Internal)
.map_err(HintError::Memory)
}

//Returns the Relocatable value stored in the given ids variable
Expand Down
Loading

0 comments on commit e0da0be

Please sign in to comment.