Add secp256k1_ec_pubkey_compress() #263
pull afk11 wants to merge 1 commits into bitcoin-core:master from afk11:add-pubkey-compress changing 3 files +45 −5-
afk11 commented at 1:34 pm on June 26, 2015: contributorAdded 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: contributorI’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: contributorPlease 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, 2015afk11 force-pushed on Jul 6, 2015afk11 commented at 3:29 pm on July 6, 2015: contributorOk, 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 secondIn:
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 shouldVERIFY_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: contributorACK, though addressing @apoelstra’s nits would be nice.afk11 force-pushed on Jul 8, 2015afk11 commented at 10:04 pm on July 8, 2015: contributorCool, rebased and addressed the above!apoelstra commented at 10:41 pm on July 8, 2015: contributorTested ACK from me. (TheVERIFY_CHECK
thing is no big deal.)Add secp256k1_ec_pubkey_compress(), with test similar to the related decompress() function. 99fd963bd5in 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, 2015sipa merged this on Jul 14, 2015sipa closed this on Jul 14, 2015
sipa referenced this in commit 4fb174df08 on Jul 14, 2015
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me