Skip to content

Commit

Permalink
Add documentation and deprecate to_bigint and to_biguint (#757)
Browse files Browse the repository at this point in the history
* Add docu comment and deprecate

* Fix broken compilation

* Clean warnings

* Run cargo fmt

* Fix docu error

---------

Co-authored-by: Juanma <juanma@Juanmas-MacBook-Air.local>
  • Loading branch information
Juan-M-V and Juanma authored Feb 14, 2023
1 parent f16b420 commit 2410f6b
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 0 deletions.
57 changes: 57 additions & 0 deletions felt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,61 @@ pub const FIELD_LOW: u128 = 1;

pub(crate) trait FeltOps {
fn new<T: Into<FeltBigInt<FIELD_HIGH, FIELD_LOW>>>(value: T) -> Self;

fn modpow(
&self,
exponent: &FeltBigInt<FIELD_HIGH, FIELD_LOW>,
modulus: &FeltBigInt<FIELD_HIGH, FIELD_LOW>,
) -> Self;

fn iter_u64_digits(&self) -> U64Digits;

fn to_signed_bytes_le(&self) -> Vec<u8>;

fn to_bytes_be(&self) -> Vec<u8>;

fn parse_bytes(buf: &[u8], radix: u32) -> Option<FeltBigInt<FIELD_HIGH, FIELD_LOW>>;

fn from_bytes_be(bytes: &[u8]) -> Self;

fn to_str_radix(&self, radix: u32) -> String;

#[deprecated]
/// Converts [`Felt`] into a [`BigInt`] number in the range: `(- FIELD / 2, FIELD / 2)`.
///
/// # Examples
///
/// ```
/// # use crate::cairo_felt::Felt;
/// # use num_bigint::BigInt;
/// # use num_traits::Bounded;
/// let positive = Felt::new(5);
/// assert_eq!(positive.to_bigint(), Into::<num_bigint::BigInt>::into(5));
///
/// let negative = Felt::max_value();
/// assert_eq!(negative.to_bigint(), Into::<num_bigint::BigInt>::into(-1));
/// ```
fn to_bigint(&self) -> BigInt;

#[deprecated]
/// Converts [`Felt`] into a [`BigUint`] number.
///
/// # Examples
///
/// ```
/// # use crate::cairo_felt::Felt;
/// # use num_bigint::BigUint;
/// # use num_traits::{Num, Bounded};
/// let positive = Felt::new(5);
/// assert_eq!(positive.to_biguint(), Into::<num_bigint::BigUint>::into(5_u32));
///
/// let negative = Felt::max_value();
/// assert_eq!(negative.to_biguint(), BigUint::from_str_radix("800000000000011000000000000000000000000000000000000000000000000", 16).unwrap());
/// ```
fn to_biguint(&self) -> BigUint;

fn sqrt(&self) -> Self;

fn bits(&self) -> u64;
}

Expand Down Expand Up @@ -117,9 +158,11 @@ impl Felt {
self.value.to_str_radix(radix)
}
pub fn to_bigint(&self) -> BigInt {
#[allow(deprecated)]
self.value.to_bigint()
}
pub fn to_biguint(&self) -> BigUint {
#[allow(deprecated)]
self.value.to_biguint()
}
pub fn sqrt(&self) -> Self {
Expand Down Expand Up @@ -781,6 +824,7 @@ mod test {

proptest! {
#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 felt values that are randomly generated each time tests are run, that a new felt doesn't fall outside the range [0, p].
// In this and some of the following tests, The value of {x} can be either [0] or a very large number, in order to try to overflow the value of {p} and thus ensure the modular arithmetic is working correctly.
fn new_in_range(ref x in "(0|[1-9][0-9]*)") {
Expand All @@ -799,6 +843,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 felt values that are randomly generated each time tests are run, that the negative of a felt doesn't fall outside the range [0, p].
fn neg_in_range(ref x in "(0|[1-9][0-9]*)") {
let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -809,6 +854,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 {x} and {y} values that are randomly generated each time tests are run, that a subtraction between two felts {x} and {y} and doesn't fall outside the range [0, p]. The values of {x} and {y} can be either [0] or a very large number.
fn sub_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)") {
let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -821,6 +867,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 {x} and {y} values that are randomly generated each time tests are run, that a subtraction with assignment between two felts {x} and {y} and doesn't fall outside the range [0, p]. The values of {x} and {y} can be either [0] or a very large number.
fn sub_assign_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)") {
let mut x = Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -833,6 +880,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 {x} and {y} values that are randomly generated each time tests are run, that a multiplication between two felts {x} and {y} and doesn't fall outside the range [0, p]. The values of {x} and {y} can be either [0] or a very large number.
fn mul_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)") {
let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -845,6 +893,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 pairs of {x} and {y} values that are randomly generated each time tests are run, that a multiplication with assignment between two felts {x} and {y} and doesn't fall outside the range [0, p]. The values of {x} and {y} can be either [0] or a very large number.
fn mul_assign_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)") {
let mut x = Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -857,6 +906,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 pairs of {x} and {y} values that are randomly generated each time tests are run, that the result of the division of {x} by {y} is the inverse multiplicative of {x} --that is, multiplying the result by {y} returns the original number {x}. The values of {x} and {y} can be either [0] or a very large number.
fn div_is_mul_inv(ref x in "(0|[1-9][0-9]*)", ref y in "[1-9][0-9]*") {
let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -871,6 +921,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 {value}s that are randomly generated each time tests are run, that performing a bit shift to the left by {shift_amount} of bits (between 0 and 999) returns a result that is inside of the range [0, p].
fn shift_left_in_range(ref value in "(0|[1-9][0-9]*)", ref shift_amount in "[0-9]{1,3}"){
let value = Felt::parse_bytes(value.as_bytes(), 10).unwrap();
Expand All @@ -881,6 +932,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 {value}s that are randomly generated each time tests are run, that performing a bit shift to the right by {shift_amount} of bits (between 0 and 999) returns a result that is inside of the range [0, p].
fn shift_right_in_range(ref value in "(0|[1-9][0-9]*)", ref shift_amount in "[0-9]{1,3}"){
let value = Felt::parse_bytes(value.as_bytes(), 10).unwrap();
Expand All @@ -891,6 +943,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 {value}s that are randomly generated each time tests are run, that performing a bit shift to the right by {shift_amount} of bits (between 0 and 999), with assignment, returns a result that is inside of the range [0, p].
// "With assignment" means that the result of the operation is autommatically assigned to the variable value, replacing its previous content.
fn shift_right_assign_in_range(ref value in "(0|[1-9][0-9]*)", ref shift_amount in "[0-9]{1,3}"){
Expand All @@ -902,6 +955,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property based test that ensures, for 100 pairs of values {x} and {y} generated at random each time tests are run, that performing a BitAnd operation between them returns a result that is inside of the range [0, p].
fn bitand_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)"){
let x = Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -913,6 +967,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property based test that ensures, for 100 pairs of values {x} and {y} generated at random each time tests are run, that performing a BitOr operation between them returns a result that is inside of the range [0, p].
fn bitor_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)"){
let x = Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -923,6 +978,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property based test that ensures, for 100 pairs of values {x} and {y} generated at random each time tests are run, that performing a BitXor operation between them returns a result that is inside of the range [0, p].
fn bitxor_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "(0|[1-9][0-9]*)"){
let x = Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand All @@ -933,6 +989,7 @@ mod test {
}

#[test]
#[allow(deprecated)]
// Property-based test that ensures, for 100 values {x} that are randomly generated each time tests are run, that raising {x} to the {y}th power returns a result that is inside of the range [0, p].
fn pow_in_range(ref x in "(0|[1-9][0-9]*)", ref y in "[0-9]{1,2}"){
let base = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();
Expand Down
5 changes: 5 additions & 0 deletions src/hint_processor/builtin_hint_processor/math_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ pub fn sqrt(
return Err(HintError::ValueOutside250BitRange(mod_value.into_owned()));
//This is equal to mod_value > bigint!(2).pow(250)
}
#[allow(deprecated)]
insert_value_from_var_name(
"root",
Felt::new(isqrt(&mod_value.to_biguint())?),
Expand Down Expand Up @@ -421,8 +422,11 @@ pub fn signed_div_rem(
_ => {}
}

#[allow(deprecated)]
let int_value = value.to_bigint();
#[allow(deprecated)]
let int_div = div.to_bigint();
#[allow(deprecated)]
let int_bound = bound.to_bigint();
let (q, r) = int_value.div_mod_floor(&int_div);

Expand Down Expand Up @@ -533,6 +537,7 @@ pub fn assert_lt_felt(
fn div_prime_by_bound(bound: Felt) -> Result<Felt, VirtualMachineError> {
let prime = BigUint::from_str_radix(&PRIME_STR[2..], 16)
.map_err(|_| VirtualMachineError::CouldntParsePrime(PRIME_STR.to_string()))?;
#[allow(deprecated)]
let limit = prime / bound.to_biguint();
Ok(Felt::new(limit))
}
Expand Down
7 changes: 7 additions & 0 deletions src/hint_processor/builtin_hint_processor/secp/ec_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn ec_negate(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = num_bigint::BigInt::one().shl(256u32)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -73,6 +74,7 @@ pub fn compute_doubling_slope(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = num_bigint::BigInt::one().shl(256usize)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -125,6 +127,7 @@ pub fn compute_slope(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256usize)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -206,6 +209,7 @@ pub fn ec_double_assign_new_x(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256usize)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -256,6 +260,7 @@ pub fn ec_double_assign_new_y(
exec_scopes: &mut ExecutionScopes,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256usize)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -296,6 +301,7 @@ pub fn fast_ec_add_assign_new_x(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256usize)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -368,6 +374,7 @@ pub fn fast_ec_add_assign_new_y(
exec_scopes: &mut ExecutionScopes,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256usize)
- constants
.get(SECP_REM)
Expand Down
4 changes: 4 additions & 0 deletions src/hint_processor/builtin_hint_processor/secp/field_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub fn verify_zero(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256_u32)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -64,6 +65,7 @@ pub fn reduce(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = num_bigint::BigInt::one().shl(256_u32)
- constants
.get(SECP_REM)
Expand All @@ -90,6 +92,7 @@ pub fn is_zero_pack(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256_u32)
- constants
.get(SECP_REM)
Expand Down Expand Up @@ -138,6 +141,7 @@ pub fn is_zero_assign_scope_variables(
exec_scopes: &mut ExecutionScopes,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256_u32)
- constants
.get(SECP_REM)
Expand Down
5 changes: 5 additions & 0 deletions src/hint_processor/builtin_hint_processor/secp/secp_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub fn split(
integer: &num_bigint::BigUint,
constants: &HashMap<String, Felt>,
) -> Result<[num_bigint::BigUint; 3], HintError> {
#[allow(deprecated)]
let base_86_max = constants
.get(BASE_86)
.ok_or(HintError::MissingConstant(BASE_86))?
Expand Down Expand Up @@ -60,6 +61,7 @@ Note that the limbs do not have to be in the range [0, BASE).
pub fn pack(d0: &Felt, d1: &Felt, d2: &Felt) -> num_bigint::BigInt {
let unreduced_big_int_3 = vec![d0, d1, d2];

#[allow(deprecated)]
unreduced_big_int_3
.into_iter()
.enumerate()
Expand Down Expand Up @@ -104,19 +106,22 @@ mod tests {
constants.insert(BASE_86.to_string(), Felt::one() << 86_usize);

let array_1 = split(&BigUint::zero(), &constants);
#[allow(deprecated)]
let array_2 = split(
&bigint!(999992)
.to_biguint()
.expect("Couldn't convert to BigUint"),
&constants,
);
#[allow(deprecated)]
let array_3 = split(
&bigint_str!("7737125245533626718119526477371252455336267181195264773712524553362")
.to_biguint()
.expect("Couldn't convert to BigUint"),
&constants,
);
//TODO, Check SecpSplitutOfRange limit
#[allow(deprecated)]
let array_4 = split(
&bigint_str!(
"773712524553362671811952647737125245533626718119526477371252455336267181195264"
Expand Down
5 changes: 5 additions & 0 deletions src/hint_processor/builtin_hint_processor/secp/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn div_mod_n_packed_divmod(
let a = pack_from_var_name("a", vm, ids_data, ap_tracking)?;
let b = pack_from_var_name("b", vm, ids_data, ap_tracking)?;

#[allow(deprecated)]
let n = {
let base = constants
.get(BASE_86)
Expand Down Expand Up @@ -78,6 +79,7 @@ pub fn div_mod_n_safe_div(
let b = exec_scopes.get_ref::<BigInt>("b")?;
let res = exec_scopes.get_ref::<BigInt>("res")?;

#[allow(deprecated)]
let n = {
let base = constants
.get(BASE_86)
Expand Down Expand Up @@ -112,10 +114,12 @@ pub fn get_point_from_x(
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt>,
) -> Result<(), HintError> {
#[allow(deprecated)]
let beta = constants
.get(BETA)
.ok_or(HintError::MissingConstant(BETA))?
.to_bigint();
#[allow(deprecated)]
let secp_p = BigInt::one().shl(256_u32)
- constants
.get(SECP_REM)
Expand All @@ -129,6 +133,7 @@ pub fn get_point_from_x(
// Divide by 4
let mut y = y_cube_int.modpow(&(&secp_p + 1_u32).shr(2_u32), &secp_p);

#[allow(deprecated)]
let v = get_integer_from_var_name("v", vm, ids_data, ap_tracking)?.to_biguint();
if v.is_even() != y.is_even() {
y = &secp_p - y;
Expand Down
1 change: 1 addition & 0 deletions src/hint_processor/builtin_hint_processor/uint256_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub fn uint256_sqrt(
//ids.root.low = root
//ids.root.high = 0

#[allow(deprecated)]
let root = isqrt(&(&n_high.to_biguint().shl(128_u32) + n_low.to_biguint()))?;

if root >= num_bigint::BigUint::one().shl(128_u32) {
Expand Down
Loading

1 comment on commit 2410f6b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.

Benchmark suite Current: 2410f6b Previous: f16b420 Ratio
cairo_run(cairo_programs/benchmarks/compare_arrays_200000.json 968041705 ns/iter (± 20800624) 681611281 ns/iter (± 5869070) 1.42
cairo_run(cairo_programs/benchmarks/factorial_multirun.json 379306237 ns/iter (± 9424159) 265607385 ns/iter (± 2372789) 1.43
cairo_run(cairo_programs/benchmarks/fibonacci_1000_multirun.json 176145368 ns/iter (± 4653705) 122077899 ns/iter (± 1011782) 1.44
cairo_run(cairo_programs/benchmarks/linear_search.json 125024345 ns/iter (± 4587126) 83224186 ns/iter (± 842111) 1.50
cairo_run(cairo_programs/benchmarks/memory_integration_benchmark.json 622686847 ns/iter (± 14222445) 436725693 ns/iter (± 1664998) 1.43

This comment was automatically generated by workflow using github-action-benchmark.

CC: @unbalancedparentheses

Please sign in to comment.