Merge branch 'master' into better-timeouts

Cargo.lock

@@ -1501,7 +1501,7 @@ dependencies = [
 [[package]]
 name = "parity-ui-precompiled"
 version = "1.4.0"
-source = "git+https://github.com/ethcore/js-precompiled.git#e21ae69190fa390f5550e5cf17f5ea362ba4db41"
+source = "git+https://github.com/ethcore/js-precompiled.git#73497e202735f4a6d2bf55de6509fc010419871b"
 dependencies = [
  "parity-dapps-glue 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

ethcore/light/src/client/mod.rs

@@ -66,6 +66,9 @@ pub trait LightChainClient: Send + Sync {
 	/// Clear the queue.
 	fn clear_queue(&self);
 
+	/// Flush the queue.
+	fn flush_queue(&self);
+
 	/// Get queue info.
 	fn queue_info(&self) -> queue::QueueInfo;
 
@@ -130,7 +133,7 @@ impl Client {
 
 		BlockChainInfo {
 			total_difficulty: best_block.total_difficulty,
-			pending_total_difficulty: best_block.total_difficulty,
+			pending_total_difficulty: best_block.total_difficulty + self.queue.total_difficulty(),
 			genesis_hash: genesis_hash,
 			best_block_hash: best_block.hash,
 			best_block_number: best_block.number,
@@ -151,6 +154,11 @@ impl Client {
 		self.chain.get_header(id)
 	}
 
+	/// Flush the header queue.
+	pub fn flush_queue(&self) {
+		self.queue.flush()
+	}
+
 	/// Get the `i`th CHT root.
 	pub fn cht_root(&self, i: usize) -> Option<H256> {
 		self.chain.cht_root(i)
@@ -211,6 +219,10 @@ impl LightChainClient for Client {
 		self.queue.clear()
 	}
 
+	fn flush_queue(&self) {
+		Client::flush_queue(self);
+	}
+
 	fn queue_info(&self) -> queue::QueueInfo {
 		self.queue.queue_info()
 	}

ethcore/light/src/net/buffer_flow.rs

@@ -23,7 +23,7 @@
 //! This module provides an interface for configuration of buffer
 //! flow costs and recharge rates.
 //!
-//! Current default costs are picked completely arbitrarily, not based 
+//! Current default costs are picked completely arbitrarily, not based
 //! on any empirical timings or mathematical models.
 
 use request;
@@ -184,6 +184,23 @@ impl FlowParams {
 		}
 	}
 
+	/// Create effectively infinite flow params.
+	pub fn free() -> Self {
+		let free_cost = Cost(0.into(), 0.into());
+		FlowParams {
+			limit: (!0u64).into(),
+			recharge: 1.into(),
+			costs: CostTable {
+				headers: free_cost.clone(),
+				bodies: free_cost.clone(),
+				receipts: free_cost.clone(),
+				state_proofs: free_cost.clone(),
+				contract_codes: free_cost.clone(),
+				header_proofs: free_cost.clone(),
+			}
+		}
+	}
+
 	/// Get a reference to the buffer limit.
 	pub fn limit(&self) -> &U256 { &self.limit }
 
@@ -209,7 +226,7 @@ impl FlowParams {
 		cost.0 + (amount * cost.1)
 	}
 
-	/// Compute the maximum number of costs of a specific kind which can be made 
+	/// Compute the maximum number of costs of a specific kind which can be made
 	/// with the given buffer.
 	/// Saturates at `usize::max()`. This is not a problem in practice because
 	/// this amount of requests is already prohibitively large.
@@ -317,4 +334,4 @@ mod tests {
 
 		assert_eq!(buffer.estimate, 100.into());
 	}
-}
+}

ethcore/light/src/net/mod.rs

@@ -42,7 +42,6 @@ use self::error::Punishment;
 use self::request_set::RequestSet;
 use self::id_guard::IdGuard;
 
-mod buffer_flow;
 mod context;
 mod error;
 mod status;
@@ -51,6 +50,8 @@ mod request_set;
 #[cfg(test)]
 mod tests;
 
+pub mod buffer_flow;
+
 pub use self::error::Error;
 pub use self::context::{BasicContext, EventContext, IoContext};
 pub use self::status::{Status, Capabilities, Announcement};
@@ -285,7 +286,7 @@ pub struct LightProtocol {
 	peers: RwLock<PeerMap>,
 	capabilities: RwLock<Capabilities>,
 	flow_params: FlowParams, // assumed static and same for every peer.
-	handlers: Vec<Box<Handler>>,
+	handlers: Vec<Arc<Handler>>,
 	req_id: AtomicUsize,
 }
 
@@ -410,11 +411,11 @@ impl LightProtocol {
 	}
 
 	/// Add an event handler.
-	/// Ownership will be transferred to the protocol structure,
-	/// and the handler will be kept alive as long as it is.
+	///
 	/// These are intended to be added when the protocol structure
-	/// is initialized as a means of customizing its behavior.
-	pub fn add_handler(&mut self, handler: Box<Handler>) {
+	/// is initialized as a means of customizing its behavior,
+	/// and dispatching requests immediately upon events.
+	pub fn add_handler(&mut self, handler: Arc<Handler>) {
 		self.handlers.push(handler);
 	}
 
@@ -484,8 +485,10 @@ impl LightProtocol {
 		}
 	}
 
-	// handle a packet using the given io context.
-	fn handle_packet(&self, io: &IoContext, peer: &PeerId, packet_id: u8, data: &[u8]) {
+	/// Handle an LES packet using the given io context.
+	/// Packet data is _untrusted_, which means that invalid data won't lead to
+	/// issues.
+	pub fn handle_packet(&self, io: &IoContext, peer: &PeerId, packet_id: u8, data: &[u8]) {
 		let rlp = UntrustedRlp::new(data);
 
 		trace!(target: "les", "Incoming packet {} from peer {}", packet_id, peer);
@@ -557,19 +560,8 @@ impl LightProtocol {
 		}
 	}
 
-	fn tick_handlers(&self, io: &IoContext) {
-		for handler in &self.handlers {
-			handler.tick(&TickCtx {
-				io: io,
-				proto: self,
-			})
-		}
-	}
-}
-
-impl LightProtocol {
-	// called when a peer connects.
-	fn on_connect(&self, peer: &PeerId, io: &IoContext) {
+	/// called when a peer connects.
+	pub fn on_connect(&self, peer: &PeerId, io: &IoContext) {
 		let proto_version = match io.protocol_version(*peer).ok_or(Error::WrongNetwork) {
 			Ok(pv) => pv,
 			Err(e) => { punish(*peer, io, e); return }
@@ -603,8 +595,8 @@ impl LightProtocol {
 		io.send(*peer, packet::STATUS, status_packet);
 	}
 
-	// called when a peer disconnects.
-	fn on_disconnect(&self, peer: PeerId, io: &IoContext) {
+	/// called when a peer disconnects.
+	pub fn on_disconnect(&self, peer: PeerId, io: &IoContext) {
 		trace!(target: "les", "Peer {} disconnecting", peer);
 
 		self.pending_peers.write().remove(&peer);
@@ -623,6 +615,27 @@ impl LightProtocol {
 		}
 	}
 
+	/// Execute the given closure with a basic context derived from the I/O context.
+	pub fn with_context<F, T>(&self, io: &IoContext, f: F) -> T
+		where F: FnOnce(&BasicContext) -> T
+	{
+		f(&TickCtx {
+			io: io,
+			proto: self,
+		})
+	}
+
+	fn tick_handlers(&self, io: &IoContext) {
+		for handler in &self.handlers {
+			handler.tick(&TickCtx {
+				io: io,
+				proto: self,
+			})
+		}
+	}
+}
+
+impl LightProtocol {
 	// Handle status message from peer.
 	fn status(&self, peer: &PeerId, io: &IoContext, data: UntrustedRlp) -> Result<(), Error> {
 		let pending = match self.pending_peers.write().remove(peer) {

ethcore/light/src/net/tests/mod.rs

@@ -18,7 +18,7 @@
 //! These don't test of the higher level logic on top of
 
 use ethcore::blockchain_info::BlockChainInfo;
-use ethcore::client::{BlockChainClient, EachBlockWith, TestBlockChainClient};
+use ethcore::client::{EachBlockWith, TestBlockChainClient};
 use ethcore::ids::BlockId;
 use ethcore::transaction::PendingTransaction;
 use ethcore::encoded;
@@ -88,7 +88,7 @@ impl Provider for TestProvider {
 	}
 
 	fn reorg_depth(&self, a: &H256, b: &H256) -> Option<u64> {
-		self.0.client.tree_route(a, b).map(|route| route.index as u64)
+		self.0.client.reorg_depth(a, b)
 	}
 
 	fn earliest_state(&self) -> Option<u64> {
@@ -305,7 +305,9 @@ fn get_block_bodies() {
 	}
 
 	let request = request::Bodies {
-		block_hashes: (0..10).map(|i| provider.client.block_hash(BlockId::Number(i)).unwrap()).collect(),
+		block_hashes: (0..10).map(|i|
+			provider.client.block_header(BlockId::Number(i)).unwrap().hash()
+		).collect()
 	};
 
 	let req_id = 111;
@@ -353,8 +355,9 @@ fn get_block_receipts() {
 
 	// find the first 10 block hashes starting with `f` because receipts are only provided
 	// by the test client in that case.
-	let block_hashes: Vec<_> = (0..1000).map(|i| provider.client.block_hash(BlockId::Number(i)).unwrap())
-		.filter(|hash| format!("{}", hash).starts_with("f")).take(10).collect();
+	let block_hashes: Vec<_> = (0..1000).map(|i|
+		provider.client.block_header(BlockId::Number(i)).unwrap().hash()
+	).filter(|hash| format!("{}", hash).starts_with("f")).take(10).collect();
 
 	let request = request::Receipts {
 		block_hashes: block_hashes.clone(),

ethcore/light/src/provider.rs

@@ -83,7 +83,7 @@ pub trait Provider: Send + Sync {
 
 		(0u64..req.max as u64)
 			.map(|x: u64| x.saturating_mul(req.skip + 1))
-			.take_while(|x| if req.reverse { x < &start_num } else { best_num - start_num >= *x })
+			.take_while(|x| if req.reverse { x < &start_num } else { best_num.saturating_sub(start_num) >= *x })
 			.map(|x| if req.reverse { start_num - x } else { start_num + x })
 			.map(|x| self.block_header(BlockId::Number(x)))
 			.take_while(|x| x.is_some())

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

@@ -848,7 +848,7 @@ impl BlockChainClient for Client {
 			difficulty: header.difficulty(),
 			last_hashes: last_hashes,
 			gas_used: U256::zero(),
-			gas_limit: header.gas_limit(),
+			gas_limit: U256::max_value(),
 		};
 		// that's just a copy of the state.
 		let mut state = self.state_at(block).ok_or(CallError::StatePruned)?;
@@ -874,6 +874,7 @@ impl BlockChainClient for Client {
 	}
 
 	fn estimate_gas(&self, t: &SignedTransaction, block: BlockId) -> Result<U256, CallError> {
+		const UPPER_CEILING: u64 = 1_000_000_000_000u64;
 		let header = self.block_header(block).ok_or(CallError::StatePruned)?;
 		let last_hashes = self.build_last_hashes(header.parent_hash());
 		let env_info = EnvInfo {
@@ -883,37 +884,38 @@ impl BlockChainClient for Client {
 			difficulty: header.difficulty(),
 			last_hashes: last_hashes,
 			gas_used: U256::zero(),
-			gas_limit: header.gas_limit(),
+			gas_limit: UPPER_CEILING.into(),
 		};
 		// that's just a copy of the state.
-		let mut original_state = self.state_at(block).ok_or(CallError::StatePruned)?;
+		let original_state = self.state_at(block).ok_or(CallError::StatePruned)?;
 		let sender = t.sender().map_err(|e| {
 			let message = format!("Transaction malformed: {:?}", e);
 			ExecutionError::TransactionMalformed(message)
 		})?;
 		let balance = original_state.balance(&sender);
-		let needed_balance = t.value + t.gas * t.gas_price;
-		if balance < needed_balance {
-			// give the sender a sufficient balance
-			original_state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty);
-		}
 		let options = TransactOptions { tracing: true, vm_tracing: false, check_nonce: false };
 		let mut tx = t.clone();
 
 		let mut cond = |gas| {
-			let mut state = original_state.clone();
 			tx.gas = gas;
+
+			let mut state = original_state.clone();
+			let needed_balance = tx.value + tx.gas * tx.gas_price;
+			if balance < needed_balance {
+				// give the sender a sufficient balance
+				state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty);
+			}
+
 			Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm)
 				.transact(&tx, options.clone())
 				.map(|r| r.trace[0].result.succeeded())
 				.unwrap_or(false)
 		};
 
-		let mut upper = env_info.gas_limit;
+		let mut upper = header.gas_limit();
 		if !cond(upper) {
 			// impossible at block gas limit - try `UPPER_CEILING` instead.
 			// TODO: consider raising limit by powers of two.
-			const UPPER_CEILING: u64 = 1_000_000_000_000u64;
 			upper = UPPER_CEILING.into();
 			if !cond(upper) {
 				trace!(target: "estimate_gas", "estimate_gas failed with {}", upper);
@@ -1669,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/client/test_client.rs

@@ -26,6 +26,7 @@ use blockchain::TreeRoute;
 use client::{
 	BlockChainClient, MiningBlockChainClient, EngineClient, BlockChainInfo, BlockStatus, BlockId,
 	TransactionId, UncleId, TraceId, TraceFilter, LastHashes, CallAnalytics, BlockImportError,
+	ProvingBlockChainClient,
 };
 use db::{NUM_COLUMNS, COL_STATE};
 use header::{Header as BlockHeader, BlockNumber};
@@ -134,6 +135,9 @@ impl TestBlockChainClient {
 
 	/// Create test client with custom spec and extra data.
 	pub fn new_with_spec_and_extra(spec: Spec, extra_data: Bytes) -> Self {
+		let genesis_block = spec.genesis_block();
+		let genesis_hash = spec.genesis_header().hash();
+
 		let mut client = TestBlockChainClient {
 			blocks: RwLock::new(HashMap::new()),
 			numbers: RwLock::new(HashMap::new()),
@@ -158,8 +162,12 @@ impl TestBlockChainClient {
 			traces: RwLock::new(None),
 			history: RwLock::new(None),
 		};
-		client.add_blocks(1, EachBlockWith::Nothing); // add genesis block
-		client.genesis_hash = client.last_hash.read().clone();
+
+		// insert genesis hash.
+		client.blocks.get_mut().insert(genesis_hash, genesis_block);
+		client.numbers.get_mut().insert(0, genesis_hash);
+		*client.last_hash.get_mut() = genesis_hash;
+		client.genesis_hash = genesis_hash;
 		client
 	}
 
@@ -720,6 +728,20 @@ impl BlockChainClient for TestBlockChainClient {
 	fn registry_address(&self, _name: String) -> Option<Address> { None }
 }
 
+impl ProvingBlockChainClient for TestBlockChainClient {
+	fn prove_storage(&self, _: H256, _: H256, _: u32, _: BlockId) -> Vec<Bytes> {
+		Vec::new()
+	}
+
+	fn prove_account(&self, _: H256, _: u32, _: BlockId) -> Vec<Bytes> {
+		Vec::new()
+	}
+
+	fn code_by_hash(&self, _: H256, _: BlockId) -> Bytes {
+		Vec::new()
+	}
+}
+
 impl EngineClient for TestBlockChainClient {
 	fn update_sealing(&self) {
 		self.miner.update_sealing(self)

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

@@ -23,15 +23,35 @@ use header::Header;
 use rlp::*;
 use ethkey::{recover, public_to_address};
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+/// Message transmitted between consensus participants.
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub struct ConsensusMessage {
+	pub vote_step: VoteStep,
+	pub block_hash: Option<BlockHash>,
 	pub signature: H520,
+}
+
+/// Complete step of the consensus process.
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
+pub struct VoteStep {
 	pub height: Height,
 	pub round: Round,
 	pub step: Step,
-	pub block_hash: Option<BlockHash>,
 }
 
+impl VoteStep {
+	pub fn new(height: Height, round: Round, step: Step) -> Self {
+		VoteStep { height: height, round: round, step: step }
+	}
+
+	pub fn is_height(&self, height: Height) -> bool {
+		self.height == height
+	}
+
+	pub fn is_round(&self, height: Height, round: Round) -> bool {
+		self.height == height && self.round == round
+	}
+}
 
 fn consensus_round(header: &Header) -> Result<Round, ::rlp::DecoderError> {
 	let round_rlp = header.seal().get(0).expect("seal passed basic verification; seal has 3 fields; qed");
@@ -42,53 +62,29 @@ impl ConsensusMessage {
 	pub fn new(signature: H520, height: Height, round: Round, step: Step, block_hash: Option<BlockHash>) -> Self {
 		ConsensusMessage {
 			signature: signature,
-			height: height,
-			round: round,
-			step: step,
 			block_hash: block_hash,
+			vote_step: VoteStep::new(height, round, step),
 		}
 	}
 
 	pub fn new_proposal(header: &Header) -> Result<Self, ::rlp::DecoderError> {
 		Ok(ConsensusMessage {
+			vote_step: VoteStep::new(header.number() as Height, consensus_round(header)?, Step::Propose),
 			signature: UntrustedRlp::new(header.seal().get(1).expect("seal passed basic verification; seal has 3 fields; qed").as_slice()).as_val()?,
-			height: header.number() as Height,
-			round: consensus_round(header)?,
-			step: Step::Propose,
 			block_hash: Some(header.bare_hash()),
 		})
 	}
 
 	pub fn new_commit(proposal: &ConsensusMessage, signature: H520) -> Self {
+		let mut vote_step = proposal.vote_step.clone();
+		vote_step.step = Step::Precommit;
 		ConsensusMessage {
-			signature: signature,
-			height: proposal.height,
-			round: proposal.round,
-			step: Step::Precommit,
+			vote_step: vote_step,
 			block_hash: proposal.block_hash,
+			signature: signature,
 		}
 	}
 
-	pub fn is_height(&self, height: Height) -> bool {
-		self.height == height
-	}
-
-	pub fn is_round(&self, height: Height, round: Round) -> bool {
-		self.height == height && self.round == round
-	}
-
-	pub fn is_step(&self, height: Height, round: Round, step: Step) -> bool {
-		self.height == height && self.round == round && self.step == step
-	}
-
-	pub fn is_block_hash(&self, h: Height, r: Round, s: Step, block_hash: Option<BlockHash>) -> bool {
-		self.height == h && self.round == r && self.step == s && self.block_hash == block_hash
-	}
-
-	pub fn is_aligned(&self, m: &ConsensusMessage) -> bool {
-		self.is_block_hash(m.height, m.round, m.step, m.block_hash)
-	}
-
 	pub fn verify(&self) -> Result<Address, Error> {
 		let full_rlp = ::rlp::encode(self);
 		let block_info = Rlp::new(&full_rlp).at(1);
@@ -97,16 +93,30 @@ impl ConsensusMessage {
 	}
 
 	pub fn precommit_hash(&self) -> H256 {
-		message_info_rlp(self.height, self.round, Step::Precommit, self.block_hash).sha3()
+		let mut vote_step = self.vote_step.clone();
+		vote_step.step = Step::Precommit;
+		message_info_rlp(&vote_step, self.block_hash).sha3()
 	}
 }
 
-impl PartialOrd for ConsensusMessage {
-	fn partial_cmp(&self, m: &ConsensusMessage) -> Option<Ordering> {
+impl PartialOrd for VoteStep {
+	fn partial_cmp(&self, m: &VoteStep) -> Option<Ordering> {
 		Some(self.cmp(m))
 	}
 }
 
+impl Ord for VoteStep {
+	fn cmp(&self, m: &VoteStep) -> Ordering {
+		if self.height != m.height {
+			self.height.cmp(&m.height)
+		} else if self.round != m.round {
+			self.round.cmp(&m.round)
+		} else {
+			self.step.number().cmp(&m.step.number())
+		}
+	}
+}
+
 impl Step {
 	fn number(&self) -> u8 {
 		match *self {
@@ -118,20 +128,6 @@ impl Step {
 	}
 }
 
-impl Ord for ConsensusMessage {
-	fn cmp(&self, m: &ConsensusMessage) -> Ordering {
-		if self.height != m.height {
-			self.height.cmp(&m.height)
-		} else if self.round != m.round {
-			self.round.cmp(&m.round)
-		} else if self.step != m.step {
-			self.step.number().cmp(&m.step.number())
-		} else {
-			self.signature.cmp(&m.signature)
-		}
-	}
-}
-
 impl Decodable for Step {
 	fn decode<D>(decoder: &D) -> Result<Self, DecoderError> where D: Decoder {
 		match decoder.as_rlp().as_val()? {
@@ -149,42 +145,39 @@ impl Encodable for Step {
 	}
 }
 
-/// (signature, height, round, step, block_hash)
+/// (signature, (height, round, step, block_hash))
 impl Decodable for ConsensusMessage {
 	fn decode<D>(decoder: &D) -> Result<Self, DecoderError> where D: Decoder {
 		let rlp = decoder.as_rlp();
 		let m = rlp.at(1)?;
 		let block_message: H256 = m.val_at(3)?;
 		Ok(ConsensusMessage {
-			signature: rlp.val_at(0)?,
-			height: m.val_at(0)?,
-			round: m.val_at(1)?,
-			step: m.val_at(2)?,
+			vote_step: VoteStep::new(m.val_at(0)?, m.val_at(1)?, m.val_at(2)?),
 			block_hash: match block_message.is_zero() {
 				true => None,
 				false => Some(block_message),
-			}
+			},
+			signature: rlp.val_at(0)?,
 		})
   }
-} 
+}
 
 impl Encodable for ConsensusMessage {
 	fn rlp_append(&self, s: &mut RlpStream) {
-		let info = message_info_rlp(self.height, self.round, self.step, self.block_hash);
+		let info = message_info_rlp(&self.vote_step, self.block_hash);
 		s.begin_list(2)
 			.append(&self.signature)
 			.append_raw(&info, 1);
 	}
 }
 
-pub fn message_info_rlp(height: Height, round: Round, step: Step, block_hash: Option<BlockHash>) -> Bytes {
+pub fn message_info_rlp(vote_step: &VoteStep, block_hash: Option<BlockHash>) -> Bytes {
 	// TODO: figure out whats wrong with nested list encoding
 	let mut s = RlpStream::new_list(5);
-	s.append(&height).append(&round).append(&step).append(&block_hash.unwrap_or_else(H256::zero));
+	s.append(&vote_step.height).append(&vote_step.round).append(&vote_step.step).append(&block_hash.unwrap_or_else(H256::zero));
 	s.out()
 }
 
-
 pub fn message_full_rlp(signature: &H520, vote_info: &Bytes) -> Bytes {
 	let mut s = RlpStream::new_list(2);
 	s.append(signature).append_raw(vote_info, 1);
@@ -199,14 +192,17 @@ mod tests {
 	use super::*;
 	use account_provider::AccountProvider;
 	use header::Header;
+	use ethkey::Secret;
 
 	#[test]
 	fn encode_decode() {
 		let message = ConsensusMessage {
 			signature: H520::default(),	
-			height: 10,
-			round: 123,
-			step: Step::Precommit,
+			vote_step: VoteStep {
+				height: 10,
+				round: 123,
+				step: Step::Precommit,
+			},
 			block_hash: Some("1".sha3())
 		};
 		let raw_rlp = ::rlp::encode(&message).to_vec();
@@ -215,9 +211,11 @@ mod tests {
 
 		let message = ConsensusMessage {
 			signature: H520::default(),	
-			height: 1314,
-			round: 0,
-			step: Step::Prevote,
+			vote_step: VoteStep {
+				height: 1314,
+				round: 0,
+				step: Step::Prevote,
+			},
 			block_hash: None
 		};
 		let raw_rlp = ::rlp::encode(&message);
@@ -228,10 +226,10 @@ 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()));
+		let mi = message_info_rlp(&VoteStep::new(123, 2, Step::Precommit), Some(H256::default()));
 
 		let raw_rlp = message_full_rlp(&tap.sign(addr, None, mi.sha3()).unwrap().into(), &mi);
 
@@ -254,9 +252,11 @@ mod tests {
 			message,
 			ConsensusMessage {
 				signature: Default::default(),
-				height: 0,
-				round: 0,
-				step: Step::Propose,
+				vote_step: VoteStep {
+					height: 0,
+					round: 0,
+					step: Step::Propose,
+				},
 				block_hash: Some(header.bare_hash())
 			}
 		);
@@ -267,12 +267,10 @@ mod tests {
 		let header = Header::default();
 		let pro = ConsensusMessage {
 			signature: Default::default(),
-			height: 0,
-			round: 0,
-			step: Step::Propose,
+			vote_step: VoteStep::new(0, 0, Step::Propose),
 			block_hash: Some(header.bare_hash())
 		};
-		let pre = message_info_rlp(0, 0, Step::Precommit, Some(header.bare_hash()));
+		let pre = message_info_rlp(&VoteStep::new(0, 0, Step::Precommit), Some(header.bare_hash()));
 
 		assert_eq!(pro.precommit_hash(), pre.sha3());
 	}

ethcore/src/engines/tendermint/mod.rs

@@ -53,7 +53,7 @@ use self::transition::TransitionHandler;
 use self::params::TendermintParams;
 use self::vote_collector::VoteCollector;
 
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 pub enum Step {
 	Propose,
 	Prevote,
@@ -163,13 +163,13 @@ impl Tendermint {
 			let h = self.height.load(AtomicOrdering::SeqCst);
 			let r = self.round.load(AtomicOrdering::SeqCst);
 			let s = self.step.read();
-			let vote_info = message_info_rlp(h, r, *s, block_hash);
+			let vote_info = message_info_rlp(&VoteStep::new(h, r, *s), block_hash);
 			let authority = self.authority.read();
 			match ap.sign(*authority, self.password.read().clone(), vote_info.sha3()).map(Into::into) {
 				Ok(signature) => {
 					let message_rlp = message_full_rlp(&signature, &vote_info);
 					let message = ConsensusMessage::new(signature, h, r, *s, block_hash);
-					self.votes.vote(message.clone(), *authority);
+					self.votes.vote(message.clone(), &*authority);
 					debug!(target: "poa", "Generated {:?} as {}.", message, *authority);
 					self.handle_valid_message(&message);
 
@@ -221,7 +221,7 @@ impl Tendermint {
 			},
 			Step::Prevote => {
 				let block_hash = match *self.lock_change.read() {
-					Some(ref m) if !self.should_unlock(m.round) => m.block_hash,
+					Some(ref m) if !self.should_unlock(m.vote_step.round) => m.block_hash,
 					_ => self.proposal.read().clone(),
 				};
 				self.generate_and_broadcast_message(block_hash);
@@ -230,8 +230,8 @@ impl Tendermint {
 				trace!(target: "poa", "to_step: Precommit.");
 				let block_hash = match *self.lock_change.read() {
 					Some(ref m) if self.is_round(m) && m.block_hash.is_some() => {
-						trace!(target: "poa", "Setting last lock: {}", m.round);
-						self.last_lock.store(m.round, AtomicOrdering::SeqCst);
+						trace!(target: "poa", "Setting last lock: {}", m.vote_step.round);
+						self.last_lock.store(m.vote_step.round, AtomicOrdering::SeqCst);
 						m.block_hash
 					},
 					_ => None,
@@ -246,7 +246,7 @@ impl Tendermint {
 				if let Some(block_hash) = *self.proposal.read() {
 					// Generate seal and remove old votes.
 					if self.is_proposer(&*self.authority.read()).is_ok() {
-						if let Some(seal) = self.votes.seal_signatures(height, round, block_hash) {
+						if let Some(seal) = self.votes.seal_signatures(height, round, &block_hash) {
 							trace!(target: "poa", "Collected seal: {:?}", seal);
 							let seal = vec![
 								::rlp::encode(&round).to_vec(),
@@ -290,11 +290,11 @@ impl Tendermint {
 	}
 
 	fn is_height(&self, message: &ConsensusMessage) -> bool {
-		message.is_height(self.height.load(AtomicOrdering::SeqCst)) 
+		message.vote_step.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.vote_step.is_round(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst)) 
 	}
 
 	fn increment_round(&self, n: Round) {
@@ -302,20 +302,20 @@ impl Tendermint {
 		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)
 	}
 
 
 	fn has_enough_any_votes(&self) -> bool {
-		let step_votes = self.votes.count_step_votes(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst), *self.step.read());
+		let step_votes = self.votes.count_step_votes(&VoteStep::new(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst), *self.step.read()));
 		self.is_above_threshold(step_votes)
 	}
 
-	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);
+	fn has_enough_future_step_votes(&self, vote_step: &VoteStep) -> bool {
+		if vote_step.round > self.round.load(AtomicOrdering::SeqCst) {
+			let step_votes = self.votes.count_step_votes(vote_step);
 			self.is_above_threshold(step_votes)	
 		} else {
 			false
@@ -328,12 +328,13 @@ impl Tendermint {
 	}
 
 	fn handle_valid_message(&self, message: &ConsensusMessage) {
+		let ref vote_step = message.vote_step;
 		let is_newer_than_lock = match *self.lock_change.read() {
-			Some(ref lock) => message > lock,
+			Some(ref lock) => vote_step > &lock.vote_step,
 			None => true,
 		};
 		let lock_change = is_newer_than_lock
-			&& message.step == Step::Prevote
+			&& vote_step.step == Step::Prevote
 			&& message.block_hash.is_some()
 			&& self.has_enough_aligned_votes(message);
 		if lock_change {
@@ -351,15 +352,15 @@ impl Tendermint {
 						Some(Step::Commit)
 					}
 				},
-				Step::Precommit if self.has_enough_future_step_votes(message) => {
-					self.increment_round(message.round - self.round.load(AtomicOrdering::SeqCst));
+				Step::Precommit if self.has_enough_future_step_votes(&vote_step) => {
+					self.increment_round(vote_step.round - self.round.load(AtomicOrdering::SeqCst));
 					Some(Step::Precommit)
 				},
 				// Avoid counting twice.
 				Step::Prevote if lock_change => Some(Step::Precommit),
 				Step::Prevote if self.has_enough_aligned_votes(message) => Some(Step::Precommit),
-				Step::Prevote if self.has_enough_future_step_votes(message) => {
-					self.increment_round(message.round - self.round.load(AtomicOrdering::SeqCst));
+				Step::Prevote if self.has_enough_future_step_votes(&vote_step) => {
+					self.increment_round(vote_step.round - self.round.load(AtomicOrdering::SeqCst));
 					Some(Step::Prevote)
 				},
 				_ => None,
@@ -390,8 +391,8 @@ impl Engine for Tendermint {
 		let message = ConsensusMessage::new_proposal(header).expect("Invalid header.");
 		map![
 			"signature".into() => message.signature.to_string(),
-			"height".into() => message.height.to_string(),
-			"round".into() => message.round.to_string(),
+			"height".into() => message.vote_step.height.to_string(),
+			"round".into() => message.vote_step.round.to_string(),
 			"block_hash".into() => message.block_hash.as_ref().map(ToString::to_string).unwrap_or("".into())
 		]
 	}
@@ -431,11 +432,11 @@ impl Engine for Tendermint {
 			let height = header.number() as Height;
 			let round = self.round.load(AtomicOrdering::SeqCst);
 			let bh = Some(header.bare_hash());
-			let vote_info = message_info_rlp(height, round, Step::Propose, bh.clone());
+			let vote_info = message_info_rlp(&VoteStep::new(height, round, Step::Propose), bh.clone());
 			if let Ok(signature) = ap.sign(*author, self.password.read().clone(), vote_info.sha3()).map(H520::from) {
 				// Insert Propose vote.
 				debug!(target: "poa", "Submitting proposal {} at height {} round {}.", header.bare_hash(), height, round);
-				self.votes.vote(ConsensusMessage::new(signature, height, round, Step::Propose, bh), *author);
+				self.votes.vote(ConsensusMessage::new(signature, height, round, Step::Propose, bh), author);
 				// Remember proposal for later seal submission.
 				*self.proposal.write() = bh;
 				Seal::Proposal(vec![
@@ -461,9 +462,11 @@ impl Engine for Tendermint {
 			if !self.is_authority(&sender) {
 				Err(EngineError::NotAuthorized(sender))?;
 			}
-			self.broadcast_message(rlp.as_raw().to_vec());
+			if self.votes.vote(message.clone(), &sender).is_some() {
+				Err(EngineError::DoubleVote(sender))?
+			}
 			trace!(target: "poa", "Handling a valid {:?} from {}.", message, sender);
-			self.votes.vote(message.clone(), sender);
+			self.broadcast_message(rlp.as_raw().to_vec());
 			self.handle_valid_message(&message);
 		}
 		Ok(())
@@ -502,7 +505,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))?
@@ -541,7 +544,7 @@ impl Engine for Tendermint {
 					found: signatures_len
 				}))?;
 			}
-			self.is_round_proposer(proposal.height, proposal.round, &proposer)?;
+			self.is_round_proposer(proposal.vote_step.height, proposal.vote_step.round, &proposer)?;
 		}
 		Ok(())
 	}
@@ -607,16 +610,16 @@ impl Engine for Tendermint {
 		let proposal = ConsensusMessage::new_proposal(header).expect("block went through full verification; this Engine verifies new_proposal creation; qed");
 		if signatures_len != 1 {
 			// New Commit received, skip to next height.
-			trace!(target: "poa", "Received a commit for height {}, round {}.", proposal.height, proposal.round);
-			self.to_next_height(proposal.height);
+			trace!(target: "poa", "Received a commit: {:?}.", proposal.vote_step);
+			self.to_next_height(proposal.vote_step.height);
 			return false;
 		}
 		let proposer = proposal.verify().expect("block went through full verification; this Engine tries verify; qed");
-		debug!(target: "poa", "Received a new proposal for height {}, round {} from {}.", proposal.height, proposal.round, proposer);
+		debug!(target: "poa", "Received a new proposal {:?} from {}.", proposal.vote_step, proposer);
 		if self.is_round(&proposal) {
 			*self.proposal.write() = proposal.block_hash.clone();
 		}
-		self.votes.vote(proposal, proposer);
+		self.votes.vote(proposal, &proposer);
 		true
 	}
 
@@ -671,6 +674,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::*;
@@ -703,7 +707,7 @@ mod tests {
 	}
 
 	fn vote<F>(engine: &Engine, signer: F, height: usize, round: usize, step: Step, block_hash: Option<H256>) -> Bytes where F: FnOnce(H256) -> Result<H520, ::account_provider::Error> {
-		let mi = message_info_rlp(height, round, step, block_hash);
+		let mi = message_info_rlp(&VoteStep::new(height, round, step), block_hash);
 		let m = message_full_rlp(&signer(mi.sha3()).unwrap().into(), &mi);
 		engine.handle_message(&m).unwrap();
 		m
@@ -711,7 +715,7 @@ mod tests {
 
 	fn proposal_seal(tap: &Arc<AccountProvider>, header: &Header, round: Round) -> Vec<Bytes> {
 		let author = header.author();
-		let vote_info = message_info_rlp(header.number() as Height, round, Step::Propose, Some(header.bare_hash()));
+		let vote_info = message_info_rlp(&VoteStep::new(header.number() as Height, round, Step::Propose), Some(header.bare_hash()));
 		let signature = tap.sign(*author, None, vote_info.sha3()).unwrap();
 		vec![
 			::rlp::encode(&round).to_vec(),
@@ -721,7 +725,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
 	}
@@ -825,7 +829,7 @@ mod tests {
 		header.set_author(proposer);
 		let mut seal = proposal_seal(&tap, &header, 0);
 
-		let vote_info = message_info_rlp(0, 0, Step::Precommit, Some(header.bare_hash()));
+		let vote_info = message_info_rlp(&VoteStep::new(0, 0, Step::Precommit), Some(header.bare_hash()));
 		let signature1 = tap.sign(proposer, None, vote_info.sha3()).unwrap();
 
 		seal[2] = ::rlp::encode(&vec![H520::from(signature1.clone())]).to_vec();
@@ -886,7 +890,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/tendermint/vote_collector.rs

@@ -17,13 +17,50 @@
 //! Collects votes on hashes at each height and round.
 
 use util::*;
-use super::message::ConsensusMessage;
-use super::{Height, Round, Step};
+use super::message::*;
+use super::{Height, Round, Step, BlockHash};
 
 #[derive(Debug)]
 pub struct VoteCollector {
 	/// Storing all Proposals, Prevotes and Precommits.
-	votes: RwLock<BTreeMap<ConsensusMessage, Address>>,
+	votes: RwLock<BTreeMap<VoteStep, StepCollector>>,
+}
+
+#[derive(Debug, Default)]
+struct StepCollector {
+	voted: HashSet<Address>,
+	pub block_votes: HashMap<Option<BlockHash>, HashMap<H520, Address>>,
+	messages: HashSet<ConsensusMessage>,
+}
+
+impl StepCollector {
+	/// Returns Some(&Address) when validator is double voting.
+	fn insert<'a>(&mut self, message: ConsensusMessage, address: &'a Address) -> Option<&'a Address> {
+		// Do nothing when message was seen.
+		if self.messages.insert(message.clone()) {
+			if self.voted.insert(address.clone()) {
+				self
+					.block_votes
+					.entry(message.block_hash)
+					.or_insert_with(HashMap::new)
+					.insert(message.signature, address.clone());
+			} else {
+				// Bad validator sent a different message.
+				return Some(address);
+			}
+		}
+		None
+	}
+
+	/// Count all votes for the given block hash at this step.
+	fn count_block(&self, block_hash: &Option<BlockHash>) -> usize {
+		self.block_votes.get(block_hash).map_or(0, HashMap::len)
+	}
+
+	/// Count all votes collected for the given step.
+	fn count(&self) -> usize {
+		self.block_votes.values().map(HashMap::len).sum()
+	}
 }
 
 #[derive(Debug)]
@@ -42,109 +79,105 @@ impl PartialEq for SealSignatures {
 impl Eq for SealSignatures {}
 
 impl VoteCollector {
-	pub fn new() -> VoteCollector {
+	pub fn new() -> Self {
 		let mut collector = BTreeMap::new();
-		// Insert dummy message to fulfill invariant: "only messages newer than the oldest are inserted".
-		collector.insert(ConsensusMessage {
-			signature: H520::default(),
-			height: 0,
-			round: 0,
-			step: Step::Propose,
-			block_hash: None
-		},
-		Address::default());
+		// Insert dummy entry to fulfill invariant: "only messages newer than the oldest are inserted".
+		collector.insert(VoteStep::new(0, 0, Step::Propose), Default::default());
 		VoteCollector { votes: RwLock::new(collector) }
 	}
 
 	/// Insert vote if it is newer than the oldest one.
-	pub fn vote(&self, message: ConsensusMessage, voter: Address) -> Option<Address> {
-		self.votes.write().insert(message, voter)
+	pub fn vote<'a>(&self, message: ConsensusMessage, voter: &'a Address) -> Option<&'a Address> {
+		self
+			.votes
+			.write()
+			.entry(message.vote_step.clone())
+			.or_insert_with(Default::default)
+			.insert(message, voter)
 	}
 
+	/// Checks if the message should be ignored.
 	pub fn is_old_or_known(&self, message: &ConsensusMessage) -> bool {
-		self.votes.read().get(message).map_or(false, |a| {
-			trace!(target: "poa", "Known message from {}: {:?}.", a, message);
-			true
-		}) || {
+		self
+			.votes
+			.read()
+			.get(&message.vote_step)
+			.map_or(false, |c| {
+				let is_known = c.messages.contains(message);
+				if is_known { trace!(target: "poa", "Known message: {:?}.", message); }
+				is_known
+			})
+		|| {
 			let guard = self.votes.read();
-			let is_old = guard.keys().next().map_or(true, |oldest| message <= oldest);
+			let is_old = guard.keys().next().map_or(true, |oldest| message.vote_step <= *oldest);
 			if is_old { trace!(target: "poa", "Old message {:?}.", message); }
 			is_old
 		}
 	}
 
 	/// Throws out messages older than message, leaves message as marker for the oldest.
-	pub fn throw_out_old(&self, message: &ConsensusMessage) {
+	pub fn throw_out_old(&self, vote_step: &VoteStep) {
 		let mut guard = self.votes.write();
-		let new_collector = guard.split_off(message);
+		let new_collector = guard.split_off(vote_step);
 		*guard = new_collector;
 	}
 
-	pub fn seal_signatures(&self, height: Height, round: Round, block_hash: H256) -> Option<SealSignatures> {
-		let bh = Some(block_hash);
-		let (proposal, votes) = {
+	/// Collects the signatures used to seal a block.
+	pub fn seal_signatures(&self, height: Height, round: Round, block_hash: &H256) -> Option<SealSignatures> {
+		let ref bh = Some(*block_hash);
+		let precommit_step = VoteStep::new(height, round, Step::Precommit);
+		let maybe_seal = {
 			let guard = self.votes.read();
-			let mut current_signatures = guard.keys().skip_while(|m| !m.is_block_hash(height, round, Step::Propose, bh));
-			let proposal = current_signatures.next().cloned();
-			let votes = current_signatures
-					.skip_while(|m| !m.is_block_hash(height, round, Step::Precommit, bh))
-					.filter(|m| m.is_block_hash(height, round, Step::Precommit, bh))
-					.cloned()
-					.collect::<Vec<_>>();
-			(proposal, votes)
+			guard
+				.get(&VoteStep::new(height, round, Step::Propose))
+				.and_then(|c| c.block_votes.get(bh))
+				.and_then(|proposals| proposals.keys().next())
+				.map(|proposal| SealSignatures {
+					proposal: proposal.clone(),
+					votes: guard
+						.get(&precommit_step)
+						.and_then(|c| c.block_votes.get(bh))
+						.map(|precommits| precommits.keys().cloned().collect())
+						.unwrap_or_else(Vec::new),
+				})
+				.and_then(|seal| if seal.votes.is_empty() { None } else { Some(seal) })
 		};
-		if votes.is_empty() {
-			return None;
+		if maybe_seal.is_some() {
+				// Remove messages that are no longer relevant.
+				self.throw_out_old(&precommit_step);
 		}
-		// Remove messages that are no longer relevant.
-		votes.last().map(|m| self.throw_out_old(m));
-		let mut votes_vec: Vec<_> = votes.into_iter().map(|m| m.signature).collect();
-		votes_vec.sort();
-		proposal.map(|p| SealSignatures {
-			proposal: p.signature,
-			votes: votes_vec,
-		})
+		maybe_seal
 	}
 
+	/// Count votes which agree with the given message.
 	pub fn count_aligned_votes(&self, message: &ConsensusMessage) -> usize {
-		let guard = self.votes.read();
-		guard.keys()
-			.skip_while(|m| !m.is_aligned(message))
-			// sorted by signature so might not be continuous
-			.filter(|m| m.is_aligned(message))
-			.count()
+		self
+			.votes
+			.read()
+			.get(&message.vote_step)
+			.map_or(0, |m| m.count_block(&message.block_hash))
 	}
 
-	pub fn count_step_votes(&self, height: Height, round: Round, step: Step) -> usize {
-		let guard = self.votes.read();
-		let current = guard.iter().skip_while(|&(m, _)| !m.is_step(height, round, step));
-		let mut origins = HashSet::new();
-		let mut n = 0;
-		for (message, origin) in current {
-			if message.is_step(height, round, step) {
-				if origins.insert(origin) {
-					n += 1;
-				} else {
-					warn!("count_step_votes: Authority {} has cast multiple step votes, this indicates malicious behaviour.", origin)
-				}
-			}
-		}
-		n
+	/// Count all votes collected for a given step.
+	pub fn count_step_votes(&self, vote_step: &VoteStep) -> usize {
+		self.votes.read().get(vote_step).map_or(0, StepCollector::count)
 	}
 
+	/// Get all messages older than the height.
 	pub fn get_up_to(&self, height: Height) -> Vec<Bytes> {
 		let guard = self.votes.read();
 		guard
-			.keys()
-			.filter(|m| m.step.is_pre())
-			.take_while(|m| m.height <= height)
-			.map(|m| ::rlp::encode(m).to_vec())
-			.collect()
+			.iter()
+			.filter(|&(s, _)| s.step.is_pre())
+			.take_while(|&(s, _)| s.height <= height)
+			.map(|(_, c)| c.messages.iter().map(|m| ::rlp::encode(m).to_vec()).collect::<Vec<_>>())
+			.fold(Vec::new(), |mut acc, mut messages| { acc.append(&mut messages); acc })
 	}
 
+	/// Retrieve address from which the message was sent from cache.
 	pub fn get(&self, message: &ConsensusMessage) -> Option<Address> {
 		let guard = self.votes.read();
-		guard.get(message).cloned()
+		guard.get(&message.vote_step).and_then(|c| c.block_votes.get(&message.block_hash)).and_then(|origins| origins.get(&message.signature).cloned())
 	}
 }
 
@@ -152,15 +185,15 @@ impl VoteCollector {
 mod tests {
 	use util::*;
 	use super::*;
-	use super::super::{Height, Round, BlockHash, Step};
-	use super::super::message::ConsensusMessage;
+	use super::super::{BlockHash, Step};
+	use super::super::message::*;
 
-	fn random_vote(collector: &VoteCollector, signature: H520, h: Height, r: Round, step: Step, block_hash: Option<BlockHash>) -> Option<H160> {
-		full_vote(collector, signature, h, r, step, block_hash, H160::random())
+	fn random_vote(collector: &VoteCollector, signature: H520, vote_step: VoteStep, block_hash: Option<BlockHash>) -> bool {
+		full_vote(collector, signature, vote_step, block_hash, &H160::random()).is_none()
 	}
 
-	fn full_vote(collector: &VoteCollector, signature: H520, h: Height, r: Round, step: Step, block_hash: Option<BlockHash>, address: Address) -> Option<H160> {
-		collector.vote(ConsensusMessage { signature: signature, height: h, round: r, step: step, block_hash: block_hash }, address)
+	fn full_vote<'a>(collector: &VoteCollector, signature: H520, vote_step: VoteStep, block_hash: Option<BlockHash>, address: &'a Address) -> Option<&'a Address> {
+		collector.vote(ConsensusMessage { signature: signature, vote_step: vote_step, block_hash: block_hash }, address)
 	}
 
 	#[test]
@@ -173,68 +206,71 @@ mod tests {
 		for _ in 0..5 {
 			signatures.push(H520::random());
 		}
+		let propose_step = VoteStep::new(h, r, Step::Propose);
+		let prevote_step = VoteStep::new(h, r, Step::Prevote);
+		let precommit_step = VoteStep::new(h, r, Step::Precommit);
 		// Wrong height proposal.
-		random_vote(&collector, signatures[4].clone(), h - 1, r, Step::Propose, bh.clone());
+		random_vote(&collector, signatures[4].clone(), VoteStep::new(h - 1, r, Step::Propose), bh.clone());
 		// Good proposal
-		random_vote(&collector, signatures[0].clone(), h, r, Step::Propose, bh.clone());
+		random_vote(&collector, signatures[0].clone(), propose_step.clone(), bh.clone());
 		// Wrong block proposal.
-		random_vote(&collector, signatures[0].clone(), h, r, Step::Propose, Some("0".sha3()));
+		random_vote(&collector, signatures[0].clone(), propose_step.clone(), Some("0".sha3()));
 		// Wrong block precommit.
-		random_vote(&collector, signatures[3].clone(), h, r, Step::Precommit, Some("0".sha3()));
+		random_vote(&collector, signatures[3].clone(), precommit_step.clone(), Some("0".sha3()));
 		// Wrong round proposal.
-		random_vote(&collector, signatures[0].clone(), h, r - 1, Step::Propose, bh.clone());
+		random_vote(&collector, signatures[0].clone(), VoteStep::new(h, r - 1, Step::Propose), bh.clone());
 		// Prevote.
-		random_vote(&collector, signatures[0].clone(), h, r, Step::Prevote, bh.clone());
+		random_vote(&collector, signatures[0].clone(), prevote_step.clone(), bh.clone());
 		// Relevant precommit.
-		random_vote(&collector, signatures[2].clone(), h, r, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[2].clone(), precommit_step.clone(), bh.clone());
 		// Replcated vote.
-		random_vote(&collector, signatures[2].clone(), h, r, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[2].clone(), precommit_step.clone(), bh.clone());
 		// Wrong round precommit.
-		random_vote(&collector, signatures[4].clone(), h, r + 1, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[4].clone(), VoteStep::new(h, r + 1, Step::Precommit), bh.clone());
 		// Wrong height precommit.
-		random_vote(&collector, signatures[3].clone(), h + 1, r, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[3].clone(), VoteStep::new(h + 1, r, Step::Precommit), bh.clone());
 		// Relevant precommit.
-		random_vote(&collector, signatures[1].clone(), h, r, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[1].clone(), precommit_step.clone(), bh.clone());
 		// Wrong round precommit, same signature.
-		random_vote(&collector, signatures[1].clone(), h, r + 1, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[1].clone(), VoteStep::new(h, r + 1, Step::Precommit), bh.clone());
 		// Wrong round precommit.
-		random_vote(&collector, signatures[4].clone(), h, r - 1, Step::Precommit, bh.clone());
+		random_vote(&collector, signatures[4].clone(), VoteStep::new(h, r - 1, Step::Precommit), bh.clone());
 		let seal = SealSignatures {
 			proposal: signatures[0],
 			votes: signatures[1..3].to_vec()
 		};
-		assert_eq!(seal, collector.seal_signatures(h, r, bh.unwrap()).unwrap());
+		assert_eq!(seal, collector.seal_signatures(h, r, &bh.unwrap()).unwrap());
 	}
 
 	#[test]
 	fn count_votes() {
 		let collector = VoteCollector::new();	
+		let prevote_step = VoteStep::new(3, 2, Step::Prevote);
+		let precommit_step = VoteStep::new(3, 2, Step::Precommit);
 		// good prevote
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 3, 1, Step::Prevote, Some("0".sha3()));
+		random_vote(&collector, H520::random(), prevote_step.clone(), Some("0".sha3()));
+		random_vote(&collector, H520::random(), VoteStep::new(3, 1, Step::Prevote), Some("0".sha3()));
 		// good precommit
-		random_vote(&collector, H520::random(), 3, 2, Step::Precommit, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 3, 3, Step::Precommit, Some("0".sha3()));
+		random_vote(&collector, H520::random(), precommit_step.clone(), Some("0".sha3()));
+		random_vote(&collector, H520::random(), VoteStep::new(3, 3, Step::Precommit), Some("0".sha3()));
 		// good prevote
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("1".sha3()));
+		random_vote(&collector, H520::random(), prevote_step.clone(), Some("1".sha3()));
 		// good prevote
 		let same_sig = H520::random();
-		random_vote(&collector, same_sig.clone(), 3, 2, Step::Prevote, Some("1".sha3()));
-		random_vote(&collector, same_sig, 3, 2, Step::Prevote, Some("1".sha3()));
+		random_vote(&collector, same_sig.clone(), prevote_step.clone(), Some("1".sha3()));
+		random_vote(&collector, same_sig, prevote_step.clone(), Some("1".sha3()));
 		// good precommit
-		random_vote(&collector, H520::random(), 3, 2, Step::Precommit, Some("1".sha3()));
+		random_vote(&collector, H520::random(), precommit_step.clone(), Some("1".sha3()));
 		// good prevote
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 2, 2, Step::Precommit, Some("2".sha3()));
+		random_vote(&collector, H520::random(), prevote_step.clone(), Some("0".sha3()));
+		random_vote(&collector, H520::random(), VoteStep::new(2, 2, Step::Precommit), Some("2".sha3()));
 
-		assert_eq!(collector.count_step_votes(3, 2, Step::Prevote), 4);
-		assert_eq!(collector.count_step_votes(3, 2, Step::Precommit), 2);
+		assert_eq!(collector.count_step_votes(&prevote_step), 4);
+		assert_eq!(collector.count_step_votes(&precommit_step), 2);
 
 		let message = ConsensusMessage {
 			signature: H520::default(),
-			height: 3,
-			round: 2,
-			step: Step::Prevote,
+			vote_step: prevote_step,
 			block_hash: Some("1".sha3())
 		};
 		assert_eq!(collector.count_aligned_votes(&message), 2);
@@ -243,30 +279,29 @@ mod tests {
 	#[test]
 	fn remove_old() {
 		let collector = VoteCollector::new();	
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 3, 1, Step::Prevote, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 3, 3, Step::Precommit, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("1".sha3()));
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("1".sha3()));
-		random_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("0".sha3()));
-		random_vote(&collector, H520::random(), 2, 2, Step::Precommit, Some("2".sha3()));
-
-		let message = ConsensusMessage {
-			signature: H520::default(),
-			height: 3,
-			round: 2,
-			step: Step::Precommit,
-			block_hash: Some("1".sha3())
+		let vote = |height, round, step, hash| {
+			random_vote(&collector, H520::random(), VoteStep::new(height, round, step), hash);
 		};
-		collector.throw_out_old(&message);
+		vote(3, 2, Step::Prevote, Some("0".sha3()));
+		vote(3, 1, Step::Prevote, Some("0".sha3()));
+		vote(3, 3, Step::Precommit, Some("0".sha3()));
+		vote(3, 2, Step::Prevote, Some("1".sha3()));
+		vote(3, 2, Step::Prevote, Some("1".sha3()));
+		vote(3, 2, Step::Prevote, Some("0".sha3()));
+		vote(2, 2, Step::Precommit, Some("2".sha3()));
+
+		collector.throw_out_old(&VoteStep::new(3, 2, Step::Precommit));
 		assert_eq!(collector.votes.read().len(), 1);
 	}
 
 	#[test]
 	fn malicious_authority() {
 		let collector = VoteCollector::new();	
-		full_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("0".sha3()), Address::default());
-		full_vote(&collector, H520::random(), 3, 2, Step::Prevote, Some("1".sha3()), Address::default());
-		assert_eq!(collector.count_step_votes(3, 2, Step::Prevote), 1);
+		let vote_step = VoteStep::new(3, 2, Step::Prevote);
+		// Vote is inserted fine.
+		assert!(full_vote(&collector, H520::random(), vote_step.clone(), Some("0".sha3()), &Address::default()).is_none());
+		// Returns the double voting address.
+		full_vote(&collector, H520::random(), vote_step.clone(), Some("1".sha3()), &Address::default()).unwrap();
+		assert_eq!(collector.count_step_votes(&vote_step), 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.

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 = unsafe { ::std::mem::transmute(secret) };
-		let shared = ecdh::SharedSecret::new_raw(context, &publ, sec);
+		let sec = key::SecretKey::from_slice(context, &secret)?;
+		let shared = ecdh::SharedSecret::new_raw(context, &publ, &sec);
 
-		let mut s = Secret::default();
-		s.copy_from_slice(&shared[0..32]);
-		Ok(s)
+		Secret::from_slice(&shared[0..32])
+			.map_err(|_| Error::Secp(SecpError::InvalidSecretKey))
 	}
 }
 

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);
@@ -34,13 +34,15 @@ impl Generator for Brain {
 		let mut i = 0;
 		loop {
 			secret = secret.keccak256();
-			
+
 			match i > 16384 {
 				false => i += 1,
 				true => {
-					let result = KeyPair::from_secret(secret.clone().into());
-					if result.as_ref().ok().map_or(false, |r| r.address()[0] == 0) {
-						return result;
+					if let Ok(secret) = Secret::from_slice(&secret) {
+						let result = KeyPair::from_secret(secret);
+						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();
-		secret.copy_from_slice(&sec[0..32]);
+		let secret = Secret::from(sec);
 		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 = unsafe { mem::transmute(secret) };
-	let s = context.sign_recoverable(&SecpMessage::from_slice(&message[..])?, sec)?;
+	let sec = SecretKey::from_slice(context, &secret)?;
+	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();

js/package.json

@@ -1,6 +1,6 @@
 {
   "name": "parity.js",
-  "version": "0.2.182",
+  "version": "0.2.184",
   "main": "release/index.js",
   "jsnext:main": "src/index.js",
   "author": "Parity Team <admin@parity.io>",

js/src/abi/spec/event/event.js

@@ -23,12 +23,12 @@ import { eventSignature } from '../../util/signature';
 
 export default class Event {
   constructor (abi) {
-    this._name = abi.name;
     this._inputs = EventParam.toEventParams(abi.inputs || []);
     this._anonymous = !!abi.anonymous;
 
-    const { id, signature } = eventSignature(this._name, this.inputParamTypes());
+    const { id, name, signature } = eventSignature(abi.name, this.inputParamTypes());
     this._id = id;
+    this._name = name;
     this._signature = signature;
   }
 

js/src/abi/spec/function.js

@@ -22,14 +22,14 @@ import { methodSignature } from '../util/signature';
 export default class Func {
   constructor (abi) {
     this._abi = abi;
-    this._name = abi.name;
     this._constant = !!abi.constant;
     this._payable = abi.payable;
     this._inputs = Param.toParams(abi.inputs || []);
     this._outputs = Param.toParams(abi.outputs || []);
 
-    const { id, signature } = methodSignature(this._name, this.inputParamTypes());
+    const { id, name, signature } = methodSignature(abi.name, this.inputParamTypes());
     this._id = id;
+    this._name = name;
     this._signature = signature;
   }
 

js/src/abi/spec/function.spec.js

@@ -35,6 +35,18 @@ describe('abi/spec/Function', () => {
   });
 
   describe('constructor', () => {
+    it('returns signature correctly if name already contains it', () => {
+      const func = new Func({
+        name: 'test(bool,string)',
+        inputs: inputsArr,
+        outputs: outputsArr
+      });
+
+      expect(func.name).to.equal('test');
+      expect(func.id).to.equal('test(bool,string)');
+      expect(func.signature).to.equal('02356205');
+    });
+
     it('stores the parameters as received', () => {
       expect(func.name).to.equal('test');
       expect(func.constant).to.be.false;

js/src/abi/util/signature.js

@@ -17,15 +17,31 @@
 import { keccak_256 } from 'js-sha3'; // eslint-disable-line camelcase
 import { fromParamType } from '../spec/paramType/format';
 
-export function eventSignature (name, params) {
+export function eventSignature (eventName, params) {
+  const { strName, name } = parseName(eventName);
   const types = (params || []).map(fromParamType).join(',');
-  const id = `${name || ''}(${types})`;
+  const id = `${strName}(${types})`;
 
-  return { id, signature: keccak_256(id) };
+  return { id, name, signature: keccak_256(id) };
 }
 
-export function methodSignature (name, params) {
-  const { id, signature } = eventSignature(name, params);
+export function methodSignature (methodName, params) {
+  const { id, name, signature } = eventSignature(methodName, params);
 
-  return { id, signature: signature.substr(0, 8) };
+  return { id, name, signature: signature.substr(0, 8) };
+}
+
+function parseName (name) {
+  const strName = `${name || ''}`;
+  const idx = strName.indexOf('(');
+
+  if (idx === -1) {
+    return { strName, name };
+  }
+
+  const trimmedName = strName.slice(0, idx);
+  return {
+    strName: trimmedName,
+    name: trimmedName
+  };
 }

js/src/abi/util/signature.spec.js

@@ -19,50 +19,93 @@ import { eventSignature, methodSignature } from './signature';
 describe('abi/util/signature', () => {
   describe('eventSignature', () => {
     it('encodes signature baz() correctly', () => {
-      expect(eventSignature('baz', []))
-        .to.deep.equal({ id: 'baz()', signature: 'a7916fac4f538170f7cd12c148552e2cba9fcd72329a2dd5b07a6fa906488ddf' });
+      expect(eventSignature('baz', [])).to.deep.equal({
+        id: 'baz()',
+        name: 'baz',
+        signature: 'a7916fac4f538170f7cd12c148552e2cba9fcd72329a2dd5b07a6fa906488ddf'
+      });
     });
 
     it('encodes signature baz(uint32) correctly', () => {
-      expect(eventSignature('baz', [{ type: 'uint', length: 32 }]))
-        .to.deep.equal({ id: 'baz(uint32)', signature: '7d68785e8fc871be024b75964bd86d093511d4bc2dc7cf7bea32c48a0efaecb1' });
+      expect(eventSignature('baz', [{ type: 'uint', length: 32 }])).to.deep.equal({
+        id: 'baz(uint32)',
+        name: 'baz',
+        signature: '7d68785e8fc871be024b75964bd86d093511d4bc2dc7cf7bea32c48a0efaecb1'
+      });
     });
 
     it('encodes signature baz(uint32, bool) correctly', () => {
-      expect(eventSignature('baz', [{ type: 'uint', length: 32 }, { type: 'bool' }]))
-        .to.deep.equal({ id: 'baz(uint32,bool)', signature: 'cdcd77c0992ec5bbfc459984220f8c45084cc24d9b6efed1fae540db8de801d2' });
+      expect(eventSignature('baz', [{ type: 'uint', length: 32 }, { type: 'bool' }])).to.deep.equal({
+        id: 'baz(uint32,bool)',
+        name: 'baz',
+        signature: 'cdcd77c0992ec5bbfc459984220f8c45084cc24d9b6efed1fae540db8de801d2'
+      });
     });
 
     it('encodes no-name signature correctly as ()', () => {
-      expect(eventSignature(undefined, []))
-        .to.deep.equal({ id: '()', signature: '861731d50c3880a2ca1994d5ec287b94b2f4bd832a67d3e41c08177bdd5674fe' });
+      expect(eventSignature(undefined, [])).to.deep.equal({
+        id: '()',
+        name: undefined,
+        signature: '861731d50c3880a2ca1994d5ec287b94b2f4bd832a67d3e41c08177bdd5674fe'
+      });
     });
 
     it('encodes no-params signature correctly as ()', () => {
-      expect(eventSignature(undefined, undefined))
-        .to.deep.equal({ id: '()', signature: '861731d50c3880a2ca1994d5ec287b94b2f4bd832a67d3e41c08177bdd5674fe' });
+      expect(eventSignature(undefined, undefined)).to.deep.equal({
+        id: '()',
+        name: undefined,
+        signature: '861731d50c3880a2ca1994d5ec287b94b2f4bd832a67d3e41c08177bdd5674fe'
+      });
     });
   });
 
   describe('methodSignature', () => {
     it('encodes signature baz() correctly', () => {
-      expect(methodSignature('baz', [])).to.deep.equal({ id: 'baz()', signature: 'a7916fac' });
+      expect(methodSignature('baz', [])).to.deep.equal({
+        id: 'baz()',
+        name: 'baz',
+        signature: 'a7916fac'
+      });
     });
 
     it('encodes signature baz(uint32) correctly', () => {
-      expect(methodSignature('baz', [{ type: 'uint', length: 32 }])).to.deep.equal({ id: 'baz(uint32)', signature: '7d68785e' });
+      expect(methodSignature('baz', [{ type: 'uint', length: 32 }])).to.deep.equal({
+        id: 'baz(uint32)',
+        name: 'baz',
+        signature: '7d68785e'
+      });
     });
 
     it('encodes signature baz(uint32, bool) correctly', () => {
-      expect(methodSignature('baz', [{ type: 'uint', length: 32 }, { type: 'bool' }])).to.deep.equal({ id: 'baz(uint32,bool)', signature: 'cdcd77c0' });
+      expect(methodSignature('baz', [{ type: 'uint', length: 32 }, { type: 'bool' }])).to.deep.equal({
+        id: 'baz(uint32,bool)',
+        name: 'baz',
+        signature: 'cdcd77c0'
+      });
+    });
+
+    it('encodes signature in name correctly', () => {
+      expect(methodSignature('baz(uint32,bool)', [{ type: 'uint', length: 32 }, { type: 'bool' }])).to.deep.equal({
+        id: 'baz(uint32,bool)',
+        name: 'baz',
+        signature: 'cdcd77c0'
+      });
     });
 
     it('encodes no-name signature correctly as ()', () => {
-      expect(methodSignature(undefined, [])).to.deep.equal({ id: '()', signature: '861731d5' });
+      expect(methodSignature(undefined, [])).to.deep.equal({
+        id: '()',
+        name: undefined,
+        signature: '861731d5'
+      });
     });
 
     it('encodes no-params signature correctly as ()', () => {
-      expect(methodSignature(undefined, undefined)).to.deep.equal({ id: '()', signature: '861731d5' });
+      expect(methodSignature(undefined, undefined)).to.deep.equal({
+        id: '()',
+        name: undefined,
+        signature: '861731d5'
+      });
     });
   });
 });

js/src/api/contract/contract.js

@@ -248,18 +248,32 @@ export default class Contract {
         .call(callParams)
         .then((encoded) => func.decodeOutput(encoded))
         .then((tokens) => tokens.map((token) => token.value))
-        .then((returns) => returns.length === 1 ? returns[0] : returns);
+        .then((returns) => returns.length === 1 ? returns[0] : returns)
+        .catch((error) => {
+          console.warn(`${func.name}.call`, values, error);
+          throw error;
+        });
     };
 
     if (!func.constant) {
       func.postTransaction = (options, values = []) => {
         const _options = this._encodeOptions(func, this._addOptionsTo(options), values);
-        return this._api.parity.postTransaction(_options);
+        return this._api.parity
+          .postTransaction(_options)
+          .catch((error) => {
+            console.warn(`${func.name}.postTransaction`, values, error);
+            throw error;
+          });
       };
 
       func.estimateGas = (options, values = []) => {
         const _options = this._encodeOptions(func, this._addOptionsTo(options), values);
-        return this._api.eth.estimateGas(_options);
+        return this._api.eth
+          .estimateGas(_options)
+          .catch((error) => {
+            console.warn(`${func.name}.estimateGas`, values, error);
+            throw error;
+          });
       };
     }
 
@@ -385,6 +399,10 @@ export default class Contract {
             this._subscribeToChanges();
             return subscriptionId;
           });
+      })
+      .catch((error) => {
+        console.warn('subscribe', event, _options, error);
+        throw error;
       });
   }
 

js/src/modals/ExecuteContract/executeContract.js

@@ -20,7 +20,6 @@ import { observer } from 'mobx-react';
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import { toWei } from '~/api/util/wei';
 import { BusyStep, Button, CompletedStep, GasPriceEditor, IdentityIcon, Modal, TxHash, Warning } from '~/ui';
@@ -470,11 +469,7 @@ function mapStateToProps (initState, initProps) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(ExecuteContract);

js/src/modals/Transfer/store.js

@@ -23,7 +23,7 @@ import { bytesToHex } from '~/api/util/format';
 import Contract from '~/api/contract';
 import ERRORS from './errors';
 import { ERROR_CODES } from '~/api/transport/error';
-import { DEFAULT_GAS, MAX_GAS_ESTIMATION } from '~/util/constants';
+import { DEFAULT_GAS, DEFAULT_GASPRICE, MAX_GAS_ESTIMATION } from '~/util/constants';
 import GasPriceStore from '~/ui/GasPriceEditor/store';
 import { getLogger, LOG_KEYS } from '~/config';
 
@@ -441,6 +441,8 @@ export default class TransferStore {
     const gasTotal = new BigNumber(_gasTotal || 0);
     const { valueAll, isEth, isWallet } = this;
 
+    log.debug('@getValues', 'gas', gasTotal.toFormat());
+
     if (!valueAll) {
       const value = this.getTokenValue();
 
@@ -568,6 +570,7 @@ export default class TransferStore {
   send () {
     const { options, values } = this._getTransferParams();
     options.minBlock = new BigNumber(this.minBlock || 0).gt(0) ? this.minBlock : null;
+    log.debug('@send', 'transfer value', options.value && options.value.toFormat());
 
     return this._getTransferMethod().postTransaction(options, values);
   }
@@ -626,7 +629,8 @@ export default class TransferStore {
       options.gas = MAX_GAS_ESTIMATION;
     }
 
-    const { token } = this.getValues(options.gas);
+    const gasTotal = new BigNumber(options.gas || DEFAULT_GAS).mul(options.gasPrice || DEFAULT_GASPRICE);
+    const { token } = this.getValues(gasTotal);
 
     if (isEth && !isWallet && !forceToken) {
       options.value = token;

js/src/redux/providers/personalActions.js

@@ -75,6 +75,9 @@ export function personalAccountsInfo (accountsInfo) {
             return wallet;
           });
       })
+      .catch(() => {
+        return [];
+      })
       .then((_wallets) => {
         _wallets.forEach((wallet) => {
           const owners = wallet.owners.map((o) => o.address);
@@ -96,6 +99,10 @@ export function personalAccountsInfo (accountsInfo) {
         dispatch(_personalAccountsInfo(data));
         dispatch(attachWallets(wallets));
         dispatch(fetchBalances());
+      })
+      .catch((error) => {
+        console.warn('personalAccountsInfo', error);
+        throw error;
       });
   };
 }

js/src/ui/Balance/balance.js

@@ -17,7 +17,6 @@
 import BigNumber from 'bignumber.js';
 import React, { Component, PropTypes } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import unknownImage from '../../../assets/images/contracts/unknown-64x64.png';
 import styles from './balance.css';
@@ -107,11 +106,7 @@ function mapStateToProps (state) {
   return { images };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(Balance);

js/src/ui/BlockStatus/blockStatus.js

@@ -17,7 +17,6 @@
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import styles from './blockStatus.css';
 
@@ -113,11 +112,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(BlockStatus);

js/src/ui/Form/InputAddress/inputAddress.js

@@ -16,7 +16,6 @@
 
 import React, { Component, PropTypes } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import util from '~/api/util';
 import { nodeOrStringProptype } from '~/util/proptypes';
@@ -162,11 +161,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(InputAddress);

js/src/ui/Form/InputAddressSelect/inputAddressSelect.js

@@ -16,7 +16,6 @@
 
 import React, { Component, PropTypes } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import AddressSelect from '../AddressSelect';
 
@@ -68,11 +67,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(InputAddressSelect);

js/src/ui/MethodDecoding/methodDecoding.js

@@ -377,11 +377,15 @@ class MethodDecoding extends Component {
     }
 
     const inputs = methodInputs.map((input, index) => {
+      const label = input.name
+        ? `${input.name}: ${input.type}`
+        : input.type;
+
       return (
         <TypedInput
           allowCopy
           className={ styles.input }
-          label={ input.type }
+          label={ label }
           key={ index }
           param={ input.type }
           readOnly

js/src/ui/MethodDecoding/methodDecodingStore.js

@@ -164,8 +164,8 @@ export default class MethodDecodingStore {
               methodInputs = this.api.util
                 .decodeMethodInput(abi, paramdata)
                 .map((value, index) => {
-                  const type = abi.inputs[index].type;
-                  return { type, value };
+                  const { name, type } = abi.inputs[index];
+                  return { name, type, value };
                 });
             }
 

js/src/ui/Modal/modal.js

@@ -16,9 +16,8 @@
 
 import { Dialog } from 'material-ui';
 import React, { Component, PropTypes } from 'react';
-import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 import ReactDOM from 'react-dom';
+import { connect } from 'react-redux';
 
 import { nodeOrStringProptype } from '~/util/proptypes';
 
@@ -113,11 +112,7 @@ function mapStateToProps (state) {
   return { settings };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(Modal);

js/src/ui/SignerIcon/signerIcon.js

@@ -16,7 +16,6 @@
 
 import React, { Component, PropTypes } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 import { keccak_256 } from 'js-sha3'; // eslint-disable-line camelcase
 import ActionFingerprint from 'material-ui/svg-icons/action/fingerprint';
 
@@ -54,11 +53,7 @@ function mapStateToProps (state) {
   return { secureToken };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(SignerIcon);

js/src/ui/TxHash/txHash.js

@@ -15,11 +15,10 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 import BigNumber from 'bignumber.js';
+import { LinearProgress } from 'material-ui';
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
-import { LinearProgress } from 'material-ui';
 
 import { txLink } from '~/3rdparty/etherscan/links';
 import ShortenedHash from '../ShortenedHash';
@@ -169,11 +168,7 @@ function mapStateToProps (state) {
   return { isTest };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(TxHash);

js/src/ui/TxList/txList.js

@@ -14,10 +14,9 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
+import { observer } from 'mobx-react';
 import React, { Component, PropTypes } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
-import { observer } from 'mobx-react';
 
 import Store from './store';
 import TxRow from './TxRow';
@@ -92,11 +91,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(TxList);

js/src/util/subscribe-to-events.spec.js

@@ -0,0 +1,115 @@
+// 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/>.
+
+import { spy, stub } from 'sinon';
+
+import subscribeToEvents from './subscribe-to-events';
+import {
+  pastLogs, liveLogs, createApi, createContract
+} from './subscribe-to-events.test.js';
+
+const delay = (t) => new Promise((resolve) => {
+  setTimeout(resolve, t);
+});
+
+describe('util/subscribe-to-events', () => {
+  beforeEach(function () {
+    this.api = createApi();
+    this.contract = createContract(this.api);
+  });
+
+  it('installs a filter', async function () {
+    const { api, contract } = this;
+
+    subscribeToEvents(contract, [ 'Foo', 'Bar' ]);
+    await delay(0);
+
+    expect(api.eth.newFilter.calledOnce).to.equal(true);
+    expect(api.eth.newFilter.firstCall.args).to.eql([ {
+      fromBlock: 0, toBlock: 'latest',
+      address: contract.address,
+      topics: [ [
+        contract.instance.Foo.signature,
+        contract.instance.Bar.signature
+      ] ]
+    } ]);
+  });
+
+  it('queries & parses logs in the beginning', async function () {
+    const { api, contract } = this;
+
+    subscribeToEvents(contract, [ 'Foo', 'Bar' ]);
+
+    await delay(0);
+    expect(api.eth.getFilterLogs.callCount).to.equal(1);
+    expect(api.eth.getFilterLogs.firstCall.args).to.eql([ 123 ]);
+
+    await delay(0);
+    expect(contract.parseEventLogs.callCount).to.equal(1);
+  });
+
+  it('emits logs in the beginning', async function () {
+    const { contract } = this;
+
+    const onLog = spy();
+    const onFoo = spy();
+    const onBar = spy();
+    subscribeToEvents(contract, [ 'Foo', 'Bar' ])
+      .on('log', onLog)
+      .on('Foo', onFoo)
+      .on('Bar', onBar);
+
+    await delay(0);
+
+    expect(onLog.callCount).to.equal(2);
+    expect(onLog.firstCall.args).to.eql([ pastLogs[0] ]);
+    expect(onLog.secondCall.args).to.eql([ pastLogs[1] ]);
+    expect(onFoo.callCount).to.equal(1);
+    expect(onFoo.firstCall.args).to.eql([ pastLogs[0] ]);
+    expect(onBar.callCount).to.equal(1);
+    expect(onBar.firstCall.args).to.eql([ pastLogs[1] ]);
+  });
+
+  it('uninstalls the filter on sunsubscribe', async function () {
+    const { api, contract } = this;
+
+    const s = subscribeToEvents(contract, [ 'Foo', 'Bar' ]);
+    await delay(0);
+    s.unsubscribe();
+    await delay(0);
+
+    expect(api.eth.uninstallFilter.calledOnce).to.equal(true);
+    expect(api.eth.uninstallFilter.firstCall.args).to.eql([ 123 ]);
+  });
+
+  it('checks for new events regularly', async function () {
+    const { api, contract } = this;
+    api.eth.getFilterLogs = stub().resolves([]);
+
+    const onLog = spy();
+    const onBar = spy();
+    const s = subscribeToEvents(contract, [ 'Bar' ], { interval: 5 })
+      .on('log', onLog)
+      .on('Bar', onBar);
+    await delay(9);
+    s.unsubscribe();
+
+    expect(onLog.callCount).to.equal(1);
+    expect(onLog.firstCall.args).to.eql([ liveLogs[0] ]);
+    expect(onBar.callCount).to.equal(1);
+    expect(onBar.firstCall.args).to.eql([ liveLogs[0] ]);
+  });
+});

js/src/util/subscribe-to-events.test.js

@@ -0,0 +1,53 @@
+// 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/>.
+
+import { stub } from 'sinon';
+
+export const ADDRESS = '0x1111111111111111111111111111111111111111';
+
+export const pastLogs = [
+  { event: 'Foo', type: 'mined', address: ADDRESS, params: {} },
+  { event: 'Bar', type: 'mined', address: ADDRESS, params: {} }
+];
+
+export const liveLogs = [
+  { event: 'Bar', type: 'mined', address: ADDRESS, params: { foo: 'bar' } }
+];
+
+export const createApi = () => ({
+  eth: {
+    newFilter: stub().resolves(123),
+    uninstallFilter: stub()
+      .rejects(new Error('unknown filter id'))
+      .withArgs(123).resolves(null),
+    getFilterLogs: stub()
+      .rejects(new Error('unknown filter id'))
+      .withArgs(123).resolves(pastLogs),
+    getFilterChanges: stub()
+      .rejects(new Error('unknown filter id'))
+      .withArgs(123).resolves(liveLogs)
+  }
+});
+
+export const createContract = (api) => ({
+  api,
+  address: ADDRESS,
+  instance: {
+    Foo: { signature: 'Foo signature' },
+    Bar: { signature: 'Bar signature' }
+  },
+  parseEventLogs: stub().returnsArg(0)
+});

js/src/views/Account/Transactions/transactions.js

@@ -18,7 +18,6 @@ import { observer } from 'mobx-react';
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import { Container, TxList, Loading } from '~/ui';
 
@@ -118,11 +117,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(Transactions);

js/src/views/Application/Status/status.js

@@ -17,7 +17,6 @@
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 
 import { BlockStatus } from '~/ui';
 
@@ -133,11 +132,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(Status);

js/src/views/Application/application.js

@@ -14,10 +14,9 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
+import { observer } from 'mobx-react';
 import React, { Component, PropTypes } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
-import { observer } from 'mobx-react';
 
 import UpgradeStore from '~/modals/UpgradeParity/store';
 
@@ -123,11 +122,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(Application);

js/src/views/Dapps/dapps.js

@@ -14,13 +14,12 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
+import { omitBy } from 'lodash';
 import { Checkbox } from 'material-ui';
 import { observer } from 'mobx-react';
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
-import { omitBy } from 'lodash';
 
 import { AddDapps, DappPermissions } from '~/modals';
 import PermissionStore from '~/modals/DappPermissions/store';
@@ -169,11 +168,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(Dapps);

js/src/views/Dapps/dappsStore.js

@@ -256,7 +256,7 @@ export default class DappsStore {
     store.set(LS_KEY_DISPLAY, this.displayApps);
   }
 
-  @action addApps = (_apps) => {
+  @action addApps = (_apps = []) => {
     transaction(() => {
       const apps = _apps.filter((app) => app);
 

js/src/views/ParityBar/parityBar.js

@@ -17,7 +17,6 @@
 import React, { Component, PropTypes } from 'react';
 import { Link } from 'react-router';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
 import ActionFingerprint from 'material-ui/svg-icons/action/fingerprint';
 import ContentClear from 'material-ui/svg-icons/content/clear';
 
@@ -162,11 +161,7 @@ function mapStateToProps (state) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(ParityBar);

js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.js

@@ -14,13 +14,12 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
+import keycode from 'keycode';
+import RaisedButton from 'material-ui/RaisedButton';
 import React, { Component, PropTypes } from 'react';
 import ReactDOM from 'react-dom';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
-import RaisedButton from 'material-ui/RaisedButton';
 import ReactTooltip from 'react-tooltip';
-import keycode from 'keycode';
 
 import { Form, Input, IdentityIcon } from '~/ui';
 
@@ -258,11 +257,7 @@ function mapStateToProps (_, initProps) {
   };
 }
 
-function mapDispatchToProps (dispatch) {
-  return bindActionCreators({}, dispatch);
-}
-
 export default connect(
   mapStateToProps,
-  mapDispatchToProps
+  null
 )(TransactionPendingFormConfirm);

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/eth.rs

@@ -660,7 +660,6 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 			num => take_weak!(self.client).call(&signed, num.into(), Default::default()),
 		};
 
-
 		result
 			.map(|b| b.output.into())
 			.map_err(errors::from_call_error)

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;
@@ -178,7 +179,7 @@ impl EthSync {
 				};
 
 				let mut light_proto = LightProtocol::new(params.provider, light_params);
-				light_proto.add_handler(Box::new(TxRelay(params.chain.clone())));
+				light_proto.add_handler(Arc::new(TxRelay(params.chain.clone())));
 
 				Arc::new(light_proto)
 			})
@@ -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
@@ -611,7 +612,7 @@ impl LightSync {
 
 			let mut light_proto = LightProtocol::new(params.client.clone(), light_params);
 			let sync_handler = try!(SyncHandler::new(params.client.clone()));
-			light_proto.add_handler(Box::new(sync_handler));
+			light_proto.add_handler(Arc::new(sync_handler));
 
 			Arc::new(light_proto)
 		};
@@ -667,3 +668,4 @@ impl ManageNetwork for LightSync {
 		NetworkConfiguration::from(self.network.config().clone())
 	}
 }
+

sync/src/light_sync/mod.rs

@@ -23,6 +23,14 @@
 //!
 //! This is written assuming that the client and sync service are running
 //! in the same binary; unlike a full node which might communicate via IPC.
+//!
+//!
+//! Sync strategy:
+//! - Find a common ancestor with peers.
+//! - Split the chain up into subchains, which are downloaded in parallel from various peers in rounds.
+//! - When within a certain distance of the head of the chain, aggressively download all
+//!   announced blocks.
+//! - On bad block/response, punish peer and reset.
 
 use std::collections::HashMap;
 use std::mem;
@@ -43,6 +51,9 @@ use self::sync_round::{AbortReason, SyncRound, ResponseContext};
 mod response;
 mod sync_round;
 
+#[cfg(test)]
+mod tests;
+
 /// Peer chain info.
 #[derive(Clone)]
 struct ChainInfo {
@@ -64,6 +75,7 @@ impl Peer {
 	}
 }
 // search for a common ancestor with the best chain.
+#[derive(Debug)]
 enum AncestorSearch {
 	Queued(u64), // queued to search for blocks starting from here.
 	Awaiting(ReqId, u64, request::Headers), // awaiting response for this request.
@@ -125,6 +137,9 @@ impl AncestorSearch {
 
 		match self {
 			AncestorSearch::Queued(start) => {
+				trace!(target: "sync", "Requesting {} reverse headers from {} to find common ancestor",
+					BATCH_SIZE, start);
+
 				let req = request::Headers {
 					start: start.into(),
 					max: ::std::cmp::min(start as usize, BATCH_SIZE),
@@ -143,8 +158,9 @@ impl AncestorSearch {
 }
 
 // synchronization state machine.
+#[derive(Debug)]
 enum SyncState {
-	// Idle (waiting for peers)
+	// Idle (waiting for peers) or at chain head.
 	Idle,
 	// searching for common ancestor with best chain.
 	// queue should be cleared at this phase.
@@ -328,19 +344,19 @@ impl<L: LightChainClient> LightSync<L> {
 			return;
 		}
 
-		trace!(target: "sync", "Beginning search for common ancestor");
-		self.client.clear_queue();
+		self.client.flush_queue();
 		let chain_info = self.client.chain_info();
 
+		trace!(target: "sync", "Beginning search for common ancestor from {:?}",
+			(chain_info.best_block_number, chain_info.best_block_hash));
 		*state = SyncState::AncestorSearch(AncestorSearch::begin(chain_info.best_block_number));
 	}
 
 	fn maintain_sync(&self, ctx: &BasicContext) {
 		const DRAIN_AMOUNT: usize = 128;
 
-		debug!(target: "sync", "Maintaining sync.");
-
 		let mut state = self.state.lock();
+		debug!(target: "sync", "Maintaining sync ({:?})", &*state);
 
 		// drain any pending blocks into the queue.
 		{
@@ -358,6 +374,7 @@ impl<L: LightChainClient> LightSync<L> {
 				};
 
 				if sink.is_empty() { break }
+				trace!(target: "sync", "Drained {} headers to import", sink.len());
 
 				for header in sink.drain(..) {
 					if let Err(e) = self.client.queue_header(header) {
@@ -372,8 +389,12 @@ impl<L: LightChainClient> LightSync<L> {
 
 		// handle state transitions.
 		{
+			let chain_info = self.client.chain_info();
+			let best_td = chain_info.total_difficulty;
 			match mem::replace(&mut *state, SyncState::Idle) {
-				SyncState::Rounds(SyncRound::Abort(reason)) => {
+				_ if self.best_seen.lock().as_ref().map_or(true, |&(_, td)| best_td >= td)
+					=> *state = SyncState::Idle,
+				SyncState::Rounds(SyncRound::Abort(reason, _)) => {
 					match reason {
 						AbortReason::BadScaffold(bad_peers) => {
 							debug!(target: "sync", "Disabling peers responsible for bad scaffold");
@@ -394,7 +415,7 @@ impl<L: LightChainClient> LightSync<L> {
 				}
 				SyncState::AncestorSearch(AncestorSearch::Genesis) => {
 					// Same here.
-					let g_hash = self.client.chain_info().genesis_hash;
+					let g_hash = chain_info.genesis_hash;
 					*state = SyncState::Rounds(SyncRound::begin(0, g_hash));
 				}
 				SyncState::Idle => self.begin_search(&mut state),

sync/src/light_sync/sync_round.rs

@@ -18,6 +18,7 @@
 
 use std::cmp::Ordering;
 use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque};
+use std::fmt;
 
 use ethcore::header::Header;
 
@@ -29,9 +30,9 @@ use util::{Bytes, H256};
 
 use super::response;
 
-// amount of blocks between each scaffold entry.
+/// amount of blocks between each scaffold entry.
 // TODO: move these into parameters for `RoundStart::new`?
-const ROUND_SKIP: u64 = 255;
+pub const ROUND_SKIP: u64 = 255;
 
 // amount of scaffold frames: these are the blank spaces in "X___X___X"
 const ROUND_FRAMES: usize = 255;
@@ -132,7 +133,7 @@ impl Fetcher {
 
 		let end = match sparse_headers.last().map(|h| (h.number(), h.hash())) {
 			Some(end) => end,
-			None => return SyncRound::abort(AbortReason::BadScaffold(contributors)),
+			None => return SyncRound::abort(AbortReason::BadScaffold(contributors), VecDeque::new()),
 		};
 
 		SyncRound::Fetch(Fetcher {
@@ -217,10 +218,11 @@ impl Fetcher {
 
 				let subchain_parent = request.subchain_parent.1;
 
+				// check if the subchain portion has been completely filled.
 				if request.headers_request.max == 0 {
 					if parent_hash.map_or(true, |hash| hash != subchain_parent) {
 						let abort = AbortReason::BadScaffold(self.scaffold_contributors);
-						return SyncRound::Abort(abort);
+						return SyncRound::abort(abort, self.ready);
 					}
 
 					self.complete_requests.insert(subchain_parent, request);
@@ -271,6 +273,7 @@ impl Fetcher {
 		headers.extend(self.ready.drain(0..max));
 
 		if self.sparse.is_empty() && self.ready.is_empty() {
+			trace!(target: "sync", "sync round complete. Starting anew from {:?}", self.end);
 			SyncRound::Start(RoundStart::new(self.end))
 		} else {
 			SyncRound::Fetch(self)
@@ -309,7 +312,7 @@ impl RoundStart {
 			if self.sparse_headers.len() > 1 {
 				Fetcher::new(self.sparse_headers, self.contributors.into_iter().collect())
 			} else {
-				SyncRound::Abort(AbortReason::NoResponses)
+				SyncRound::Abort(AbortReason::NoResponses, self.sparse_headers.into())
 			}
 		} else {
 			SyncRound::Start(self)
@@ -375,14 +378,19 @@ impl RoundStart {
 			let start = (self.start_block.0 + 1)
 				+ self.sparse_headers.len() as u64 * (ROUND_SKIP + 1);
 
+			let max = (ROUND_FRAMES - 1) - self.sparse_headers.len();
+
 			let headers_request = HeadersRequest {
 				start: start.into(),
-				max: (ROUND_FRAMES - 1) - self.sparse_headers.len(),
+				max: max,
 				skip: ROUND_SKIP,
 				reverse: false,
 			};
 
 			if let Some(req_id) = dispatcher(headers_request.clone()) {
+				trace!(target: "sync", "Requesting scaffold: {} headers forward from {}, skip={}",
+					max, start, ROUND_SKIP);
+
 				self.pending_req = Some((req_id, headers_request));
 			}
 		}
@@ -397,15 +405,15 @@ pub enum SyncRound {
 	Start(RoundStart),
 	/// Fetching intermediate blocks during a sync round.
 	Fetch(Fetcher),
-	/// Aborted.
-	Abort(AbortReason),
+	/// Aborted + Sequential headers
+	Abort(AbortReason, VecDeque<Header>),
 }
 
 impl SyncRound {
-	fn abort(reason: AbortReason) -> Self {
-		trace!(target: "sync", "Aborting sync round: {:?}", reason);
+	fn abort(reason: AbortReason, remaining: VecDeque<Header>) -> Self {
+		trace!(target: "sync", "Aborting sync round: {:?}. To drain: {:?}", reason, remaining);
 
-		SyncRound::Abort(reason)
+		SyncRound::Abort(reason, remaining)
 	}
 
 	/// Begin sync rounds from a starting block.
@@ -450,7 +458,23 @@ impl SyncRound {
 	pub fn drain(self, v: &mut Vec<Header>, max: Option<usize>) -> Self {
 		match self {
 			SyncRound::Fetch(fetcher) => fetcher.drain(v, max),
+			SyncRound::Abort(reason, mut remaining) => {
+				let len = ::std::cmp::min(max.unwrap_or(usize::max_value()), remaining.len());
+				v.extend(remaining.drain(..len));
+				SyncRound::Abort(reason, remaining)
+			}
 			other => other,
 		}
 	}
 }
+
+impl fmt::Debug for SyncRound {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		match *self {
+			SyncRound::Start(ref state) => write!(f, "Scaffolding from {:?}", state.start_block),
+			SyncRound::Fetch(ref fetcher) => write!(f, "Filling scaffold up to {:?}", fetcher.end),
+			SyncRound::Abort(ref reason, ref remaining) =>
+				write!(f, "Aborted: {:?}, {} remain", reason, remaining.len()),
+		}
+	}
+}

sync/src/light_sync/tests/mod.rs

@@ -0,0 +1,19 @@
+// 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/>.
+
+#![allow(dead_code)]
+
+mod test_net;

sync/src/light_sync/tests/test_net.rs

@@ -0,0 +1,211 @@
+// 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/>.
+
+//! TestNet peer definition.
+
+use std::collections::{HashSet, VecDeque};
+use std::sync::Arc;
+
+use light_sync::*;
+use tests::helpers::{TestNet, Peer as PeerLike, TestPacket};
+
+use ethcore::client::TestBlockChainClient;
+use ethcore::spec::Spec;
+use io::IoChannel;
+use light::client::Client as LightClient;
+use light::net::{LightProtocol, IoContext, Capabilities, Params as LightParams};
+use light::net::buffer_flow::FlowParams;
+use network::{NodeId, PeerId};
+use util::RwLock;
+
+const NETWORK_ID: u64 = 0xcafebabe;
+
+struct TestIoContext<'a> {
+	queue: &'a RwLock<VecDeque<TestPacket>>,
+	sender: Option<PeerId>,
+	to_disconnect: RwLock<HashSet<PeerId>>,
+}
+
+impl<'a> IoContext for TestIoContext<'a> {
+	fn send(&self, peer: PeerId, packet_id: u8, packet_body: Vec<u8>) {
+		self.queue.write().push_back(TestPacket {
+			data: packet_body,
+			packet_id: packet_id,
+			recipient: peer,
+		})
+	}
+
+	fn respond(&self, packet_id: u8, packet_body: Vec<u8>) {
+		if let Some(sender) = self.sender {
+			self.send(sender, packet_id, packet_body);
+		}
+	}
+
+	fn disconnect_peer(&self, peer: PeerId) {
+		self.to_disconnect.write().insert(peer);
+	}
+
+	fn disable_peer(&self, peer: PeerId) { self.disconnect_peer(peer) }
+	fn protocol_version(&self, _peer: PeerId) -> Option<u8> { Some(::light::net::MAX_PROTOCOL_VERSION) }
+
+	fn persistent_peer_id(&self, _peer: PeerId) -> Option<NodeId> { unimplemented!() }
+}
+
+// peer-specific data.
+enum PeerData {
+	Light(Arc<LightSync<LightClient>>, Arc<LightClient>),
+	Full(Arc<TestBlockChainClient>)
+}
+
+// test peer type.
+// Either a full peer or a LES peer.
+pub struct Peer {
+	proto: LightProtocol,
+	queue: RwLock<VecDeque<TestPacket>>,
+	data: PeerData,
+}
+
+impl Peer {
+	// create a new full-client peer for light client peers to sync to.
+	// buffer flow is made negligible.
+	pub fn new_full(chain: Arc<TestBlockChainClient>) -> Self {
+		let params = LightParams {
+			network_id: NETWORK_ID,
+			flow_params: FlowParams::free(),
+			capabilities: Capabilities {
+				serve_headers: true,
+				serve_chain_since: None,
+				serve_state_since: None,
+				tx_relay: true,
+			},
+		};
+
+		let proto = LightProtocol::new(chain.clone(), params);
+		Peer {
+			proto: proto,
+			queue: RwLock::new(VecDeque::new()),
+			data: PeerData::Full(chain),
+		}
+	}
+
+	// create a new light-client peer to sync to full peers.
+	pub fn new_light(chain: Arc<LightClient>) -> Self {
+		let sync = Arc::new(LightSync::new(chain.clone()).unwrap());
+		let params = LightParams {
+			network_id: NETWORK_ID,
+			flow_params: FlowParams::default(),
+			capabilities: Capabilities {
+				serve_headers: false,
+				serve_chain_since: None,
+				serve_state_since: None,
+				tx_relay: false,
+			},
+		};
+
+		let mut proto = LightProtocol::new(chain.clone(), params);
+		proto.add_handler(sync.clone());
+		Peer {
+			proto: proto,
+			queue: RwLock::new(VecDeque::new()),
+			data: PeerData::Light(sync, chain),
+		}
+	}
+
+	// get the chain from the client, asserting that it is a full node.
+	pub fn chain(&self) -> &TestBlockChainClient {
+		match self.data {
+			PeerData::Full(ref chain) => &*chain,
+			_ => panic!("Attempted to access full chain on light peer."),
+		}
+	}
+
+	// get the light chain from the peer, asserting that it is a light node.
+	pub fn light_chain(&self) -> &LightClient {
+		match self.data {
+			PeerData::Light(_, ref chain) => &*chain,
+			_ => panic!("Attempted to access light chain on full peer."),
+		}
+	}
+
+	// get a test Io context based on
+	fn io(&self, sender: Option<PeerId>) -> TestIoContext {
+		TestIoContext {
+			queue: &self.queue,
+			sender: sender,
+			to_disconnect: RwLock::new(HashSet::new()),
+		}
+	}
+}
+
+impl PeerLike for Peer {
+	type Message = TestPacket;
+
+	fn on_connect(&self, other: PeerId) {
+		let io = self.io(Some(other));
+		self.proto.on_connect(&other, &io);
+	}
+
+	fn on_disconnect(&self, other: PeerId){
+		let io = self.io(Some(other));
+		self.proto.on_disconnect(other, &io);
+	}
+
+	fn receive_message(&self, from: PeerId, msg: TestPacket) -> HashSet<PeerId> {
+		let io = self.io(Some(from));
+		self.proto.handle_packet(&io, &from, msg.packet_id, &msg.data);
+		io.to_disconnect.into_inner()
+	}
+
+	fn pending_message(&self) -> Option<TestPacket> {
+		self.queue.write().pop_front()
+	}
+
+	fn is_done(&self) -> bool {
+		self.queue.read().is_empty()
+	}
+
+	fn sync_step(&self) {
+		if let PeerData::Light(_, ref client) = self.data {
+			client.flush_queue();
+			client.import_verified();
+		}
+	}
+
+	fn restart_sync(&self) { }
+}
+
+impl TestNet<Peer> {
+	/// Create a new `TestNet` for testing light synchronization.
+	/// The first parameter is the number of light nodes,
+	/// the second is the number of full nodes.
+	pub fn light(n_light: usize, n_full: usize) -> Self {
+		let mut peers = Vec::with_capacity(n_light + n_full);
+		for _ in 0..n_light {
+			let client = LightClient::new(Default::default(), &Spec::new_test(), IoChannel::disconnected());
+			peers.push(Arc::new(Peer::new_light(Arc::new(client))))
+		}
+
+		for _ in 0..n_full {
+			peers.push(Arc::new(Peer::new_full(Arc::new(TestBlockChainClient::new()))))
+		}
+
+		TestNet {
+			peers: peers,
+			started: false,
+			disconnect_events: Vec::new(),
+		}
+	}
+}

sync/src/tests/chain.rs

@@ -100,8 +100,11 @@ fn forked() {
 fn forked_with_misbehaving_peer() {
 	::env_logger::init().ok();
 	let mut net = TestNet::new(3);
+
+	let mut alt_spec = ::ethcore::spec::Spec::new_test();
+	alt_spec.extra_data = b"fork".to_vec();
 	// peer 0 is on a totally different chain with higher total difficulty
-	net.peer_mut(0).chain = Arc::new(TestBlockChainClient::new_with_extra_data(b"fork".to_vec()));
+	net.peer_mut(0).chain = Arc::new(TestBlockChainClient::new_with_spec(alt_spec));
 	net.peer(0).chain.add_blocks(50, EachBlockWith::Nothing);
 	net.peer(1).chain.add_blocks(10, EachBlockWith::Nothing);
 	net.peer(2).chain.add_blocks(10, EachBlockWith::Nothing);

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 s1 = KeyPair::from_secret("0".sha3()).unwrap();
+	let s0 = KeyPair::from_secret_slice(&"1".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 s1 = KeyPair::from_secret("0".sha3()).unwrap();
+	let s0 = KeyPair::from_secret_slice(&"1".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;