grandpa: remove duplicate function to fetch local authority id (#7359)

* grandpa: remove duplicate authority_id function

* grandpa: rename is_voter to local_authority_id

* grandpa: cleanup rebuild_voter telemetry event

substrate/client/finality-grandpa/src/environment.rs

@@ -724,11 +724,11 @@ where
 		let prevote_timer = Delay::new(self.config.gossip_duration * 2);
 		let precommit_timer = Delay::new(self.config.gossip_duration * 4);
 
-		let local_key = crate::is_voter(&self.voters, self.config.keystore.as_ref());
+		let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref());
 
 		let has_voted = match self.voter_set_state.has_voted(round) {
 			HasVoted::Yes(id, vote) => {
-				if local_key.as_ref().map(|k| k == &id).unwrap_or(false) {
+				if local_id.as_ref().map(|k| k == &id).unwrap_or(false) {
 					HasVoted::Yes(id, vote)
 				} else {
 					HasVoted::No
@@ -739,7 +739,7 @@ where
 
 		// we can only sign when we have a local key in the authority set
 		// and we have a reference to the keystore.
-		let keystore = match (local_key.as_ref(), self.config.keystore.as_ref()) {
+		let keystore = match (local_id.as_ref(), self.config.keystore.as_ref()) {
 			(Some(id), Some(keystore)) => Some((id.clone(), keystore.clone()).into()),
 			_ => None,
 		};
@@ -767,7 +767,7 @@ where
 		let outgoing = Box::pin(outgoing.sink_err_into());
 
 		voter::RoundData {
-			voter_id: local_key,
+			voter_id: local_id,
 			prevote_timer: Box::pin(prevote_timer.map(Ok)),
 			precommit_timer: Box::pin(precommit_timer.map(Ok)),
 			incoming,
@@ -776,7 +776,7 @@ where
 	}
 
 	fn proposed(&self, round: RoundNumber, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> {
-		let local_id = crate::is_voter(&self.voters, self.config.keystore.as_ref());
+		let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref());
 
 		let local_id = match local_id {
 			Some(id) => id,
@@ -815,7 +815,7 @@ where
 	}
 
 	fn prevoted(&self, round: RoundNumber, prevote: Prevote<Block>) -> Result<(), Self::Error> {
-		let local_id = crate::is_voter(&self.voters, self.config.keystore.as_ref());
+		let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref());
 
 		let local_id = match local_id {
 			Some(id) => id,
@@ -876,7 +876,7 @@ where
 		round: RoundNumber,
 		precommit: Precommit<Block>,
 	) -> Result<(), Self::Error> {
-		let local_id = crate::is_voter(&self.voters, self.config.keystore.as_ref());
+		let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref());
 
 		let local_id = match local_id {
 			Some(id) => id,

substrate/client/finality-grandpa/src/lib.rs

@@ -624,7 +624,7 @@ fn global_communication<BE, Block: BlockT, C, N>(
 	N: NetworkT<Block>,
 	NumberFor<Block>: BlockNumberOps,
 {
-	let is_voter = is_voter(voters, keystore).is_some();
+	let is_voter = local_authority_id(voters, keystore).is_some();
 
 	// verification stream
 	let (global_in, global_out) = network.global_communication(
@@ -671,7 +671,8 @@ pub struct GrandpaParams<Block: BlockT, C, N, SC, VR> {
 /// block import worker that has already been instantiated with `block_import`.
 pub fn run_grandpa_voter<Block: BlockT, BE: 'static, C, N, SC, VR>(
 	grandpa_params: GrandpaParams<Block, C, N, SC, VR>,
-) -> sp_blockchain::Result<impl Future<Output = ()> + Unpin + Send + 'static> where
+) -> sp_blockchain::Result<impl Future<Output = ()> + Unpin + Send + 'static>
+where
 	Block::Hash: Ord,
 	BE: Backend<Block> + 'static,
 	N: NetworkT<Block> + Send + Sync + Clone + 'static,
@@ -719,21 +720,27 @@ pub fn run_grandpa_voter<Block: BlockT, BE: 'static, C, N, SC, VR>(
 		let authorities = persistent_data.authority_set.clone();
 		let events = telemetry_on_connect
 			.for_each(move |_| {
-				let curr = authorities.current_authorities();
-				let mut auths = curr.iter().map(|(p, _)| p);
-				let maybe_authority_id = authority_id(&mut auths, conf.keystore.as_ref())
+				let current_authorities = authorities.current_authorities();
+				let set_id = authorities.set_id();
+				let authority_id = local_authority_id(&current_authorities, conf.keystore.as_ref())
 					.unwrap_or_default();
 
-				telemetry!(CONSENSUS_INFO; "afg.authority_set";
-					"authority_id" => maybe_authority_id.to_string(),
-					"authority_set_id" => ?authorities.set_id(),
-					"authorities" => {
-						let authorities: Vec<String> = curr.iter()
-							.map(|(id, _)| id.to_string()).collect();
-						serde_json::to_string(&authorities)
-							.expect("authorities is always at least an empty vector; elements are always of type string")
-					}
+				let authorities = current_authorities
+					.iter()
+					.map(|(id, _)| id.to_string())
+					.collect::<Vec<_>>();
+
+				let authorities = serde_json::to_string(&authorities).expect(
+					"authorities is always at least an empty vector; \
+					 elements are always of type string",
 				);
+
+				telemetry!(CONSENSUS_INFO; "afg.authority_set";
+					"authority_id" => authority_id.to_string(),
+					"authority_set_id" => ?set_id,
+					"authorities" => authorities,
+				);
+
 				future::ready(())
 			});
 		future::Either::Left(events)
@@ -864,7 +871,7 @@ where
 	fn rebuild_voter(&mut self) {
 		debug!(target: "afg", "{}: Starting new voter with set ID {}", self.env.config.name(), self.env.set_id);
 
-		let authority_id = is_voter(&self.env.voters, self.env.config.keystore.as_ref())
+		let authority_id = local_authority_id(&self.env.voters, self.env.config.keystore.as_ref())
 			.unwrap_or_default();
 
 		telemetry!(CONSENSUS_DEBUG; "afg.starting_new_voter";
@@ -874,17 +881,24 @@ where
 		);
 
 		let chain_info = self.env.client.info();
+
+		let authorities = self
+			.env
+			.voters
+			.iter()
+			.map(|(id, _)| id.to_string())
+			.collect::<Vec<_>>();
+
+		let authorities = serde_json::to_string(&authorities).expect(
+			"authorities is always at least an empty vector; elements are always of type string",
+		);
+
 		telemetry!(CONSENSUS_INFO; "afg.authority_set";
 			"number" => ?chain_info.finalized_number,
 			"hash" => ?chain_info.finalized_hash,
 			"authority_id" => authority_id.to_string(),
 			"authority_set_id" => ?self.env.set_id,
-			"authorities" => {
-				let authorities: Vec<String> = self.env.voters
-					.iter().map(|(id, _)| id.to_string()).collect();
-				serde_json::to_string(&authorities)
-					.expect("authorities is always at least an empty vector; elements are always of type string")
-			},
+			"authorities" => authorities,
 		);
 
 		match &*self.env.voter_set_state.read() {
@@ -1078,12 +1092,12 @@ where
 	Ok(())
 }
 
-/// Checks if this node is a voter in the given voter set.
-///
-/// Returns the key pair of the node that is being used in the current voter set or `None`.
-fn is_voter(
-	voters: &Arc<VoterSet<AuthorityId>>,
-	keystore: Option<& SyncCryptoStorePtr>,
+/// Checks if this node has any available keys in the keystore for any authority id in the given
+/// voter set.  Returns the authority id for which keys are available, or `None` if no keys are
+/// available.
+fn local_authority_id(
+	voters: &VoterSet<AuthorityId>,
+	keystore: Option<&SyncCryptoStorePtr>,
 ) -> Option<AuthorityId> {
 	match keystore {
 		Some(keystore) => voters
@@ -1095,20 +1109,3 @@ fn is_voter(
 		None => None,
 	}
 }
-
-/// Returns the authority id of this node, if available.
-fn authority_id<'a, I>(
-	authorities: &mut I,
-	keystore: Option<&SyncCryptoStorePtr>,
-) -> Option<AuthorityId> where
-	I: Iterator<Item = &'a AuthorityId>,
-{
-	match keystore {
-		Some(keystore) => {
-			authorities
-				.find(|p| SyncCryptoStore::has_keys(&**keystore, &[(p.to_raw_vec(), AuthorityId::ID)]))
-				.cloned()
-		},
-		None => None,
-	}
-}