Skip to content

Commit

Permalink
Make fe_cmov take max of magnitudes
Browse files Browse the repository at this point in the history
  • Loading branch information
sipa committed May 15, 2023
1 parent 83186db commit 31b4bbe
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 17 deletions.
4 changes: 3 additions & 1 deletion src/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ static void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_f
*
* On input, both r and a must be valid field elements. Flag must be 0 or 1.
* Performs {r = flag ? a : r}.
* On output, r's magnitude and normalized will equal a's in case of flag=1, unchanged otherwise.
*
* On output, r's magnitude will be the maximum of both input magnitudes.
* It will be normalized if and only if both inputs were normalized.
*/
static void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag);

Expand Down
6 changes: 2 additions & 4 deletions src/field_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,8 @@ SECP256K1_INLINE static void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_
secp256k1_fe_verify(a);
secp256k1_fe_verify(r);
secp256k1_fe_impl_cmov(r, a, flag);
if (flag) {
r->magnitude = a->magnitude;
r->normalized = a->normalized;
}
if (a->magnitude > r->magnitude) r->magnitude = a->magnitude;
if (!a->normalized) r->normalized = 0;
secp256k1_fe_verify(r);
}

Expand Down
26 changes: 14 additions & 12 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -3101,10 +3101,6 @@ static void run_field_be32_overflow(void) {
/* Returns true if two field elements have the same representation. */
static int fe_identical(const secp256k1_fe *a, const secp256k1_fe *b) {
int ret = 1;
#ifdef VERIFY
ret &= (a->magnitude == b->magnitude);
ret &= (a->normalized == b->normalized);
#endif
/* Compare the struct member that holds the limbs. */
ret &= (secp256k1_memcmp_var(a->n, b->n, sizeof(a->n)) == 0);
return ret;
Expand Down Expand Up @@ -3192,16 +3188,22 @@ static void run_field_misc(void) {
q = x;
secp256k1_fe_cmov(&x, &z, 0);
#ifdef VERIFY
CHECK(x.normalized && x.magnitude == 1);
CHECK(!x.normalized);
CHECK((x.magnitude == q.magnitude) || (x.magnitude == z.magnitude));
CHECK((x.magnitude >= q.magnitude) && (x.magnitude >= z.magnitude));
#endif
x = q;
secp256k1_fe_cmov(&x, &x, 1);
CHECK(!fe_identical(&x, &z));
CHECK(fe_identical(&x, &q));
secp256k1_fe_cmov(&q, &z, 1);
#ifdef VERIFY
CHECK(!q.normalized && q.magnitude == z.magnitude);
CHECK(!q.normalized);
CHECK((q.magnitude == x.magnitude) || (q.magnitude == z.magnitude));
CHECK((q.magnitude >= x.magnitude) && (q.magnitude >= z.magnitude));
#endif
CHECK(fe_identical(&q, &z));
q = z;
secp256k1_fe_normalize_var(&x);
secp256k1_fe_normalize_var(&z);
CHECK(!secp256k1_fe_equal_var(&x, &z));
Expand All @@ -3215,7 +3217,7 @@ static void run_field_misc(void) {
secp256k1_fe_normalize_var(&q);
secp256k1_fe_cmov(&q, &z, (j&1));
#ifdef VERIFY
CHECK((q.normalized != (j&1)) && q.magnitude == ((j&1) ? z.magnitude : 1));
CHECK(!q.normalized && q.magnitude == z.magnitude);
#endif
}
secp256k1_fe_normalize_var(&z);
Expand Down Expand Up @@ -7558,23 +7560,23 @@ static void fe_cmov_test(void) {
secp256k1_fe a = zero;

secp256k1_fe_cmov(&r, &a, 0);
CHECK(secp256k1_memcmp_var(&r, &max, sizeof(r)) == 0);
CHECK(fe_identical(&r, &max));

r = zero; a = max;
secp256k1_fe_cmov(&r, &a, 1);
CHECK(secp256k1_memcmp_var(&r, &max, sizeof(r)) == 0);
CHECK(fe_identical(&r, &max));

a = zero;
secp256k1_fe_cmov(&r, &a, 1);
CHECK(secp256k1_memcmp_var(&r, &zero, sizeof(r)) == 0);
CHECK(fe_identical(&r, &zero));

a = one;
secp256k1_fe_cmov(&r, &a, 1);
CHECK(secp256k1_memcmp_var(&r, &one, sizeof(r)) == 0);
CHECK(fe_identical(&r, &one));

r = one; a = zero;
secp256k1_fe_cmov(&r, &a, 0);
CHECK(secp256k1_memcmp_var(&r, &one, sizeof(r)) == 0);
CHECK(fe_identical(&r, &one));
}

static void fe_storage_cmov_test(void) {
Expand Down

0 comments on commit 31b4bbe

Please sign in to comment.