fix: Handle triple dereference references (#1708)

* Add outer & inner dereference * Update tests * Add handling to Reference + test * Handle edge case * Remove dbg commands * Fix code * Revert "Fix code" This reverts commit ce7c07e. * Fix handling * fmt * Add triple dereference case to docs * Add changelog entry --------- Co-authored-by: Pedro Fontana <[email protected]>
lambdaclass · Apr 17, 2024 · b05b541 · b05b541
1 parent 584eaa2
commit b05b541
Show file tree

Hide file tree

Showing 8 changed files with 138 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 #### Upcoming Changes
 
+* fix(BREAKING): Handle triple dereference references[#1708](https://github.com/lambdaclass/cairo-vm/pull/1708)
+  * Replace `ValueAddress` boolean field `dereference` with boolean fields `outer_dereference` & `inner_dereference`
+  * Replace `HintReference` boolean field `dereference` with boolean fields `outer_dereference` & `inner_dereference`
+  * Reference parsing now handles the case of dereferences inside the cast. Aka references of type `cast([A + B], type)` such as `cast([[fp + 2] + 2], felt)`.
+
 * Bump `starknet-types-core` version + Use the lib's pedersen hash [#1692](https://github.com/lambdaclass/cairo-vm/pull/1692)
 
 * refactor: Remove unused code & use constants whenever possible for builtin instance definitions[#1707](https://github.com/lambdaclass/cairo-vm/pull/1707)

diff --git a/docs/references_parsing/README.md b/docs/references_parsing/README.md
@@ -63,6 +63,7 @@ There are some other, more rare cases of reference values found when implementin
 
 * ```cast(number, felt)```
 * ```[cast(reg + offset1 + offset2, type)]```
+* ```[cast([[reg + offset1] + offset2], type)]```
 
 ## To do
 For the moment the type of the reference is not being used, this will be included in the future to make the hints code cleaner.

diff --git a/vm/src/hint_processor/hint_processor_definition.rs b/vm/src/hint_processor/hint_processor_definition.rs
@@ -103,7 +103,8 @@ fn get_ids_data(
 pub struct HintReference {
     pub offset1: OffsetValue,
     pub offset2: OffsetValue,
-    pub dereference: bool,
+    pub inner_dereference: bool,
+    pub outer_dereference: bool,
     pub ap_tracking_data: Option<ApTracking>,
     pub cairo_type: Option<String>,
 }
@@ -114,7 +115,8 @@ impl HintReference {
             offset1: OffsetValue::Reference(Register::FP, offset1, false),
             offset2: OffsetValue::Value(0),
             ap_tracking_data: None,
-            dereference: true,
+            outer_dereference: true,
+            inner_dereference: false,
             cairo_type: None,
         }
     }
@@ -124,7 +126,8 @@ impl HintReference {
             offset1: OffsetValue::Reference(Register::FP, offset1, inner_dereference),
             offset2: OffsetValue::Value(offset2),
             ap_tracking_data: None,
-            dereference,
+            outer_dereference: dereference,
+            inner_dereference: false,
             cairo_type: None,
         }
     }
@@ -135,7 +138,8 @@ impl From<Reference> for HintReference {
         HintReference {
             offset1: reference.value_address.offset1.clone(),
             offset2: reference.value_address.offset2.clone(),
-            dereference: reference.value_address.dereference,
+            outer_dereference: reference.value_address.outer_dereference,
+            inner_dereference: reference.value_address.inner_dereference,
             // only store `ap` tracking data if the reference is referred to it
             ap_tracking_data: match (
                 &reference.value_address.offset1,

diff --git a/vm/src/hint_processor/hint_processor_utils.rs b/vm/src/hint_processor/hint_processor_utils.rs
@@ -59,7 +59,7 @@ pub fn get_ptr_from_reference(
 ) -> Result<Relocatable, HintError> {
     let var_addr = compute_addr_from_reference(hint_reference, vm, ap_tracking)
         .ok_or(HintError::UnknownIdentifierInternal)?;
-    if hint_reference.dereference {
+    if hint_reference.outer_dereference {
         vm.get_relocatable(var_addr)
             .map_err(|_| HintError::WrongIdentifierTypeInternal(Box::new(var_addr)))
     } else {
@@ -79,7 +79,7 @@ pub fn get_maybe_relocatable_from_reference(
     }
     //Then calculate address
     let var_addr = compute_addr_from_reference(hint_reference, vm, ap_tracking)?;
-    if hint_reference.dereference {
+    if hint_reference.outer_dereference {
         vm.get_maybe(&var_addr)
     } else {
         Some(MaybeRelocatable::from(var_addr))
@@ -94,7 +94,7 @@ pub fn compute_addr_from_reference(
     //ApTracking of the Hint itself
     hint_ap_tracking: &ApTracking,
 ) -> Option<Relocatable> {
-    let offset1 =
+    let mut offset1 =
         if let OffsetValue::Reference(_register, _offset, _deref) = &hint_reference.offset1 {
             get_offset_value_reference(
                 vm,
@@ -118,11 +118,15 @@ pub fn compute_addr_from_reference(
                 &hint_reference.offset2,
             )?;
 
-            Some((offset1 + value.get_int_ref()?.to_usize()?).ok()?)
+            offset1 += value.get_int_ref()?.to_usize()?
         }
-        OffsetValue::Value(value) => Some((offset1 + *value).ok()?),
-        _ => Some(offset1),
+        OffsetValue::Value(value) => offset1 = (offset1 + *value).ok()?,
+        _ => {}
     }
+    if hint_reference.inner_dereference {
+        offset1 = vm.get_relocatable(offset1).ok()?
+    }
+    Some(offset1)
 }
 
 fn apply_ap_tracking_correction(
@@ -217,7 +221,8 @@ mod tests {
         let hint_ref = HintReference {
             offset1: OffsetValue::Reference(Register::FP, 0, false),
             offset2: OffsetValue::Immediate(Felt252::TWO),
-            dereference: false,
+            outer_dereference: false,
+            inner_dereference: false,
             ap_tracking_data: Default::default(),
             cairo_type: None,
         };
@@ -382,4 +387,30 @@ mod tests {
             None
         );
     }
+
+    #[test]
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+    fn get_integer_from_reference_with_triple_deref() {
+        // Reference: [cast([[fp + 2)] + 2], felt*)]
+        let mut vm = vm!();
+        vm.segments = segments![
+            ((1, 2), (0, 0)), // [fp + 2] -> [(1, 0) + 2] -> [(1, 2)] -> (0, 0)
+            ((0, 2), (0, 5)), // [[fp + 2] + 2] -> [(0, 0) + 2] -> [(0, 2)] -> (0, 5)
+            ((0, 5), 3)       // [[[fp + 2] + 2]] -> [(0, 5)] -> 3
+        ];
+        let hint_ref = HintReference {
+            offset1: OffsetValue::Reference(Register::FP, 2, true),
+            offset2: OffsetValue::Value(2),
+            outer_dereference: true,
+            inner_dereference: true,
+            ap_tracking_data: Default::default(),
+            cairo_type: None,
+        };
+
+        assert_eq!(
+            get_integer_from_reference(&vm, &hint_ref, &ApTracking::new())
+                .expect("Unexpected get integer fail"),
+            Felt252::THREE
+        );
+    }
 }
diff --git a/vm/src/serde/deserialize_program.rs b/vm/src/serde/deserialize_program.rs
@@ -323,10 +323,11 @@ pub enum OffsetValue {
 #[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct ValueAddress {
-    pub offset1: OffsetValue,
-    pub offset2: OffsetValue,
-    pub dereference: bool,
-    pub value_type: String,
+    pub offset1: OffsetValue,    // A in cast(A + B, type)
+    pub offset2: OffsetValue,    // B in cast(A + B, type)
+    pub outer_dereference: bool, // [] in [cast(A + B, type)]
+    pub inner_dereference: bool, // [] in cast([A + B], type)
+    pub value_type: String,      // type in cast(A + B, type)
 }
 
 impl ValueAddress {
@@ -341,7 +342,8 @@ impl ValueAddress {
         ValueAddress {
             offset1: OffsetValue::Value(99),
             offset2: OffsetValue::Value(99),
-            dereference: false,
+            outer_dereference: false,
+            inner_dereference: false,
             value_type: String::from("felt"),
         }
     }
@@ -756,7 +758,8 @@ mod tests {
                     value_address: ValueAddress {
                         offset1: OffsetValue::Reference(Register::FP, -4, false),
                         offset2: OffsetValue::Value(0),
-                        dereference: true,
+                        outer_dereference: true,
+                        inner_dereference: false,
                         value_type: "felt".to_string(),
                     },
                 },
@@ -769,7 +772,8 @@ mod tests {
                     value_address: ValueAddress {
                         offset1: OffsetValue::Reference(Register::FP, -3, false),
                         offset2: OffsetValue::Value(0),
-                        dereference: true,
+                        outer_dereference: true,
+                        inner_dereference: false,
                         value_type: "felt".to_string(),
                     },
                 },
@@ -782,7 +786,8 @@ mod tests {
                     value_address: ValueAddress {
                         offset1: OffsetValue::Reference(Register::FP, -3, true),
                         offset2: OffsetValue::Immediate(Felt252::from(2)),
-                        dereference: false,
+                        outer_dereference: false,
+                        inner_dereference: false,
                         value_type: "felt".to_string(),
                     },
                 },
@@ -795,7 +800,8 @@ mod tests {
                     value_address: ValueAddress {
                         offset1: OffsetValue::Reference(Register::FP, 0, false),
                         offset2: OffsetValue::Value(0),
-                        dereference: true,
+                        outer_dereference: true,
+                        inner_dereference: false,
                         value_type: "felt*".to_string(),
                     },
                 },