@@ -319,11 +319,14 @@ impl<F: PrimeField> FieldElement<F> {
319319///
320320/// An i128 can represent values in the range [i128::MIN, i128::MAX], which corresponds
321321/// to field elements in [0, 2^127 - 1] (positive) and [p - 2^127, p - 1] (negative),
322- /// where p is the field modulus. Note that 2^127 itself cannot be represented as i128
323- /// since it is not in the negative range.
322+ /// where p is the field modulus. The positive value 2^127 does not fit (it exceeds
323+ /// i128::MAX), but the field element representing -2^127 (i.e. p - 2^127) does fit
324+ /// (it is i128::MIN).
324325pub fn fits_in_i128 ( & self ) -> bool {
325- let num_bits = u32:: min ( self . neg ( ) . num_bits ( ) , self . num_bits ( ) ) ;
326- num_bits <= 127 && self != & FieldElement :: from ( I128_SIGN_BOUNDARY )
326+ let neg = self . neg ( ) ;
327+ self . num_bits ( ) <= 127
328+ || neg. num_bits ( ) <= 127
329+ || self . neg ( ) == FieldElement :: from ( I128_SIGN_BOUNDARY )
327330 }
328331
329332 /// Returns None, if the string is not a canonical
@@ -445,7 +448,10 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
445448 // We can then differentiate positive from negative values by their MSB.
446449 if self . neg ( ) . num_bits ( ) < self . num_bits ( ) {
447450 let bytes = self . neg ( ) . to_be_bytes ( ) ;
448- i128:: from_be_bytes ( bytes[ 16 ..32 ] . try_into ( ) . unwrap ( ) ) . neg ( )
451+ // wrapping_neg handles i128::MIN: bytes of 2^127 decode to i128::MIN.
452+ // Because it fits in i128, we know the value is a valid i128 value
453+ // so using wrapping_neg() cannot not silently miss an overflow.
454+ i128:: from_be_bytes ( bytes[ 16 ..32 ] . try_into ( ) . unwrap ( ) ) . wrapping_neg ( )
449455 } else {
450456 let bytes = self . to_be_bytes ( ) ;
451457 i128:: from_be_bytes ( bytes[ 16 ..32 ] . try_into ( ) . unwrap ( ) )
@@ -716,19 +722,18 @@ mod tests {
716722 assert ! ( F :: from( 42_i128 ) . fits_in_i128( ) ) ;
717723 assert ! ( F :: from( i128 :: MAX ) . fits_in_i128( ) ) ;
718724
719- // Negative values that fit (except i128::MIN)
725+ // Negative values that fit
720726 assert ! ( F :: from( -1_i128 ) . fits_in_i128( ) ) ;
721727 assert ! ( F :: from( -42_i128 ) . fits_in_i128( ) ) ;
722728 assert ! ( F :: from( i128 :: MIN + 1 ) . fits_in_i128( ) ) ;
729+ assert ! ( F :: from( i128 :: MIN ) . fits_in_i128( ) ) ;
723730
724731 // Boundary: 2^127 - 1 fits (i128::MAX)
725732 assert ! ( F :: from( ( 1_u128 << 127 ) - 1 ) . fits_in_i128( ) ) ;
726733
727- // Boundary: 2^127 does NOT fit (exceeds i128::MAX, not negative)
728- // Note: This also means i128::MIN doesn't fit, as it converts to a field element
729- // that when interpreted as unsigned equals 2^127
734+ // Boundary: the positive field element 2^127 does NOT fit (exceeds i128::MAX).
735+ // This is distinct from F::from(i128::MIN), which is the field element p - 2^127.
730736 assert ! ( !F :: from( 1_u128 << 127 ) . fits_in_i128( ) ) ;
731- assert ! ( !F :: from( i128 :: MIN ) . fits_in_i128( ) ) ;
732737
733738 // Values that don't fit
734739 let too_large = F :: from ( u128:: MAX ) ;
@@ -759,18 +764,26 @@ mod tests {
759764 // Test boundary values
760765 assert_eq ! ( F :: from( -i128 :: MAX ) . to_i128( ) , -i128 :: MAX ) ;
761766 assert_eq ! ( F :: from( i128 :: MIN + 1 ) . to_i128( ) , i128 :: MIN + 1 ) ;
762-
763- // i128::MIN doesn't fit
767+ assert_eq ! ( F :: from( i128 :: MIN ) . to_i128( ) , i128 :: MIN ) ;
764768 }
765769
766770 #[ test]
767771 fn test_to_i128_roundtrip ( ) {
768772 type F = FieldElement < ark_bn254:: Fr > ;
769773
770774 // Test roundtrip for various values
771- // i128::MIN doesn't fit
772- let test_values =
773- vec ! [ 0_i128 , 1 , -1 , 42 , -42 , i128 :: MAX , i128 :: MAX - 1 , i128 :: MIN + 1 , -i128 :: MAX ] ;
775+ let test_values = vec ! [
776+ 0_i128 ,
777+ 1 ,
778+ -1 ,
779+ 42 ,
780+ -42 ,
781+ i128 :: MAX ,
782+ i128 :: MAX - 1 ,
783+ i128 :: MIN ,
784+ i128 :: MIN + 1 ,
785+ -i128 :: MAX ,
786+ ] ;
774787
775788 for value in test_values {
776789 let field = F :: from ( value) ;
@@ -802,7 +815,6 @@ mod tests {
802815 #[ test]
803816 fn test_try_into_i128 ( ) {
804817 type F = FieldElement < ark_bn254:: Fr > ;
805-
806818 // Valid positive conversions
807819 assert_eq ! ( F :: zero( ) . try_into_i128( ) , Some ( 0 ) ) ;
808820 assert_eq ! ( F :: from( 42_i128 ) . try_into_i128( ) , Some ( 42 ) ) ;
@@ -816,15 +828,16 @@ mod tests {
816828 assert_eq ! ( F :: from( i128 :: MAX - 1 ) . try_into_i128( ) , Some ( i128 :: MAX - 1 ) ) ;
817829 assert_eq ! ( F :: from( 1_i128 << 126 ) . try_into_i128( ) , Some ( 1_i128 << 126 ) ) ;
818830 assert_eq ! ( F :: from( -( ( 1_i128 << 126 ) - 1 ) ) . try_into_i128( ) , Some ( -( ( 1_i128 << 126 ) - 1 ) ) ) ;
831+ // i128::MIN (= -2^127) fits: its field representation is p - 2^127, which is
832+ // the same field element as F::from(1_u128 << 127).neg().
833+ assert_eq ! ( F :: from( i128 :: MIN ) . try_into_i128( ) , Some ( i128 :: MIN ) ) ;
834+ assert_eq ! ( F :: from( 1_u128 << 127 ) . neg( ) . try_into_i128( ) , Some ( i128 :: MIN ) ) ;
819835 // Invalid conversions
820836 assert_eq ! ( F :: from( 1_u128 << 127 ) . try_into_i128( ) , None ) ;
821837 assert_eq ! ( F :: from( u128 :: MAX ) . try_into_i128( ) , None ) ;
822- // i128::MIN doesn't fit due to implementation
823- assert_eq ! ( F :: from( i128 :: MIN ) . try_into_i128( ) , None ) ;
824838 // A few other invalid values
825839 assert_eq ! ( F :: from( ( 1_u128 << 127 ) + 1 ) . try_into_i128( ) , None ) ;
826840 assert_eq ! ( F :: from( ( 1_u128 << 127 ) + 1000 ) . try_into_i128( ) , None ) ;
827- assert_eq ! ( F :: from( 1_u128 << 127 ) . neg( ) . try_into_i128( ) , None ) ;
828841 assert_eq ! ( F :: from( ( 1_u128 << 127 ) + 1 ) . neg( ) . try_into_i128( ) , None ) ;
829842 assert_eq ! ( F :: from( ( 1_u128 << 127 ) + 100 ) . try_into_i128( ) , None ) ;
830843 assert_eq ! ( F :: from( ( 1_u128 << 127 ) + 100 ) . neg( ) . try_into_i128( ) , None ) ;
0 commit comments