Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unecessary conversion functions between Felt & BigUint/BigInt #1562

Merged
merged 13 commits into from
Feb 1, 2024
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

#### Upcoming Changes

* feat(BREAKING): Remove unecessary conversion functions between `Felt` & `BigUint`/`BigInt` [#1562](https://github.com/lambdaclass/cairo-vm/pull/1562)
* Remove the following functions:
* felt_from_biguint
* felt_from_bigint
* felt_to_biguint
* felt_to_bigint

* perf: optimize instruction cache allocations by using `VirtualMachine::load_data` [#1441](https://github.com/lambdaclass/cairo-vm/pull/1441)

* feat: Add `print_output` flag to `cairo-1` crate [#1575] (https://github.com/lambdaclass/cairo-vm/pull/1575)
Expand Down
3 changes: 1 addition & 2 deletions cairo1-run/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ use cairo_vm::serde::deserialize_program::BuiltinName;
use cairo_vm::serde::deserialize_program::{ApTracking, FlowTrackingData, HintParams};
use cairo_vm::types::errors::program_errors::ProgramError;
use cairo_vm::types::relocatable::Relocatable;
use cairo_vm::utils::bigint_to_felt;
use cairo_vm::vm::decoding::decoder::decode_instruction;
use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
use cairo_vm::vm::errors::memory_errors::MemoryError;
Expand Down Expand Up @@ -326,7 +325,7 @@ fn run(args: impl Iterator<Item = String>) -> Result<Option<String>, Error> {

let data: Vec<MaybeRelocatable> = instructions
.flat_map(|inst| inst.assemble().encode())
.map(|x| bigint_to_felt(&x).unwrap_or_default())
.map(|x| Felt252::from(&x))
.map(MaybeRelocatable::from)
.collect();

Expand Down
16 changes: 8 additions & 8 deletions vm/src/hint_processor/builtin_hint_processor/ec_utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::stdlib::{borrow::Cow, boxed::Box, collections::HashMap, prelude::*};
use crate::utils::{bigint_to_felt, biguint_to_felt, felt_to_biguint, CAIRO_PRIME};
use crate::utils::CAIRO_PRIME;
use crate::Felt252;
use crate::{
hint_processor::{
Expand Down Expand Up @@ -144,10 +144,10 @@ pub fn recover_y_hint(
let p_x = get_integer_from_var_name("x", vm, ids_data, ap_tracking)?.into_owned();
let p_addr = get_relocatable_from_var_name("p", vm, ids_data, ap_tracking)?;
vm.insert_value(p_addr, p_x)?;
let p_y = biguint_to_felt(
&recover_y(&felt_to_biguint(p_x))
let p_y = Felt252::from(
&recover_y(&p_x.to_biguint())
Oppen marked this conversation as resolved.
Show resolved Hide resolved
.ok_or_else(|| HintError::RecoverYPointNotOnCurve(Box::new(p_x)))?,
)?;
);
vm.insert_value((p_addr + 1)?, p_y)?;
Ok(())
}
Expand All @@ -174,8 +174,8 @@ fn random_ec_point_seeded(seed_bytes: Vec<u8>) -> Result<(Felt252, Felt252), Hin
if let Some(y) = y {
// Conversion from BigUint to BigInt doesnt fail
return Ok((
biguint_to_felt(&x)?,
bigint_to_felt(&(y.to_bigint().unwrap() * y_coef))?,
Felt252::from(&x),
Felt252::from(&(y.to_bigint().unwrap() * y_coef)),
));
}
}
Expand All @@ -188,7 +188,7 @@ lazy_static! {
10
)
.unwrap();
static ref FELT_MAX_HALVED: BigUint = felt_to_biguint(Felt252::MAX) / 2_u32;
static ref FELT_MAX_HALVED: BigUint = Felt252::MAX.to_biguint() / 2_u32;
}

// Recovers the corresponding y coordinate on the elliptic curve
Expand All @@ -198,7 +198,7 @@ lazy_static! {
fn recover_y(x: &BigUint) -> Option<BigUint> {
let y_squared: BigUint = x.modpow(&BigUint::from(3_u32), &CAIRO_PRIME) + ALPHA * x + &*BETA;
if is_quad_residue(&y_squared) {
Some(felt_to_biguint(biguint_to_felt(&y_squared).ok()?.sqrt()?))
Some(Felt252::from(&y_squared).sqrt()?.to_biguint())
} else {
None
}
Expand Down
69 changes: 33 additions & 36 deletions vm/src/hint_processor/builtin_hint_processor/math_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
math_utils::signed_felt,
stdlib::{boxed::Box, collections::HashMap, prelude::*},
types::errors::math_errors::MathError,
utils::{bigint_to_felt, biguint_to_felt, felt_to_bigint, felt_to_biguint},
};
use lazy_static::lazy_static;
use num_traits::{Signed, Zero};
Expand Down Expand Up @@ -112,8 +111,8 @@
let prime_over_2_high = constants
.get(PRIME_OVER_2_HIGH)
.ok_or_else(|| HintError::MissingConstant(Box::new(PRIME_OVER_2_HIGH)))?;
let a = felt_to_biguint(*get_integer_from_var_name("a", vm, ids_data, ap_tracking)?);
let b = felt_to_biguint(*get_integer_from_var_name("b", vm, ids_data, ap_tracking)?);
let a = get_integer_from_var_name("a", vm, ids_data, ap_tracking)?.to_biguint();
let b = get_integer_from_var_name("b", vm, ids_data, ap_tracking)?.to_biguint();
let range_check_ptr = get_ptr_from_var_name("range_check_ptr", vm, ids_data, ap_tracking)?;

// TODO: use UnsignedInteger for this
Expand All @@ -122,8 +121,8 @@

if a > b {
return Err(HintError::NonLeFelt252(Box::new((
biguint_to_felt(&a)?,
biguint_to_felt(&b)?,
Felt252::from(&a),
Felt252::from(&b),
))));
}

Expand All @@ -134,23 +133,23 @@
// TODO: I believe this check can be removed
if lengths_and_indices[0].0 > &prime_div3 || lengths_and_indices[1].0 > &prime_div2 {
return Err(HintError::ArcTooBig(Box::new((
biguint_to_felt(&lengths_and_indices[0].0.clone())?,
biguint_to_felt(&prime_div2)?,
biguint_to_felt(&lengths_and_indices[1].0.clone())?,
biguint_to_felt(&prime_div3)?,
Felt252::from(&lengths_and_indices[0].0.clone()),
Felt252::from(&prime_div2),
Felt252::from(&lengths_and_indices[1].0.clone()),
Felt252::from(&prime_div3),

Check warning on line 139 in vm/src/hint_processor/builtin_hint_processor/math_utils.rs

View check run for this annotation

Codecov / codecov/patch

vm/src/hint_processor/builtin_hint_processor/math_utils.rs#L136-L139

Added lines #L136 - L139 were not covered by tests
))));
}

let excluded = lengths_and_indices[2].1;
exec_scopes.assign_or_update_variable("excluded", any_box!(Felt252::from(excluded)));

let (q_0, r_0) = (lengths_and_indices[0].0).div_mod_floor(&felt_to_biguint(*prime_over_3_high));
let (q_1, r_1) = (lengths_and_indices[1].0).div_mod_floor(&felt_to_biguint(*prime_over_2_high));
let (q_0, r_0) = (lengths_and_indices[0].0).div_mod_floor(&prime_over_3_high.to_biguint());
let (q_1, r_1) = (lengths_and_indices[1].0).div_mod_floor(&prime_over_2_high.to_biguint());

vm.insert_value(range_check_ptr, biguint_to_felt(&r_0)?)?;
vm.insert_value((range_check_ptr + 1_i32)?, biguint_to_felt(&q_0)?)?;
vm.insert_value((range_check_ptr + 2_i32)?, biguint_to_felt(&r_1)?)?;
vm.insert_value((range_check_ptr + 3_i32)?, biguint_to_felt(&q_1)?)?;
vm.insert_value(range_check_ptr, Felt252::from(&r_0))?;
vm.insert_value((range_check_ptr + 1_i32)?, Felt252::from(&q_0))?;
vm.insert_value((range_check_ptr + 2_i32)?, Felt252::from(&r_1))?;
vm.insert_value((range_check_ptr + 3_i32)?, Felt252::from(&q_1))?;
Ok(())
}

Expand Down Expand Up @@ -383,7 +382,7 @@
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 > felt_to_biguint(*bound) => {
Some(bound) if abs_value > bound.to_biguint() => {
return Err(HintError::ValueOutsideValidRange(Box::new(
value.into_owned(),
)))
Expand Down Expand Up @@ -457,7 +456,7 @@
#[allow(deprecated)]
insert_value_from_var_name(
"root",
biguint_to_felt(&isqrt(&felt_to_biguint(*mod_value))?)?,
Felt252::from(&isqrt(&mod_value.to_biguint())?),
vm,
ids_data,
ap_tracking,
Expand Down Expand Up @@ -491,22 +490,22 @@
}

let int_value = signed_felt(*value);
let int_div = felt_to_bigint(*div);
let int_bound = felt_to_bigint(*bound);
let int_div = div.to_bigint();
let int_bound = bound.to_bigint();
let (q, r) = int_value.div_mod_floor(&int_div);

if int_bound.abs() < q.abs() {
return Err(HintError::OutOfValidRange(Box::new((
bigint_to_felt(&q)?,
Felt252::from(&q),

Check warning on line 499 in vm/src/hint_processor/builtin_hint_processor/math_utils.rs

View check run for this annotation

Codecov / codecov/patch

vm/src/hint_processor/builtin_hint_processor/math_utils.rs#L499

Added line #L499 was not covered by tests
bound.into_owned(),
))));
}

let biased_q = q + int_bound;
insert_value_from_var_name("r", bigint_to_felt(&r)?, vm, ids_data, ap_tracking)?;
insert_value_from_var_name("r", Felt252::from(&r), vm, ids_data, ap_tracking)?;
insert_value_from_var_name(
"biased_q",
bigint_to_felt(&biased_q)?,
Felt252::from(&biased_q),
vm,
ids_data,
ap_tracking,
Expand Down Expand Up @@ -576,12 +575,12 @@
let shift = constants
.get(SHIFT)
.map_or_else(|| get_constant_from_var_name("SHIFT", constants), Ok)?;
let value = bigint_to_felt(&signed_felt(*get_integer_from_var_name(
let value = Felt252::from(&signed_felt(*get_integer_from_var_name(
"value",
vm,
ids_data,
ap_tracking,
)?))?;
)?));
//Main logic
if &value > upper_bound {
return Err(HintError::ValueOutside250BitRange(Box::new(value)));
Expand Down Expand Up @@ -625,11 +624,10 @@
) -> Result<(), HintError> {
let addr = get_integer_from_var_name("addr", vm, ids_data, ap_tracking)?;

let addr_bound = felt_to_biguint(
*constants
.get(ADDR_BOUND)
.ok_or_else(|| HintError::MissingConstant(Box::new(ADDR_BOUND)))?,
);
let addr_bound = constants
.get(ADDR_BOUND)
.ok_or_else(|| HintError::MissingConstant(Box::new(ADDR_BOUND)))?
.to_biguint();

let lower_bound = BigUint::one() << 250_usize;
let upper_bound = BigUint::one() << 251_usize;
Expand All @@ -650,7 +648,7 @@
}

// Main logic: ids.is_small = 1 if ids.addr < ADDR_BOUND else 0
let is_small = Felt252::from((addr.as_ref() < &biguint_to_felt(&addr_bound)?) as u8);
let is_small = Felt252::from((addr.as_ref() < &Felt252::from(&addr_bound)) as u8);

insert_value_from_var_name("is_small", is_small, vm, ids_data, ap_tracking)
}
Expand Down Expand Up @@ -714,9 +712,8 @@

fn div_prime_by_bound(bound: Felt252) -> Result<Felt252, VirtualMachineError> {
let prime: &BigUint = &CAIRO_PRIME;
#[allow(deprecated)]
let limit = prime / felt_to_biguint(bound);
Ok(biguint_to_felt(&limit)?)
let limit = prime / bound.to_biguint();
Ok(Felt252::from(&limit))
}

fn prime_div_constant(bound: u32) -> Result<BigUint, VirtualMachineError> {
Expand Down Expand Up @@ -780,7 +777,7 @@
) -> Result<(), HintError> {
let xx = Uint256::from_var_name("xx", vm, ids_data, ap_tracking)?;
let x_addr = get_relocatable_from_var_name("x", vm, ids_data, ap_tracking)?;
let xx: BigUint = felt_to_biguint(*xx.low) + felt_to_biguint(*xx.high * pow2_const(128));
let xx: BigUint = xx.low.to_biguint() + (*xx.high * pow2_const(128)).to_biguint();
let mut x = xx.modpow(
&(&*SPLIT_XX_PRIME + 3_u32).div_floor(&BigUint::from(8_u32)),
&SPLIT_XX_PRIME,
Expand All @@ -794,9 +791,9 @@

vm.insert_value(
x_addr,
biguint_to_felt(&(&x & &BigUint::from(u128::max_value())))?,
Felt252::from(&(&x & &BigUint::from(u128::max_value()))),
)?;
vm.insert_value((x_addr + 1)?, biguint_to_felt(&(x >> 128_u32))?)?;
vm.insert_value((x_addr + 1)?, Felt252::from(&(x >> 128_u32)))?;

Ok(())
}
Expand Down
3 changes: 1 addition & 2 deletions vm/src/hint_processor/builtin_hint_processor/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use crate::stdlib::collections::HashMap;

use crate::types::exec_scope::ExecutionScopes;
use crate::types::relocatable::MaybeRelocatable;
use crate::utils::felt_to_bigint;
use crate::vm::errors::hint_errors::HintError;
use crate::{
hint_processor::hint_processor_definition::HintReference, vm::vm_core::VirtualMachine,
Expand All @@ -34,7 +33,7 @@ fn print_name(
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let name = get_integer_from_var_name("name", vm, ids_data, ap_tracking)?;
let name = String::from_utf8(felt_to_bigint(*name.as_ref()).to_signed_bytes_be())
let name = String::from_utf8(name.as_ref().to_bigint().to_signed_bytes_be())
.map_err(|err| HintError::CustomHint(err.to_string().into_boxed_str()))?;
println!("{name}");
Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use core::ops::Shl;
use crate::hint_processor::builtin_hint_processor::uint_utils::{pack, split};
use crate::math_utils::signed_felt;
use crate::stdlib::{borrow::Cow, boxed::Box, collections::HashMap, prelude::*};
use crate::types::errors::math_errors::MathError;
use crate::utils::biguint_to_felt;
use crate::Felt252;
use crate::{
hint_processor::{
Expand Down Expand Up @@ -130,8 +128,8 @@ pub fn nondet_bigint3(
.ok_or(HintError::BigIntToBigUintFail)?;
let arg: Vec<MaybeRelocatable> = bigint3_split(&value)?
.into_iter()
.map(|ref n| biguint_to_felt(n).map(MaybeRelocatable::from))
.collect::<Result<Vec<MaybeRelocatable>, MathError>>()?;
.map(|ref n| Felt252::from(n).into())
.collect::<Vec<MaybeRelocatable>>();
vm.write_arg(res_reloc, &arg).map_err(HintError::Memory)?;
Ok(())
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::utils::{biguint_to_felt, felt_to_biguint};
use crate::Felt252;
use crate::{
hint_processor::{
Expand Down Expand Up @@ -503,7 +502,7 @@ pub fn n_pair_bits(
return Err(HintError::NPairBitsTooLowM);
}

let (scalar_v, scalar_u) = (felt_to_biguint(*scalar_v), felt_to_biguint(*scalar_u));
let (scalar_v, scalar_u) = (scalar_v.to_biguint(), scalar_u.to_biguint());

// Each step, fetches the bits in mth position for v and u,
// and appends them to the accumulator. i.e:
Expand All @@ -512,7 +511,7 @@ pub fn n_pair_bits(
// 1010101__ -> 101010110
let get_bit =
|x: &BigUint, i| m.checked_sub(i).map(|i| x.bit(i.into())).unwrap_or(false) as u32;
let res: Felt252 = biguint_to_felt(
let res = Felt252::from(
&(0..number_of_pairs)
.map(|i| {
// This code is definitely verbose, but it's the only way I found to avoid a `panic`
Expand All @@ -527,7 +526,7 @@ pub fn n_pair_bits(
acc += x;
acc
}),
)?;
);
/*
ids.quad_bit = (
8 * ((ids.scalar_v >> ids.m) & 1)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::utils::bigint_to_felt;
use crate::Felt252;
use crate::{
hint_processor::{
Expand Down Expand Up @@ -42,7 +41,7 @@ pub fn verify_zero(
return Err(HintError::SecpVerifyZero(Box::new(val)));
}

insert_value_from_var_name("q", bigint_to_felt(&q)?, vm, ids_data, ap_tracking)
insert_value_from_var_name("q", Felt252::from(&q), vm, ids_data, ap_tracking)
}

/*
Expand All @@ -68,7 +67,7 @@ pub fn verify_zero_with_external_const(
return Err(HintError::SecpVerifyZero(Box::new(val)));
}

insert_value_from_var_name("q", bigint_to_felt(&q)?, vm, ids_data, ap_tracking)
insert_value_from_var_name("q", Felt252::from(&q), vm, ids_data, ap_tracking)
}

/*
Expand Down
12 changes: 5 additions & 7 deletions vm/src/hint_processor/builtin_hint_processor/secp/signature.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::utils::{felt_to_bigint, felt_to_biguint};
use crate::Felt252;
use crate::{
any_box,
Expand Down Expand Up @@ -111,11 +110,10 @@ pub fn get_point_from_x(
) -> Result<(), HintError> {
exec_scopes.insert_value("SECP_P", SECP_P.clone());
#[allow(deprecated)]
let beta = felt_to_bigint(
*constants
.get(BETA)
.ok_or_else(|| HintError::MissingConstant(Box::new(BETA)))?,
);
let beta = constants
.get(BETA)
.ok_or_else(|| HintError::MissingConstant(Box::new(BETA)))?
.to_bigint();

let x_cube_int = Uint384::from_var_name("x_cube", vm, ids_data, ap_tracking)?
.pack86()
Expand All @@ -125,7 +123,7 @@ pub fn get_point_from_x(
let mut y = y_cube_int.modpow(&(&*SECP_P + 1_u32).shr(2_u32), &SECP_P);

#[allow(deprecated)]
let v = felt_to_biguint(*get_integer_from_var_name("v", vm, ids_data, ap_tracking)?);
let v = get_integer_from_var_name("v", vm, ids_data, ap_tracking)?.to_bigint();
if v.is_even() != y.is_even() {
y = &*SECP_P - y;
}
Expand Down
Loading
Loading