RPC method for clearing the engine signer

ethcore/engine/src/engine.rs

@@ -293,7 +293,7 @@ pub trait Engine: Sync + Send {
 	fn handle_message(&self, _message: &[u8]) -> Result<(), EngineError> { Err(EngineError::UnexpectedMessage) }
 
 	/// Register a component which signs consensus messages.
-	fn set_signer(&self, _signer: Box<dyn EngineSigner>) {}
+	fn set_signer(&self, _signer: Option<Box<dyn EngineSigner>>) {}
 
 	/// 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

@@ -1622,8 +1622,8 @@ impl Engine for AuthorityRound {
 		self.validators.register_client(client);
 	}
 
-	fn set_signer(&self, signer: Box<dyn EngineSigner>) {
-		*self.signer.write() = Some(signer);
+	fn set_signer(&self, signer: Option<Box<dyn EngineSigner>>) {
+		*self.signer.write() = signer;
 	}
 
 	fn sign(&self, hash: H256) -> Result<Signature, Error> {
@@ -1751,31 +1751,72 @@ mod tests {
 	fn generates_seal_and_does_not_double_propose() {
 		let tap = Arc::new(AccountProvider::transient_provider());
 		let addr1 = tap.insert_account(keccak("1").into(), &"1".into()).unwrap();
-		let addr2 = tap.insert_account(keccak("2").into(), &"2".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 db2 = 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();
 		let b1 = b1.close_and_lock().unwrap();
-		let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes, addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap();
-		let b2 = b2.close_and_lock().unwrap();
 
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(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");
 		}
+	}
 
-		engine.set_signer(Box::new((tap, addr2, "2".into())));
-		if let Seal::Regular(seal) = engine.generate_seal(&b2, &genesis_header) {
+	#[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(Some(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.set_signer(None);
+		// 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(Some(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, &genesis_header) == Seal::None);
+			assert!(engine.generate_seal(&b2, &header2) == Seal::None);
+		} else {
+			panic!("block 2 not sealed");
 		}
 	}
 
@@ -1798,13 +1839,13 @@ mod tests {
 		let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes, addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap();
 		let b2 = b2.close_and_lock().unwrap();
 
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		match engine.generate_seal(&b1, &genesis_header) {
 			Seal::None => panic!("wrong seal"),
 			Seal::Regular(_) => {
 				engine.step();
 
-				engine.set_signer(Box::new((tap.clone(), addr2, "0".into())));
+				engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into()))));
 				match engine.generate_seal(&b2, &genesis_header) {
 					Seal::Regular(_) => panic!("sealed despite wrong difficulty"),
 					Seal::None => {}
@@ -1922,11 +1963,11 @@ mod tests {
 		assert!(aura.verify_block_family(&header, &parent_header).is_ok());
 		assert_eq!(last_benign.load(AtomicOrdering::SeqCst), 0);
 
-		aura.set_signer(Box::new((
+		aura.set_signer(Some(Box::new((
 			Arc::new(AccountProvider::transient_provider()),
 			validator2,
 			"".into(),
-		)));
+		))));
 
 		// Do not report on steps skipped between genesis and first block.
 		header.set_number(1);
@@ -2041,7 +2082,7 @@ mod tests {
 		client.add_notify(notify.clone());
 		engine.register_client(Arc::downgrade(&client) as _);
 
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 
 		let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap();
 		let b1 = b1.close_and_lock().unwrap();
@@ -2085,7 +2126,7 @@ mod tests {
 		let b1 = b1.close_and_lock().unwrap();
 
 		// since the block is empty it isn't sealed and we generate empty steps
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None);
 		engine.step();
 
@@ -2102,9 +2143,9 @@ mod tests {
 		let b2 = b2.close_and_lock().unwrap();
 
 		// we will now seal a block with 1tx and include the accumulated empty step message
-		engine.set_signer(Box::new((tap.clone(), addr2, "0".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into()))));
 		if let Seal::Regular(seal) = engine.generate_seal(&b2, &genesis_header) {
-			engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+			engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 			let empty_step2 = sealed_empty_step(engine, 2, &genesis_header.hash());
 			let empty_steps = ::rlp::encode_list(&vec![empty_step2]);
 
@@ -2138,14 +2179,14 @@ mod tests {
 		let b1 = b1.close_and_lock().unwrap();
 
 		// since the block is empty it isn't sealed and we generate empty steps
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None);
 		engine.step();
 
 		// step 3
 		let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes.clone(), addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap();
 		let b2 = b2.close_and_lock().unwrap();
-		engine.set_signer(Box::new((tap.clone(), addr2, "0".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into()))));
 		assert_eq!(engine.generate_seal(&b2, &genesis_header), Seal::None);
 		engine.step();
 
@@ -2154,10 +2195,10 @@ mod tests {
 		let b3 = OpenBlock::new(engine, Default::default(), false, db3, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap();
 		let b3 = b3.close_and_lock().unwrap();
 
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		if let Seal::Regular(seal) = engine.generate_seal(&b3, &genesis_header) {
 			let empty_step2 = sealed_empty_step(engine, 2, &genesis_header.hash());
-			engine.set_signer(Box::new((tap.clone(), addr2, "0".into())));
+			engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into()))));
 			let empty_step3 = sealed_empty_step(engine, 3, &genesis_header.hash());
 
 			let empty_steps = ::rlp::encode_list(&vec![empty_step2, empty_step3]);
@@ -2188,7 +2229,7 @@ mod tests {
 		let b1 = b1.close_and_lock().unwrap();
 
 		// since the block is empty it isn't sealed and we generate empty steps
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None);
 		engine.step();
 
@@ -2242,7 +2283,7 @@ mod tests {
 		);
 
 		// empty step with valid signature from incorrect proposer for step
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		let empty_steps = vec![sealed_empty_step(engine, 1, &parent_header.hash())];
 		set_empty_steps_seal(&mut header, 2, &signature, &empty_steps);
 
@@ -2252,9 +2293,9 @@ mod tests {
 		);
 
 		// valid empty steps
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		let empty_step2 = sealed_empty_step(engine, 2, &parent_header.hash());
-		engine.set_signer(Box::new((tap.clone(), addr2, "0".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into()))));
 		let empty_step3 = sealed_empty_step(engine, 3, &parent_header.hash());
 
 		let empty_steps = vec![empty_step2, empty_step3];
@@ -2298,7 +2339,7 @@ mod tests {
 		let b1 = b1.close_and_lock().unwrap();
 
 		// since the block is empty it isn't sealed and we generate empty steps
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 		assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None);
 		engine.step();
 
@@ -2335,7 +2376,7 @@ mod tests {
 		let engine = &*spec.engine;
 
 		let addr1 = accounts[0];
-		engine.set_signer(Box::new((tap.clone(), addr1, "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into()))));
 
 		let mut header: Header = Header::default();
 		let empty_step = empty_step(engine, 1, &header.parent_hash());
@@ -2416,7 +2457,7 @@ mod tests {
 		header.set_author(accounts[0]);
 
 		// when
-		engine.set_signer(Box::new((tap.clone(), accounts[1], "0".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), accounts[1], "0".into()))));
 		let empty_steps = vec![
 			sealed_empty_step(&*engine, 1, &parent.hash()),
 			sealed_empty_step(&*engine, 1, &parent.hash()),
@@ -2453,9 +2494,9 @@ mod tests {
 		header.set_author(accounts[0]);
 
 		// when
-		engine.set_signer(Box::new((tap.clone(), accounts[1], "0".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), accounts[1], "0".into()))));
 		let es1 = sealed_empty_step(&*engine, 1, &parent.hash());
-		engine.set_signer(Box::new((tap.clone(), accounts[0], "1".into())));
+		engine.set_signer(Some(Box::new((tap.clone(), accounts[0], "1".into()))));
 		let es2 = sealed_empty_step(&*engine, 2, &parent.hash());
 
 		let mut empty_steps = vec![es2, es1];

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

@@ -194,8 +194,8 @@ impl Engine for BasicAuthority {
 		}
 	}
 
-	fn set_signer(&self, signer: Box<dyn EngineSigner>) {
-		*self.signer.write() = Some(signer);
+	fn set_signer(&self, signer: Option<Box<dyn EngineSigner>>) {
+		*self.signer.write() = signer;
 	}
 
 	fn sign(&self, hash: H256) -> Result<Signature, Error> {
@@ -269,7 +269,7 @@ mod tests {
 
 		let spec = new_test_authority();
 		let engine = &*spec.engine;
-		engine.set_signer(Box::new((Arc::new(tap), addr, "".into())));
+		engine.set_signer(Some(Box::new((Arc::new(tap), addr, "".into()))));
 		let genesis_header = spec.genesis_header();
 		let db = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap();
 		let last_hashes = Arc::new(vec![genesis_header.hash()]);
@@ -287,7 +287,9 @@ mod tests {
 
 		let engine = new_test_authority().engine;
 		assert_eq!(SealingState::NotReady, engine.sealing_state());
-		engine.set_signer(Box::new((Arc::new(tap), authority, "".into())));
+		engine.set_signer(Some(Box::new((Arc::new(tap), authority, "".into()))));
 		assert_eq!(SealingState::Ready, engine.sealing_state());
+		engine.set_signer(None);
+		assert_eq!(SealingState::NotReady, engine.sealing_state());
 	}
 }

ethcore/engines/clique/src/lib.rs

@@ -762,9 +762,13 @@ impl Engine for Clique {
 		}
 	}
 
-	fn set_signer(&self, signer: Box<dyn EngineSigner>) {
-		trace!(target: "engine", "set_signer: {}", signer.address());
-		*self.signer.write() = Some(signer);
+	fn set_signer(&self, signer: Option<Box<dyn EngineSigner>>) {
+		if let Some(signer) = signer.as_ref() {
+			trace!(target: "engine", "set_signer: {:?}", signer.address());
+		} else if let Some(signer) = &*self.signer.read() {
+			trace!(target: "engine", "set_signer: cleared; previous signer: {:?})", signer.address());
+		}
+		*self.signer.write() = signer;
 	}
 
 	fn register_client(&self, client: Weak<dyn EngineClient>) {

ethcore/src/miner/miner.rs

@@ -893,21 +893,38 @@ impl miner::MinerService for Miner {
 		self.params.write().extra_data = extra_data;
 	}
 
-	fn set_author(&self, author: Author) {
-		self.params.write().author = author.address();
-
-		if let Author::Sealer(signer) = author {
-			if self.engine.sealing_state() != SealingState::External {
-				// Enable sealing
-				self.sealing.lock().enabled = true;
-				// --------------------------------------------------------------------------
-				// | NOTE Code below may require author and sealing locks                   |
-				// | (some `Engine`s call `EngineClient.update_sealing()`)                  |
-				// | Make sure to release the locks before calling that method.             |
-				// --------------------------------------------------------------------------
-				self.engine.set_signer(signer);
-			} else {
-				warn!("Setting an EngineSigner while Engine does not require one.");
+	fn set_author<T: Into<Option<Author>>>(&self, author: T) {
+		let author_opt = author.into();
+		self.params.write().author = author_opt.as_ref().map(Author::address).unwrap_or_default();
+
+		match author_opt {
+			Some(Author::Sealer(signer)) => {
+				if self.engine.sealing_state() != SealingState::External {
+					// Enable sealing
+					self.sealing.lock().enabled = true;
+					// --------------------------------------------------------------------------
+					// | NOTE Code below may require author and sealing locks                   |
+					// | (some `Engine`s call `EngineClient.update_sealing()`)                  |
+					// | Make sure to release the locks before calling that method.             |
+					// --------------------------------------------------------------------------
+					self.engine.set_signer(Some(signer));
+				} else {
+					warn!("Setting an EngineSigner while Engine does not require one.");
+				}
+			}
+			Some(Author::External(_address)) => (),
+			None => {
+				// Clear the author.
+				if self.engine.sealing_state() != SealingState::External {
+					// Disable sealing.
+					self.sealing.lock().enabled = false;
+					// --------------------------------------------------------------------------
+					// | NOTE Code below may require author and sealing locks                   |
+					// | (some `Engine`s call `EngineClient.update_sealing()`)                  |
+					// | Make sure to release the locks before calling that method.             |
+					// --------------------------------------------------------------------------
+					self.engine.set_signer(None);
+				}
 			}
 		}
 	}

ethcore/src/miner/mod.rs

@@ -132,7 +132,7 @@ pub trait MinerService : Send + Sync {
 	/// Set info necessary to sign consensus messages and block authoring.
 	///
 	/// On chains where sealing is done externally (e.g. PoW) we provide only reward beneficiary.
-	fn set_author(&self, author: Author);
+	fn set_author<T: Into<Option<Author>>>(&self, author: T);
 
 	// Transaction Pool
 

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.set_author(None);
+		Ok(true)
+	}
+
 	fn add_reserved_peer(&self, peer: String) -> Result<bool> {
 		match self.net.add_reserved_peer(peer) {
 			Ok(()) => Ok(true),