bugfix: Don't assume outer deref when fetching integer values from re…

…ferences (#1732) * Add test * Refactor how references are computes * Fix some tests * Fix tests * Add test * Clippy * Fix tests * Update old test * Add changelog entry * Remove misleading comment --------- Co-authored-by: Pedro Fontana <[email protected]>
lambdaclass · Apr 26, 2024 · 59840b3 · 59840b3
1 parent c5839fd
commit 59840b3
Show file tree

Hide file tree

Showing 11 changed files with 149 additions and 167 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,8 @@
   * Add `CairoPie` methods `run_validity_checks` & `check_pie_compatibility`
   * Add `Program` method `from_stripped_program`
 
+* bugfix: Don't assume outer deref when fetching integer values from references[#1732](https://github.com/lambdaclass/cairo-vm/pull/1732)
+
 * feat: Implement `extend_additional_data` for `BuiltinRunner`[#1726](https://github.com/lambdaclass/cairo-vm/pull/1726)
 
 * BREAKING: Set dynamic params as null by default on air public input [#1716](https://github.com/lambdaclass/cairo-vm/pull/1716)

diff --git a/vm/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/vm/src/hint_processor/builtin_hint_processor/blake2s_utils.rs
@@ -367,7 +367,7 @@ mod tests {
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
             Err(HintError::IdentifierNotRelocatable(bx))
-            if *bx == ("output".to_string(), (1,0).into())
+            if bx.as_ref() == "output"
         );
     }
 

diff --git a/vm/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs b/vm/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs
@@ -1003,7 +1003,7 @@ mod tests {
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
             Err(HintError::IdentifierNotInteger(bx))
-            if *bx == ("len".to_string(), (1,1).into())
+            if bx.as_ref() == "len"
         );
     }
 

diff --git a/vm/src/hint_processor/builtin_hint_processor/find_element_hint.rs b/vm/src/hint_processor/builtin_hint_processor/find_element_hint.rs
@@ -270,7 +270,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::IdentifierNotInteger(bx)) if *bx == ("key".to_string(), (1,4).into())
+            Err(HintError::UnknownIdentifier(bx)) if bx.as_ref() == "key"
         );
     }
 
@@ -283,7 +283,7 @@ mod tests {
         )]));
         assert_matches!(
             run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("elm_size".to_string(), (1,1).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "elm_size"
         );
     }
 
@@ -321,7 +321,7 @@ mod tests {
             init_vm_ids_data(HashMap::from([("n_elms".to_string(), relocatable)]));
         assert_matches!(
             run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("n_elms".to_string(), (1,2).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "n_elms"
         );
     }
 
@@ -364,7 +364,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::IdentifierNotInteger(bx)) if *bx == ("key".to_string(), (1,4).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "key"
         );
     }
 
@@ -403,7 +403,7 @@ mod tests {
         )]));
         assert_matches!(
             run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("elm_size".to_string(), (1,1).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "elm_size"
         );
     }
 
@@ -429,7 +429,7 @@ mod tests {
         )]));
         assert_matches!(
             run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("n_elms".to_string(), (1,2).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "n_elms"
         );
     }
 

diff --git a/vm/src/hint_processor/builtin_hint_processor/hint_utils.rs b/vm/src/hint_processor/builtin_hint_processor/hint_utils.rs
@@ -48,12 +48,10 @@ pub fn get_ptr_from_var_name(
     match get_ptr_from_reference(vm, reference, ap_tracking) {
         // Map internal errors into more descriptive variants
         Ok(val) => Ok(val),
-        Err(HintError::WrongIdentifierTypeInternal(var_addr)) => Err(
-            HintError::IdentifierNotRelocatable(Box::new((var_name.to_string(), *var_addr))),
-        ),
-        _ => Err(HintError::UnknownIdentifier(
-            var_name.to_string().into_boxed_str(),
+        Err(HintError::WrongIdentifierTypeInternal) => Err(HintError::IdentifierNotRelocatable(
+            Box::<str>::from(var_name),
         )),
+        _ => Err(HintError::UnknownIdentifier(Box::<str>::from(var_name))),
     }
 }
 
@@ -77,7 +75,7 @@ pub fn get_relocatable_from_var_name(
     ids_data
         .get(var_name)
         .and_then(|x| compute_addr_from_reference(x, vm, ap_tracking))
-        .ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string().into_boxed_str()))
+        .ok_or_else(|| HintError::UnknownIdentifier(Box::<str>::from(var_name)))
 }
 
 //Gets the value of a variable name.
@@ -93,12 +91,10 @@ pub fn get_integer_from_var_name(
     match get_integer_from_reference(vm, reference, ap_tracking) {
         // Map internal errors into more descriptive variants
         Ok(val) => Ok(val),
-        Err(HintError::WrongIdentifierTypeInternal(var_addr)) => Err(
-            HintError::IdentifierNotInteger(Box::new((var_name.to_string(), *var_addr))),
-        ),
-        _ => Err(HintError::UnknownIdentifier(
-            var_name.to_string().into_boxed_str(),
-        )),
+        Err(HintError::WrongIdentifierTypeInternal) => {
+            Err(HintError::IdentifierNotInteger(Box::<str>::from(var_name)))
+        }
+        _ => Err(HintError::UnknownIdentifier(Box::<str>::from(var_name))),
     }
 }
 
@@ -111,7 +107,7 @@ pub fn get_maybe_relocatable_from_var_name<'a>(
 ) -> Result<MaybeRelocatable, HintError> {
     let reference = get_reference_from_var_name(var_name, ids_data)?;
     get_maybe_relocatable_from_reference(vm, reference, ap_tracking)
-        .ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string().into_boxed_str()))
+        .ok_or_else(|| HintError::UnknownIdentifier(Box::<str>::from(var_name)))
 }
 
 pub fn get_reference_from_var_name<'a>(
@@ -120,7 +116,7 @@ pub fn get_reference_from_var_name<'a>(
 ) -> Result<&'a HintReference, HintError> {
     ids_data
         .get(var_name)
-        .ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string().into_boxed_str()))
+        .ok_or_else(|| HintError::UnknownIdentifier(Box::<str>::from(var_name)))
 }
 
 pub fn get_constant_from_var_name<'a>(
@@ -137,7 +133,6 @@ pub fn get_constant_from_var_name<'a>(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::stdlib::string::ToString;
 
     use crate::{
         hint_processor::hint_processor_definition::HintReference,
@@ -218,7 +213,7 @@ mod tests {
 
         assert_matches!(
             get_ptr_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
-            Err(HintError::IdentifierNotRelocatable(bx)) if *bx == ("value".to_string(), (1,0).into())
+            Err(HintError::IdentifierNotRelocatable(bx)) if bx.as_ref() == "value"
         );
     }
 
@@ -274,7 +269,7 @@ mod tests {
 
         assert_matches!(
             get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("value".to_string(), (1,0).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "value"
         );
     }
 }
diff --git a/vm/src/hint_processor/builtin_hint_processor/math_utils.rs b/vm/src/hint_processor/builtin_hint_processor/math_utils.rs
@@ -894,7 +894,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("a".to_string(), (1,4).into())
+            Err(HintError::UnknownIdentifier(bx)) if bx.as_ref() == "a"
         );
     }
 
@@ -912,7 +912,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("a".to_string(), (1,4).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "a"
         );
     }
 
@@ -1061,14 +1061,14 @@ mod tests {
         let hint_code = "from starkware.cairo.common.math_utils import assert_integer\nassert_integer(ids.a)\nassert 0 <= ids.a % PRIME < range_check_builtin.bound, f'a = {ids.a} is out of range.'";
         let mut vm = vm_with_range_check!();
         //Initialize fp
-        vm.run_context.fp = 4;
+        vm.run_context.fp = 1;
         //Insert ids into memory
         vm.segments = segments![((1, 0), (10, 10))];
         let ids_data = ids_data!["a"];
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("a".to_string(), (1,3).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "a"
         );
     }
 
@@ -1103,7 +1103,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("a".to_string(), (1,3).into())
+            Err(HintError::UnknownIdentifier(bx)) if bx.as_ref() == "a"
         );
     }
 
@@ -1156,7 +1156,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code::ASSERT_LE_FELT, &mut exec_scopes, &constants),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("a".to_string(), (1,0).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "a"
         );
     }
 
@@ -1182,7 +1182,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code::ASSERT_LE_FELT, &mut exec_scopes, &constants),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("b".to_string(), (1,1).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "b"
         );
     }
 
@@ -1412,7 +1412,7 @@ mod tests {
         let ids_data = ids_data!["value"];
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("value".to_string(), (1,4).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "value"
         );
     }
 
@@ -2234,7 +2234,7 @@ mod tests {
                     )
                 ])
             ),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("value".to_string(), (1,3).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "value"
         );
     }
 
@@ -2404,7 +2404,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("a".to_string(), (1,1).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "a"
         );
     }
 
@@ -2421,7 +2421,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("b".to_string(), (1,2).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "b"
         );
     }
 
@@ -2439,7 +2439,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("b".to_string(), (1,2).into())
+            Err(HintError::UnknownIdentifier(bx)) if bx.as_ref() == "b"
         );
     }
 

diff --git a/vm/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs b/vm/src/hint_processor/builtin_hint_processor/memcpy_hint_utils.rs
@@ -96,7 +96,7 @@ mod tests {
 
         assert_matches!(
             get_integer_from_var_name(var_name, &vm, &ids_data, &ApTracking::default()),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == (var_name.to_string(), (1,0).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == var_name
         );
     }
 }
diff --git a/vm/src/hint_processor/builtin_hint_processor/memset_utils.rs b/vm/src/hint_processor/builtin_hint_processor/memset_utils.rs
@@ -55,7 +55,6 @@ pub fn memset_step_loop(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::stdlib::string::ToString;
     use crate::types::relocatable::Relocatable;
     use crate::{
         any_box,
@@ -100,7 +99,7 @@ mod tests {
         let ids_data = ids_data!["n"];
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("n".to_string(), (1,1).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "n"
         );
     }
 

diff --git a/vm/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs b/vm/src/hint_processor/builtin_hint_processor/squash_dict_utils.rs
@@ -700,7 +700,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
-            Err(HintError::IdentifierNotInteger(bx)) if *bx == ("n_used_accesses".to_string(), (1,0).into())
+            Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "n_used_accesses"
         );
     }