Improve overall performance (#6699)

* Improve overall performance

* Clean up code

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Remove needless ::

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Remove needless ::

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

substrate/client/api/src/in_mem.rs

@@ -124,7 +124,7 @@ impl<Block: BlockT + Clone> Clone for Blockchain<Block> {
 	fn clone(&self) -> Self {
 		let storage = Arc::new(RwLock::new(self.storage.read().clone()));
 		Blockchain {
-			storage: storage.clone(),
+			storage,
 		}
 	}
 }
@@ -155,7 +155,7 @@ impl<Block: BlockT> Blockchain<Block> {
 				aux: HashMap::new(),
 			}));
 		Blockchain {
-			storage: storage.clone(),
+			storage,
 		}
 	}
 
@@ -346,7 +346,7 @@ impl<Block: BlockT> HeaderMetadata<Block> for Blockchain<Block> {
 
 	fn header_metadata(&self, hash: Block::Hash) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
 		self.header(BlockId::hash(hash))?.map(|header| CachedHeaderMetadata::from(&header))
-			.ok_or(sp_blockchain::Error::UnknownBlock(format!("header not found: {}", hash)))
+			.ok_or_else(|| sp_blockchain::Error::UnknownBlock(format!("header not found: {}", hash)))
 	}
 
 	fn insert_header_metadata(&self, _hash: Block::Hash, _metadata: CachedHeaderMetadata<Block>) {

substrate/client/cli/src/config.rs

@@ -158,7 +158,7 @@ pub trait CliConfiguration: Sized {
 	fn database_cache_size(&self) -> Result<Option<usize>> {
 		Ok(self.database_params()
 			.map(|x| x.database_cache_size())
-			.unwrap_or(Default::default()))
+			.unwrap_or_default())
 	}
 
 	/// Get the database backend variant.
@@ -195,7 +195,7 @@ pub trait CliConfiguration: Sized {
 	fn state_cache_size(&self) -> Result<usize> {
 		Ok(self.import_params()
 			.map(|x| x.state_cache_size())
-			.unwrap_or(Default::default()))
+			.unwrap_or_default())
 	}
 
 	/// Get the state cache child ratio (if any).
@@ -212,7 +212,7 @@ pub trait CliConfiguration: Sized {
 	fn pruning(&self, unsafe_pruning: bool, role: &Role) -> Result<PruningMode> {
 		self.pruning_params()
 			.map(|x| x.pruning(unsafe_pruning, role))
-			.unwrap_or(Ok(Default::default()))
+			.unwrap_or_else(|| Ok(Default::default()))
 	}
 
 	/// Get the chain ID (string).
@@ -236,7 +236,7 @@ pub trait CliConfiguration: Sized {
 	fn wasm_method(&self) -> Result<WasmExecutionMethod> {
 		Ok(self.import_params()
 			.map(|x| x.wasm_method())
-			.unwrap_or(Default::default()))
+			.unwrap_or_default())
 	}
 
 	/// Get the execution strategies.
@@ -251,7 +251,7 @@ pub trait CliConfiguration: Sized {
 		Ok(self
 			.import_params()
 			.map(|x| x.execution_strategies(is_dev, is_validator))
-			.unwrap_or(Default::default()))
+			.unwrap_or_default())
 	}
 
 	/// Get the RPC HTTP address (`None` if disabled).
@@ -365,7 +365,7 @@ pub trait CliConfiguration: Sized {
 	fn tracing_targets(&self) -> Result<Option<String>> {
 		Ok(self.import_params()
 			.map(|x| x.tracing_targets())
-			.unwrap_or(Default::default()))
+			.unwrap_or_else(|| Default::default()))
 	}
 
 	/// Get the TracingReceiver value from the current object
@@ -375,7 +375,7 @@ pub trait CliConfiguration: Sized {
 	fn tracing_receiver(&self) -> Result<TracingReceiver> {
 		Ok(self.import_params()
 			.map(|x| x.tracing_receiver())
-			.unwrap_or(Default::default()))
+			.unwrap_or_default())
 	}
 
 	/// Get the node key from the current object
@@ -385,7 +385,7 @@ pub trait CliConfiguration: Sized {
 	fn node_key(&self, net_config_dir: &PathBuf) -> Result<NodeKeyConfig> {
 		self.node_key_params()
 			.map(|x| x.node_key(net_config_dir))
-			.unwrap_or(Ok(Default::default()))
+			.unwrap_or_else(|| Ok(Default::default()))
 	}
 
 	/// Get maximum runtime instances

substrate/client/cli/src/params/import_params.rs

@@ -113,7 +113,7 @@ impl ImportParams {
 				default
 			};
 
-			exec.execution.unwrap_or(strat.unwrap_or(default)).into()
+			exec.execution.unwrap_or_else(|| strat.unwrap_or(default)).into()
 		};
 
 		let default_execution_import_block = if is_validator {

substrate/client/cli/src/params/keystore_params.rs

@@ -94,7 +94,7 @@ impl KeystoreParams {
 		let path = self
 			.keystore_path
 			.clone()
-			.unwrap_or(base_path.join(DEFAULT_KEYSTORE_CONFIG_PATH));
+			.unwrap_or_else(|| base_path.join(DEFAULT_KEYSTORE_CONFIG_PATH));
 
 		Ok(KeystoreConfig::Path { path, password })
 	}

substrate/client/consensus/aura/src/lib.rs

@@ -165,7 +165,7 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
 	CAW: CanAuthorWith<B> + Send,
 {
 	let worker = AuraWorker {
-		client: client.clone(),
+		client,
 		block_import: Arc::new(Mutex::new(block_import)),
 		env,
 		keystore,
@@ -839,7 +839,7 @@ pub fn import_queue<B, I, C, P, S>(
 	initialize_authorities_cache(&*client)?;
 
 	let verifier = AuraVerifier {
-		client: client.clone(),
+		client,
 		inherent_data_providers,
 		phantom: PhantomData,
 	};

substrate/client/db/src/lib.rs

@@ -512,7 +512,7 @@ impl<Block: BlockT> HeaderMetadata<Block> for BlockchainDb<Block> {
 					header_metadata.clone(),
 				);
 				header_metadata
-			}).ok_or(ClientError::UnknownBlock(format!("header not found in db: {}", hash)))
+			}).ok_or_else(|| ClientError::UnknownBlock(format!("header not found in db: {}", hash)))
 		}, Ok)
 	}
 

substrate/client/db/src/light.rs

@@ -200,7 +200,7 @@ impl<Block: BlockT> HeaderMetadata<Block> for LightStorage<Block> {
 					header_metadata.clone(),
 				);
 				header_metadata
-			}).ok_or(ClientError::UnknownBlock(format!("header not found in db: {}", hash)))
+			}).ok_or_else(|| ClientError::UnknownBlock(format!("header not found in db: {}", hash)))
 		}, Ok)
 	}
 

substrate/client/db/src/utils.rs

@@ -181,8 +181,8 @@ pub fn insert_hash_to_key_mapping<N: TryInto<u32>, H: AsRef<[u8]> + Clone>(
 ) -> sp_blockchain::Result<()> {
 	transaction.set_from_vec(
 		key_lookup_col,
-		hash.clone().as_ref(),
-		number_and_hash_to_lookup_key(number, hash)?,
+		hash.as_ref(),
+		number_and_hash_to_lookup_key(number, hash.clone())?,
 	);
 	Ok(())
 }

substrate/client/executor/runtime-test/src/lib.rs

@@ -353,7 +353,7 @@ fn execute_sandboxed(
 				Memory::new() can't return a Error qed"
 			),
 		};
-		env_builder.add_memory("env", "memory", memory.clone());
+		env_builder.add_memory("env", "memory", memory);
 		env_builder
 	};
 

substrate/client/executor/src/native_executor.rs

@@ -336,7 +336,7 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeExecutor<D> {
 						let res = with_externalities_safe(&mut **ext, move || (call)())
 							.and_then(|r| r
 								.map(NativeOrEncoded::Native)
-								.map_err(|s| Error::ApiError(s.to_string()))
+								.map_err(|s| Error::ApiError(s))
 							);
 
 						Ok(res)

substrate/client/executor/wasmi/src/lib.rs

@@ -234,7 +234,6 @@ impl<'a> Sandbox for FunctionExecutor<'a> {
 			table.get(dispatch_thunk_id)
 				.map_err(|_| "dispatch_thunk_idx is out of the table bounds")?
 				.ok_or_else(|| "dispatch_thunk_idx points on an empty table entry")?
-				.clone()
 		};
 
 		let guest_env = match sandbox::GuestEnvironment::decode(&self.sandbox_store, raw_env_def) {

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

@@ -701,8 +701,8 @@ impl<Block: BlockT> Sink<Message<Block>> for OutgoingMessages<Block>
 				keystore.local_id().clone(),
 				self.round,
 				self.set_id,
-			).ok_or(
-				Error::Signing(format!(
+			).ok_or_else(
+				|| Error::Signing(format!(
 					"Failed to sign GRANDPA vote for round {} targetting {:?}", self.round, target_hash
 				))
 			)?;

substrate/client/keystore/src/lib.rs

@@ -310,7 +310,7 @@ impl BareCryptoStore for Store {
 			.fold(Vec::new(), |mut v, k| {
 				v.push(CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()));
 				v.push(CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()));
-				v.push(CryptoTypePublicPair(ecdsa::CRYPTO_ID, k.clone()));
+				v.push(CryptoTypePublicPair(ecdsa::CRYPTO_ID, k));
 				v
 			}))
 	}

substrate/client/network-gossip/src/state_machine.rs

@@ -180,7 +180,7 @@ impl<B: BlockT> ConsensusGossip<B> {
 
 		let validator = self.validator.clone();
 		let mut context = NetworkContext { gossip: self, network };
-		validator.new_peer(&mut context, &who, role.clone());
+		validator.new_peer(&mut context, &who, role);
 	}
 
 	fn register_message_hashed(

substrate/client/network/src/block_requests.rs

@@ -409,7 +409,7 @@ where
 				},
 				body: if get_body {
 					self.chain.block_body(&BlockId::Hash(hash))?
-						.unwrap_or(Vec::new())
+						.unwrap_or_default()
 						.iter_mut()
 						.map(|extrinsic| extrinsic.encode())
 						.collect()
@@ -418,7 +418,7 @@ where
 				},
 				receipt: Vec::new(),
 				message_queue: Vec::new(),
-				justification: justification.unwrap_or(Vec::new()),
+				justification: justification.unwrap_or_default(),
 				is_empty_justification,
 			};
 

substrate/client/network/src/error.rs

@@ -32,7 +32,7 @@ pub enum Error {
 	/// Io error
 	Io(std::io::Error),
 	/// Client error
-	Client(sp_blockchain::Error),
+	Client(Box<sp_blockchain::Error>),
 	/// The same bootnode (based on address) is registered with two different peer ids.
 	#[display(
 		fmt = "The same bootnode (`{}`) is registered with two different peer ids: `{}` and `{}`",

substrate/client/network/src/finality_requests.rs

@@ -206,7 +206,7 @@ where
 		let finality_proof = if let Some(provider) = &self.finality_proof_provider {
 			provider
 				.prove_finality(block_hash, &request.request)?
-				.unwrap_or(Vec::new())
+				.unwrap_or_default()
 		} else {
 			log::error!("Answering a finality proof request while finality provider is empty");
 			return Err(From::from("Empty finality proof provider".to_string()))

substrate/client/network/src/protocol/generic_proto/behaviour.rs

@@ -806,7 +806,7 @@ impl GenericProto {
 			debug!(target: "sub-libp2p", "PSM => Accept({:?}, {:?}): Obsolete incoming,
 				sending back dropped", index, incoming.peer_id);
 			debug!(target: "sub-libp2p", "PSM <= Dropped({:?})", incoming.peer_id);
-			self.peerset.dropped(incoming.peer_id.clone());
+			self.peerset.dropped(incoming.peer_id);
 			return
 		}
 

substrate/client/network/src/protocol/sync/extra_requests.rs

@@ -141,7 +141,7 @@ impl<B: BlockT> ExtraRequests<B> {
 					request,
 				);
 			}
-			self.failed_requests.entry(request).or_insert(Vec::new()).push((who, Instant::now()));
+			self.failed_requests.entry(request).or_default().push((who, Instant::now()));
 			self.pending_requests.push_front(request);
 		} else {
 			trace!(target: "sync", "No active {} request to {:?}",

substrate/client/network/test/src/lib.rs

@@ -678,7 +678,7 @@ pub trait TestNetFactory: Sized {
 			protocol_id: ProtocolId::from(&b"test-protocol-name"[..]),
 			import_queue,
 			block_announce_validator: config.block_announce_validator
-				.unwrap_or(Box::new(DefaultBlockAnnounceValidator)),
+				.unwrap_or_else(|| Box::new(DefaultBlockAnnounceValidator)),
 			metrics_registry: None,
 		}).unwrap();
 

substrate/client/rpc/src/state/state_light.rs

@@ -539,7 +539,7 @@ fn resolve_header<Block: BlockT, F: Fetcher<Block>>(
 
 	maybe_header.then(move |result|
 		ready(result.and_then(|maybe_header|
-			maybe_header.ok_or(ClientError::UnknownBlock(format!("{}", block)))
+			maybe_header.ok_or_else(|| ClientError::UnknownBlock(format!("{}", block)))
 		).map_err(client_err)),
 	)
 }

substrate/client/service/src/builder.rs

@@ -438,7 +438,7 @@ impl ServiceBuilder<(), (), (), (), (), (), (), (), (), (), ()> {
 			backend,
 			task_manager,
 			keystore,
-			fetcher: Some(fetcher.clone()),
+			fetcher: Some(fetcher),
 			select_chain: None,
 			import_queue: (),
 			finality_proof_request_builder: None,
@@ -1286,7 +1286,7 @@ fn gen_handler<TBl, TBackend, TExPool, TRpc, TCl>(
 			client.clone(),
 			subscriptions.clone(),
 			remote_backend.clone(),
-			on_demand.clone()
+			on_demand,
 		);
 		(chain, state, child_state)
 
@@ -1298,15 +1298,15 @@ fn gen_handler<TBl, TBackend, TExPool, TRpc, TCl>(
 	};
 
 	let author = sc_rpc::author::Author::new(
-		client.clone(),
-		transaction_pool.clone(),
+		client,
+		transaction_pool,
 		subscriptions,
-		keystore.clone(),
+		keystore,
 		deny_unsafe,
 	);
-	let system = system::System::new(system_info, system_rpc_tx.clone(), deny_unsafe);
+	let system = system::System::new(system_info, system_rpc_tx, deny_unsafe);
 
-	let maybe_offchain_rpc = offchain_storage.clone()
+	let maybe_offchain_rpc = offchain_storage
 	.map(|storage| {
 		let offchain = sc_rpc::offchain::Offchain::new(storage, deny_unsafe);
 		// FIXME: Use plain Option (don't collect into HashMap) when we upgrade to jsonrpc 14.1
@@ -1357,7 +1357,7 @@ fn build_network<TBl, TExPool, TImpQu, TCl>(
 {
 	let transaction_pool_adapter = Arc::new(TransactionPoolAdapter {
 		imports_external_transactions: !matches!(config.role, Role::Light),
-		pool: transaction_pool.clone(),
+		pool: transaction_pool,
 		client: client.clone(),
 	});
 
@@ -1391,8 +1391,8 @@ fn build_network<TBl, TExPool, TImpQu, TCl>(
 		chain: client.clone(),
 		finality_proof_provider,
 		finality_proof_request_builder,
-		on_demand: on_demand.clone(),
-		transaction_pool: transaction_pool_adapter.clone() as _,
+		on_demand: on_demand,
+		transaction_pool: transaction_pool_adapter as _,
 		import_queue: Box::new(import_queue),
 		protocol_id,
 		block_announce_validator,
@@ -1407,7 +1407,7 @@ fn build_network<TBl, TExPool, TImpQu, TCl>(
 	let future = build_network_future(
 		config.role.clone(),
 		network_mut,
-		client.clone(),
+		client,
 		network_status_sinks.clone(),
 		system_rpc_rx,
 		has_bootnodes,

substrate/client/service/src/client/block_rules.rs

@@ -52,8 +52,8 @@ impl<B: BlockT> BlockRules<B> {
 		bad_blocks: BadBlocks<B>,
 	) -> Self {
 		Self {
-			bad: bad_blocks.unwrap_or(HashSet::new()),
-			forks: fork_blocks.unwrap_or(vec![]).into_iter().collect(),
+			bad: bad_blocks.unwrap_or_else(|| HashSet::new()),
+			forks: fork_blocks.unwrap_or_else(|| vec![]).into_iter().collect(),
 		}
 	}
 

substrate/client/service/test/src/lib.rs

@@ -518,7 +518,7 @@ pub fn sync<G, E, Fb, F, Lb, L, B, ExF, U>(
 	let temp = tempdir_with_prefix("substrate-sync-test");
 	let mut network = TestNet::new(
 		&temp,
-		spec.clone(),
+		spec,
 		(0..NUM_FULL_NODES).map(|_| { |cfg| full_builder(cfg) }),
 		(0..NUM_LIGHT_NODES).map(|_| { |cfg| light_builder(cfg) }),
 		// Note: this iterator is empty but we can't just use `iter::empty()`, otherwise
@@ -592,7 +592,7 @@ pub fn consensus<G, E, Fb, F, Lb, L>(
 	let temp = tempdir_with_prefix("substrate-consensus-test");
 	let mut network = TestNet::new(
 		&temp,
-		spec.clone(),
+		spec,
 		(0..NUM_FULL_NODES / 2).map(|_| { |cfg| full_builder(cfg).map(|s| (s, ())) }),
 		(0..NUM_LIGHT_NODES / 2).map(|_| { |cfg| light_builder(cfg) }),
 		authorities.into_iter().map(|key| (key, { |cfg| full_builder(cfg).map(|s| (s, ())) })),

substrate/client/transaction-pool/graph/src/base_pool.rs

@@ -278,7 +278,7 @@ impl<Hash: hash::Hash + Member + Serialize, Ex: std::fmt::Debug> BasePool<Hash,
 		tx: Transaction<Hash, Ex>,
 	) -> error::Result<Imported<Hash, Ex>> {
 		if self.is_imported(&tx.hash) {
-			return Err(error::Error::AlreadyImported(Box::new(tx.hash.clone())))
+			return Err(error::Error::AlreadyImported(Box::new(tx.hash)))
 		}
 
 		let tx = WaitingTransaction::new(

substrate/client/transaction-pool/graph/src/ready.rs

@@ -538,7 +538,7 @@ impl<Hash: hash::Hash + Member, Ex> Iterator for BestIterator<Hash, Ex> {
 				}
 			}
 
-			return Some(best.transaction.clone())
+			return Some(best.transaction)
 		}
 	}
 }

substrate/client/transaction-pool/src/api.rs

@@ -305,7 +305,7 @@ impl<Client, F, Block> sc_transaction_graph::ChainApi for
 
 	fn block_body(&self, id: &BlockId<Self::Block>) -> Self::BodyFuture {
 		let header = self.client.header(*id)
-			.and_then(|h| h.ok_or(sp_blockchain::Error::UnknownBlock(format!("{}", id))));
+			.and_then(|h| h.ok_or_else(|| sp_blockchain::Error::UnknownBlock(format!("{}", id))));
 		let header = match header {
 			Ok(header) => header,
 			Err(err) => {