Skip to content

Commit

Permalink
Handle off2 immediate case in get_integer_from_reference (#1701)
Browse files Browse the repository at this point in the history
* Handle off2 immediate case in get_integer_from_reference

* Add changelog entry
  • Loading branch information
fmoletta authored Apr 8, 2024
1 parent c692b75 commit 404407d
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 106 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

#### Upcoming Changes

* bugfix(BREAKING): Handle off2 immediate case in `get_integer_from_reference`[#1701](https://github.com/lambdaclass/cairo-vm/pull/1701)
* `get_integer_from_reference` & `get_integer_from_var_name` output changed from `Result<Cow<'a, Felt252>, HintError>` to `Result<Felt252, HintError>`

* feat: Reorganized builtins to be in the top of stack at the end of a run (Cairo1).

* BREAKING: Remove `CairoRunner::add_additional_hash_builtin` & `VirtualMachine::disable_trace`[#1658](https://github.com/lambdaclass/cairo-vm/pull/1658)
Expand Down
5 changes: 2 additions & 3 deletions vm/src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,8 @@ pub fn example_blake2s_compress(
let blake2s_start = get_ptr_from_var_name("blake2s_start", vm, ids_data, ap_tracking)?;
let output = get_ptr_from_var_name("output", vm, ids_data, ap_tracking)?;
let n_bytes = get_integer_from_var_name("n_bytes", vm, ids_data, ap_tracking).map(|x| {
x.to_u32().ok_or_else(|| {
HintError::Math(MathError::Felt252ToU32Conversion(Box::new(x.into_owned())))
})
x.to_u32()
.ok_or_else(|| HintError::Math(MathError::Felt252ToU32Conversion(Box::new(x))))
})??;

let message = get_fixed_size_u32_array::<16>(&vm.get_integer_range(blake2s_start, 16)?)?;
Expand Down
6 changes: 3 additions & 3 deletions vm/src/hint_processor/builtin_hint_processor/ec_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub fn random_ec_point_hint(
) -> Result<(), HintError> {
let p = EcPoint::from_var_name("p", vm, ids_data, ap_tracking)?;
let q = EcPoint::from_var_name("q", vm, ids_data, ap_tracking)?;
let m = get_integer_from_var_name("m", vm, ids_data, ap_tracking)?;
let m = Cow::Owned(get_integer_from_var_name("m", vm, ids_data, ap_tracking)?);
let bytes: Vec<u8> = [p.x, p.y, m, q.x, q.y]
.iter()
.flat_map(|x| x.to_bytes_be())
Expand Down Expand Up @@ -109,7 +109,7 @@ pub fn chained_ec_op_random_ec_point_hint(
) -> Result<(), HintError> {
let n_elms = get_integer_from_var_name("len", vm, ids_data, ap_tracking)?;
if n_elms.is_zero() || n_elms.to_usize().is_none() {
return Err(HintError::InvalidLenValue(Box::new(n_elms.into_owned())));
return Err(HintError::InvalidLenValue(Box::new(n_elms)));
}
let n_elms = n_elms.to_usize().unwrap();
let p = EcPoint::from_var_name("p", vm, ids_data, ap_tracking)?;
Expand Down Expand Up @@ -141,7 +141,7 @@ pub fn recover_y_hint(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let p_x = get_integer_from_var_name("x", vm, ids_data, ap_tracking)?.into_owned();
let p_x = get_integer_from_var_name("x", vm, ids_data, ap_tracking)?;
let p_addr = get_relocatable_from_var_name("p", vm, ids_data, ap_tracking)?;
vm.insert_value(p_addr, p_x)?;
let p_y = Felt252::from(
Expand Down
20 changes: 8 additions & 12 deletions vm/src/hint_processor/builtin_hint_processor/find_element_hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ pub fn find_element(
.to_usize()
.ok_or_else(|| HintError::ValueOutOfRange(Box::new(*elm_size_bigint.as_ref())))?;
if elm_size == 0 {
return Err(HintError::ValueOutOfRange(Box::new(
elm_size_bigint.into_owned(),
)));
return Err(HintError::ValueOutOfRange(Box::new(elm_size_bigint)));
}

if let Some(find_element_index_value) = find_element_index {
Expand All @@ -44,7 +42,7 @@ pub fn find_element(
if found_key.as_ref() != key.as_ref() {
return Err(HintError::InvalidIndex(Box::new((
find_element_index_value,
key.into_owned(),
key,
found_key.into_owned(),
))));
}
Expand All @@ -56,13 +54,13 @@ pub fn find_element(
if n_elms.as_ref() > find_element_max_size {
return Err(HintError::FindElemMaxSize(Box::new((
*find_element_max_size,
n_elms.into_owned(),
n_elms,
))));
}
}
let n_elms_iter: u32 = n_elms
.to_u32()
.ok_or_else(|| MathError::Felt252ToI32Conversion(Box::new(n_elms.into_owned())))?;
.ok_or_else(|| MathError::Felt252ToI32Conversion(Box::new(n_elms)))?;

for i in 0..n_elms_iter {
let iter_key = vm
Expand All @@ -80,9 +78,7 @@ pub fn find_element(
}
}

Err(HintError::NoValueForKeyFindElement(Box::new(
key.into_owned(),
)))
Err(HintError::NoValueForKeyFindElement(Box::new(key)))
}
}

Expand All @@ -93,10 +89,10 @@ pub fn search_sorted_lower(
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let find_element_max_size = exec_scopes.get::<Felt252>("find_element_max_size");
let n_elms = *get_integer_from_var_name("n_elms", vm, ids_data, ap_tracking)?;
let n_elms = get_integer_from_var_name("n_elms", vm, ids_data, ap_tracking)?;
let rel_array_ptr = get_relocatable_from_var_name("array_ptr", vm, ids_data, ap_tracking)?;
let elm_size = *get_integer_from_var_name("elm_size", vm, ids_data, ap_tracking)?;
let key = *get_integer_from_var_name("key", vm, ids_data, ap_tracking)?;
let elm_size = get_integer_from_var_name("elm_size", vm, ids_data, ap_tracking)?;
let key = get_integer_from_var_name("key", vm, ids_data, ap_tracking)?;

if elm_size == Felt252::ZERO {
return Err(HintError::ValueOutOfRange(Box::new(elm_size)));
Expand Down
14 changes: 7 additions & 7 deletions vm/src/hint_processor/builtin_hint_processor/hint_utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::stdlib::{borrow::Cow, boxed::Box, collections::HashMap, prelude::*};
use crate::stdlib::{boxed::Box, collections::HashMap, prelude::*};

use crate::Felt252;

Expand Down Expand Up @@ -83,12 +83,12 @@ pub fn get_relocatable_from_var_name(
//Gets the value of a variable name.
//If the value is an MaybeRelocatable::Int(Bigint) return &Bigint
//else raises Err
pub fn get_integer_from_var_name<'a>(
var_name: &'a str,
vm: &'a VirtualMachine,
ids_data: &'a HashMap<String, HintReference>,
pub fn get_integer_from_var_name(
var_name: &str,
vm: &VirtualMachine,
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<Cow<'a, Felt252>, HintError> {
) -> Result<Felt252, HintError> {
let reference = get_reference_from_var_name(var_name, ids_data)?;
match get_integer_from_reference(vm, reference, ap_tracking) {
// Map internal errors into more descriptive variants
Expand Down Expand Up @@ -260,7 +260,7 @@ mod tests {

assert_matches!(
get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Ok(Cow::Borrowed(x)) if x == &Felt252::from(1)
Ok(x) if x == Felt252::from(1)
);
}

Expand Down
9 changes: 4 additions & 5 deletions vm/src/hint_processor/builtin_hint_processor/keccak_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub fn unsafe_keccak(
if let Ok(keccak_max_size) = exec_scopes.get::<Felt252>("__keccak_max_size") {
if length.as_ref() > &keccak_max_size {
return Err(HintError::KeccakMaxSize(Box::new((
length.into_owned(),
length,
keccak_max_size,
))));
}
Expand All @@ -71,7 +71,7 @@ pub fn unsafe_keccak(
// transform to u64 to make ranges cleaner in the for loop below
let u64_length = length
.to_u64()
.ok_or_else(|| HintError::InvalidKeccakInputLength(Box::new(length.into_owned())))?;
.ok_or_else(|| HintError::InvalidKeccakInputLength(Box::new(length)))?;

const ZEROES: [u8; 32] = [0u8; 32];
let mut keccak_input = Vec::new();
Expand Down Expand Up @@ -243,9 +243,8 @@ pub fn split_n_bytes(
) -> Result<(), HintError> {
let n_bytes =
get_integer_from_var_name("n_bytes", vm, ids_data, ap_tracking).and_then(|x| {
x.to_u64().ok_or_else(|| {
HintError::Math(MathError::Felt252ToU64Conversion(Box::new(x.into_owned())))
})
x.to_u64()
.ok_or_else(|| HintError::Math(MathError::Felt252ToU64Conversion(Box::new(x))))
})?;
let bytes_in_word = constants
.get(BYTES_IN_WORD)
Expand Down
64 changes: 21 additions & 43 deletions vm/src/hint_processor/builtin_hint_processor/math_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,8 @@ pub fn assert_le_felt_v_0_6(
let a = &get_integer_from_var_name("a", vm, ids_data, ap_tracking)?;
let b = &get_integer_from_var_name("b", vm, ids_data, ap_tracking)?;

if a.as_ref() > b.as_ref() {
return Err(HintError::NonLeFelt252(Box::new((
a.clone().into_owned(),
b.clone().into_owned(),
))));
if a > b {
return Err(HintError::NonLeFelt252(Box::new((*a, *b))));
}
Ok(())
}
Expand All @@ -178,15 +175,11 @@ pub fn assert_le_felt_v_0_8(
let a = &get_integer_from_var_name("a", vm, ids_data, ap_tracking)?;
let b = &get_integer_from_var_name("b", vm, ids_data, ap_tracking)?;

if a.as_ref() > b.as_ref() {
return Err(HintError::NonLeFelt252(Box::new((
a.clone().into_owned(),
b.clone().into_owned(),
))));
if a > b {
return Err(HintError::NonLeFelt252(Box::new((*a, *b))));
}
let bound = vm.get_range_check_builtin()?._bound.unwrap_or_default();
let small_inputs =
Felt252::from((a.as_ref() < &bound && b.as_ref() - a.as_ref() < bound) as u8);
let small_inputs = Felt252::from((a < &bound && b - a < bound) as u8);
insert_value_from_var_name("small_inputs", small_inputs, vm, ids_data, ap_tracking)
}

Expand Down Expand Up @@ -300,9 +293,7 @@ pub fn assert_nn(
// assert 0 <= ids.a % PRIME < range_check_builtin.bound
// as prime > 0, a % prime will always be > 0
match &range_check_builtin._bound {
Some(bound) if a.as_ref() >= bound => {
Err(HintError::AssertNNValueOutOfRange(Box::new(a.into_owned())))
}
Some(bound) if a.as_ref() >= bound => Err(HintError::AssertNNValueOutOfRange(Box::new(a))),
_ => Ok(()),
}
}
Expand All @@ -321,7 +312,7 @@ pub fn assert_not_zero(
let value = get_integer_from_var_name("value", vm, ids_data, ap_tracking)?;
if value.is_zero() {
return Err(HintError::AssertNotZero(Box::new((
value.into_owned(),
value,
crate::utils::PRIME_STR.to_string(),
))));
};
Expand Down Expand Up @@ -375,17 +366,15 @@ pub fn is_positive(
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let value = get_integer_from_var_name("value", vm, ids_data, ap_tracking)?;
let value_as_int = signed_felt(*value);
let value_as_int = signed_felt(value);
let range_check_builtin = vm.get_range_check_builtin()?;

// Avoid using abs so we don't allocate a new BigInt
let (sign, abs_value) = value_as_int.into_parts();
//Main logic (assert a is positive)
match &range_check_builtin._bound {
Some(bound) if abs_value > bound.to_biguint() => {
return Err(HintError::ValueOutsideValidRange(Box::new(
value.into_owned(),
)))
return Err(HintError::ValueOutsideValidRange(Box::new(value)))
}
_ => {}
};
Expand Down Expand Up @@ -447,10 +436,8 @@ pub fn sqrt(
) -> Result<(), HintError> {
let mod_value = get_integer_from_var_name("value", vm, ids_data, ap_tracking)?;
//This is equal to mod_value > Felt252::from(2).pow(250)
if *mod_value > pow2_const(250) {
return Err(HintError::ValueOutside250BitRange(Box::new(
mod_value.into_owned(),
)));
if mod_value > pow2_const(250) {
return Err(HintError::ValueOutside250BitRange(Box::new(mod_value)));
//This is equal to mod_value > bigint!(2).pow(250)
}
insert_value_from_var_name(
Expand All @@ -475,15 +462,12 @@ pub fn signed_div_rem(

let builtin_bound = &builtin._bound.unwrap_or(Felt252::MAX);
if div.is_zero() || div.as_ref() > &div_prime_by_bound(*builtin_bound)? {
return Err(HintError::OutOfValidRange(Box::new((
div.into_owned(),
*builtin_bound,
))));
return Err(HintError::OutOfValidRange(Box::new((div, *builtin_bound))));
}
let builtin_bound_div_2 = builtin_bound.field_div(&Felt252::TWO.try_into().unwrap());
if *bound > builtin_bound_div_2 {
if bound > builtin_bound_div_2 {
return Err(HintError::OutOfValidRange(Box::new((
bound.into_owned(),
bound,
builtin_bound_div_2,
))));
}
Expand All @@ -496,7 +480,7 @@ pub fn signed_div_rem(
if int_bound.abs() < q.abs() {
return Err(HintError::OutOfValidRange(Box::new((
Felt252::from(&q),
bound.into_owned(),
bound,
))));
}

Expand Down Expand Up @@ -534,21 +518,18 @@ pub fn unsigned_div_rem(
Some(builtin_bound)
if div.is_zero() || div.as_ref() > &div_prime_by_bound(*builtin_bound)? =>
{
return Err(HintError::OutOfValidRange(Box::new((
div.into_owned(),
*builtin_bound,
))));
return Err(HintError::OutOfValidRange(Box::new((div, *builtin_bound))));
}
None if div.is_zero() => {
return Err(HintError::OutOfValidRange(Box::new((
div.into_owned(),
div,
Felt252::ZERO - Felt252::ONE,
))));
}
_ => {}
}

let (q, r) = value.div_rem(&(*div).try_into().map_err(|_| MathError::DividedByZero)?);
let (q, r) = value.div_rem(&(div).try_into().map_err(|_| MathError::DividedByZero)?);
insert_value_from_var_name("r", r, vm, ids_data, ap_tracking)?;
insert_value_from_var_name("q", q, vm, ids_data, ap_tracking)
}
Expand All @@ -574,7 +555,7 @@ pub fn assert_250_bit(
let shift = constants
.get(SHIFT)
.map_or_else(|| get_constant_from_var_name("SHIFT", constants), Ok)?;
let value = Felt252::from(&signed_felt(*get_integer_from_var_name(
let value = Felt252::from(&signed_felt(get_integer_from_var_name(
"value",
vm,
ids_data,
Expand Down Expand Up @@ -675,10 +656,7 @@ pub fn assert_lt_felt(
// assert (ids.a % PRIME) < (ids.b % PRIME), \
// f'a = {ids.a % PRIME} is not less than b = {ids.b % PRIME}.'
if a >= b {
return Err(HintError::AssertLtFelt252(Box::new((
a.into_owned(),
b.into_owned(),
))));
return Err(HintError::AssertLtFelt252(Box::new((a, b))));
};
Ok(())
}
Expand All @@ -688,7 +666,7 @@ pub fn is_quad_residue(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let x = *get_integer_from_var_name("x", vm, ids_data, ap_tracking)?;
let x = get_integer_from_var_name("x", vm, ids_data, ap_tracking)?;

if x.is_zero() || x == Felt252::ONE {
insert_value_from_var_name("y", *x.as_ref(), vm, ids_data, ap_tracking)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ pub fn memcpy_enter_scope(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let len: Box<dyn Any> =
Box::new(get_integer_from_var_name("len", vm, ids_data, ap_tracking)?.into_owned());
let len: Box<dyn Any> = Box::new(get_integer_from_var_name("len", vm, ids_data, ap_tracking)?);
exec_scopes.enter_scope(HashMap::from([(String::from("n"), len)]));
Ok(())
}
Expand Down
3 changes: 1 addition & 2 deletions vm/src/hint_processor/builtin_hint_processor/memset_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ pub fn memset_enter_scope(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let n: Box<dyn Any> =
Box::new(get_integer_from_var_name("n", vm, ids_data, ap_tracking)?.into_owned());
let n: Box<dyn Any> = Box::new(get_integer_from_var_name("n", vm, ids_data, ap_tracking)?);
exec_scopes.enter_scope(HashMap::from([(String::from("n"), n)]));
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion vm/src/hint_processor/builtin_hint_processor/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn set_add(
let elm_size =
get_integer_from_var_name("elm_size", vm, ids_data, ap_tracking).and_then(|x| {
x.to_usize()
.ok_or_else(|| MathError::Felt252ToUsizeConversion(Box::new(x.into_owned())).into())
.ok_or_else(|| MathError::Felt252ToUsizeConversion(Box::new(x)).into())
})?;
let elm_ptr = get_ptr_from_var_name("elm_ptr", vm, ids_data, ap_tracking)?;
let set_end_ptr = get_ptr_from_var_name("set_end_ptr", vm, ids_data, ap_tracking)?;
Expand Down
6 changes: 2 additions & 4 deletions vm/src/hint_processor/builtin_hint_processor/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ pub fn verify_ecdsa_signature(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let signature_r =
get_integer_from_var_name("signature_r", vm, ids_data, ap_tracking)?.into_owned();
let signature_s =
get_integer_from_var_name("signature_s", vm, ids_data, ap_tracking)?.into_owned();
let signature_r = get_integer_from_var_name("signature_r", vm, ids_data, ap_tracking)?;
let signature_s = get_integer_from_var_name("signature_s", vm, ids_data, ap_tracking)?;
let ecdsa_ptr = get_ptr_from_var_name("ecdsa_ptr", vm, ids_data, ap_tracking)?;
let ecdsa_builtin = &mut vm.get_signature_builtin()?;
if ecdsa_ptr.segment_index != ecdsa_builtin.base() as isize {
Expand Down
Loading

0 comments on commit 404407d

Please sign in to comment.