Fix bug in ec_point_from_x (#386)

* Add NonZero case to check_next_type

* stash

* Minimal solution

* Improve solution

* Try much easier solution

* Clean code

* Clippy + Fmt

* Unleash the tests

runtime/Cargo.toml

@@ -12,3 +12,4 @@ cairo-lang-runner = "=2.3"
 libc = "0.2"
 starknet-crypto = "0.6"
 starknet-curve = "0.4"
+lazy_static = "1.4"

runtime/src/lib.rs

@@ -1,9 +1,16 @@
 #![allow(non_snake_case)]
 
+use lazy_static::lazy_static;
 use starknet_crypto::FieldElement;
 use starknet_curve::AffinePoint;
 use starknet_types_core::felt::{felt_to_bigint, Felt};
 use std::{collections::HashMap, fs::File, io::Write, os::fd::FromRawFd, ptr::NonNull, slice};
+lazy_static! {
+    pub static ref HALF_PRIME: FieldElement = FieldElement::from_dec_str(
+        "1809251394333065606848661391547535052811553607665798349986546028067936010240"
+    )
+    .unwrap();
+}
 
 pub(crate) fn as_cairo_short_string(value: &Felt) -> Option<String> {
     let mut as_string = String::default();
@@ -235,7 +242,11 @@ pub unsafe extern "C" fn cairo_native__libfunc__ec__ec_point_from_x_nz(
     .unwrap();
 
     match AffinePoint::from_x(x) {
-        Some(point) => {
+        Some(mut point) => {
+            // If y > PRIME/ 2 use PRIME - y
+            if point.y >= *HALF_PRIME {
+                point.y = -point.y
+            }
             point_ptr.as_mut()[1].copy_from_slice(&point.y.to_bytes_be());
             point_ptr.as_mut()[1].reverse();
 

tests/common.rs

@@ -495,7 +495,15 @@ pub fn compare_outputs(
                 prop_assert_eq!(vm_value, native_value)
             }
             CoreTypeConcrete::Uint128MulGuarantee(_) => todo!(),
-            CoreTypeConcrete::NonZero(_) => todo!(),
+            CoreTypeConcrete::NonZero(info) => {
+                prop_assert!(native_rets.peek().is_some());
+                check_next_type(
+                    reg.get_type(&info.ty).expect("type should exist"),
+                    &mut [native_rets.next().unwrap()].into_iter(),
+                    vm_rets,
+                    reg,
+                )?;
+            }
             CoreTypeConcrete::Nullable(_) => todo!(),
             CoreTypeConcrete::RangeCheck(_) => {
                 // runner: ignore

tests/ec.rs

@@ -55,7 +55,6 @@ fn ec_point_zero() {
     .unwrap();
 }
 
-#[ignore = "TODO: possible bug in ec_point_from_x_nz"]
 #[test]
 fn ec_point_from_x_big() {
     let x = DeprecatedFelt::new(
@@ -82,7 +81,6 @@ fn ec_point_from_x_big() {
     .unwrap();
 }
 
-#[ignore = "TODO: Still have to implement NonZero type comparisons"]
 #[test]
 fn ec_point_from_x_small() {
     let x = DeprecatedFelt::new(BigUint::from_str("1234").unwrap());
@@ -125,7 +123,6 @@ proptest! {
         )?;
     }
 
-    #[ignore = "TODO: possible bug in ec_point_from_x_nz"]
     #[test]
     fn ec_point_from_x_proptest(a in any_felt()) {
         let program = &EC_POINT_FROM_X;