diff --git a/CHANGELOG.md b/CHANGELOG.md index a9cc5a97e9..fe85171fd2 100644 --- 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 index 95ffd9975a..59cc044298 100644 --- 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 index a096d45521..fbf3050b89 100644 --- 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, pub cairo_type: Option, } @@ -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 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 index 31c60a5ce0..f952a9d4e7 100644 --- 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 { 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 { - 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 index b897223f87..401a9b7b6b 100644 --- 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(), }, }, diff --git a/vm/src/serde/deserialize_utils.rs b/vm/src/serde/deserialize_utils.rs index 7462a53cb6..8179605c52 100644 --- a/vm/src/serde/deserialize_utils.rs +++ b/vm/src/serde/deserialize_utils.rs @@ -52,7 +52,14 @@ fn outer_brackets(input: &str) -> IResult<&str, bool> { ))(input) .map(|(rem_input, res_opt)| { if let Some(res) = res_opt { - (res, true) + if !rem_input.is_empty() { + // This means that the parser mistook an offset value's inner dereference for a reference's inner dereference + // For example: [fp + 2] + 2 being parsed as "fp + 2" with "+2" as remaining output + // In this case we discard this parsing step + (input, false) + } else { + (res, true) + } } else { (rem_input, false) } @@ -144,12 +151,14 @@ fn no_inner_dereference(input: &str) -> IResult<&str, OffsetValue> { } pub(crate) fn parse_value(input: &str) -> IResult<&str, ValueAddress> { - let (rem_input, (dereference, second_arg, fst_offset, snd_offset)) = tuple(( - outer_brackets, - take_cast_first_arg, - opt(alt((inner_dereference, no_inner_dereference))), - opt(alt((inner_dereference, no_inner_dereference))), - ))(input)?; + let (rem_input, (outer_dereference, second_arg, inner_dereference, fst_offset, snd_offset)) = + tuple(( + outer_brackets, + take_cast_first_arg, + outer_brackets, + opt(alt((inner_dereference, no_inner_dereference))), + opt(alt((inner_dereference, no_inner_dereference))), + ))(input)?; let (indirection_level, (_, struct_)) = tuple((tag(", "), take_till(|c: char| c == '*')))(second_arg)?; @@ -185,7 +194,8 @@ pub(crate) fn parse_value(input: &str) -> IResult<&str, ValueAddress> { let value_address = ValueAddress { offset1, offset2, - dereference, + outer_dereference, + inner_dereference, value_type: type_, }; @@ -387,7 +397,8 @@ mod tests { ValueAddress { offset2: OffsetValue::Value(2), offset1: OffsetValue::Reference(Register::FP, -1_i32, true), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -407,7 +418,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 2_i32, false), offset2: OffsetValue::Value(0), - dereference: false, + outer_dereference: false, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -426,7 +438,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Value(825323), offset2: OffsetValue::Value(0), - dereference: false, + outer_dereference: false, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -446,7 +459,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, false), offset2: OffsetValue::Value(-1), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -466,7 +480,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, true), offset2: OffsetValue::Value(1), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "__main__.felt".to_string(), } )) @@ -486,7 +501,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, true), offset2: OffsetValue::Immediate(Felt252::ONE), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -506,7 +522,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 1_i32, true), offset2: OffsetValue::Value(1), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -526,7 +543,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, true), offset2: OffsetValue::Reference(Register::FP, 1_i32, true), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "__main__.felt".to_string(), } )) @@ -546,7 +564,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 1_i32, true), offset2: OffsetValue::Reference(Register::FP, 1_i32, true), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "__main__.felt".to_string(), } )) @@ -566,7 +585,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Immediate(Felt252::from(825323_i32)), offset2: OffsetValue::Immediate(Felt252::ZERO), - dereference: false, + outer_dereference: false, + inner_dereference: false, value_type: "felt".to_string(), } )) @@ -586,7 +606,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, true), offset2: OffsetValue::Value(1), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "starkware.cairo.common.cairo_secp.ec.EcPoint".to_string(), } )) @@ -606,7 +627,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, true), offset2: OffsetValue::Value(1), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "starkware.cairo.common.cairo_secp.ec.EcPoint*".to_string(), } )) @@ -626,7 +648,29 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 0_i32, true), offset2: OffsetValue::Reference(Register::AP, 0_i32, true), - dereference: true, + outer_dereference: true, + inner_dereference: false, + value_type: "felt".to_string(), + } + )) + ); + } + + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn parse_value_to_felt_with_triple_dereference() { + let value = "[cast([[fp + (-3)] + 5], felt*)]"; + let parsed = parse_value(value); + + assert_eq!( + parsed, + Ok(( + "", + ValueAddress { + offset1: OffsetValue::Reference(Register::FP, -3_i32, true), + offset2: OffsetValue::Value(5), + outer_dereference: true, + inner_dereference: true, value_type: "felt".to_string(), } )) @@ -646,7 +690,8 @@ mod tests { ValueAddress { offset1: OffsetValue::Reference(Register::AP, 1_i32, true), offset2: OffsetValue::Reference(Register::AP, 2_i32, true), - dereference: true, + outer_dereference: true, + inner_dereference: false, value_type: "felt".to_string(), } )) diff --git a/vm/src/serde/serialize_program.rs b/vm/src/serde/serialize_program.rs index e50ba1c872..922769732a 100644 --- a/vm/src/serde/serialize_program.rs +++ b/vm/src/serde/serialize_program.rs @@ -195,7 +195,8 @@ impl From<&Program> for ProgramSerializer { value_address: ValueAddress { offset1: r.offset1, offset2: r.offset2, - dereference: r.dereference, + outer_dereference: r.outer_dereference, + inner_dereference: r.inner_dereference, value_type: r.cairo_type.unwrap_or_default(), }, ap_tracking_data: r.ap_tracking_data.unwrap_or_default(), diff --git a/vm/src/types/program.rs b/vm/src/types/program.rs index f5399d059a..a56558395b 100644 --- a/vm/src/types/program.rs +++ b/vm/src/types/program.rs @@ -339,7 +339,8 @@ impl Program { HintReference { offset1: r.value_address.offset1.clone(), offset2: r.value_address.offset2.clone(), - dereference: r.value_address.dereference, + outer_dereference: r.value_address.outer_dereference, + inner_dereference: r.value_address.inner_dereference, // only store `ap` tracking data if the reference is referred to it ap_tracking_data: match (&r.value_address.offset1, &r.value_address.offset2) { (OffsetValue::Reference(Register::AP, _, _), _)