Repository: bitcoin-core/secp256k1
Pull request #401 (2016-06-30 10:02:25)

ecmult_const: unify endomorphism and non-endomorphism skew cases
We now do a skew correction even without the endomorphism optimization,
which costs one additional group addition but unifies a lot of code.

apoelstra: I think it can, e.g. if the scalar is low and odd.

Patch:
diff --git a/.gitignore b/.gitignore
index efb277d..e0b7b7a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,24 +25,17 @@ config.status
 libtool
 .deps/
 .dirstamp
+build-aux/
 *.lo
 *.o
 *~
 src/libsecp256k1-config.h
 src/libsecp256k1-config.h.in
 src/ecmult_static_context.h
-build-aux/config.guess
-build-aux/config.sub
-build-aux/depcomp
-build-aux/install-sh
-build-aux/ltmain.sh
-build-aux/m4/libtool.m4
-build-aux/m4/lt~obsolete.m4
-build-aux/m4/ltoptions.m4
-build-aux/m4/ltsugar.m4
-build-aux/m4/ltversion.m4
-build-aux/missing
-build-aux/compile
-build-aux/test-driver
+m4/libtool.m4
+m4/ltoptions.m4
+m4/ltsugar.m4
+m4/ltversion.m4
+m4/lt~obsolete.m4
 src/stamp-h1
 libsecp256k1.pc
diff --git a/configure.ac b/configure.ac
index 46ccf0b..47889d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -107,7 +107,7 @@ AC_ARG_ENABLE(endomorphism,
 AC_ARG_ENABLE(ecmult_static_precomputation,
     AS_HELP_STRING([--enable-ecmult-static-precomputation],[enable precomputed ecmult table for signing (default is yes)]),
     [use_ecmult_static_precomputation=$enableval],
-    [use_ecmult_static_precomputation=auto])
+    [use_ecmult_static_precomputation=yes])
 
 AC_ARG_ENABLE(module_ecdh,
     AS_HELP_STRING([--enable-module-ecdh],[enable ECDH shared secret computation (experimental)]),
@@ -149,34 +149,6 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([[void myfunc() {__builtin_expect(0,0);}]])],
     [ AC_MSG_RESULT([no])
     ])
 
-if test x"$use_ecmult_static_precomputation" != x"no"; then
-  save_cross_compiling=$cross_compiling
-  cross_compiling=no
-  TEMP_CC="$CC"
-  CC="$CC_FOR_BUILD"
-  AC_MSG_CHECKING([native compiler: ${CC_FOR_BUILD}])
-  AC_RUN_IFELSE(
-    [AC_LANG_PROGRAM([], [return 0])],
-    [working_native_cc=yes],
-    [working_native_cc=no],[dnl])
-  CC="$TEMP_CC"
-  cross_compiling=$save_cross_compiling
-
-  if test x"$working_native_cc" = x"no"; then
-    set_precomp=no
-    if test x"$use_ecmult_static_precomputation" = x"yes";  then
-      AC_MSG_ERROR([${CC_FOR_BUILD} does not produce working binaries. Please set CC_FOR_BUILD])
-    else
-      AC_MSG_RESULT([${CC_FOR_BUILD} does not produce working binaries. Please set CC_FOR_BUILD])
-    fi
-  else
-    AC_MSG_RESULT([ok])
-    set_precomp=yes
-  fi
-else
-  set_precomp=no
-fi
-
 if test x"$req_asm" = x"auto"; then
   SECP_64BIT_ASM_CHECK
   if test x"$has_64bit_asm" = x"yes"; then
@@ -421,7 +393,6 @@ if test x"$use_external_asm" = x"yes"; then
   AC_DEFINE(USE_EXTERNAL_ASM, 1, [Define this symbol if an external (non-inline) assembly implementation is used])
 fi
 
-AC_MSG_NOTICE([Using static precomputation: $set_precomp])
 AC_MSG_NOTICE([Using assembly optimizations: $set_asm])
 AC_MSG_NOTICE([Using field implementation: $set_field])
 AC_MSG_NOTICE([Using bignum implementation: $set_bignum])
diff --git a/src/ecmult_const_impl.h b/src/ecmult_const_impl.h
index 90ac947..7a6a253 100644
--- a/src/ecmult_const_impl.h
+++ b/src/ecmult_const_impl.h
@@ -58,22 +58,24 @@ static int secp256k1_wnaf_const(int *wnaf, secp256k1_scalar s, int w) {
     int global_sign;
     int skew = 0;
     int word = 0;
+
     /* 1 2 3 */
     int u_last;
     int u;
 
-#ifdef USE_ENDOMORPHISM
     int flip;
     int bit;
     secp256k1_scalar neg_s;
     int not_neg_one;
-    /* If we are using the endomorphism, we cannot handle even numbers by negating
-     * them, since we are working with 128-bit numbers whose negations would be 256
-     * bits, eliminating the performance advantage. Instead we use a technique from
+    /* Note that we cannot handle even numbers by negating them to be odd, as is
+     * done in other implementations, since if our scalars were specified to have
+     * width < 256 for performance reasons, their negations would have width 256
+     * and we'd lose any performance benefit. Instead, we use a technique from
      * Section 4.2 of the Okeya/Tagaki paper, which is to add either 1 (for even)
-     * or 2 (for odd) to the number we are encoding, then compensating after the
-     * multiplication. */
-    /* Negative 128-bit numbers will be negated, since otherwise they are 256-bit */
+     * or 2 (for odd) to the number we are encoding, returning a skew value indicating
+     * this, and having the caller compensate after doing the multiplication. */
+
+    /* Negative numbers will be negated to keep their bit representation below the maximum width */
     flip = secp256k1_scalar_is_high(&s);
     /* We add 1 to even numbers, 2 to odd ones, noting that negation flips parity */
     bit = flip ^ (s.d[0] & 1);
@@ -89,11 +91,6 @@ static int secp256k1_wnaf_const(int *wnaf, secp256k1_scalar s, int w) {
     global_sign = secp256k1_scalar_cond_negate(&s, flip);
     global_sign *= not_neg_one * 2 - 1;
     skew = 1 << bit;
-#else
-    /* Otherwise, we just negate to force oddness */
-    int is_even = secp256k1_scalar_is_even(&s);
-    global_sign = secp256k1_scalar_cond_negate(&s, is_even);
-#endif
 
     /* 4 */
     u_last = secp256k1_scalar_shr_int(&s, w);
@@ -127,15 +124,13 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
     secp256k1_ge tmpa;
     secp256k1_fe Z;
 
+    int skew_1;
+    int wnaf_1[1 + WNAF_SIZE(WINDOW_A - 1)];
 #ifdef USE_ENDOMORPHISM
     secp256k1_ge pre_a_lam[ECMULT_TABLE_SIZE(WINDOW_A)];
-    int wnaf_1[1 + WNAF_SIZE(WINDOW_A - 1)];
     int wnaf_lam[1 + WNAF_SIZE(WINDOW_A - 1)];
-    int skew_1;
     int skew_lam;
     secp256k1_scalar q_1, q_lam;
-#else
-    int wnaf[1 + WNAF_SIZE(WINDOW_A - 1)];
 #endif
 
     int i;
@@ -145,18 +140,10 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
 #ifdef USE_ENDOMORPHISM
     /* split q into q_1 and q_lam (where q = q_1 + q_lam*lambda, and q_1 and q_lam are ~128 bit) */
     secp256k1_scalar_split_lambda(&q_1, &q_lam, &sc);
-    /* no need for zero correction when using endomorphism since even
-     * numbers have one added to them anyway */
     skew_1   = secp256k1_wnaf_const(wnaf_1,   q_1,   WINDOW_A - 1);
     skew_lam = secp256k1_wnaf_const(wnaf_lam, q_lam, WINDOW_A - 1);
 #else
-    int is_zero = secp256k1_scalar_is_zero(scalar);
-    /* the wNAF ladder cannot handle zero, so bump this to one .. we will
-     * correct the result after the fact */
-    sc.d[0] += is_zero;
-    VERIFY_CHECK(!secp256k1_scalar_is_zero(&sc));
-
-    secp256k1_wnaf_const(wnaf, sc, WINDOW_A - 1);
+    skew_1   = secp256k1_wnaf_const(wnaf_1, sc, WINDOW_A - 1);
 #endif
 
     /* Calculate odd multiples of a.
@@ -179,21 +166,15 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
     /* first loop iteration (separated out so we can directly set r, rather
      * than having it start at infinity, get doubled several times, then have
      * its new value added to it) */
-#ifdef USE_ENDOMORPHISM
     i = wnaf_1[WNAF_SIZE(WINDOW_A - 1)];
     VERIFY_CHECK(i != 0);
     ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a, i, WINDOW_A);
     secp256k1_gej_set_ge(r, &tmpa);
-
+#ifdef USE_ENDOMORPHISM
     i = wnaf_lam[WNAF_SIZE(WINDOW_A - 1)];
     VERIFY_CHECK(i != 0);
     ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a_lam, i, WINDOW_A);
     secp256k1_gej_add_ge(r, r, &tmpa);
-#else
-    i = wnaf[WNAF_SIZE(WINDOW_A - 1)];
-    VERIFY_CHECK(i != 0);
-    ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a, i, WINDOW_A);
-    secp256k1_gej_set_ge(r, &tmpa);
 #endif
     /* remaining loop iterations */
     for (i = WNAF_SIZE(WINDOW_A - 1) - 1; i >= 0; i--) {
@@ -202,59 +183,57 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
         for (j = 0; j < WINDOW_A - 1; ++j) {
             secp256k1_gej_double_nonzero(r, r, NULL);
         }
-#ifdef USE_ENDOMORPHISM
+
         n = wnaf_1[i];
         ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a, n, WINDOW_A);
         VERIFY_CHECK(n != 0);
         secp256k1_gej_add_ge(r, r, &tmpa);
-
+#ifdef USE_ENDOMORPHISM
         n = wnaf_lam[i];
         ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a_lam, n, WINDOW_A);
         VERIFY_CHECK(n != 0);
         secp256k1_gej_add_ge(r, r, &tmpa);
-#else
-        n = wnaf[i];
-        VERIFY_CHECK(n != 0);
-        ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a, n, WINDOW_A);
-        secp256k1_gej_add_ge(r, r, &tmpa);
 #endif
     }
 
     secp256k1_fe_mul(&r->z, &r->z, &Z);
 
-#ifdef USE_ENDOMORPHISM
     {
         /* Correct for wNAF skew */
         secp256k1_ge correction = *a;
         secp256k1_ge_storage correction_1_stor;
+#ifdef USE_ENDOMORPHISM
         secp256k1_ge_storage correction_lam_stor;
+#endif
         secp256k1_ge_storage a2_stor;
         secp256k1_gej tmpj;
         secp256k1_gej_set_ge(&tmpj, &correction);
         secp256k1_gej_double_var(&tmpj, &tmpj, NULL);
         secp256k1_ge_set_gej(&correction, &tmpj);
         secp256k1_ge_to_storage(&correction_1_stor, a);
+#ifdef USE_ENDOMORPHISM
         secp256k1_ge_to_storage(&correction_lam_stor, a);
+#endif
         secp256k1_ge_to_storage(&a2_stor, &correction);
 
         /* For odd numbers this is 2a (so replace it), for even ones a (so no-op) */
         secp256k1_ge_storage_cmov(&correction_1_stor, &a2_stor, skew_1 == 2);
+#ifdef USE_ENDOMORPHISM
         secp256k1_ge_storage_cmov(&correction_lam_stor, &a2_stor, skew_lam == 2);
+#endif
 
         /* Apply the correction */
         secp256k1_ge_from_storage(&correction, &correction_1_stor);
         secp256k1_ge_neg(&correction, &correction);
         secp256k1_gej_add_ge(r, r, &correction);
 
+#ifdef USE_ENDOMORPHISM
         secp256k1_ge_from_storage(&correction, &correction_lam_stor);
         secp256k1_ge_neg(&correction, &correction);
         secp256k1_ge_mul_lambda(&correction, &correction);
         secp256k1_gej_add_ge(r, r, &correction);
-    }
-#else
-    /* correct for zero */
-    r->infinity |= is_zero;
 #endif
+    }
 }
 
 #endif
diff --git a/src/java/org/bitcoin/NativeSecp256k1.java b/src/java/org/bitcoin/NativeSecp256k1.java
index be67048..f4d85c0 100644
--- a/src/java/org/bitcoin/NativeSecp256k1.java
+++ b/src/java/org/bitcoin/NativeSecp256k1.java
@@ -1,20 +1,3 @@
-/*
- * Copyright 2013 Google Inc.
- * Copyright 2014-2016 the libsecp256k1 contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 package org.bitcoin;
 
 import java.nio.ByteBuffer;
@@ -27,15 +10,9 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
 import static org.bitcoin.NativeSecp256k1Util.*;
 
 /**
- * <p>This class holds native methods to handle ECDSA verification.</p>
- *
- * <p>You can find an example library that can be used for this at https://github.com/bitcoin/secp256k1</p>
- *
- * <p>To build secp256k1 for use with bitcoinj, run
- * `./configure --enable-jni --enable-experimental --enable-module-schnorr --enable-module-ecdh`
- * and `make` then copy `.libs/libsecp256k1.so` to your system library path
- * or point the JVM to the folder containing it with -Djava.library.path
- * </p>
+ * This class holds native methods to handle ECDSA verification.
+ * You can find an example library that can be used for this at
+ * https://github.com/sipa/secp256k1
  */
 public class NativeSecp256k1 {
 
diff --git a/src/java/org/bitcoin/NativeSecp256k1Util.java b/src/java/org/bitcoin/NativeSecp256k1Util.java
index 04732ba..d3f46f2 100644
--- a/src/java/org/bitcoin/NativeSecp256k1Util.java
+++ b/src/java/org/bitcoin/NativeSecp256k1Util.java
@@ -1,19 +1,3 @@
-/*
- * Copyright 2014-2016 the libsecp256k1 contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 package org.bitcoin;
 
 public class NativeSecp256k1Util{
diff --git a/src/java/org/bitcoin/Secp256k1Context.java b/src/java/org/bitcoin/Secp256k1Context.java
index 216c986..1f09185 100644
--- a/src/java/org/bitcoin/Secp256k1Context.java
+++ b/src/java/org/bitcoin/Secp256k1Context.java
@@ -1,24 +1,8 @@
-/*
- * Copyright 2014-2016 the libsecp256k1 contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 package org.bitcoin;
 
 /**
  * This class holds the context reference used in native methods 
- * to handle ECDSA operations.
+   to handle ECDSA operations.
  */
 public class Secp256k1Context {
   private static final boolean enabled; //true if the library is loaded


Test patch:
diff --git a/src/tests.c b/src/tests.c
index 1690c68..5300560 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -2399,9 +2399,7 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
     secp256k1_scalar x, shift;
     int wnaf[256] = {0};
     int i;
-#ifdef USE_ENDOMORPHISM
     int skew;
-#endif
     secp256k1_scalar num = *number;
 
     secp256k1_scalar_set_int(&x, 0);
@@ -2411,10 +2409,8 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
     for (i = 0; i < 16; ++i) {
         secp256k1_scalar_shr_int(&num, 8);
     }
-    skew = secp256k1_wnaf_const(wnaf, num, w);
-#else
-    secp256k1_wnaf_const(wnaf, num, w);
 #endif
+    skew = secp256k1_wnaf_const(wnaf, num, w);
 
     for (i = WNAF_SIZE(w); i >= 0; --i) {
         secp256k1_scalar t;
@@ -2433,10 +2429,8 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
         }
         secp256k1_scalar_add(&x, &x, &t);
     }
-#ifdef USE_ENDOMORPHISM
-    /* Skew num because when encoding 128-bit numbers as odd we use an offset */
+    /* Skew num because when encoding numbers as odd we use an offset */
     secp256k1_scalar_cadd_bit(&num, skew == 2, 1);
-#endif
     CHECK(secp256k1_scalar_eq(&x, &num));
 }