Fix cairo1 Felt252DictEntryUpdate hint (#1624)

* change felt for maybe relocatable

* add changelog and improve code

* Add unit test

* cippy

---------

Co-authored-by: Pedro Fontana <[email protected]>

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 #### Upcoming Changes
 
+* fix: make Felt252DictEntryUpdate work with MaybeRelocatable instead of only Felt [#1624](https://github.com/lambdaclass/cairo-vm/pull/1624).
+
 * chore: bump `cairo-lang-` dependencies to 2.5.3 [#1596](https://github.com/lambdaclass/cairo-vm/pull/1596)
 
 * refactor: Refactor `cairo1-run` crate [#1601](https://github.com/lambdaclass/cairo-vm/pull/1601)

cairo1-run/src/main.rs

@@ -554,4 +554,12 @@ mod tests {
         let args = args.iter().cloned().map(String::from);
         assert_matches!(run(args), Ok(Some(res)) if res == "1");
     }
+
+    #[rstest]
+    #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/nullable_dict.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--cairo_pie_output", "/dev/null"].as_slice())]
+    #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/nullable_dict.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--proof_mode", "--air_public_input", "/dev/null", "--air_private_input", "/dev/null"].as_slice())]
+    fn test_run_nullable_dict_ok(#[case] args: &[&str]) {
+        let args = args.iter().cloned().map(String::from);
+        assert_matches!(run(args), Ok(Some(res)) if res.is_empty());
+    }
 }

cairo_programs/cairo-1-programs/nullable_dict.cairo

@@ -0,0 +1,12 @@
+use core::nullable::{nullable_from_box, match_nullable, FromNullableResult};
+
+fn main() {
+    // Create the dictionary
+    let mut d: Felt252Dict<Nullable<Span<felt252>>> = Default::default();
+
+    // Create the array to insert
+    let a = array![8, 9, 10];
+
+    // Insert it as a `Span`
+    d.insert(0, nullable_from_box(BoxTrait::new(a.span())));
+}

vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs

@@ -3,14 +3,15 @@ use num_traits::One;
 use crate::stdlib::collections::HashMap;
 use crate::stdlib::prelude::*;
 
+use crate::types::relocatable::MaybeRelocatable;
 use crate::vm::errors::hint_errors::HintError;
 use crate::Felt252;
 use crate::{types::relocatable::Relocatable, vm::vm_core::VirtualMachine};
 
 /// Stores the data of a specific dictionary.
 pub struct DictTrackerExecScope {
     /// The data of the dictionary.
-    data: HashMap<Felt252, Felt252>,
+    data: HashMap<Felt252, MaybeRelocatable>,
     /// The index of the dictionary in the dict_infos segment.
     #[allow(dead_code)]
     idx: usize,
@@ -80,13 +81,22 @@ impl DictManagerExecScope {
     }
 
     /// Inserts a value to the dict tracker corresponding to a given pointer to a dict segment.
-    pub fn insert_to_tracker(&mut self, dict_end: Relocatable, key: Felt252, value: Felt252) {
+    pub fn insert_to_tracker(
+        &mut self,
+        dict_end: Relocatable,
+        key: Felt252,
+        value: MaybeRelocatable,
+    ) {
         self.get_dict_tracker_mut(dict_end).data.insert(key, value);
     }
 
     /// Gets a value from the dict tracker corresponding to a given pointer to a dict segment.
     /// None if the key does not exist in the tracker data.
-    pub fn get_from_tracker(&self, dict_end: Relocatable, key: &Felt252) -> Option<Felt252> {
+    pub fn get_from_tracker(
+        &self,
+        dict_end: Relocatable,
+        key: &Felt252,
+    ) -> Option<MaybeRelocatable> {
         self.get_dict_tracker(dict_end).ok()?.data.get(key).cloned()
     }
 }

vm/src/hint_processor/cairo_1_hint_processor/hint_processor.rs

@@ -4,7 +4,7 @@ use crate::any_box;
 use crate::hint_processor::cairo_1_hint_processor::dict_manager::DictSquashExecScope;
 use crate::hint_processor::hint_processor_definition::HintReference;
 use crate::stdlib::{boxed::Box, collections::HashMap, prelude::*};
-use crate::types::relocatable::Relocatable;
+use crate::types::relocatable::{MaybeRelocatable, Relocatable};
 use crate::vm::runners::cairo_runner::ResourceTracker;
 use crate::vm::runners::cairo_runner::RunResources;
 use crate::Felt252;
@@ -588,7 +588,11 @@ impl Cairo1HintProcessor {
         let (dict_base, dict_offset) = extract_buffer(dict_ptr)?;
         let dict_address = get_ptr(vm, dict_base, &dict_offset)?;
         let key = res_operand_get_val(vm, key)?;
-        let value = res_operand_get_val(vm, value)?;
+        let value = if let ResOperand::Deref(cell) = value {
+            get_mayberelocatable(vm, cell)?
+        } else {
+            MaybeRelocatable::Int(res_operand_get_val(vm, value)?)
+        };
         let dict_manager_exec_scope =
             exec_scopes.get_mut_ref::<DictManagerExecScope>("dict_manager_exec_scope")?;
 
@@ -829,7 +833,11 @@ impl Cairo1HintProcessor {
         let (dict_base, dict_offset) = extract_buffer(dict_ptr)?;
         let dict_address = get_ptr(vm, dict_base, &dict_offset)?;
         let key = get_double_deref_val(vm, dict_base, &(dict_offset + Felt252::from(-3)))?;
-        let value = res_operand_get_val(vm, value)?;
+        let value: MaybeRelocatable = if let ResOperand::Deref(cell) = value {
+            get_mayberelocatable(vm, cell)?
+        } else {
+            MaybeRelocatable::Int(res_operand_get_val(vm, value)?)
+        };
         let dict_manager_exec_scope = exec_scopes
             .get_mut_ref::<DictManagerExecScope>("dict_manager_exec_scope")
             .map_err(|_| {

vm/src/hint_processor/cairo_1_hint_processor/hint_processor_utils.rs

@@ -1,4 +1,5 @@
 use crate::stdlib::prelude::*;
+use crate::types::relocatable::MaybeRelocatable;
 use crate::types::{errors::math_errors::MathError, relocatable::Relocatable};
 use crate::vm::errors::{hint_errors::HintError, vm_errors::VirtualMachineError};
 use crate::vm::vm_core::VirtualMachine;
@@ -29,6 +30,16 @@ pub(crate) fn extract_buffer(buffer: &ResOperand) -> Result<(&CellRef, Felt252),
     Ok((cell, base_offset))
 }
 
+pub(crate) fn get_mayberelocatable(
+    vm: &VirtualMachine,
+    cell: &CellRef,
+) -> Result<MaybeRelocatable, VirtualMachineError> {
+    let relocatable = cell_ref_to_relocatable(cell, vm)?;
+    vm.get_maybe(&relocatable).ok_or_else(|| {
+        VirtualMachineError::InvalidMemoryValueTemporaryAddress(Box::new(relocatable))
+    })
+}
+
 /// Fetches the value of `res_operand` from the vm.
 pub(crate) fn get_val(
     vm: &VirtualMachine,