From 766c9acddbf9a8146eaee537342c5ce2f1f20c20 Mon Sep 17 00:00:00 2001 From: gabrielbosio Date: Thu, 26 Mar 2026 18:18:34 -0300 Subject: [PATCH] Return MaybeRelocatable directly from Memory::get instead of Cow --- .../cairo_keccak/keccak_hints.rs | 11 ++-- .../builtin_hint_processor/dict_hint_utils.rs | 5 +- .../builtin_hint_processor/set.rs | 5 +- vm/src/tests/segment_arena_test.rs | 6 +- vm/src/utils.rs | 8 +-- vm/src/vm/runners/builtin_runner/ec_op.rs | 16 ++--- vm/src/vm/runners/builtin_runner/hash.rs | 9 ++- vm/src/vm/runners/builtin_runner/modulo.rs | 7 +- vm/src/vm/vm_core.rs | 57 +++++------------ vm/src/vm/vm_memory/memory.rs | 64 +++++++------------ vm/src/vm/vm_memory/memory_segments.rs | 18 +++--- 11 files changed, 76 insertions(+), 130 deletions(-) diff --git a/vm/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs b/vm/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs index 75bc1dc412..2f7b83165c 100644 --- a/vm/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs +++ b/vm/src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs @@ -16,7 +16,7 @@ use crate::{ Felt252, }; use num_traits::ToPrimitive; -use std::{borrow::Cow, collections::HashMap}; +use std::collections::HashMap; // Constants in package "starkware.cairo.common.cairo_keccak.keccak". const BYTES_IN_WORD: &str = "starkware.cairo.common.cairo_keccak.keccak.BYTES_IN_WORD"; @@ -335,18 +335,15 @@ pub(crate) fn cairo_keccak_finalize_v2( // Helper function to transform a vector of MaybeRelocatables into a vector // of u64. Raises error if there are None's or if MaybeRelocatables are not Bigints. pub(crate) fn maybe_reloc_vec_to_u64_array( - vec: &[Option>], + vec: &[Option], ) -> Result, HintError> { let array = vec .iter() .map(|n| match n { - Some(Cow::Owned(MaybeRelocatable::Int(ref num))) - | Some(Cow::Borrowed(MaybeRelocatable::Int(ref num))) => num + Some(MaybeRelocatable::Int(ref num)) => num .to_u64() .ok_or_else(|| MathError::Felt252ToU64Conversion(Box::new(*num)).into()), - _ => Err(VirtualMachineError::ExpectedIntAtRange(Box::new( - n.as_ref().map(|x| x.as_ref().to_owned()), - ))), + _ => Err(VirtualMachineError::ExpectedIntAtRange(Box::new(n.clone()))), }) .collect::, VirtualMachineError>>()?; diff --git a/vm/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs b/vm/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs index 0e4993bb5e..d28633f3a7 100644 --- a/vm/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs +++ b/vm/src/hint_processor/builtin_hint_processor/dict_hint_utils.rs @@ -351,9 +351,8 @@ mod tests { vm.segments .memory .get(&MaybeRelocatable::from((1, 1))) - .unwrap() - .as_ref(), - &MaybeRelocatable::from(12) + .unwrap(), + MaybeRelocatable::from(12) ); //Check that the tracker's current_ptr has moved accordingly check_dict_ptr!(&exec_scopes, 2, (2, 3)); diff --git a/vm/src/hint_processor/builtin_hint_processor/set.rs b/vm/src/hint_processor/builtin_hint_processor/set.rs index 9f3a3f9dd8..9ad289b14a 100644 --- a/vm/src/hint_processor/builtin_hint_processor/set.rs +++ b/vm/src/hint_processor/builtin_hint_processor/set.rs @@ -121,9 +121,8 @@ mod tests { vm.segments .memory .get(&MaybeRelocatable::from((1, 0))) - .unwrap() - .as_ref(), - &MaybeRelocatable::Int(Felt252::ZERO) + .unwrap(), + MaybeRelocatable::Int(Felt252::ZERO) ) } diff --git a/vm/src/tests/segment_arena_test.rs b/vm/src/tests/segment_arena_test.rs index 61257d4c56..1767bd68f9 100644 --- a/vm/src/tests/segment_arena_test.rs +++ b/vm/src/tests/segment_arena_test.rs @@ -1,7 +1,7 @@ // This test mirrors the test on cairo-lang for segment_arena // https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/starknet/builtins/segment_arena/segment_arena_test.py use crate::types::layout_name::LayoutName; -use std::{borrow::Cow, collections::HashMap, rc::Rc}; +use std::{collections::HashMap, rc::Rc}; use crate::any_box; use crate::cairo_run::{cairo_run, CairoRunConfig}; @@ -174,7 +174,7 @@ fn test_segment_arena() { Some(7), ] .into_iter() - .map(|val| val.map(|int| Cow::Owned(MaybeRelocatable::Int(Felt252::from(int))))) + .map(|val| val.map(|int| MaybeRelocatable::Int(Felt252::from(int)))) .collect(); assert_eq!(concat_segments_data, expected_concat_segment_data); @@ -200,7 +200,7 @@ fn test_segment_arena() { MaybeRelocatable::Int(Felt252::from(2)), ] .into_iter() - .map(|val| Some(Cow::Owned::(val))) + .map(|val| Some(val)) .collect(); assert_eq!(infos_data, expected_infos_data); diff --git a/vm/src/utils.rs b/vm/src/utils.rs index 247443ac31..45a5be25f0 100644 --- a/vm/src/utils.rs +++ b/vm/src/utils.rs @@ -197,14 +197,14 @@ 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().as_ref(), - &mayberelocatable!($sival, $offval) + $mem.get(&mayberelocatable!($si, $off)).unwrap(), + mayberelocatable!($sival, $offval) ) }; ($mem:expr, ($si:expr, $off:expr), $val:expr) => { assert_eq!( - $mem.get(&mayberelocatable!($si, $off)).unwrap().as_ref(), - &mayberelocatable!($val) + $mem.get(&mayberelocatable!($si, $off)).unwrap(), + mayberelocatable!($val) ) }; } diff --git a/vm/src/vm/runners/builtin_runner/ec_op.rs b/vm/src/vm/runners/builtin_runner/ec_op.rs index a719edc8d0..7655a6d0f7 100644 --- a/vm/src/vm/runners/builtin_runner/ec_op.rs +++ b/vm/src/vm/runners/builtin_runner/ec_op.rs @@ -125,15 +125,13 @@ impl EcOpBuiltinRunner { for i in 0..INPUT_CELLS_PER_EC_OP as usize { match memory.get(&(instance + i)?) { None => return Ok(None), - Some(addr) => { - input_cells.push(match addr.as_ref() { - MaybeRelocatable::Int(num) => *num, - _ => { - return Err(RunnerError::Memory(MemoryError::ExpectedInteger( - Box::new((instance + i)?), - ))) - } - }); + Some(MaybeRelocatable::Int(num)) => { + input_cells.push(num); + } + Some(_) => { + return Err(RunnerError::Memory(MemoryError::ExpectedInteger(Box::new( + (instance + i)?, + )))) } }; } diff --git a/vm/src/vm/runners/builtin_runner/hash.rs b/vm/src/vm/runners/builtin_runner/hash.rs index aa925f9072..caa4baaa60 100644 --- a/vm/src/vm/runners/builtin_runner/hash.rs +++ b/vm/src/vm/runners/builtin_runner/hash.rs @@ -78,10 +78,9 @@ impl HashBuiltinRunner { segment_index: address.segment_index, offset: address.offset - 2, })); - 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()), - ) { + if let (Some(MaybeRelocatable::Int(num_a)), Some(MaybeRelocatable::Int(num_b))) = + (num_a, num_b) + { if self.verified_addresses.borrow().len() <= address.offset { self.verified_addresses .borrow_mut() @@ -89,7 +88,7 @@ impl HashBuiltinRunner { } self.verified_addresses.borrow_mut()[address.offset] = true; //Compute pedersen Hash - let result = starknet_types_core::hash::Pedersen::hash(num_b, num_a); + let result = starknet_types_core::hash::Pedersen::hash(&num_b, &num_a); return Ok(Some(MaybeRelocatable::from(result))); } Ok(None) diff --git a/vm/src/vm/runners/builtin_runner/modulo.rs b/vm/src/vm/runners/builtin_runner/modulo.rs index 2a0c42726e..54c525a411 100644 --- a/vm/src/vm/runners/builtin_runner/modulo.rs +++ b/vm/src/vm/runners/builtin_runner/modulo.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::BTreeMap}; +use std::collections::BTreeMap; use crate::{ air_private_input::{ModInput, ModInputInstance, ModInputMemoryVars, PrivateInput}, @@ -283,7 +283,7 @@ impl ModBuiltinRunner { let mut value = BigUint::zero(); for i in 0..N_WORDS { let addr_i = (addr + i)?; - match memory.get(&addr_i).map(Cow::into_owned) { + match memory.get(&addr_i) { None => return Ok((words, None)), Some(MaybeRelocatable::RelocatableValue(_)) => { return Err(MemoryError::ExpectedInteger(Box::new(addr_i)).into()) @@ -411,8 +411,7 @@ impl ModBuiltinRunner { let addr = (offsets_ptr + i)?; let offset = memory .get(&((offsets_ptr + i)?)) - .ok_or_else(|| MemoryError::UnknownMemoryCell(Box::new(addr)))? - .into_owned(); + .ok_or_else(|| MemoryError::UnknownMemoryCell(Box::new(addr)))?; for copy_i in 0..n_copies { memory.insert_as_accessed((offsets_ptr + (3 * (index + copy_i) + i))?, &offset)?; } diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 8611d49a8b..5233a03fd8 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -731,15 +731,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(Cow::into_owned); + let dst_op = self.segments.memory.get(&dst_addr); let op0_addr = self.run_context.compute_op0_addr(instruction)?; - let op0_op = self.segments.memory.get(&op0_addr).map(Cow::into_owned); + let op0_op = self.segments.memory.get(&op0_addr); let op1_addr = self .run_context .compute_op1_addr(instruction, op0_op.as_ref())?; - let op1_op = self.segments.memory.get(&op1_addr).map(Cow::into_owned); + let op1_op = self.segments.memory.get(&op1_addr); let mut res: Option = None; @@ -885,7 +885,7 @@ impl VirtualMachine { None => return Ok(()), }; let current_value = match self.segments.memory.get(&addr) { - Some(value) => value.into_owned(), + Some(value) => value, None => return Ok(()), }; if value != current_value { @@ -1022,7 +1022,7 @@ impl VirtualMachine { where Relocatable: TryFrom<&'a K>, { - self.segments.memory.get(key).map(|x| x.into_owned()) + self.segments.memory.get(key) } /// Returns a reference to the vector with all builtins present in the virtual machine @@ -1106,11 +1106,7 @@ impl VirtualMachine { } ///Gets n elements from memory starting from addr (n being size) - pub fn get_range( - &'_ self, - addr: Relocatable, - size: usize, - ) -> Vec>> { + pub fn get_range(&self, addr: Relocatable, size: usize) -> Vec> { self.segments.memory.get_range(addr, size) } @@ -1256,8 +1252,8 @@ impl VirtualMachine { .memory .get(&Relocatable::from((segment_index as isize, i))) { - Some(val) => match val.as_ref() { - MaybeRelocatable::Int(num) => format!("{}", signed_felt(*num)), + Some(val) => match val { + MaybeRelocatable::Int(num) => format!("{}", signed_felt(num)), MaybeRelocatable::RelocatableValue(rel) => format!("{}", rel), }, _ => "".to_string(), @@ -3746,12 +3742,8 @@ mod tests { assert_eq!(vm.run_context.ap, 2); assert_eq!( - vm.segments - .memory - .get(&vm.run_context.get_ap()) - .unwrap() - .as_ref(), - &MaybeRelocatable::Int(Felt252::from(0x4)), + vm.segments.memory.get(&vm.run_context.get_ap()).unwrap(), + MaybeRelocatable::Int(Felt252::from(0x4)), ); let mut hint_processor = BuiltinHintProcessor::new_empty(); assert_matches!( @@ -3770,12 +3762,8 @@ mod tests { assert_eq!(vm.run_context.ap, 3); assert_eq!( - vm.segments - .memory - .get(&vm.run_context.get_ap()) - .unwrap() - .as_ref(), - &MaybeRelocatable::Int(Felt252::from(0x5)) + vm.segments.memory.get(&vm.run_context.get_ap()).unwrap(), + MaybeRelocatable::Int(Felt252::from(0x5)) ); let mut hint_processor = BuiltinHintProcessor::new_empty(); @@ -3795,12 +3783,8 @@ mod tests { assert_eq!(vm.run_context.ap, 4); assert_eq!( - vm.segments - .memory - .get(&vm.run_context.get_ap()) - .unwrap() - .as_ref(), - &MaybeRelocatable::Int(Felt252::from(0x14)), + vm.segments.memory.get(&vm.run_context.get_ap()).unwrap(), + MaybeRelocatable::Int(Felt252::from(0x14)), ); } @@ -4423,11 +4407,7 @@ mod tests { let value2 = MaybeRelocatable::from(Felt252::from(3_i32)); let value3 = MaybeRelocatable::from(Felt252::from(4_i32)); - let expected_vec = vec![ - Some(Cow::Borrowed(&value1)), - Some(Cow::Borrowed(&value2)), - Some(Cow::Borrowed(&value3)), - ]; + let expected_vec = vec![Some(value1), Some(value2), Some(value3)]; assert_eq!(vm.get_range(Relocatable::from((1, 0)), 3), expected_vec); } @@ -4440,12 +4420,7 @@ mod tests { let value2 = MaybeRelocatable::from(Felt252::from(3_i32)); let value3 = MaybeRelocatable::from(Felt252::from(4_i32)); - let expected_vec = vec![ - Some(Cow::Borrowed(&value1)), - Some(Cow::Borrowed(&value2)), - None, - Some(Cow::Borrowed(&value3)), - ]; + let expected_vec = vec![Some(value1), Some(value2), None, Some(value3)]; assert_eq!(vm.get_range(Relocatable::from((1, 0)), 4), expected_vec); } diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 58898fa5e8..8216808e86 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -260,7 +260,7 @@ 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) -> Option> + pub(crate) fn get<'a, K: 'a>(&self, key: &'a K) -> Option where Relocatable: TryFrom<&'a K>, { @@ -269,7 +269,10 @@ impl Memory { .get_segment_cells(relocatable.segment_index)? .get(relocatable.offset)? .get_value()?; - Some(Cow::Owned(self.relocate_value(&value).ok()?.into_owned())) + if self.relocation_rules.is_empty() { + return Some(value); + } + Some(self.relocate_value(&value).ok()?.into_owned()) } // Version of Memory.relocate_value() that doesn't require a self reference @@ -513,13 +516,12 @@ impl Memory { /// Gets the value from memory address as a Felt252 value. /// Returns an Error if the value at the memory address is missing or not a Felt252. - pub fn get_integer(&'_ self, key: Relocatable) -> Result, MemoryError> { + pub fn get_integer(&self, key: Relocatable) -> Result, MemoryError> { match self .get(&key) .ok_or_else(|| MemoryError::UnknownMemoryCell(Box::new(key)))? { - Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)), - Cow::Owned(MaybeRelocatable::Int(int)) => Ok(Cow::Owned(int)), + MaybeRelocatable::Int(int) => Ok(Cow::Owned(int)), _ => Err(MemoryError::ExpectedInteger(Box::new(key))), } } @@ -547,8 +549,7 @@ impl Memory { .get(&key) .ok_or_else(|| MemoryError::UnknownMemoryCell(Box::new(key)))? { - Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel), - Cow::Owned(MaybeRelocatable::RelocatableValue(rel)) => Ok(rel), + MaybeRelocatable::RelocatableValue(rel) => Ok(rel), _ => Err(MemoryError::ExpectedRelocatable(Box::new(key))), } } @@ -556,14 +557,8 @@ impl Memory { /// Gets the value from memory address as a MaybeRelocatable value. /// Returns an Error if the value at the memory address is missing or not a MaybeRelocatable. pub fn get_maybe_relocatable(&self, key: Relocatable) -> Result { - match self - .get(&key) - .ok_or_else(|| MemoryError::UnknownMemoryCell(Box::new(key)))? - { - // Note: the `Borrowed` variant will never occur. - Cow::Borrowed(maybe_rel) => Ok(maybe_rel.clone()), - Cow::Owned(maybe_rel) => Ok(maybe_rel), - } + self.get(&key) + .ok_or_else(|| MemoryError::UnknownMemoryCell(Box::new(key))) } /// Inserts a value into memory @@ -705,11 +700,7 @@ impl Memory { /// Gets a range of memory values from addr to addr + size /// The outputed range may contain gaps if the original memory has them - pub fn get_range( - &'_ self, - addr: Relocatable, - size: usize, - ) -> Vec>> { + pub fn get_range(&self, addr: Relocatable, size: usize) -> Vec> { let mut values = Vec::new(); for i in 0..size { @@ -730,7 +721,7 @@ impl Memory { for i in 0..size { values.push(match self.get(&(addr + i)?) { - Some(elem) => elem.into_owned(), + Some(elem) => elem, None => return Err(MemoryError::GetRangeMemoryGap(Box::new((addr, size)))), }); } @@ -969,8 +960,8 @@ mod memory_tests { memory.data.push(Vec::new()); memory.insert(key, &val).unwrap(); assert_eq!( - memory.get(&key).unwrap().as_ref(), - &MaybeRelocatable::from(Felt252::from(5_u64)) + memory.get(&key).unwrap(), + MaybeRelocatable::from(Felt252::from(5_u64)) ); } @@ -983,8 +974,8 @@ mod memory_tests { MemoryCell::new(mayberelocatable!(8)), ]]; assert_eq!( - memory.get(&mayberelocatable!(-1, 2)).unwrap().as_ref(), - &mayberelocatable!(8), + memory.get(&mayberelocatable!(-1, 2)).unwrap(), + mayberelocatable!(8), ); } @@ -1009,8 +1000,8 @@ mod memory_tests { memory.temp_data.push(Vec::new()); memory.insert(key, &val).unwrap(); assert_eq!( - memory.get(&key).unwrap().as_ref(), - &MaybeRelocatable::from(Felt252::from(5_u64)), + memory.get(&key).unwrap(), + MaybeRelocatable::from(Felt252::from(5_u64)), ); } @@ -1086,7 +1077,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().as_ref(), &val); + assert_eq!(memory.get(&key_b).unwrap(), val); } #[test] @@ -1098,7 +1089,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().as_ref(), &val); + assert_eq!(memory.get(&key_b).unwrap(), 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); @@ -1332,7 +1323,7 @@ mod memory_tests { let val = MaybeRelocatable::from(Felt252::from(5)); memory.insert(key, &val).unwrap(); - assert_eq!(memory.get(&key).unwrap().as_ref(), &val); + assert_eq!(memory.get(&key).unwrap(), val); } #[test] @@ -1471,11 +1462,7 @@ mod memory_tests { let value2 = MaybeRelocatable::from(Felt252::from(3)); let value3 = MaybeRelocatable::from(Felt252::from(4)); - let expected_vec = vec![ - Some(Cow::Borrowed(&value1)), - Some(Cow::Borrowed(&value2)), - Some(Cow::Borrowed(&value3)), - ]; + let expected_vec = vec![Some(value1), Some(value2), Some(value3)]; assert_eq!(memory.get_range(Relocatable::from((1, 0)), 3), expected_vec); } @@ -1487,12 +1474,7 @@ mod memory_tests { let value2 = MaybeRelocatable::from(Felt252::from(3)); let value3 = MaybeRelocatable::from(Felt252::from(4)); - let expected_vec = vec![ - Some(Cow::Borrowed(&value1)), - Some(Cow::Borrowed(&value2)), - None, - Some(Cow::Borrowed(&value3)), - ]; + let expected_vec = vec![Some(value1), Some(value2), None, Some(value3)]; assert_eq!(memory.get_range(Relocatable::from((1, 0)), 4), expected_vec); } diff --git a/vm/src/vm/vm_memory/memory_segments.rs b/vm/src/vm/vm_memory/memory_segments.rs index 85aecb3849..aaecb21ee2 100644 --- a/vm/src/vm/vm_memory/memory_segments.rs +++ b/vm/src/vm/vm_memory/memory_segments.rs @@ -428,8 +428,8 @@ mod tests { let current_ptr = segments.load_data(ptr, &data).unwrap(); assert_eq!(current_ptr, Relocatable::from((0, 1))); assert_eq!( - segments.memory.get(&ptr).unwrap().as_ref(), - &MaybeRelocatable::from(Felt252::from(4)) + segments.memory.get(&ptr).unwrap(), + MaybeRelocatable::from(Felt252::from(4)) ); } @@ -447,24 +447,22 @@ mod tests { assert_eq!(current_ptr, Relocatable::from((0, 3))); assert_eq!( - segments.memory.get(&ptr).unwrap().as_ref(), - &MaybeRelocatable::from(Felt252::from(4)) + segments.memory.get(&ptr).unwrap(), + MaybeRelocatable::from(Felt252::from(4)) ); assert_eq!( segments .memory .get(&MaybeRelocatable::from((0, 1))) - .unwrap() - .as_ref(), - &MaybeRelocatable::from(Felt252::from(5)) + .unwrap(), + MaybeRelocatable::from(Felt252::from(5)) ); assert_eq!( segments .memory .get(&MaybeRelocatable::from((0, 2))) - .unwrap() - .as_ref(), - &MaybeRelocatable::from(Felt252::from(6)) + .unwrap(), + MaybeRelocatable::from(Felt252::from(6)) ); } #[test]