From 1c746c7233379026562d04dcd385821e8b4001fc Mon Sep 17 00:00:00 2001
From: subtly <subtly@users.noreply.github.com>
Date: Tue, 4 Nov 2014 10:54:46 +0100
Subject: [PATCH] codereview fixes

---
 libdevcrypto/Common.cpp | 34 ++++++++++++++++------------------
 libdevcrypto/CryptoPP.h |  2 +-
 libdevcrypto/EC.cpp     | 14 ++++++++------
 test/crypto.cpp         |  4 ++--
 4 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/libdevcrypto/Common.cpp b/libdevcrypto/Common.cpp
index b06a16e57..bd1c80268 100644
--- a/libdevcrypto/Common.cpp
+++ b/libdevcrypto/Common.cpp
@@ -114,28 +114,26 @@ h256 Nonce::get(bool _commit)
 	static string seedFile(getDataDir() + "/seed");
 	static mutex x;
 	lock_guard<mutex> l(x);
+	if (!seed)
 	{
-		if (!seed)
+		static Nonce nonce;
+		bytes b = contents(seedFile);
+		if (b.size() == 32)
+			memcpy(seed.data(), b.data(), 32);
+		else
 		{
-			static Nonce nonce;
-			bytes b = contents(seedFile);
-			if (b.size() == 32)
-				memcpy(seed.data(), b.data(), 32);
-			else
-			{
-				std::mt19937_64 s_eng(time(0));
-				std::uniform_int_distribution<uint16_t> d(0, 255);
-				for (unsigned i = 0; i < 32; ++i)
-					seed[i] = (byte)d(s_eng);
-			}
-			writeFile(seedFile, bytes());
+			std::mt19937_64 s_eng(time(0));
+			std::uniform_int_distribution<uint16_t> d(0, 255);
+			for (unsigned i = 0; i < 32; ++i)
+				seed[i] = (byte)d(s_eng);
 		}
-		assert(seed);
-		h256 prev(seed);
-		sha3(prev.ref(), seed.ref());
-		if (_commit)
-			writeFile(seedFile, seed.asBytes());
+		writeFile(seedFile, bytes());
 	}
+	assert(seed);
+	h256 prev(seed);
+	sha3(prev.ref(), seed.ref());
+	if (_commit)
+		writeFile(seedFile, seed.asBytes());
 	return seed;
 }
 
diff --git a/libdevcrypto/CryptoPP.h b/libdevcrypto/CryptoPP.h
index 756bbb72a..dc5d6a610 100644
--- a/libdevcrypto/CryptoPP.h
+++ b/libdevcrypto/CryptoPP.h
@@ -67,7 +67,7 @@ static const CryptoPP::OID secp256k1Curve = CryptoPP::ASN1::secp256k1();
 	
 static const CryptoPP::DL_GroupParameters_EC<CryptoPP::ECP> secp256k1Params(secp256k1Curve);
 	
-static ECP::Point publicToPoint(Public const& _p) { Integer x(_p.data(), 32); Integer y(_p.data()+32, 32); return std::move(ECP::Point(x,y)); }
+static ECP::Point publicToPoint(Public const& _p) { Integer x(_p.data(), 32); Integer y(_p.data() + 32, 32); return std::move(ECP::Point(x,y)); }
 	
 static Integer secretToExponent(Secret const& _s) { return std::move(Integer(_s.data(), Secret::size)); }
 
diff --git a/libdevcrypto/EC.cpp b/libdevcrypto/EC.cpp
index 16f3c6830..890b7e2f4 100644
--- a/libdevcrypto/EC.cpp
+++ b/libdevcrypto/EC.cpp
@@ -39,7 +39,7 @@ using namespace pp;
 
 void crypto::toPublic(Secret const& _s, Public& o_public)
 {
-	exponentToPublic(Integer(_s.data(),sizeof(_s)), o_public);
+	exponentToPublic(Integer(_s.data(), sizeof(_s)), o_public);
 }
 
 h256 crypto::kdf(Secret const& _priv, h256 const& _hash)
@@ -92,10 +92,12 @@ Signature crypto::sign(Secret const& _key, h256 const& _hash)
 	initializeDLScheme(_key, signer);
 
 	Integer const& q = secp256k1Params.GetGroupOrder();
+	Integer const& qs = secp256k1Params.GetSubgroupOrder();
 	Integer e(_hash.asBytes().data(), 32);
 
 	Integer k(kdf(_key, _hash).data(), 32);
-	k %= secp256k1Params.GetSubgroupOrder()-1;
+	assert(k);
+	k = 1 + (k % (qs - 1));
 	
 	ECP::Point rp = secp256k1Params.ExponentiateBase(k);
 	Integer r = secp256k1Params.ConvertElementToInteger(rp);
@@ -105,7 +107,7 @@ Signature crypto::sign(Secret const& _key, h256 const& _hash)
 	Integer s = (kInv * (Integer(_key.asBytes().data(), 32)*r + e)) % q;
 	assert(!!r && !!s);
 	
-	if (s > secp256k1Params.GetSubgroupOrder())
+	if (s > qs)
 	{
 		s = q - s;
 		if (recid)
@@ -114,7 +116,7 @@ Signature crypto::sign(Secret const& _key, h256 const& _hash)
 	
 	Signature sig;
 	r.Encode(sig.data(), 32);
-	s.Encode(sig.data()+32, 32);
+	s.Encode(sig.data() + 32, 32);
 	sig[64] = recid;
 	return sig;
 }
@@ -147,8 +149,8 @@ Public crypto::recover(Signature _signature, bytesConstRef _message)
 {
 	secp256k1_start();
 	
-	byte pubkey[65];
 	int pubkeylen = 65;
+	byte pubkey[pubkeylen];
 	if (!secp256k1_ecdsa_recover_compact(_message.data(), 32, _signature.data(), pubkey, &pubkeylen, 0, (int)_signature[64]))
 		return Public();
 	
@@ -172,8 +174,8 @@ bool crypto::verifySecret(Secret const& _s, Public const& _p)
 	if (!ok)
 		return false;
 	
-	byte pubkey[65];
 	int pubkeylen = 65;
+	byte pubkey[pubkeylen];
 	ok = secp256k1_ecdsa_pubkey_create(pubkey, &pubkeylen, _s.data(), 0);
 	if (!ok || pubkeylen != 65)
 		return false;
diff --git a/test/crypto.cpp b/test/crypto.cpp
index ab384a038..82950f09d 100644
--- a/test/crypto.cpp
+++ b/test/crypto.cpp
@@ -138,7 +138,7 @@ BOOST_AUTO_TEST_CASE(cryptopp_cryptopp_ecdsav)
 
 		Signature sig;
 		r.Encode(sig.data(), 32);
-		s.Encode(sig.data()+32, 32);
+		s.Encode(sig.data() + 32, 32);
 		sig[64] = recid;
 
 		Public p = dev::recover(sig, he);
@@ -188,7 +188,7 @@ BOOST_AUTO_TEST_CASE(cryptopp_ecdsa_sipaseckp256k1)
 		pp::initializeDLScheme(key.pub(), verifier);
 		Signature sigppraw;
 		r.Encode(sigppraw.data(), 32);
-		s.Encode(sigppraw.data()+32, 32);
+		s.Encode(sigppraw.data() + 32, 32);
 		BOOST_REQUIRE(verifier.VerifyMessage(m.data(), m.size(), sigppraw.data(), 64));
 		BOOST_REQUIRE(crypto::verify(key.pub(), sigppraw, bytesConstRef(&m)));
 		BOOST_REQUIRE(dev::verify(key.pub(), sigppraw, hm));