From 1bfeae2cb2abd79358082c51f08c35e213f2805e Mon Sep 17 00:00:00 2001 From: TomerStarkware Date: Thu, 27 Aug 2026 16:22:10 +0300 Subject: [PATCH] fix(felt252): canonicalize non-canonical felt252_const literals felt_to_unsigned only mapped negatives to PRIME - |v| and left positives untouched, so felt252_const (or any literal >= PRIME, or a negative with |v| > PRIME) produced a non-canonical i252 bit pattern. felt252_is_zero and other raw-bit comparisons then diverged from the VM (const took the non-zero branch). Reduce mod PRIME in both arms so the result is always in [0, PRIME). Adds a unit test for felt_to_unsigned and a hand-written Sierra regression test for felt252_const -> felt252_is_zero. Co-Authored-By: Claude Fable 5 --- src/libfuncs/felt252.rs | 63 +++++++++++++++++++++++++++++++++++++++++ src/utils.rs | 34 +++++++++++++++++++--- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/src/libfuncs/felt252.rs b/src/libfuncs/felt252.rs index 612dc3456..bb2a45be6 100644 --- a/src/libfuncs/felt252.rs +++ b/src/libfuncs/felt252.rs @@ -263,6 +263,69 @@ pub mod test { Felt::from_dec_str(val).unwrap() } + /// `felt252_const` accepts any integer literal, including values outside `[0, PRIME)`. + /// Those must be canonicalized so that bit-pattern comparisons (`felt252_is_zero`) agree + /// with the field semantics the VM uses. + #[test] + fn felt252_const_non_canonical_is_zero() { + use crate::{context::NativeContext, executor::JitNativeExecutor, utils::PRIME, OptLevel}; + use cairo_lang_sierra::ProgramParser; + use num_bigint::BigInt; + + let prime = BigInt::from(PRIME.clone()); + let cases: [(BigInt, u32); 6] = [ + (BigInt::from(0), 111), + (BigInt::from(5), 222), + (prime.clone(), 111), + (&prime + 5, 222), + (-&prime, 111), + (-&prime * 2 - 7, 222), + ]; + + for (c, expected) in cases { + let program = ProgramParser::new() + .parse(&format!( + r#" + type felt252 = felt252; + type NonZero = NonZero; + + libfunc felt252_const<{c}> = felt252_const<{c}>; + libfunc felt252_const<111> = felt252_const<111>; + libfunc felt252_const<222> = felt252_const<222>; + libfunc felt252_is_zero = felt252_is_zero; + libfunc drop> = drop>; + libfunc branch_align = branch_align; + + felt252_const<{c}>() -> ([0]); + felt252_is_zero([0]) {{ fallthrough() 5([1]) }}; + branch_align() -> (); + felt252_const<111>() -> ([2]); + return([2]); + branch_align() -> (); + drop>([1]) -> (); + felt252_const<222>() -> ([3]); + return([3]); + + [0]@0() -> (felt252); + "# + )) + .unwrap(); + + let context = NativeContext::new(); + let module = context.compile(&program, false, None, None).unwrap(); + let executor = JitNativeExecutor::from_native_module(module, OptLevel::None).unwrap(); + let result = executor + .invoke_dynamic(&program.funcs[0].id, &[], None) + .unwrap(); + + assert_eq!( + result.return_value, + Value::Felt252(Felt::from(expected)), + "felt252_const<{c}> -> felt252_is_zero took the wrong branch" + ); + } + } + #[test] fn felt252_add() { let program = &get_compiled_program("programs/libfuncs/felt252_add"); diff --git a/src/utils.rs b/src/utils.rs index 49c1d2adc..f2881ef0b 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -242,13 +242,18 @@ pub fn find_function_id<'a>(program: &'a Program, function_name: &str) -> Option .map(|func| &func.id) } -/// Normalize a signed BigInt felt value to its unsigned field representation. +/// Normalize a signed BigInt felt value to its canonical unsigned field representation +/// in `[0, PRIME)`. /// -/// Negative values are mapped to `PRIME - |value|`. +/// The value is reduced modulo `PRIME`, so out-of-range literals (e.g. `PRIME` itself or +/// large negatives, which Sierra permits in `felt252_const`) produce the same bit pattern +/// as their canonical equivalents. This matters because `felt252_is_zero` and friends compare +/// the raw i252 bits. pub fn felt_to_unsigned(value: &BigInt) -> BigUint { + let reduced = value.magnitude() % &*PRIME; match value.sign() { - Sign::Minus => &*PRIME - value.magnitude(), - _ => value.magnitude().clone(), + Sign::Minus if reduced != BigUint::ZERO => &*PRIME - reduced, + _ => reduced, } } @@ -444,6 +449,27 @@ mod tests { program::{FunctionSignature, GenFunction, Program, StatementIdx}, }; + // ============================== + // == TESTS: felt_to_unsigned + // ============================== + #[test] + fn test_felt_to_unsigned_canonicalizes() { + use super::{felt_to_unsigned, PRIME}; + use num_bigint::{BigInt, BigUint}; + + let prime = BigInt::from(PRIME.clone()); + assert_eq!(felt_to_unsigned(&BigInt::from(0)), BigUint::ZERO); + assert_eq!(felt_to_unsigned(&BigInt::from(5)), BigUint::from(5u8)); + assert_eq!(felt_to_unsigned(&BigInt::from(-1)), &*PRIME - 1u8); + // Values >= PRIME wrap around. + assert_eq!(felt_to_unsigned(&prime), BigUint::ZERO); + assert_eq!(felt_to_unsigned(&(&prime + 5)), BigUint::from(5u8)); + // Large negatives wrap around too; -PRIME is zero, not PRIME. + assert_eq!(felt_to_unsigned(&(-&prime)), BigUint::ZERO); + assert_eq!(felt_to_unsigned(&(-&prime - 1)), &*PRIME - 1u8); + assert_eq!(felt_to_unsigned(&(-&prime * 2 + 3)), BigUint::from(3u8)); + } + // ============================== // == TESTS: get_integer_layout // ==============================