Fix minor clippy lints in grandpa (#5988)

* grandpa: fix clippy lints about identity conversions

* grandpa: fix clippy lints about unwrap_or_default

* grandpa: fix clippy lints about explicit return

* grandpa: fix clippy lints about unnecessary intermediary

* grandpa: fix clippy lints about to_string

* grandpa: fix clippy lints about unused imports

* grandpa: fix clippy lints about increments

* grandpa: fix clippy lints about unnecessary matches

* grandpa: fix clippy lints about struct arguments

* Fix clippy::redundant_clone

* Fix clippy::clone_on_copy

* Fix clippy::or_fun_call

* Fix clippy::identity_conversion

substrate/client/finality-grandpa/rpc/src/error.rs

@@ -33,7 +33,7 @@ pub enum Error {
 impl From<Error> for jsonrpc_core::Error {
 	fn from(error: Error) -> Self {
 		jsonrpc_core::Error {
-			message: format!("{}", error).into(),
+			message: format!("{}", error),
 			code: jsonrpc_core::ErrorCode::ServerError(NOT_READY_ERROR_CODE),
 			data: None,
 		}

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

@@ -229,8 +229,8 @@ where
 			   (&number, &hash), pending.delay);
 
 		self.pending_standard_changes.import(
-			hash.clone(),
-			number.clone(),
+			hash,
+			number,
 			pending,
 			is_descendent_of,
 		)?;

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

@@ -328,7 +328,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 		}
 		Some(other) => return Err(ClientError::Backend(
 			format!("Unsupported GRANDPA DB version: {:?}", other)
-		).into()),
+		)),
 	}
 
 	// genesis.
@@ -336,7 +336,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 		from genesis on what appears to be first startup.");
 
 	let genesis_authorities = genesis_authorities()?;
-	let genesis_set = AuthoritySet::genesis(genesis_authorities.clone())
+	let genesis_set = AuthoritySet::genesis(genesis_authorities)
 		.expect("genesis authorities is non-empty; all weights are non-zero; qed.");
 	let state = make_genesis_round();
 	let base = state.prevote_ghost

substrate/client/finality-grandpa/src/communication/gossip.rs

@@ -887,7 +887,7 @@ impl<Block: BlockT> Inner<Block> {
 				// any catch up requests until we import this one (either with a
 				// success or failure).
 				self.pending_catch_up = PendingCatchUp::Processing {
-					instant: instant.clone(),
+					instant: *instant,
 				};
 
 				// always discard catch up messages, they're point-to-point
@@ -1281,7 +1281,7 @@ impl<Block: BlockT> GossipValidator<Block> {
 			inner: parking_lot::RwLock::new(Inner::new(config)),
 			set_state,
 			report_sender: tx,
-			metrics: metrics,
+			metrics,
 		};
 
 		(val, rx)

substrate/client/finality-grandpa/src/communication/mod.rs

@@ -236,16 +236,14 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
 
 		let (neighbor_packet_worker, neighbor_packet_sender) = periodic::NeighborPacketWorker::new();
 
-		let bridge = NetworkBridge {
+		NetworkBridge {
 			service,
 			gossip_engine,
 			validator,
 			neighbor_sender: neighbor_packet_sender,
 			neighbor_packet_worker: Arc::new(Mutex::new(neighbor_packet_worker)),
 			gossip_validator_report_stream: Arc::new(Mutex::new(report_stream)),
-		};
-
-		bridge
+		}
 	}
 
 	/// Note the beginning of a new round to the `GossipValidator`.
@@ -304,7 +302,7 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
 				match decoded {
 					Err(ref e) => {
 						debug!(target: "afg", "Skipping malformed message {:?}: {}", notification, e);
-						return future::ready(None);
+						future::ready(None)
 					}
 					Ok(GossipMessage::Vote(msg)) => {
 						// check signature.
@@ -343,7 +341,7 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
 					}
 					_ => {
 						debug!(target: "afg", "Skipping unknown message type");
-						return future::ready(None);
+						future::ready(None)
 					}
 				}
 			});
@@ -666,7 +664,7 @@ impl<Block: BlockT> Sink<Message<Block>> for OutgoingMessages<Block>
 
 		// when locals exist, sign messages on import
 		if let Some((ref pair, _)) = self.locals {
-			let target_hash = msg.target().0.clone();
+			let target_hash = *(msg.target().0);
 			let signed = sp_finality_grandpa::sign_message(
 				msg,
 				pair,

substrate/client/finality-grandpa/src/communication/periodic.rs

@@ -86,7 +86,7 @@ impl <B: BlockT> Stream for NeighborPacketWorker<B> {
 				this.delay.reset(REBROADCAST_AFTER);
 				this.last = Some((to.clone(), packet.clone()));
 
-				return Poll::Ready(Some((to, GossipMessage::<B>::from(packet.clone()))));
+				return Poll::Ready(Some((to, GossipMessage::<B>::from(packet))));
 			}
 			// Don't return yet, maybe the timer fired.
 			Poll::Pending => {},
@@ -108,6 +108,6 @@ impl <B: BlockT> Stream for NeighborPacketWorker<B> {
 			return Poll::Ready(Some((to.clone(), GossipMessage::<B>::from(packet.clone()))));
 		}
 
-		return Poll::Pending;
+		Poll::Pending
 	}
 }

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

@@ -108,7 +108,7 @@ impl<Block: BlockT> Decode for CompletedRounds<Block> {
 	fn decode<I: parity_scale_codec::Input>(value: &mut I) -> Result<Self, parity_scale_codec::Error> {
 		<(Vec<CompletedRound<Block>>, SetId, Vec<AuthorityId>)>::decode(value)
 			.map(|(rounds, set_id, voters)| CompletedRounds {
-				rounds: rounds.into(),
+				rounds,
 				set_id,
 				voters,
 			})
@@ -248,14 +248,14 @@ impl<Block: BlockT> VoterSetState<Block> {
 	{
 		if let VoterSetState::Live { completed_rounds, current_rounds } = self {
 			if current_rounds.contains_key(&round) {
-				return Ok((completed_rounds, current_rounds));
+				Ok((completed_rounds, current_rounds))
 			} else {
 				let msg = "Voter acting on a live round we are not tracking.";
-				return Err(Error::Safety(msg.to_string()));
+				Err(Error::Safety(msg.to_string()))
 			}
 		} else {
 			let msg = "Voter acting while in paused state.";
-			return Err(Error::Safety(msg.to_string()));
+			Err(Error::Safety(msg.to_string()))
 		}
 	}
 }
@@ -622,7 +622,7 @@ where
 						restricted_number >= base_header.number() &&
 							restricted_number < target_header.number()
 					})
-					.or(Some((target_header.hash(), *target_header.number())))
+					.or_else(|| Some((target_header.hash(), *target_header.number())))
 			},
 			Ok(None) => {
 				debug!(target: "afg", "Encountered error finding best chain containing {:?}: couldn't find target block", block);

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

@@ -183,7 +183,7 @@ impl<B, Block> sc_network::config::FinalityProofProvider<Block> for FinalityProo
 		let request: FinalityProofRequest<Block::Hash> = Decode::decode(&mut &request[..])
 			.map_err(|e| {
 				warn!(target: "afg", "Unable to decode finality proof request: {}", e.what());
-				ClientError::Backend(format!("Invalid finality proof request"))
+				ClientError::Backend("Invalid finality proof request".to_string())
 			})?;
 		match request {
 			FinalityProofRequest::Original(request) => prove_finality::<_, _, GrandpaJustification<Block>>(
@@ -397,7 +397,7 @@ pub(crate) fn prove_finality<Block: BlockT, B: BlockchainBackend<Block>, J>(
 		}
 
 		// else search for the next justification
-		current_number = current_number + One::one();
+		current_number += One::one();
 	}
 
 	if finality_proof.is_empty() {
@@ -513,7 +513,7 @@ fn check_finality_proof_fragment<Block: BlockT, B, J>(
 			new_authorities_proof,
 		)?;
 
-		current_set_id = current_set_id + 1;
+		current_set_id += 1;
 	}
 
 	Ok(AuthoritiesOrEffects::Effects(FinalityEffects {

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

@@ -294,7 +294,7 @@ where
 			}
 		}
 
-		let number = block.header.number().clone();
+		let number = *(block.header.number());
 		let maybe_change = self.check_new_change(
 			&block.header,
 			hash,
@@ -326,7 +326,7 @@ where
 			guard.as_mut().add_pending_change(
 				change,
 				&is_descendent_of,
-			).map_err(|e| ConsensusError::from(ConsensusError::ClientImport(e.to_string())))?;
+			).map_err(|e| ConsensusError::ClientImport(e.to_string()))?;
 		}
 
 		let applied_changes = {
@@ -417,14 +417,14 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block>
 		new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {
 		let hash = block.post_hash();
-		let number = block.header.number().clone();
+		let number = *block.header.number();
 
 		// early exit if block already in chain, otherwise the check for
 		// authority changes will error when trying to re-import a change block
 		match self.inner.status(BlockId::Hash(hash)) {
 			Ok(BlockStatus::InChain) => return Ok(ImportResult::AlreadyInChain),
 			Ok(BlockStatus::Unknown) => {},
-			Err(e) => return Err(ConsensusError::ClientImport(e.to_string()).into()),
+			Err(e) => return Err(ConsensusError::ClientImport(e.to_string())),
 		}
 
 		// on initial sync we will restrict logging under info to avoid spam.
@@ -456,7 +456,7 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block>
 						e,
 					);
 					pending_changes.revert();
-					return Err(ConsensusError::ClientImport(e.to_string()).into());
+					return Err(ConsensusError::ClientImport(e.to_string()));
 				},
 			}
 		};
@@ -466,7 +466,7 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block>
 		// Send the pause signal after import but BEFORE sending a `ChangeAuthorities` message.
 		if do_pause {
 			let _ = self.send_voter_commands.unbounded_send(
-				VoterCommand::Pause(format!("Forced change scheduled after inactivity"))
+				VoterCommand::Pause("Forced change scheduled after inactivity".to_string())
 			);
 		}
 
@@ -633,7 +633,7 @@ where
 		);
 
 		let justification = match justification {
-			Err(e) => return Err(ConsensusError::ClientImport(e.to_string()).into()),
+			Err(e) => return Err(ConsensusError::ClientImport(e.to_string())),
 			Ok(justification) => justification,
 		};
 
@@ -668,7 +668,7 @@ where
 					Error::Client(error) => ConsensusError::ClientImport(error.to_string()),
 					Error::Safety(error) => ConsensusError::ClientImport(error),
 					Error::Timer(error) => ConsensusError::ClientImport(error.to_string()),
-				}.into());
+				});
 			},
 			Ok(_) => {
 				assert!(!enacts_change, "returns Ok when no authority set change should be enacted; qed;");

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

@@ -61,7 +61,7 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 		};
 
 		for signed in commit.precommits.iter() {
-			let mut current_hash = signed.precommit.target_hash.clone();
+			let mut current_hash = signed.precommit.target_hash;
 			loop {
 				if current_hash == commit.target_hash { break; }
 
@@ -71,7 +71,7 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 							return error();
 						}
 
-						let parent_hash = current_header.parent_hash().clone();
+						let parent_hash = *current_header.parent_hash();
 						if votes_ancestries_hashes.insert(current_hash) {
 							votes_ancestries.push(current_header);
 						}
@@ -131,16 +131,16 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 		let mut buf = Vec::new();
 		let mut visited_hashes = HashSet::new();
 		for signed in self.commit.precommits.iter() {
-			if let Err(_) = sp_finality_grandpa::check_message_signature_with_buffer(
+			if sp_finality_grandpa::check_message_signature_with_buffer(
 				&finality_grandpa::Message::Precommit(signed.precommit.clone()),
 				&signed.id,
 				&signed.signature,
 				self.round,
 				set_id,
 				&mut buf,
-			) {
+			).is_err() {
 				return Err(ClientError::BadJustification(
-					"invalid signature for precommit in grandpa justification".to_string()).into());
+					"invalid signature for precommit in grandpa justification".to_string()));
 			}
 
 			if self.commit.target_hash == signed.precommit.target_hash {
@@ -157,7 +157,7 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 				},
 				_ => {
 					return Err(ClientError::BadJustification(
-						"invalid precommit ancestry proof in grandpa justification".to_string()).into());
+						"invalid precommit ancestry proof in grandpa justification".to_string()));
 				},
 			}
 		}
@@ -169,7 +169,7 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 
 		if visited_hashes != ancestry_hashes {
 			return Err(ClientError::BadJustification(
-				"invalid precommit ancestries in grandpa justification with unused headers".to_string()).into());
+				"invalid precommit ancestries in grandpa justification with unused headers".to_string()));
 		}
 
 		Ok(())

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

@@ -74,11 +74,8 @@ use sp_consensus::{SelectChain, BlockImport};
 use sp_core::Pair;
 use sp_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver};
 use sc_telemetry::{telemetry, CONSENSUS_INFO, CONSENSUS_DEBUG};
-use serde_json;
 use parking_lot::RwLock;
 
-use sp_finality_tracker;
-
 use finality_grandpa::Error as GrandpaError;
 use finality_grandpa::{voter, BlockNumberOps, voter_set::VoterSet};
 
@@ -474,7 +471,7 @@ impl<Block: BlockT, E> GenesisAuthoritySetProvider<Block> for Arc<dyn ExecutorPr
 			.and_then(|call_result| {
 				Decode::decode(&mut &call_result[..])
 					.map_err(|err| ClientError::CallResultDecode(
-						"failed to decode GRANDPA authorities set proof".into(), err
+						"failed to decode GRANDPA authorities set proof", err
 					))
 			})
 	}
@@ -649,7 +646,7 @@ fn register_finality_tracker_inherent_data_provider<Block: BlockT, Client>(
 					Ok(info.finalized_number)
 				}
 			}))
-			.map_err(|err| sp_consensus::Error::InherentData(err.into()))
+			.map_err(|err| sp_consensus::Error::InherentData(err))
 	} else {
 		Ok(())
 	}
@@ -731,7 +728,7 @@ pub fn run_grandpa_voter<Block: BlockT, BE: 'static, C, N, SC, VR>(
 				let curr = authorities.current_authorities();
 				let mut auths = curr.iter().map(|(p, _)| p);
 				let maybe_authority_id = authority_id(&mut auths, &conf.keystore)
-					.unwrap_or(Default::default());
+					.unwrap_or_default();
 
 				telemetry!(CONSENSUS_INFO; "afg.authority_set";
 					"authority_id" => maybe_authority_id.to_string(),
@@ -841,7 +838,7 @@ where
 			set_id: persistent_data.authority_set.set_id(),
 			authority_set: persistent_data.authority_set.clone(),
 			consensus_changes: persistent_data.consensus_changes.clone(),
-			voter_set_state: persistent_data.set_state.clone(),
+			voter_set_state: persistent_data.set_state,
 			metrics: metrics.as_ref().map(|m| m.environment.clone()),
 			_phantom: PhantomData,
 		});
@@ -868,7 +865,7 @@ where
 
 		let authority_id = is_voter(&self.env.voters, &self.env.config.keystore)
 			.map(|ap| ap.public())
-			.unwrap_or(Default::default());
+			.unwrap_or_default();
 
 		telemetry!(CONSENSUS_DEBUG; "afg.starting_new_voter";
 			"name" => ?self.env.config.name(),
@@ -914,12 +911,12 @@ where
 					global_comms,
 					last_completed_round.number,
 					last_completed_round.votes.clone(),
-					last_completed_round.base.clone(),
+					last_completed_round.base,
 					last_finalized,
 				);
 
 				// Repoint shared_voter_state so that the RPC endpoint can query the state
-				if let None = self.shared_voter_state.reset(voter.voter_state()) {
+				if self.shared_voter_state.reset(voter.voter_state()).is_none() {
 					info!(target: "afg",
 						"Timed out trying to update shared GRANDPA voter state. \
 						RPC endpoints may return stale data."

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

@@ -169,7 +169,7 @@ impl<BE, Block: BlockT, Client> FinalityProofImport<Block>
 			if *pending_number > chain_info.finalized_number
 				&& *pending_number <= chain_info.best_number
 			{
-				out.push((pending_hash.clone(), *pending_number));
+				out.push((*pending_hash, *pending_number));
 			}
 		}
 
@@ -253,7 +253,7 @@ fn do_import_block<B, C, Block: BlockT, J>(
 		J: ProvableJustification<Block::Header>,
 {
 	let hash = block.post_hash();
-	let number = block.header.number().clone();
+	let number = *block.header.number();
 
 	// we don't want to finalize on `inner.import_block`
 	let justification = block.justification.take();
@@ -263,7 +263,7 @@ fn do_import_block<B, C, Block: BlockT, J>(
 	let mut imported_aux = match import_result {
 		Ok(ImportResult::Imported(aux)) => aux,
 		Ok(r) => return Ok(r),
-		Err(e) => return Err(ConsensusError::ClientImport(e.to_string()).into()),
+		Err(e) => return Err(ConsensusError::ClientImport(e.to_string())),
 	};
 
 	match justification {
@@ -435,7 +435,7 @@ fn do_import_justification<B, C, Block: BlockT, J>(
 				hash,
 			);
 
-			return Err(ConsensusError::ClientImport(e.to_string()).into());
+			return Err(ConsensusError::ClientImport(e.to_string()));
 		},
 		Ok(justification) => {
 			trace!(

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

@@ -111,7 +111,7 @@ fn grandpa_observer<BE, Block: BlockT, Client, S, F>(
 			Err(e) => return future::err(e.into()),
 		};
 
-		if let Some(_) = validation_result.ghost() {
+		if validation_result.ghost().is_some() {
 			let finalized_hash = commit.target_hash;
 			let finalized_number = commit.target_number;
 
@@ -189,7 +189,7 @@ where
 		client,
 		network,
 		persistent_data,
-		config.keystore.clone(),
+		config.keystore,
 		voter_commands_rx
 	);
 

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

@@ -258,7 +258,7 @@ impl<Block, BStatus, BSyncRequester, I, M> Stream for UntilImported<Block, BStat
 				Poll::Ready(Some(notification)) => {
 					// new block imported. queue up all messages tied to that hash.
 					if let Some((_, _, messages)) = this.pending.remove(&notification.hash) {
-						let canon_number = notification.header.number().clone();
+						let canon_number = *notification.header.number();
 						let ready_messages = messages.into_iter()
 							.filter_map(|m| m.wait_completed(canon_number));
 
@@ -359,7 +359,7 @@ impl<Block: BlockT> BlockUntilImported<Block> for SignedMessage<Block> {
 			}
 		}
 
-		return Ok(DiscardWaitOrReady::Wait(vec![(target_hash, target_number, msg)]))
+		Ok(DiscardWaitOrReady::Wait(vec![(target_hash, target_number, msg)]))
 	}
 
 	fn wait_completed(self, canon_number: NumberFor<Block>) -> Option<Self::Blocked> {
@@ -430,7 +430,7 @@ impl<Block: BlockT> BlockUntilImported<Block> for BlockGlobalMessage<Block> {
 			let mut query_known = |target_hash, perceived_number| -> Result<bool, Error> {
 				// check integrity: all votes for same hash have same number.
 				let canon_number = match checked_hashes.entry(target_hash) {
-					Entry::Occupied(entry) => entry.get().number().clone(),
+					Entry::Occupied(entry) => *entry.get().number(),
 					Entry::Vacant(entry) => {
 						if let Some(number) = status_check.block_number(target_hash)? {
 							entry.insert(KnownOrUnknown::Known(number));