changes for code review

libdevcrypto/AES.cpp

@@ -37,10 +37,9 @@ struct aes::Aes128Ctr
 	CTR_Mode<AES>::Encryption mode;
 };
 
-Stream::Stream(StreamType _t, h128 _ckey):
+Stream::Stream(StreamType, h128 _ckey):
 	m_cSecret(_ckey)
 {
-	(void)_t; // encrypt and decrypt are same operation w/ctr
 	cryptor = new Aes128Ctr(_ckey);
 }
 

libdevcrypto/Common.h

@@ -52,7 +52,7 @@ using Address = h160;
 /// A vector of Ethereum addresses.
 using Addresses = h160s;
 
-/// A vector of Ethereum addresses.
+/// A set of Ethereum addresses.
 using AddressSet = std::set<h160>;
 
 /// A vector of secrets.

libdevcrypto/CryptoPP.cpp

@@ -80,6 +80,9 @@ Signature Secp256k1::sign(Secret const& _k, bytesConstRef _message)
 
 Signature Secp256k1::sign(Secret const& _key, h256 const& _hash)
 {
+	// assumption made by signing alogrithm
+	asserts(m_q == m_qs);
+	
 	Signature sig;
 	
 	Integer k(kdf(_key, _hash).data(), 32);
@@ -196,7 +199,7 @@ void Secp256k1::agree(Secret const& _s, Public const& _r, h256& o_s)
 	assert(d.Agree(o_s.data(), _s.data(), remote));
 }
 
-void Secp256k1::exportPublicKey(CryptoPP::DL_PublicKey_EC<CryptoPP::ECP> const& _k, Public& _p)
+void Secp256k1::exportPublicKey(CryptoPP::DL_PublicKey_EC<CryptoPP::ECP> const& _k, Public& o_p)
 {
 	bytes prefixedKey(_k.GetGroupParameters().GetEncodedElementSize(true));
 	
@@ -206,10 +209,10 @@ void Secp256k1::exportPublicKey(CryptoPP::DL_PublicKey_EC<CryptoPP::ECP> const&
 		assert(Public::size + 1 == _k.GetGroupParameters().GetEncodedElementSize(true));
 	}
 
-	memcpy(_p.data(), &prefixedKey[1], Public::size);
+	memcpy(o_p.data(), &prefixedKey[1], Public::size);
 }
 
-void Secp256k1::exponentToPublic(Integer const& _e, Public& _p)
+void Secp256k1::exponentToPublic(Integer const& _e, Public& o_p)
 {
 	CryptoPP::DL_PublicKey_EC<CryptoPP::ECP> pk;
 	
@@ -218,6 +221,6 @@ void Secp256k1::exponentToPublic(Integer const& _e, Public& _p)
 		pk.Initialize(m_params, m_params.ExponentiateBase(_e));
 	}
 	
-	exportPublicKey(pk, _p);
+	exportPublicKey(pk, o_p);
 }
 

libdevcrypto/CryptoPP.h

@@ -84,10 +84,10 @@ public:
 	/// @returns siganture of message.
 	Signature sign(Secret const& _k, bytesConstRef _message);
 	
-	/// @returns compact siganture of message hash.
+	/// @returns compact siganture of provided hash.
 	Signature sign(Secret const& _k, h256 const& _hash);
 	
-	/// Verify compact signature (public key is extracted from message).
+	/// Verify compact signature (public key is extracted from signature).
 	bool verify(Signature const& _signature, bytesConstRef _message);
 	
 	/// Verify signature.
@@ -96,17 +96,17 @@ public:
 	/// Recovers public key from compact signature. Uses libsecp256k1.
 	Public recover(Signature _signature, bytesConstRef _message);
 	
-	/// Verify secret key is valid.
+	/// Verifies _s is a valid secret key and returns corresponding public key in o_p.
 	bool verifySecret(Secret const& _s, Public& o_p);
 	
 	void agree(Secret const& _s, Public const& _r, h256& o_s);
 	
 protected:
-	void exportPrivateKey(DL_PrivateKey_EC<ECP> const& _k, Secret& _s) { _k.GetPrivateExponent().Encode(_s.data(), Secret::size); }
+	void exportPrivateKey(DL_PrivateKey_EC<ECP> const& _k, Secret& o_s) { _k.GetPrivateExponent().Encode(o_s.data(), Secret::size); }
 	
-	void exportPublicKey(DL_PublicKey_EC<ECP> const& _k, Public& _p);
+	void exportPublicKey(DL_PublicKey_EC<ECP> const& _k, Public& o_p);
 	
-	void exponentToPublic(Integer const& _e, Public& _p);
+	void exponentToPublic(Integer const& _e, Public& o_p);
 	
 	template <class T> void initializeDLScheme(Secret const& _s, T& io_operator) { std::lock_guard<std::mutex> l(x_params); io_operator.AccessKey().Initialize(m_params, secretToExponent(_s)); }
 	

libdevcrypto/ECDHE.cpp

@@ -58,11 +58,11 @@ void ECDHEKeyExchange::exchange(bytes& o_exchange)
 	// If a session previously exists:
 	//	prefix is sha3(token) // todo: ephemeral entropy from both sides
 	// If a session doesn't exist:
-	//	prefix is sha3mac(m_ephemeralSecret,
+	//	prefix is sha3(m_ephemeralSecret)
 	//
 	// The second part is encrypted using the public key which relates to the prefix.
 	
-	Public encpk = m_known.first|m_remoteEphemeral;
+	Public encpk = m_known.first | m_remoteEphemeral;
 	bytes exchange(encpk.asBytes());
 	
 	// This is the public key which we would like the remote to use,
@@ -71,24 +71,23 @@ void ECDHEKeyExchange::exchange(bytes& o_exchange)
 	// Here we should pick an appropriate alias or generate a new one,
 	// but for now, we use static alias passed to constructor.
 	//
-	Public p;
-	s_secp256k1.toPublic(m_alias.m_secret, p);
+	Public p = toPublic(m_alias.m_secret);
 	exchange.resize(exchange.size() + sizeof(p));
-	memcpy(exchange.data() - sizeof(p), p.data(), sizeof(p));
+	memcpy(&exchange[exchange.size() - sizeof(p)], p.data(), sizeof(p));
 	
 	// protocol parameters; should be fixed size
-	bytes v(asBytes("\x80"));
+	bytes v({0x80});
 	exchange.resize(exchange.size() + v.size());
-	memcpy(exchange.data() - v.size(), v.data(), v.size());
+	memcpy(&exchange[exchange.size() - v.size()], v.data(), v.size());
 	
 	h256 auth;
 	sha3mac(m_alias.m_secret.ref(), m_ephemeralSecret.ref(), auth.ref());
 	Signature sig = s_secp256k1.sign(m_alias.m_secret, auth);
 	exchange.resize(exchange.size() + sizeof(sig));
-	memcpy(exchange.data() - sizeof(sig), sig.data(), sizeof(sig));
+	memcpy(&exchange[exchange.size() - sizeof(sig)], sig.data(), sizeof(sig));
 	
 	aes::AuthenticatedStream aes(aes::Encrypt, m_ephemeralSecret, 0);
-	h256 prefix(sha3((h256)(m_known.second|m_remoteEphemeral)));
+	h256 prefix(sha3((h256)m_remoteEphemeral | m_known.second));
 	aes.update(prefix.ref());
 	
 	s_secp256k1.encrypt(encpk, exchange);

libdevcrypto/ECDHE.h

@@ -30,6 +30,7 @@ namespace dev
 namespace crypto
 {
 	
+/// Public key of remote and corresponding shared secret.
 typedef std::pair<Public,h256> AliasSession;
 	
 /**
@@ -41,7 +42,7 @@ class Alias
 public:
 	Alias(Secret _s): m_secret(_s) {};
 	
-	AliasSession session(Address _a) { return m_sessions.count(_a) ? AliasSession() : m_sessions.find(_a)->second; }
+	AliasSession session(Address _a) { return m_sessions.count(_a) ? m_sessions.find(_a)->second : AliasSession(); }
 	
 private:
 	std::map<Address,AliasSession> m_sessions;

test/crypto.cpp

@@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(cryptopp_ecdsa_sipaseckp256k1)
 	Secret secret(sha3(sbytes));
 	KeyPair key(secret);
 	
-	bytes m(fromHex("0xFF"));
+	bytes m({0xFF});
 	int tests = 2;
 	while (m[0]++, tests--)
 	{