Skip to content

Commit 2bc7808

Browse files
committed
fix: overflow in n_pair_bits
Replace a substraction and shift-based logic by `.checked_sub` and finding the right bit to avoid a `panic`. Also fixed a bug where `m >= 253` would store the result with the wrong id for `DI_BIT` hint. Fixes #1205
1 parent 02f3c55 commit 2bc7808

File tree

1 file changed

+72
-8
lines changed
  • src/hint_processor/builtin_hint_processor/secp

1 file changed

+72
-8
lines changed

src/hint_processor/builtin_hint_processor/secp/ec_utils.rs

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::{
1919
vm::{errors::hint_errors::HintError, vm_core::VirtualMachine},
2020
};
2121
use felt::Felt252;
22-
use num_bigint::BigInt;
22+
use num_bigint::{BigInt, BigUint};
2323
use num_integer::Integer;
2424

2525
use num_traits::{One, ToPrimitive, Zero};
@@ -496,28 +496,45 @@ pub fn n_pair_bits(
496496
// If m is too high the shift result will always be zero
497497
let m = m_cow.as_ref().to_u32().unwrap_or(253);
498498
if m >= 253 {
499-
return insert_value_from_var_name("quad_bit", 0, vm, ids_data, ap_tracking);
499+
return insert_value_from_var_name(result_name, 0, vm, ids_data, ap_tracking);
500500
}
501501
if m.is_zero() {
502502
return Err(HintError::NPairBitsMZero);
503503
}
504504

505-
let one = &Felt252::one();
506-
let two = &Felt252::from(2);
505+
let (scalar_v, scalar_u) = (scalar_v.to_biguint(), scalar_u.to_biguint());
507506

508507
// Each step, fetches the bits in mth position for v and u,
509508
// and appends them to the accumulator. i.e:
510509
// 10
511510
// ↓↓
512511
// 1010101__ -> 101010110
512+
let get_bit =
513+
|x: &BigUint, i| m.checked_sub(i).map(|i| x.bit(i.into())).unwrap_or(false) as u32;
513514
let res: Felt252 = (0..number_of_pairs)
514515
.map(|i| {
515-
let bit_1 = (scalar_v >> (m - i - 1)) & two;
516+
// This code is definitely verbose, but it's the only way I found to avoid a `panic`
517+
// when `m < number_of_pairs` while still being correct and hopefully fast.
518+
let bit_1 = get_bit(&scalar_v, i);
516519
// 1 * ((ids.scalar_u >> ids.m) & 1)
517-
let bit_0 = (scalar_u >> (m - i)) & one;
518-
bit_0 + bit_1
520+
let bit_0 = get_bit(&scalar_u, i);
521+
bit_0 | (bit_1 << 1)
519522
})
520-
.fold(Felt252::zero(), |acc, x| (acc << 2_u32) + x);
523+
.fold(BigUint::zero(), |mut acc, x| {
524+
acc <<= 2_u32;
525+
acc += x;
526+
acc
527+
})
528+
.into();
529+
/*
530+
ids.quad_bit = (
531+
8 * ((ids.scalar_v >> ids.m) & 1)
532+
+ 4 * ((ids.scalar_u >> ids.m) & 1)
533+
+ 2 * ((ids.scalar_v >> (ids.m - 1)) & 1)
534+
+ ((ids.scalar_u >> (ids.m - 1)) & 1)
535+
)
536+
%{ ids.dibit = ((ids.scalar_u >> ids.m) & 1) + 2 * ((ids.scalar_v >> ids.m) & 1) %}
537+
*/
521538

522539
insert_value_from_var_name(result_name, res, vm, ids_data, ap_tracking)
523540
}
@@ -1311,6 +1328,29 @@ mod tests {
13111328
check_memory![vm.segments.memory, ((1, 3), 14)];
13121329
}
13131330

1331+
#[test]
1332+
fn run_quad_bit_for_m_1() {
1333+
let hint_code = hint_code::QUAD_BIT;
1334+
let mut vm = vm_with_range_check!();
1335+
1336+
let scalar_u = 89712;
1337+
let scalar_v = 1478396;
1338+
let m = 1;
1339+
// Insert ids.scalar into memory
1340+
vm.segments = segments![((1, 0), scalar_u), ((1, 1), scalar_v), ((1, 2), m)];
1341+
1342+
// Initialize RunContext
1343+
run_context!(vm, 0, 4, 4);
1344+
1345+
let ids_data = ids_data!["scalar_u", "scalar_v", "m", "quad_bit"];
1346+
1347+
// Execute the hint
1348+
assert_matches!(run_hint!(vm, ids_data, hint_code), Ok(()));
1349+
1350+
// Check hint memory inserts
1351+
check_memory![vm.segments.memory, ((1, 3), 0)];
1352+
}
1353+
13141354
#[test]
13151355
fn run_quad_bit_with_max_m_ok() {
13161356
let hint_code = hint_code::QUAD_BIT;
@@ -1358,6 +1398,30 @@ mod tests {
13581398
check_memory![vm.segments.memory, ((1, 3), 2)];
13591399
}
13601400

1401+
#[test]
1402+
fn run_di_bit_with_max_m_ok() {
1403+
let hint_code = hint_code::DI_BIT;
1404+
let mut vm = vm_with_range_check!();
1405+
1406+
let scalar_u = 89712;
1407+
let scalar_v = 1478396;
1408+
// Value is so high the result will always be zero
1409+
let m = i128::MAX;
1410+
// Insert ids.scalar into memory
1411+
vm.segments = segments![((1, 0), scalar_u), ((1, 1), scalar_v), ((1, 2), m)];
1412+
1413+
// Initialize RunContext
1414+
run_context!(vm, 0, 4, 4);
1415+
1416+
let ids_data = ids_data!["scalar_u", "scalar_v", "m", "dibit"];
1417+
1418+
// Execute the hint
1419+
assert_matches!(run_hint!(vm, ids_data, hint_code), Ok(()));
1420+
1421+
// Check hint memory inserts
1422+
check_memory![vm.segments.memory, ((1, 3), 0)];
1423+
}
1424+
13611425
#[test]
13621426
fn run_di_bit_m_zero() {
13631427
let hint_code = hint_code::DI_BIT;

0 commit comments

Comments
 (0)