Repository: bitcoin-core/secp256k1
Pull request #787 (2020-08-07 14:27:17)

Use preprocessor macros instead of autoconf to detect endianness
This does not fix any particular issue but it's preferable to not
rely on autoconf. This avoids endianness mess for users on BE hosts
if they use their build without autoconf.

The macros are carefully written to err on the side of the caution,
e.g., we #error if the user manually configures a different endianness
than what we detect.

Supersedes #770 .

gmaxwell: I like this approach.
gmaxwell: Travis has a zero log apparently unexplained failure on s390 ... the only test that really mattered for this pr.  :)

I tested it by hand on s390 and it works. sooo ?!
sipa: I checked this PR a few hours ago, and all tests had finished, except the s390x one, which hadn't started.

So I think there may be some provisioning issue that prevents travis from assigning an s390x worker, and it somehow timed out?
JeremyRubin: This is also the case on Bitcoin Core right now too :/
real-or-random: Yeah, first I felt brave when I squashed https://github.com/bitcoin-core/secp256k1/pull/696 and then merged it immediately. And then I got a notification from Travis that I broke master... :/ But yes, it's an issue on their side...
gmaxwell: ACK
sipa: Opened a PR for Bitcoin Core https://github.com/bitcoin/bitcoin/pull/19695 to test if this may impact any of the systems tested by its CI (which includes an MSVC based one).
gmaxwell: It's _MSC_VER not _MSVC_VER  (thanks to sipa abusing bitcoin's CI setup to test this.)

Patch:
diff --git a/.travis.yml b/.travis.yml
index 491da6c..a6ad6fb 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -83,10 +83,6 @@ matrix:
             - valgrind
             - libtool-bin
             - libc6-dbg:i386
-    # S390x build (big endian system)
-    - compiler: gcc
-      env: HOST=s390x-unknown-linux-gnu ECDH=yes RECOVERY=yes EXPERIMENTAL=yes CTIMETEST=
-      arch: s390x
 
 # We use this to install macOS dependencies instead of the built in `homebrew` plugin,
 # because in xcode earlier than 11 they have a bug requiring updating the system which overall takes ~8 minutes.
diff --git a/Makefile.am b/Makefile.am
index a30a84c..d8c1c79 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -99,7 +99,7 @@ if VALGRIND_ENABLED
 tests_CPPFLAGS += -DVALGRIND
 noinst_PROGRAMS += valgrind_ctime_test
 valgrind_ctime_test_SOURCES = src/valgrind_ctime_test.c
-valgrind_ctime_test_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_LIBS) $(COMMON_LIB)
+valgrind_ctime_test_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
 endif
 if !ENABLE_COVERAGE
 tests_CPPFLAGS += -DVERIFY
diff --git a/configure.ac b/configure.ac
index 6021b76..2fd340a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -493,8 +493,6 @@ if test x"$enable_module_recovery" = x"yes"; then
   AC_DEFINE(ENABLE_MODULE_RECOVERY, 1, [Define this symbol to enable the ECDSA pubkey recovery module])
 fi
 
-AC_C_BIGENDIAN()
-
 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
diff --git a/contrib/travis.sh b/contrib/travis.sh
index 2c0cbcd..3909d16 100755
--- a/contrib/travis.sh
+++ b/contrib/travis.sh
@@ -3,6 +3,10 @@
 set -e
 set -x
 
+if [ -n "$HOST" ]
+then
+    export USE_HOST="--host=$HOST"
+fi
 if [ "$HOST" = "i686-linux-gnu" ]
 then
     export CC="$CC -m32"
@@ -16,8 +20,7 @@ fi
     --enable-experimental="$EXPERIMENTAL" --enable-endomorphism="$ENDOMORPHISM" \
     --with-field="$FIELD" --with-bignum="$BIGNUM" --with-asm="$ASM" --with-scalar="$SCALAR" \
     --enable-ecmult-static-precomputation="$STATICPRECOMPUTATION" --with-ecmult-gen-precision="$ECMULTGENPRECISION" \
-    --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \
-    --host="$HOST" $EXTRAFLAGS
+    --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" "$EXTRAFLAGS" "$USE_HOST"
 
 if [ -n "$BUILD" ]
 then
diff --git a/include/secp256k1.h b/include/secp256k1.h
index 2178c8e..2ba2dca 100644
--- a/include/secp256k1.h
+++ b/include/secp256k1.h
@@ -134,7 +134,7 @@ typedef int (*secp256k1_nonce_function)(
 #  else
 #   define SECP256K1_API
 #  endif
-# elif defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD)
+# elif defined(__GNUC__) && defined(SECP256K1_BUILD)
 #  define SECP256K1_API __attribute__ ((visibility ("default")))
 # else
 #  define SECP256K1_API
diff --git a/src/basic-config.h b/src/basic-config.h
index b5ccd83..e9be39d 100644
--- a/src/basic-config.h
+++ b/src/basic-config.h
@@ -25,7 +25,6 @@
 #undef USE_SCALAR_INV_BUILTIN
 #undef USE_SCALAR_INV_NUM
 #undef ECMULT_WINDOW_SIZE
-#undef HAVE___INT128 /* used in util.h */
 
 #define USE_NUM_NONE 1
 #define USE_FIELD_INV_BUILTIN 1
diff --git a/src/ecmult_const_impl.h b/src/ecmult_const_impl.h
index 55b61e4..6d6d354 100644
--- a/src/ecmult_const_impl.h
+++ b/src/ecmult_const_impl.h
@@ -105,22 +105,16 @@ 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);
-        /* 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);
+        sign = 2 * (u_last > 0) - 1;
+        u += sign * even;
+        u_last -= sign * even * (1 << w);
 
         /* 4.3, adapted for global sign change */
         wnaf[word++] = u_last * global_sign;
@@ -208,7 +202,7 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
         int n;
         int j;
         for (j = 0; j < WINDOW_A - 1; ++j) {
-            secp256k1_gej_double(r, r);
+            secp256k1_gej_double_nonzero(r, r);
         }
 
         n = wnaf_1[i];
diff --git a/src/group.h b/src/group.h
index 6185be0..863644f 100644
--- a/src/group.h
+++ b/src/group.h
@@ -95,8 +95,8 @@ static int secp256k1_gej_is_infinity(const secp256k1_gej *a);
 /** Check whether a group element's y coordinate is a quadratic residue. */
 static int secp256k1_gej_has_quad_y_var(const secp256k1_gej *a);
 
-/** Set r equal to the double of a. Constant time. */
-static void secp256k1_gej_double(secp256k1_gej *r, const secp256k1_gej *a);
+/** Set r equal to the double of a, a cannot be infinity. Constant time. */
+static void secp256k1_gej_double_nonzero(secp256k1_gej *r, const secp256k1_gej *a);
 
 /** Set r equal to the double of a. If rzr is not-NULL this sets *rzr such that r->z == a->z * *rzr (where infinity means an implicit z = 0). */
 static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, secp256k1_fe *rzr);
diff --git a/src/group_impl.h b/src/group_impl.h
index fbfd348..43b039b 100644
--- a/src/group_impl.h
+++ b/src/group_impl.h
@@ -303,7 +303,7 @@ static int secp256k1_ge_is_valid_var(const secp256k1_ge *a) {
     return secp256k1_fe_equal_var(&y2, &x3);
 }
 
-static SECP256K1_INLINE void secp256k1_gej_double(secp256k1_gej *r, const secp256k1_gej *a) {
+static SECP256K1_INLINE void secp256k1_gej_double_nonzero(secp256k1_gej *r, const secp256k1_gej *a) {
     /* Operations: 3 mul, 4 sqr, 0 normalize, 12 mul_int/add/negate.
      *
      * Note that there is an implementation described at
@@ -313,7 +313,8 @@ static SECP256K1_INLINE void secp256k1_gej_double(secp256k1_gej *r, const secp25
      */
     secp256k1_fe t1,t2,t3,t4;
 
-    r->infinity = a->infinity;
+    VERIFY_CHECK(!secp256k1_gej_is_infinity(a));
+    r->infinity = 0;
 
     secp256k1_fe_mul(&r->z, &a->z, &a->y);
     secp256k1_fe_mul_int(&r->z, 2);       /* Z' = 2*Y*Z (2) */
@@ -362,7 +363,7 @@ static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, s
         secp256k1_fe_mul_int(rzr, 2);
     }
 
-    secp256k1_gej_double(r, a);
+    secp256k1_gej_double_nonzero(r, a);
 }
 
 static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_gej *b, secp256k1_fe *rzr) {
diff --git a/src/hash_impl.h b/src/hash_impl.h
index 782f972..1985a07 100644
--- a/src/hash_impl.h
+++ b/src/hash_impl.h
@@ -8,6 +8,7 @@
 #define SECP256K1_HASH_IMPL_H
 
 #include "hash.h"
+#include "util.h"
 
 #include <stdlib.h>
 #include <stdint.h>
@@ -27,9 +28,9 @@
     (h) = t1 + t2; \
 } while(0)
 
-#ifdef WORDS_BIGENDIAN
+#if defined(SECP256K1_BIG_ENDIAN)
 #define BE32(x) (x)
-#else
+#elif defined(SECP256K1_LITTLE_ENDIAN)
 #define BE32(p) ((((p) & 0xFF) << 24) | (((p) & 0xFF00) << 8) | (((p) & 0xFF0000) >> 8) | (((p) & 0xFF000000) >> 24))
 #endif
 
diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h
index 7f39927..8f539c4 100644
--- a/src/scalar_4x64_impl.h
+++ b/src/scalar_4x64_impl.h
@@ -192,9 +192,9 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
         tl = t; \
     } \
     c0 += tl;                 /* overflow is handled on the next line */ \
-    th += (c0 < tl);          /* at most 0xFFFFFFFFFFFFFFFF */ \
+    th += (c0 < tl) ? 1 : 0;  /* at most 0xFFFFFFFFFFFFFFFF */ \
     c1 += th;                 /* overflow is handled on the next line */ \
-    c2 += (c1 < th);          /* never overflows by contract (verified in the next line) */ \
+    c2 += (c1 < th) ? 1 : 0;  /* never overflows by contract (verified in the next line) */ \
     VERIFY_CHECK((c1 >= th) || (c2 != 0)); \
 }
 
@@ -207,7 +207,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
         tl = t; \
     } \
     c0 += tl;                 /* overflow is handled on the next line */ \
-    th += (c0 < tl);          /* at most 0xFFFFFFFFFFFFFFFF */ \
+    th += (c0 < tl) ? 1 : 0;  /* at most 0xFFFFFFFFFFFFFFFF */ \
     c1 += th;                 /* never overflows by contract (verified in the next line) */ \
     VERIFY_CHECK(c1 >= th); \
 }
@@ -221,16 +221,16 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
         tl = t; \
     } \
     th2 = th + th;                  /* at most 0xFFFFFFFFFFFFFFFE (in case th was 0x7FFFFFFFFFFFFFFF) */ \
-    c2 += (th2 < th);               /* never overflows by contract (verified the next line) */ \
+    c2 += (th2 < th) ? 1 : 0;       /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((th2 >= th) || (c2 != 0)); \
     tl2 = tl + tl;                  /* at most 0xFFFFFFFFFFFFFFFE (in case the lowest 63 bits of tl were 0x7FFFFFFFFFFFFFFF) */ \
-    th2 += (tl2 < tl);              /* at most 0xFFFFFFFFFFFFFFFF */ \
+    th2 += (tl2 < tl) ? 1 : 0;      /* at most 0xFFFFFFFFFFFFFFFF */ \
     c0 += tl2;                      /* overflow is handled on the next line */ \
-    th2 += (c0 < tl2);              /* second overflow is handled on the next line */ \
+    th2 += (c0 < tl2) ? 1 : 0;      /* second overflow is handled on the next line */ \
     c2 += (c0 < tl2) & (th2 == 0);  /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((c0 >= tl2) || (th2 != 0) || (c2 != 0)); \
     c1 += th2;                      /* overflow is handled on the next line */ \
-    c2 += (c1 < th2);               /* never overflows by contract (verified the next line) */ \
+    c2 += (c1 < th2) ? 1 : 0;       /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((c1 >= th2) || (c2 != 0)); \
 }
 
@@ -238,15 +238,15 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
 #define sumadd(a) { \
     unsigned int over; \
     c0 += (a);                  /* overflow is handled on the next line */ \
-    over = (c0 < (a));         \
+    over = (c0 < (a)) ? 1 : 0; \
     c1 += over;                 /* overflow is handled on the next line */ \
-    c2 += (c1 < over);          /* never overflows by contract */ \
+    c2 += (c1 < over) ? 1 : 0;  /* never overflows by contract */ \
 }
 
 /** Add a to the number defined by (c0,c1). c1 must never overflow, c2 must be zero. */
 #define sumadd_fast(a) { \
     c0 += (a);                 /* overflow is handled on the next line */ \
-    c1 += (c0 < (a));          /* never overflows by contract (verified the next line) */ \
+    c1 += (c0 < (a)) ? 1 : 0;  /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((c1 != 0) | (c0 >= (a))); \
     VERIFY_CHECK(c2 == 0); \
 }
diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h
index f8c7fa7..3c372f3 100644
--- a/src/scalar_8x32_impl.h
+++ b/src/scalar_8x32_impl.h
@@ -271,9 +271,9 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
         tl = t; \
     } \
     c0 += tl;                 /* overflow is handled on the next line */ \
-    th += (c0 < tl);          /* at most 0xFFFFFFFF */ \
+    th += (c0 < tl) ? 1 : 0;  /* at most 0xFFFFFFFF */ \
     c1 += th;                 /* overflow is handled on the next line */ \
-    c2 += (c1 < th);          /* never overflows by contract (verified in the next line) */ \
+    c2 += (c1 < th) ? 1 : 0;  /* never overflows by contract (verified in the next line) */ \
     VERIFY_CHECK((c1 >= th) || (c2 != 0)); \
 }
 
@@ -286,7 +286,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
         tl = t; \
     } \
     c0 += tl;                 /* overflow is handled on the next line */ \
-    th += (c0 < tl);          /* at most 0xFFFFFFFF */ \
+    th += (c0 < tl) ? 1 : 0;  /* at most 0xFFFFFFFF */ \
     c1 += th;                 /* never overflows by contract (verified in the next line) */ \
     VERIFY_CHECK(c1 >= th); \
 }
@@ -300,16 +300,16 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
         tl = t; \
     } \
     th2 = th + th;                  /* at most 0xFFFFFFFE (in case th was 0x7FFFFFFF) */ \
-    c2 += (th2 < th);               /* never overflows by contract (verified the next line) */ \
+    c2 += (th2 < th) ? 1 : 0;       /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((th2 >= th) || (c2 != 0)); \
     tl2 = tl + tl;                  /* at most 0xFFFFFFFE (in case the lowest 63 bits of tl were 0x7FFFFFFF) */ \
-    th2 += (tl2 < tl);              /* at most 0xFFFFFFFF */ \
+    th2 += (tl2 < tl) ? 1 : 0;      /* at most 0xFFFFFFFF */ \
     c0 += tl2;                      /* overflow is handled on the next line */ \
-    th2 += (c0 < tl2);              /* second overflow is handled on the next line */ \
+    th2 += (c0 < tl2) ? 1 : 0;      /* second overflow is handled on the next line */ \
     c2 += (c0 < tl2) & (th2 == 0);  /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((c0 >= tl2) || (th2 != 0) || (c2 != 0)); \
     c1 += th2;                      /* overflow is handled on the next line */ \
-    c2 += (c1 < th2);               /* never overflows by contract (verified the next line) */ \
+    c2 += (c1 < th2) ? 1 : 0;       /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((c1 >= th2) || (c2 != 0)); \
 }
 
@@ -317,15 +317,15 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) {
 #define sumadd(a) { \
     unsigned int over; \
     c0 += (a);                  /* overflow is handled on the next line */ \
-    over = (c0 < (a)); \
+    over = (c0 < (a)) ? 1 : 0; \
     c1 += over;                 /* overflow is handled on the next line */ \
-    c2 += (c1 < over);          /* never overflows by contract */ \
+    c2 += (c1 < over) ? 1 : 0;  /* never overflows by contract */ \
 }
 
 /** Add a to the number defined by (c0,c1). c1 must never overflow, c2 must be zero. */
 #define sumadd_fast(a) { \
     c0 += (a);                 /* overflow is handled on the next line */ \
-    c1 += (c0 < (a));          /* never overflows by contract (verified the next line) */ \
+    c1 += (c0 < (a)) ? 1 : 0;  /* never overflows by contract (verified the next line) */ \
     VERIFY_CHECK((c1 != 0) | (c0 >= (a))); \
     VERIFY_CHECK(c2 == 0); \
 }
diff --git a/src/secp256k1.c b/src/secp256k1.c
index 3e79265..b03a6e6 100644
--- a/src/secp256k1.c
+++ b/src/secp256k1.c
@@ -291,7 +291,7 @@ int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *o
 
     VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(outputlen != NULL);
-    ARG_CHECK(*outputlen >= ((flags & SECP256K1_FLAGS_BIT_COMPRESSION) ? 33u : 65u));
+    ARG_CHECK(*outputlen >= ((flags & SECP256K1_FLAGS_BIT_COMPRESSION) ? 33 : 65));
     len = *outputlen;
     *outputlen = 0;
     ARG_CHECK(output != NULL);
diff --git a/src/util.h b/src/util.h
index 17f6b18..befb9dc 100644
--- a/src/util.h
+++ b/src/util.h
@@ -179,6 +179,20 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz
 SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;
 #endif
 
+#if defined(__BYTE_ORDER__)
+# if defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && !defined(SECP256K1_LITTLE_ENDIAN)
+#  define SECP256K1_LITTLE_ENDIAN
+# elif defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ && !defined(SECP256K1_BIG_ENDIAN)
+#  define SECP256K1_BIG_ENDIAN
+# endif
+#endif
+#if defined(_MSC_VER) && defined(_WIN32) && !defined(SECP256K1_LITTLE_ENDIAN)
+# define SECP256K1_LITTLE_ENDIAN
+#endif
+#if defined(SECP256K1_LITTLE_ENDIAN) == defined(SECP256K1_BIG_ENDIAN)
+# error Please make sure that either SECP256K1_LITTLE_ENDIAN or SECP256K1_BIG_ENDIAN is set, see src/util.h.
+#endif
+
 /* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */
 static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
     unsigned char *p = (unsigned char *)s;
@@ -197,15 +211,10 @@ static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
 /** If flag is true, set *r equal to *a; otherwise leave it. Constant-time.  Both *r and *a must be initialized and non-negative.*/
 static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) {
     unsigned int mask0, mask1, r_masked, a_masked;
-    /* Access flag with a volatile-qualified lvalue.
-       This prevents clang from figuring out (after inlining) that flag can
-       take only be 0 or 1, which leads to variable time code. */
-    volatile int vflag = flag;
-
     /* Casting a negative int to unsigned and back to int is implementation defined behavior */
     VERIFY_CHECK(*r >= 0 && *a >= 0);
 
-    mask0 = (unsigned int)vflag + ~0u;
+    mask0 = (unsigned int)flag + ~0u;
     mask1 = ~mask0;
     r_masked = ((unsigned int)*r & mask0);
     a_masked = ((unsigned int)*a & mask1);


Test patch:
diff --git a/src/tests.c b/src/tests.c
index 58d2ad2..b07a8e6 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -182,10 +182,8 @@ void run_context_tests(int use_prealloc) {
     ecount2 = 10;
     secp256k1_context_set_illegal_callback(vrfy, counting_illegal_callback_fn, &ecount);
     secp256k1_context_set_illegal_callback(sign, counting_illegal_callback_fn, &ecount2);
-    /* set error callback (to a function that still aborts in case malloc() fails in secp256k1_context_clone() below) */
-    secp256k1_context_set_error_callback(sign, secp256k1_default_illegal_callback_fn, NULL);
-    CHECK(sign->error_callback.fn != vrfy->error_callback.fn);
-    CHECK(sign->error_callback.fn == secp256k1_default_illegal_callback_fn);
+    secp256k1_context_set_error_callback(sign, counting_illegal_callback_fn, NULL);
+    CHECK(vrfy->error_callback.fn != sign->error_callback.fn);
 
     /* check if sizes for cloning are consistent */
     CHECK(secp256k1_context_preallocated_clone_size(none) == secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE));
@@ -241,8 +239,7 @@ void run_context_tests(int use_prealloc) {
     }
 
     /* Verify that the error callback makes it across the clone. */
-    CHECK(sign->error_callback.fn != vrfy->error_callback.fn);
-    CHECK(sign->error_callback.fn == secp256k1_default_illegal_callback_fn);
+    CHECK(vrfy->error_callback.fn != sign->error_callback.fn);
     /* And that it resets back to default. */
     secp256k1_context_set_error_callback(sign, NULL, NULL);
     CHECK(vrfy->error_callback.fn == sign->error_callback.fn);
@@ -2218,9 +2215,6 @@ void test_ge(void) {
                 /* Normal doubling. */
                 secp256k1_gej_double_var(&resj, &gej[i2], NULL);
                 ge_equals_gej(&ref, &resj);
-                /* Constant-time doubling. */
-                secp256k1_gej_double(&resj, &gej[i2]);
-                ge_equals_gej(&ref, &resj);
             }
 
             /* Test adding opposites. */
@@ -3240,7 +3234,6 @@ 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);
@@ -3271,8 +3264,7 @@ 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_set_int(&scalar_skew, 1 << (skew == 2));
-    secp256k1_scalar_add(&num, &num, &scalar_skew);
+    secp256k1_scalar_cadd_bit(&num, skew == 2, 1);
     CHECK(secp256k1_scalar_eq(&x, &num));
 }
 
@@ -3384,32 +3376,13 @@ 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 -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 0 */
     test_fixed_wnaf_small();
     /* Random tests */
     for (i = 0; i < count; i++) {
diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c
index 8f34670..8cca1ce 100644
--- a/src/tests_exhaustive.c
+++ b/src/tests_exhaustive.c
@@ -141,8 +141,10 @@ void test_exhaustive_addition(const secp256k1_ge *group, const secp256k1_gej *gr
     /* Check doubling */
     for (i = 0; i < order; i++) {
         secp256k1_gej tmp;
-        secp256k1_gej_double(&tmp, &groupj[i]);
-        ge_equals_gej(&group[(2 * i) % order], &tmp);
+        if (i > 0) {
+            secp256k1_gej_double_nonzero(&tmp, &groupj[i]);
+            ge_equals_gej(&group[(2 * i) % order], &tmp);
+        }
         secp256k1_gej_double_var(&tmp, &groupj[i], NULL);
         ge_equals_gej(&group[(2 * i) % order], &tmp);
     }