Repository: bitcoin-core/secp256k1
Pull request #1004 (2021-11-06 20:45:48)

ecmult: fix definition of STRAUSS_SCRATCH_OBJECTS
This bug was introduced in 7506e064d791e529d2e57bb52c156deb33b897ef by adding
an allocation but not updating the constant.

robot-dreams: Thanks for catching this, I confirmed that:
- `secp256k1_ecmult_strauss_batch` now has 7 calls to `secp256k1_scratch_alloc` instead of 6
- What matters for `STRAUSS_SCRATCH_OBJECTS` is the number of allocations (not the total size being allocated), since it's used to account for per-allocation alignment padding

I agree with @real-or-random 's suggestion. In addition, do the functions `secp256k1_strauss_scratch_size` and `secp256k1_pippenger_scratch_size` also need to be kept in sync when allocations are updated?

Patch:
diff --git a/.cirrus.yml b/.cirrus.yml
index 82a0a99..8111551 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -293,8 +293,8 @@ task:
         TEST_ITERS: 16
     - name: "UBSan, ASan, LSan"
       env:
-        CFLAGS: "-fsanitize=undefined,address -g"
-        CFLAGS_FOR_BUILD: "-fsanitize=undefined,address -g"
+        CFLAGS: "-fsanitize=undefined,address"
+        CFLAGS_FOR_BUILD: "-fsanitize=undefined,address"
         UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
         ASAN_OPTIONS: "strict_string_checks=1:detect_stack_use_after_return=1:detect_leaks=1"
         LSAN_OPTIONS: "use_unaligned=1"
@@ -329,7 +329,7 @@ task:
     # ./configure correctly errors out when given CC=g++.
     # We hack around this by passing CC=g++ only to make.
     CC: gcc
-    MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive\ -g
+    MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive
     WERROR_CFLAGS:
     EXPERIMENTAL: yes
     ECDH: yes
diff --git a/.gitignore b/.gitignore
index f3ed1c4..aa7009a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,10 @@
-bench
+bench_inv
+bench_ecdh
 bench_ecmult
+bench_schnorrsig
+bench_sign
+bench_verify
+bench_recover
 bench_internal
 tests
 exhaustive_tests
diff --git a/Makefile.am b/Makefile.am
index aa8014f..a04ad05 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -81,9 +81,11 @@ endif
 
 noinst_PROGRAMS =
 if USE_BENCHMARK
-noinst_PROGRAMS += bench bench_internal bench_ecmult
-bench_SOURCES = src/bench.c
-bench_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
+noinst_PROGRAMS += bench_verify bench_sign bench_internal bench_ecmult
+bench_verify_SOURCES = src/bench_verify.c
+bench_verify_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
+bench_sign_SOURCES = src/bench_sign.c
+bench_sign_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
 bench_internal_SOURCES = src/bench_internal.c
 bench_internal_LDADD = $(SECP_LIBS) $(COMMON_LIB)
 bench_internal_CPPFLAGS = $(SECP_INCLUDES)
diff --git a/SECURITY.md b/SECURITY.md
index b515cc1..0e4d588 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -9,7 +9,7 @@ The following keys may be used to communicate sensitive information to developer
 | Name | Fingerprint |
 |------|-------------|
 | Pieter Wuille | 133E AC17 9436 F14A 5CF1  B794 860F EB80 4E66 9320 |
-| Jonas Nick | 36C7 1A37 C9D9 88BD E825  08D9 B1A7 0E4F 8DCD 0366 |
+| Andrew Poelstra | 699A 63EF C17A D3A9 A34C  FFC0 7AD0 A91C 40BD 0091 |
 | Tim Ruffing | 09E0 3F87 1092 E40E 106E  902B 33BC 86AB 80FF 5516 |
 
-You can import a key by running the following command with that individual’s fingerprint: `gpg --keyserver hkps://keys.openpgp.org --recv-keys "<fingerprint>"` Ensure that you put quotes around fingerprints containing spaces.
+You can import a key by running the following command with that individual’s fingerprint: `gpg --recv-keys "<fingerprint>"` Ensure that you put quotes around fingerprints containing spaces.
diff --git a/ci/cirrus.sh b/ci/cirrus.sh
index 39ec2ad..27db1e6 100755
--- a/ci/cirrus.sh
+++ b/ci/cirrus.sh
@@ -26,7 +26,7 @@ make
 
 # Print information about binaries so that we can see that the architecture is correct
 file *tests* || true
-file bench* || true
+file bench_* || true
 file .libs/* || true
 
 # This tells `make check` to wrap test invocations.
@@ -49,8 +49,21 @@ then
     {
         $EXEC ./bench_ecmult
         $EXEC ./bench_internal
-        $EXEC ./bench
+        $EXEC ./bench_sign
+        $EXEC ./bench_verify
     } >> bench.log 2>&1
+    if [ "$RECOVERY" = "yes" ]
+    then
+        $EXEC ./bench_recover >> bench.log 2>&1
+    fi
+    if [ "$ECDH" = "yes" ]
+    then
+        $EXEC ./bench_ecdh >> bench.log 2>&1
+    fi
+    if [ "$SCHNORRSIG" = "yes" ]
+    then
+        $EXEC ./bench_schnorrsig >> bench.log 2>&1
+    fi
 fi
 if [ "$CTIMETEST" = "yes" ]
 then
diff --git a/configure.ac b/configure.ac
index c3c213a..906f63c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,7 +21,7 @@ AC_PATH_TOOL(STRIP, strip)
 
 # Save definition of AC_PROG_CC because AM_PROG_CC_C_O in automake<=1.13 will
 # redefine AC_PROG_CC to exit with an error, which avoids the user calling it
-# accidentally and screwing up the effect of AM_PROG_CC_C_O. However, we'll need
+# accidently and screwing up the effect of AM_PROG_CC_C_O. However, we'll need
 # AC_PROG_CC later on in AX_PROG_CC_FOR_BUILD, where its usage is fine, and
 # we'll carefully make sure not to call AC_PROG_CC anywhere else.
 m4_copy([AC_PROG_CC], [saved_AC_PROG_CC])
@@ -43,8 +43,8 @@ case $host_os in
          # These Homebrew packages may be keg-only, meaning that they won't be found
          # in expected paths because they may conflict with system files. Ask
          # Homebrew where each one is located, then adjust paths accordingly.
-         if $BREW list --versions valgrind >/dev/null; then
-           valgrind_prefix=`$BREW --prefix valgrind 2>/dev/null`
+         valgrind_prefix=`$BREW --prefix valgrind 2>/dev/null`
+         if test x$valgrind_prefix != x; then
            VALGRIND_CPPFLAGS="-I$valgrind_prefix/include"
          fi
        else
@@ -160,7 +160,7 @@ AC_ARG_ENABLE(external_default_callbacks,
 AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=auto])
 
 AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto],
-[assembly optimizations to use (experimental: arm) [default=auto]])],[req_asm=$withval], [req_asm=auto])
+[assembly optimizations to use (experimental: arm) [default=auto]])],[req_asm=$withval], [req_asm=auto])
 
 AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto],
 [window size for ecmult precomputation for verification, specified as integer in range [2..24].]
diff --git a/doc/safegcd_implementation.md b/doc/safegcd_implementation.md
index 063aa8e..3ae556f 100644
--- a/doc/safegcd_implementation.md
+++ b/doc/safegcd_implementation.md
@@ -569,14 +569,8 @@ bits efficiently, which is possible on most platforms; it is abstracted here as
 
 ```python
 def count_trailing_zeros(v):
-    """
-    When v is zero, consider all N zero bits as "trailing".
-    For a non-zero value v, find z such that v=(d<<z) for some odd d.
-    """
-    if v == 0:
-        return N
-    else:
-        return (v & -v).bit_length() - 1
+    """For a non-zero value v, find z such that v=(d<<z) for some odd d."""
+    return (v & -v).bit_length() - 1
 
 i = N # divsteps left to do
 while True:
@@ -607,7 +601,7 @@ becomes negative, or when *i* reaches *0*. Combined, this is equivalent to addin
 It is easy to find what that multiple is: we want a number *w* such that *g+w&thinsp;f* has a few bottom
 zero bits. If that number of bits is *L*, we want *g+w&thinsp;f mod 2<sup>L</sup> = 0*, or *w = -g/f mod 2<sup>L</sup>*. Since *f*
 is odd, such a *w* exists for any *L*. *L* cannot be more than *i* steps (as we'd finish the loop before
-doing more) or more than *&eta;+1* steps (as we'd run `eta, f, g = -eta, g, -f` at that point), but
+doing more) or more than *&eta;+1* steps (as we'd run `eta, f, g = -eta, g, f` at that point), but
 apart from that, we're only limited by the complexity of computing *w*.
 
 This code demonstrates how to cancel up to 4 bits per step:
@@ -624,7 +618,7 @@ while True:
         break
     # We know g is odd now
     if eta < 0:
-        eta, f, g = -eta, g, -f
+        eta, f, g = -eta, g, f
     # Compute limit on number of bits to cancel
     limit = min(min(eta + 1, i), 4)
     # Compute w = -g/f mod 2**limit, using the table value for -1/f mod 2**4. Note that f is
diff --git a/include/secp256k1.h b/include/secp256k1.h
index 57114b8..576953f 100644
--- a/include/secp256k1.h
+++ b/include/secp256k1.h
@@ -264,7 +264,7 @@ SECP256K1_API void secp256k1_context_destroy(
  *  undefined.
  *
  *  When this function has not been called (or called with fn==NULL), then the
- *  default handler will be used. The library provides a default handler which
+ *  default handler will be used. The library provides a default handler which
  *  writes the message to stderr and calls abort. This default handler can be
  *  replaced at link time if the preprocessor macro
  *  USE_EXTERNAL_DEFAULT_CALLBACKS is defined, which is the case if the build
diff --git a/src/modules/ecdh/bench_impl.h b/src/bench_ecdh.c
similarity index 82%
rename from src/modules/ecdh/bench_impl.h
rename to src/bench_ecdh.c
index 94d8334..b6d4ee2 100644
--- a/src/modules/ecdh/bench_impl.h
+++ b/src/bench_ecdh.c
@@ -4,10 +4,12 @@
  * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
  ***********************************************************************/
 
-#ifndef SECP256K1_MODULE_ECDH_BENCH_H
-#define SECP256K1_MODULE_ECDH_BENCH_H
+#include <string.h>
 
+#include "../include/secp256k1.h"
 #include "../include/secp256k1_ecdh.h"
+#include "util.h"
+#include "bench.h"
 
 typedef struct {
     secp256k1_context *ctx;
@@ -42,16 +44,18 @@ static void bench_ecdh(void* arg, int iters) {
     }
 }
 
-void run_ecdh_bench(int iters, int argc, char** argv) {
+int main(void) {
     bench_ecdh_data data;
-    int d = argc == 1;
+
+    int iters = get_iters(20000);
 
     /* create a context with no capabilities */
     data.ctx = secp256k1_context_create(SECP256K1_FLAGS_TYPE_CONTEXT);
 
-    if (d || have_flag(argc, argv, "ecdh")) run_benchmark("ecdh", bench_ecdh, bench_ecdh_setup, NULL, &data, 10, iters);
+    print_output_table_header_row();
+
+    run_benchmark("ecdh", bench_ecdh, bench_ecdh_setup, NULL, &data, 10, iters);
 
     secp256k1_context_destroy(data.ctx);
+    return 0;
 }
-
-#endif /* SECP256K1_MODULE_ECDH_BENCH_H */
diff --git a/src/modules/recovery/bench_impl.h b/src/bench_recover.c
similarity index 81%
rename from src/modules/recovery/bench_impl.h
rename to src/bench_recover.c
index 4a9e886..3ccd86d 100644
--- a/src/modules/recovery/bench_impl.h
+++ b/src/bench_recover.c
@@ -4,10 +4,10 @@
  * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
  ***********************************************************************/
 
-#ifndef SECP256K1_MODULE_RECOVERY_BENCH_H
-#define SECP256K1_MODULE_RECOVERY_BENCH_H
-
+#include "../include/secp256k1.h"
 #include "../include/secp256k1_recovery.h"
+#include "util.h"
+#include "bench.h"
 
 typedef struct {
     secp256k1_context *ctx;
@@ -48,15 +48,17 @@ void bench_recover_setup(void* arg) {
     }
 }
 
-void run_recovery_bench(int iters, int argc, char** argv) {
+int main(void) {
     bench_recover_data data;
-    int d = argc == 1;
+
+    int iters = get_iters(20000);
 
     data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
 
-    if (d || have_flag(argc, argv, "ecdsa") || have_flag(argc, argv, "recover") || have_flag(argc, argv, "ecdsa_recover")) run_benchmark("ecdsa_recover", bench_recover, bench_recover_setup, NULL, &data, 10, iters);
+    print_output_table_header_row();
+
+    run_benchmark("ecdsa_recover", bench_recover, bench_recover_setup, NULL, &data, 10, iters);
 
     secp256k1_context_destroy(data.ctx);
+    return 0;
 }
-
-#endif /* SECP256K1_MODULE_RECOVERY_BENCH_H */
diff --git a/src/modules/schnorrsig/bench_impl.h b/src/bench_schnorrsig.c
similarity index 82%
rename from src/modules/schnorrsig/bench_impl.h
rename to src/bench_schnorrsig.c
index 41f393c..e30735d 100644
--- a/src/modules/schnorrsig/bench_impl.h
+++ b/src/bench_schnorrsig.c
@@ -4,10 +4,14 @@
  * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
  ***********************************************************************/
 
-#ifndef SECP256K1_MODULE_SCHNORRSIG_BENCH_H
-#define SECP256K1_MODULE_SCHNORRSIG_BENCH_H
+#include <string.h>
+#include <stdlib.h>
 
-#include "../../../include/secp256k1_schnorrsig.h"
+
+#include "../include/secp256k1.h"
+#include "../include/secp256k1_schnorrsig.h"
+#include "util.h"
+#include "bench.h"
 
 #define MSGLEN 32
 
@@ -45,10 +49,10 @@ void bench_schnorrsig_verify(void* arg, int iters) {
     }
 }
 
-void run_schnorrsig_bench(int iters, int argc, char** argv) {
+int main(void) {
     int i;
     bench_schnorrsig_data data;
-    int d = argc == 1;
+    int iters = get_iters(10000);
 
     data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY | SECP256K1_CONTEXT_SIGN);
     data.keypairs = (const secp256k1_keypair **)malloc(iters * sizeof(secp256k1_keypair *));
@@ -82,8 +86,9 @@ void run_schnorrsig_bench(int iters, int argc, char** argv) {
         CHECK(secp256k1_xonly_pubkey_serialize(data.ctx, pk_char, &pk) == 1);
     }
 
-    if (d || have_flag(argc, argv, "schnorrsig") || have_flag(argc, argv, "sign") || have_flag(argc, argv, "schnorrsig_sign")) run_benchmark("schnorrsig_sign", bench_schnorrsig_sign, NULL, NULL, (void *) &data, 10, iters);
-    if (d || have_flag(argc, argv, "schnorrsig") || have_flag(argc, argv, "verify") || have_flag(argc, argv, "schnorrsig_verify")) run_benchmark("schnorrsig_verify", bench_schnorrsig_verify, NULL, NULL, (void *) &data, 10, iters);
+    print_output_table_header_row();
+    run_benchmark("schnorrsig_sign", bench_schnorrsig_sign, NULL, NULL, (void *) &data, 10, iters);
+    run_benchmark("schnorrsig_verify", bench_schnorrsig_verify, NULL, NULL, (void *) &data, 10, iters);
 
     for (i = 0; i < iters; i++) {
         free((void *)data.keypairs[i]);
@@ -97,6 +102,5 @@ void run_schnorrsig_bench(int iters, int argc, char** argv) {
     free(data.sigs);
 
     secp256k1_context_destroy(data.ctx);
+    return 0;
 }
-
-#endif
diff --git a/src/bench_sign.c b/src/bench_sign.c
new file mode 100644
index 0000000..c44f7b1
--- /dev/null
+++ b/src/bench_sign.c
@@ -0,0 +1,60 @@
+/***********************************************************************
+ * Copyright (c) 2014 Pieter Wuille                                    *
+ * Distributed under the MIT software license, see the accompanying    *
+ * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
+ ***********************************************************************/
+
+#include "../include/secp256k1.h"
+#include "util.h"
+#include "bench.h"
+
+typedef struct {
+    secp256k1_context* ctx;
+    unsigned char msg[32];
+    unsigned char key[32];
+} bench_sign_data;
+
+static void bench_sign_setup(void* arg) {
+    int i;
+    bench_sign_data *data = (bench_sign_data*)arg;
+
+    for (i = 0; i < 32; i++) {
+        data->msg[i] = i + 1;
+    }
+    for (i = 0; i < 32; i++) {
+        data->key[i] = i + 65;
+    }
+}
+
+static void bench_sign_run(void* arg, int iters) {
+    int i;
+    bench_sign_data *data = (bench_sign_data*)arg;
+
+    unsigned char sig[74];
+    for (i = 0; i < iters; i++) {
+        size_t siglen = 74;
+        int j;
+        secp256k1_ecdsa_signature signature;
+        CHECK(secp256k1_ecdsa_sign(data->ctx, &signature, data->msg, data->key, NULL, NULL));
+        CHECK(secp256k1_ecdsa_signature_serialize_der(data->ctx, sig, &siglen, &signature));
+        for (j = 0; j < 32; j++) {
+            data->msg[j] = sig[j];
+            data->key[j] = sig[j + 32];
+        }
+    }
+}
+
+int main(void) {
+    bench_sign_data data;
+
+    int iters = get_iters(20000);
+
+    data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
+
+    print_output_table_header_row();
+    
+    run_benchmark("ecdsa_sign", bench_sign_run, bench_sign_setup, NULL, &data, 10, iters);
+
+    secp256k1_context_destroy(data.ctx);
+    return 0;
+}
diff --git a/src/bench.c b/src/bench_verify.c
similarity index 52%
rename from src/bench.c
rename to src/bench_verify.c
index dfc0470..d62e34b 100644
--- a/src/bench.c
+++ b/src/bench_verify.c
@@ -40,64 +40,14 @@ static void bench_verify(void* arg, int iters) {
     }
 }
 
-typedef struct {
-    secp256k1_context* ctx;
-    unsigned char msg[32];
-    unsigned char key[32];
-} bench_sign_data;
-
-static void bench_sign_setup(void* arg) {
-    int i;
-    bench_sign_data *data = (bench_sign_data*)arg;
-
-    for (i = 0; i < 32; i++) {
-        data->msg[i] = i + 1;
-    }
-    for (i = 0; i < 32; i++) {
-        data->key[i] = i + 65;
-    }
-}
-
-static void bench_sign_run(void* arg, int iters) {
-    int i;
-    bench_sign_data *data = (bench_sign_data*)arg;
-
-    unsigned char sig[74];
-    for (i = 0; i < iters; i++) {
-        size_t siglen = 74;
-        int j;
-        secp256k1_ecdsa_signature signature;
-        CHECK(secp256k1_ecdsa_sign(data->ctx, &signature, data->msg, data->key, NULL, NULL));
-        CHECK(secp256k1_ecdsa_signature_serialize_der(data->ctx, sig, &siglen, &signature));
-        for (j = 0; j < 32; j++) {
-            data->msg[j] = sig[j];
-            data->key[j] = sig[j + 32];
-        }
-    }
-}
-
-#ifdef ENABLE_MODULE_ECDH
-# include "modules/ecdh/bench_impl.h"
-#endif
-
-#ifdef ENABLE_MODULE_RECOVERY
-# include "modules/recovery/bench_impl.h"
-#endif
-
-#ifdef ENABLE_MODULE_SCHNORRSIG
-# include "modules/schnorrsig/bench_impl.h"
-#endif
-
-int main(int argc, char** argv) {
+int main(void) {
     int i;
     secp256k1_pubkey pubkey;
     secp256k1_ecdsa_signature sig;
     bench_verify_data data;
 
-    int d = argc == 1;
     int iters = get_iters(20000);
 
-    /* ECDSA verification benchmark */
     data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
 
     for (i = 0; i < 32; i++) {
@@ -114,31 +64,8 @@ int main(int argc, char** argv) {
     CHECK(secp256k1_ec_pubkey_serialize(data.ctx, data.pubkey, &data.pubkeylen, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
 
     print_output_table_header_row();
-    if (d || have_flag(argc, argv, "ecdsa") || have_flag(argc, argv, "verify") || have_flag(argc, argv, "ecdsa_verify")) run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters);
-
-    secp256k1_context_destroy(data.ctx);
-
-    /* ECDSA signing benchmark */
-    data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
-
-    if (d || have_flag(argc, argv, "ecdsa") || have_flag(argc, argv, "sign") || have_flag(argc, argv, "ecdsa_sign")) run_benchmark("ecdsa_sign", bench_sign_run, bench_sign_setup, NULL, &data, 10, iters);
+    run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters);
 
     secp256k1_context_destroy(data.ctx);
-
-#ifdef ENABLE_MODULE_ECDH
-    /* ECDH benchmarks */
-    run_ecdh_bench(iters, argc, argv);
-#endif
-
-#ifdef ENABLE_MODULE_RECOVERY
-    /* ECDSA recovery benchmarks */
-    run_recovery_bench(iters, argc, argv);
-#endif
-
-#ifdef ENABLE_MODULE_SCHNORRSIG
-    /* Schnorr signature benchmarks */
-    run_schnorrsig_bench(iters, argc, argv);
-#endif
-
     return 0;
 }
diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h
index 90b4b22..a75a088 100644
--- a/src/ecdsa_impl.h
+++ b/src/ecdsa_impl.h
@@ -304,12 +304,12 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec
     high = secp256k1_scalar_is_high(sigs);
     secp256k1_scalar_cond_negate(sigs, high);
     if (recid) {
-        *recid ^= high;
+            *recid ^= high;
     }
     /* P.x = order is on the curve, so technically sig->r could end up being zero, which would be an invalid signature.
      * This is cryptographically unreachable as hitting it requires finding the discrete log of P.x = N.
      */
-    return (int)(!secp256k1_scalar_is_zero(sigr)) & (int)(!secp256k1_scalar_is_zero(sigs));
+    return !secp256k1_scalar_is_zero(sigr) & !secp256k1_scalar_is_zero(sigs);
 }
 
 #endif /* SECP256K1_ECDSA_IMPL_H */
diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h
index 5d94091..0402f84 100644
--- a/src/ecmult_impl.h
+++ b/src/ecmult_impl.h
@@ -47,7 +47,7 @@
 
 /* The number of objects allocated on the scratch space for ecmult_multi algorithms */
 #define PIPPENGER_SCRATCH_OBJECTS 6
-#define STRAUSS_SCRATCH_OBJECTS 6
+#define STRAUSS_SCRATCH_OBJECTS 7
 
 #define PIPPENGER_MAX_BUCKET_WINDOW 12
 
@@ -220,7 +220,7 @@ struct secp256k1_strauss_state {
 static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state *state, secp256k1_gej *r, size_t num, const secp256k1_gej *a, const secp256k1_scalar *na, const secp256k1_scalar *ng) {
     secp256k1_ge tmpa;
     secp256k1_fe Z;
-    /* Split G factors. */
+    /* Splitted G factors. */
     secp256k1_scalar ng_1, ng_128;
     int wnaf_ng_1[129];
     int bits_ng_1 = 0;
@@ -365,6 +365,9 @@ static int secp256k1_ecmult_strauss_batch(const secp256k1_callback* error_callba
         return 1;
     }
 
+    /* We allocate STRAUSS_SCRATCH_OBJECTS objects on the scratch space. If these
+     * allocations change, make sure to update the STRAUSS_SCRATCH_OBJECTS
+     * constant and strauss_scratch_size accordingly. */
     points = (secp256k1_gej*)secp256k1_scratch_alloc(error_callback, scratch, n_points * sizeof(secp256k1_gej));
     scalars = (secp256k1_scalar*)secp256k1_scratch_alloc(error_callback, scratch, n_points * sizeof(secp256k1_scalar));
     state.prej = (secp256k1_gej*)secp256k1_scratch_alloc(error_callback, scratch, n_points * ECMULT_TABLE_SIZE(WINDOW_A) * sizeof(secp256k1_gej));
@@ -659,8 +662,12 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call
     if (inp_g_sc == NULL && n_points == 0) {
         return 1;
     }
-
     bucket_window = secp256k1_pippenger_bucket_window(n_points);
+
+    /* We allocate PIPPENGER_SCRATCH_OBJECTS objects on the scratch space. If
+     * these allocations change, make sure to update the
+     * PIPPENGER_SCRATCH_OBJECTS constant and pippenger_scratch_size
+     * accordingly. */
     points = (secp256k1_ge *) secp256k1_scratch_alloc(error_callback, scratch, entries * sizeof(*points));
     scalars = (secp256k1_scalar *) secp256k1_scratch_alloc(error_callback, scratch, entries * sizeof(*scalars));
     state_space = (struct secp256k1_pippenger_state *) secp256k1_scratch_alloc(error_callback, scratch, sizeof(*state_space));
@@ -668,7 +675,6 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call
         secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
         return 0;
     }
-
     state_space->ps = (struct secp256k1_pippenger_point_state *) secp256k1_scratch_alloc(error_callback, scratch, entries * sizeof(*state_space->ps));
     state_space->wnaf_na = (int *) secp256k1_scratch_alloc(error_callback, scratch, entries*(WNAF_SIZE(bucket_window+1)) * sizeof(int));
     buckets = (secp256k1_gej *) secp256k1_scratch_alloc(error_callback, scratch, (1<<bucket_window) * sizeof(*buckets));
diff --git a/src/modules/ecdh/Makefile.am.include b/src/modules/ecdh/Makefile.am.include
index 7f7f95f..e3088b4 100644
--- a/src/modules/ecdh/Makefile.am.include
+++ b/src/modules/ecdh/Makefile.am.include
@@ -1,4 +1,8 @@
 include_HEADERS += include/secp256k1_ecdh.h
 noinst_HEADERS += src/modules/ecdh/main_impl.h
 noinst_HEADERS += src/modules/ecdh/tests_impl.h
-noinst_HEADERS += src/modules/ecdh/bench_impl.h
+if USE_BENCHMARK
+noinst_PROGRAMS += bench_ecdh
+bench_ecdh_SOURCES = src/bench_ecdh.c
+bench_ecdh_LDADD = libsecp256k1.la $(SECP_LIBS) $(COMMON_LIB)
+endif
diff --git a/src/modules/recovery/Makefile.am.include b/src/modules/recovery/Makefile.am.include
index 156ea69..e2d3f12 100644
--- a/src/modules/recovery/Makefile.am.include
+++ b/src/modules/recovery/Makefile.am.include
@@ -2,4 +2,8 @@ include_HEADERS += include/secp256k1_recovery.h
 noinst_HEADERS += src/modules/recovery/main_impl.h
 noinst_HEADERS += src/modules/recovery/tests_impl.h
 noinst_HEADERS += src/modules/recovery/tests_exhaustive_impl.h
-noinst_HEADERS += src/modules/recovery/bench_impl.h
+if USE_BENCHMARK
+noinst_PROGRAMS += bench_recover
+bench_recover_SOURCES = src/bench_recover.c
+bench_recover_LDADD = libsecp256k1.la $(SECP_LIBS) $(COMMON_LIB)
+endif
diff --git a/src/modules/schnorrsig/Makefile.am.include b/src/modules/schnorrsig/Makefile.am.include
index 654fa2e..568bcc3 100644
--- a/src/modules/schnorrsig/Makefile.am.include
+++ b/src/modules/schnorrsig/Makefile.am.include
@@ -2,4 +2,8 @@ include_HEADERS += include/secp256k1_schnorrsig.h
 noinst_HEADERS += src/modules/schnorrsig/main_impl.h
 noinst_HEADERS += src/modules/schnorrsig/tests_impl.h
 noinst_HEADERS += src/modules/schnorrsig/tests_exhaustive_impl.h
-noinst_HEADERS += src/modules/schnorrsig/bench_impl.h
+if USE_BENCHMARK
+noinst_PROGRAMS += bench_schnorrsig
+bench_schnorrsig_SOURCES = src/bench_schnorrsig.c
+bench_schnorrsig_LDADD = libsecp256k1.la $(SECP_LIBS) $(COMMON_LIB)
+endif
diff --git a/src/scalar_impl.h b/src/scalar_impl.h
index 1b690e3..e124474 100644
--- a/src/scalar_impl.h
+++ b/src/scalar_impl.h
@@ -74,7 +74,7 @@ static void secp256k1_scalar_split_lambda_verify(const secp256k1_scalar *r1, con
  * Both lambda and beta are primitive cube roots of unity.  That is lamba^3 == 1 mod n and
  * beta^3 == 1 mod p, where n is the curve order and p is the field order.
  *
- * Furthermore, because (X^3 - 1) = (X - 1)(X^2 + X + 1), the primitive cube roots of unity are
+ * Futhermore, because (X^3 - 1) = (X - 1)(X^2 + X + 1), the primitive cube roots of unity are
  * roots of X^2 + X + 1.  Therefore lambda^2 + lamba == -1 mod n and beta^2 + beta == -1 mod p.
  * (The other primitive cube roots of unity are lambda^2 and beta^2 respectively.)
  *


Test patch:
diff --git a/src/tests.c b/src/tests.c
index 6027ebc..ebeef51 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -3225,7 +3225,7 @@ void test_intialized_inf(void) {
     secp256k1_gej pj, npj, infj1, infj2, infj3;
     secp256k1_fe zinv;
 
-    /* Test that adding P+(-P) results in a fully initialized infinity*/
+    /* Test that adding P+(-P) results in a fully initalized infinity*/
     random_group_element_test(&p);
     secp256k1_gej_set_ge(&pj, &p);
     secp256k1_gej_neg(&npj, &pj);