ecmult: make strauss_ and pippenger_max_points more precise

Take actual alignment into account instead of assuming the worst case. This
allows more precise tests for strauss, because if a scratch space has exactly
strauss_scratch_size(n_points) left, then secp256k1_strauss_max_points(cb,
scratch) = n_points.

Author: jonasnick

src/ecmult_impl.h

@@ -437,8 +437,27 @@ static int secp256k1_ecmult_strauss_batch_single(const secp256k1_callback* error
     return secp256k1_ecmult_strauss_batch(error_callback, scratch, r, inp_g_sc, cb, cbdata, n, 0);
 }
 
+/* Returns the maximum number of points in addition to G that can be used with a
+ * given scratch space. If a scratch space has exactly
+ * `strauss_scratch_size(n_points)` left, then
+ * `strauss_max_points(cb, scratch) = n_points`. */
 static size_t secp256k1_strauss_max_points(const secp256k1_callback* error_callback, secp256k1_scratch *scratch) {
-    return secp256k1_scratch_max_allocation(error_callback, scratch, STRAUSS_SCRATCH_OBJECTS) / secp256k1_strauss_scratch_size(1);
+    /* Call max_allocation with 0 objects because otherwise it would assume
+     * worst case padding but in this function we want to be exact. */
+    size_t max_alloc = secp256k1_scratch_max_allocation(error_callback, scratch, 0);
+    size_t unpadded_single_size = secp256k1_strauss_scratch_size_raw(1, 0);
+    size_t n_points = max_alloc / unpadded_single_size;
+    if (n_points > 0
+              && max_alloc < secp256k1_strauss_scratch_size(n_points)) {
+        /* If there's not enough space after alignment is taken into
+         * account, it suffices to decrease n_points by one. This is because
+         * the maximum padding required is less than an entry. */
+        n_points -= 1;
+        VERIFY_CHECK(max_alloc >= secp256k1_strauss_scratch_size(n_points));
+        VERIFY_CHECK(max_alloc - secp256k1_scratch_max_allocation(error_callback, scratch, STRAUSS_SCRATCH_OBJECTS) < unpadded_single_size);
+    }
+
+    return n_points;
 }
 
 /** Convert a number to WNAF notation.
@@ -801,13 +820,17 @@ static int secp256k1_ecmult_pippenger_batch_single(const secp256k1_callback* err
     return secp256k1_ecmult_pippenger_batch(error_callback, scratch, r, inp_g_sc, cb, cbdata, n, 0);
 }
 
-/**
- * Returns the maximum number of points in addition to G that can be used with
- * a given scratch space. The function ensures that fewer points may also be
- * used.
+/* Returns the (near) maximum number of points in addition to G that can be
+ * used with a given scratch space. It may not return the actual maximum number
+ * of points possible. Otherwise, fewer points would not fit into the scratch
+ * space in general. If a scratch space has exactly
+ * `pippenger_scratch_size(n_points)` left, then
+ * `pippenger_max_points(cb, scratch) <= n_points`.
  */
 static size_t secp256k1_pippenger_max_points(const secp256k1_callback* error_callback, secp256k1_scratch *scratch) {
-    size_t max_alloc = secp256k1_scratch_max_allocation(error_callback, scratch, PIPPENGER_SCRATCH_OBJECTS);
+    /* Call max_allocation with 0 objects because otherwise it would assume
+     * worst case padding but in this function we want to be exact. */
+    size_t max_alloc = secp256k1_scratch_max_allocation(error_callback, scratch, 0);
     int bucket_window;
     size_t res = 0;
 
@@ -828,6 +851,20 @@ static size_t secp256k1_pippenger_max_points(const secp256k1_callback* error_cal
         /* Compute an upper bound for the number excluding the base point G.
          * It's an upper bound because alignment is not taken into account. */
         n_points = space_for_points / entry_size - 1;
+        /* Compute an upper bound for the number of points after subtracting
+         * space for the base point G. It's an upper bound because alignment is
+         * not taken into account. */
+        n_points = (space_for_points - entry_size)/entry_size;
+        if (n_points > 0
+              && space_for_points < secp256k1_pippenger_scratch_size_points(n_points, bucket_window, 1)) {
+            /* If there's not enough space after alignment is taken into
+             * account, it suffices to decrease n_points by one. This is because
+             * the maximum padding required is less than an entry. */
+            n_points -= 1;
+            VERIFY_CHECK(max_alloc - secp256k1_scratch_max_allocation(error_callback, scratch, PIPPENGER_SCRATCH_OBJECTS) < entry_size);
+            VERIFY_CHECK(space_for_points >= secp256k1_pippenger_scratch_size_points(n_points, bucket_window, 1));
+        }
+
         n_points = n_points > max_points ? max_points : n_points;
         if (n_points > res) {
             res = n_points;

src/tests.c

@@ -4085,6 +4085,7 @@ void test_ecmult_multi_strauss_scratch_size(void) {
     for(n_points = 0; n_points < ECMULT_PIPPENGER_THRESHOLD*2; n_points++) {
         size_t scratch_size = secp256k1_strauss_scratch_size(n_points);
         secp256k1_scratch *scratch = secp256k1_scratch_create(&ctx->error_callback, scratch_size);
+        CHECK(n_points == secp256k1_strauss_max_points(&ctx->error_callback, scratch));
         {
             secp256k1_gej *points;
             secp256k1_scalar *scalars;
@@ -4098,6 +4099,20 @@ void test_ecmult_multi_strauss_scratch_size(void) {
     }
 }
 
+/* Spot check that any scratch space is large enough to fit
+ * `strauss_max_points(scratch)` many points. */
+void test_ecmult_multi_strauss_max_points(void) {
+    size_t scratch_size = secp256k1_strauss_scratch_size_raw(1, 0);
+    size_t max_scratch_size = secp256k1_strauss_scratch_size_raw(1, 1) + 1;
+    for (; scratch_size < max_scratch_size; scratch_size++) {
+        secp256k1_scratch *scratch = secp256k1_scratch_create(&ctx->error_callback, scratch_size);
+        size_t n_points = secp256k1_strauss_max_points(&ctx->error_callback, scratch);
+        CHECK(secp256k1_scratch_max_allocation(&ctx->error_callback, scratch, 0) == scratch_size);
+        CHECK(scratch_size >= secp256k1_strauss_scratch_size(n_points));
+        secp256k1_scratch_destroy(&ctx->error_callback, scratch);
+    }
+}
+
 void test_secp256k1_pippenger_bucket_window_inv(void) {
     int i;
 
@@ -4277,6 +4292,7 @@ void run_ecmult_multi_tests(void) {
     secp256k1_scratch *scratch;
 
     test_ecmult_multi_strauss_scratch_size();
+    test_ecmult_multi_strauss_max_points();
     test_secp256k1_pippenger_bucket_window_inv();
     test_ecmult_multi_pippenger_max_points();
     scratch = secp256k1_scratch_create(&ctx->error_callback, 819200);