Skip to content

Commit

Permalink
Merge #850: add secp256k1_ec_pubkey_cmp method
Browse files Browse the repository at this point in the history
6eceec6 add `secp256k1_xonly_pubkey_cmp` method (Andrew Poelstra)
0d9561a add `secp256k1_ec_pubkey_cmp` method (Andrew Poelstra)

Pull request description:

ACKs for top commit:
  elichai:
    Code review ACK 6eceec6
  jonasnick:
    ACK 6eceec6
  real-or-random:
    ACK 6eceec6

Tree-SHA512: f95cbf65f16c88a4adfa1ea7cc6ddabab14baa3b68fa069e78e6faad4852cdbfaea42ee72590d2e0b8f3159cf9b37969511550eb6b2d256b101e2147711cc817
  • Loading branch information
jonasnick committed May 13, 2021
2 parents 1e78c18 + 6eceec6 commit 202a030
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 5 deletions.
20 changes: 18 additions & 2 deletions include/secp256k1.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ typedef struct secp256k1_scratch_space_struct secp256k1_scratch_space;
* The exact representation of data inside is implementation defined and not
* guaranteed to be portable between different platforms or versions. It is
* however guaranteed to be 64 bytes in size, and can be safely copied/moved.
* If you need to convert to a format suitable for storage, transmission, or
* comparison, use secp256k1_ec_pubkey_serialize and secp256k1_ec_pubkey_parse.
* If you need to convert to a format suitable for storage or transmission,
* use secp256k1_ec_pubkey_serialize and secp256k1_ec_pubkey_parse. To
* compare keys, use secp256k1_ec_pubkey_cmp.
*/
typedef struct {
unsigned char data[64];
Expand Down Expand Up @@ -383,6 +384,21 @@ SECP256K1_API int secp256k1_ec_pubkey_serialize(
unsigned int flags
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);

/** Compare two public keys using lexicographic (of compressed serialization) order
*
* Returns: <0 if the first public key is less than the second
* >0 if the first public key is greater than the second
* 0 if the two public keys are equal
* Args: ctx: a secp256k1 context object.
* In: pubkey1: first public key to compare
* pubkey2: second public key to compare
*/
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_cmp(
const secp256k1_context* ctx,
const secp256k1_pubkey* pubkey1,
const secp256k1_pubkey* pubkey2
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);

/** Parse an ECDSA signature in compact (64 bytes) format.
*
* Returns: 1 when the signature could be parsed, 0 otherwise.
Expand Down
21 changes: 18 additions & 3 deletions include/secp256k1_extrakeys.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ extern "C" {
* The exact representation of data inside is implementation defined and not
* guaranteed to be portable between different platforms or versions. It is
* however guaranteed to be 64 bytes in size, and can be safely copied/moved.
* If you need to convert to a format suitable for storage, transmission, or
* comparison, use secp256k1_xonly_pubkey_serialize and
* secp256k1_xonly_pubkey_parse.
* If you need to convert to a format suitable for storage, transmission, use
* use secp256k1_xonly_pubkey_serialize and secp256k1_xonly_pubkey_parse. To
* compare keys, use secp256k1_xonly_pubkey_cmp.
*/
typedef struct {
unsigned char data[64];
Expand Down Expand Up @@ -67,6 +67,21 @@ SECP256K1_API int secp256k1_xonly_pubkey_serialize(
const secp256k1_xonly_pubkey* pubkey
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);

/** Compare two x-only public keys using lexicographic order
*
* Returns: <0 if the first public key is less than the second
* >0 if the first public key is greater than the second
* 0 if the two public keys are equal
* Args: ctx: a secp256k1 context object.
* In: pubkey1: first public key to compare
* pubkey2: second public key to compare
*/
SECP256K1_API int secp256k1_xonly_pubkey_cmp(
const secp256k1_context* ctx,
const secp256k1_xonly_pubkey* pk1,
const secp256k1_xonly_pubkey* pk2
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);

/** Converts a secp256k1_pubkey into a secp256k1_xonly_pubkey.
*
* Returns: 1 if the public key was successfully converted
Expand Down
26 changes: 26 additions & 0 deletions src/modules/extrakeys/main_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,32 @@ int secp256k1_xonly_pubkey_serialize(const secp256k1_context* ctx, unsigned char
return 1;
}

int secp256k1_xonly_pubkey_cmp(const secp256k1_context* ctx, const secp256k1_xonly_pubkey* pk0, const secp256k1_xonly_pubkey* pk1) {
unsigned char out[2][32];
const secp256k1_xonly_pubkey* pk[2];
int i;

VERIFY_CHECK(ctx != NULL);
pk[0] = pk0; pk[1] = pk1;
for (i = 0; i < 2; i++) {
/* If the public key is NULL or invalid, xonly_pubkey_serialize will
* call the illegal_callback and return 0. In that case we will
* serialize the key as all zeros which is less than any valid public
* key. This results in consistent comparisons even if NULL or invalid
* pubkeys are involved and prevents edge cases such as sorting
* algorithms that use this function and do not terminate as a
* result. */
if (!secp256k1_xonly_pubkey_serialize(ctx, out[i], pk[i])) {
/* Note that xonly_pubkey_serialize should already set the output to
* zero in that case, but it's not guaranteed by the API, we can't
* test it and writing a VERIFY_CHECK is more complex than
* explicitly memsetting (again). */
memset(out[i], 0, sizeof(out[i]));
}
}
return secp256k1_memcmp_var(out[0], out[1], sizeof(out[1]));
}

/** Keeps a group element as is if it has an even Y and otherwise negates it.
* y_parity is set to 0 in the former case and to 1 in the latter case.
* Requires that the coordinates of r are normalized. */
Expand Down
38 changes: 38 additions & 0 deletions src/modules/extrakeys/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,43 @@ void test_xonly_pubkey(void) {
secp256k1_context_destroy(verify);
}

void test_xonly_pubkey_comparison(void) {
unsigned char pk1_ser[32] = {
0x58, 0x84, 0xb3, 0xa2, 0x4b, 0x97, 0x37, 0x88, 0x92, 0x38, 0xa6, 0x26, 0x62, 0x52, 0x35, 0x11,
0xd0, 0x9a, 0xa1, 0x1b, 0x80, 0x0b, 0x5e, 0x93, 0x80, 0x26, 0x11, 0xef, 0x67, 0x4b, 0xd9, 0x23
};
const unsigned char pk2_ser[32] = {
0xde, 0x36, 0x0e, 0x87, 0x59, 0x8f, 0x3c, 0x01, 0x36, 0x2a, 0x2a, 0xb8, 0xc6, 0xf4, 0x5e, 0x4d,
0xb2, 0xc2, 0xd5, 0x03, 0xa7, 0xf9, 0xf1, 0x4f, 0xa8, 0xfa, 0x95, 0xa8, 0xe9, 0x69, 0x76, 0x1c
};
secp256k1_xonly_pubkey pk1;
secp256k1_xonly_pubkey pk2;
int ecount = 0;
secp256k1_context *none = api_test_context(SECP256K1_CONTEXT_NONE, &ecount);

CHECK(secp256k1_xonly_pubkey_parse(none, &pk1, pk1_ser) == 1);
CHECK(secp256k1_xonly_pubkey_parse(none, &pk2, pk2_ser) == 1);

CHECK(secp256k1_xonly_pubkey_cmp(none, NULL, &pk2) < 0);
CHECK(ecount == 1);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk1, NULL) > 0);
CHECK(ecount == 2);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk1, &pk2) < 0);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk2, &pk1) > 0);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk1, &pk1) == 0);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk2, &pk2) == 0);
CHECK(ecount == 2);
memset(&pk1, 0, sizeof(pk1)); /* illegal pubkey */
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk1, &pk2) < 0);
CHECK(ecount == 3);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk1, &pk1) == 0);
CHECK(ecount == 5);
CHECK(secp256k1_xonly_pubkey_cmp(none, &pk2, &pk1) > 0);
CHECK(ecount == 6);

secp256k1_context_destroy(none);
}

void test_xonly_pubkey_tweak(void) {
unsigned char zeros64[64] = { 0 };
unsigned char overflows[32];
Expand Down Expand Up @@ -540,6 +577,7 @@ void run_extrakeys_tests(void) {
test_xonly_pubkey_tweak();
test_xonly_pubkey_tweak_check();
test_xonly_pubkey_tweak_recursive();
test_xonly_pubkey_comparison();

/* keypair tests */
test_keypair();
Expand Down
26 changes: 26 additions & 0 deletions src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,32 @@ int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *o
return ret;
}

int secp256k1_ec_pubkey_cmp(const secp256k1_context* ctx, const secp256k1_pubkey* pubkey0, const secp256k1_pubkey* pubkey1) {
unsigned char out[2][33];
const secp256k1_pubkey* pk[2];
int i;

VERIFY_CHECK(ctx != NULL);
pk[0] = pubkey0; pk[1] = pubkey1;
for (i = 0; i < 2; i++) {
size_t out_size = sizeof(out[i]);
/* If the public key is NULL or invalid, ec_pubkey_serialize will call
* the illegal_callback and return 0. In that case we will serialize the
* key as all zeros which is less than any valid public key. This
* results in consistent comparisons even if NULL or invalid pubkeys are
* involved and prevents edge cases such as sorting algorithms that use
* this function and do not terminate as a result. */
if (!secp256k1_ec_pubkey_serialize(ctx, out[i], &out_size, pk[i], SECP256K1_EC_COMPRESSED)) {
/* Note that ec_pubkey_serialize should already set the output to
* zero in that case, but it's not guaranteed by the API, we can't
* test it and writing a VERIFY_CHECK is more complex than
* explicitly memsetting (again). */
memset(out[i], 0, sizeof(out[i]));
}
}
return secp256k1_memcmp_var(out[0], out[1], sizeof(out[0]));
}

static void secp256k1_ecdsa_signature_load(const secp256k1_context* ctx, secp256k1_scalar* r, secp256k1_scalar* s, const secp256k1_ecdsa_signature* sig) {
(void)ctx;
if (sizeof(secp256k1_scalar) == 32) {
Expand Down
50 changes: 50 additions & 0 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -5448,6 +5448,55 @@ void test_random_pubkeys(void) {
}
}

void run_pubkey_comparison(void) {
unsigned char pk1_ser[33] = {
0x02,
0x58, 0x84, 0xb3, 0xa2, 0x4b, 0x97, 0x37, 0x88, 0x92, 0x38, 0xa6, 0x26, 0x62, 0x52, 0x35, 0x11,
0xd0, 0x9a, 0xa1, 0x1b, 0x80, 0x0b, 0x5e, 0x93, 0x80, 0x26, 0x11, 0xef, 0x67, 0x4b, 0xd9, 0x23
};
const unsigned char pk2_ser[33] = {
0x02,
0xde, 0x36, 0x0e, 0x87, 0x59, 0x8f, 0x3c, 0x01, 0x36, 0x2a, 0x2a, 0xb8, 0xc6, 0xf4, 0x5e, 0x4d,
0xb2, 0xc2, 0xd5, 0x03, 0xa7, 0xf9, 0xf1, 0x4f, 0xa8, 0xfa, 0x95, 0xa8, 0xe9, 0x69, 0x76, 0x1c
};
secp256k1_pubkey pk1;
secp256k1_pubkey pk2;
int32_t ecount = 0;

CHECK(secp256k1_ec_pubkey_parse(ctx, &pk1, pk1_ser, sizeof(pk1_ser)) == 1);
CHECK(secp256k1_ec_pubkey_parse(ctx, &pk2, pk2_ser, sizeof(pk2_ser)) == 1);

secp256k1_context_set_illegal_callback(ctx, counting_illegal_callback_fn, &ecount);
CHECK(secp256k1_ec_pubkey_cmp(ctx, NULL, &pk2) < 0);
CHECK(ecount == 1);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk1, NULL) > 0);
CHECK(ecount == 2);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk1, &pk2) < 0);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk2, &pk1) > 0);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk1, &pk1) == 0);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk2, &pk2) == 0);
CHECK(ecount == 2);
{
secp256k1_pubkey pk_tmp;
memset(&pk_tmp, 0, sizeof(pk_tmp)); /* illegal pubkey */
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk_tmp, &pk2) < 0);
CHECK(ecount == 3);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk_tmp, &pk_tmp) == 0);
CHECK(ecount == 5);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk2, &pk_tmp) > 0);
CHECK(ecount == 6);
}

secp256k1_context_set_illegal_callback(ctx, NULL, NULL);

/* Make pk2 the same as pk1 but with 3 rather than 2. Note that in
* an uncompressed encoding, these would have the opposite ordering */
pk1_ser[0] = 3;
CHECK(secp256k1_ec_pubkey_parse(ctx, &pk2, pk1_ser, sizeof(pk1_ser)) == 1);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk1, &pk2) < 0);
CHECK(secp256k1_ec_pubkey_cmp(ctx, &pk2, &pk1) > 0);
}

void run_random_pubkeys(void) {
int i;
for (i = 0; i < 10*count; i++) {
Expand Down Expand Up @@ -6499,6 +6548,7 @@ int main(int argc, char **argv) {
#endif

/* ecdsa tests */
run_pubkey_comparison();
run_random_pubkeys();
run_ecdsa_der_parse();
run_ecdsa_sign_verify();
Expand Down

0 comments on commit 202a030

Please sign in to comment.