Add secp256k1_ec_pubkey

afk11 commented at 1:34 pm on June 26, 2015: contributor

Added general function to reserialize a public key, and added secp256k1_ec_pubkey_compress() after seeing https://github.com/bitcoin/secp256k1/issues/259

afk11 commented at 8:58 pm on June 26, 2015: contributor

I’m not yet sure why travis failed, it was a seemingly unrelated check. Will take another look.

afk11 commented at 6:55 pm on July 5, 2015: contributor

Ok - nothing major here, just added a function that does the opposite of secp256k1_ec_pubkey_decompress() Ended up removing the new function in eckey_impl.h - it wasn’t really worth it as the code is just a few lines.

Anyways, hope this proves useful enough to merge. I wouldn’t push C code too often - any feedback/criticism would be welcome.

I added a test also, similar to how decompress is tested here: https://github.com/bitcoin/secp256k1/pull/263/files#diff-4655d106bf03045a3a50beefc800db21R1521

in src/eckey.h: in 8633317d00 outdated

13@@ -14,6 +14,7 @@
14 
15 static int secp256k1_eckey_pubkey_parse(secp256k1_ge_t *elem, const unsigned char *pub, int size);
16 static int secp256k1_eckey_pubkey_serialize(secp256k1_ge_t *elem, unsigned char *pub, int *size, int compressed);
17+static int secp256k1_eckey_pubkey_reserialize( const unsigned char *pub, unsigned char *pubkeyout, int *size, int compressed);

sipa commented at 12:21 pm on July 6, 2015:

This function does not seem to exist.

afk11 commented at 1:37 pm on July 6, 2015:

Thanks, removed it now.

sipa commented at 12:37 pm on July 6, 2015: contributor

Please squash your last commit into the previous ones. All commits should result in a tree that passes tests, or we may break the ability to bisect.

afk11 force-pushed on Jul 6, 2015

afk11 commented at 3:29 pm on July 6, 2015: contributor

Ok, just pushed them once more.

in src/eckey.h: in fead8c19c8 outdated

23@@ -24,3 +24,4 @@ static int secp256k1_eckey_privkey_tweak_mul(secp256k1_scalar_t *key, const secp
24 static int secp256k1_eckey_pubkey_tweak_mul(const secp256k1_ecmult_context_t *ctx, secp256k1_ge_t *key, const secp256k1_scalar_t *tweak);
25 
26 #endif
27+

apoelstra commented at 6:30 pm on July 7, 2015:

Can you rebase without this newline?

Normally I wouldn’t complain but this is the only place you touch this file.

afk11 commented at 10:04 pm on July 8, 2015:

Woops - hadn’t spotted that, thanks.

in src/secp256k1.c: in fead8c19c8 outdated

432@@ -418,3 +433,4 @@ int secp256k1_context_randomize(secp256k1_context_t* ctx, const unsigned char *s
433     secp256k1_ecmult_gen_blind(&ctx->ecmult_gen_ctx, seed32);
434     return 1;
435 }
436+

apoelstra commented at 6:30 pm on July 7, 2015:

And this one, while you’re rebasing anyway.

in include/secp256k1.h: in fead8c19c8 outdated

259@@ -260,6 +260,23 @@ SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create(
260   int compressed
261 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
262 
263+/** Compress a public key.
264+ * In:     ctx:       pointer to a context object (cannot be NULL)
265+ * In:     pubkeyin:  pointer to a 33-byte or 65-byte public key (cannot be NULL)

apoelstra commented at 6:32 pm on July 7, 2015:

Typo: drop second In:

in src/secp256k1.c: in fead8c19c8 outdated

292+    DEBUG_CHECK(pubkeyout != NULL);
293+    DEBUG_CHECK(pubkeylen != NULL);
294+    (void)ctx;
295+
296+    if (secp256k1_eckey_pubkey_parse(&p, pubkeyin, *pubkeylen)) {
297+        ret = secp256k1_eckey_pubkey_serialize(&p, pubkeyout, pubkeylen, 1);

apoelstra commented at 6:33 pm on July 7, 2015:

I wonder if we should VERIFY_CHECK this instead of returning the return value. Is there a way this can fail if the parsing was successful?

sipa commented at 9:09 pm on July 8, 2015:

Nope.

sipa commented at 9:10 pm on July 8, 2015: contributor

ACK, though addressing @apoelstra’s nits would be nice.

afk11 force-pushed on Jul 8, 2015

afk11 commented at 10:04 pm on July 8, 2015: contributor

Cool, rebased and addressed the above!

apoelstra commented at 10:41 pm on July 8, 2015: contributor

Tested ACK from me. (The VERIFY_CHECK thing is no big deal.)

Add secp256k1_ec_pubkey_compress(), with test similar to the related decompress() function. 99fd963bd5

in include/secp256k1.h: in 8671d19243 outdated

259@@ -260,9 +260,26 @@ SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create(
260   int compressed
261 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
262 
263+/** Compress a public key.
264+ * In:     ctx:       pointer to a context object (cannot be NULL)
265+ *         pubkeyin:  pointer to a 33-byte or 65-byte public key (cannot be NULL)
266+ * In/Out: pubkeyout: pointer to a 33-byte array to put the compressed public key (cannot be NULL)

sipa commented at 4:17 pm on July 10, 2015:

pubkeyout is Out only, no?

afk11 commented at 4:55 pm on July 10, 2015:

You’re right. Could still be the same for decompress as well, I’ll take a look now.

afk11 force-pushed on Jul 13, 2015

sipa merged this on Jul 14, 2015

sipa closed this on Jul 14, 2015

sipa referenced this in commit 4fb174df08 on Jul 14, 2015

Add secp256k1_ec_pubkey_compress() #263