Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1205: field: Improve docs +tests of secp…
Browse files Browse the repository at this point in the history 
…256k1_fe_set_b32

162da73 tests: Add debug helper for printing buffers (Tim Ruffing)
e9fd3df field: Improve docs and tests of secp256k1_fe_set_b32 (Tim Ruffing)
ca92a35 field: Simplify code in secp256k1_fe_set_b32 (Tim Ruffing)
d93f62e field: Verify field element even after secp256k1_fe_set_b32 fails (Tim Ruffing)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK 162da73

Tree-SHA512: b3ed8e45c969d0420275ff154462f3820b72b57832ccba1f6f427e0cfd9cff3e27440c20994f69ea33a576b1903eb7f04a989f0dbd574bbd96ee56c6dd4500f7
  • Loading branch information
@jonasnick
jonasnick committed Apr 21, 2023
2 parents f6bef03 + 162da73 commit 1f33bb2
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 13 deletions.
4 changes: 3 additions & 1 deletion src/field.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b);
/** Compare two field elements. Requires both inputs to be normalized */
static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b);

/** Set a field element equal to 32-byte big endian value. If successful, the resulting field element is normalized. */
/** Set a field element equal to 32-byte big endian value.
* Returns 1 if no overflow occurred, and then the output is normalized.
* Returns 0 if overflow occurred, and then the output is only weakly normalized. */
static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a);

/** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */
Expand Down
8 changes: 2 additions & 6 deletions src/field_10x26_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,8 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) {
ret = !((r->n[9] == 0x3FFFFFUL) & ((r->n[8] & r->n[7] & r->n[6] & r->n[5] & r->n[4] & r->n[3] & r->n[2]) == 0x3FFFFFFUL) & ((r->n[1] + 0x40UL + ((r->n[0] + 0x3D1UL) >> 26)) > 0x3FFFFFFUL));
#ifdef VERIFY
r->magnitude = 1;
if (ret) {
r->normalized = 1;
secp256k1_fe_verify(r);
} else {
r->normalized = 0;
}
r->normalized = ret;
secp256k1_fe_verify(r);
#endif
return ret;
}
Expand Down
8 changes: 2 additions & 6 deletions src/field_5x52_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,8 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) {
ret = !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL));
#ifdef VERIFY
r->magnitude = 1;
if (ret) {
r->normalized = 1;
secp256k1_fe_verify(r);
} else {
r->normalized = 0;
}
r->normalized = ret;
secp256k1_fe_verify(r);
#endif
return ret;
}
Expand Down
83 changes: 83 additions & 0 deletions src/tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,25 @@ static int all_bytes_equal(const void* s, unsigned char value, size_t n) {
return 1;
}

/* Debug helper for printing arrays of unsigned char. */
#define PRINT_BUF(buf, len) do { \
printf("%s[%lu] = ", #buf, (unsigned long)len); \
print_buf_plain(buf, len); \
} while(0);
static void print_buf_plain(const unsigned char *buf, size_t len) {
size_t i;
printf("{");
for (i = 0; i < len; i++) {
if (i % 8 == 0) {
printf("\n ");
} else {
printf(" ");
}
printf("0x%02X,", buf[i]);
}
printf("\n}\n");
}

/* TODO Use CHECK_ILLEGAL(_VOID) everywhere and get rid of the uncounting callback */
/* CHECK that expr_or_stmt calls the illegal callback of ctx exactly once
*
Expand Down Expand Up @@ -3027,6 +3046,69 @@ static void run_field_convert(void) {
CHECK(secp256k1_memcmp_var(&fes2, &fes, sizeof(fes)) == 0);
}

static void run_field_be32_overflow(void) {
{
static const unsigned char zero_overflow[32] = {
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFC, 0x2F,
};
static const unsigned char zero[32] = { 0x00 };
unsigned char out[32];
secp256k1_fe fe;
CHECK(secp256k1_fe_set_b32(&fe, zero_overflow) == 0);
CHECK(secp256k1_fe_normalizes_to_zero(&fe) == 1);
secp256k1_fe_normalize(&fe);
CHECK(secp256k1_fe_is_zero(&fe) == 1);
secp256k1_fe_get_b32(out, &fe);
CHECK(secp256k1_memcmp_var(out, zero, 32) == 0);
}
{
static const unsigned char one_overflow[32] = {
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFC, 0x30,
};
static const unsigned char one[32] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
};
unsigned char out[32];
secp256k1_fe fe;
CHECK(secp256k1_fe_set_b32(&fe, one_overflow) == 0);
secp256k1_fe_normalize(&fe);
CHECK(secp256k1_fe_cmp_var(&fe, &secp256k1_fe_one) == 0);
secp256k1_fe_get_b32(out, &fe);
CHECK(secp256k1_memcmp_var(out, one, 32) == 0);
}
{
static const unsigned char ff_overflow[32] = {
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
};
static const unsigned char ff[32] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x03, 0xD0,
};
unsigned char out[32];
secp256k1_fe fe;
const secp256k1_fe fe_ff = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0x01, 0x000003d0);
CHECK(secp256k1_fe_set_b32(&fe, ff_overflow) == 0);
secp256k1_fe_normalize(&fe);
CHECK(secp256k1_fe_cmp_var(&fe, &fe_ff) == 0);
secp256k1_fe_get_b32(out, &fe);
CHECK(secp256k1_memcmp_var(out, ff, 32) == 0);
}
}

/* 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;
Expand Down Expand Up @@ -7693,6 +7775,7 @@ int main(int argc, char **argv) {
run_field_half();
run_field_misc();
run_field_convert();
run_field_be32_overflow();
run_fe_mul();
run_sqr();
run_sqrt();
Expand Down

0 comments on commit 1f33bb2

Please sign in to comment.