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,23 +1875,23 @@ 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(),
 			gas: 100_000.into(),
 			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(),
 			gas: 100_000.into(),
 			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

@@ -1671,7 +1671,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 addr2 = tap.insert_account("2".sha3(), "2").unwrap();
+		let addr1 = tap.insert_account(Secret::from_slice(&"1".sha3()).unwrap(), "1").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

@@ -166,7 +166,7 @@ impl Decodable for ConsensusMessage {
 			}
 		})
   }
-} 
+}
 
 impl Encodable for ConsensusMessage {
 	fn rlp_append(&self, s: &mut RlpStream) {
@@ -199,11 +199,12 @@ mod tests {
 	use super::*;
 	use account_provider::AccountProvider;
 	use header::Header;
+	use ethkey::Secret;
 
 	#[test]
 	fn encode_decode() {
 		let message = ConsensusMessage {
-			signature: H520::default(),	
+			signature: H520::default(),
 			height: 10,
 			round: 123,
 			step: Step::Precommit,
@@ -214,7 +215,7 @@ mod tests {
 		assert_eq!(message, rlp.as_val());
 
 		let message = ConsensusMessage {
-			signature: H520::default(),	
+			signature: H520::default(),
 			height: 1314,
 			round: 0,
 			step: Step::Prevote,
@@ -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

@@ -290,19 +290,19 @@ impl Tendermint {
 	}
 
 	fn is_height(&self, message: &ConsensusMessage) -> bool {
-		message.is_height(self.height.load(AtomicOrdering::SeqCst)) 
+		message.is_height(self.height.load(AtomicOrdering::SeqCst))
 	}
 
 	fn is_round(&self, message: &ConsensusMessage) -> bool {
-		message.is_round(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst)) 
+		message.is_round(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst))
 	}
 
 	fn increment_round(&self, n: Round) {
 		trace!(target: "poa", "increment_round: New round.");
 		self.round.fetch_add(n, AtomicOrdering::SeqCst);
 	}
 
-	fn should_unlock(&self, lock_change_round: Round) -> bool { 
+	fn should_unlock(&self, lock_change_round: Round) -> bool {
 		self.last_lock.load(AtomicOrdering::SeqCst) < lock_change_round
 			&& lock_change_round < self.round.load(AtomicOrdering::SeqCst)
 	}
@@ -316,7 +316,7 @@ impl Tendermint {
 	fn has_enough_future_step_votes(&self, message: &ConsensusMessage) -> bool {
 		if message.round > self.round.load(AtomicOrdering::SeqCst) {
 			let step_votes = self.votes.count_step_votes(message.height, message.round, message.step);
-			self.is_above_threshold(step_votes)	
+			self.is_above_threshold(step_votes)
 		} else {
 			false
 		}
@@ -502,7 +502,7 @@ impl Engine for Tendermint {
 	}
 
 	fn verify_block_unordered(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
-		let proposal = ConsensusMessage::new_proposal(header)?;	
+		let proposal = ConsensusMessage::new_proposal(header)?;
 		let proposer = proposal.verify()?;
 		if !self.is_authority(&proposer) {
 			Err(EngineError::NotAuthorized(proposer))?
@@ -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
 	}
@@ -886,7 +887,7 @@ mod tests {
 	fn relays_messages() {
 		let (spec, tap) = setup();
 		let engine = spec.engine.clone();
-		
+
 		let v0 = insert_and_register(&tap, engine.as_ref(), "0");
 		let v1 = insert_and_register(&tap, engine.as_ref(), "1");
 

ethcore/src/engines/validator_set/contract.rs

@@ -118,17 +118,17 @@ mod provider {
 			}
 		}
 		fn as_string<T: fmt::Debug>(e: T) -> String { format!("{:?}", e) }
-		
+
 		/// Auto-generated from: `{"constant":true,"inputs":[],"name":"getValidators","outputs":[{"name":"","type":"address[]"}],"payable":false,"type":"function"}`
 		#[allow(dead_code)]
-		pub fn get_validators(&self) -> Result<Vec<util::Address>, String> { 
+		pub fn get_validators(&self) -> Result<Vec<util::Address>, String> {
 			let call = self.contract.function("getValidators".into()).map_err(Self::as_string)?;
 			let data = call.encode_call(
 				vec![]
 			).map_err(Self::as_string)?;
 			let output = call.decode_output((self.do_call)(self.address.clone(), data)?).map_err(Self::as_string)?;
 			let mut result = output.into_iter().rev().collect::<Vec<_>>();
-			Ok(({ let r = result.pop().ok_or("Invalid return arity")?; let r = r.to_array().and_then(|v| v.into_iter().map(|a| a.to_address()).collect::<Option<Vec<[u8; 20]>>>()).ok_or("Invalid type returned")?; r.into_iter().map(|a| util::Address::from(a)).collect::<Vec<_>>() })) 
+			Ok(({ let r = result.pop().ok_or("Invalid return arity")?; let r = r.to_array().and_then(|v| v.into_iter().map(|a| a.to_address()).collect::<Option<Vec<[u8; 20]>>>()).ok_or("Invalid type returned")?; r.into_iter().map(|a| util::Address::from(a)).collect::<Vec<_>>() }))
 		}
 	}
 }
@@ -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 v1 = tap.insert_account("0".sha3(), "").unwrap();
+		let s0 = Secret::from_slice(&"1".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.