RPC method parity_clearEngineSigner (#114)

Add RPC method parity_clearEngineSigner
Fixes #113

ethcore/src/block.rs

@@ -60,7 +60,7 @@ use types::receipt::{Receipt, TransactionOutcome};
 /// It's a bit like a Vec<Transaction>, except that whenever a transaction is pushed, we execute it and
 /// maintain the system `state()`. We also archive execution receipts in preparation for later block creation.
 pub struct OpenBlock<'x> {
-	pub(crate) block: ExecutedBlock,
+	block: ExecutedBlock,
 	engine: &'x EthEngine,
 }
 

ethcore/src/engines/authority_round/mod.rs

@@ -1582,6 +1582,10 @@ impl Engine<EthereumMachine> 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()
@@ -1720,12 +1724,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, &mut Vec::new().into_iter())
+			.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(b1.header.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, &mut Vec::new().into_iter())
+			.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/src/engines/basic_authority.rs

@@ -193,6 +193,10 @@ impl Engine<EthereumMachine> 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()
@@ -280,5 +284,7 @@ mod tests {
 		assert!(!engine.seals_internally().unwrap());
 		engine.set_signer(Box::new((Arc::new(tap), authority, "".into())));
 		assert!(engine.seals_internally().unwrap());
+		engine.clear_signer();
+		assert!(!engine.seals_internally().unwrap());
 	}
 }

ethcore/src/engines/mod.rs

@@ -434,6 +434,9 @@ pub trait Engine<M: Machine>: Sync + Send {
 	/// Register a component which signs consensus messages.
 	fn set_signer(&self, _signer: Box<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, M::Error> { unimplemented!() }
 

ethcore/src/miner/miner.rs

@@ -441,7 +441,7 @@ impl Miner {
 					};
 
 					// Before adding from the queue to the new block, give the engine a chance to add transactions.
-					engine_pending = match self.engine.on_prepare_block(&open_block.block) {
+					engine_pending = match self.engine.on_prepare_block(&open_block) {
 						Ok(transactions) => transactions,
 						Err(err) => {
 							error!(target: "miner", "Failed to prepare engine transactions for new block: {:?}. \
@@ -893,6 +893,14 @@ impl miner::MinerService for Miner {
 		}
 	}
 
+	fn clear_author(&self) {
+		self.params.write().author = Default::default();
+		if self.engine.seals_internally().is_some() {
+			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

@@ -165,6 +165,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

@@ -130,6 +130,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>;