Remove &mut constraint from BlockImport API

bin/node/executor/tests/basic.rs

@@ -810,7 +810,7 @@ fn should_import_block_with_test_client() {
 		sp_consensus::BlockOrigin,
 	};
 
-	let mut client = TestClientBuilder::new().build();
+	let client = TestClientBuilder::new().build();
 	let block1 = changes_trie_block();
 	let block_data = block1.0;
 	let block = node_primitives::Block::decode(&mut &block_data[..]).unwrap();

client/basic-authorship/src/basic_authorship.rs

@@ -516,7 +516,7 @@ mod tests {
 	#[test]
 	fn should_not_remove_invalid_transactions_when_skipping() {
 		// given
-		let mut client = Arc::new(substrate_test_runtime_client::new());
+		let client = Arc::new(substrate_test_runtime_client::new());
 		let txpool = Arc::new(
 			BasicPool::new(
 				Default::default(),

client/consensus/aura/src/lib.rs

@@ -774,14 +774,14 @@ impl<Block: BlockT, C, I, P> BlockImport<Block> for AuraBlockImport<Block, C, I,
 	type Transaction = sp_api::TransactionFor<C, Block>;
 
 	fn check_block(
-		&mut self,
+		&self,
 		block: BlockCheckParams<Block>,
 	) -> Result<ImportResult, Self::Error> {
 		self.inner.check_block(block).map_err(Into::into)
 	}
 
 	fn import_block(
-		&mut self,
+		&self,
 		block: BlockImportParams<Block, Self::Transaction>,
 		new_cache: HashMap<CacheKeyId, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {

client/consensus/babe/src/lib.rs

@@ -966,7 +966,7 @@ impl<Block, Client, Inner> BlockImport<Block> for BabeBlockImport<Block, Client,
 	type Transaction = sp_api::TransactionFor<Client, Block>;
 
 	fn import_block(
-		&mut self,
+		&self,
 		mut block: BlockImportParams<Block, Self::Transaction>,
 		new_cache: HashMap<CacheKeyId, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {
@@ -1199,7 +1199,7 @@ impl<Block, Client, Inner> BlockImport<Block> for BabeBlockImport<Block, Client,
 	}
 
 	fn check_block(
-		&mut self,
+		&self,
 		block: BlockCheckParams<Block>,
 	) -> Result<ImportResult, Self::Error> {
 		self.inner.check_block(block).map_err(Into::into)

client/consensus/babe/src/tests.rs

@@ -192,15 +192,15 @@ impl<B: BlockImport<TestBlock>> BlockImport<TestBlock> for PanickingBlockImport<
 	type Transaction = B::Transaction;
 
 	fn import_block(
-		&mut self,
+		&self,
 		block: BlockImportParams<TestBlock, Self::Transaction>,
 		new_cache: HashMap<CacheKeyId, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {
 		Ok(self.0.import_block(block, new_cache).expect("importing block failed"))
 	}
 
 	fn check_block(
-		&mut self,
+		&self,
 		block: BlockCheckParams<TestBlock>,
 	) -> Result<ImportResult, Self::Error> {
 		Ok(self.0.check_block(block).expect("checking block failed"))

client/consensus/pow/src/lib.rs

@@ -284,14 +284,14 @@ impl<B, I, C, S, Algorithm> BlockImport<B> for PowBlockImport<B, I, C, S, Algori
 	type Transaction = sp_api::TransactionFor<C, B>;
 
 	fn check_block(
-		&mut self,
+		&self,
 		block: BlockCheckParams<B>,
 	) -> Result<ImportResult, Self::Error> {
 		self.inner.check_block(block).map_err(Into::into)
 	}
 
 	fn import_block(
-		&mut self,
+		&self,
 		mut block: BlockImportParams<B, Self::Transaction>,
 		new_cache: HashMap<CacheKeyId, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {

client/finality-grandpa/src/import.rs

@@ -23,9 +23,8 @@ use parity_scale_codec::Encode;
 use parking_lot::RwLockWriteGuard;
 
 use sp_blockchain::{BlockStatus, well_known_cache_keys};
-use sc_client_api::{backend::Backend, utils::is_descendent_of};
+use sc_client_api::{backend::Backend, utils::is_descendent_of, TransactionFor};
 use sp_utils::mpsc::TracingUnboundedSender;
-use sp_api::{TransactionFor};
 
 use sp_consensus::{
 	BlockImport, Error as ConsensusError,
@@ -255,7 +254,7 @@ where
 
 	fn make_authorities_changes(
 		&self,
-		block: &mut BlockImportParams<Block, TransactionFor<Client, Block>>,
+		block: &mut BlockImportParams<Block, TransactionFor<BE, Block>>,
 		hash: Block::Hash,
 		initial_sync: bool,
 	) -> Result<PendingSetChanges<Block>, ConsensusError> {
@@ -406,15 +405,14 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block>
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
 		DigestFor<Block>: Encode,
 		BE: Backend<Block>,
-		Client: crate::ClientForGrandpa<Block, BE>,
-		for<'a> &'a Client:
-			BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<Client, Block>>,
+		Client: crate::ClientForGrandpa<Block, BE> +
+			BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>>,
 {
 	type Error = ConsensusError;
-	type Transaction = TransactionFor<Client, Block>;
+	type Transaction = TransactionFor<BE, Block>;
 
 	fn import_block(
-		&mut self,
+		&self,
 		mut block: BlockImportParams<Block, Self::Transaction>,
 		new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {
@@ -437,7 +435,7 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block>
 		// we don't want to finalize on `inner.import_block`
 		let mut justification = block.justification.take();
 		let enacts_consensus_change = !new_cache.is_empty();
-		let import_result = (&*self.inner).import_block(block, new_cache);
+		let import_result = self.inner.import_block(block, new_cache);
 
 		let mut imported_aux = {
 			match import_result {
@@ -545,7 +543,7 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block>
 	}
 
 	fn check_block(
-		&mut self,
+		&self,
 		block: BlockCheckParams<Block>,
 	) -> Result<ImportResult, Self::Error> {
 		self.inner.check_block(block)
@@ -620,7 +618,7 @@ where
 	/// If `enacts_change` is set to true, then finalizing this block *must*
 	/// enact an authority set change, the function will panic otherwise.
 	fn import_justification(
-		&mut self,
+		&self,
 		hash: Block::Hash,
 		number: NumberFor<Block>,
 		justification: Justification,

client/finality-grandpa/src/light_import.rs

@@ -120,7 +120,7 @@ impl<BE, Block: BlockT, Client> BlockImport<Block>
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
 		DigestFor<Block>: Encode,
 		BE: Backend<Block> + 'static,
-		for<'a> &'a Client:
+		Client:
 			HeaderBackend<Block>
 			+ BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>>
 			+ Finalizer<Block, BE>
@@ -130,17 +130,17 @@ impl<BE, Block: BlockT, Client> BlockImport<Block>
 	type Transaction = TransactionFor<BE, Block>;
 
 	fn import_block(
-		&mut self,
+		&self,
 		block: BlockImportParams<Block, Self::Transaction>,
 		new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error> {
 		do_import_block::<_, _, _, GrandpaJustification<Block>>(
-			&*self.client, &mut *self.data.write(), block, new_cache
+			self.client.clone(), &mut *self.data.write(), block, new_cache
 		)
 	}
 
 	fn check_block(
-		&mut self,
+		&self,
 		block: BlockCheckParams<Block>,
 	) -> Result<ImportResult, Self::Error> {
 		self.client.check_block(block)
@@ -152,7 +152,7 @@ impl<BE, Block: BlockT, Client> FinalityProofImport<Block>
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
 		DigestFor<Block>: Encode,
 		BE: Backend<Block> + 'static,
-		for<'a> &'a Client:
+		Client:
 			HeaderBackend<Block>
 			+ BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>>
 			+ Finalizer<Block, BE>
@@ -184,7 +184,7 @@ impl<BE, Block: BlockT, Client> FinalityProofImport<Block>
 		verifier: &mut dyn Verifier<Block>,
 	) -> Result<(Block::Hash, NumberFor<Block>), Self::Error> {
 		do_import_finality_proof::<_, _, _, GrandpaJustification<Block>>(
-			&*self.client,
+			self.client.clone(),
 			self.backend.clone(),
 			&*self.authority_set_provider,
 			&mut *self.data.write(),
@@ -236,7 +236,7 @@ impl<B: BlockT> FinalityProofRequestBuilder<B> for GrandpaFinalityProofRequestBu
 
 /// Try to import new block.
 fn do_import_block<B, C, Block: BlockT, J>(
-	mut client: C,
+	client: Arc<C>,
 	data: &mut LightImportData<Block>,
 	mut block: BlockImportParams<Block, TransactionFor<B, Block>>,
 	new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
@@ -245,8 +245,7 @@ fn do_import_block<B, C, Block: BlockT, J>(
 		C: HeaderBackend<Block>
 			+ AuxStore
 			+ Finalizer<Block, B>
-			+ BlockImport<Block, Transaction = TransactionFor<B, Block>>
-			+ Clone,
+			+ BlockImport<Block, Transaction = TransactionFor<B, Block>>,
 		B: Backend<Block> + 'static,
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
 		DigestFor<Block>: Encode,
@@ -295,7 +294,7 @@ fn do_import_block<B, C, Block: BlockT, J>(
 
 /// Try to import finality proof.
 fn do_import_finality_proof<B, C, Block: BlockT, J>(
-	client: C,
+	client: Arc<C>,
 	backend: Arc<B>,
 	authority_set_provider: &dyn AuthoritySetForFinalityChecker<Block>,
 	data: &mut LightImportData<Block>,
@@ -308,8 +307,7 @@ fn do_import_finality_proof<B, C, Block: BlockT, J>(
 		C: HeaderBackend<Block>
 			+ AuxStore
 			+ Finalizer<Block, B>
-			+ BlockImport<Block, Transaction = TransactionFor<B, Block>>
-			+ Clone,
+			+ BlockImport<Block, Transaction = TransactionFor<B, Block>>,
 		B: Backend<Block> + 'static,
 		DigestFor<Block>: Encode,
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
@@ -372,7 +370,7 @@ fn do_import_finality_proof<B, C, Block: BlockT, J>(
 
 	// store new authorities set
 	require_insert_aux(
-		&client,
+		&*client,
 		LIGHT_AUTHORITY_SET_KEY,
 		&data.authority_set,
 		"authority set",
@@ -383,7 +381,7 @@ fn do_import_finality_proof<B, C, Block: BlockT, J>(
 
 /// Try to import justification.
 fn do_import_justification<B, C, Block: BlockT, J>(
-	client: C,
+	client: Arc<C>,
 	data: &mut LightImportData<Block>,
 	hash: Block::Hash,
 	number: NumberFor<Block>,
@@ -392,8 +390,7 @@ fn do_import_justification<B, C, Block: BlockT, J>(
 	where
 		C: HeaderBackend<Block>
 			+ AuxStore
-			+ Finalizer<Block, B>
-			+ Clone,
+			+ Finalizer<Block, B>,
 		B: Backend<Block> + 'static,
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
 		J: ProvableJustification<Block::Header>,
@@ -454,7 +451,7 @@ fn do_import_justification<B, C, Block: BlockT, J>(
 
 /// Finalize the block.
 fn do_finalize_block<B, C, Block: BlockT>(
-	client: C,
+	client: Arc<C>,
 	data: &mut LightImportData<Block>,
 	hash: Block::Hash,
 	number: NumberFor<Block>,
@@ -463,8 +460,7 @@ fn do_finalize_block<B, C, Block: BlockT>(
 	where
 		C: HeaderBackend<Block>
 			+ AuxStore
-			+ Finalizer<Block, B>
-			+ Clone,
+			+ Finalizer<Block, B>,
 		B: Backend<Block> + 'static,
 		NumberFor<Block>: finality_grandpa::BlockNumberOps,
 {
@@ -478,11 +474,11 @@ fn do_finalize_block<B, C, Block: BlockT>(
 	let consensus_finalization_res = data.consensus_changes
 		.finalize(
 			(number, hash),
-			|at_height| canonical_at_height(&client, (hash, number), true, at_height)
+			|at_height| canonical_at_height(&*client, (hash, number), true, at_height)
 		);
 	match consensus_finalization_res {
 		Ok((true, _)) => require_insert_aux(
-			&client,
+			&*client,
 			LIGHT_CONSENSUS_CHANGES_KEY,
 			&data.consensus_changes,
 			"consensus changes",
@@ -615,7 +611,7 @@ pub mod tests {
 			NumberFor<Block>: finality_grandpa::BlockNumberOps,
 			DigestFor<Block>: Encode,
 			BE: Backend<Block> + 'static,
-			for <'a > &'a Client:
+			Client:
 				HeaderBackend<Block>
 				+ BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>>
 				+ Finalizer<Block, BE>
@@ -627,7 +623,7 @@ pub mod tests {
 		type Transaction = TransactionFor<BE, Block>;
 
 		fn import_block(
-			&mut self,
+			&self,
 			mut block: BlockImportParams<Block, Self::Transaction>,
 			new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
 		) -> Result<ImportResult, Self::Error> {
@@ -636,7 +632,7 @@ pub mod tests {
 		}
 
 		fn check_block(
-			&mut self,
+			&self,
 			block: BlockCheckParams<Block>,
 		) -> Result<ImportResult, Self::Error> {
 			self.0.check_block(block)
@@ -648,7 +644,7 @@ pub mod tests {
 			NumberFor<Block>: finality_grandpa::BlockNumberOps,
 			BE: Backend<Block> + 'static,
 			DigestFor<Block>: Encode,
-			for <'a > &'a Client:
+			Client:
 				HeaderBackend<Block>
 				+ BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>>
 				+ Finalizer<Block, BE>
@@ -691,7 +687,12 @@ pub mod tests {
 		justification: Option<Justification>,
 	) -> (
 		ImportResult,
-		substrate_test_runtime_client::client::Client<substrate_test_runtime_client::LightBackend, substrate_test_runtime_client::LightExecutor, Block, substrate_test_runtime_client::runtime::RuntimeApi>,
+		Arc<substrate_test_runtime_client::client::Client<
+			substrate_test_runtime_client::LightBackend,
+			substrate_test_runtime_client::LightExecutor,
+			Block,
+			substrate_test_runtime_client::runtime::RuntimeApi,
+		>>,
 		Arc<substrate_test_runtime_client::LightBackend>,
 	) {
 		let (client, backend) = substrate_test_runtime_client::new_light();
@@ -715,7 +716,7 @@ pub mod tests {
 
 		(
 			do_import_block::<_, _, _, TestJustification>(
-				&client,
+				client.clone(),
 				&mut import_data,
 				block,
 				new_cache,
@@ -834,7 +835,7 @@ pub mod tests {
 		let initial_set = vec![(AuthorityId::from_slice(&[1; 32]), 1)];
 		let updated_set = vec![(AuthorityId::from_slice(&[2; 32]), 2)];
 		let babe_set_signal = vec![AuthorityId::from_slice(&[42; 32])].encode();
-		
+
 		// import block #1 without justification
 		let mut cache = HashMap::new();
 		cache.insert(well_known_cache_keys::AUTHORITIES, babe_set_signal);
@@ -851,7 +852,7 @@ pub mod tests {
 
 		// import finality proof
 		do_import_finality_proof::<_, _, _, TestJustification>(
-			&client,
+			client.clone(),
 			backend,
 			&ClosureAuthoritySetForFinalityChecker(
 				|_, _, _| Ok(updated_set.clone())
@@ -874,7 +875,7 @@ pub mod tests {
 		).unwrap();
 
 		// verify that new authorities set has been saved to the aux storage
-		let data = load_aux_import_data(Default::default(), &client, &TestApi::new(initial_set)).unwrap();
+		let data = load_aux_import_data(Default::default(), &*client, &TestApi::new(initial_set)).unwrap();
 		assert_eq!(data.authority_set.authorities(), updated_set);
 	}
 }