Set limit on n_sigs in batch verification and simplify randomizer_cac…

…he (h/t Tim Ruffing)

.gitignore

@@ -4,7 +4,6 @@ bench_ecmult
 bench_schnorrsig
 bench_sign
 bench_verify
-bench_schnorr_verify
 bench_recover
 bench_internal
 tests

include/secp256k1_schnorrsig.h

@@ -19,7 +19,7 @@ typedef struct {
     unsigned char data[64];
 } secp256k1_schnorrsig;
 
-/** Serialize a Schnorr signature
+/** Serialize a Schnorr signature.
  *
  *  Returns: 1
  *  Args:    ctx: a secp256k1 context object
@@ -91,7 +91,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify(
     const secp256k1_pubkey *pubkey
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
 
-/** Verifies a set of Schnorr signatures
+/** Verifies a set of Schnorr signatures.
  *
  * Returns 1 if all succeeded, 0 otherwise. In particular, returns 1 if n_sigs is 0.
  *
@@ -100,7 +100,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify(
  *  In:      sig: array of signatures, or NULL if there are no signatures
  *         msg32: array of messages, or NULL if there are no signatures
  *            pk: array of public keys, or NULL if there are no signatures
- *        n_sigs: number of signatures in above arrays (must be 0 if they are NULL)
+ *        n_sigs: number of signatures in above arrays. Must be smaller than
+ *                2^31 and smaller than 2^(sizeof(size_t)*8-1) i.e. half the
+ *                maximum size_t value. Must be 0 if above arrays are NULL.
  */
 SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify_batch(
     const secp256k1_context* ctx,

src/modules/schnorrsig/main_impl.h

@@ -163,7 +163,8 @@ typedef struct {
     /* Seed for the random number generator */
     unsigned char chacha_seed[32];
     /* Caches randomizers generated by the PRNG which returns two randomizers per call. Caching
-     * avoids having to call the PRNG twice as often. */
+     * avoids having to call the PRNG twice as often. The very first randomizer will be set to 1 and
+     * the PRNG is called at every odd indexed schnorrsig to fill the cache. */
     secp256k1_scalar randomizer_cache[2];
     /* Signature, message, public key tuples to verify */
     const secp256k1_schnorrsig *const *sig;
@@ -172,29 +173,19 @@ typedef struct {
     size_t n_sigs;
 } secp256k1_schnorrsig_verify_ecmult_context;
 
-/* Helper function for batch verification.
- * One call to the chacha20 returns two random scalars which are returned by this function as r[0]
- * and r[1] except for the first randomizer which is set to 1. */
-static void secp256k1_schnorrsig_batch_randomizer(secp256k1_scalar *r, const unsigned char *seed, uint64_t idx) {
-    secp256k1_scalar_chacha20(&r[0], &r[1], seed, idx);
-    if (idx == 0) {
-        secp256k1_scalar_set_int(&r[0], 1);
-    }
-}
-
 /* Callback function which is called by ecmult_multi in order to convert the ecmult_context
  * consisting of signature, message and public key tuples into scalars and points. */
 static int secp256k1_schnorrsig_verify_batch_ecmult_callback(secp256k1_scalar *sc, secp256k1_ge *pt, size_t idx, void *data) {
     secp256k1_schnorrsig_verify_ecmult_context *ecmult_context = (secp256k1_schnorrsig_verify_ecmult_context *) data;
 
-    if (idx % 4 == 0) {
-        /* Every idx corresponds to a (scalar,point)-tuple. So this callback is called on 4
+    if (idx % 4 == 2) {
+        /* Every idx corresponds to a (scalar,point)-tuple. So this callback is called with 4
          * consecutive tuples before we need to call the RNG for new randomizers:
          * (-randomizer_cache[0], R1)
          * (-randomizer_cache[0]*e1, P1)
          * (-randomizer_cache[1], R2)
          * (-randomizer_cache[1]*e2, P2) */
-        secp256k1_schnorrsig_batch_randomizer(ecmult_context->randomizer_cache, ecmult_context->chacha_seed, idx / 4);
+        secp256k1_scalar_chacha20(&ecmult_context->randomizer_cache[0], &ecmult_context->randomizer_cache[1], ecmult_context->chacha_seed, idx / 4);
     }
 
     /* R */
@@ -284,11 +275,12 @@ int secp256k1_schnorrsig_verify_batch_sum_s(secp256k1_scalar *s, unsigned char *
     secp256k1_scalar randomizer_cache[2];
     size_t i;
 
+    secp256k1_scalar_set_int(&randomizer_cache[0], 1);
     for (i = 0; i < n_sigs; i++) {
         int overflow;
         secp256k1_scalar term;
-        if (i % 2 == 0) {
-            secp256k1_schnorrsig_batch_randomizer(randomizer_cache, chacha_seed, i / 2);
+        if (i % 2 == 1) {
+            secp256k1_scalar_chacha20(&randomizer_cache[0], &randomizer_cache[1], chacha_seed, i / 2);
         }
 
         secp256k1_scalar_set_b32(&term, &sig[i]->data[32], &overflow);
@@ -314,12 +306,19 @@ int secp256k1_schnorrsig_verify_batch(const secp256k1_context *ctx, secp256k1_sc
     VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(secp256k1_ecmult_context_is_built(&ctx->ecmult_ctx));
     ARG_CHECK(scratch != NULL);
+    /* Check that n_sigs is less than half of the maximum size_t value. This is necessary because
+     * the number of points given to ecmult_multi is 2*n_sigs. */
+    ARG_CHECK(n_sigs < (size_t)1 << (sizeof(size_t)*8-1));
+    /* Check that n_sigs is less 2^31 to ensure the same behavior of this function on 32-bit and
+     * 64-bit platforms. */
+    ARG_CHECK(n_sigs < (size_t)(1 << 31));
 
     secp256k1_sha256_initialize(&sha);
     if (!secp256k1_schnorrsig_verify_batch_init_randomizer(ctx, &ecmult_context, &sha, sig, msg32, pk, n_sigs)) {
         return 0;
     }
     secp256k1_sha256_finalize(&sha, ecmult_context.chacha_seed);
+    secp256k1_scalar_set_int(&ecmult_context.randomizer_cache[0], 1);
 
     secp256k1_scalar_clear(&s);
     if (!secp256k1_schnorrsig_verify_batch_sum_s(&s, ecmult_context.chacha_seed, sig, n_sigs)) {

src/modules/schnorrsig/tests_impl.h

@@ -114,6 +114,10 @@ void test_schnorrsig_api(secp256k1_scratch_space *scratch) {
     CHECK(ecount == 4);
     CHECK(secp256k1_schnorrsig_verify_batch(vrfy, scratch, &sigptr, &msgptr, NULL, 1) == 0);
     CHECK(ecount == 5);
+    CHECK(secp256k1_schnorrsig_verify_batch(vrfy, scratch, &sigptr, &msgptr, &pkptr, (size_t)1 << (sizeof(size_t)*8-1)) == 0);
+    CHECK(ecount == 6);
+    CHECK(secp256k1_schnorrsig_verify_batch(vrfy, scratch, &sigptr, &msgptr, &pkptr, 1 << 31) == 0);
+    CHECK(ecount == 7);
 
     secp256k1_context_destroy(none);
     secp256k1_context_destroy(sign);