Remove unnecessary sign variable from wnaf_const

Summary:
 * Fix test_constant_wnaf for -1 and add a test for it.

Before, test_constant_wnaf used scalar_cadd_bit to correct for the skew. But
this function does not correctly deal with overflows which is why num = -1
couldn't be tested.

This commit also adds tests for 0, 1/2 and 1/2-1 as they are corner cases
in constant_wnaf.

 * Remove unnecessary sign variable from wnaf_const

This is a backport of libsecp256k1 [[bitcoin-core/secp256k1#741 | PR741]]

Test Plan:
  ninja check-secp256k1

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D7595

Author: jonasnick

src/secp256k1/src/ecmult_const_impl.h

@@ -105,16 +105,22 @@ static int secp256k1_wnaf_const(int *wnaf, const secp256k1_scalar *scalar, int w
     /* 4 */
     u_last = secp256k1_scalar_shr_int(&s, w);
     do {
-        int sign;
         int even;
 
         /* 4.1 4.4 */
         u = secp256k1_scalar_shr_int(&s, w);
         /* 4.2 */
         even = ((u & 1) == 0);
-        sign = 2 * (u_last > 0) - 1;
-        u += sign * even;
-        u_last -= sign * even * (1 << w);
+        /* In contrast to the original algorithm, u_last is always > 0 and
+         * therefore we do not need to check its sign. In particular, it's easy
+         * to see that u_last is never < 0 because u is never < 0. Moreover,
+         * u_last is never = 0 because u is never even after a loop
+         * iteration. The same holds analogously for the initial value of
+         * u_last (in the first loop iteration). */
+        VERIFY_CHECK(u_last > 0);
+        VERIFY_CHECK((u_last & 1) == 1);
+        u += even;
+        u_last -= even * (1 << w);
 
         /* 4.3, adapted for global sign change */
         wnaf[word++] = u_last * global_sign;

src/secp256k1/src/tests.c

@@ -3231,6 +3231,7 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
     int skew;
     int bits = 256;
     secp256k1_scalar num = *number;
+    secp256k1_scalar scalar_skew;
 
     secp256k1_scalar_set_int(&x, 0);
     secp256k1_scalar_set_int(&shift, 1 << w);
@@ -3261,7 +3262,8 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
         secp256k1_scalar_add(&x, &x, &t);
     }
     /* Skew num because when encoding numbers as odd we use an offset */
-    secp256k1_scalar_cadd_bit(&num, skew == 2, 1);
+    secp256k1_scalar_set_int(&scalar_skew, 1 << (skew == 2));
+    secp256k1_scalar_add(&num, &num, &scalar_skew);
     CHECK(secp256k1_scalar_eq(&x, &num));
 }
 
@@ -3373,13 +3375,32 @@ void run_wnaf(void) {
     int i;
     secp256k1_scalar n = {{0}};
 
+    test_constant_wnaf(&n, 4);
     /* Sanity check: 1 and 2 are the smallest odd and even numbers and should
      *               have easier-to-diagnose failure modes  */
     n.d[0] = 1;
     test_constant_wnaf(&n, 4);
     n.d[0] = 2;
     test_constant_wnaf(&n, 4);
-    /* Test 0 */
+    /* Test -1, because it's a special case in wnaf_const */
+    n = secp256k1_scalar_one;
+    secp256k1_scalar_negate(&n, &n);
+    test_constant_wnaf(&n, 4);
+
+    /* Test -2, which may not lead to overflows in wnaf_const */
+    secp256k1_scalar_add(&n, &secp256k1_scalar_one, &secp256k1_scalar_one);
+    secp256k1_scalar_negate(&n, &n);
+    test_constant_wnaf(&n, 4);
+
+    /* Test (1/2) - 1 = 1/-2 and 1/2 = (1/-2) + 1
+       as corner cases of negation handling in wnaf_const */
+    secp256k1_scalar_inverse(&n, &n);
+    test_constant_wnaf(&n, 4);
+
+    secp256k1_scalar_add(&n, &n, &secp256k1_scalar_one);
+    test_constant_wnaf(&n, 4);
+
+    /* Test 0 for fixed wnaf */
     test_fixed_wnaf_small();
     /* Random tests */
     for (i = 0; i < count; i++) {