Remove unnecessary Engine method

ethcore/src/engines/authority_round.rs

@@ -234,7 +234,7 @@ impl Engine for AuthorityRound {
 		let header = block.header();
 		let step = self.step.load(AtomicOrdering::SeqCst);
 		if self.is_step_proposer(step, header.author()) {
-			let ref ap = *self.account_provider.lock();
+			let ap = self.account_provider.lock();
 			if let Ok(signature) = ap.sign(*header.author(), self.password.read().clone(), header.bare_hash()) {
 				trace!(target: "poa", "generate_seal: Issuing a block for step {}.", step);
 				self.proposed.store(true, AtomicOrdering::SeqCst);
@@ -329,12 +329,10 @@ impl Engine for AuthorityRound {
 		self.validators.register_call_contract(client);
 	}
 
-	fn set_signer(&self, _address: Address, password: String) {
+	fn set_signer(&self, ap: Arc<AccountProvider>, address: Address, password: String) {
+		*self.account_provider.lock() = ap;	
 		*self.password.write() = Some(password);
-	}
-
-	fn register_account_provider(&self, account_provider: Arc<AccountProvider>) {
-		*self.account_provider.lock() = account_provider;
+		debug!(target: "poa", "Setting Engine signer to {}", address);
 	}
 }
 
@@ -401,13 +399,12 @@ mod tests {
 
 	#[test]
 	fn generates_seal_and_does_not_double_propose() {
-		let tap = AccountProvider::transient_provider();
+		let tap = Arc::new(AccountProvider::transient_provider());
 		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;
-		engine.register_account_provider(Arc::new(tap));
 		let genesis_header = spec.genesis_header();
 		let db1 = spec.ensure_db_good(get_temp_state_db().take(), &Default::default()).unwrap();
 		let db2 = spec.ensure_db_good(get_temp_state_db().take(), &Default::default()).unwrap();
@@ -417,14 +414,14 @@ mod tests {
 		let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes, addr2, (3141562.into(), 31415620.into()), vec![]).unwrap();
 		let b2 = b2.close_and_lock();
 
-		engine.set_signer(addr1, "1".into());
+		engine.set_signer(tap.clone(), addr1, "1".into());
 		if let Seal::Regular(seal) = engine.generate_seal(b1.block()) {
 			assert!(b1.clone().try_seal(engine, seal).is_ok());
 			// Second proposal is forbidden.
 			assert!(engine.generate_seal(b1.block()) == Seal::None);
 		}
 
-		engine.set_signer(addr2, "2".into());
+		engine.set_signer(tap, addr2, "2".into());
 		if let Seal::Regular(seal) = engine.generate_seal(b2.block()) {
 			assert!(b2.clone().try_seal(engine, seal).is_ok());
 			// Second proposal is forbidden.

ethcore/src/engines/basic_authority.rs

@@ -56,7 +56,7 @@ pub struct BasicAuthority {
 	params: CommonParams,
 	gas_limit_bound_divisor: U256,
 	builtins: BTreeMap<Address, Builtin>,
-	account_provider: Mutex<Option<Arc<AccountProvider>>>,
+	account_provider: Mutex<Arc<AccountProvider>>,
 	password: RwLock<Option<String>>,
 	validators: Box<ValidatorSet + Send + Sync>,
 }
@@ -69,7 +69,7 @@ impl BasicAuthority {
 			gas_limit_bound_divisor: our_params.gas_limit_bound_divisor,
 			builtins: builtins,
 			validators: new_validator_set(our_params.validators),
-			account_provider: Mutex::new(None),
+			account_provider: Mutex::new(Arc::new(AccountProvider::transient_provider())),
 			password: RwLock::new(None),
 		}
 	}
@@ -110,20 +110,17 @@ impl Engine for BasicAuthority {
 
 	/// Attempt to seal the block internally.
 	fn generate_seal(&self, block: &ExecutedBlock) -> Seal {
-		if let Some(ref ap) = *self.account_provider.lock() {
-			let header = block.header();
-			let author = header.author();
-			if self.validators.contains(author) {
-				let message = header.bare_hash();
-				// account should be pernamently unlocked, otherwise sealing will fail
-				if let Ok(signature) = ap.sign(*author, self.password.read().clone(), message) {
-					return Seal::Regular(vec![::rlp::encode(&(&*signature as &[u8])).to_vec()]);
-				} else {
-					trace!(target: "basicauthority", "generate_seal: FAIL: accounts secret key unavailable");
-				}
+		let ref ap = *self.account_provider.lock();
+		let header = block.header();
+		let author = header.author();
+		if self.validators.contains(author) {
+			let message = header.bare_hash();
+			// account should be pernamently unlocked, otherwise sealing will fail
+			if let Ok(signature) = ap.sign(*author, self.password.read().clone(), message) {
+				return Seal::Regular(vec![::rlp::encode(&(&*signature as &[u8])).to_vec()]);
+			} else {
+				trace!(target: "basicauthority", "generate_seal: FAIL: accounts secret key unavailable");
 			}
-		} else {
-			trace!(target: "basicauthority", "generate_seal: FAIL: accounts not provided");
 		}
 		Seal::None
 	}
@@ -174,12 +171,9 @@ impl Engine for BasicAuthority {
 		self.validators.register_call_contract(client);
 	}
 
-	fn set_signer(&self, _address: Address, password: String) {
+	fn set_signer(&self, ap: Arc<AccountProvider>, _address: Address, password: String) {
 		*self.password.write() = Some(password);
-	}
-
-	fn register_account_provider(&self, ap: Arc<AccountProvider>) {
-		*self.account_provider.lock() = Some(ap);
+		*self.account_provider.lock() = ap;
 	}
 }
 
@@ -256,8 +250,7 @@ mod tests {
 
 		let spec = new_test_authority();
 		let engine = &*spec.engine;
-		engine.set_signer(addr, "".into());
-		engine.register_account_provider(Arc::new(tap));
+		engine.set_signer(Arc::new(tap), addr, "".into());
 		let genesis_header = spec.genesis_header();
 		let mut db_result = get_temp_state_db();
 		let db = spec.ensure_db_good(db_result.take(), &Default::default()).unwrap();

ethcore/src/engines/mod.rs

@@ -212,14 +212,11 @@ pub trait Engine : Sync + Send {
 	fn is_proposal(&self, _verified_header: &Header) -> bool { false }
 
 	/// Register an account which signs consensus messages.
-	fn set_signer(&self, _address: Address, _password: String) {}
+	fn set_signer(&self, _account_provider: Arc<AccountProvider>, _address: Address, _password: String) {}
 
 	/// Add Client which can be used for sealing, querying the state and sending messages.
 	fn register_client(&self, _client: Weak<Client>) {}
 
-	/// Add an account provider useful for Engines that sign stuff.
-	fn register_account_provider(&self, _account_provider: Arc<AccountProvider>) {}
-
 	/// Trigger next step of the consensus engine.
 	fn step(&self) {}
 

ethcore/src/engines/tendermint/mod.rs

@@ -94,7 +94,7 @@ pub struct Tendermint {
 	/// Vote accumulator.
 	votes: VoteCollector,
 	/// Used to sign messages and proposals.
-	account_provider: Mutex<Option<Arc<AccountProvider>>>,
+	account_provider: Mutex<Arc<AccountProvider>>,
 	/// Message for the last PoLC.
 	lock_change: RwLock<Option<ConsensusMessage>>,
 	/// Last lock round.
@@ -122,7 +122,7 @@ impl Tendermint {
 				round: AtomicUsize::new(0),
 				step: RwLock::new(Step::Propose),
 				votes: VoteCollector::new(),
-				account_provider: Mutex::new(None),
+				account_provider: Mutex::new(Arc::new(AccountProvider::transient_provider())),
 				lock_change: RwLock::new(None),
 				last_lock: AtomicUsize::new(0),
 				proposal: RwLock::new(None),
@@ -158,30 +158,26 @@ impl Tendermint {
 	}
 
 	fn generate_message(&self, block_hash: Option<BlockHash>) -> Option<Bytes> {
-		if let Some(ref ap) = *self.account_provider.lock() {
-			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(&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);
-					debug!(target: "poa", "Generated {:?} as {}.", message, *authority);
-					self.handle_valid_message(&message);
-
-					Some(message_rlp)
-				},
-				Err(e) => {
-					trace!(target: "poa", "Could not sign the message {}", e);
-					None
-				},
-			}
-		} else {
-			warn!(target: "poa", "No AccountProvider available.");
-			None
+		let ref ap = *self.account_provider.lock();
+		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(&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);
+				debug!(target: "poa", "Generated {:?} as {}.", message, *authority);
+				self.handle_valid_message(&message);
+
+				Some(message_rlp)
+			},
+			Err(e) => {
+				trace!(target: "poa", "Could not sign the message {}", e);
+				None
+			},
 		}
 	}
 
@@ -420,35 +416,31 @@ impl Engine for Tendermint {
 
 	/// Attempt to seal generate a proposal seal.
 	fn generate_seal(&self, block: &ExecutedBlock) -> Seal {
-		if let Some(ref ap) = *self.account_provider.lock() {
-			let header = block.header();
-			let author = header.author();
-			// Only proposer can generate seal if None was generated.
-			if self.is_proposer(author).is_err() || self.proposal.read().is_some() {
-				return Seal::None;
-			}
+		let ref ap = *self.account_provider.lock();
+		let header = block.header();
+		let author = header.author();
+		// Only proposer can generate seal if None was generated.
+		if self.is_proposer(author).is_err() || self.proposal.read().is_some() {
+			return Seal::None;
+		}
 
-			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(&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);
-				// Remember proposal for later seal submission.
-				*self.proposal.write() = bh;
-				Seal::Proposal(vec![
-					::rlp::encode(&round).to_vec(),
-					::rlp::encode(&signature).to_vec(),
-					::rlp::EMPTY_LIST_RLP.to_vec()
-				])
-			} else {
-				warn!(target: "poa", "generate_seal: FAIL: accounts secret key unavailable");
-				Seal::None
-			}
+		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(&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);
+			// Remember proposal for later seal submission.
+			*self.proposal.write() = bh;
+			Seal::Proposal(vec![
+				::rlp::encode(&round).to_vec(),
+				::rlp::encode(&signature).to_vec(),
+				::rlp::EMPTY_LIST_RLP.to_vec()
+			])
 		} else {
-			warn!(target: "poa", "generate_seal: FAIL: accounts not provided");
+			warn!(target: "poa", "generate_seal: FAIL: accounts secret key unavailable");
 			Seal::None
 		}
 	}
@@ -563,9 +555,12 @@ impl Engine for Tendermint {
 		Ok(())
 	}
 
-	fn set_signer(&self, address: Address, password: String) {
-		*self.authority.write()	= address;
-		*self.password.write() = Some(password);
+	fn set_signer(&self, ap: Arc<AccountProvider>, address: Address, password: String) {
+		{
+			*self.authority.write()	= address;
+			*self.password.write() = Some(password);
+			*self.account_provider.lock() = ap;
+		}
 		self.to_step(Step::Propose);
 	}
 
@@ -651,10 +646,6 @@ impl Engine for Tendermint {
 		*self.client.write() = Some(client.clone());
 		self.validators.register_call_contract(client);
 	}
-
-	fn register_account_provider(&self, account_provider: Arc<AccountProvider>) {
-		*self.account_provider.lock() = Some(account_provider);
-	}
 }
 
 #[cfg(test)]
@@ -678,7 +669,6 @@ mod tests {
 	fn setup() -> (Spec, Arc<AccountProvider>) {
 		let tap = Arc::new(AccountProvider::transient_provider());
 		let spec = Spec::new_test_tendermint();
-		spec.engine.register_account_provider(tap.clone());
 		(spec, tap)
 	}
 
@@ -722,7 +712,7 @@ mod tests {
 
 	fn insert_and_register(tap: &Arc<AccountProvider>, engine: &Engine, acc: &str) -> Address {
 		let addr = insert_and_unlock(tap, acc);
-		engine.set_signer(addr.clone(), acc.into());
+		engine.set_signer(tap.clone(), addr.clone(), acc.into());
 		addr
 	}
 
@@ -884,7 +874,7 @@ mod tests {
 		let v0 = insert_and_register(&tap, engine.as_ref(), "0");
 		let v1 = insert_and_register(&tap, engine.as_ref(), "1");
 
-		let h = 0;
+		let h = 1;
 		let r = 0;
 
 		// Propose
@@ -915,16 +905,16 @@ mod tests {
 		use types::transaction::{Transaction, Action};
 		use client::BlockChainClient;
 
-		let client = generate_dummy_client_with_spec_and_data(Spec::new_test_tendermint, 0, 0, &[]);
-		let engine = client.engine();
 		let tap = Arc::new(AccountProvider::transient_provider());
-
 		// Accounts for signing votes.
 		let v0 = insert_and_unlock(&tap, "0");
 		let v1 = insert_and_unlock(&tap, "1");
+		let client = generate_dummy_client_with_spec_and_accounts(Spec::new_test_tendermint, Some(tap.clone()));
+		let engine = client.engine();
+
+		client.miner().set_engine_signer(v1.clone(), "1".into()).unwrap();
 
 		let notify = Arc::new(TestNotify::default());
-		engine.register_account_provider(tap.clone());
 		client.add_notify(notify.clone());
 		engine.register_client(Arc::downgrade(&client));
 
@@ -939,8 +929,6 @@ mod tests {
 		}.sign(keypair.secret(), None);
 		client.miner().import_own_transaction(client.as_ref(), transaction.into()).unwrap();
 
-		client.miner().set_engine_signer(v1.clone(), "1".into()).unwrap();
-
 		// Propose
 		let proposal = Some(client.miner().pending_block().unwrap().header.bare_hash());
 		// Propose timeout

ethcore/src/engines/validator_set/contract.rs

@@ -142,13 +142,13 @@ mod tests {
 	use client::{BlockChainClient, EngineClient};
 	use ethkey::Secret;
 	use miner::MinerService;
-	use tests::helpers::generate_dummy_client_with_spec_and_data;
+	use tests::helpers::generate_dummy_client_with_spec_and_accounts;
 	use super::super::ValidatorSet;
 	use super::ValidatorContract;
 
 	#[test]
 	fn fetches_validators() {
-		let client = generate_dummy_client_with_spec_and_data(Spec::new_validator_contract, 0, 0, &[]);
+		let client = generate_dummy_client_with_spec_and_accounts(Spec::new_validator_contract, None);
 		let vc = Arc::new(ValidatorContract::new(Address::from_str("0000000000000000000000000000000000000005").unwrap()));
 		vc.register_call_contract(Arc::downgrade(&client));
 		vc.update();
@@ -162,12 +162,7 @@ mod tests {
 		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());
-			spec
-		};
-		let client = generate_dummy_client_with_spec_and_data(spec_factory, 0, 0, &[]);
+		let client = generate_dummy_client_with_spec_and_accounts(Spec::new_validator_contract, Some(tap));
 		client.engine().register_client(Arc::downgrade(&client));
 		let validator_contract = Address::from_str("0000000000000000000000000000000000000005").unwrap();