Skip to content

Commit 0cbc860

Browse files
sipasipa
committed
Merge pull request bitcoin#266
3f3964e Add specific VERIFY tests for _fe_cmov (Peter Dettman) a0601cd Fix VERIFY calculations in _fe_cmov methods (Peter Dettman)
2 parents 06ff7fe + 3f3964e commit 0cbc860

File tree

3 files changed

+38
-14
lines changed

3 files changed

+38
-14
lines changed

‎src/field_10x26_impl.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,8 +1083,10 @@ static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe_t *r, const secp256k
10831083
r->n[8] = (r->n[8] & mask0) | (a->n[8] & mask1);
10841084
r->n[9] = (r->n[9] & mask0) | (a->n[9] & mask1);
10851085
#ifdef VERIFY
1086-
r->magnitude = (r->magnitude & mask0) | (a->magnitude & mask1);
1087-
r->normalized = (r->normalized & mask0) | (a->normalized & mask1);
1086+
if (a->magnitude > r->magnitude) {
1087+
r->magnitude = a->magnitude;
1088+
}
1089+
r->normalized &= a->normalized;
10881090
#endif
10891091
}
10901092

‎src/field_5x52_impl.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,10 @@ static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe_t *r, const secp256k
414414
r->n[3] = (r->n[3] & mask0) | (a->n[3] & mask1);
415415
r->n[4] = (r->n[4] & mask0) | (a->n[4] & mask1);
416416
#ifdef VERIFY
417-
r->magnitude = (r->magnitude & mask0) | (a->magnitude & mask1);
418-
r->normalized = (r->normalized & mask0) | (a->normalized & mask1);
417+
if (a->magnitude > r->magnitude) {
418+
r->magnitude = a->magnitude;
419+
}
420+
r->normalized &= a->normalized;
419421
#endif
420422
}
421423

‎src/tests.c‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ void random_field_element_magnitude(secp256k1_fe_t *fe) {
4747
secp256k1_fe_negate(&zero, &zero, 0);
4848
secp256k1_fe_mul_int(&zero, n - 1);
4949
secp256k1_fe_add(fe, &zero);
50-
#ifdef VERIFY
51-
CHECK(fe->magnitude == n);
52-
#endif
50+
VERIFY_CHECK(fe->magnitude == n);
5351
}
5452

5553
void random_group_element_test(secp256k1_ge_t *ge) {
@@ -737,13 +735,22 @@ void run_field_convert(void) {
737735
CHECK(memcmp(&fes2, &fes, sizeof(fes)) == 0);
738736
}
739737

738+
int fe_memcmp(const secp256k1_fe_t *a, const secp256k1_fe_t *b) {
739+
secp256k1_fe_t t = *b;
740+
#ifdef VERIFY
741+
t.magnitude = a->magnitude;
742+
t.normalized = a->normalized;
743+
#endif
744+
return memcmp(a, &t, sizeof(secp256k1_fe_t));
745+
}
746+
740747
void run_field_misc(void) {
741748
secp256k1_fe_t x;
742749
secp256k1_fe_t y;
743750
secp256k1_fe_t z;
744751
secp256k1_fe_t q;
745752
secp256k1_fe_t fe5 = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0, 5);
746-
int i;
753+
int i, j;
747754
for (i = 0; i < 5*count; i++) {
748755
secp256k1_fe_storage_t xs, ys, zs;
749756
random_fe(&x);
@@ -756,14 +763,27 @@ void run_field_misc(void) {
756763
/* Test fe conditional move; z is not normalized here. */
757764
q = x;
758765
secp256k1_fe_cmov(&x, &z, 0);
766+
VERIFY_CHECK(!x.normalized && x.magnitude == z.magnitude);
759767
secp256k1_fe_cmov(&x, &x, 1);
760-
CHECK(memcmp(&x, &z, sizeof(x)) != 0);
761-
CHECK(memcmp(&x, &q, sizeof(x)) == 0);
768+
CHECK(fe_memcmp(&x, &z) != 0);
769+
CHECK(fe_memcmp(&x, &q) == 0);
762770
secp256k1_fe_cmov(&q, &z, 1);
763-
CHECK(memcmp(&q, &z, sizeof(q)) == 0);
764-
/* Test storage conversion and conditional moves. */
765-
secp256k1_fe_normalize(&z);
771+
VERIFY_CHECK(!q.normalized && q.magnitude == z.magnitude);
772+
CHECK(fe_memcmp(&q, &z) == 0);
773+
secp256k1_fe_normalize_var(&x);
774+
secp256k1_fe_normalize_var(&z);
766775
CHECK(!secp256k1_fe_equal_var(&x, &z));
776+
secp256k1_fe_normalize_var(&q);
777+
secp256k1_fe_cmov(&q, &z, (i&1));
778+
VERIFY_CHECK(q.normalized && q.magnitude == 1);
779+
for (j = 0; j < 6; j++) {
780+
secp256k1_fe_negate(&z, &z, j+1);
781+
secp256k1_fe_normalize_var(&q);
782+
secp256k1_fe_cmov(&q, &z, (j&1));
783+
VERIFY_CHECK(!q.normalized && q.magnitude == (j+2));
784+
}
785+
secp256k1_fe_normalize_var(&z);
786+
/* Test storage conversion and conditional moves. */
767787
secp256k1_fe_to_storage(&xs, &x);
768788
secp256k1_fe_to_storage(&ys, &y);
769789
secp256k1_fe_to_storage(&zs, &z);
@@ -1661,7 +1681,7 @@ void test_ecdsa_end_to_end(void) {
16611681
extra[31] = 0;
16621682
extra[0] = 1;
16631683
CHECK(secp256k1_ecdsa_sign(ctx, message, signature4, &signaturelen4, privkey, NULL, extra) == 1);
1664-
CHECK(signaturelen3 > 0);
1684+
CHECK(signaturelen4 > 0);
16651685
CHECK((signaturelen != signaturelen2) || (memcmp(signature, signature2, signaturelen) != 0));
16661686
CHECK((signaturelen != signaturelen3) || (memcmp(signature, signature3, signaturelen) != 0));
16671687
CHECK((signaturelen3 != signaturelen2) || (memcmp(signature3, signature2, signaturelen3) != 0));

0 commit comments

Comments
 (0)