From 646662517fb39afa90788392463b1043fd960c35 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter.wuille@gmail.com>
Date: Mon, 2 Nov 2015 01:42:53 +0100
Subject: [PATCH] Improvements for coordinate decompression

---
 src/field.h      |  8 +++---
 src/field_impl.h |  9 +++++++
 src/group.h      |  6 +++++
 src/group_impl.h |  9 +++++--
 src/tests.c      | 67 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/src/field.h b/src/field.h
index b49912d2560..2d52af5e366 100644
--- a/src/field.h
+++ b/src/field.h
@@ -87,9 +87,11 @@ static void secp256k1_fe_mul(secp256k1_fe *r, const secp256k1_fe *a, const secp2
  *  The output magnitude is 1 (but not guaranteed to be normalized). */
 static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a);
 
-/** Sets a field element to be the (modular) square root (if any exist) of another. Requires the
- *  input's magnitude to be at most 8. The output magnitude is 1 (but not guaranteed to be
- *  normalized). Return value indicates whether a square root was found. */
+/** If a has a square root, it is computed in r and 1 is returned. If a does not
+ *  have a square root, the root of its negation is computed and 0 is returned.
+ *  The input's magnitude can be at most 8. The output magnitude is 1 (but not
+ *  guaranteed to be normalized). The result in r will always be a square
+ *  itself. */
 static int secp256k1_fe_sqrt_var(secp256k1_fe *r, const secp256k1_fe *a);
 
 /** Sets a field element to be the (modular) inverse of another. Requires the input's magnitude to be
diff --git a/src/field_impl.h b/src/field_impl.h
index 5781167d4e4..77f4aae2f97 100644
--- a/src/field_impl.h
+++ b/src/field_impl.h
@@ -29,6 +29,15 @@ SECP256K1_INLINE static int secp256k1_fe_equal_var(const secp256k1_fe *a, const
 }
 
 static int secp256k1_fe_sqrt_var(secp256k1_fe *r, const secp256k1_fe *a) {
+    /** Given that p is congruent to 3 mod 4, we can compute the square root of
+     *  a mod p as the (p+1)/4'th power of a.
+     *
+     *  As (p+1)/4 is an even number, it will have the same result for a and for
+     *  (-a). Only one of these two numbers actually has a square root however,
+     *  so we test at the end by squaring and comparing to the input.
+     *  Also because (p+1)/4 is an even number, the computed square root is
+     *  itself always a square (a ** ((p+1)/4) is the square of a ** ((p+1)/8)).
+     */
     secp256k1_fe x2, x3, x6, x9, x11, x22, x44, x88, x176, x220, x223, t1;
     int j;
 
diff --git a/src/group.h b/src/group.h
index 3310af9a71a..ebfe1ca70cc 100644
--- a/src/group.h
+++ b/src/group.h
@@ -43,6 +43,12 @@ typedef struct {
 /** Set a group element equal to the point with given X and Y coordinates */
 static void secp256k1_ge_set_xy(secp256k1_ge *r, const secp256k1_fe *x, const secp256k1_fe *y);
 
+/** Set a group element (affine) equal to the point with the given X coordinate
+ *  and a Y coordinate that is a quadratic residue modulo p. The return value
+ *  is true iff a coordinate with the given X coordinate exists.
+ */
+static int secp256k1_ge_set_xquad_var(secp256k1_ge *r, const secp256k1_fe *x);
+
 /** Set a group element (affine) equal to the point with the given X coordinate, and given oddness
  *  for Y. Return value indicates whether the result is valid. */
 static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd);
diff --git a/src/group_impl.h b/src/group_impl.h
index bb999df3e69..42e2f6e6ebf 100644
--- a/src/group_impl.h
+++ b/src/group_impl.h
@@ -165,7 +165,7 @@ static void secp256k1_ge_clear(secp256k1_ge *r) {
     secp256k1_fe_clear(&r->y);
 }
 
-static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd) {
+static int secp256k1_ge_set_xquad_var(secp256k1_ge *r, const secp256k1_fe *x) {
     secp256k1_fe x2, x3, c;
     r->x = *x;
     secp256k1_fe_sqr(&x2, x);
@@ -173,7 +173,11 @@ static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int o
     r->infinity = 0;
     secp256k1_fe_set_int(&c, 7);
     secp256k1_fe_add(&c, &x3);
-    if (!secp256k1_fe_sqrt_var(&r->y, &c)) {
+    return secp256k1_fe_sqrt_var(&r->y, &c);
+}
+
+static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd) {
+    if (!secp256k1_ge_set_xquad_var(r, x)) {
         return 0;
     }
     secp256k1_fe_normalize_var(&r->y);
@@ -181,6 +185,7 @@ static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int o
         secp256k1_fe_negate(&r->y, &r->y, 1);
     }
     return 1;
+
 }
 
 static void secp256k1_gej_set_ge(secp256k1_gej *r, const secp256k1_ge *a) {
diff --git a/src/tests.c b/src/tests.c
index 7409453c17d..687a5f2fdd8 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -1420,6 +1420,16 @@ void random_fe(secp256k1_fe *x) {
     } while(1);
 }
 
+void random_fe_test(secp256k1_fe *x) {
+    unsigned char bin[32];
+    do {
+        secp256k1_rand256_test(bin);
+        if (secp256k1_fe_set_b32(x, bin)) {
+            return;
+        }
+    } while(1);
+}
+
 void random_fe_non_zero(secp256k1_fe *nz) {
     int tries = 10;
     while (--tries >= 0) {
@@ -2038,6 +2048,62 @@ void run_ec_combine(void) {
     }
 }
 
+void test_group_decompress(const secp256k1_fe* x) {
+    /* The input itself, normalized. */
+    secp256k1_fe fex = *x;
+    secp256k1_fe tmp;
+    /* Results of set_xquad_var, set_xo_var(..., 0), set_xo_var(..., 1). */
+    secp256k1_ge ge_quad, ge_even, ge_odd;
+    /* Return values of the above calls. */
+    int res_quad, res_even, res_odd;
+
+    secp256k1_fe_normalize_var(&fex);
+
+    res_quad = secp256k1_ge_set_xquad_var(&ge_quad, &fex);
+    res_even = secp256k1_ge_set_xo_var(&ge_even, &fex, 0);
+    res_odd = secp256k1_ge_set_xo_var(&ge_odd, &fex, 1);
+
+    CHECK(res_quad == res_even);
+    CHECK(res_quad == res_odd);
+
+    if (res_quad) {
+        secp256k1_fe_normalize_var(&ge_quad.x);
+        secp256k1_fe_normalize_var(&ge_odd.x);
+        secp256k1_fe_normalize_var(&ge_even.x);
+        secp256k1_fe_normalize_var(&ge_quad.y);
+        secp256k1_fe_normalize_var(&ge_odd.y);
+        secp256k1_fe_normalize_var(&ge_even.y);
+
+        /* No infinity allowed. */
+        CHECK(!ge_quad.infinity);
+        CHECK(!ge_even.infinity);
+        CHECK(!ge_odd.infinity);
+
+        /* Check that the x coordinates check out. */
+        CHECK(secp256k1_fe_equal_var(&ge_quad.x, x));
+        CHECK(secp256k1_fe_equal_var(&ge_even.x, x));
+        CHECK(secp256k1_fe_equal_var(&ge_odd.x, x));
+
+        /* Check that the Y coordinate result in ge_quad is a square. */
+        CHECK(secp256k1_fe_sqrt_var(&tmp, &ge_quad.y));
+        secp256k1_fe_sqr(&tmp, &tmp);
+        CHECK(secp256k1_fe_equal_var(&tmp, &ge_quad.y));
+
+        /* Check odd/even Y in ge_odd, ge_even. */
+        CHECK(secp256k1_fe_is_odd(&ge_odd.y));
+        CHECK(!secp256k1_fe_is_odd(&ge_even.y));
+    }
+}
+
+void run_group_decompress(void) {
+    int i;
+    for (i = 0; i < count * 4; i++) {
+        secp256k1_fe fe;
+        random_fe_test(&fe);
+        test_group_decompress(&fe);
+    }
+}
+
 /***** ECMULT TESTS *****/
 
 void run_ecmult_chain(void) {
@@ -4259,6 +4325,7 @@ int main(int argc, char **argv) {
 
     /* group tests */
     run_ge();
+    run_group_decompress();
 
     /* ecmult tests */
     run_wnaf();