Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
#### Upcoming Changes

#### [0.5.0] - 2023-6-7
* fix: fix overflow for `QUAD_BIT` and `DI_BIT` hints [#1209](https://github.com/lambdaclass/cairo-rs/pull/1209)
Fixes [#1205](https://github.com/lambdaclass/cairo-rs/issue/1205)

* fix: Fix hints `UINT256_UNSIGNED_DIV_REM` && `UINT256_EXPANDED_UNSIGNED_DIV_REM` [#1203](https://github.com/lambdaclass/cairo-rs/pull/1203)
* fix: fix hints `UINT256_UNSIGNED_DIV_REM` && `UINT256_EXPANDED_UNSIGNED_DIV_REM` [#1203](https://github.com/lambdaclass/cairo-rs/pull/1203)

* bugfix: Fix deserialization of scientific notation with fractional values [#1202](https://github.com/lambdaclass/cairo-rs/pull/1202)
* bugfix: fix deserialization of scientific notation with fractional values [#1202](https://github.com/lambdaclass/cairo-rs/pull/1202)

* feat: implement `mem_eq` function to test for equality of two ranges in memory [#1198](https://github.com/lambdaclass/cairo-rs/pull/1198)

Expand Down
80 changes: 72 additions & 8 deletions src/hint_processor/builtin_hint_processor/secp/ec_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
vm::{errors::hint_errors::HintError, vm_core::VirtualMachine},
};
use felt::Felt252;
use num_bigint::BigInt;
use num_bigint::{BigInt, BigUint};
use num_integer::Integer;

use num_traits::{One, ToPrimitive, Zero};
Expand Down Expand Up @@ -496,28 +496,45 @@ pub fn n_pair_bits(
// If m is too high the shift result will always be zero
let m = m_cow.as_ref().to_u32().unwrap_or(253);
if m >= 253 {
return insert_value_from_var_name("quad_bit", 0, vm, ids_data, ap_tracking);
return insert_value_from_var_name(result_name, 0, vm, ids_data, ap_tracking);
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

}
if m.is_zero() {
return Err(HintError::NPairBitsMZero);
}

let one = &Felt252::one();
let two = &Felt252::from(2);
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:
// 10
// ↓↓
// 1010101__ -> 101010110
let get_bit =
|x: &BigUint, i| m.checked_sub(i).map(|i| x.bit(i.into())).unwrap_or(false) as u32;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
|x: &BigUint, i| m.checked_sub(i).map(|i| x.bit(i.into())).unwrap_or(false) as u32;
|x: &BigUint, i| m.checked_sub(i).map(|j| x.bit(j.into())).unwrap_or(false) as u32;

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think your suggestion needs to change the closure parameter name too right?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. Both the closure and the closure inside the closure have a parameter named i. My suggestion is to rename the inner one to j to avoid variable shadowing.

let res: Felt252 = (0..number_of_pairs)
.map(|i| {
let bit_1 = (scalar_v >> (m - i - 1)) & two;
// This code is definitely verbose, but it's the only way I found to avoid a `panic`
// when `m < number_of_pairs` while still being correct and hopefully fast.
let bit_1 = get_bit(&scalar_v, i);
// 1 * ((ids.scalar_u >> ids.m) & 1)
let bit_0 = (scalar_u >> (m - i)) & one;
bit_0 + bit_1
let bit_0 = get_bit(&scalar_u, i);
bit_0 | (bit_1 << 1)
})
.fold(Felt252::zero(), |acc, x| (acc << 2_u32) + x);
.fold(BigUint::zero(), |mut acc, x| {
acc <<= 2_u32;
acc += x;
acc
})
.into();
/*
ids.quad_bit = (
8 * ((ids.scalar_v >> ids.m) & 1)
+ 4 * ((ids.scalar_u >> ids.m) & 1)
+ 2 * ((ids.scalar_v >> (ids.m - 1)) & 1)
+ ((ids.scalar_u >> (ids.m - 1)) & 1)
)
%{ ids.dibit = ((ids.scalar_u >> ids.m) & 1) + 2 * ((ids.scalar_v >> ids.m) & 1) %}
*/

insert_value_from_var_name(result_name, res, vm, ids_data, ap_tracking)
}
Expand Down Expand Up @@ -1311,6 +1328,29 @@ mod tests {
check_memory![vm.segments.memory, ((1, 3), 14)];
}

#[test]
fn run_quad_bit_for_m_1() {
let hint_code = hint_code::QUAD_BIT;
let mut vm = vm_with_range_check!();

let scalar_u = 89712;
let scalar_v = 1478396;
let m = 1;
// Insert ids.scalar into memory
vm.segments = segments![((1, 0), scalar_u), ((1, 1), scalar_v), ((1, 2), m)];

// Initialize RunContext
run_context!(vm, 0, 4, 4);

let ids_data = ids_data!["scalar_u", "scalar_v", "m", "quad_bit"];

// Execute the hint
assert_matches!(run_hint!(vm, ids_data, hint_code), Ok(()));

// Check hint memory inserts
check_memory![vm.segments.memory, ((1, 3), 0)];
}

#[test]
fn run_quad_bit_with_max_m_ok() {
let hint_code = hint_code::QUAD_BIT;
Expand Down Expand Up @@ -1358,6 +1398,30 @@ mod tests {
check_memory![vm.segments.memory, ((1, 3), 2)];
}

#[test]
fn run_di_bit_with_max_m_ok() {
let hint_code = hint_code::DI_BIT;
let mut vm = vm_with_range_check!();

let scalar_u = 89712;
let scalar_v = 1478396;
// Value is so high the result will always be zero
let m = i128::MAX;
// Insert ids.scalar into memory
vm.segments = segments![((1, 0), scalar_u), ((1, 1), scalar_v), ((1, 2), m)];

// Initialize RunContext
run_context!(vm, 0, 4, 4);

let ids_data = ids_data!["scalar_u", "scalar_v", "m", "dibit"];

// Execute the hint
assert_matches!(run_hint!(vm, ids_data, hint_code), Ok(()));

// Check hint memory inserts
check_memory![vm.segments.memory, ((1, 3), 0)];
}

#[test]
fn run_di_bit_m_zero() {
let hint_code = hint_code::DI_BIT;
Expand Down