Merge branch 'main' of github.com:lambdaclass/cairo-rs into math-error

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

@@ -143,12 +143,10 @@ pub fn block_permutation(
     let keccak_ptr = get_ptr_from_var_name("keccak_ptr", vm, ids_data, ap_tracking)?;
 
     let keccak_state_size_felts = keccak_state_size_felts.to_usize().unwrap();
-    let values = vm
-        .get_range(
-            (keccak_ptr - keccak_state_size_felts)?,
-            keccak_state_size_felts,
-        )
-        .map_err(HintError::Memory)?;
+    let values = vm.get_range(
+        (keccak_ptr - keccak_state_size_felts)?,
+        keccak_state_size_felts,
+    );
 
     let mut u64_values = maybe_reloc_vec_to_u64_array(&values)?
         .try_into()

src/hint_processor/builtin_hint_processor/set.rs

@@ -7,10 +7,7 @@ use crate::{
     },
     serde::deserialize_program::ApTracking,
     types::{errors::math_errors::MathError, 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 num_traits::{One, ToPrimitive, Zero};
@@ -35,9 +32,7 @@ pub fn set_add(
             "assert ids.elm_size > 0",
         )))?;
     }
-    let elm = vm
-        .get_range(elm_ptr, elm_size)
-        .map_err(VirtualMachineError::Memory)?;
+    let elm = vm.get_range(elm_ptr, elm_size);
 
     if set_ptr > set_end_ptr {
         return Err(HintError::InvalidSetRange(
@@ -49,9 +44,7 @@ pub fn set_add(
     let range_limit = (set_end_ptr - set_ptr)?;
 
     for i in (0..range_limit).step_by(elm_size) {
-        let set_iter = vm
-            .get_range((set_ptr + i)?, elm_size)
-            .map_err(VirtualMachineError::Memory)?;
+        let set_iter = vm.get_range((set_ptr + i)?, elm_size);
 
         if set_iter == elm {
             insert_value_from_var_name(

src/types/relocatable.rs

@@ -284,15 +284,15 @@ impl MaybeRelocatable {
         }
     }
 
-    //Returns reference to Felt inside self if Int variant or Error if RelocatableValue variant
+    /// Returns a reference to the inner value if it is a Felt, returns None otherwise.
     pub fn get_int_ref(&self) -> Option<&Felt> {
         match self {
             MaybeRelocatable::Int(num) => Some(num),
             MaybeRelocatable::RelocatableValue(_) => None,
         }
     }
 
-    //Returns reference to Relocatable inside self if Relocatable variant or Error if Int variant
+    /// Returns the inner value if it is a Relocatable, returns None otherwise.
     pub fn get_relocatable(&self) -> Option<Relocatable> {
         match self {
             MaybeRelocatable::RelocatableValue(rel) => Some(*rel),
@@ -314,7 +314,7 @@ impl<'a> Add<usize> for &'a Relocatable {
 
 /// Turns a MaybeRelocatable into a Felt value.
 /// If the value is an Int, it will extract the Felt value from it.
-/// If the value is Relocatable, it will return an error since it should've already been relocated.
+/// If the value is RelocatableValue, it will relocate it according to the relocation_table
 pub fn relocate_value(
     value: MaybeRelocatable,
     relocation_table: &Vec<usize>,
@@ -327,6 +327,7 @@ pub fn relocate_value(
     }
 }
 
+// Relocates a Relocatable value according to the relocation_table
 pub fn relocate_address(
     relocatable: Relocatable,
     relocation_table: &Vec<usize>,

src/vm/vm_core.rs

@@ -764,7 +764,7 @@ impl VirtualMachine {
         entries
     }
 
-    ///Adds a new segment and to the VirtualMachine.memory returns its starting location as a RelocatableValue.
+    ///Adds a new segment and to the memory and returns its starting location as a Relocatable value.
     pub fn add_memory_segment(&mut self) -> Relocatable {
         self.segments.add()
     }
@@ -804,6 +804,7 @@ impl VirtualMachine {
         &self.builtin_runners
     }
 
+    /// Returns a mutable reference to the vector with all builtins present in the virtual machine
     pub fn get_builtin_runners_as_mut(&mut self) -> &mut Vec<(&'static str, BuiltinRunner)> {
         &mut self.builtin_runners
     }
@@ -817,7 +818,7 @@ impl VirtualMachine {
         self.segments.memory.insert_value(key, val)
     }
 
-    ///Writes data into the memory at address ptr and returns the first address after the data.
+    ///Writes data into the memory from address ptr and returns the first address after the data.
     pub fn load_data(
         &mut self,
         ptr: Relocatable,
@@ -826,8 +827,7 @@ impl VirtualMachine {
         self.segments.load_data(ptr, data)
     }
 
-    /// Writes args into the memory at address ptr and returns the first address after the data.
-    /// Perfroms modulo on each element
+    /// Writes args into the memory from address ptr and returns the first address after the data.
     pub fn write_arg(
         &mut self,
         ptr: Relocatable,
@@ -844,11 +844,7 @@ impl VirtualMachine {
     }
 
     ///Gets n elements from memory starting from addr (n being size)
-    pub fn get_range(
-        &self,
-        addr: Relocatable,
-        size: usize,
-    ) -> Result<Vec<Option<Cow<MaybeRelocatable>>>, MemoryError> {
+    pub fn get_range(&self, addr: Relocatable, size: usize) -> Vec<Option<Cow<MaybeRelocatable>>> {
         self.segments.memory.get_range(addr, size)
     }
 
@@ -944,8 +940,7 @@ impl VirtualMachine {
         self.segments.gen_arg(arg)
     }
 
-    /// Proxy to MemorySegmentManager::compute_effective_sizes() to make it accessible from outside
-    /// cairo-rs.
+    /// Calls MemorySegmentManager::compute_effective_sizes()
     pub fn compute_effective_sizes(&mut self) -> &Vec<usize> {
         self.segments.compute_effective_sizes()
     }
@@ -3715,7 +3710,7 @@ mod tests {
             Some(Cow::Borrowed(&value2)),
             Some(Cow::Borrowed(&value3)),
         ];
-        assert_eq!(vm.get_range(Relocatable::from((1, 0)), 3), Ok(expected_vec));
+        assert_eq!(vm.get_range(Relocatable::from((1, 0)), 3), expected_vec);
     }
 
     #[test]
@@ -3733,7 +3728,7 @@ mod tests {
             None,
             Some(Cow::Borrowed(&value3)),
         ];
-        assert_eq!(vm.get_range(Relocatable::from((1, 0)), 4), Ok(expected_vec));
+        assert_eq!(vm.get_range(Relocatable::from((1, 0)), 4), expected_vec);
     }
 
     #[test]

src/vm/vm_memory/memory.rs

@@ -60,9 +60,10 @@ impl Memory {
             validation_rules: HashMap::new(),
         }
     }
-    ///Inserts an MaybeRelocatable value into an address given by a MaybeRelocatable::Relocatable
-    /// Will panic if the segment index given by the address corresponds to a non-allocated segment
-    /// If the address isnt contiguous with previously inserted data, memory gaps will be represented by inserting None values
+    /// Inserts a value into a memory address
+    /// Will return an Error if the segment index given by the address corresponds to a non-allocated segment,
+    /// or if the inserted value is inconsistent with the current value at the memory cell
+    /// If the address isnt contiguous with previously inserted data, memory gaps will be represented by None values
     pub fn insert<'a, K: 'a, V: 'a>(&mut self, key: &'a K, val: &'a V) -> Result<(), MemoryError>
     where
         Relocatable: TryFrom<&'a K>,
@@ -211,9 +212,8 @@ impl Memory {
         Ok(())
     }
 
-    //Gets the value from memory address.
-    //If the value is an MaybeRelocatable::Int(Bigint) return &Bigint
-    //else raises Err
+    /// Gets the value from memory address as a Felt value.
+    /// Returns an Error if the value at the memory address is missing or not a Felt.
     pub fn get_integer(&self, key: Relocatable) -> Result<Cow<Felt>, MemoryError> {
         match self.get(&key).ok_or(MemoryError::UnknownMemoryCell(key))? {
             Cow::Borrowed(MaybeRelocatable::Int(int)) => Ok(Cow::Borrowed(int)),
@@ -222,6 +222,8 @@ impl Memory {
         }
     }
 
+    /// Gets the value from memory address as a Relocatable value.
+    /// Returns an Error if the value at the memory address is missing or not a Relocatable.
     pub fn get_relocatable(&self, key: Relocatable) -> Result<Relocatable, MemoryError> {
         match self.get(&key).ok_or(MemoryError::UnknownMemoryCell(key))? {
             Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel)) => Ok(*rel),
@@ -230,6 +232,8 @@ impl Memory {
         }
     }
 
+    /// Inserts a value into memory
+    /// Returns an error if the memory cell asignment is invalid
     pub fn insert_value<T: Into<MaybeRelocatable>>(
         &mut self,
         key: Relocatable,
@@ -254,6 +258,7 @@ impl Memory {
         }
         Ok(())
     }
+
     ///Applies validation_rules to the current memory
     pub fn validate_existing_memory(&mut self) -> Result<(), MemoryError> {
         for (index, rule) in &self.validation_rules {
@@ -269,20 +274,20 @@ impl Memory {
         Ok(())
     }
 
-    pub fn get_range(
-        &self,
-        addr: Relocatable,
-        size: usize,
-    ) -> Result<Vec<Option<Cow<MaybeRelocatable>>>, MemoryError> {
+    /// 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<Option<Cow<MaybeRelocatable>>> {
         let mut values = Vec::new();
 
         for i in 0..size {
-            values.push(self.get(&(addr + i)?));
+            values.push((addr + i).ok().and_then(|x| self.get(&x)));
         }
 
-        Ok(values)
+        values
     }
 
+    /// Gets a range of memory values from addr to addr + size
+    /// Fails if there if any of the values inside the range is missing (memory gap)
     pub fn get_continuous_range(
         &self,
         addr: Relocatable,
@@ -300,6 +305,9 @@ impl Memory {
         Ok(values)
     }
 
+    /// Gets a range of Felt memory values from addr to addr + size
+    /// Fails if there if any of the values inside the range is missing (memory gap),
+    /// or is not a Felt
     pub fn get_integer_range(
         &self,
         addr: Relocatable,
@@ -370,6 +378,7 @@ impl Display for Memory {
     }
 }
 
+/// Applies `relocation_rules` to a value
 pub(crate) trait RelocateValue<'a, Input: 'a, Output: 'a> {
     fn relocate_value(&self, value: Input) -> Output;
 }
@@ -935,10 +944,7 @@ mod memory_tests {
             Some(Cow::Borrowed(&value2)),
             Some(Cow::Borrowed(&value3)),
         ];
-        assert_eq!(
-            memory.get_range(Relocatable::from((1, 0)), 3),
-            Ok(expected_vec)
-        );
+        assert_eq!(memory.get_range(Relocatable::from((1, 0)), 3), expected_vec);
     }
 
     #[test]
@@ -955,10 +961,7 @@ mod memory_tests {
             None,
             Some(Cow::Borrowed(&value3)),
         ];
-        assert_eq!(
-            memory.get_range(Relocatable::from((1, 0)), 4),
-            Ok(expected_vec)
-        );
+        assert_eq!(memory.get_range(Relocatable::from((1, 0)), 4), expected_vec);
     }
 
     #[test]

src/vm/vm_memory/memory_segments.rs

@@ -28,7 +28,7 @@ impl MemorySegmentManager {
         self.memory.temp_data.len()
     }
 
-    ///Adds a new segment and returns its starting location as a RelocatableValue.
+    ///Adds a new segment and returns its starting location as a Relocatable value. Its segment index will always be positive.
     pub fn add(&mut self) -> Relocatable {
         self.memory.data.push(Vec::new());
         Relocatable {
@@ -37,8 +37,7 @@ impl MemorySegmentManager {
         }
     }
 
-    ///Adds a new temporary segment and returns its starting location as a RelocatableValue.
-    ///Negative segment_index indicates its refer to a temporary segment
+    /// Adds a new temporary segment and returns its starting location as a Relocatable value. Its segment index will always be negative.
     pub fn add_temporary_segment(&mut self) -> Relocatable {
         self.memory.temp_data.push(Vec::new());
         Relocatable {
@@ -48,7 +47,7 @@ impl MemorySegmentManager {
         }
     }
 
-    ///Writes data into the memory at address ptr and returns the first address after the data.
+    ///Writes data into the memory from address ptr and returns the first address after the data.
     pub fn load_data(
         &mut self,
         ptr: Relocatable,
@@ -75,7 +74,7 @@ impl MemorySegmentManager {
             .get_or_insert_with(|| self.memory.data.iter().map(Vec::len).collect())
     }
 
-    ///Returns the number of used segments when they are already computed.
+    ///Returns the number of used segments if they have been computed.
     ///Returns None otherwise.
     pub fn get_segment_used_size(&self, index: usize) -> Option<usize> {
         self.segment_used_sizes.as_ref()?.get(index).copied()
@@ -88,7 +87,7 @@ impl MemorySegmentManager {
             .or_else(|| self.get_segment_used_size(index))
     }
 
-    ///Returns a vector that contains the first relocated address of each memory segment
+    ///Returns a vector containing the first relocated address of each memory segment
     pub fn relocate_segments(&self) -> Result<Vec<usize>, MemoryError> {
         let first_addr = 1;
         let mut relocation_table = vec![first_addr];