Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: take use of secp256k1_scalar_{zero,one} constants #1330

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/bench_ecmult.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,10 @@ static void bench_ecmult_1p_teardown(void* arg, int iters) {

static void bench_ecmult_0p_g(void* arg, int iters) {
bench_data* data = (bench_data*)arg;
secp256k1_scalar zero;
int i;

secp256k1_scalar_set_int(&zero, 0);
for (i = 0; i < iters; ++i) {
secp256k1_ecmult(&data->output[i], NULL, &zero, &data->scalars[(data->offset1+i) % POINTS]);
secp256k1_ecmult(&data->output[i], NULL, &secp256k1_scalar_zero, &data->scalars[(data->offset1+i) % POINTS]);
}
}

Expand Down
8 changes: 2 additions & 6 deletions src/eckey_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp25

static int secp256k1_eckey_pubkey_tweak_add(secp256k1_ge *key, const secp256k1_scalar *tweak) {
secp256k1_gej pt;
secp256k1_scalar one;
secp256k1_gej_set_ge(&pt, key);
secp256k1_scalar_set_int(&one, 1);
secp256k1_ecmult(&pt, &pt, &one, tweak);
secp256k1_ecmult(&pt, &pt, &secp256k1_scalar_one, tweak);

if (secp256k1_gej_is_infinity(&pt)) {
return 0;
Expand All @@ -80,15 +78,13 @@ static int secp256k1_eckey_privkey_tweak_mul(secp256k1_scalar *key, const secp25
}

static int secp256k1_eckey_pubkey_tweak_mul(secp256k1_ge *key, const secp256k1_scalar *tweak) {
secp256k1_scalar zero;
secp256k1_gej pt;
if (secp256k1_scalar_is_zero(tweak)) {
return 0;
}

secp256k1_scalar_set_int(&zero, 0);
secp256k1_gej_set_ge(&pt, key);
secp256k1_ecmult(&pt, &pt, tweak, &zero);
secp256k1_ecmult(&pt, &pt, tweak, &secp256k1_scalar_zero);
secp256k1_ge_set_gej(key, &pt);
return 1;
}
Expand Down
8 changes: 2 additions & 6 deletions src/ecmult_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -770,14 +770,12 @@ static size_t secp256k1_pippenger_max_points(const secp256k1_callback* error_cal
* require a scratch space */
static int secp256k1_ecmult_multi_simple_var(secp256k1_gej *r, const secp256k1_scalar *inp_g_sc, secp256k1_ecmult_multi_callback cb, void *cbdata, size_t n_points) {
size_t point_idx;
secp256k1_scalar szero;
secp256k1_gej tmpj;

secp256k1_scalar_set_int(&szero, 0);
secp256k1_gej_set_infinity(r);
secp256k1_gej_set_infinity(&tmpj);
/* r = inp_g_sc*G */
secp256k1_ecmult(r, &tmpj, &szero, inp_g_sc);
secp256k1_ecmult(r, &tmpj, &secp256k1_scalar_zero, inp_g_sc);
for (point_idx = 0; point_idx < n_points; point_idx++) {
secp256k1_ge point;
secp256k1_gej pointj;
Expand Down Expand Up @@ -825,9 +823,7 @@ static int secp256k1_ecmult_multi_var(const secp256k1_callback* error_callback,
if (inp_g_sc == NULL && n == 0) {
return 1;
} else if (n == 0) {
secp256k1_scalar szero;
secp256k1_scalar_set_int(&szero, 0);
secp256k1_ecmult(r, r, &szero, inp_g_sc);
secp256k1_ecmult(r, r, &secp256k1_scalar_zero, inp_g_sc);
return 1;
}
if (scratch == NULL) {
Expand Down
95 changes: 48 additions & 47 deletions src/tests.c
theStack marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2304,26 +2304,23 @@ static void scalar_test(void) {

{
/* Test multiplicative identity. */
secp256k1_scalar r1, v1;
secp256k1_scalar_set_int(&v1,1);
secp256k1_scalar_mul(&r1, &s1, &v1);
secp256k1_scalar r1;
secp256k1_scalar_mul(&r1, &s1, &secp256k1_scalar_one);
CHECK(secp256k1_scalar_eq(&r1, &s1));
}

{
/* Test additive identity. */
secp256k1_scalar r1, v0;
secp256k1_scalar_set_int(&v0,0);
secp256k1_scalar_add(&r1, &s1, &v0);
secp256k1_scalar r1;
secp256k1_scalar_add(&r1, &s1, &secp256k1_scalar_zero);
CHECK(secp256k1_scalar_eq(&r1, &s1));
}

{
/* Test zero product property. */
secp256k1_scalar r1, v0;
secp256k1_scalar_set_int(&v0,0);
secp256k1_scalar_mul(&r1, &s1, &v0);
CHECK(secp256k1_scalar_eq(&r1, &v0));
secp256k1_scalar r1;
secp256k1_scalar_mul(&r1, &s1, &secp256k1_scalar_zero);
CHECK(secp256k1_scalar_eq(&r1, &secp256k1_scalar_zero));
}

}
Expand Down Expand Up @@ -2354,13 +2351,25 @@ static void run_scalar_tests(void) {
run_scalar_set_b32_seckey_tests();
}

{
/* Check that the scalar constants secp256k1_scalar_zero and
secp256k1_scalar_one contain the expected values. */
secp256k1_scalar zero, one;

CHECK(secp256k1_scalar_is_zero(&secp256k1_scalar_zero));
secp256k1_scalar_set_int(&zero, 0);
CHECK(secp256k1_scalar_eq(&zero, &secp256k1_scalar_zero));

CHECK(secp256k1_scalar_is_one(&secp256k1_scalar_one));
secp256k1_scalar_set_int(&one, 1);
CHECK(secp256k1_scalar_eq(&one, &secp256k1_scalar_one));
}

{
/* (-1)+1 should be zero. */
secp256k1_scalar s, o;
secp256k1_scalar_set_int(&s, 1);
CHECK(secp256k1_scalar_is_one(&s));
secp256k1_scalar_negate(&o, &s);
secp256k1_scalar_add(&o, &o, &s);
secp256k1_scalar o;
secp256k1_scalar_negate(&o, &secp256k1_scalar_one);
secp256k1_scalar_add(&o, &o, &secp256k1_scalar_one);
CHECK(secp256k1_scalar_is_zero(&o));
secp256k1_scalar_negate(&o, &o);
CHECK(secp256k1_scalar_is_zero(&o));
Expand All @@ -2385,7 +2394,6 @@ static void run_scalar_tests(void) {
secp256k1_scalar y;
secp256k1_scalar z;
secp256k1_scalar zz;
secp256k1_scalar one;
secp256k1_scalar r1;
secp256k1_scalar r2;
secp256k1_scalar zzv;
Expand Down Expand Up @@ -2922,7 +2930,6 @@ static void run_scalar_tests(void) {
0x1e, 0x86, 0x5d, 0x89, 0x63, 0xe6, 0x0a, 0x46,
0x5c, 0x02, 0x97, 0x1b, 0x62, 0x43, 0x86, 0xf5}}
};
secp256k1_scalar_set_int(&one, 1);
for (i = 0; i < 33; i++) {
secp256k1_scalar_set_b32(&x, chal[i][0], &overflow);
CHECK(!overflow);
Expand All @@ -2945,7 +2952,7 @@ static void run_scalar_tests(void) {
CHECK(secp256k1_scalar_eq(&x, &z));
secp256k1_scalar_mul(&zz, &zz, &y);
CHECK(!secp256k1_scalar_check_overflow(&zz));
CHECK(secp256k1_scalar_eq(&one, &zz));
CHECK(secp256k1_scalar_eq(&secp256k1_scalar_one, &zz));
}
}
}
Expand Down Expand Up @@ -4643,7 +4650,6 @@ static int ecmult_multi_false_callback(secp256k1_scalar *sc, secp256k1_ge *pt, s

static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi_func ecmult_multi) {
int ncount;
secp256k1_scalar szero;
secp256k1_scalar sc[32];
secp256k1_ge pt[32];
secp256k1_gej r;
Expand All @@ -4652,7 +4658,6 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi

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

/* No points to multiply */
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, NULL, ecmult_multi_callback, &data, 0));
Expand All @@ -4670,21 +4675,21 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
pt[1] = secp256k1_ge_const_g;

/* only G scalar */
secp256k1_ecmult(&r2, &ptgj, &szero, &sc[0]);
secp256k1_ecmult(&r2, &ptgj, &secp256k1_scalar_zero, &sc[0]);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &sc[0], ecmult_multi_callback, &data, 0));
CHECK(secp256k1_gej_eq_var(&r, &r2));

/* 1-point */
secp256k1_ecmult(&r2, &ptgj, &sc[0], &szero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 1));
secp256k1_ecmult(&r2, &ptgj, &sc[0], &secp256k1_scalar_zero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 1));
CHECK(secp256k1_gej_eq_var(&r, &r2));

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

/* 2-point */
secp256k1_ecmult(&r2, &ptgj, &sc[0], &sc[1]);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 2));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 2));
CHECK(secp256k1_gej_eq_var(&r, &r2));

/* 2-point with G scalar */
Expand All @@ -4704,7 +4709,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
random_scalar_order(&sc[i]);
secp256k1_ge_set_infinity(&pt[i]);
}
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, sizes[j]));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, sizes[j]));
CHECK(secp256k1_gej_is_infinity(&r));
}

Expand All @@ -4714,7 +4719,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
pt[i] = ptg;
secp256k1_scalar_set_int(&sc[i], 0);
}
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, sizes[j]));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, sizes[j]));
CHECK(secp256k1_gej_is_infinity(&r));
}

Expand All @@ -4727,7 +4732,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
pt[2 * i + 1] = ptg;
}

CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, sizes[j]));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, sizes[j]));
CHECK(secp256k1_gej_is_infinity(&r));

random_scalar_order(&sc[0]);
Expand All @@ -4740,7 +4745,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
secp256k1_ge_neg(&pt[2*i+1], &pt[2*i]);
}

CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, sizes[j]));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, sizes[j]));
CHECK(secp256k1_gej_is_infinity(&r));
}

Expand All @@ -4755,7 +4760,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
secp256k1_scalar_negate(&sc[i], &sc[i]);
}

CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 32));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 32));
CHECK(secp256k1_gej_is_infinity(&r));
}

Expand All @@ -4773,8 +4778,8 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
secp256k1_gej_add_ge_var(&r, &r, &pt[i], NULL);
}

secp256k1_ecmult(&r2, &r, &sc[0], &szero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 20));
secp256k1_ecmult(&r2, &r, &sc[0], &secp256k1_scalar_zero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 20));
CHECK(secp256k1_gej_eq_var(&r, &r2));
}

Expand All @@ -4794,8 +4799,8 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
}

secp256k1_gej_set_ge(&p0j, &pt[0]);
secp256k1_ecmult(&r2, &p0j, &rs, &szero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 20));
secp256k1_ecmult(&r2, &p0j, &rs, &secp256k1_scalar_zero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 20));
CHECK(secp256k1_gej_eq_var(&r, &r2));
}

Expand All @@ -4806,13 +4811,13 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
}

secp256k1_scalar_clear(&sc[0]);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 20));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 20));
secp256k1_scalar_clear(&sc[1]);
secp256k1_scalar_clear(&sc[2]);
secp256k1_scalar_clear(&sc[3]);
secp256k1_scalar_clear(&sc[4]);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 6));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &szero, ecmult_multi_callback, &data, 5));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 6));
CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 5));
CHECK(secp256k1_gej_is_infinity(&r));

/* Run through s0*(t0*P) + s1*(t1*P) exhaustively for many small values of s0, s1, t0, t1 */
Expand All @@ -4836,8 +4841,8 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
secp256k1_scalar_set_int(&t1, (t1i + 1) / 2);
secp256k1_scalar_cond_negate(&t1, t1i & 1);

secp256k1_ecmult(&t0p, &ptgj, &t0, &szero);
secp256k1_ecmult(&t1p, &ptgj, &t1, &szero);
secp256k1_ecmult(&t0p, &ptgj, &t0, &secp256k1_scalar_zero);
secp256k1_ecmult(&t1p, &ptgj, &t1, &secp256k1_scalar_zero);

for(s0i = 0; s0i < TOP; s0i++) {
for(s1i = 0; s1i < TOP; s1i++) {
Expand All @@ -4856,8 +4861,8 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi
secp256k1_scalar_mul(&tmp2, &t1, &sc[1]);
secp256k1_scalar_add(&tmp1, &tmp1, &tmp2);

secp256k1_ecmult(&expected, &ptgj, &tmp1, &szero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &actual, &szero, ecmult_multi_callback, &data, 2));
secp256k1_ecmult(&expected, &ptgj, &tmp1, &secp256k1_scalar_zero);
CHECK(ecmult_multi(&CTX->error_callback, scratch, &actual, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 2));
CHECK(secp256k1_gej_eq_var(&actual, &expected));
}
}
Expand Down Expand Up @@ -5033,7 +5038,6 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) {
}

static void test_ecmult_multi_batch_single(secp256k1_ecmult_multi_func ecmult_multi) {
secp256k1_scalar szero;
secp256k1_scalar sc;
secp256k1_ge pt;
secp256k1_gej r;
Expand All @@ -5044,11 +5048,10 @@ static void test_ecmult_multi_batch_single(secp256k1_ecmult_multi_func ecmult_mu
random_scalar_order(&sc);
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->error_callback, scratch_empty, &r, &szero, ecmult_multi_callback, &data, 1));
CHECK(!ecmult_multi(&CTX->error_callback, scratch_empty, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 1));
secp256k1_scratch_destroy(&CTX->error_callback, scratch_empty);
}

Expand Down Expand Up @@ -5156,7 +5159,6 @@ static void test_ecmult_multi_batch_size_helper(void) {
static void test_ecmult_multi_batching(void) {
static const int n_points = 2*ECMULT_PIPPENGER_THRESHOLD;
secp256k1_scalar scG;
secp256k1_scalar szero;
secp256k1_scalar *sc = (secp256k1_scalar *)checked_malloc(&CTX->error_callback, sizeof(secp256k1_scalar) * n_points);
secp256k1_ge *pt = (secp256k1_ge *)checked_malloc(&CTX->error_callback, sizeof(secp256k1_ge) * n_points);
secp256k1_gej r;
Expand All @@ -5166,11 +5168,10 @@ static void test_ecmult_multi_batching(void) {
secp256k1_scratch *scratch;

secp256k1_gej_set_infinity(&r2);
secp256k1_scalar_set_int(&szero, 0);

/* Get random scalars and group elements and compute result */
random_scalar_order(&scG);
secp256k1_ecmult(&r2, &r2, &szero, &scG);
secp256k1_ecmult(&r2, &r2, &secp256k1_scalar_zero, &scG);
for(i = 0; i < n_points; i++) {
secp256k1_ge ptg;
secp256k1_gej ptgj;
Expand Down