Merge #95: [upstream PR #741]: Remove unnecessary sign variable from wnaf_const

37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0 Remove unnecessary sign variable from wnaf_const (Jonas Nick)
6bb0b77e158fc2f9e56e4b65b08bcb660d4c588b Fix test_constant_wnaf for -1 and add a test for it. (Jonas Nick)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK 37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0

Tree-SHA512: 4a529579f04dfa8d5abded16cbc0ad2747ccdbef41501c984c348ffd154afdd28333d739db60c869410211187850e3c05dfe91dfb184ad8dca8d5c59b3a158ed
This commit is contained in:
Jonas Nick 2020-09-25 21:17:16 +00:00
commit a39b08d672
No known key found for this signature in database
GPG Key ID: 4861DBF262123605
2 changed files with 33 additions and 6 deletions

View File

@ -105,16 +105,22 @@ static int secp256k1_wnaf_const(int *wnaf, const secp256k1_scalar *scalar, int w
/* 4 */
u_last = secp256k1_scalar_shr_int(&s, w);
do {
int sign;
int even;
/* 4.1 4.4 */
u = secp256k1_scalar_shr_int(&s, w);
/* 4.2 */
even = ((u & 1) == 0);
sign = 2 * (u_last > 0) - 1;
u += sign * even;
u_last -= sign * even * (1 << w);
/* In contrast to the original algorithm, u_last is always > 0 and
* therefore we do not need to check its sign. In particular, it's easy
* to see that u_last is never < 0 because u is never < 0. Moreover,
* u_last is never = 0 because u is never even after a loop
* iteration. The same holds analogously for the initial value of
* u_last (in the first loop iteration). */
VERIFY_CHECK(u_last > 0);
VERIFY_CHECK((u_last & 1) == 1);
u += even;
u_last -= even * (1 << w);
/* 4.3, adapted for global sign change */
wnaf[word++] = u_last * global_sign;

View File

@ -3390,6 +3390,7 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
int skew;
int bits = 256;
secp256k1_scalar num = *number;
secp256k1_scalar scalar_skew;
secp256k1_scalar_set_int(&x, 0);
secp256k1_scalar_set_int(&shift, 1 << w);
@ -3420,7 +3421,8 @@ void test_constant_wnaf(const secp256k1_scalar *number, int w) {
secp256k1_scalar_add(&x, &x, &t);
}
/* Skew num because when encoding numbers as odd we use an offset */
secp256k1_scalar_cadd_bit(&num, skew == 2, 1);
secp256k1_scalar_set_int(&scalar_skew, 1 << (skew == 2));
secp256k1_scalar_add(&num, &num, &scalar_skew);
CHECK(secp256k1_scalar_eq(&x, &num));
}
@ -3532,13 +3534,32 @@ void run_wnaf(void) {
int i;
secp256k1_scalar n = {{0}};
test_constant_wnaf(&n, 4);
/* Sanity check: 1 and 2 are the smallest odd and even numbers and should
* have easier-to-diagnose failure modes */
n.d[0] = 1;
test_constant_wnaf(&n, 4);
n.d[0] = 2;
test_constant_wnaf(&n, 4);
/* Test 0 */
/* Test -1, because it's a special case in wnaf_const */
n = secp256k1_scalar_one;
secp256k1_scalar_negate(&n, &n);
test_constant_wnaf(&n, 4);
/* Test -2, which may not lead to overflows in wnaf_const */
secp256k1_scalar_add(&n, &secp256k1_scalar_one, &secp256k1_scalar_one);
secp256k1_scalar_negate(&n, &n);
test_constant_wnaf(&n, 4);
/* Test (1/2) - 1 = 1/-2 and 1/2 = (1/-2) + 1
as corner cases of negation handling in wnaf_const */
secp256k1_scalar_inverse(&n, &n);
test_constant_wnaf(&n, 4);
secp256k1_scalar_add(&n, &n, &secp256k1_scalar_one);
test_constant_wnaf(&n, 4);
/* Test 0 for fixed wnaf */
test_fixed_wnaf_small();
/* Random tests */
for (i = 0; i < count; i++) {