RPC method parity_clearEngineSigner

Add RPC method parity_clearEngineSigner
Fixes #113

ethcore/engine/src/engine.rs

@@ -295,6 +295,9 @@ pub trait Engine: Sync + Send {
 	/// Register a component which signs consensus messages.
 	fn set_signer(&self, _signer: Box<dyn EngineSigner>) {}
 
+	/// Unregisters the engine signer address to stop signing consensus messages.
+	fn clear_signer(&self) {}
+
 	/// Sign using the EngineSigner, to be used for consensus tx signing.
 	fn sign(&self, _hash: H256) -> Result<Signature, Error> { unimplemented!() }
 

ethcore/engines/authority-round/src/lib.rs

@@ -1626,6 +1626,10 @@ impl Engine for AuthorityRound {
 		*self.signer.write() = Some(signer);
 	}
 
+	fn clear_signer(&self) {
+		*self.signer.write() = Default::default();
+	}
+
 	fn sign(&self, hash: H256) -> Result<Signature, Error> {
 		Ok(self.signer.read()
 			.as_ref()
@@ -1773,12 +1777,68 @@ mod tests {
 
 		engine.set_signer(Box::new((tap, addr2, "2".into())));
 		if let Seal::Regular(seal) = engine.generate_seal(&b2, &genesis_header) {
+			// FIXME: This branch is unreachable because the call to `generate_seal` above always
+			// returns `Seal::None`. Meanwhile it looks as if the intention of the test writer was
+			// to receive a `Seal::Regular` here. This can be achieved similarly to how it's done in
+			// `generates_seal_iff_sealer_is_set()`, by stepping the engine and issuing a block in
+			// the new step signed by `keccak("0")`.
 			assert!(b2.clone().try_seal(engine, seal).is_ok());
 			// Second proposal is forbidden.
 			assert!(engine.generate_seal(&b2, &genesis_header) == Seal::None);
 		}
 	}
 
+	#[test]
+	fn generates_seal_iff_sealer_is_set() {
+		let tap = Arc::new(AccountProvider::transient_provider());
+		let addr1 = tap.insert_account(keccak("1").into(), &"1".into()).unwrap();
+		let spec = Spec::new_test_round();
+		let engine = &*spec.engine;
+		let genesis_header = spec.genesis_header();
+		let db1 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap();
+		let last_hashes = Arc::new(vec![genesis_header.hash()]);
+		let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header,
+								last_hashes.clone(), addr1, (3141562.into(), 31415620.into()),
+								vec![], false)
+			.unwrap().close_and_lock().unwrap();
+		// Not a signer. A seal cannot be generated.
+		assert!(engine.generate_seal(&b1, &genesis_header) == Seal::None);
+		// Become a signer.
+		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		if let Seal::Regular(seal) = engine.generate_seal(&b1, &genesis_header) {
+			assert!(b1.clone().try_seal(engine, seal).is_ok());
+			// Second proposal is forbidden.
+			assert!(engine.generate_seal(&b1, &genesis_header) == Seal::None);
+		} else {
+			panic!("block 1 not sealed");
+		}
+		// Stop being a signer.
+		engine.clear_signer();
+		// Make a step first and then create a new block in that new step.
+		engine.step();
+		let addr2 = tap.insert_account(keccak("0").into(), &"0".into()).unwrap();
+		let mut header2 = genesis_header.clone();
+		header2.set_number(2);
+		header2.set_author(addr2);
+		header2.set_parent_hash(header2.hash());
+		let db2 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap();
+		let b2 = OpenBlock::new(engine, Default::default(), false, db2, &header2,
+								last_hashes, addr2, (3141562.into(), 31415620.into()),
+								vec![], false)
+			.unwrap().close_and_lock().unwrap();
+		// Not a signer. A seal cannot be generated.
+		assert!(engine.generate_seal(&b2, &header2) == Seal::None);
+		// Become a signer once more.
+		engine.set_signer(Box::new((tap, addr2, "0".into())));
+		if let Seal::Regular(seal) = engine.generate_seal(&b2, &header2) {
+			assert!(b2.clone().try_seal(engine, seal).is_ok());
+			// Second proposal is forbidden.
+			assert!(engine.generate_seal(&b2, &header2) == Seal::None);
+		} else {
+			panic!("block 2 not sealed");
+		}
+	}
+
 	#[test]
 	fn checks_difficulty_in_generate_seal() {
 		let tap = Arc::new(AccountProvider::transient_provider());

ethcore/engines/basic-authority/src/lib.rs

@@ -198,6 +198,10 @@ impl Engine for BasicAuthority {
 		*self.signer.write() = Some(signer);
 	}
 
+	fn clear_signer(&self) {
+		*self.signer.write() = Default::default();
+	}
+
 	fn sign(&self, hash: H256) -> Result<Signature, Error> {
 		Ok(self.signer.read()
 			.as_ref()
@@ -289,5 +293,7 @@ mod tests {
 		assert_eq!(SealingState::NotReady, engine.sealing_state());
 		engine.set_signer(Box::new((Arc::new(tap), authority, "".into())));
 		assert_eq!(SealingState::Ready, engine.sealing_state());
+		engine.clear_signer();
+		assert_eq!(SealingState::NotReady, engine.sealing_state());
 	}
 }

ethcore/src/miner/miner.rs

@@ -912,6 +912,14 @@ impl miner::MinerService for Miner {
 		}
 	}
 
+	fn clear_author(&self) {
+		self.params.write().author = Default::default();
+		if self.engine.sealing_state() == SealingState::Ready {
+			self.sealing.lock().enabled = false;
+			self.engine.clear_signer();
+		}
+	}
+
 	fn sensible_gas_price(&self) -> U256 {
 		// 10% above our minimum.
 		self.transaction_queue.current_worst_gas_price() * 110u32 / 100

ethcore/src/miner/mod.rs

@@ -133,6 +133,9 @@ pub trait MinerService : Send + Sync {
 	/// On chains where sealing is done externally (e.g. PoW) we provide only reward beneficiary.
 	fn set_author(&self, author: Author);
 
+	/// Clears the engine signer and stops signing.
+	fn clear_author(&self);
+
 	// Transaction Pool
 
 	/// Imports transactions to transaction queue.

rpc/src/v1/impls/light/parity_set.rs

@@ -75,6 +75,10 @@ impl<F: Fetch> ParitySet for ParitySetClient<F> {
 		Err(errors::light_unimplemented(None))
 	}
 
+	fn clear_engine_signer(&self) -> Result<bool> {
+		Err(errors::light_unimplemented(None))
+	}
+
 	fn set_transactions_limit(&self, _limit: usize) -> Result<bool> {
 		Err(errors::light_unimplemented(None))
 	}

rpc/src/v1/impls/parity_set.rs

@@ -166,6 +166,11 @@ impl<C, M, U, F> ParitySet for ParitySetClient<C, M, U, F> where
 		Ok(true)
 	}
 
+	fn clear_engine_signer(&self) -> Result<bool> {
+		self.miner.clear_author();
+		Ok(true)
+	}
+
 	fn add_reserved_peer(&self, peer: String) -> Result<bool> {
 		match self.net.add_reserved_peer(peer) {
 			Ok(()) => Ok(true),

rpc/src/v1/tests/helpers/miner_service.rs

@@ -134,6 +134,11 @@ impl MinerService for TestMinerService {
 		}
 	}
 
+	fn clear_author(&self) -> Result<(), AccountError> {
+		*self.authoring_params.write() = Default::default();
+		Ok(())
+	}
+
 	fn set_extra_data(&self, extra_data: Bytes) {
 		self.authoring_params.write().extra_data = extra_data;
 	}

rpc/src/v1/traits/parity_set.rs

@@ -57,6 +57,10 @@ pub trait ParitySet {
 	#[rpc(name = "parity_setEngineSignerSecret")]
 	fn set_engine_signer_secret(&self, H256) -> Result<bool>;
 
+	/// Unsets the engine signer account address.
+	#[rpc(name = "parity_clearEngineSigner")]
+	fn clear_engine_signer(&self) -> Result<bool>;
+
 	/// Sets the limits for transaction queue.
 	#[rpc(name = "parity_setTransactionsLimit")]
 	fn set_transactions_limit(&self, usize) -> Result<bool>;