From 092af21e0ab4a1e4c6fa9b6c856c57abb3e0cf2d Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 10:42:55 -0300 Subject: [PATCH 01/18] Use only option for Memory.get --- src/vm/vm_memory/memory.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 6dd02c0423..c0033a2282 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -84,16 +84,11 @@ impl Memory { } /// Retrieve a value from memory (either normal or temporary) and apply relocation rules - pub(crate) fn get<'a, 'b: 'a, K: 'a>( - &'b self, - key: &'a K, - ) -> Result>, MemoryError> + pub(crate) fn get<'a, 'b: 'a, K: 'a>(&'b self, key: &'a K) -> Option> where Relocatable: TryFrom<&'a K>, { - let relocatable: Relocatable = key - .try_into() - .map_err(|_| MemoryError::AddressNotRelocatable)?; + let relocatable: Relocatable = key.try_into().ok()?; let data = if relocatable.segment_index.is_negative() { &self.temp_data @@ -101,13 +96,7 @@ impl Memory { &self.data }; let (i, j) = from_relocatable_to_indexes(relocatable); - if data.len() > i && data[i].len() > j { - if let Some(ref element) = data[i][j] { - return Ok(Some(self.relocate_value(element))); - } - } - - Ok(None) + Some(self.relocate_value(data.get(i)?.get(j)?.as_ref()?)) } // Version of Memory.relocate_value() that doesn't require a self reference From 66d4161b070cf21709d1f65dec9f359f3577036b Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 11:12:42 -0300 Subject: [PATCH 02/18] Fix some tests + refactor range_check validation --- src/utils.rs | 10 +--- src/vm/errors/memory_errors.rs | 8 +-- src/vm/errors/vm_errors.rs | 2 + src/vm/runners/builtin_runner/range_check.rs | 21 ++++--- src/vm/runners/cairo_runner.rs | 42 +++++++------- src/vm/vm_core.rs | 53 ++++-------------- src/vm/vm_memory/memory.rs | 59 ++++++++++---------- src/vm/vm_memory/memory_segments.rs | 6 +- 8 files changed, 79 insertions(+), 122 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 56370f1013..97737dce58 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -163,19 +163,13 @@ pub mod test_utils { macro_rules! check_memory_address { ($mem:expr, ($si:expr, $off:expr), ($sival:expr, $offval: expr)) => { assert_eq!( - $mem.get(&mayberelocatable!($si, $off)) - .unwrap() - .unwrap() - .as_ref(), + $mem.get(&mayberelocatable!($si, $off)).unwrap().as_ref(), &mayberelocatable!($sival, $offval) ) }; ($mem:expr, ($si:expr, $off:expr), $val:expr) => { assert_eq!( - $mem.get(&mayberelocatable!($si, $off)) - .unwrap() - .unwrap() - .as_ref(), + $mem.get(&mayberelocatable!($si, $off)).unwrap().as_ref(), &mayberelocatable!($val) ) }; diff --git a/src/vm/errors/memory_errors.rs b/src/vm/errors/memory_errors.rs index f8500409f5..3ee84eabd1 100644 --- a/src/vm/errors/memory_errors.rs +++ b/src/vm/errors/memory_errors.rs @@ -9,10 +9,10 @@ pub enum MemoryError { UnallocatedSegment(usize, usize), #[error("Memory addresses must be relocatable")] AddressNotRelocatable, - #[error("Range-check validation failed, number is out of valid range")] - NumOutOfBounds, - #[error("Range-check validation failed, encountered non-int value")] - FoundNonInt, + #[error("Range-check validation failed, number {0} is out of valid range [0, {1}]")] + RangeCheckNumOutOfBounds(Felt, Felt), + #[error("Range-check validation failed, encountered non-int value at address {0}")] + RangeCheckFoundNonInt(Relocatable), #[error("Inconsistent memory assignment at address {0:?}. {1:?} != {2:?}")] InconsistentMemory(MaybeRelocatable, MaybeRelocatable, MaybeRelocatable), #[error("compute_effective_sizes should be called before relocate_segments")] diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs index 9b4529765f..8304a4ba27 100644 --- a/src/vm/errors/vm_errors.rs +++ b/src/vm/errors/vm_errors.rs @@ -150,6 +150,8 @@ pub enum VirtualMachineError { InvalidMemoryValueTemporaryAddress(Relocatable), #[error("accessed_addresses is None.")] MissingAccessedAddresses, + #[error("Unknown memory cell at address {0}")] + UnknownMemoryCell(Relocatable), #[error(transparent)] Other(Box), } diff --git a/src/vm/runners/builtin_runner/range_check.rs b/src/vm/runners/builtin_runner/range_check.rs index 59b17f2701..3725a17bb6 100644 --- a/src/vm/runners/builtin_runner/range_check.rs +++ b/src/vm/runners/builtin_runner/range_check.rs @@ -88,18 +88,17 @@ impl RangeCheckBuiltinRunner { pub fn add_validation_rule(&self, memory: &mut Memory) { let rule: ValidationRule = ValidationRule(Box::new( |memory: &Memory, address: Relocatable| -> Result, MemoryError> { - if let MaybeRelocatable::Int(ref num) = memory - .get(&address)? - .ok_or(MemoryError::FoundNonInt)? - .into_owned() - { - if &Felt::zero() <= num && num < &Felt::one().shl(128_usize) { - Ok(vec![address.to_owned()]) - } else { - Err(MemoryError::NumOutOfBounds) - } + let num = memory + .get_integer(address) + .map_err(|_| MemoryError::RangeCheckFoundNonInt(address))? + .as_ref(); + if &Felt::zero() <= num && num < &Felt::one().shl(128_usize) { + Ok(vec![address.to_owned()]) } else { - Err(MemoryError::FoundNonInt) + Err(MemoryError::RangeCheckNumOutOfBounds( + num.clone(), + Felt::one().shl(128_usize), + )) } }, )); diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index a4deac800a..53b45d31a5 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -2041,10 +2041,11 @@ mod tests { ((2, 0), 7), ((2, 1), 18446744073709551608_i128) ); - assert_eq!( - vm.segments.memory.get(&MaybeRelocatable::from((2, 2))), - Ok(None) - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 2))) + .is_none()); } #[test] @@ -2152,10 +2153,11 @@ mod tests { assert_eq!(vm.builtin_runners[0].0, OUTPUT_BUILTIN_NAME); assert_eq!(vm.builtin_runners[0].1.base(), 2); check_memory!(vm.segments.memory, ((2, 0), 1), ((2, 1), 17)); - assert_eq!( - vm.segments.memory.get(&MaybeRelocatable::from((2, 2))), - Ok(None) - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 2))) + .is_none()); } #[test] @@ -2299,26 +2301,22 @@ mod tests { ((3, 0), 7), ((3, 1), 18446744073709551608_i128) ); - assert_eq!( - vm.segments - .memory - .get(&MaybeRelocatable::from((2, 2))) - .unwrap(), - None - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 2))) + .is_none()); //Check the output segment assert_eq!(vm.builtin_runners[0].0, OUTPUT_BUILTIN_NAME); assert_eq!(vm.builtin_runners[0].1.base(), 2); check_memory!(vm.segments.memory, ((2, 0), 7)); - assert_eq!( - vm.segments - .memory - .get(&(MaybeRelocatable::from((2, 1)))) - .unwrap(), - None - ); + assert!(vm + .segments + .memory + .get(&(MaybeRelocatable::from((2, 1)))) + .is_none()); } #[test] diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index a87d100808..c9ec32ad1e 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -140,16 +140,13 @@ impl VirtualMachine { &self, ) -> Result<(Cow, Option>), VirtualMachineError> { let encoding_ref = match self.segments.memory.get(&self.run_context.pc) { - Ok(Some(Cow::Owned(MaybeRelocatable::Int(encoding)))) => Cow::Owned(encoding), - Ok(Some(Cow::Borrowed(MaybeRelocatable::Int(encoding)))) => Cow::Borrowed(encoding), + Some(Cow::Owned(MaybeRelocatable::Int(encoding))) => Cow::Owned(encoding), + Some(Cow::Borrowed(MaybeRelocatable::Int(encoding))) => Cow::Borrowed(encoding), _ => return Err(VirtualMachineError::InvalidInstructionEncoding), }; let imm_addr = &self.run_context.pc + 1_i32; - Ok(( - encoding_ref, - self.segments.memory.get(&imm_addr).ok().flatten(), - )) + Ok((encoding_ref, self.segments.memory.get(&imm_addr))) } fn update_fp( @@ -581,30 +578,15 @@ impl VirtualMachine { ) -> Result<(Operands, OperandsAddresses, DeducedOperands), VirtualMachineError> { //Get operands from memory let dst_addr = self.run_context.compute_dst_addr(instruction)?; - let dst_op = self - .segments - .memory - .get(&dst_addr) - .map_err(VirtualMachineError::MemoryError)? - .map(Cow::into_owned); + let dst_op = self.segments.memory.get(&dst_addr).map(Cow::into_owned); let op0_addr = self.run_context.compute_op0_addr(instruction)?; - let op0_op = self - .segments - .memory - .get(&op0_addr) - .map_err(VirtualMachineError::MemoryError)? - .map(Cow::into_owned); + let op0_op = self.segments.memory.get(&op0_addr).map(Cow::into_owned); let op1_addr = self .run_context .compute_op1_addr(instruction, op0_op.as_ref())?; - let op1_op = self - .segments - .memory - .get(&op1_addr) - .map_err(VirtualMachineError::MemoryError)? - .map(Cow::into_owned); + let op1_op = self.segments.memory.get(&op1_addr).map(Cow::into_owned); let mut res: Option = None; @@ -688,7 +670,7 @@ impl VirtualMachine { Some(value) => value, None => return Ok(()), }; - let current_value = match self.segments.memory.get(&addr)? { + let current_value = match self.segments.memory.get(&addr) { Some(value) => value.into_owned(), None => return Ok(()), }; @@ -815,18 +797,11 @@ impl VirtualMachine { } ///Gets a MaybeRelocatable value from memory indicated by a generic address - pub fn get_maybe<'a, 'b: 'a, K: 'a>( - &'b self, - key: &'a K, - ) -> Result, MemoryError> + pub fn get_maybe<'a, 'b: 'a, K: 'a>(&'b self, key: &'a K) -> Option where Relocatable: TryFrom<&'a K>, { - match self.segments.memory.get(key) { - Ok(Some(cow)) => Ok(Some(cow.into_owned())), - Ok(None) => Ok(None), - Err(error) => Err(error), - } + self.segments.memory.get(key).map(|x| x.into_owned()) } /// Returns a reference to the vector with all builtins present in the virtual machine @@ -2993,7 +2968,6 @@ mod tests { .memory .get(&vm.run_context.get_ap()) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::Int(Felt::new(0x4)), ); @@ -3015,7 +2989,6 @@ mod tests { .memory .get(&vm.run_context.get_ap()) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::Int(Felt::new(0x5)) ); @@ -3038,7 +3011,6 @@ mod tests { .memory .get(&vm.run_context.get_ap()) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::Int(Felt::new(0x14)), ); @@ -3772,17 +3744,14 @@ mod tests { segment_index: 5, offset: 2 }), - Ok(None) + None ); } #[test] fn get_maybe_error() { let vm = vm!(); - assert_eq!( - vm.get_maybe(&MaybeRelocatable::Int(Felt::new(0_i32))), - Err(MemoryError::AddressNotRelocatable) - ); + assert_eq!(vm.get_maybe(&MaybeRelocatable::Int(Felt::new(0_i32))), None,); } #[test] diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index c0033a2282..8ea4a1aac6 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -189,9 +189,12 @@ impl Memory { //If the value is an MaybeRelocatable::Int(Bigint) return &Bigint //else raises Err pub fn get_integer(&self, key: Relocatable) -> Result, VirtualMachineError> { - match self.get(&key).map_err(VirtualMachineError::MemoryError)? { - Some(Cow::Borrowed(MaybeRelocatable::Int(int))) => Ok(Cow::Borrowed(int)), - Some(Cow::Owned(MaybeRelocatable::Int(int))) => Ok(Cow::Owned(int)), + match self + .get(&key) + .ok_or_else(|| VirtualMachineError::UnknownMemoryCell(key))? + { + Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)), + Cow::Owned(MaybeRelocatable::Int(int)) => Ok(Cow::Owned(int)), _ => Err(VirtualMachineError::ExpectedInteger( MaybeRelocatable::from(key), )), @@ -199,9 +202,12 @@ impl Memory { } pub fn get_relocatable(&self, key: Relocatable) -> Result { - match self.get(&key).map_err(VirtualMachineError::MemoryError)? { - Some(Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel))) => Ok(*rel), - Some(Cow::Owned(MaybeRelocatable::RelocatableValue(rel))) => Ok(rel), + match self + .get(&key) + .ok_or_else(|| VirtualMachineError::UnknownMemoryCell(key))? + { + Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel), + Cow::Owned(MaybeRelocatable::RelocatableValue(rel)) => Ok(rel), _ => Err(VirtualMachineError::ExpectedRelocatable( MaybeRelocatable::from(key), )), @@ -256,7 +262,7 @@ impl Memory { let mut values = Vec::new(); for i in 0..size { - values.push(self.get(&addr.add_usize(i))?); + values.push(self.get(&addr.add_usize(i))); } Ok(values) @@ -270,7 +276,7 @@ impl Memory { let mut values = Vec::with_capacity(size); for i in 0..size { - values.push(match self.get(&addr.add_usize(i))? { + values.push(match self.get(&addr.add_usize(i)) { Some(elem) => elem.into_owned(), None => return Err(MemoryError::GetRangeMemoryGap), }); @@ -399,7 +405,7 @@ mod memory_tests { memory.data.push(Vec::new()); memory.insert(&key, &val).unwrap(); assert_eq!( - memory.get(&key).unwrap().unwrap().as_ref(), + memory.get(&key).unwrap().as_ref(), &MaybeRelocatable::from(Felt::new(5)) ); } @@ -409,11 +415,7 @@ mod memory_tests { let mut memory = Memory::new(); memory.temp_data = vec![vec![None, None, Some(mayberelocatable!(8))]]; assert_eq!( - memory - .get(&mayberelocatable!(-1, 2)) - .unwrap() - .unwrap() - .as_ref(), + memory.get(&mayberelocatable!(-1, 2)).unwrap().as_ref(), &mayberelocatable!(8), ); } @@ -439,7 +441,7 @@ mod memory_tests { memory.temp_data.push(Vec::new()); memory.insert(&key, &val).unwrap(); assert_eq!( - memory.get(&key).unwrap().unwrap().as_ref(), + memory.get(&key).unwrap().as_ref(), &MaybeRelocatable::from(Felt::new(5)), ); } @@ -463,26 +465,21 @@ mod memory_tests { fn get_non_allocated_memory() { let key = MaybeRelocatable::from((0, 0)); let memory = Memory::new(); - assert_eq!(memory.get(&key).unwrap(), None); + assert_eq!(memory.get(&key), None); } #[test] fn get_non_existant_element() { let key = MaybeRelocatable::from((0, 0)); let memory = Memory::new(); - assert_eq!(memory.get(&key).unwrap(), None); + assert_eq!(memory.get(&key), None); } #[test] fn get_non_relocatable_key() { let key = MaybeRelocatable::from(Felt::new(0)); let memory = Memory::new(); - let error = memory.get(&key); - assert_eq!(error, Err(MemoryError::AddressNotRelocatable)); - assert_eq!( - error.unwrap_err().to_string(), - "Memory addresses must be relocatable" - ); + assert!(memory.get(&key).is_none()); } #[test] @@ -533,7 +530,7 @@ mod memory_tests { memory.data.push(Vec::new()); memory.insert(&key_a, &val).unwrap(); memory.insert(&key_b, &val).unwrap(); - assert_eq!(memory.get(&key_b).unwrap().unwrap().as_ref(), &val); + assert_eq!(memory.get(&key_b).unwrap().as_ref(), &val); } #[test] @@ -545,11 +542,11 @@ mod memory_tests { memory.data.push(Vec::new()); memory.insert(&key_a, &val).unwrap(); memory.insert(&key_b, &val).unwrap(); - assert_eq!(memory.get(&key_b).unwrap().unwrap().as_ref(), &val); - assert_eq!(memory.get(&MaybeRelocatable::from((0, 1))).unwrap(), None); - assert_eq!(memory.get(&MaybeRelocatable::from((0, 2))).unwrap(), None); - assert_eq!(memory.get(&MaybeRelocatable::from((0, 3))).unwrap(), None); - assert_eq!(memory.get(&MaybeRelocatable::from((0, 4))).unwrap(), None); + assert_eq!(memory.get(&key_b).unwrap().as_ref(), &val); + assert_eq!(memory.get(&MaybeRelocatable::from((0, 1))), None); + assert_eq!(memory.get(&MaybeRelocatable::from((0, 2))), None); + assert_eq!(memory.get(&MaybeRelocatable::from((0, 3))), None); + assert_eq!(memory.get(&MaybeRelocatable::from((0, 4))), None); } #[test] @@ -562,7 +559,7 @@ mod memory_tests { 2, ) .unwrap(); - assert_matches!(mem.get(&MaybeRelocatable::from((1, 0))), Ok(Some(inner)) if inner.clone().into_owned() == MaybeRelocatable::Int(Felt::new(5))); + assert_matches!(mem.get(&MaybeRelocatable::from((1, 0))), Some(inner) if inner.clone().into_owned() == MaybeRelocatable::Int(Felt::new(5))); } #[test] @@ -753,7 +750,7 @@ mod memory_tests { let val = MaybeRelocatable::from(Felt::new(5)); memory.insert(&key, &val).unwrap(); - assert_eq!(memory.get(&key).unwrap().unwrap().as_ref(), &val); + assert_eq!(memory.get(&key).unwrap().as_ref(), &val); } #[test] diff --git a/src/vm/vm_memory/memory_segments.rs b/src/vm/vm_memory/memory_segments.rs index 0eac2de3ce..8abe878952 100644 --- a/src/vm/vm_memory/memory_segments.rs +++ b/src/vm/vm_memory/memory_segments.rs @@ -329,7 +329,7 @@ mod tests { let current_ptr = segments.load_data(&ptr, &data).unwrap(); assert_eq!(current_ptr, MaybeRelocatable::from((0, 1))); assert_eq!( - segments.memory.get(&ptr).unwrap().unwrap().as_ref(), + segments.memory.get(&ptr).unwrap().as_ref(), &MaybeRelocatable::from(Felt::new(4)) ); } @@ -348,7 +348,7 @@ mod tests { assert_eq!(current_ptr, MaybeRelocatable::from((0, 3))); assert_eq!( - segments.memory.get(&ptr).unwrap().unwrap().as_ref(), + segments.memory.get(&ptr).unwrap().as_ref(), &MaybeRelocatable::from(Felt::new(4)) ); assert_eq!( @@ -356,7 +356,6 @@ mod tests { .memory .get(&MaybeRelocatable::from((0, 1))) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::from(Felt::new(5)) ); @@ -365,7 +364,6 @@ mod tests { .memory .get(&MaybeRelocatable::from((0, 2))) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::from(Felt::new(6)) ); From f41c5b5dde69af9504fefbbd2d94916dc58c9557 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 11:57:37 -0300 Subject: [PATCH 03/18] use proper error for get_memory_holes --- .../builtin_hint_processor/blake2s_utils.rs | 36 ++++++++++--------- .../builtin_hint_processor/dict_hint_utils.rs | 1 - .../builtin_hint_processor/math_utils.rs | 2 +- .../builtin_hint_processor/set.rs | 1 - src/hint_processor/hint_processor_utils.rs | 2 -- src/vm/errors/memory_errors.rs | 2 ++ src/vm/runners/builtin_runner/bitwise.rs | 9 ++--- src/vm/runners/builtin_runner/ec_op.rs | 5 +-- src/vm/runners/builtin_runner/hash.rs | 6 ++-- src/vm/runners/builtin_runner/keccak.rs | 2 +- src/vm/runners/builtin_runner/range_check.rs | 7 ++-- src/vm/runners/cairo_runner.rs | 4 ++- src/vm/trace/mod.rs | 2 +- src/vm/vm_memory/memory.rs | 17 ++++++--- src/vm/vm_memory/memory_segments.rs | 12 +++++-- 15 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs index 62680add63..f93d67b6d6 100644 --- a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs +++ b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs @@ -454,10 +454,11 @@ mod tests { ((2, 6), 0), ((2, 7), 0) ]; - assert_eq!( - vm.segments.memory.get(&MaybeRelocatable::from((2, 8))), - Ok(None) - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 8))) + .is_none()); } #[test] @@ -485,10 +486,11 @@ mod tests { ((2, 6), 0), ((2, 7), 0) ]; - assert_eq!( - vm.segments.memory.get(&MaybeRelocatable::from((2, 8))), - Ok(None) - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 8))) + .is_none()); } #[test] @@ -516,10 +518,11 @@ mod tests { ((2, 6), 0), ((2, 7), 0) ]; - assert_eq!( - vm.segments.memory.get(&MaybeRelocatable::from((2, 8))), - Ok(None) - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 8))) + .is_none()); } #[test] @@ -547,9 +550,10 @@ mod tests { ((2, 6), 0), ((2, 7), 20) ]; - assert_eq!( - vm.segments.memory.get(&MaybeRelocatable::from((2, 8))), - Ok(None) - ); + assert!(vm + .segments + .memory + .get(&MaybeRelocatable::from((2, 8))) + .is_none()); } } 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 2227c2bd38..74882d2570 100644 --- a/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs @@ -357,7 +357,6 @@ mod tests { .memory .get(&MaybeRelocatable::from((1, 1))) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::from(12) ); diff --git a/src/hint_processor/builtin_hint_processor/math_utils.rs b/src/hint_processor/builtin_hint_processor/math_utils.rs index 82edc6e73e..7b6dfa0ba5 100644 --- a/src/hint_processor/builtin_hint_processor/math_utils.rs +++ b/src/hint_processor/builtin_hint_processor/math_utils.rs @@ -195,7 +195,7 @@ pub fn assert_not_equal( let b_addr = get_address_from_var_name("b", vm, ids_data, ap_tracking)?; //Check that the ids are in memory match (vm.get_maybe(&a_addr), vm.get_maybe(&b_addr)) { - (Ok(Some(maybe_rel_a)), Ok(Some(maybe_rel_b))) => { + (Some(maybe_rel_a), Some(maybe_rel_b)) => { let maybe_rel_a = maybe_rel_a; let maybe_rel_b = maybe_rel_b; match (maybe_rel_a, maybe_rel_b) { diff --git a/src/hint_processor/builtin_hint_processor/set.rs b/src/hint_processor/builtin_hint_processor/set.rs index da893cbac4..5cb9185ed9 100644 --- a/src/hint_processor/builtin_hint_processor/set.rs +++ b/src/hint_processor/builtin_hint_processor/set.rs @@ -141,7 +141,6 @@ mod tests { .memory .get(&MaybeRelocatable::from((1, 0))) .unwrap() - .unwrap() .as_ref(), &MaybeRelocatable::Int(Felt::zero()) ) diff --git a/src/hint_processor/hint_processor_utils.rs b/src/hint_processor/hint_processor_utils.rs index 9be4601dda..61c05bf4ef 100644 --- a/src/hint_processor/hint_processor_utils.rs +++ b/src/hint_processor/hint_processor_utils.rs @@ -72,7 +72,6 @@ pub fn get_maybe_relocatable_from_reference( let var_addr = compute_addr_from_reference(hint_reference, vm, ap_tracking)?; let value = if hint_reference.dereference { vm.get_maybe(&var_addr) - .map_err(|error| HintError::Internal(VirtualMachineError::MemoryError(error)))? } else { return Ok(MaybeRelocatable::from(var_addr)); }; @@ -184,7 +183,6 @@ fn get_offset_value_reference( if *deref { Ok(vm .get_maybe(&(base_addr + *offset)) - .map_err(|_| HintError::FailedToGetIds)? .ok_or(HintError::FailedToGetIds)?) } else { Ok((base_addr + *offset).into()) diff --git a/src/vm/errors/memory_errors.rs b/src/vm/errors/memory_errors.rs index 3ee84eabd1..19d8b27a68 100644 --- a/src/vm/errors/memory_errors.rs +++ b/src/vm/errors/memory_errors.rs @@ -77,6 +77,8 @@ pub enum MemoryError { FailedToGetReturnValues(usize, Relocatable), #[error(transparent)] InsufficientAllocatedCells(#[from] InsufficientAllocatedCellsError), + #[error("Accessed address {0} has higher offset than the maximal offset {1} encountered in the memory segment.")] + AccessedAddressOffsetBiggerThanSegmentSize(Relocatable, usize), } #[derive(Debug, PartialEq, Eq, Error)] diff --git a/src/vm/runners/builtin_runner/bitwise.rs b/src/vm/runners/builtin_runner/bitwise.rs index f9f7c46af3..8569f6b6f7 100644 --- a/src/vm/runners/builtin_runner/bitwise.rs +++ b/src/vm/runners/builtin_runner/bitwise.rs @@ -81,12 +81,9 @@ impl BitwiseBuiltinRunner { let num_x = memory.get(&x_addr); let num_y = memory.get(&y_addr); - if let ( - Ok(Some(MaybeRelocatable::Int(ref num_x))), - Ok(Some(MaybeRelocatable::Int(ref num_y))), - ) = ( - num_x.as_ref().map(|x| x.as_ref().map(|x| x.as_ref())), - num_y.as_ref().map(|x| x.as_ref().map(|x| x.as_ref())), + if let (Some(MaybeRelocatable::Int(ref num_x)), Some(MaybeRelocatable::Int(ref num_y))) = ( + num_x.as_ref().map(|x| x.as_ref()), + num_y.as_ref().map(|x| x.as_ref()), ) { if num_x.bits() > self.bitwise_builtin.total_n_bits as u64 { return Err(RunnerError::IntegerBiggerThanPowerOfTwo( diff --git a/src/vm/runners/builtin_runner/ec_op.rs b/src/vm/runners/builtin_runner/ec_op.rs index 6b50946c43..618e7c3eff 100644 --- a/src/vm/runners/builtin_runner/ec_op.rs +++ b/src/vm/runners/builtin_runner/ec_op.rs @@ -157,10 +157,7 @@ impl EcOpBuiltinRunner { //If an input cell is not filled, return None let mut input_cells = Vec::>::with_capacity(self.n_input_cells as usize); for i in 0..self.n_input_cells as usize { - match memory - .get(&instance.add_usize(i)) - .map_err(RunnerError::FailedMemoryGet)? - { + match memory.get(&instance.add_usize(i)) { None => return Ok(None), Some(addr) => { input_cells.push(match addr { diff --git a/src/vm/runners/builtin_runner/hash.rs b/src/vm/runners/builtin_runner/hash.rs index 843a9ccf06..a30d8eca88 100644 --- a/src/vm/runners/builtin_runner/hash.rs +++ b/src/vm/runners/builtin_runner/hash.rs @@ -88,9 +88,9 @@ impl HashBuiltinRunner { segment_index: address.segment_index, offset: address.offset - 2, })); - if let (Ok(Some(MaybeRelocatable::Int(num_a))), Ok(Some(MaybeRelocatable::Int(num_b)))) = ( - num_a.as_ref().map(|x| x.as_ref().map(|x| x.as_ref())), - num_b.as_ref().map(|x| x.as_ref().map(|x| x.as_ref())), + if let (Some(MaybeRelocatable::Int(num_a)), Some(MaybeRelocatable::Int(num_b))) = ( + num_a.as_ref().map(|x| x.as_ref()), + num_b.as_ref().map(|x| x.as_ref()), ) { self.verified_addresses.borrow_mut().push(address); diff --git a/src/vm/runners/builtin_runner/keccak.rs b/src/vm/runners/builtin_runner/keccak.rs index 96f4fab64c..a8a0131959 100644 --- a/src/vm/runners/builtin_runner/keccak.rs +++ b/src/vm/runners/builtin_runner/keccak.rs @@ -87,7 +87,7 @@ impl KeccakBuiltinRunner { for i in 0..self.n_input_cells { let val = match memory.get(&(first_input_addr + i as usize)) { - Ok(Some(val)) => val + Some(val) => val .as_ref() .get_int_ref() .ok() diff --git a/src/vm/runners/builtin_runner/range_check.rs b/src/vm/runners/builtin_runner/range_check.rs index 3725a17bb6..971b794951 100644 --- a/src/vm/runners/builtin_runner/range_check.rs +++ b/src/vm/runners/builtin_runner/range_check.rs @@ -90,13 +90,12 @@ impl RangeCheckBuiltinRunner { |memory: &Memory, address: Relocatable| -> Result, MemoryError> { let num = memory .get_integer(address) - .map_err(|_| MemoryError::RangeCheckFoundNonInt(address))? - .as_ref(); - if &Felt::zero() <= num && num < &Felt::one().shl(128_usize) { + .map_err(|_| MemoryError::RangeCheckFoundNonInt(address))?; + if &Felt::zero() <= num.as_ref() && num.as_ref() < &Felt::one().shl(128_usize) { Ok(vec![address.to_owned()]) } else { Err(MemoryError::RangeCheckNumOutOfBounds( - num.clone(), + num.into_owned(), Felt::one().shl(128_usize), )) } diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 53b45d31a5..ec5ddc5a82 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -1632,7 +1632,9 @@ mod tests { assert_eq!( cairo_runner.initialize_vm(&mut vm), - Err(RunnerError::MemoryValidationError(MemoryError::FoundNonInt)) + Err(RunnerError::MemoryValidationError( + MemoryError::RangeCheckFoundNonInt((2, 0).into()) + )) ); } diff --git a/src/vm/trace/mod.rs b/src/vm/trace/mod.rs index e1efb0ace2..3b288359d8 100644 --- a/src/vm/trace/mod.rs +++ b/src/vm/trace/mod.rs @@ -19,7 +19,7 @@ pub fn get_perm_range_check_limits( .try_fold(None, |offsets: Option<(isize, isize)>, trace| { let instruction = memory.get_integer(trace.pc)?; let immediate = - memory.get::(&(trace.pc.segment_index, trace.pc.offset + 1).into())?; + memory.get::(&(trace.pc.segment_index, trace.pc.offset + 1).into()); let instruction = instruction .to_i64() diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 8ea4a1aac6..10a8d95728 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -366,8 +366,11 @@ impl Default for Memory { #[cfg(test)] mod memory_tests { + use std::ops::Shl; + use super::*; use crate::{ + relocatable, types::instance_definitions::ecdsa_instance_def::EcdsaInstanceDef, utils::test_utils::{mayberelocatable, memory}, vm::{ @@ -377,6 +380,7 @@ mod memory_tests { }; use assert_matches::assert_matches; use felt::felt_str; + use num_traits::One; use crate::vm::errors::memory_errors::MemoryError; @@ -601,10 +605,12 @@ mod memory_tests { .unwrap(); builtin.add_validation_rule(&mut segments.memory); let error = segments.memory.validate_existing_memory(); - assert_eq!(error, Err(MemoryError::NumOutOfBounds)); assert_eq!( - error.unwrap_err().to_string(), - "Range-check validation failed, number is out of valid range" + error, + Err(MemoryError::RangeCheckNumOutOfBounds( + Felt::new(-10), + Felt::one().shl(128_u32) + )) ); } @@ -679,7 +685,10 @@ mod memory_tests { segments.memory = memory![((0, 7), (0, 4))]; builtin.add_validation_rule(&mut segments.memory); let error = segments.memory.validate_existing_memory(); - assert_eq!(error, Err(MemoryError::FoundNonInt)); + assert_eq!( + error, + Err(MemoryError::RangeCheckFoundNonInt(relocatable!(0, 7))) + ); assert_eq!( error.unwrap_err().to_string(), "Range-check validation failed, encountered non-int value" diff --git a/src/vm/vm_memory/memory_segments.rs b/src/vm/vm_memory/memory_segments.rs index 8abe878952..3df938d021 100644 --- a/src/vm/vm_memory/memory_segments.rs +++ b/src/vm/vm_memory/memory_segments.rs @@ -217,7 +217,10 @@ impl MemorySegmentManager { } }; if offset > *segment_size { - return Err(MemoryError::NumOutOfBounds); + return Err(MemoryError::AccessedAddressOffsetBiggerThanSegmentSize( + (index as isize, offset).into(), + *segment_size, + )); } offset_set.insert(offset); @@ -607,14 +610,17 @@ mod tests { } #[test] - fn get_memory_holes_out_of_bounds() { + fn get_memory_holes_out_of_address_offset_bigger_than_size() { let mut memory_segment_manager = MemorySegmentManager::new(); memory_segment_manager.segment_used_sizes = Some(vec![2]); let accessed_addresses = vec![(0, 0).into(), (0, 1).into(), (0, 2).into(), (0, 3).into()]; assert_eq!( memory_segment_manager.get_memory_holes(accessed_addresses.into_iter()), - Err(MemoryError::NumOutOfBounds), + Err(MemoryError::AccessedAddressOffsetBiggerThanSegmentSize( + relocatable!(0, 3), + 2 + )), ); } From d73299535223f545da59ee203d1ecfb1735535bf Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 12:30:58 -0300 Subject: [PATCH 04/18] Move MaybeRelocatable methods get_int_ref & get_reloctable to Option --- .../builtin_hint_processor/blake2s_utils.rs | 6 +-- .../builtin_hint_processor_definition.rs | 3 +- .../find_element_hint.rs | 20 +++++---- .../builtin_hint_processor/hint_utils.rs | 4 +- .../builtin_hint_processor/keccak_utils.rs | 41 +++---------------- .../builtin_hint_processor/math_utils.rs | 22 +++++----- .../memcpy_hint_utils.rs | 3 +- .../builtin_hint_processor/memset_utils.rs | 3 +- .../builtin_hint_processor/pow_utils.rs | 5 ++- .../secp/field_utils.rs | 6 +-- .../squash_dict_utils.rs | 8 ++-- src/hint_processor/hint_processor_utils.rs | 6 ++- src/types/relocatable.rs | 25 ++++------- src/vm/errors/vm_errors.rs | 4 +- src/vm/runners/builtin_runner/keccak.rs | 1 - src/vm/runners/builtin_runner/range_check.rs | 3 +- src/vm/vm_core.rs | 4 +- src/vm/vm_memory/memory.rs | 16 +++----- 18 files changed, 72 insertions(+), 108 deletions(-) diff --git a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs index f93d67b6d6..2d941c3e50 100644 --- a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs +++ b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs @@ -272,7 +272,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((2, 0)) + ))) if x == Relocatable::from((2, 0)) ); } @@ -292,7 +292,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal( VirtualMachineError::ExpectedRelocatable(x) - )) if x == MaybeRelocatable::from((1, 0)) + )) if x == Relocatable::from((1, 0)) ); } @@ -340,7 +340,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((2, 0)) + ))) if x == Relocatable::from((2, 0)) ); } diff --git a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs index 6751289e47..b6369dfbb0 100644 --- a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs +++ b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs @@ -447,6 +447,7 @@ impl HintProcessor for BuiltinHintProcessor { #[cfg(test)] mod tests { use super::*; + use crate::types::relocatable::Relocatable; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ any_box, @@ -559,7 +560,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 1)) + ))) if x == Relocatable::from((1, 1)) ); } 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 4a18f6cb1c..675bb12988 100644 --- a/src/hint_processor/builtin_hint_processor/find_element_hint.rs +++ b/src/hint_processor/builtin_hint_processor/find_element_hint.rs @@ -271,7 +271,7 @@ mod tests { run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 4)) + ))) if x == Relocatable::from((1, 4)) ); } @@ -285,7 +285,7 @@ mod tests { run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 1)) + ))) if x == Relocatable::from((1, 1)) ); } @@ -315,9 +315,11 @@ mod tests { #[test] fn find_elm_not_int_n_elms() { - let relocatable = MaybeRelocatable::from((1, 2)); - let (mut vm, ids_data) = - init_vm_ids_data(HashMap::from([("n_elms".to_string(), relocatable.clone())])); + let relocatable = Relocatable::from((1, 2)); + let (mut vm, ids_data) = init_vm_ids_data(HashMap::from([( + "n_elms".to_string(), + MaybeRelocatable::from(relocatable), + )])); assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( @@ -362,10 +364,10 @@ mod tests { assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( - MaybeRelocatable::RelocatableValue(Relocatable { + Relocatable { segment_index: 1, offset: 4 - }) + } ))) ); } @@ -404,7 +406,7 @@ mod tests { run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 1)) + ))) if x == Relocatable::from((1, 1)) ); } @@ -442,7 +444,7 @@ mod tests { run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 2)) + ))) if x == Relocatable::from((1, 2)) ); } diff --git a/src/hint_processor/builtin_hint_processor/hint_utils.rs b/src/hint_processor/builtin_hint_processor/hint_utils.rs index 4a22e4bc0f..304ade7122 100644 --- a/src/hint_processor/builtin_hint_processor/hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/hint_utils.rs @@ -195,7 +195,7 @@ mod tests { get_ptr_from_var_name("value", &vm, &ids_data, &ApTracking::new()), Err(HintError::Internal( VirtualMachineError::ExpectedRelocatable(x) - )) if x == MaybeRelocatable::from((1, 0)) + )) if x == Relocatable::from((1, 0)) ); } @@ -249,7 +249,7 @@ mod tests { get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 0)) + ))) if x == Relocatable::from((1, 0)) ); } } diff --git a/src/hint_processor/builtin_hint_processor/keccak_utils.rs b/src/hint_processor/builtin_hint_processor/keccak_utils.rs index ab19f5475e..f6bbfee3a6 100644 --- a/src/hint_processor/builtin_hint_processor/keccak_utils.rs +++ b/src/hint_processor/builtin_hint_processor/keccak_utils.rs @@ -6,14 +6,8 @@ use crate::{ hint_processor_definition::HintReference, }, serde::deserialize_program::ApTracking, - types::{ - exec_scope::ExecutionScopes, - relocatable::{MaybeRelocatable, Relocatable}, - }, - vm::{ - errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, - vm_core::VirtualMachine, - }, + types::{exec_scope::ExecutionScopes, relocatable::Relocatable}, + vm::{errors::hint_errors::HintError, vm_core::VirtualMachine}, }; use felt::Felt; use num_traits::{One, Signed, ToPrimitive}; @@ -150,27 +144,12 @@ pub fn unsafe_keccak_finalize( offset: keccak_state_ptr.offset + 1, })?; - // this is not very nice code, we should consider adding the sub() method for Relocatable's - let maybe_rel_start_ptr = MaybeRelocatable::RelocatableValue(start_ptr); - let maybe_rel_end_ptr = MaybeRelocatable::RelocatableValue(end_ptr); - - let n_elems = maybe_rel_end_ptr - .sub(&maybe_rel_start_ptr)? - .get_int_ref()? - .to_usize() - .ok_or(VirtualMachineError::BigintToUsizeFail)?; + let n_elems = end_ptr.sub(&start_ptr)?; let mut keccak_input = Vec::new(); - let range = vm - .get_range(&maybe_rel_start_ptr, n_elems) - .map_err(VirtualMachineError::MemoryError)?; - - check_no_nones_in_range(&range)?; - - for maybe_reloc_word in range.into_iter() { - let word = maybe_reloc_word.ok_or(VirtualMachineError::ExpectedIntAtRange(None))?; - let word = word.get_int_ref()?; + let range = vm.get_integer_range(start_ptr, n_elems)?; + for word in range.into_iter() { let mut bytes = word.to_bytes_be(); let mut bytes = { let n_word_bytes = &bytes.len(); @@ -208,13 +187,3 @@ pub(crate) fn left_pad_u64(bytes_vector: &mut [u64], n_zeros: usize) -> Vec res } - -fn check_no_nones_in_range(range: &Vec>) -> Result<(), VirtualMachineError> { - for memory_cell in range { - memory_cell - .as_ref() - .ok_or(VirtualMachineError::NoneInMemoryRange)?; - } - - Ok(()) -} diff --git a/src/hint_processor/builtin_hint_processor/math_utils.rs b/src/hint_processor/builtin_hint_processor/math_utils.rs index 7b6dfa0ba5..322cd073ce 100644 --- a/src/hint_processor/builtin_hint_processor/math_utils.rs +++ b/src/hint_processor/builtin_hint_processor/math_utils.rs @@ -674,7 +674,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 4)) + ))) if x == Relocatable::from((1, 4)) ); } @@ -693,7 +693,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 4)) + ))) if x == Relocatable::from((1, 4)) ); } @@ -842,7 +842,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 3)) + ))) if x == Relocatable::from((1, 3)) ); } @@ -877,7 +877,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 3)) + ))) if x == Relocatable::from((1, 3)) ); } @@ -932,7 +932,7 @@ mod tests { run_hint!(vm, ids_data, hint_code, &mut exec_scopes, &constants), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 0)) + ))) if x == Relocatable::from((1, 0)) ); } @@ -960,7 +960,7 @@ mod tests { run_hint!(vm, ids_data, hint_code, &mut exec_scopes, &constants), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 1)) + ))) if x == Relocatable::from((1, 1)) ); } @@ -1193,7 +1193,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 4)) + ))) if x == Relocatable::from((1, 4)) ); } @@ -1791,7 +1791,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 3)) + ))) if x == Relocatable::from((1, 3)) ); } @@ -1857,7 +1857,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 1)) + ))) if x == Relocatable::from((1, 1)) ); } @@ -1875,7 +1875,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 2)) + ))) if x == Relocatable::from((1, 2)) ); } @@ -1894,7 +1894,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 2)) + ))) if x == Relocatable::from((1, 2)) ); } } diff --git a/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs b/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs index 398ff52ad7..c3f4d09f78 100644 --- a/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs @@ -74,6 +74,7 @@ pub fn memcpy_continue_copying( #[cfg(test)] mod tests { use super::*; + use crate::types::relocatable::Relocatable; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ types::relocatable::MaybeRelocatable, @@ -129,7 +130,7 @@ mod tests { get_integer_from_var_name(var_name, &vm, &ids_data, &ApTracking::default()), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 0)) + ))) if x == Relocatable::from((1, 0)) ); } } diff --git a/src/hint_processor/builtin_hint_processor/memset_utils.rs b/src/hint_processor/builtin_hint_processor/memset_utils.rs index 42e5e44c56..595f58dd8d 100644 --- a/src/hint_processor/builtin_hint_processor/memset_utils.rs +++ b/src/hint_processor/builtin_hint_processor/memset_utils.rs @@ -56,6 +56,7 @@ pub fn memset_continue_loop( #[cfg(test)] mod tests { use super::*; + use crate::types::relocatable::Relocatable; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ any_box, @@ -101,7 +102,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 1)) + ))) if x == Relocatable::from((1, 1)) ); } diff --git a/src/hint_processor/builtin_hint_processor/pow_utils.rs b/src/hint_processor/builtin_hint_processor/pow_utils.rs index 7313d6386b..6688f6bbbd 100644 --- a/src/hint_processor/builtin_hint_processor/pow_utils.rs +++ b/src/hint_processor/builtin_hint_processor/pow_utils.rs @@ -31,6 +31,7 @@ pub fn pow( #[cfg(test)] mod tests { use super::*; + use crate::types::relocatable::Relocatable; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ any_box, @@ -95,7 +96,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 10)) + ))) if x == Relocatable::from((1, 10)) ); } @@ -115,7 +116,7 @@ mod tests { run_hint!(vm, ids_data, hint_code), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 10)) + ))) if x == Relocatable::from((1, 10)) ); } diff --git a/src/hint_processor/builtin_hint_processor/secp/field_utils.rs b/src/hint_processor/builtin_hint_processor/secp/field_utils.rs index 1247fca15c..30176f405e 100644 --- a/src/hint_processor/builtin_hint_processor/secp/field_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/field_utils.rs @@ -387,9 +387,9 @@ mod tests { .map(|(k, v)| (k.to_string(), v)) .collect() ), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x - ))) if x == MaybeRelocatable::from((1, 20)) + ))) if x == Relocatable::from((1, 20)) ); } @@ -484,7 +484,7 @@ mod tests { ), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 10)) + ))) if x == Relocatable::from((1, 10)) ); } 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 94b8dc972b..cae277b33d 100644 --- a/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs +++ b/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs @@ -10,7 +10,7 @@ use crate::{ hint_processor_definition::HintReference, }, serde::deserialize_program::ApTracking, - types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable}, + types::{exec_scope::ExecutionScopes, relocatable::Relocatable}, vm::{ errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, vm_core::VirtualMachine, @@ -268,7 +268,7 @@ pub fn squash_dict( let key_addr = address + DICT_ACCESS_SIZE * i; let key = vm .get_integer(key_addr) - .map_err(|_| VirtualMachineError::ExpectedInteger(MaybeRelocatable::from(key_addr)))?; + .map_err(|_| VirtualMachineError::ExpectedInteger(Relocatable::from(key_addr)))?; access_indices .entry(key.into_owned()) .or_default() @@ -297,6 +297,8 @@ pub fn squash_dict( #[cfg(test)] mod tests { use super::*; + use crate::types::relocatable::MaybeRelocatable; + use crate::types::relocatable::Relocatable; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::{ any_box, @@ -648,7 +650,7 @@ mod tests { run_hint!(vm, ids_data, hint_code, &mut exec_scopes), Err(HintError::Internal(VirtualMachineError::ExpectedInteger( x - ))) if x == MaybeRelocatable::from((1, 0)) + ))) if x == Relocatable::from((1, 0)) ); } diff --git a/src/hint_processor/hint_processor_utils.rs b/src/hint_processor/hint_processor_utils.rs index 61c05bf4ef..5fa2a5fde8 100644 --- a/src/hint_processor/hint_processor_utils.rs +++ b/src/hint_processor/hint_processor_utils.rs @@ -95,7 +95,8 @@ pub fn compute_addr_from_reference( hint_ap_tracking, &hint_reference.offset1, )? - .get_relocatable()? + .get_relocatable() + .ok_or(HintError::FailedToGetIds)? } else { return Err(HintError::NoRegisterInReference); }; @@ -113,7 +114,8 @@ pub fn compute_addr_from_reference( Ok(offset1 + value - .get_int_ref()? + .get_int_ref() + .ok_or(HintError::FailedToGetIds)? .to_usize() .ok_or(VirtualMachineError::BigintToUsizeFail)?) } diff --git a/src/types/relocatable.rs b/src/types/relocatable.rs index f9ba753146..d7580fc74f 100644 --- a/src/types/relocatable.rs +++ b/src/types/relocatable.rs @@ -181,7 +181,7 @@ impl Relocatable { pub fn add_maybe(&self, other: &MaybeRelocatable) -> Result { let num_ref = other .get_int_ref() - .map_err(|_| VirtualMachineError::RelocatableAdd)?; + .ok_or(VirtualMachineError::RelocatableAdd)?; let big_offset: Felt = num_ref + self.offset; let new_offset = big_offset @@ -312,20 +312,18 @@ impl MaybeRelocatable { } //Returns reference to Felt inside self if Int variant or Error if RelocatableValue variant - pub fn get_int_ref(&self) -> Result<&Felt, VirtualMachineError> { + pub fn get_int_ref(&self) -> Option<&Felt> { match self { - MaybeRelocatable::Int(num) => Ok(num), - MaybeRelocatable::RelocatableValue(_) => { - Err(VirtualMachineError::ExpectedInteger(self.clone())) - } + MaybeRelocatable::Int(num) => Some(num), + MaybeRelocatable::RelocatableValue(_) => None, } } //Returns reference to Relocatable inside self if Relocatable variant or Error if Int variant - pub fn get_relocatable(&self) -> Result { + pub fn get_relocatable(&self) -> Option { match self { - MaybeRelocatable::RelocatableValue(rel) => Ok(*rel), - MaybeRelocatable::Int(_) => Err(VirtualMachineError::ExpectedRelocatable(self.clone())), + MaybeRelocatable::RelocatableValue(rel) => Some(*rel), + MaybeRelocatable::Int(_) => None, } } } @@ -798,14 +796,9 @@ mod tests { fn get_relocatable_test() { assert_matches!( mayberelocatable!(1, 2).get_relocatable(), - Ok::(x) if x == relocatable!(1, 2) + Some(x) if x == relocatable!(1, 2) ); - assert_matches!( - mayberelocatable!(3).get_relocatable(), - Err::(VirtualMachineError::ExpectedRelocatable( - x - )) if x == mayberelocatable!(3) - ) + assert_matches!(mayberelocatable!(3).get_relocatable(), None) } #[test] diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs index 8304a4ba27..470591c15c 100644 --- a/src/vm/errors/vm_errors.rs +++ b/src/vm/errors/vm_errors.rs @@ -75,9 +75,9 @@ pub enum VirtualMachineError { #[error("Failed to retrieve value from address {0}")] MemoryGet(MaybeRelocatable), #[error("Expected integer at address {0}")] - ExpectedInteger(MaybeRelocatable), + ExpectedInteger(Relocatable), #[error("Expected relocatable at address {0}")] - ExpectedRelocatable(MaybeRelocatable), + ExpectedRelocatable(Relocatable), #[error("Value: {0} should be positive")] ValueNotPositive(Felt), #[error("Div out of range: 0 < {0} <= {1}")] diff --git a/src/vm/runners/builtin_runner/keccak.rs b/src/vm/runners/builtin_runner/keccak.rs index a8a0131959..4d5b909437 100644 --- a/src/vm/runners/builtin_runner/keccak.rs +++ b/src/vm/runners/builtin_runner/keccak.rs @@ -90,7 +90,6 @@ impl KeccakBuiltinRunner { Some(val) => val .as_ref() .get_int_ref() - .ok() .and_then(|x| x.to_u64()) .ok_or(RunnerError::KeccakInputCellsNotU64)?, _ => return Ok(None), diff --git a/src/vm/runners/builtin_runner/range_check.rs b/src/vm/runners/builtin_runner/range_check.rs index 971b794951..c68f47ab8d 100644 --- a/src/vm/runners/builtin_runner/range_check.rs +++ b/src/vm/runners/builtin_runner/range_check.rs @@ -171,8 +171,7 @@ impl RangeCheckBuiltinRunner { for _ in 0..self.n_parts { let part_val = value .as_ref()? - .get_int_ref() - .ok()? + .get_int_ref()? .mod_floor(&inner_rc_bound) .to_usize()?; rc_bounds = Some(match rc_bounds { diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index c9ec32ad1e..522a11c5f9 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -193,8 +193,8 @@ impl VirtualMachine { ) -> Result<(), VirtualMachineError> { let new_pc: Relocatable = match instruction.pc_update { PcUpdate::Regular => self.run_context.pc + instruction.size(), - PcUpdate::Jump => match &operands.res { - Some(ref res) => res.get_relocatable()?, + 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() { diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 10a8d95728..eabf9d98b4 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -195,9 +195,7 @@ impl Memory { { Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)), Cow::Owned(MaybeRelocatable::Int(int)) => Ok(Cow::Owned(int)), - _ => Err(VirtualMachineError::ExpectedInteger( - MaybeRelocatable::from(key), - )), + _ => Err(VirtualMachineError::ExpectedInteger(Relocatable::from(key))), } } @@ -208,9 +206,9 @@ impl Memory { { Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel), Cow::Owned(MaybeRelocatable::RelocatableValue(rel)) => Ok(rel), - _ => Err(VirtualMachineError::ExpectedRelocatable( - MaybeRelocatable::from(key), - )), + _ => Err(VirtualMachineError::ExpectedRelocatable(Relocatable::from( + key, + ))), } } @@ -689,10 +687,6 @@ mod memory_tests { error, Err(MemoryError::RangeCheckFoundNonInt(relocatable!(0, 7))) ); - assert_eq!( - error.unwrap_err().to_string(), - "Range-check validation failed, encountered non-int value" - ); } #[test] @@ -740,7 +734,7 @@ mod memory_tests { segments.memory.get_integer(Relocatable::from((0, 0))), Err(VirtualMachineError::ExpectedInteger( e - )) if e == MaybeRelocatable::from((0, 0)) + )) if e == Relocatable::from((0, 0)) ); } From 18e8fe06ddab5eb43820e58ce04416bade38e27a Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 12:36:25 -0300 Subject: [PATCH 05/18] Fix tests --- .../builtin_hint_processor/blake2s_utils.rs | 2 +- .../builtin_hint_processor_definition.rs | 4 +++- .../builtin_hint_processor/find_element_hint.rs | 2 +- src/hint_processor/builtin_hint_processor/math_utils.rs | 8 ++++---- src/hint_processor/builtin_hint_processor/pow_utils.rs | 2 +- .../builtin_hint_processor/secp/field_utils.rs | 2 +- src/vm/vm_memory/memory.rs | 4 ++-- 7 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs index 2d941c3e50..cff3881f7a 100644 --- a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs +++ b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs @@ -270,7 +270,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((2, 0)) ); diff --git a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs index b6369dfbb0..d6dca62547 100644 --- a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs +++ b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs @@ -803,7 +803,9 @@ 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::NoneInMemoryRange)) + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( + _ + ))) ); } 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 675bb12988..cd43d28185 100644 --- a/src/hint_processor/builtin_hint_processor/find_element_hint.rs +++ b/src/hint_processor/builtin_hint_processor/find_element_hint.rs @@ -269,7 +269,7 @@ mod tests { let ids_data = ids_data!["array_ptr", "elm_size", "n_elms", "index", "key"]; assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 4)) ); diff --git a/src/hint_processor/builtin_hint_processor/math_utils.rs b/src/hint_processor/builtin_hint_processor/math_utils.rs index 322cd073ce..0fbfb70862 100644 --- a/src/hint_processor/builtin_hint_processor/math_utils.rs +++ b/src/hint_processor/builtin_hint_processor/math_utils.rs @@ -672,7 +672,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 4)) ); @@ -840,7 +840,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 3)) ); @@ -875,7 +875,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 3)) ); @@ -1892,7 +1892,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 2)) ); diff --git a/src/hint_processor/builtin_hint_processor/pow_utils.rs b/src/hint_processor/builtin_hint_processor/pow_utils.rs index 6688f6bbbd..c1cc8ef235 100644 --- a/src/hint_processor/builtin_hint_processor/pow_utils.rs +++ b/src/hint_processor/builtin_hint_processor/pow_utils.rs @@ -94,7 +94,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 10)) ); diff --git a/src/hint_processor/builtin_hint_processor/secp/field_utils.rs b/src/hint_processor/builtin_hint_processor/secp/field_utils.rs index 30176f405e..f572e45ef2 100644 --- a/src/hint_processor/builtin_hint_processor/secp/field_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/field_utils.rs @@ -482,7 +482,7 @@ mod tests { .map(|(k, v)| (k.to_string(), v)) .collect() ), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 10)) ); diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index eabf9d98b4..7526158445 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -680,12 +680,12 @@ mod memory_tests { let mut builtin = RangeCheckBuiltinRunner::new(8, 8, true); let mut segments = MemorySegmentManager::new(); builtin.initialize_segments(&mut segments); - segments.memory = memory![((0, 7), (0, 4))]; + segments.memory = memory![((0, 0), (0, 4))]; builtin.add_validation_rule(&mut segments.memory); let error = segments.memory.validate_existing_memory(); assert_eq!( error, - Err(MemoryError::RangeCheckFoundNonInt(relocatable!(0, 7))) + Err(MemoryError::RangeCheckFoundNonInt(relocatable!(0, 0))) ); } From 2016148dd6f33e4b64771b6827389a0a52817df6 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 12:37:43 -0300 Subject: [PATCH 06/18] Clippy --- .../builtin_hint_processor/squash_dict_utils.rs | 4 ++-- src/vm/vm_memory/memory.rs | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) 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 cae277b33d..596e7dad8e 100644 --- a/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs +++ b/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs @@ -10,7 +10,7 @@ use crate::{ hint_processor_definition::HintReference, }, serde::deserialize_program::ApTracking, - types::{exec_scope::ExecutionScopes, relocatable::Relocatable}, + types::exec_scope::ExecutionScopes, vm::{ errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, vm_core::VirtualMachine, @@ -268,7 +268,7 @@ pub fn squash_dict( let key_addr = address + DICT_ACCESS_SIZE * i; let key = vm .get_integer(key_addr) - .map_err(|_| VirtualMachineError::ExpectedInteger(Relocatable::from(key_addr)))?; + .map_err(|_| VirtualMachineError::ExpectedInteger(key_addr))?; access_indices .entry(key.into_owned()) .or_default() diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 7526158445..497c3a3392 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -195,7 +195,7 @@ impl Memory { { Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)), Cow::Owned(MaybeRelocatable::Int(int)) => Ok(Cow::Owned(int)), - _ => Err(VirtualMachineError::ExpectedInteger(Relocatable::from(key))), + _ => Err(VirtualMachineError::ExpectedInteger(key)), } } @@ -206,9 +206,7 @@ impl Memory { { Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel), Cow::Owned(MaybeRelocatable::RelocatableValue(rel)) => Ok(rel), - _ => Err(VirtualMachineError::ExpectedRelocatable(Relocatable::from( - key, - ))), + _ => Err(VirtualMachineError::ExpectedRelocatable(key)), } } From e410bbe85fd0956032b3abb785e27b0bcd923013 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 15:33:24 -0300 Subject: [PATCH 07/18] Print relocatables & missing members in write_output --- src/vm/runners/cairo_runner.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index ec5ddc5a82..b4c7dbfd0c 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -882,11 +882,11 @@ impl CairoRunner { vm: &mut VirtualMachine, stdout: &mut dyn io::Write, ) -> Result<(), RunnerError> { - let builtin = vm.builtin_runners.iter().find_map(|(k, v)| match k { - &OUTPUT_BUILTIN_NAME => Some(v), - _ => None, - }); - let builtin = match builtin { + let (_, builtin) = match vm + .builtin_runners + .iter() + .find(|(k, _)| k == &OUTPUT_BUILTIN_NAME) + { Some(x) => x, _ => return Ok(()), }; @@ -895,13 +895,18 @@ impl CairoRunner { let segment_index = builtin.base(); #[allow(deprecated)] for i in 0..segment_used_sizes[segment_index] { - let value = vm + let formatted_value = match vm .segments .memory - .get_integer((segment_index as isize, i).into()) - .map_err(|_| RunnerError::MemoryGet((segment_index as isize, i).into()))? - .to_bigint(); - writeln!(stdout, "{value}").map_err(|_| RunnerError::WriteFail)?; + .get(&Relocatable::from((segment_index as isize, i))) + { + Some(val) => match val.as_ref() { + MaybeRelocatable::Int(num) => format!("{}", num.to_bigint()), + MaybeRelocatable::RelocatableValue(rel) => format!("{}", rel), + }, + _ => "".to_string(), + }; + writeln!(stdout, "{formatted_value}").map_err(|_| RunnerError::WriteFail)?; } Ok(()) From ee0b2c8eca0635421c8ec5ed457fbb30ff12de36 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 15:44:02 -0300 Subject: [PATCH 08/18] Add test --- src/vm/runners/cairo_runner.rs | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index b4c7dbfd0c..5e300d7153 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -2821,6 +2821,49 @@ mod tests { assert_eq!(String::from_utf8(stdout), Ok(String::from("1\n17\n"))); } + #[test] + /*Program used: + %builtins output + + func main{output_ptr: felt*}() { + //Memory Gap + Relocatable value + assert [output_ptr + 1] = cast(output_ptr, felt); + let output_ptr = output_ptr + 2; + return (); + }*/ + fn write_output_from_program_gap_relocatable_output() { + //Initialization Phase + let program = program!( + builtins = vec![OUTPUT_BUILTIN_NAME], + data = vec_data!( + (4612671187288162301), + (5198983563776458752), + (2), + (2345108766317314046) + ), + main = Some(0), + ); + let mut cairo_runner = cairo_runner!(program); + let mut vm = vm!(); + cairo_runner.initialize_builtins(&mut vm).unwrap(); + cairo_runner.initialize_segments(&mut vm, None); + let end = cairo_runner.initialize_main_entrypoint(&mut vm).unwrap(); + cairo_runner.initialize_vm(&mut vm).unwrap(); + //Execution Phase + let mut hint_processor = BuiltinHintProcessor::new_empty(); + assert_matches!( + cairo_runner.run_until_pc(end, &mut vm, &mut hint_processor), + Ok(()) + ); + + let mut stdout = Vec::::new(); + cairo_runner.write_output(&mut vm, &mut stdout).unwrap(); + assert_eq!( + String::from_utf8(stdout), + Ok(String::from("\n2:0\n")) + ); + } + #[test] fn write_output_from_preset_memory_neg_output() { let program = program![OUTPUT_BUILTIN_NAME]; From 1dd77a8e8c00b3af35e1a7d25a82e796f704bbd6 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 16:22:06 -0300 Subject: [PATCH 09/18] Remove unused memory errors from RunnerError --- src/vm/errors/runner_errors.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/vm/errors/runner_errors.rs b/src/vm/errors/runner_errors.rs index 00ba382a70..f375746d59 100644 --- a/src/vm/errors/runner_errors.rs +++ b/src/vm/errors/runner_errors.rs @@ -39,10 +39,6 @@ pub enum RunnerError { FailedStringConversion, #[error("Expected integer at address {0:?}")] ExpectedInteger(MaybeRelocatable), - #[error("Failed to retrieve value from address {0:?}")] - MemoryGet(MaybeRelocatable), - #[error(transparent)] - FailedMemoryGet(MemoryError), #[error("EcOpBuiltin: m should be at most {0}")] EcOpBuiltinScalarLimit(Felt), #[error("Given builtins are not in appropiate order")] From ff8c8ce7f5ff38f26f45f21cc1c39d91fc5b6074 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 16:23:06 -0300 Subject: [PATCH 10/18] Remove NegativeBuiltinBase error --- src/vm/errors/runner_errors.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/vm/errors/runner_errors.rs b/src/vm/errors/runner_errors.rs index f375746d59..a89c06b274 100644 --- a/src/vm/errors/runner_errors.rs +++ b/src/vm/errors/runner_errors.rs @@ -95,8 +95,6 @@ pub enum RunnerError { SafeDivFailUsize(usize, usize), #[error(transparent)] MemoryError(#[from] MemoryError), - #[error("Negative builtin base")] - NegBuiltinBase, #[error("keccak_builtin: Failed to get first input address")] KeccakNoFirstInput, #[error("keccak_builtin: Failed to convert input cells to u64 values")] From f15baf44f1de1b07ff56fdeceeca691403f14565 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 17:11:02 -0300 Subject: [PATCH 11/18] Move memory specific errors to MemoryError --- .../builtin_hint_processor/blake2s_utils.rs | 30 ++++++----- .../builtin_hint_processor_definition.rs | 19 +++---- .../cairo_keccak/keccak_hints.rs | 15 +++--- .../builtin_hint_processor/dict_hint_utils.rs | 5 +- .../builtin_hint_processor/dict_manager.rs | 13 +---- .../find_element_hint.rs | 14 +++--- .../builtin_hint_processor/hint_utils.rs | 10 ++-- .../builtin_hint_processor/math_utils.rs | 50 +++++++++---------- .../memcpy_hint_utils.rs | 7 +-- .../builtin_hint_processor/memset_utils.rs | 11 ++-- .../builtin_hint_processor/pow_utils.rs | 14 +++--- .../secp/bigint_utils.rs | 8 +-- .../secp/field_utils.rs | 15 +++--- .../builtin_hint_processor/segments.rs | 3 +- .../builtin_hint_processor/set.rs | 4 +- .../builtin_hint_processor/sha256_utils.rs | 4 +- .../builtin_hint_processor/signature.rs | 2 +- .../squash_dict_utils.rs | 6 +-- .../builtin_hint_processor/uint256_utils.rs | 26 +++++----- src/vm/context/run_context.rs | 4 +- src/vm/errors/hint_errors.rs | 6 ++- src/vm/errors/memory_errors.rs | 7 +++ src/vm/errors/runner_errors.rs | 6 --- src/vm/errors/vm_errors.rs | 10 +--- src/vm/runners/builtin_runner/ec_op.rs | 39 +++++++-------- src/vm/runners/builtin_runner/mod.rs | 20 ++++---- src/vm/runners/cairo_runner.rs | 17 +++---- src/vm/trace/mod.rs | 5 +- src/vm/vm_core.rs | 16 ++++-- src/vm/vm_memory/memory.rs | 16 +++--- 30 files changed, 182 insertions(+), 220 deletions(-) diff --git a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs index cff3881f7a..a53adfa5e5 100644 --- a/src/hint_processor/builtin_hint_processor/blake2s_utils.rs +++ b/src/hint_processor/builtin_hint_processor/blake2s_utils.rs @@ -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; @@ -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(()) } @@ -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(()) } @@ -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::::new(); for i in 0..4 { @@ -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(()) } @@ -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::::new(); for i in 0..4 { @@ -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, @@ -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)) ); @@ -290,8 +288,8 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal( - VirtualMachineError::ExpectedRelocatable(x) + Err(HintError::Memory( + MemoryError::ExpectedRelocatable(x) )) if x == Relocatable::from((1, 0)) ); } @@ -338,7 +336,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((2, 0)) ); @@ -403,13 +401,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)) ); diff --git a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs index d6dca62547..72e6cb0935 100644 --- a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs +++ b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs @@ -455,10 +455,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, }, @@ -507,13 +504,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)) ); @@ -558,7 +555,7 @@ mod tests { let ids_data = ids_data!["len"]; assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 1)) ); @@ -616,13 +613,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()) ); @@ -803,9 +800,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(_))) ); } 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 8b23a3b4d8..c1dcd3ea9d 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 @@ -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(()) } @@ -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() @@ -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(()) } @@ -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(()) } @@ -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] 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 74882d2570..463b69a040 100644 --- a/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs @@ -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::{ @@ -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)) ); diff --git a/src/hint_processor/builtin_hint_processor/dict_manager.rs b/src/hint_processor/builtin_hint_processor/dict_manager.rs index bb70de09c1..5d1b1ff5a9 100644 --- a/src/hint_processor/builtin_hint_processor/dict_manager.rs +++ b/src/hint_processor/builtin_hint_processor/dict_manager.rs @@ -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)] @@ -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( 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 cd43d28185..080cbafda2 100644 --- a/src/hint_processor/builtin_hint_processor/find_element_hint.rs +++ b/src/hint_processor/builtin_hint_processor/find_element_hint.rs @@ -146,7 +146,7 @@ mod tests { }, types::relocatable::{MaybeRelocatable, Relocatable}, utils::test_utils::*, - vm::vm_core::VirtualMachine, + vm::{errors::memory_errors::MemoryError, vm_core::VirtualMachine}, }; use assert_matches::assert_matches; use num_traits::{One, Zero}; @@ -269,7 +269,7 @@ mod tests { let ids_data = ids_data!["array_ptr", "elm_size", "n_elms", "index", "key"]; assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), - Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( + Err(HintError::Memory(MemoryError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 4)) ); @@ -283,7 +283,7 @@ mod tests { )])); assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 1)) ); @@ -322,7 +322,7 @@ mod tests { )])); assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( inner ))) if inner == relocatable ); @@ -363,7 +363,7 @@ mod tests { init_vm_ids_data(HashMap::from([("key".to_string(), relocatable)])); assert_matches!( run_hint!(vm, ids_data, hint_code::FIND_ELEMENT), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( Relocatable { segment_index: 1, offset: 4 @@ -404,7 +404,7 @@ mod tests { )])); assert_matches!( run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 1)) ); @@ -442,7 +442,7 @@ mod tests { )])); assert_matches!( run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 2)) ); diff --git a/src/hint_processor/builtin_hint_processor/hint_utils.rs b/src/hint_processor/builtin_hint_processor/hint_utils.rs index 304ade7122..03c43c2ef9 100644 --- a/src/hint_processor/builtin_hint_processor/hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/hint_utils.rs @@ -124,9 +124,7 @@ mod tests { serde::deserialize_program::OffsetValue, utils::test_utils::*, vm::{ - errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, - vm_core::VirtualMachine, - vm_memory::memory::Memory, + errors::memory_errors::MemoryError, vm_core::VirtualMachine, vm_memory::memory::Memory, }, }; use assert_matches::assert_matches; @@ -193,8 +191,8 @@ mod tests { assert_matches!( get_ptr_from_var_name("value", &vm, &ids_data, &ApTracking::new()), - Err(HintError::Internal( - VirtualMachineError::ExpectedRelocatable(x) + Err(HintError::Memory( + MemoryError::ExpectedRelocatable(x) )) if x == Relocatable::from((1, 0)) ); } @@ -247,7 +245,7 @@ mod tests { assert_matches!( get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 0)) ); diff --git a/src/hint_processor/builtin_hint_processor/math_utils.rs b/src/hint_processor/builtin_hint_processor/math_utils.rs index 0fbfb70862..07deb89e8d 100644 --- a/src/hint_processor/builtin_hint_processor/math_utils.rs +++ b/src/hint_processor/builtin_hint_processor/math_utils.rs @@ -672,7 +672,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((1, 4)) ); @@ -691,7 +691,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 4)) ); @@ -754,13 +754,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((1, 0)) && + )) if x == MaybeRelocatable::from((1, 0)) && y == MaybeRelocatable::Int(Felt::one()) && z == MaybeRelocatable::Int(Felt::zero()) ); @@ -840,7 +840,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((1, 3)) ); @@ -875,7 +875,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((1, 3)) ); @@ -930,7 +930,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code, &mut exec_scopes, &constants), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 0)) ); @@ -958,7 +958,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code, &mut exec_scopes, &constants), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 1)) ); @@ -1191,7 +1191,7 @@ mod tests { let ids_data = ids_data!["value"]; assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 4)) ); @@ -1337,13 +1337,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((1, 1)) && + )) if x == MaybeRelocatable::from((1, 1)) && y == MaybeRelocatable::from(Felt::new(4)) && z == MaybeRelocatable::from(Felt::one()) ); @@ -1397,13 +1397,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((1, 1)) && + )) if x == MaybeRelocatable::from((1, 1)) && y == MaybeRelocatable::from(Felt::new(7)) && z == MaybeRelocatable::from(Felt::new(9)) ); @@ -1475,13 +1475,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((1, 0)) && + )) if x == MaybeRelocatable::from((1, 0)) && y == MaybeRelocatable::Int(Felt::new(5)) && z == MaybeRelocatable::Int(Felt::new(2)) ); @@ -1585,13 +1585,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((1, 1)) && + )) if x == MaybeRelocatable::from((1, 1)) && y == MaybeRelocatable::Int(Felt::new(10)) && z == MaybeRelocatable::Int(Felt::new(31)) ); @@ -1726,13 +1726,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(Felt::new(99)) && z == MaybeRelocatable::from(felt_str!("335438970432432812899076431678123043273")) ); @@ -1760,13 +1760,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, 1)) && + )) if x == MaybeRelocatable::from((2, 1)) && y == MaybeRelocatable::from(Felt::new(99)) && z == MaybeRelocatable::from(Felt::new(0)) ); @@ -1789,7 +1789,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 3)) ); @@ -1855,7 +1855,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 1)) ); @@ -1873,7 +1873,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 2)) ); @@ -1892,7 +1892,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((1, 2)) ); diff --git a/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs b/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs index c3f4d09f78..08ac141d7f 100644 --- a/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs @@ -79,10 +79,7 @@ mod tests { use crate::{ types::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; @@ -128,7 +125,7 @@ mod tests { assert_matches!( get_integer_from_var_name(var_name, &vm, &ids_data, &ApTracking::default()), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 0)) ); diff --git a/src/hint_processor/builtin_hint_processor/memset_utils.rs b/src/hint_processor/builtin_hint_processor/memset_utils.rs index 595f58dd8d..ae8e453848 100644 --- a/src/hint_processor/builtin_hint_processor/memset_utils.rs +++ b/src/hint_processor/builtin_hint_processor/memset_utils.rs @@ -68,10 +68,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::{One, Zero}; @@ -100,7 +97,7 @@ mod tests { let ids_data = ids_data!["n"]; assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 1)) ); @@ -176,13 +173,13 @@ mod tests { let ids_data = ids_data!["continue_loop"]; 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, 0)) && + )) if x == MaybeRelocatable::from((1, 0)) && y == MaybeRelocatable::from(Felt::new(5)) && z == MaybeRelocatable::from(Felt::zero()) ); diff --git a/src/hint_processor/builtin_hint_processor/pow_utils.rs b/src/hint_processor/builtin_hint_processor/pow_utils.rs index c1cc8ef235..732918c486 100644 --- a/src/hint_processor/builtin_hint_processor/pow_utils.rs +++ b/src/hint_processor/builtin_hint_processor/pow_utils.rs @@ -43,10 +43,8 @@ mod tests { types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable}, utils::test_utils::*, vm::{ - errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, - runners::builtin_runner::RangeCheckBuiltinRunner, - vm_core::VirtualMachine, - vm_memory::memory::Memory, + errors::memory_errors::MemoryError, runners::builtin_runner::RangeCheckBuiltinRunner, + vm_core::VirtualMachine, vm_memory::memory::Memory, }, }; use assert_matches::assert_matches; @@ -94,7 +92,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((1, 10)) ); @@ -114,7 +112,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 10)) ); @@ -133,13 +131,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((1, 11)) && + )) if x == MaybeRelocatable::from((1, 11)) && y == MaybeRelocatable::from(Felt::new(3)) && z == MaybeRelocatable::from(Felt::one()) ); 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 ad76586b7f..b6fba2edd6 100644 --- a/src/hint_processor/builtin_hint_processor/secp/bigint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/bigint_utils.rs @@ -8,10 +8,7 @@ use crate::{ }, serde::deserialize_program::ApTracking, types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable}, - vm::{ - errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, - vm_core::VirtualMachine, - }, + vm::{errors::hint_errors::HintError, vm_core::VirtualMachine}, }; use felt::Felt; use std::collections::HashMap; @@ -39,8 +36,7 @@ pub fn nondet_bigint3( .into_iter() .map(|n| MaybeRelocatable::from(Felt::new(n))) .collect(); - vm.write_arg(res_reloc, &arg) - .map_err(VirtualMachineError::MemoryError)?; + vm.write_arg(res_reloc, &arg).map_err(HintError::Memory)?; Ok(()) } diff --git a/src/hint_processor/builtin_hint_processor/secp/field_utils.rs b/src/hint_processor/builtin_hint_processor/secp/field_utils.rs index f572e45ef2..d979046d45 100644 --- a/src/hint_processor/builtin_hint_processor/secp/field_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/field_utils.rs @@ -175,8 +175,7 @@ mod tests { }, utils::test_utils::*, vm::{ - errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, - runners::builtin_runner::RangeCheckBuiltinRunner, + errors::memory_errors::MemoryError, runners::builtin_runner::RangeCheckBuiltinRunner, vm_memory::memory::Memory, }, }; @@ -290,13 +289,13 @@ mod tests { .map(|(k, v)| (k.to_string(), v)) .collect() ), - Err(HintError::Internal(VirtualMachineError::MemoryError( + Err(HintError::Memory( MemoryError::InconsistentMemory( x, y, z ) - ))) if x == MaybeRelocatable::from((1, 9)) && + )) if x == MaybeRelocatable::from((1, 9)) && y == MaybeRelocatable::from(Felt::new(55_i32)) && z == MaybeRelocatable::from(Felt::zero()) ); @@ -387,7 +386,7 @@ mod tests { .map(|(k, v)| (k.to_string(), v)) .collect() ), - Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( + Err(HintError::Memory(MemoryError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 20)) ); @@ -482,7 +481,7 @@ mod tests { .map(|(k, v)| (k.to_string(), v)) .collect() ), - Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell( + Err(HintError::Memory(MemoryError::UnknownMemoryCell( x ))) if x == Relocatable::from((1, 10)) ); @@ -577,13 +576,13 @@ mod tests { //Execute the hint 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(vm.run_context.get_ap()) && + )) if x == MaybeRelocatable::from(vm.run_context.get_ap()) && y == MaybeRelocatable::from(Felt::new(55i32)) && z == MaybeRelocatable::from(Felt::new(1i32)) ); diff --git a/src/hint_processor/builtin_hint_processor/segments.rs b/src/hint_processor/builtin_hint_processor/segments.rs index 42fbd328d8..45a779ca26 100644 --- a/src/hint_processor/builtin_hint_processor/segments.rs +++ b/src/hint_processor/builtin_hint_processor/segments.rs @@ -5,7 +5,6 @@ use crate::hint_processor::{ }; use crate::serde::deserialize_program::ApTracking; use crate::vm::errors::hint_errors::HintError; -use crate::vm::errors::vm_errors::VirtualMachineError; use crate::vm::vm_core::VirtualMachine; use std::collections::HashMap; @@ -22,7 +21,7 @@ pub fn relocate_segment( let dest_ptr = get_ptr_from_var_name("dest_ptr", vm, ids_data, ap_tracking)?; vm.add_relocation_rule(src_ptr, dest_ptr) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(HintError::Memory)?; Ok(()) } diff --git a/src/hint_processor/builtin_hint_processor/set.rs b/src/hint_processor/builtin_hint_processor/set.rs index 5cb9185ed9..a7d249fead 100644 --- a/src/hint_processor/builtin_hint_processor/set.rs +++ b/src/hint_processor/builtin_hint_processor/set.rs @@ -33,7 +33,7 @@ pub fn set_add( } let elm = vm .get_range(&MaybeRelocatable::from(elm_ptr), elm_size) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; if set_ptr > set_end_ptr { return Err(HintError::InvalidSetRange( @@ -47,7 +47,7 @@ pub fn set_add( for i in (0..range_limit).step_by(elm_size) { let set_iter = vm .get_range(&MaybeRelocatable::from(set_ptr + i), elm_size) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; if set_iter == elm { insert_value_from_var_name( diff --git a/src/hint_processor/builtin_hint_processor/sha256_utils.rs b/src/hint_processor/builtin_hint_processor/sha256_utils.rs index fe78128c5b..df8672e035 100644 --- a/src/hint_processor/builtin_hint_processor/sha256_utils.rs +++ b/src/hint_processor/builtin_hint_processor/sha256_utils.rs @@ -74,7 +74,7 @@ pub fn sha256_main( let output_base = get_ptr_from_var_name("output", vm, ids_data, ap_tracking)?; vm.write_arg(output_base, &output) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; Ok(()) } @@ -110,7 +110,7 @@ pub fn sha256_finalize( } vm.write_arg(sha256_ptr_end, &padding) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; Ok(()) } diff --git a/src/hint_processor/builtin_hint_processor/signature.rs b/src/hint_processor/builtin_hint_processor/signature.rs index 2ef5d5a517..b02011b406 100644 --- a/src/hint_processor/builtin_hint_processor/signature.rs +++ b/src/hint_processor/builtin_hint_processor/signature.rs @@ -36,7 +36,7 @@ pub fn verify_ecdsa_signature( } ecdsa_builtin .add_signature(ecdsa_ptr, &(signature_r, signature_s)) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; Ok(()) } 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 596e7dad8e..6dc06cd369 100644 --- a/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs +++ b/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs @@ -12,7 +12,7 @@ use crate::{ serde::deserialize_program::ApTracking, types::exec_scope::ExecutionScopes, vm::{ - errors::{hint_errors::HintError, vm_errors::VirtualMachineError}, + errors::{hint_errors::HintError, memory_errors::MemoryError}, vm_core::VirtualMachine, }, }; @@ -268,7 +268,7 @@ pub fn squash_dict( let key_addr = address + DICT_ACCESS_SIZE * i; let key = vm .get_integer(key_addr) - .map_err(|_| VirtualMachineError::ExpectedInteger(key_addr))?; + .map_err(|_| MemoryError::ExpectedInteger(key_addr))?; access_indices .entry(key.into_owned()) .or_default() @@ -648,7 +648,7 @@ mod tests { //Execute the hint assert_matches!( run_hint!(vm, ids_data, hint_code, &mut exec_scopes), - Err(HintError::Internal(VirtualMachineError::ExpectedInteger( + Err(HintError::Memory(MemoryError::ExpectedInteger( x ))) if x == Relocatable::from((1, 0)) ); diff --git a/src/hint_processor/builtin_hint_processor/uint256_utils.rs b/src/hint_processor/builtin_hint_processor/uint256_utils.rs index 0b27826390..79ed537e65 100644 --- a/src/hint_processor/builtin_hint_processor/uint256_utils.rs +++ b/src/hint_processor/builtin_hint_processor/uint256_utils.rs @@ -234,10 +234,8 @@ mod tests { }, utils::test_utils::*, vm::{ - errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, - runners::builtin_runner::RangeCheckBuiltinRunner, - vm_core::VirtualMachine, - vm_memory::memory::Memory, + errors::memory_errors::MemoryError, runners::builtin_runner::RangeCheckBuiltinRunner, + vm_core::VirtualMachine, vm_memory::memory::Memory, }, }; use assert_matches::assert_matches; @@ -285,13 +283,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((1, 12)) && + )) if x == MaybeRelocatable::from((1, 12)) && y == MaybeRelocatable::from(Felt::new(2)) && z == MaybeRelocatable::from(Felt::zero()) ); @@ -356,13 +354,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((1, 10)) && + )) if x == MaybeRelocatable::from((1, 10)) && y == MaybeRelocatable::from(Felt::zero()) && z == MaybeRelocatable::from(felt_str!("7249717543555297151")) ); @@ -422,13 +420,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((1, 5)) && + )) if x == MaybeRelocatable::from((1, 5)) && y == MaybeRelocatable::from(Felt::one()) && z == MaybeRelocatable::from(felt_str!("48805497317890012913")) ); @@ -492,13 +490,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((1, 5)) && + )) if x == MaybeRelocatable::from((1, 5)) && y == MaybeRelocatable::from(Felt::new(55)) && z == MaybeRelocatable::from(Felt::one()) ); @@ -548,13 +546,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((1, 10)) && + )) if x == MaybeRelocatable::from((1, 10)) && y == MaybeRelocatable::from(Felt::zero()) && z == MaybeRelocatable::from(Felt::new(10)) ); diff --git a/src/vm/context/run_context.rs b/src/vm/context/run_context.rs index ed4f503ed2..804184d944 100644 --- a/src/vm/context/run_context.rs +++ b/src/vm/context/run_context.rs @@ -74,7 +74,7 @@ impl RunContext { }, Op1Addr::Op0 => match op0 { Some(MaybeRelocatable::RelocatableValue(addr)) => *addr, - Some(_) => return Err(VirtualMachineError::MemoryError(AddressNotRelocatable)), + Some(_) => return Err(VirtualMachineError::Memory(AddressNotRelocatable)), None => return Err(VirtualMachineError::UnknownOp0), }, }; @@ -397,7 +397,7 @@ mod tests { let op0 = MaybeRelocatable::from(Felt::new(7)); assert_matches!( run_context.compute_op1_addr(&instruction, Some(&op0)), - Err::(VirtualMachineError::MemoryError( + Err::(VirtualMachineError::Memory( MemoryError::AddressNotRelocatable )) ); diff --git a/src/vm/errors/hint_errors.rs b/src/vm/errors/hint_errors.rs index 3018382e4a..02dfdb3481 100644 --- a/src/vm/errors/hint_errors.rs +++ b/src/vm/errors/hint_errors.rs @@ -4,7 +4,9 @@ use thiserror::Error; use crate::types::relocatable::{MaybeRelocatable, Relocatable}; -use super::{exec_scope_errors::ExecScopeError, vm_errors::VirtualMachineError}; +use super::{ + exec_scope_errors::ExecScopeError, memory_errors::MemoryError, vm_errors::VirtualMachineError, +}; #[derive(Debug, Error)] pub enum HintError { @@ -80,6 +82,8 @@ pub enum HintError { NAccessesTooBig(Felt), #[error(transparent)] Internal(#[from] VirtualMachineError), + #[error(transparent)] + Memory(#[from] MemoryError), #[error("Couldn't convert BigInt to usize")] BigintToUsizeFail, #[error("usort() can only be used with input_len<={0}. Got: input_len={1}.")] diff --git a/src/vm/errors/memory_errors.rs b/src/vm/errors/memory_errors.rs index 19d8b27a68..d07abe73d8 100644 --- a/src/vm/errors/memory_errors.rs +++ b/src/vm/errors/memory_errors.rs @@ -79,6 +79,13 @@ pub enum MemoryError { InsufficientAllocatedCells(#[from] InsufficientAllocatedCellsError), #[error("Accessed address {0} has higher offset than the maximal offset {1} encountered in the memory segment.")] AccessedAddressOffsetBiggerThanSegmentSize(Relocatable, usize), + // Memory.get() errors + #[error("Expected integer at address {0}")] + ExpectedInteger(Relocatable), + #[error("Expected relocatable at address {0}")] + ExpectedRelocatable(Relocatable), + #[error("Unknown memory cell at address {0}")] + UnknownMemoryCell(Relocatable), } #[derive(Debug, PartialEq, Eq, Error)] diff --git a/src/vm/errors/runner_errors.rs b/src/vm/errors/runner_errors.rs index a89c06b274..04f0a58471 100644 --- a/src/vm/errors/runner_errors.rs +++ b/src/vm/errors/runner_errors.rs @@ -15,8 +15,6 @@ pub enum RunnerError { NoProgBase, #[error("Missing main()")] MissingMain, - #[error("Uninitialized base for builtin")] - UninitializedBase, #[error("Base for builtin is not finished")] BaseNotFinished, #[error("Failed to write program output")] @@ -31,10 +29,6 @@ pub enum RunnerError { MemoryValidationError(MemoryError), #[error("Memory loading failed during state initialization: {0}")] MemoryInitializationError(MemoryError), - #[error("Memory addresses must be relocatable")] - NonRelocatableAddress, - #[error("Runner base mustn't be in a TemporarySegment, segment: {0}")] - RunnerInTemporarySegment(isize), #[error("Failed to convert string to FieldElement")] FailedStringConversion, #[error("Expected integer at address {0:?}")] diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs index 470591c15c..c11c78873a 100644 --- a/src/vm/errors/vm_errors.rs +++ b/src/vm/errors/vm_errors.rs @@ -67,17 +67,11 @@ pub enum VirtualMachineError { #[error("Invalid hint encoding at pc: {0}")] InvalidHintEncoding(MaybeRelocatable), #[error(transparent)] - MemoryError(#[from] MemoryError), + Memory(#[from] MemoryError), #[error("Expected range_check builtin to be present")] NoRangeCheckBuiltin, #[error("Expected ecdsa builtin to be present")] NoSignatureBuiltin, - #[error("Failed to retrieve value from address {0}")] - MemoryGet(MaybeRelocatable), - #[error("Expected integer at address {0}")] - ExpectedInteger(Relocatable), - #[error("Expected relocatable at address {0}")] - ExpectedRelocatable(Relocatable), #[error("Value: {0} should be positive")] ValueNotPositive(Felt), #[error("Div out of range: 0 < {0} <= {1}")] @@ -150,8 +144,6 @@ pub enum VirtualMachineError { InvalidMemoryValueTemporaryAddress(Relocatable), #[error("accessed_addresses is None.")] MissingAccessedAddresses, - #[error("Unknown memory cell at address {0}")] - UnknownMemoryCell(Relocatable), #[error(transparent)] Other(Box), } diff --git a/src/vm/runners/builtin_runner/ec_op.rs b/src/vm/runners/builtin_runner/ec_op.rs index 618e7c3eff..cfceb3099e 100644 --- a/src/vm/runners/builtin_runner/ec_op.rs +++ b/src/vm/runners/builtin_runner/ec_op.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use crate::math_utils::{ec_add, ec_double, safe_div_usize}; use crate::types::instance_definitions::ec_op_instance_def::{ EcOpInstanceDef, CELLS_PER_EC_OP, INPUT_CELLS_PER_EC_OP, @@ -12,7 +14,6 @@ use felt::Felt; use num_bigint::{BigInt, ToBigInt}; use num_integer::{div_ceil, Integer}; use num_traits::{Num, One, Pow, Zero}; -use std::borrow::Cow; use super::EC_OP_BUILTIN_NAME; @@ -152,18 +153,22 @@ impl EcOpBuiltinRunner { if index != OUTPUT_INDICES.0 && index != OUTPUT_INDICES.1 { return Ok(None); } - let instance = MaybeRelocatable::from((address.segment_index, address.offset - index)); + let instance = Relocatable::from((address.segment_index, address.offset - index)); //All input cells should be filled, and be integer values //If an input cell is not filled, return None - let mut input_cells = Vec::>::with_capacity(self.n_input_cells as usize); + 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.add_usize(i)) { + match memory.get(&(instance + i)) { None => return Ok(None), Some(addr) => { input_cells.push(match addr { - Cow::Borrowed(MaybeRelocatable::Int(num)) => Cow::Borrowed(num), - Cow::Owned(MaybeRelocatable::Int(num)) => Cow::Owned(num), - _ => return Err(RunnerError::ExpectedInteger(instance.add_usize(i))), + // Only relocatable values can be owned + Cow::Borrowed(MaybeRelocatable::Int(ref num)) => num, + _ => { + return Err(RunnerError::MemoryError(MemoryError::ExpectedInteger( + instance + i, + ))) + } }); } }; @@ -178,29 +183,23 @@ impl EcOpBuiltinRunner { // Assert that if the current address is part of a point, the point is on the curve for pair in &EC_POINT_INDICES[0..2] { if !EcOpBuiltinRunner::point_on_curve( - input_cells[pair.0].as_ref(), - input_cells[pair.1].as_ref(), + input_cells[pair.0], + input_cells[pair.1], &alpha, &beta, ) { return Err(RunnerError::PointNotOnCurve(( - input_cells[pair.0].clone().into_owned(), - input_cells[pair.1].clone().into_owned(), + input_cells[pair.0].clone(), + input_cells[pair.1].clone(), ))); }; } let prime = BigInt::from_str_radix(&felt::PRIME_STR[2..], 16) .map_err(|_| RunnerError::CouldntParsePrime)?; let result = EcOpBuiltinRunner::ec_op_impl( - ( - input_cells[0].as_ref().to_owned(), - input_cells[1].as_ref().to_owned(), - ), - ( - input_cells[2].as_ref().to_owned(), - input_cells[3].as_ref().to_owned(), - ), - input_cells[4].as_ref(), + (input_cells[0].to_owned(), input_cells[1].to_owned()), + (input_cells[2].to_owned(), input_cells[3].to_owned()), + input_cells[4], #[allow(deprecated)] &alpha.to_bigint(), &prime, diff --git a/src/vm/runners/builtin_runner/mod.rs b/src/vm/runners/builtin_runner/mod.rs index 232164563e..d3fe20f70a 100644 --- a/src/vm/runners/builtin_runner/mod.rs +++ b/src/vm/runners/builtin_runner/mod.rs @@ -1026,7 +1026,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCellsWithOffsets(BITWISE_BUILTIN_NAME, x) )) if x == vec![0] ); @@ -1054,7 +1054,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCells(BITWISE_BUILTIN_NAME) )) ); @@ -1075,7 +1075,7 @@ mod tests { ]]; assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCellsWithOffsets(HASH_BUILTIN_NAME, x) )) if x == vec![0] ); @@ -1093,7 +1093,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCells(HASH_BUILTIN_NAME) )) ); @@ -1118,7 +1118,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCells(RANGE_CHECK_BUILTIN_NAME) )) ); @@ -1134,7 +1134,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCells(RANGE_CHECK_BUILTIN_NAME) )) ); @@ -1200,7 +1200,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCells(EC_OP_BUILTIN_NAME) )) ); @@ -1222,7 +1222,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCells(EC_OP_BUILTIN_NAME) )) ); @@ -1246,7 +1246,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCellsWithOffsets(EC_OP_BUILTIN_NAME, x) )) if x == vec![0] ); @@ -1277,7 +1277,7 @@ mod tests { assert_matches!( builtin.run_security_checks(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::MissingMemoryCellsWithOffsets(EC_OP_BUILTIN_NAME, x) )) if x == vec![7] ); diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 5e300d7153..fc3268ee2a 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -736,9 +736,8 @@ impl CairoRunner { match self.check_used_cells(vm) { Ok(_) => break, Err(e) => match e { - VirtualMachineError::MemoryError( - MemoryError::InsufficientAllocatedCells(_), - ) => {} + VirtualMachineError::Memory(MemoryError::InsufficientAllocatedCells(_)) => { + } e => return Err(e), }, } @@ -1279,7 +1278,7 @@ mod tests { vm.segments.segment_used_sizes = Some(vec![4, 12]); assert_matches!( cairo_runner.check_memory_usage(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::InsufficientAllocatedCells(_) )) ); @@ -3351,7 +3350,7 @@ mod tests { vm.builtin_runners = vec![]; assert_matches!( cairo_runner.check_diluted_check_usage(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::InsufficientAllocatedCells(_) )) ); @@ -3807,7 +3806,7 @@ mod tests { assert_matches!( cairo_runner.check_range_check_usage(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::InsufficientAllocatedCells(_) )) ); @@ -3871,7 +3870,7 @@ mod tests { assert_matches!( cairo_runner.check_used_cells(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::InsufficientAllocatedCells(_) )) ); @@ -3896,7 +3895,7 @@ mod tests { assert_matches!( cairo_runner.check_used_cells(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::InsufficientAllocatedCells(_) )) ); @@ -3912,7 +3911,7 @@ mod tests { assert_matches!( cairo_runner.check_used_cells(&vm), - Err(VirtualMachineError::MemoryError( + Err(VirtualMachineError::Memory( MemoryError::InsufficientAllocatedCells(_) )) ); diff --git a/src/vm/trace/mod.rs b/src/vm/trace/mod.rs index 3b288359d8..6f699c403a 100644 --- a/src/vm/trace/mod.rs +++ b/src/vm/trace/mod.rs @@ -1,6 +1,7 @@ use self::trace_entry::TraceEntry; use super::{ - decoding::decoder::decode_instruction, errors::vm_errors::VirtualMachineError, + decoding::decoder::decode_instruction, + errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, vm_memory::memory::Memory, }; use crate::types::relocatable::{MaybeRelocatable, Relocatable}; @@ -28,7 +29,7 @@ pub fn get_perm_range_check_limits( .map(|x| match x { Cow::Borrowed(MaybeRelocatable::Int(value)) => Ok(value.clone()), Cow::Owned(MaybeRelocatable::Int(value)) => Ok(value), - _ => Err(VirtualMachineError::ExpectedInteger( + _ => Err(MemoryError::ExpectedInteger( (trace.pc.segment_index, trace.pc.offset + 1).into(), )), }) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 522a11c5f9..dea0ca4864 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -395,19 +395,19 @@ impl VirtualMachine { self.segments .memory .insert(&operands_addresses.op0_addr, &operands.op0) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; } if deduced_operands.was_op1_deducted() { self.segments .memory .insert(&operands_addresses.op1_addr, &operands.op1) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; } if deduced_operands.was_dest_deducted() { self.segments .memory .insert(&operands_addresses.dst_addr, &operands.dst) - .map_err(VirtualMachineError::MemoryError)?; + .map_err(VirtualMachineError::Memory)?; } Ok(()) @@ -788,12 +788,18 @@ impl VirtualMachine { ///Gets the integer value corresponding to the Relocatable address pub fn get_integer(&self, key: Relocatable) -> Result, VirtualMachineError> { - self.segments.memory.get_integer(key) + self.segments + .memory + .get_integer(key) + .map_err(VirtualMachineError::Memory) } ///Gets the relocatable value corresponding to the Relocatable address pub fn get_relocatable(&self, key: Relocatable) -> Result { - self.segments.memory.get_relocatable(key) + self.segments + .memory + .get_relocatable(key) + .map_err(VirtualMachineError::Memory) } ///Gets a MaybeRelocatable value from memory indicated by a generic address diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 497c3a3392..06b399aa2d 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -188,25 +188,25 @@ impl Memory { //Gets the value from memory address. //If the value is an MaybeRelocatable::Int(Bigint) return &Bigint //else raises Err - pub fn get_integer(&self, key: Relocatable) -> Result, VirtualMachineError> { + pub fn get_integer(&self, key: Relocatable) -> Result, MemoryError> { match self .get(&key) - .ok_or_else(|| VirtualMachineError::UnknownMemoryCell(key))? + .ok_or_else(|| MemoryError::UnknownMemoryCell(key))? { Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)), Cow::Owned(MaybeRelocatable::Int(int)) => Ok(Cow::Owned(int)), - _ => Err(VirtualMachineError::ExpectedInteger(key)), + _ => Err(MemoryError::ExpectedInteger(key)), } } - pub fn get_relocatable(&self, key: Relocatable) -> Result { + pub fn get_relocatable(&self, key: Relocatable) -> Result { match self .get(&key) - .ok_or_else(|| VirtualMachineError::UnknownMemoryCell(key))? + .ok_or_else(|| MemoryError::UnknownMemoryCell(key))? { Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel), Cow::Owned(MaybeRelocatable::RelocatableValue(rel)) => Ok(rel), - _ => Err(VirtualMachineError::ExpectedRelocatable(key)), + _ => Err(MemoryError::ExpectedRelocatable(key)), } } @@ -216,7 +216,7 @@ impl Memory { val: T, ) -> Result<(), VirtualMachineError> { self.insert(&key, &val.into()) - .map_err(VirtualMachineError::MemoryError) + .map_err(VirtualMachineError::Memory) } pub fn add_validation_rule(&mut self, segment_index: usize, rule: ValidationRule) { @@ -730,7 +730,7 @@ mod memory_tests { .unwrap(); assert_matches!( segments.memory.get_integer(Relocatable::from((0, 0))), - Err(VirtualMachineError::ExpectedInteger( + Err(MemoryError::ExpectedInteger( e )) if e == Relocatable::from((0, 0)) ); From 02af0a350de694efafd72e2fd3837a1ea711183b Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 17:35:53 -0300 Subject: [PATCH 12/18] Remove errors --- src/vm/errors/vm_errors.rs | 2 -- src/vm/runners/builtin_runner/mod.rs | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs index c11c78873a..46f17d5b13 100644 --- a/src/vm/errors/vm_errors.rs +++ b/src/vm/errors/vm_errors.rs @@ -138,8 +138,6 @@ pub enum VirtualMachineError { OutOfBoundsBuiltinSegmentAccess, #[error("Out of bounds access to program segment")] OutOfBoundsProgramSegmentAccess, - #[error("Negative builtin base")] - NegBuiltinBase, #[error("Security Error: Invalid Memory Value: temporary address not relocated: {0}")] InvalidMemoryValueTemporaryAddress(Relocatable), #[error("accessed_addresses is None.")] diff --git a/src/vm/runners/builtin_runner/mod.rs b/src/vm/runners/builtin_runner/mod.rs index d3fe20f70a..b69dffdbf3 100644 --- a/src/vm/runners/builtin_runner/mod.rs +++ b/src/vm/runners/builtin_runner/mod.rs @@ -19,7 +19,6 @@ pub use bitwise::BitwiseBuiltinRunner; pub use ec_op::EcOpBuiltinRunner; pub use hash::HashBuiltinRunner; use num_integer::div_floor; -use num_traits::ToPrimitive; pub use output::OutputBuiltinRunner; pub use range_check::RangeCheckBuiltinRunner; pub use signature::SignatureBuiltinRunner; @@ -305,10 +304,7 @@ impl BuiltinRunner { } let cells_per_instance = self.cells_per_instance() as usize; let n_input_cells = self.n_input_cells() as usize; - let builtin_segment_index = self - .base() - .to_usize() - .ok_or(VirtualMachineError::NegBuiltinBase)?; + let builtin_segment_index = self.base(); // If the builtin's segment is empty, there are no security checks to run let builtin_segment = match vm.segments.memory.data.get(builtin_segment_index) { Some(segment) if !segment.is_empty() => segment, From 750acddecc4470a0c556aa656e8fa4d783a6eadf Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 17:49:57 -0300 Subject: [PATCH 13/18] Change error mapping --- .../builtin_hint_processor/hint_utils.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/hint_processor/builtin_hint_processor/hint_utils.rs b/src/hint_processor/builtin_hint_processor/hint_utils.rs index 03c43c2ef9..8bafd3bb12 100644 --- a/src/hint_processor/builtin_hint_processor/hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/hint_utils.rs @@ -22,8 +22,10 @@ pub fn insert_value_from_var_name( ap_tracking: &ApTracking, ) -> 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) + vm.segments + .memory + .insert(&var_address, &value.into()) + .map_err(HintError::Memory) } //Inserts value into ap @@ -31,8 +33,10 @@ pub fn insert_value_into_ap( vm: &mut VirtualMachine, value: impl Into, ) -> Result<(), HintError> { - vm.insert_value(vm.get_ap(), value) - .map_err(HintError::Internal) + vm.segments + .memory + .insert(&vm.get_ap(), &value.into()) + .map_err(HintError::Memory) } //Returns the Relocatable value stored in the given ids variable From 2a01736cd92322c28d64dde45788cbc3452d3b1b Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 17:54:30 -0300 Subject: [PATCH 14/18] Remove ExpectedInteger from RunnerError --- src/vm/errors/runner_errors.rs | 4 +--- src/vm/runners/builtin_runner/ec_op.rs | 6 ++++-- src/vm/runners/builtin_runner/keccak.rs | 12 +++++++----- src/vm/runners/builtin_runner/output.rs | 4 +--- src/vm/runners/builtin_runner/signature.rs | 4 +--- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/vm/errors/runner_errors.rs b/src/vm/errors/runner_errors.rs index 04f0a58471..452028da79 100644 --- a/src/vm/errors/runner_errors.rs +++ b/src/vm/errors/runner_errors.rs @@ -31,8 +31,6 @@ pub enum RunnerError { MemoryInitializationError(MemoryError), #[error("Failed to convert string to FieldElement")] FailedStringConversion, - #[error("Expected integer at address {0:?}")] - ExpectedInteger(MaybeRelocatable), #[error("EcOpBuiltin: m should be at most {0}")] EcOpBuiltinScalarLimit(Felt), #[error("Given builtins are not in appropiate order")] @@ -88,7 +86,7 @@ pub enum RunnerError { #[error("{0} is not divisible by {1}")] SafeDivFailUsize(usize, usize), #[error(transparent)] - MemoryError(#[from] MemoryError), + Memory(#[from] MemoryError), #[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/runners/builtin_runner/ec_op.rs b/src/vm/runners/builtin_runner/ec_op.rs index cfceb3099e..10de79728b 100644 --- a/src/vm/runners/builtin_runner/ec_op.rs +++ b/src/vm/runners/builtin_runner/ec_op.rs @@ -165,7 +165,7 @@ impl EcOpBuiltinRunner { // Only relocatable values can be owned Cow::Borrowed(MaybeRelocatable::Int(ref num)) => num, _ => { - return Err(RunnerError::MemoryError(MemoryError::ExpectedInteger( + return Err(RunnerError::Memory(MemoryError::ExpectedInteger( instance + i, ))) } @@ -895,7 +895,9 @@ mod tests { assert_eq!( builtin.deduce_memory_cell(Relocatable::from((3, 6)), &memory), - Err(RunnerError::ExpectedInteger(MaybeRelocatable::from((3, 3)))) + Err(RunnerError::Memory(MemoryError::ExpectedInteger( + Relocatable::from((3, 3)) + ))) ); } diff --git a/src/vm/runners/builtin_runner/keccak.rs b/src/vm/runners/builtin_runner/keccak.rs index 4d5b909437..b6a60e21df 100644 --- a/src/vm/runners/builtin_runner/keccak.rs +++ b/src/vm/runners/builtin_runner/keccak.rs @@ -99,10 +99,7 @@ impl KeccakBuiltinRunner { } if let Some((i, bits)) = self.state_rep.iter().enumerate().next() { - let val = memory - .get_integer(first_input_addr + i) - .map_err(|_| RunnerError::ExpectedInteger((first_input_addr + i).into()))?; - + 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(), @@ -605,7 +602,12 @@ mod tests { let result = builtin.deduce_memory_cell(Relocatable::from((0, 99)), &memory); - assert_eq!(result, Err(RunnerError::ExpectedInteger((0, 0).into()))); + assert_eq!( + result, + Err(RunnerError::Memory(MemoryError::ExpectedInteger( + (0, 0).into() + ))) + ); } #[test] diff --git a/src/vm/runners/builtin_runner/output.rs b/src/vm/runners/builtin_runner/output.rs index d77a95db4c..820942c4d5 100644 --- a/src/vm/runners/builtin_runner/output.rs +++ b/src/vm/runners/builtin_runner/output.rs @@ -99,9 +99,7 @@ impl OutputBuiltinRunner { )); } let stop_ptr = stop_pointer.offset; - let used = self - .get_used_cells(segments) - .map_err(RunnerError::MemoryError)?; + let used = self.get_used_cells(segments).map_err(RunnerError::Memory)?; if stop_ptr != used { return Err(RunnerError::InvalidStopPointer( OUTPUT_BUILTIN_NAME, diff --git a/src/vm/runners/builtin_runner/signature.rs b/src/vm/runners/builtin_runner/signature.rs index 839f9b0c1a..8019ef63a4 100644 --- a/src/vm/runners/builtin_runner/signature.rs +++ b/src/vm/runners/builtin_runner/signature.rs @@ -595,9 +595,7 @@ mod tests { vm.segments = segments![((0, 0), (0, 0))]; assert_eq!( builtin.final_stack(&vm.segments, (0, 1).into()), - Err(RunnerError::MemoryError( - MemoryError::MissingSegmentUsedSizes - )) + Err(RunnerError::Memory(MemoryError::MissingSegmentUsedSizes)) ) } } From 4c8fa30e8d4b301f49334ad84e81aa9f47f9de9b Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 18:05:14 -0300 Subject: [PATCH 15/18] Use MemoryError for memory-related errors --- .../builtin_hint_processor/hint_utils.rs | 8 ++------ .../builtin_hint_processor/math_utils.rs | 2 +- .../squash_dict_utils.rs | 4 ++-- .../builtin_hint_processor/uint256_utils.rs | 2 +- src/hint_processor/hint_processor_utils.rs | 5 ++--- src/vm/vm_core.rs | 18 ++++++------------ src/vm/vm_memory/memory.rs | 7 +++---- 7 files changed, 17 insertions(+), 29 deletions(-) diff --git a/src/hint_processor/builtin_hint_processor/hint_utils.rs b/src/hint_processor/builtin_hint_processor/hint_utils.rs index 8bafd3bb12..867742b521 100644 --- a/src/hint_processor/builtin_hint_processor/hint_utils.rs +++ b/src/hint_processor/builtin_hint_processor/hint_utils.rs @@ -22,9 +22,7 @@ pub fn insert_value_from_var_name( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let var_address = get_relocatable_from_var_name(var_name, vm, ids_data, ap_tracking)?; - vm.segments - .memory - .insert(&var_address, &value.into()) + vm.insert_value(var_address, value) .map_err(HintError::Memory) } @@ -33,9 +31,7 @@ pub fn insert_value_into_ap( vm: &mut VirtualMachine, value: impl Into, ) -> Result<(), HintError> { - vm.segments - .memory - .insert(&vm.get_ap(), &value.into()) + vm.insert_value(vm.get_ap(), value) .map_err(HintError::Memory) } diff --git a/src/hint_processor/builtin_hint_processor/math_utils.rs b/src/hint_processor/builtin_hint_processor/math_utils.rs index 07deb89e8d..04335830fb 100644 --- a/src/hint_processor/builtin_hint_processor/math_utils.rs +++ b/src/hint_processor/builtin_hint_processor/math_utils.rs @@ -306,7 +306,7 @@ pub fn split_int( if &res > bound { return Err(HintError::SplitIntLimbOutOfRange(res)); } - vm.insert_value(output, res).map_err(HintError::Internal) + vm.insert_value(output, res).map_err(HintError::Memory) } //from starkware.cairo.common.math_utils import is_positive 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 6dc06cd369..3e30913506 100644 --- a/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs +++ b/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs @@ -67,7 +67,7 @@ pub fn squash_dict_inner_first_iteration( exec_scopes.insert_value("current_access_index", first_val.clone()); //Insert current_accesss_index into range_check_ptr vm.insert_value(range_check_ptr, first_val) - .map_err(HintError::Internal) + .map_err(HintError::Memory) } // Implements Hint: ids.should_skip_loop = 0 if current_access_indices else 1 @@ -143,7 +143,7 @@ pub fn squash_dict_inner_continue_loop( //Insert loop_temps.delta_minus1 into memory let should_continue_addr = loop_temps_addr + 3_i32; vm.insert_value(should_continue_addr, should_continue) - .map_err(HintError::Internal) + .map_err(HintError::Memory) } // Implements Hint: assert len(current_access_indices) == 0 diff --git a/src/hint_processor/builtin_hint_processor/uint256_utils.rs b/src/hint_processor/builtin_hint_processor/uint256_utils.rs index 79ed537e65..d7ba38457d 100644 --- a/src/hint_processor/builtin_hint_processor/uint256_utils.rs +++ b/src/hint_processor/builtin_hint_processor/uint256_utils.rs @@ -128,7 +128,7 @@ pub fn uint256_sqrt( } vm.insert_value(root_addr, Felt::new(root))?; vm.insert_value(root_addr + 1_i32, Felt::zero()) - .map_err(HintError::Internal) + .map_err(HintError::Memory) } /* diff --git a/src/hint_processor/hint_processor_utils.rs b/src/hint_processor/hint_processor_utils.rs index 5fa2a5fde8..e8fbbfba13 100644 --- a/src/hint_processor/hint_processor_utils.rs +++ b/src/hint_processor/hint_processor_utils.rs @@ -23,8 +23,7 @@ pub fn insert_value_from_reference( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let var_addr = compute_addr_from_reference(hint_reference, vm, ap_tracking)?; - vm.insert_value(var_addr, value) - .map_err(HintError::Internal) + vm.insert_value(var_addr, value).map_err(HintError::Memory) } ///Returns the Integer value stored in the given ids variable @@ -41,7 +40,7 @@ pub fn get_integer_from_reference<'a>( } let var_addr = compute_addr_from_reference(hint_reference, vm, ap_tracking)?; - vm.get_integer(var_addr).map_err(HintError::Internal) + vm.get_integer(var_addr).map_err(HintError::Memory) } ///Returns the Relocatable value stored in the given ids variable diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index dea0ca4864..a6a8f80f12 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -787,19 +787,13 @@ impl VirtualMachine { } ///Gets the integer value corresponding to the Relocatable address - pub fn get_integer(&self, key: Relocatable) -> Result, VirtualMachineError> { - self.segments - .memory - .get_integer(key) - .map_err(VirtualMachineError::Memory) + pub fn get_integer(&self, key: Relocatable) -> Result, MemoryError> { + self.segments.memory.get_integer(key) } ///Gets the relocatable value corresponding to the Relocatable address - pub fn get_relocatable(&self, key: Relocatable) -> Result { - self.segments - .memory - .get_relocatable(key) - .map_err(VirtualMachineError::Memory) + pub fn get_relocatable(&self, key: Relocatable) -> Result { + self.segments.memory.get_relocatable(key) } ///Gets a MaybeRelocatable value from memory indicated by a generic address @@ -824,7 +818,7 @@ impl VirtualMachine { &mut self, key: Relocatable, val: T, - ) -> Result<(), VirtualMachineError> { + ) -> Result<(), MemoryError> { self.segments.memory.insert_value(key, val) } @@ -882,7 +876,7 @@ impl VirtualMachine { &self, addr: Relocatable, size: usize, - ) -> Result>, VirtualMachineError> { + ) -> Result>, MemoryError> { self.segments.memory.get_integer_range(addr, size) } diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 06b399aa2d..e9f0e95500 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -1,7 +1,7 @@ use crate::{ types::relocatable::{MaybeRelocatable, Relocatable}, utils::from_relocatable_to_indexes, - vm::errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, + vm::errors::memory_errors::MemoryError, }; use felt::Felt; use num_traits::ToPrimitive; @@ -214,9 +214,8 @@ impl Memory { &mut self, key: Relocatable, val: T, - ) -> Result<(), VirtualMachineError> { + ) -> Result<(), MemoryError> { self.insert(&key, &val.into()) - .map_err(VirtualMachineError::Memory) } pub fn add_validation_rule(&mut self, segment_index: usize, rule: ValidationRule) { @@ -285,7 +284,7 @@ impl Memory { &self, addr: Relocatable, size: usize, - ) -> Result>, VirtualMachineError> { + ) -> Result>, MemoryError> { let mut values = Vec::new(); for i in 0..size { From 26d165d8a37348ca25c156daa276a913f5c1a950 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 18:06:59 -0300 Subject: [PATCH 16/18] Fix test --- src/vm/runners/builtin_runner/keccak.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/runners/builtin_runner/keccak.rs b/src/vm/runners/builtin_runner/keccak.rs index b6a60e21df..65e507c100 100644 --- a/src/vm/runners/builtin_runner/keccak.rs +++ b/src/vm/runners/builtin_runner/keccak.rs @@ -593,7 +593,7 @@ mod tests { #[test] fn deduce_memory_cell_expected_integer() { - let memory = memory![((0, 35), 0)]; + let memory = memory![((0, 0), (1, 2))]; let mut builtin = KeccakBuiltinRunner::new(&KeccakInstanceDef::default(), true); From bd42352229f20715abbe97e9af8c1ebd514cd99d Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 18:15:16 -0300 Subject: [PATCH 17/18] Use memory errors in memory segments methods --- src/vm/errors/memory_errors.rs | 2 ++ src/vm/vm_core.rs | 2 +- src/vm/vm_memory/memory_segments.rs | 8 ++++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/vm/errors/memory_errors.rs b/src/vm/errors/memory_errors.rs index d07abe73d8..13cc358c41 100644 --- a/src/vm/errors/memory_errors.rs +++ b/src/vm/errors/memory_errors.rs @@ -79,6 +79,8 @@ pub enum MemoryError { InsufficientAllocatedCells(#[from] InsufficientAllocatedCellsError), #[error("Accessed address {0} has higher offset than the maximal offset {1} encountered in the memory segment.")] AccessedAddressOffsetBiggerThanSegmentSize(Relocatable, usize), + #[error("gen_arg: found argument of invalid type.")] + GenArgInvalidType, // Memory.get() errors #[error("Expected integer at address {0}")] ExpectedInteger(Relocatable), diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index a6a8f80f12..db58c74ee1 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -950,7 +950,7 @@ impl VirtualMachine { self.segments.memory.add_relocation_rule(src_ptr, dst_ptr) } - pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { + pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { self.segments.gen_arg(arg) } diff --git a/src/vm/vm_memory/memory_segments.rs b/src/vm/vm_memory/memory_segments.rs index 3df938d021..72f9d0e8a3 100644 --- a/src/vm/vm_memory/memory_segments.rs +++ b/src/vm/vm_memory/memory_segments.rs @@ -114,7 +114,7 @@ impl MemorySegmentManager { Ok(relocation_table) } - pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { + pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { if let Some(value) = arg.downcast_ref::() { Ok(value.clone()) } else if let Some(value) = arg.downcast_ref::>() { @@ -126,7 +126,7 @@ impl MemorySegmentManager { self.write_arg(base, value)?; Ok(base.into()) } else { - Err(VirtualMachineError::NotImplemented) + Err(MemoryError::GenArgInvalidType) } } @@ -792,12 +792,12 @@ mod tests { /// Test that the call to .gen_arg() with any other argument returns a not /// implemented error. #[test] - fn gen_arg_not_implemented() { + fn gen_arg_invalid_type() { let mut memory_segment_manager = MemorySegmentManager::new(); assert_matches!( memory_segment_manager.gen_arg(&""), - Err(VirtualMachineError::NotImplemented) + Err(MemoryError::GenArgInvalidType) ); } From 90fff63a69b746951e71a30dd5288f68a808cf6c Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 17 Feb 2023 18:31:23 -0300 Subject: [PATCH 18/18] Clippy --- src/vm/vm_memory/memory.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index e9f0e95500..2b47909f45 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -189,10 +189,7 @@ impl Memory { //If the value is an MaybeRelocatable::Int(Bigint) return &Bigint //else raises Err pub fn get_integer(&self, key: Relocatable) -> Result, MemoryError> { - match self - .get(&key) - .ok_or_else(|| MemoryError::UnknownMemoryCell(key))? - { + match self.get(&key).ok_or(MemoryError::UnknownMemoryCell(key))? { Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)), Cow::Owned(MaybeRelocatable::Int(int)) => Ok(Cow::Owned(int)), _ => Err(MemoryError::ExpectedInteger(key)), @@ -200,10 +197,7 @@ impl Memory { } pub fn get_relocatable(&self, key: Relocatable) -> Result { - match self - .get(&key) - .ok_or_else(|| MemoryError::UnknownMemoryCell(key))? - { + match self.get(&key).ok_or(MemoryError::UnknownMemoryCell(key))? { Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel), Cow::Owned(MaybeRelocatable::RelocatableValue(rel)) => Ok(rel), _ => Err(MemoryError::ExpectedRelocatable(key)),