Get rid of unsafe code in ethkey, propagate incorrect Secret errors. (#4119)

* Implementing secret

* Fixing tests

ethcore/src/blockchain/blockchain.rs

@@ -1329,6 +1329,7 @@ mod tests {
 	use transaction::{Transaction, Action};
 	use log_entry::{LogEntry, LocalizedLogEntry};
 	use spec::Spec;
+	use ethkey::Secret;
 
 	fn new_db(path: &str) -> Arc<Database> {
 		Arc::new(Database::open(&DatabaseConfig::with_columns(::db::NUM_COLUMNS), path).unwrap())
@@ -1467,6 +1468,10 @@ mod tests {
 		// TODO: insert block that already includes one of them as an uncle to check it's not allowed.
 	}
 
+	fn secret() -> Secret {
+		Secret::from_slice(&"".sha3()).unwrap()
+	}
+
 	#[test]
 	fn test_fork_transaction_addresses() {
 		let mut canon_chain = ChainGenerator::default();
@@ -1482,7 +1487,7 @@ mod tests {
 			action: Action::Create,
 			value: 100.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 
 		let b1a = canon_chain
@@ -1546,7 +1551,7 @@ mod tests {
 			action: Action::Create,
 			value: 100.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		let t2 = Transaction {
 			nonce: 1.into(),
@@ -1555,7 +1560,7 @@ mod tests {
 			action: Action::Create,
 			value: 100.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		let t3 = Transaction {
 			nonce: 2.into(),
@@ -1564,7 +1569,7 @@ mod tests {
 			action: Action::Create,
 			value: 100.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		let b1a = canon_chain
 			.with_transaction(t1.clone())
@@ -1870,7 +1875,7 @@ mod tests {
 			action: Action::Create,
 			value: 101.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 		let t2 = Transaction {
 			nonce: 0.into(),
 			gas_price: 0.into(),
@@ -1878,7 +1883,7 @@ mod tests {
 			action: Action::Create,
 			value: 102.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 		let t3 = Transaction {
 			nonce: 0.into(),
 			gas_price: 0.into(),
@@ -1886,7 +1891,7 @@ mod tests {
 			action: Action::Create,
 			value: 103.into(),
 			data: "601080600c6000396000f3006000355415600957005b60203560003555".from_hex().unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 		let tx_hash1 = t1.hash();
 		let tx_hash2 = t2.hash();
 		let tx_hash3 = t3.hash();

ethcore/src/client/client.rs

@@ -1669,7 +1669,7 @@ mod tests {
 		use util::Hashable;
 
 		// given
-		let key = KeyPair::from_secret("test".sha3()).unwrap();
+		let key = KeyPair::from_secret_slice(&"test".sha3()).unwrap();
 		let secret = key.secret();
 
 		let block_number = 1;

ethcore/src/engines/authority_round.rs

@@ -354,6 +354,7 @@ mod tests {
 	use env_info::EnvInfo;
 	use header::Header;
 	use error::{Error, BlockError};
+	use ethkey::Secret;
 	use rlp::encode;
 	use block::*;
 	use tests::helpers::*;
@@ -411,8 +412,8 @@ mod tests {
 	#[test]
 	fn generates_seal_and_does_not_double_propose() {
 		let tap = AccountProvider::transient_provider();
-		let addr1 = tap.insert_account("1".sha3(), "1").unwrap();
+		let addr1 = tap.insert_account(Secret::from_slice(&"1".sha3()).unwrap(), "1").unwrap();
-		let addr2 = tap.insert_account("2".sha3(), "2").unwrap();
+		let addr2 = tap.insert_account(Secret::from_slice(&"2".sha3()).unwrap(), "2").unwrap();
 
 		let spec = Spec::new_test_round();
 		let engine = &*spec.engine;
@@ -445,7 +446,7 @@ mod tests {
 	fn proposer_switching() {
 		let mut header: Header = Header::default();
 		let tap = AccountProvider::transient_provider();
-		let addr = tap.insert_account("0".sha3(), "0").unwrap();
+		let addr = tap.insert_account(Secret::from_slice(&"0".sha3()).unwrap(), "0").unwrap();
 
 		header.set_author(addr);
 
@@ -464,7 +465,7 @@ mod tests {
 	fn rejects_future_block() {
 		let mut header: Header = Header::default();
 		let tap = AccountProvider::transient_provider();
-		let addr = tap.insert_account("0".sha3(), "0").unwrap();
+		let addr = tap.insert_account(Secret::from_slice(&"0".sha3()).unwrap(), "0").unwrap();
 
 		header.set_author(addr);
 

ethcore/src/engines/basic_authority.rs

@@ -201,6 +201,7 @@ mod tests {
 	use error::{BlockError, Error};
 	use tests::helpers::*;
 	use account_provider::AccountProvider;
+	use ethkey::Secret;
 	use header::Header;
 	use spec::Spec;
 	use engines::Seal;
@@ -261,7 +262,7 @@ mod tests {
 	#[test]
 	fn can_generate_seal() {
 		let tap = AccountProvider::transient_provider();
-		let addr = tap.insert_account("".sha3(), "").unwrap();
+		let addr = tap.insert_account(Secret::from_slice(&"".sha3()).unwrap(), "").unwrap();
 
 		let spec = new_test_authority();
 		let engine = &*spec.engine;
@@ -281,7 +282,7 @@ mod tests {
 	#[test]
 	fn seals_internally() {
 		let tap = AccountProvider::transient_provider();
-		let authority = tap.insert_account("".sha3(), "").unwrap();
+		let authority = tap.insert_account(Secret::from_slice(&"".sha3()).unwrap(), "").unwrap();
 
 		let engine = new_test_authority().engine;
 		assert!(!engine.is_sealer(&Address::default()).unwrap());

ethcore/src/engines/tendermint/message.rs

@@ -199,6 +199,7 @@ mod tests {
 	use super::*;
 	use account_provider::AccountProvider;
 	use header::Header;
+	use ethkey::Secret;
 
 	#[test]
 	fn encode_decode() {
@@ -228,7 +229,7 @@ mod tests {
 	#[test]
 	fn generate_and_verify() {
 		let tap = Arc::new(AccountProvider::transient_provider());
-		let addr = tap.insert_account("0".sha3(), "0").unwrap();
+		let addr = tap.insert_account(Secret::from_slice(&"0".sha3()).unwrap(), "0").unwrap();
 		tap.unlock_account_permanently(addr, "0".into()).unwrap();
 
 		let mi = message_info_rlp(123, 2, Step::Precommit, Some(H256::default()));

ethcore/src/engines/tendermint/mod.rs

@@ -671,6 +671,7 @@ mod tests {
 	use error::{Error, BlockError};
 	use header::Header;
 	use env_info::EnvInfo;
+	use ethkey::Secret;
 	use client::chain_notify::ChainNotify;
 	use miner::MinerService;
 	use tests::helpers::*;
@@ -721,7 +722,7 @@ mod tests {
 	}
 
 	fn insert_and_unlock(tap: &Arc<AccountProvider>, acc: &str) -> Address {
-		let addr = tap.insert_account(acc.sha3(), acc).unwrap();
+		let addr = tap.insert_account(Secret::from_slice(&acc.sha3()).unwrap(), acc).unwrap();
 		tap.unlock_account_permanently(addr, acc.into()).unwrap();
 		addr
 	}

ethcore/src/engines/validator_set/contract.rs

@@ -140,6 +140,7 @@ mod tests {
 	use account_provider::AccountProvider;
 	use transaction::{Transaction, Action};
 	use client::{BlockChainClient, EngineClient};
+	use ethkey::Secret;
 	use miner::MinerService;
 	use tests::helpers::generate_dummy_client_with_spec_and_data;
 	use super::super::ValidatorSet;
@@ -158,8 +159,9 @@ mod tests {
 	#[test]
 	fn changes_validators() {
 		let tap = Arc::new(AccountProvider::transient_provider());
-		let v0 = tap.insert_account("1".sha3(), "").unwrap();
+		let s0 = Secret::from_slice(&"1".sha3()).unwrap();
-		let v1 = tap.insert_account("0".sha3(), "").unwrap();
+		let v0 = tap.insert_account(s0.clone(), "").unwrap();
+		let v1 = tap.insert_account(Secret::from_slice(&"0".sha3()).unwrap(), "").unwrap();
 		let spec_factory = || {
 			let spec = Spec::new_validator_contract();
 			spec.engine.register_account_provider(tap.clone());
@@ -178,7 +180,7 @@ mod tests {
 			action: Action::Call(validator_contract),
 			value: 0.into(),
 			data: "f94e18670000000000000000000000000000000000000000000000000000000000000001".from_hex().unwrap(),
-		}.sign(&"1".sha3(), None);
+		}.sign(&s0, None);
 		client.miner().import_own_transaction(client.as_ref(), tx.into()).unwrap();
 		client.update_sealing();
 		assert_eq!(client.chain_info().best_block_number, 1);
@@ -190,7 +192,7 @@ mod tests {
 			action: Action::Call(validator_contract),
 			value: 0.into(),
 			data: "4d238c8e00000000000000000000000082a978b3f5962a5b0957d9ee9eef472ee55b42f1".from_hex().unwrap(),
-		}.sign(&"1".sha3(), None);
+		}.sign(&s0, None);
 		client.miner().import_own_transaction(client.as_ref(), tx.into()).unwrap();
 		client.update_sealing();
 		// The transaction is not yet included so still unable to seal.
@@ -209,7 +211,7 @@ mod tests {
 			action: Action::Call(Address::default()),
 			value: 0.into(),
 			data: Vec::new(),
-		}.sign(&"1".sha3(), None);
+		}.sign(&s0, None);
 		client.miner().import_own_transaction(client.as_ref(), tx.into()).unwrap();
 		client.update_sealing();
 		// Able to seal again.

ethcore/src/state/mod.rs

@@ -844,6 +844,7 @@ mod tests {
 	use std::str::FromStr;
 	use rustc_serialize::hex::FromHex;
 	use super::*;
+	use ethkey::Secret;
 	use util::{U256, H256, FixedHash, Address, Hashable};
 	use tests::helpers::*;
 	use devtools::*;
@@ -854,6 +855,10 @@ mod tests {
 	use trace::{FlatTrace, TraceError, trace};
 	use types::executed::CallType;
 
+	fn secret() -> Secret {
+		Secret::from_slice(&"".sha3()).unwrap()
+	}
+
 	#[test]
 	fn should_apply_create_transaction() {
 		init_log();
@@ -872,7 +877,7 @@ mod tests {
 			action: Action::Create,
 			value: 100.into(),
 			data: FromHex::from_hex("601080600c6000396000f3006000355415600957005b60203560003555").unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
@@ -932,7 +937,7 @@ mod tests {
 			action: Action::Create,
 			value: 100.into(),
 			data: FromHex::from_hex("5b600056").unwrap(),
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
@@ -969,7 +974,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("6000").unwrap());
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
@@ -1012,7 +1017,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
@@ -1054,7 +1059,7 @@ mod tests {
 			action: Action::Call(0x1.into()),
 			value: 0.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		let result = state.apply(&info, engine, &t, true).unwrap();
 
@@ -1096,7 +1101,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 0.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060006001610be0f1").unwrap());
 		let result = state.apply(&info, engine, &t, true).unwrap();
@@ -1139,7 +1144,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 0.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b611000f2").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap());
@@ -1201,7 +1206,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 0.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("6000600060006000600b618000f4").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap());
@@ -1260,7 +1265,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("5b600056").unwrap());
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
@@ -1300,7 +1305,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap());
@@ -1360,7 +1365,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006045600b6000f1").unwrap());
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
@@ -1415,7 +1420,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060ff600b6000f1").unwrap());	// not enough funds.
 		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
@@ -1458,7 +1463,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],//600480600b6000396000f35b600056
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("5b600056").unwrap());
@@ -1514,7 +1519,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1").unwrap());
@@ -1589,7 +1594,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],//600480600b6000396000f35b600056
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1505b601256").unwrap());
@@ -1662,7 +1667,7 @@ mod tests {
 			action: Action::Call(0xa.into()),
 			value: 100.into(),
 			data: vec![],
-		}.sign(&"".sha3(), None);
+		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("73000000000000000000000000000000000000000bff").unwrap());
 		state.add_balance(&0xa.into(), &50.into(), CleanupMode::NoEmpty);

ethcore/src/tests/client.rs

@@ -28,7 +28,7 @@ use rlp::View;
 use spec::Spec;
 use views::BlockView;
 use util::stats::Histogram;
-use ethkey::KeyPair;
+use ethkey::{KeyPair, Secret};
 use transaction::{PendingTransaction, Transaction, Action};
 use miner::MinerService;
 
@@ -290,7 +290,7 @@ fn change_history_size() {
 
 #[test]
 fn does_not_propagate_delayed_transactions() {
-	let key = KeyPair::from_secret("test".sha3()).unwrap();
+	let key = KeyPair::from_secret(Secret::from_slice(&"test".sha3()).unwrap()).unwrap();
 	let secret = key.secret();
 	let tx0 = PendingTransaction::new(Transaction {
 		nonce: 0.into(),

ethcore/src/tests/helpers.rs

@@ -163,7 +163,7 @@ pub fn generate_dummy_client_with_spec_and_data<F>(get_test_spec: F, block_numbe
 	let mut last_hashes = vec![];
 	let mut last_header = genesis_header.clone();
 
-	let kp = KeyPair::from_secret("".sha3()).unwrap();
+	let kp = KeyPair::from_secret_slice(&"".sha3()).unwrap();
 	let author = kp.address();
 
 	let mut n = 0;

ethcore/src/types/transaction.rs

@@ -102,6 +102,7 @@ impl HeapSizeOf for Transaction {
 impl From<ethjson::state::Transaction> for SignedTransaction {
 	fn from(t: ethjson::state::Transaction) -> Self {
 		let to: Option<ethjson::hash::Address> = t.to.into();
+		let secret = Secret::from_slice(&t.secret.0).expect("Valid secret expected.");
 		Transaction {
 			nonce: t.nonce.into(),
 			gas_price: t.gas_price.into(),
@@ -112,7 +113,7 @@ impl From<ethjson::state::Transaction> for SignedTransaction {
 			},
 			value: t.value.into(),
 			data: t.data.into(),
-		}.sign(&t.secret.into(), None)
+		}.sign(&secret, None)
 	}
 }
 

ethcrypto/src/lib.rs

@@ -166,7 +166,7 @@ pub mod aes {
 /// ECDH functions
 #[cfg_attr(feature="dev", allow(similar_names))]
 pub mod ecdh {
-	use secp256k1::{ecdh, key};
+	use secp256k1::{ecdh, key, Error as SecpError};
 	use ethkey::{Secret, Public, SECP256K1};
 	use Error;
 
@@ -180,13 +180,11 @@ pub mod ecdh {
 		};
 
 		let publ = key::PublicKey::from_slice(context, &pdata)?;
-		// no way to create SecretKey from raw byte array.
+		let sec = key::SecretKey::from_slice(context, &secret)?;
-		let sec: &key::SecretKey = unsafe { ::std::mem::transmute(secret) };
+		let shared = ecdh::SharedSecret::new_raw(context, &publ, &sec);
-		let shared = ecdh::SharedSecret::new_raw(context, &publ, sec);
 
-		let mut s = Secret::default();
+		Secret::from_slice(&shared[0..32])
-		s.copy_from_slice(&shared[0..32]);
+			.map_err(|_| Error::Secp(SecpError::InvalidSecretKey))
-		Ok(s)
 	}
 }
 

ethkey/src/brain.rs

@@ -15,7 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use keccak::Keccak256;
-use super::{KeyPair, Error, Generator};
+use super::{KeyPair, Error, Generator, Secret};
 
 /// Simple brainwallet.
 pub struct Brain(String);
@@ -38,9 +38,11 @@ impl Generator for Brain {
 			match i > 16384 {
 				false => i += 1,
 				true => {
-					let result = KeyPair::from_secret(secret.clone().into());
+					if let Ok(secret) = Secret::from_slice(&secret) {
-					if result.as_ref().ok().map_or(false, |r| r.address()[0] == 0) {
+						let result = KeyPair::from_secret(secret);
-						return result;
+						if result.as_ref().ok().map_or(false, |r| r.address()[0] == 0) {
+							return result;
+						}
 					}
 				},
 			}

ethkey/src/keypair.rs

@@ -60,11 +60,14 @@ impl KeyPair {
 		Ok(keypair)
 	}
 
+	pub fn from_secret_slice(slice: &[u8]) -> Result<KeyPair, Error> {
+		Self::from_secret(Secret::from_slice(slice)?)
+	}
+
 	pub fn from_keypair(sec: key::SecretKey, publ: key::PublicKey) -> Self {
 		let context = &SECP256K1;
 		let serialized = publ.serialize_vec(context, false);
-		let mut secret = Secret::default();
+		let secret = Secret::from(sec);
-		secret.copy_from_slice(&sec[0..32]);
 		let mut public = Public::default();
 		public.copy_from_slice(&serialized[1..65]);
 

ethkey/src/lib.rs

@@ -29,6 +29,7 @@ mod keccak;
 mod prefix;
 mod random;
 mod signature;
+mod secret;
 
 lazy_static! {
 	pub static ref SECP256K1: secp256k1::Secp256k1 = secp256k1::Secp256k1::new();
@@ -46,10 +47,10 @@ pub use self::keypair::{KeyPair, public_to_address};
 pub use self::prefix::Prefix;
 pub use self::random::Random;
 pub use self::signature::{sign, verify_public, verify_address, recover, Signature};
+pub use self::secret::Secret;
 
 use bigint::hash::{H160, H256, H512};
 
 pub type Address = H160;
-pub type Secret = H256;
 pub type Message = H256;
 pub type Public = H512;

ethkey/src/secret.rs

@@ -0,0 +1,69 @@
+// Copyright 2015, 2016 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+use std::fmt;
+use std::ops::Deref;
+use std::str::FromStr;
+use secp256k1::key;
+use bigint::hash::H256;
+use {Error};
+
+#[derive(Clone, PartialEq, Eq)]
+pub struct Secret {
+	inner: H256,
+}
+
+impl fmt::Debug for Secret {
+	fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+		write!(fmt, "Secret: 0x{:x}{:x}..{:x}{:x}", self.inner[0], self.inner[1], self.inner[30], self.inner[31])
+	}
+}
+
+impl Secret {
+	pub fn from_slice(key: &[u8]) -> Result<Self, Error> {
+		if key.len() != 32 {
+			return Err(Error::InvalidSecret);
+		}
+
+		let mut h = H256::default();
+		h.copy_from_slice(&key[0..32]);
+		Ok(Secret { inner: h })
+	}
+}
+
+impl FromStr for Secret {
+	type Err = Error;
+	fn from_str(s: &str) -> Result<Self, Self::Err> {
+		let hash = H256::from_str(s).map_err(|e| Error::Custom(format!("{:?}", e)))?;
+		Self::from_slice(&hash)
+	}
+}
+
+impl From<key::SecretKey> for Secret {
+	fn from(key: key::SecretKey) -> Self {
+		Self::from_slice(&key[0..32])
+			.expect("`key::SecretKey` is valid (no way to construct invalid one); qed")
+	}
+}
+
+impl Deref for Secret {
+	type Target = H256;
+
+	fn deref(&self) -> &Self::Target {
+		&self.inner
+	}
+}
+

ethkey/src/signature.rs

@@ -16,7 +16,7 @@
 
 use std::ops::{Deref, DerefMut};
 use std::cmp::PartialEq;
-use std::{mem, fmt};
+use std::fmt;
 use std::str::FromStr;
 use std::hash::{Hash, Hasher};
 use secp256k1::{Message as SecpMessage, RecoverableSignature, RecoveryId, Error as SecpError};
@@ -169,9 +169,8 @@ impl DerefMut for Signature {
 
 pub fn sign(secret: &Secret, message: &Message) -> Result<Signature, Error> {
 	let context = &SECP256K1;
-	// no way to create from raw byte array.
+	let sec = SecretKey::from_slice(context, &secret)?;
-	let sec: &SecretKey = unsafe { mem::transmute(secret) };
+	let s = context.sign_recoverable(&SecpMessage::from_slice(&message[..])?, &sec)?;
-	let s = context.sign_recoverable(&SecpMessage::from_slice(&message[..])?, sec)?;
 	let (rec_id, data) = s.serialize_compact(context);
 	let mut data_arr = [0; 65];
 

ethstore/src/account/safe_account.rs

@@ -122,16 +122,14 @@ impl Crypto {
 			return Err(Error::InvalidPassword);
 		}
 
-		let mut secret = Secret::default();
-
 		match self.cipher {
 			Cipher::Aes128Ctr(ref params) => {
 				let from = 32 - self.ciphertext.len();
-				crypto::aes::decrypt(&derived_left_bits, &params.iv, &self.ciphertext, &mut (&mut *secret)[from..])
+				let mut secret = [0; 32];
+				crypto::aes::decrypt(&derived_left_bits, &params.iv, &self.ciphertext, &mut secret[from..]);
+				Ok(Secret::from_slice(&secret)?)
 			},
 		}
-
-		Ok(secret)
 	}
 }
 

ethstore/src/presale.rs

@@ -47,7 +47,7 @@ impl PresaleWallet {
 		let len = crypto::aes::decrypt_cbc(&derived_key, &self.iv, &self.ciphertext, &mut key).map_err(|_| Error::InvalidPassword)?;
 		let unpadded = &key[..len];
 
-		let secret = Secret::from(unpadded.keccak256());
+		let secret = Secret::from_slice(&unpadded.keccak256())?;
 		if let Ok(kp) = KeyPair::from_secret(secret) {
 			if kp.address() == self.address {
 				return Ok(kp)

ethstore/tests/api.rs

@@ -133,9 +133,9 @@ fn secret_store_load_pat_files() {
 #[test]
 fn test_decrypting_files_with_short_ciphertext() {
 	// 31e9d1e6d844bd3a536800ef8d8be6a9975db509, 30
-	let kp1 = KeyPair::from_secret("000081c29e8142bb6a81bef5a92bda7a8328a5c85bb2f9542e76f9b0f94fc018".into()).unwrap();
+	let kp1 = KeyPair::from_secret("000081c29e8142bb6a81bef5a92bda7a8328a5c85bb2f9542e76f9b0f94fc018".parse().unwrap()).unwrap();
 	// d1e64e5480bfaf733ba7d48712decb8227797a4e , 31
-	let kp2 = KeyPair::from_secret("00fa7b3db73dc7dfdf8c5fbdb796d741e4488628c41fc4febd9160a866ba0f35".into()).unwrap();
+	let kp2 = KeyPair::from_secret("00fa7b3db73dc7dfdf8c5fbdb796d741e4488628c41fc4febd9160a866ba0f35".parse().unwrap()).unwrap();
 	let dir = DiskDirectory::at(ciphertext_path());
 	let store = EthStore::open(Box::new(dir)).unwrap();
 	let accounts = store.accounts().unwrap();

parity/configuration.rs

@@ -20,9 +20,10 @@ use std::net::SocketAddr;
 use std::path::{Path, PathBuf};
 use std::cmp::max;
 use cli::{Args, ArgsError};
-use util::{Hashable, U256, Uint, Bytes, version_data, Secret, Address};
+use util::{Hashable, U256, Uint, Bytes, version_data, Address};
 use util::log::Colour;
 use ethsync::{NetworkConfiguration, is_valid_node_url, AllowIP};
+use ethcore::ethstore::ethkey::Secret;
 use ethcore::client::{VMType};
 use ethcore::miner::{MinerOptions, Banning};
 use ethcore::verification::queue::VerifierSettings;
@@ -603,7 +604,13 @@ impl Configuration {
 		let (listen, public) = self.net_addresses()?;
 		ret.listen_address = listen.map(|l| format!("{}", l));
 		ret.public_address = public.map(|p| format!("{}", p));
-		ret.use_secret = self.args.flag_node_key.as_ref().map(|s| s.parse::<Secret>().unwrap_or_else(|_| s.sha3()));
+		ret.use_secret = match self.args.flag_node_key.as_ref()
+			.map(|s| s.parse::<Secret>().or_else(|_| Secret::from_slice(&s.sha3())).map_err(|e| format!("Invalid key: {:?}", e))
+			) {
+			None => None,
+			Some(Ok(key)) => Some(key),
+			Some(Err(err)) => return Err(err),
+		};
 		ret.discovery_enabled = !self.args.flag_no_discovery && !self.args.flag_nodiscover;
 		ret.max_peers = self.max_peers();
 		ret.min_peers = self.min_peers();

parity/presale.rs

@@ -40,6 +40,6 @@ pub fn execute(cmd: ImportWallet) -> Result<String, String> {
 	let acc_provider = AccountProvider::new(secret_store);
 	let wallet = PresaleWallet::open(cmd.wallet_path).map_err(|_| "Unable to open presale wallet.")?;
 	let kp = wallet.decrypt(&password).map_err(|_| "Invalid password.")?;
-	let address = acc_provider.insert_account(*kp.secret(), &password).unwrap();
+	let address = acc_provider.insert_account(kp.secret().clone(), &password).unwrap();
 	Ok(format!("{:?}", address))
 }

rpc/src/v1/impls/parity_accounts.rs

@@ -19,7 +19,7 @@ use std::sync::{Arc, Weak};
 use std::collections::BTreeMap;
 use util::{Address};
 
-use ethkey::{Brain, Generator};
+use ethkey::{Brain, Generator, Secret};
 use ethcore::account_provider::AccountProvider;
 use ethcore::client::MiningBlockChainClient;
 
@@ -73,7 +73,8 @@ impl<C: 'static> ParityAccounts for ParityAccountsClient<C> where C: MiningBlock
 		self.active()?;
 		let store = take_weak!(self.accounts);
 
-		store.insert_account(*Brain::new(phrase).generate().unwrap().secret(), &pass)
+		let brain = Brain::new(phrase).generate().unwrap();
+		store.insert_account(brain.secret().clone(), &pass)
 			.map(Into::into)
 			.map_err(|e| errors::account("Could not create account.", e))
 	}
@@ -92,7 +93,9 @@ impl<C: 'static> ParityAccounts for ParityAccountsClient<C> where C: MiningBlock
 		self.active()?;
 		let store = take_weak!(self.accounts);
 
-		store.insert_account(secret.into(), &pass)
+		let secret = Secret::from_slice(&secret.0)
+			.map_err(|e| errors::account("Could not create account.", e))?;
+		store.insert_account(secret, &pass)
 			.map(Into::into)
 			.map_err(|e| errors::account("Could not create account.", e))
 	}

rpc/src/v1/tests/eth.rs

@@ -307,7 +307,7 @@ const POSITIVE_NONCE_SPEC: &'static [u8] = br#"{
 
 #[test]
 fn eth_transaction_count() {
-	let secret = "8a283037bb19c4fed7b1c569e40c7dcff366165eb869110a1b11532963eb9cb2".into();
+	let secret = "8a283037bb19c4fed7b1c569e40c7dcff366165eb869110a1b11532963eb9cb2".parse().unwrap();
 	let tester = EthTester::from_spec(Spec::load(TRANSACTION_COUNT_SPEC).expect("invalid chain spec"));
 	let address = tester.accounts.insert_account(secret, "").unwrap();
 	tester.accounts.unlock_account_permanently(address, "".into()).unwrap();

rpc/src/v1/types/hash.rs

@@ -25,7 +25,7 @@ use util::{H64 as Eth64, H160 as Eth160, H256 as Eth256, H520 as Eth520, H512 as
 macro_rules! impl_hash {
 	($name: ident, $other: ident, $size: expr) => {
 		/// Hash serialization
-		pub struct $name([u8; $size]);
+		pub struct $name(pub [u8; $size]);
 
 		impl Eq for $name { }
 

sync/src/api.rs

@@ -23,6 +23,7 @@ use network::{NetworkProtocolHandler, NetworkService, NetworkContext, PeerId, Pr
 	AllowIP as NetworkAllowIP};
 use util::{U256, H256, H512};
 use io::{TimerToken};
+use ethcore::ethstore::ethkey::Secret;
 use ethcore::client::{BlockChainClient, ChainNotify};
 use ethcore::snapshot::SnapshotService;
 use ethcore::header::BlockNumber;
@@ -476,7 +477,7 @@ pub struct NetworkConfiguration {
 	/// List of initial node addresses
 	pub boot_nodes: Vec<String>,
 	/// Use provided node key instead of default
-	pub use_secret: Option<H256>,
+	pub use_secret: Option<Secret>,
 	/// Max number of connected peers to maintain
 	pub max_peers: u32,
 	/// Min number of connected peers to maintain
@@ -667,3 +668,4 @@ impl ManageNetwork for LightSync {
 		NetworkConfiguration::from(self.network.config().clone())
 	}
 }
+

sync/src/tests/consensus.rs

@@ -22,7 +22,7 @@ use ethcore::spec::Spec;
 use ethcore::miner::MinerService;
 use ethcore::transaction::*;
 use ethcore::account_provider::AccountProvider;
-use ethkey::KeyPair;
+use ethkey::{KeyPair, Secret};
 use super::helpers::*;
 use SyncConfig;
 
@@ -41,7 +41,7 @@ impl IoHandler<ClientIoMessage> for TestIoHandler {
 	}
 }
 
-fn new_tx(secret: &H256, nonce: U256) -> PendingTransaction {
+fn new_tx(secret: &Secret, nonce: U256) -> PendingTransaction {
 	let signed = Transaction {
 		nonce: nonce.into(),
 		gas_price: 0.into(),
@@ -55,8 +55,8 @@ fn new_tx(secret: &H256, nonce: U256) -> PendingTransaction {
 
 #[test]
 fn authority_round() {
-	let s0 = KeyPair::from_secret("1".sha3()).unwrap();
+	let s0 = KeyPair::from_secret_slice(&"1".sha3()).unwrap();
-	let s1 = KeyPair::from_secret("0".sha3()).unwrap();
+	let s1 = KeyPair::from_secret_slice(&"0".sha3()).unwrap();
 	let spec_factory = || {
 		let spec = Spec::new_test_round();
 		let account_provider = AccountProvider::transient_provider();
@@ -118,8 +118,8 @@ fn authority_round() {
 
 #[test]
 fn tendermint() {
-	let s0 = KeyPair::from_secret("1".sha3()).unwrap();
+	let s0 = KeyPair::from_secret_slice(&"1".sha3()).unwrap();
-	let s1 = KeyPair::from_secret("0".sha3()).unwrap();
+	let s1 = KeyPair::from_secret_slice(&"0".sha3()).unwrap();
 	let spec_factory = || {
 		let spec = Spec::new_test_tendermint();
 		let account_provider = AccountProvider::transient_provider();

util/network/src/handshake.rs

@@ -165,7 +165,7 @@ impl Handshake {
 		self.id.clone_from_slice(remote_public);
 		self.remote_nonce.clone_from_slice(remote_nonce);
 		self.remote_version = remote_version;
-		let shared = ecdh::agree(host_secret, &self.id)?;
+		let shared = *ecdh::agree(host_secret, &self.id)?;
 		let signature = H520::from_slice(sig);
 		self.remote_ephemeral = recover(&signature.into(), &(&shared ^ &self.remote_nonce))?;
 		Ok(())
@@ -271,7 +271,7 @@ impl Handshake {
 			let (nonce, _) = rest.split_at_mut(32);
 
 			// E(remote-pubk, S(ecdhe-random, ecdh-shared-secret^nonce) || H(ecdhe-random-pubk) || pubk || nonce || 0x0)
-			let shared = ecdh::agree(secret, &self.id)?;
+			let shared = *ecdh::agree(secret, &self.id)?;
 			sig.copy_from_slice(&*sign(self.ecdhe.secret(), &(&shared ^ &self.nonce))?);
 			self.ecdhe.public().sha3_into(hepubk);
 			pubk.copy_from_slice(public);
@@ -366,7 +366,7 @@ mod test {
 	#[test]
 	fn test_handshake_auth_plain() {
 		let mut h = create_handshake(None);
-		let secret = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291".into();
+		let secret = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291".parse().unwrap();
 		let auth =
 			"\
 			048ca79ad18e4b0659fab4853fe5bc58eb83992980f4c9cc147d2aa31532efd29a3d3dc6a3d89eaf\
@@ -387,7 +387,7 @@ mod test {
 	#[test]
 	fn test_handshake_auth_eip8() {
 		let mut h = create_handshake(None);
-		let secret = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291".into();
+		let secret = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291".parse().unwrap();
 		let auth =
 			"\
 			01b304ab7578555167be8154d5cc456f567d5ba302662433674222360f08d5f1534499d3678b513b\
@@ -413,7 +413,7 @@ mod test {
 	#[test]
 	fn test_handshake_auth_eip8_2() {
 		let mut h = create_handshake(None);
-		let secret = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291".into();
+		let secret = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291".parse().unwrap();
 		let auth =
 			"\
 			01b8044c6c312173685d1edd268aa95e1d495474c6959bcdd10067ba4c9013df9e40ff45f5bfd6f7\
@@ -444,7 +444,7 @@ mod test {
 	fn test_handshake_ack_plain() {
 		let remote = "fda1cff674c90c9a197539fe3dfb53086ace64f83ed7c6eabec741f7f381cc803e52ab2cd55d5569bce4347107a310dfd5f88a010cd2ffd1005ca406f1842877".into();
 		let mut h = create_handshake(Some(&remote));
-		let secret = "49a7b37aa6f6645917e7b807e9d1c00d4fa71f18343b0d4122a4d2df64dd6fee".into();
+		let secret = "49a7b37aa6f6645917e7b807e9d1c00d4fa71f18343b0d4122a4d2df64dd6fee".parse().unwrap();
 		let ack =
 			"\
 			049f8abcfa9c0dc65b982e98af921bc0ba6e4243169348a236abe9df5f93aa69d99cadddaa387662\
@@ -464,7 +464,7 @@ mod test {
 	fn test_handshake_ack_eip8() {
 		let remote = "fda1cff674c90c9a197539fe3dfb53086ace64f83ed7c6eabec741f7f381cc803e52ab2cd55d5569bce4347107a310dfd5f88a010cd2ffd1005ca406f1842877".into();
 		let mut h = create_handshake(Some(&remote));
-		let secret = "49a7b37aa6f6645917e7b807e9d1c00d4fa71f18343b0d4122a4d2df64dd6fee".into();
+		let secret = "49a7b37aa6f6645917e7b807e9d1c00d4fa71f18343b0d4122a4d2df64dd6fee".parse().unwrap();
 		let ack =
 			"\
 			01ea0451958701280a56482929d3b0757da8f7fbe5286784beead59d95089c217c9b917788989470\
@@ -493,7 +493,7 @@ mod test {
 	fn test_handshake_ack_eip8_2() {
 		let remote = "fda1cff674c90c9a197539fe3dfb53086ace64f83ed7c6eabec741f7f381cc803e52ab2cd55d5569bce4347107a310dfd5f88a010cd2ffd1005ca406f1842877".into();
 		let mut h = create_handshake(Some(&remote));
-		let secret = "49a7b37aa6f6645917e7b807e9d1c00d4fa71f18343b0d4122a4d2df64dd6fee".into();
+		let secret = "49a7b37aa6f6645917e7b807e9d1c00d4fa71f18343b0d4122a4d2df64dd6fee".parse().unwrap();
 		let ack =
 			"\
 			01f004076e58aae772bb101ab1a8e64e01ee96e64857ce82b1113817c6cdd52c09d26f7b90981cd7\

util/network/src/host.rs

@@ -1207,7 +1207,7 @@ fn load_key(path: &Path) -> Option<Secret> {
 fn key_save_load() {
 	use ::devtools::RandomTempPath;
 	let temp_path = RandomTempPath::create_dir();
-	let key = H256::random();
+	let key = Secret::from_slice(&H256::random()).unwrap();
 	save_key(temp_path.as_path(), &key);
 	let r = load_key(temp_path.as_path());
 	assert_eq!(key, r.unwrap());
@@ -1217,8 +1217,9 @@ fn key_save_load() {
 #[test]
 fn host_client_url() {
 	let mut config = NetworkConfiguration::new_local();
-	let key = "6f7b0d801bc7b5ce7bbd930b84fd0369b3eb25d09be58d64ba811091046f3aa2".into();
+	let key = "6f7b0d801bc7b5ce7bbd930b84fd0369b3eb25d09be58d64ba811091046f3aa2".parse().unwrap();
 	config.use_secret = Some(key);
 	let host: Host = Host::new(config, Arc::new(NetworkStats::new())).unwrap();
 	assert!(host.local_url().starts_with("enode://101b3ef5a4ea7a1c7928e24c4c75fd053c235d7b80c22ae5c03d145d0ac7396e2a4ffff9adee3133a7b05044a5cee08115fd65145e5165d646bde371010d803c@"));
 }
+

util/src/lib.rs

@@ -164,6 +164,3 @@ pub use timer::*;
 
 /// 160-bit integer representing account address
 pub type Address = H160;
-
-/// Secret
-pub type Secret = H256;