phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-25 10:43:19 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Eliminate multiple-returns from secp256k1.c.

Browse source 
Goto, multiple returns, continue, and/or multiple breaks in a
 loop are often used to build complex or non-local control
 flow in software.

(They're all basically the same thing, and anyone axiomatically
 opposing goto and not the rest is probably cargo-culting from
 the title of Dijkstra's essay without thinking hard about it.)

Personally, I think the current use of these constructs in the
 code base is fine: no where are we using them to create control-
 flow that couldn't easily be described in plain English, which
 is hard to read or reason about, or which looks like a trap for
 future developers.

Some, however, prefer a more rules based approach to software
 quality.  In particular, MISRA forbids all of these constructs,
 and for good experience based reasons.  Rules also have the
 benefit of being machine checkable and surviving individual
 developers.

(To be fair-- MISRA also has a process for accommodating code that
 breaks the rules for good reason).

I think that in general we should also try to satisfy the rules-
 based measures of software quality, except where there is an
 objective reason not do: a measurable performance difference,
 logic that turns to spaghetti, etc.

Changing out all the multiple returns in secp256k1.c appears to
 be basically neutral:  Some parts become slightly less clear,
 some parts slightly more.
This commit is contained in:
Gregory Maxwell 2015-02-17 01:01:48 -08:00
parent 354ffa33e6
commit 0065a8fb9c
1 changed files with 110 additions and 104 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

214
src/secp256k1.c
View file

@ -37,6 +37,7 @@ int secp256k1_ecdsa_verify(const unsigned char *msg32, const unsigned char *sig,
secp256k1_ge_t q;
secp256k1_ecdsa_sig_t s;
secp256k1_scalar_t m;
int ret = -3;
DEBUG_CHECK(secp256k1_ecmult_consts != NULL);
DEBUG_CHECK(msg32 != NULL);
DEBUG_CHECK(sig != NULL);
@ -44,17 +45,22 @@ int secp256k1_ecdsa_verify(const unsigned char *msg32, const unsigned char *sig,
secp256k1_scalar_set_b32(&m, msg32, NULL);
if (!secp256k1_eckey_pubkey_parse(&q, pubkey, pubkeylen)) {
return -1;
if (secp256k1_eckey_pubkey_parse(&q, pubkey, pubkeylen)) {
if (secp256k1_ecdsa_sig_parse(&s, sig, siglen)) {
if (secp256k1_ecdsa_sig_verify(&s, &q, &m)) {
/* success is 1, all other values are fail */
ret = 1;
} else {
ret = 0;
}
} else {
ret = -2;
}
} else {
ret = -1;
}
if (!secp256k1_ecdsa_sig_parse(&s, sig, siglen)) {
return -2;
}
if (!secp256k1_ecdsa_sig_verify(&s, &q, &m)) {
return 0;
}
/* success is 1, all other values are fail */
return 1;
return ret;
}
static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, unsigned int counter, const void *data) {
@ -88,35 +94,34 @@ int secp256k1_ecdsa_sign(const unsigned char *msg32, unsigned char *signature, i
}
secp256k1_scalar_set_b32(&sec, seckey, &overflow);
if (overflow || secp256k1_scalar_is_zero(&sec)) {
*signaturelen = 0;
return 0;
}
secp256k1_scalar_set_b32(&msg, msg32, NULL);
while (1) {
unsigned char nonce32[32];
ret = noncefp(nonce32, msg32, seckey, count, noncedata);
if (!ret) {
break;
}
secp256k1_scalar_set_b32(&non, nonce32, &overflow);
memset(nonce32, 0, 32);
if (!secp256k1_scalar_is_zero(&non) && !overflow) {
if (secp256k1_ecdsa_sig_sign(&sig, &sec, &msg, &non, NULL)) {
/* Fail if the secret key is invalid. */
if (!overflow && !secp256k1_scalar_is_zero(&sec)) {
secp256k1_scalar_set_b32(&msg, msg32, NULL);
while (1) {
unsigned char nonce32[32];
ret = noncefp(nonce32, msg32, seckey, count, noncedata);
if (!ret) {
break;
}
secp256k1_scalar_set_b32(&non, nonce32, &overflow);
memset(nonce32, 0, 32);
if (!secp256k1_scalar_is_zero(&non) && !overflow) {
if (secp256k1_ecdsa_sig_sign(&sig, &sec, &msg, &non, NULL)) {
break;
}
}
count++;
}
count++;
}
if (ret) {
ret = secp256k1_ecdsa_sig_serialize(signature, signaturelen, &sig);
if (ret) {
ret = secp256k1_ecdsa_sig_serialize(signature, signaturelen, &sig);
}
secp256k1_scalar_clear(&msg);
secp256k1_scalar_clear(&non);
secp256k1_scalar_clear(&sec);
}
if (!ret) {
*signaturelen = 0;
}
secp256k1_scalar_clear(&msg);
secp256k1_scalar_clear(&non);
secp256k1_scalar_clear(&sec);
return ret;
}
@ -135,36 +140,35 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *msg32, unsigned char *sig6
}
secp256k1_scalar_set_b32(&sec, seckey, &overflow);
if (overflow || secp256k1_scalar_is_zero(&sec)) {
memset(sig64, 0, 64);
return 0;
}
secp256k1_scalar_set_b32(&msg, msg32, NULL);
while (1) {
unsigned char nonce32[32];
ret = noncefp(nonce32, msg32, seckey, count, noncedata);
if (!ret) {
break;
}
secp256k1_scalar_set_b32(&non, nonce32, &overflow);
memset(nonce32, 0, 32);
if (!secp256k1_scalar_is_zero(&non) && !overflow) {
if (secp256k1_ecdsa_sig_sign(&sig, &sec, &msg, &non, recid)) {
/* Fail if the secret key is invalid. */
if (!overflow && !secp256k1_scalar_is_zero(&sec)) {
secp256k1_scalar_set_b32(&msg, msg32, NULL);
while (1) {
unsigned char nonce32[32];
ret = noncefp(nonce32, msg32, seckey, count, noncedata);
if (!ret) {
break;
}
secp256k1_scalar_set_b32(&non, nonce32, &overflow);
memset(nonce32, 0, 32);
if (!secp256k1_scalar_is_zero(&non) && !overflow) {
if (secp256k1_ecdsa_sig_sign(&sig, &sec, &msg, &non, recid)) {
break;
}
}
count++;
}
count++;
}
if (ret) {
secp256k1_scalar_get_b32(sig64, &sig.r);
secp256k1_scalar_get_b32(sig64 + 32, &sig.s);
if (ret) {
secp256k1_scalar_get_b32(sig64, &sig.r);
secp256k1_scalar_get_b32(sig64 + 32, &sig.s);
}
secp256k1_scalar_clear(&msg);
secp256k1_scalar_clear(&non);
secp256k1_scalar_clear(&sec);
}
if (!ret) {
memset(sig64, 0, 64);
}
secp256k1_scalar_clear(&msg);
secp256k1_scalar_clear(&non);
secp256k1_scalar_clear(&sec);
return ret;
}
@ -182,17 +186,15 @@ int secp256k1_ecdsa_recover_compact(const unsigned char *msg32, const unsigned c
DEBUG_CHECK(recid >= 0 && recid <= 3);
secp256k1_scalar_set_b32(&sig.r, sig64, &overflow);
if (overflow) {
return 0;
}
secp256k1_scalar_set_b32(&sig.s, sig64 + 32, &overflow);
if (overflow) {
return 0;
}
secp256k1_scalar_set_b32(&m, msg32, NULL);
if (!overflow) {
secp256k1_scalar_set_b32(&sig.s, sig64 + 32, &overflow);
if (!overflow) {
secp256k1_scalar_set_b32(&m, msg32, NULL);
if (secp256k1_ecdsa_sig_recover(&sig, &q, &m, recid)) {
ret = secp256k1_eckey_pubkey_serialize(&q, pubkey, pubkeylen, compressed);
if (secp256k1_ecdsa_sig_recover(&sig, &q, &m, recid)) {
ret = secp256k1_eckey_pubkey_serialize(&q, pubkey, pubkeylen, compressed);
}
}
}
return ret;
}
@ -221,36 +223,41 @@ int secp256k1_ec_pubkey_create(unsigned char *pubkey, int *pubkeylen, const unsi
secp256k1_ge_t p;
secp256k1_scalar_t sec;
int overflow;
int ret = 0;
DEBUG_CHECK(secp256k1_ecmult_gen_consts != NULL);
DEBUG_CHECK(pubkey != NULL);
DEBUG_CHECK(pubkeylen != NULL);
DEBUG_CHECK(seckey != NULL);
secp256k1_scalar_set_b32(&sec, seckey, &overflow);
if (overflow) {
*pubkeylen = 0;
return 0;
if (!overflow) {
secp256k1_ecmult_gen(&pj, &sec);
secp256k1_scalar_clear(&sec);
secp256k1_ge_set_gej(&p, &pj);
ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, compressed);
}
secp256k1_ecmult_gen(&pj, &sec);
secp256k1_scalar_clear(&sec);
secp256k1_ge_set_gej(&p, &pj);
return secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, compressed);
if (!ret) {
*pubkeylen = 0;
}
return ret;
}
int secp256k1_ec_pubkey_decompress(unsigned char *pubkey, int *pubkeylen) {
secp256k1_ge_t p;
int ret = 0;
DEBUG_CHECK(pubkey != NULL);
DEBUG_CHECK(pubkeylen != NULL);
if (!secp256k1_eckey_pubkey_parse(&p, pubkey, *pubkeylen))
return 0;
return secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, 0);
if (secp256k1_eckey_pubkey_parse(&p, pubkey, *pubkeylen)) {
ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, pubkeylen, 0);
}
return ret;
}
int secp256k1_ec_privkey_tweak_add(unsigned char *seckey, const unsigned char *tweak) {
secp256k1_scalar_t term;
secp256k1_scalar_t sec;
int ret;
int ret = 0;
int overflow = 0;
DEBUG_CHECK(seckey != NULL);
DEBUG_CHECK(tweak != NULL);
@ -271,24 +278,23 @@ int secp256k1_ec_privkey_tweak_add(unsigned char *seckey, const unsigned char *t
int secp256k1_ec_pubkey_tweak_add(unsigned char *pubkey, int pubkeylen, const unsigned char *tweak) {
secp256k1_ge_t p;
secp256k1_scalar_t term;
int ret;
int ret = 0;
int overflow = 0;
DEBUG_CHECK(secp256k1_ecmult_consts != NULL);
DEBUG_CHECK(pubkey != NULL);
DEBUG_CHECK(tweak != NULL);
secp256k1_scalar_set_b32(&term, tweak, &overflow);
if (overflow) {
return 0;
}
ret = secp256k1_eckey_pubkey_parse(&p, pubkey, pubkeylen);
if (ret) {
ret = secp256k1_eckey_pubkey_tweak_add(&p, &term);
}
if (ret) {
int oldlen = pubkeylen;
ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
VERIFY_CHECK(pubkeylen == oldlen);
if (!overflow) {
ret = secp256k1_eckey_pubkey_parse(&p, pubkey, pubkeylen);
if (ret) {
ret = secp256k1_eckey_pubkey_tweak_add(&p, &term);
}
if (ret) {
int oldlen = pubkeylen;
ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
VERIFY_CHECK(pubkeylen == oldlen);
}
}
return ret;
@ -297,7 +303,7 @@ int secp256k1_ec_pubkey_tweak_add(unsigned char *pubkey, int pubkeylen, const un
int secp256k1_ec_privkey_tweak_mul(unsigned char *seckey, const unsigned char *tweak) {
secp256k1_scalar_t factor;
secp256k1_scalar_t sec;
int ret;
int ret = 0;
int overflow = 0;
DEBUG_CHECK(seckey != NULL);
DEBUG_CHECK(tweak != NULL);
@ -317,24 +323,23 @@ int secp256k1_ec_privkey_tweak_mul(unsigned char *seckey, const unsigned char *t
int secp256k1_ec_pubkey_tweak_mul(unsigned char *pubkey, int pubkeylen, const unsigned char *tweak) {
secp256k1_ge_t p;
secp256k1_scalar_t factor;
int ret;
int ret = 0;
int overflow = 0;
DEBUG_CHECK(secp256k1_ecmult_consts != NULL);
DEBUG_CHECK(pubkey != NULL);
DEBUG_CHECK(tweak != NULL);
secp256k1_scalar_set_b32(&factor, tweak, &overflow);
if (overflow) {
return 0;
}
ret = secp256k1_eckey_pubkey_parse(&p, pubkey, pubkeylen);
if (ret) {
ret = secp256k1_eckey_pubkey_tweak_mul(&p, &factor);
}
if (ret) {
int oldlen = pubkeylen;
ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
VERIFY_CHECK(pubkeylen == oldlen);
if (!overflow) {
ret = secp256k1_eckey_pubkey_parse(&p, pubkey, pubkeylen);
if (ret) {
ret = secp256k1_eckey_pubkey_tweak_mul(&p, &factor);
}
if (ret) {
int oldlen = pubkeylen;
ret = secp256k1_eckey_pubkey_serialize(&p, pubkey, &pubkeylen, oldlen <= 33);
VERIFY_CHECK(pubkeylen == oldlen);
}
}
return ret;
@ -342,7 +347,7 @@ int secp256k1_ec_pubkey_tweak_mul(unsigned char *pubkey, int pubkeylen, const un
int secp256k1_ec_privkey_export(const unsigned char *seckey, unsigned char *privkey, int *privkeylen, int compressed) {
secp256k1_scalar_t key;
int ret;
int ret = 0;
DEBUG_CHECK(seckey != NULL);
DEBUG_CHECK(privkey != NULL);
DEBUG_CHECK(privkeylen != NULL);
@ -355,13 +360,14 @@ int secp256k1_ec_privkey_export(const unsigned char *seckey, unsigned char *priv
int secp256k1_ec_privkey_import(unsigned char *seckey, const unsigned char *privkey, int privkeylen) {
secp256k1_scalar_t key;
int ret;
int ret = 0;
DEBUG_CHECK(seckey != NULL);
DEBUG_CHECK(privkey != NULL);
ret = secp256k1_eckey_privkey_parse(&key, privkey, privkeylen);
if (ret)
if (ret) {
secp256k1_scalar_get_b32(seckey, &key);
}
secp256k1_scalar_clear(&key);
return ret;
}