Skip to content

Commit

Permalink
Merge dashpay#592: Use trivial algorithm in ecmult_multi if scratch s…
Browse files Browse the repository at this point in the history
…pace is small

9ab96f7 Use trivial algorithm in ecmult_multi if scratch space is small (Jonas Nick)

Pull request description:

  `ecmult_multi` already selects the trivial algorithm if the scratch space is NULL. With this PR the trivial algorithm is also selected if the scratch space is too small to use pippenger or strauss instead of returning 0. That makes it more easier to avoid consensus relevant inconsistencies just because scratch space construction was messed up.

ACKs for commit 9ab96f:
  real-or-random:
    utACK 9ab96f7

Tree-SHA512: aa451adf8880af15cf167a59cb07fc411edc43f26c8eb0873bdae2774382ba182e2a1c54487912f8f2999cb0402d554b9d293e2fb9483234471348a1f43c6653
  • Loading branch information
gmaxwell committed May 25, 2019
2 parents a484e00 + 9ab96f7 commit 40839e2
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 15 deletions.
10 changes: 6 additions & 4 deletions src/ecmult_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1174,16 +1174,18 @@ static int secp256k1_ecmult_multi_var(const secp256k1_ecmult_context *ctx, secp2
return secp256k1_ecmult_multi_simple_var(ctx, r, inp_g_sc, cb, cbdata, n);
}

/* Compute the batch sizes for pippenger given a scratch space. If it's greater than a threshold
* use pippenger. Otherwise use strauss */
/* Compute the batch sizes for Pippenger's algorithm given a scratch space. If it's greater than
* a threshold use Pippenger's algorithm. Otherwise use Strauss' algorithm.
* As a first step check if there's enough space for Pippenger's algo (which requires less space
* than Strauss' algo) and if not, use the simple algorithm. */
if (!secp256k1_ecmult_multi_batch_size_helper(&n_batches, &n_batch_points, secp256k1_pippenger_max_points(scratch), n)) {
return 0;
return secp256k1_ecmult_multi_simple_var(ctx, r, inp_g_sc, cb, cbdata, n);
}
if (n_batch_points >= ECMULT_PIPPENGER_THRESHOLD) {
f = secp256k1_ecmult_pippenger_batch;
} else {
if (!secp256k1_ecmult_multi_batch_size_helper(&n_batches, &n_batch_points, secp256k1_strauss_max_points(scratch), n)) {
return 0;
return secp256k1_ecmult_multi_simple_var(ctx, r, inp_g_sc, cb, cbdata, n);
}
f = secp256k1_ecmult_strauss_batch;
}
Expand Down
42 changes: 31 additions & 11 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -2649,7 +2649,6 @@ void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi_func e
secp256k1_gej r;
secp256k1_gej r2;
ecmult_multi_data data;
secp256k1_scratch *scratch_empty;

data.sc = sc;
data.pt = pt;
Expand Down Expand Up @@ -2684,11 +2683,6 @@ void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi_func e
secp256k1_gej_add_var(&r, &r, &r2, NULL);
CHECK(secp256k1_gej_is_infinity(&r));

/* Try to multiply 1 point, but scratch space is empty */
scratch_empty = secp256k1_scratch_create(&ctx->error_callback, 0);
CHECK(!ecmult_multi(&ctx->ecmult_ctx, scratch_empty, &r, &szero, ecmult_multi_callback, &data, 1));
secp256k1_scratch_destroy(scratch_empty);

/* Try to multiply 1 point, but callback returns false */
CHECK(!ecmult_multi(&ctx->ecmult_ctx, scratch, &r, &szero, ecmult_multi_false_callback, &data, 1));

Expand Down Expand Up @@ -2886,6 +2880,24 @@ void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi_func e
}
}

void test_ecmult_multi_batch_single(secp256k1_ecmult_multi_func ecmult_multi) {
secp256k1_scalar szero;
secp256k1_scalar sc[32];
secp256k1_ge pt[32];
secp256k1_gej r;
ecmult_multi_data data;
secp256k1_scratch *scratch_empty;

data.sc = sc;
data.pt = pt;
secp256k1_scalar_set_int(&szero, 0);

/* Try to multiply 1 point, but scratch space is empty.*/
scratch_empty = secp256k1_scratch_create(&ctx->error_callback, 0);
CHECK(!ecmult_multi(&ctx->ecmult_ctx, scratch_empty, &r, &szero, ecmult_multi_callback, &data, 1));
secp256k1_scratch_destroy(scratch_empty);
}

void test_secp256k1_pippenger_bucket_window_inv(void) {
int i;

Expand Down Expand Up @@ -3009,19 +3021,25 @@ void test_ecmult_multi_batching(void) {
}
data.sc = sc;
data.pt = pt;
secp256k1_gej_neg(&r2, &r2);

/* Test with empty scratch space */
/* Test with empty scratch space. It should compute the correct result using
* ecmult_mult_simple algorithm which doesn't require a scratch space. */
scratch = secp256k1_scratch_create(&ctx->error_callback, 0);
CHECK(!secp256k1_ecmult_multi_var(&ctx->ecmult_ctx, scratch, &r, &scG, ecmult_multi_callback, &data, 1));
CHECK(secp256k1_ecmult_multi_var(&ctx->ecmult_ctx, scratch, &r, &scG, ecmult_multi_callback, &data, n_points));
secp256k1_gej_add_var(&r, &r, &r2, NULL);
CHECK(secp256k1_gej_is_infinity(&r));
secp256k1_scratch_destroy(scratch);

/* Test with space for 1 point in pippenger. That's not enough because
* ecmult_multi selects strauss which requires more memory. */
* ecmult_multi selects strauss which requires more memory. It should
* therefore select the simple algorithm. */
scratch = secp256k1_scratch_create(&ctx->error_callback, secp256k1_pippenger_scratch_size(1, 1) + PIPPENGER_SCRATCH_OBJECTS*ALIGNMENT);
CHECK(!secp256k1_ecmult_multi_var(&ctx->ecmult_ctx, scratch, &r, &scG, ecmult_multi_callback, &data, 1));
CHECK(secp256k1_ecmult_multi_var(&ctx->ecmult_ctx, scratch, &r, &scG, ecmult_multi_callback, &data, n_points));
secp256k1_gej_add_var(&r, &r, &r2, NULL);
CHECK(secp256k1_gej_is_infinity(&r));
secp256k1_scratch_destroy(scratch);

secp256k1_gej_neg(&r2, &r2);
for(i = 1; i <= n_points; i++) {
if (i > ECMULT_PIPPENGER_THRESHOLD) {
int bucket_window = secp256k1_pippenger_bucket_window(i);
Expand Down Expand Up @@ -3049,7 +3067,9 @@ void run_ecmult_multi_tests(void) {
test_ecmult_multi(scratch, secp256k1_ecmult_multi_var);
test_ecmult_multi(NULL, secp256k1_ecmult_multi_var);
test_ecmult_multi(scratch, secp256k1_ecmult_pippenger_batch_single);
test_ecmult_multi_batch_single(secp256k1_ecmult_pippenger_batch_single);
test_ecmult_multi(scratch, secp256k1_ecmult_strauss_batch_single);
test_ecmult_multi_batch_single(secp256k1_ecmult_strauss_batch_single);
secp256k1_scratch_destroy(scratch);

/* Run test_ecmult_multi with space for exactly one point */
Expand Down

0 comments on commit 40839e2

Please sign in to comment.