BlockId removal: refactor: Finalizer (#12528)

* BlockId removal: refactor: Finalizer

It changes the arguments of methods of `Finalizer` trait from:
block: `BlockId<Block>` to: hash: `&Block::Hash`

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

* minor corrections

* failing test corrected

* minor rework

substrate/client/api/src/backend.rs

@@ -252,7 +252,7 @@ pub trait Finalizer<Block: BlockT, B: Backend<Block>> {
 	fn apply_finality(
 		&self,
 		operation: &mut ClientImportOperation<Block, B>,
-		id: BlockId<Block>,
+		block: &Block::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> sp_blockchain::Result<()>;
@@ -272,7 +272,7 @@ pub trait Finalizer<Block: BlockT, B: Backend<Block>> {
 	/// while performing major synchronization work.
 	fn finalize_block(
 		&self,
-		id: BlockId<Block>,
+		block: &Block::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> sp_blockchain::Result<()>;

substrate/client/beefy/src/tests.rs

@@ -509,14 +509,10 @@ fn finalize_block_and_wait_for_beefy(
 	let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone());
 
 	for block in finalize_targets {
-		let finalize = BlockId::number(*block);
 		peers.clone().for_each(|(index, _)| {
-			net.lock()
-				.peer(index)
-				.client()
-				.as_client()
-				.finalize_block(finalize, None)
-				.unwrap();
+			let client = net.lock().peer(index).client().as_client();
+			let finalize = client.expect_block_hash_from_id(&BlockId::number(*block)).unwrap();
+			client.finalize_block(&finalize, None).unwrap();
 		})
 	}
 
@@ -604,9 +600,15 @@ fn lagging_validators() {
 	);
 
 	// Alice finalizes #25, Bob lags behind
-	let finalize = BlockId::number(25);
+	let finalize = net
+		.lock()
+		.peer(0)
+		.client()
+		.as_client()
+		.expect_block_hash_from_id(&BlockId::number(25))
+		.unwrap();
 	let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone());
-	net.lock().peer(0).client().as_client().finalize_block(finalize, None).unwrap();
+	net.lock().peer(0).client().as_client().finalize_block(&finalize, None).unwrap();
 	// verify nothing gets finalized by BEEFY
 	let timeout = Some(Duration::from_millis(250));
 	streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout);
@@ -614,7 +616,7 @@ fn lagging_validators() {
 
 	// Bob catches up and also finalizes #25
 	let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone());
-	net.lock().peer(1).client().as_client().finalize_block(finalize, None).unwrap();
+	net.lock().peer(1).client().as_client().finalize_block(&finalize, None).unwrap();
 	// expected beefy finalizes block #17 from diff-power-of-two
 	wait_for_best_beefy_blocks(best_blocks, &net, &mut runtime, &[23, 24, 25]);
 	wait_for_beefy_signed_commitments(versioned_finality_proof, &net, &mut runtime, &[23, 24, 25]);
@@ -628,8 +630,14 @@ fn lagging_validators() {
 
 	// Alice finalizes session-boundary mandatory block #60, Bob lags behind
 	let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone());
-	let finalize = BlockId::number(60);
-	net.lock().peer(0).client().as_client().finalize_block(finalize, None).unwrap();
+	let finalize = net
+		.lock()
+		.peer(0)
+		.client()
+		.as_client()
+		.expect_block_hash_from_id(&BlockId::number(60))
+		.unwrap();
+	net.lock().peer(0).client().as_client().finalize_block(&finalize, None).unwrap();
 	// verify nothing gets finalized by BEEFY
 	let timeout = Some(Duration::from_millis(250));
 	streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout);
@@ -637,7 +645,7 @@ fn lagging_validators() {
 
 	// Bob catches up and also finalizes #60 (and should have buffered Alice's vote on #60)
 	let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers);
-	net.lock().peer(1).client().as_client().finalize_block(finalize, None).unwrap();
+	net.lock().peer(1).client().as_client().finalize_block(&finalize, None).unwrap();
 	// verify beefy skips intermediary votes, and successfully finalizes mandatory block #60
 	wait_for_best_beefy_blocks(best_blocks, &net, &mut runtime, &[60]);
 	wait_for_beefy_signed_commitments(versioned_finality_proof, &net, &mut runtime, &[60]);
@@ -681,24 +689,16 @@ fn correct_beefy_payload() {
 		get_beefy_streams(&mut net.lock(), [(0, BeefyKeyring::Alice)].into_iter());
 
 	// now 2 good validators and 1 bad one are voting
-	net.lock()
+	let hashof11 = net
+		.lock()
 		.peer(0)
 		.client()
 		.as_client()
-		.finalize_block(BlockId::number(11), None)
-		.unwrap();
-	net.lock()
-		.peer(1)
-		.client()
-		.as_client()
-		.finalize_block(BlockId::number(11), None)
-		.unwrap();
-	net.lock()
-		.peer(3)
-		.client()
-		.as_client()
-		.finalize_block(BlockId::number(11), None)
+		.expect_block_hash_from_id(&BlockId::number(11))
 		.unwrap();
+	net.lock().peer(0).client().as_client().finalize_block(&hashof11, None).unwrap();
+	net.lock().peer(1).client().as_client().finalize_block(&hashof11, None).unwrap();
+	net.lock().peer(3).client().as_client().finalize_block(&hashof11, None).unwrap();
 
 	// verify consensus is _not_ reached
 	let timeout = Some(Duration::from_millis(250));
@@ -708,12 +708,7 @@ fn correct_beefy_payload() {
 	// 3rd good validator catches up and votes as well
 	let (best_blocks, versioned_finality_proof) =
 		get_beefy_streams(&mut net.lock(), [(0, BeefyKeyring::Alice)].into_iter());
-	net.lock()
-		.peer(2)
-		.client()
-		.as_client()
-		.finalize_block(BlockId::number(11), None)
-		.unwrap();
+	net.lock().peer(2).client().as_client().finalize_block(&hashof11, None).unwrap();
 
 	// verify consensus is reached
 	wait_for_best_beefy_blocks(best_blocks, &net, &mut runtime, &[11]);
@@ -923,12 +918,9 @@ fn on_demand_beefy_justification_sync() {
 
 	let (dave_best_blocks, _) =
 		get_beefy_streams(&mut net.lock(), [(dave_index, BeefyKeyring::Dave)].into_iter());
-	net.lock()
-		.peer(dave_index)
-		.client()
-		.as_client()
-		.finalize_block(BlockId::number(1), None)
-		.unwrap();
+	let client = net.lock().peer(dave_index).client().as_client();
+	let hashof1 = client.expect_block_hash_from_id(&BlockId::number(1)).unwrap();
+	client.finalize_block(&hashof1, None).unwrap();
 	// Give Dave task some cpu cycles to process the finality notification,
 	run_for(Duration::from_millis(100), &net, &mut runtime);
 	// freshly spun up Dave now needs to listen for gossip to figure out the state of his peers.

substrate/client/beefy/src/worker.rs

@@ -1506,11 +1506,9 @@ pub(crate) mod tests {
 		// push 15 blocks with `AuthorityChange` digests every 10 blocks
 		net.generate_blocks_and_sync(15, 10, &validator_set, false);
 		// finalize 13 without justifications
-		net.peer(0)
-			.client()
-			.as_client()
-			.finalize_block(BlockId::number(13), None)
-			.unwrap();
+		let hashof13 =
+			backend.blockchain().expect_block_hash_from_id(&BlockId::Number(13)).unwrap();
+		net.peer(0).client().as_client().finalize_block(&hashof13, None).unwrap();
 
 		// Test initialization at session boundary.
 		{

substrate/client/consensus/babe/src/tests.rs

@@ -820,7 +820,7 @@ fn revert_not_allowed_for_finalized() {
 	let canon = propose_and_import_blocks_wrap(BlockId::Number(0), 3);
 
 	// Finalize best block
-	client.finalize_block(BlockId::Hash(canon[2]), None, false).unwrap();
+	client.finalize_block(&canon[2], None, false).unwrap();
 
 	// Revert canon chain to last finalized block
 	revert(client.clone(), backend, 100).expect("revert should work for baked test scenario");
@@ -882,7 +882,7 @@ fn importing_epoch_change_block_prunes_tree() {
 
 	// We finalize block #13 from the canon chain, so on the next epoch
 	// change the tree should be pruned, to not contain F (#7).
-	client.finalize_block(BlockId::Hash(canon_hashes[12]), None, false).unwrap();
+	client.finalize_block(&canon_hashes[12], None, false).unwrap();
 	propose_and_import_blocks_wrap(BlockId::Hash(client.chain_info().best_hash), 7);
 
 	// at this point no hashes from the first fork must exist on the tree
@@ -909,7 +909,7 @@ fn importing_epoch_change_block_prunes_tree() {
 		.any(|h| fork_3.contains(h)),);
 
 	// finalizing block #25 from the canon chain should prune out the second fork
-	client.finalize_block(BlockId::Hash(canon_hashes[24]), None, false).unwrap();
+	client.finalize_block(&canon_hashes[24], None, false).unwrap();
 	propose_and_import_blocks_wrap(BlockId::Hash(client.chain_info().best_hash), 8);
 
 	// at this point no hashes from the second fork must exist on the tree
@@ -1049,7 +1049,7 @@ fn obsolete_blocks_aux_data_cleanup() {
 	assert!(aux_data_check(&fork3_hashes, true));
 
 	// Finalize A3
-	client.finalize_block(BlockId::Number(3), None, true).unwrap();
+	client.finalize_block(&fork1_hashes[2], None, true).unwrap();
 
 	// Wiped: A1, A2
 	assert!(aux_data_check(&fork1_hashes[..2], false));
@@ -1060,7 +1060,7 @@ fn obsolete_blocks_aux_data_cleanup() {
 	// Present C4, C5
 	assert!(aux_data_check(&fork3_hashes, true));
 
-	client.finalize_block(BlockId::Number(4), None, true).unwrap();
+	client.finalize_block(&fork1_hashes[3], None, true).unwrap();
 
 	// Wiped: A3
 	assert!(aux_data_check(&fork1_hashes[2..3], false));

substrate/client/consensus/manual-seal/src/finalize_block.rs

@@ -20,7 +20,7 @@
 
 use crate::rpc;
 use sc_client_api::backend::{Backend as ClientBackend, Finalizer};
-use sp_runtime::{generic::BlockId, traits::Block as BlockT, Justification};
+use sp_runtime::{traits::Block as BlockT, Justification};
 use std::{marker::PhantomData, sync::Arc};
 
 /// params for block finalization.
@@ -46,7 +46,7 @@ where
 {
 	let FinalizeBlockParams { hash, mut sender, justification, finalizer, .. } = params;
 
-	match finalizer.finalize_block(BlockId::Hash(hash), justification, true) {
+	match finalizer.finalize_block(&hash, justification, true) {
 		Err(e) => {
 			log::warn!("Failed to finalize block {}", e);
 			rpc::send_result(&mut sender, Err(e.into()))

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

@@ -1352,7 +1352,7 @@ where
 		// ideally some handle to a synchronization oracle would be used
 		// to avoid unconditionally notifying.
 		client
-			.apply_finality(import_op, BlockId::Hash(hash), persisted_justification, true)
+			.apply_finality(import_op, &hash, persisted_justification, true)
 			.map_err(|e| {
 				warn!(target: "afg", "Error applying finality to block {:?}: {}", (hash, number), e);
 				e

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

@@ -309,7 +309,8 @@ mod tests {
 		}
 
 		for block in to_finalize {
-			client.finalize_block(BlockId::Number(*block), None).unwrap();
+			let hash = blocks[*block as usize - 1].hash();
+			client.finalize_block(&hash, None).unwrap();
 		}
 		(client, backend, blocks)
 	}
@@ -489,7 +490,7 @@ mod tests {
 		let grandpa_just8 = GrandpaJustification::from_commit(&client, round, commit).unwrap();
 
 		client
-			.finalize_block(BlockId::Number(8), Some((ID, grandpa_just8.encode().clone())))
+			.finalize_block(&block8.hash(), Some((ID, grandpa_just8.encode().clone())))
 			.unwrap();
 
 		// Authority set change at block 8, so the justification stored there will be used in the

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

@@ -1457,7 +1457,12 @@ fn grandpa_environment_respects_voting_rules() {
 	);
 
 	// we finalize block 19 with block 21 being the best block
-	peer.client().finalize_block(BlockId::Number(19), None, false).unwrap();
+	let hashof19 = peer
+		.client()
+		.as_client()
+		.expect_block_hash_from_id(&BlockId::Number(19))
+		.unwrap();
+	peer.client().finalize_block(&hashof19, None, false).unwrap();
 
 	// the 3/4 environment should propose block 21 for voting
 	assert_eq!(
@@ -1479,7 +1484,12 @@ fn grandpa_environment_respects_voting_rules() {
 	);
 
 	// we finalize block 21 with block 21 being the best block
-	peer.client().finalize_block(BlockId::Number(21), None, false).unwrap();
+	let hashof21 = peer
+		.client()
+		.as_client()
+		.expect_block_hash_from_id(&BlockId::Number(21))
+		.unwrap();
+	peer.client().finalize_block(&hashof21, None, false).unwrap();
 
 	// even though the default environment will always try to not vote on the
 	// best block, there's a hard rule that we can't cast any votes lower than

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

@@ -327,7 +327,7 @@ mod tests {
 	use sp_consensus::BlockOrigin;
 	use sp_finality_grandpa::GRANDPA_ENGINE_ID;
 	use sp_keyring::Ed25519Keyring;
-	use sp_runtime::{generic::BlockId, traits::Header as _};
+	use sp_runtime::traits::Header as _;
 	use std::sync::Arc;
 	use substrate_test_runtime_client::{
 		ClientBlockImportExt, ClientExt, DefaultTestClientBuilderExt, TestClientBuilder,
@@ -412,10 +412,7 @@ mod tests {
 				let justification = GrandpaJustification::from_commit(&client, 42, commit).unwrap();
 
 				client
-					.finalize_block(
-						BlockId::Hash(target_hash),
-						Some((GRANDPA_ENGINE_ID, justification.encode())),
-					)
+					.finalize_block(&target_hash, Some((GRANDPA_ENGINE_ID, justification.encode())))
 					.unwrap();
 
 				authority_set_changes.push((current_set_id, n));

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

@@ -3172,9 +3172,7 @@ mod test {
 
 		let finalized_block = blocks[MAX_BLOCKS_TO_LOOK_BACKWARDS as usize * 2 - 1].clone();
 		let just = (*b"TEST", Vec::new());
-		client
-			.finalize_block(BlockId::Hash(finalized_block.hash()), Some(just))
-			.unwrap();
+		client.finalize_block(&finalized_block.hash(), Some(just)).unwrap();
 		sync.update_chain_info(&info.best_hash, info.best_number);
 
 		let peer_id1 = PeerId::random();
@@ -3303,9 +3301,7 @@ mod test {
 
 		let finalized_block = blocks[MAX_BLOCKS_TO_LOOK_BACKWARDS as usize * 2 - 1].clone();
 		let just = (*b"TEST", Vec::new());
-		client
-			.finalize_block(BlockId::Hash(finalized_block.hash()), Some(just))
-			.unwrap();
+		client.finalize_block(&finalized_block.hash(), Some(just)).unwrap();
 		sync.update_chain_info(&info.best_hash, info.best_number);
 
 		let peer_id1 = PeerId::random();

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

@@ -190,11 +190,11 @@ impl PeersClient {
 
 	pub fn finalize_block(
 		&self,
-		id: BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> ClientResult<()> {
-		self.client.finalize_block(id, justification, notify)
+		self.client.finalize_block(hash, justification, notify)
 	}
 }
 
@@ -1113,7 +1113,7 @@ impl JustificationImport<Block> for ForceFinalized {
 		justification: Justification,
 	) -> Result<(), Self::Error> {
 		self.0
-			.finalize_block(BlockId::Hash(hash), Some(justification), true)
+			.finalize_block(&hash, Some(justification), true)
 			.map_err(|_| ConsensusError::InvalidJustification)
 	}
 }

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

@@ -251,18 +251,22 @@ fn sync_justifications() {
 	assert_eq!(net.peer(1).client().justifications(&BlockId::Number(10)).unwrap(), None);
 
 	// we finalize block #10, #15 and #20 for peer 0 with a justification
+	let backend = net.peer(0).client().as_backend();
+	let hashof10 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(10)).unwrap();
+	let hashof15 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(15)).unwrap();
+	let hashof20 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(20)).unwrap();
 	let just = (*b"FRNK", Vec::new());
 	net.peer(0)
 		.client()
-		.finalize_block(BlockId::Number(10), Some(just.clone()), true)
+		.finalize_block(&hashof10, Some(just.clone()), true)
 		.unwrap();
 	net.peer(0)
 		.client()
-		.finalize_block(BlockId::Number(15), Some(just.clone()), true)
+		.finalize_block(&hashof15, Some(just.clone()), true)
 		.unwrap();
 	net.peer(0)
 		.client()
-		.finalize_block(BlockId::Number(20), Some(just.clone()), true)
+		.finalize_block(&hashof20, Some(just.clone()), true)
 		.unwrap();
 
 	let h1 = net.peer(1).client().header(&BlockId::Number(10)).unwrap().unwrap();
@@ -309,10 +313,7 @@ fn sync_justifications_across_forks() {
 	net.block_until_sync();
 
 	let just = (*b"FRNK", Vec::new());
-	net.peer(0)
-		.client()
-		.finalize_block(BlockId::Hash(f1_best), Some(just), true)
-		.unwrap();
+	net.peer(0).client().finalize_block(&f1_best, Some(just), true).unwrap();
 
 	net.peer(1).request_justification(&f1_best, 10);
 	net.peer(1).request_justification(&f2_best, 11);
@@ -655,14 +656,8 @@ fn can_sync_to_peers_with_wrong_common_block() {
 
 	// both peers re-org to the same fork without notifying each other
 	let just = Some((*b"FRNK", Vec::new()));
-	net.peer(0)
-		.client()
-		.finalize_block(BlockId::Hash(fork_hash), just.clone(), true)
-		.unwrap();
-	net.peer(1)
-		.client()
-		.finalize_block(BlockId::Hash(fork_hash), just, true)
-		.unwrap();
+	net.peer(0).client().finalize_block(&fork_hash, just.clone(), true).unwrap();
+	net.peer(1).client().finalize_block(&fork_hash, just, true).unwrap();
 	let final_hash = net.peer(0).push_blocks(1, false);
 
 	net.block_until_sync();
@@ -976,10 +971,17 @@ fn multiple_requests_are_accepted_as_long_as_they_are_not_fulfilled() {
 		assert_eq!(1, net.peer(0).num_peers());
 	}
 
+	let hashof10 = net
+		.peer(0)
+		.client()
+		.as_backend()
+		.blockchain()
+		.expect_block_hash_from_id(&BlockId::Number(10))
+		.unwrap();
 	// Finalize the block and make the justification available.
 	net.peer(0)
 		.client()
-		.finalize_block(BlockId::Number(10), Some((*b"FRNK", Vec::new())), true)
+		.finalize_block(&hashof10, Some((*b"FRNK", Vec::new())), true)
 		.unwrap();
 
 	block_on(futures::future::poll_fn::<(), _>(|cx| {
@@ -1100,10 +1102,14 @@ fn syncs_state() {
 		assert!(!net.peer(1).client().has_state_at(&BlockId::Number(64)));
 
 		let just = (*b"FRNK", Vec::new());
-		net.peer(1)
+		let hashof60 = net
+			.peer(0)
 			.client()
-			.finalize_block(BlockId::Number(60), Some(just), true)
+			.as_backend()
+			.blockchain()
+			.expect_block_hash_from_id(&BlockId::Number(60))
 			.unwrap();
+		net.peer(1).client().finalize_block(&hashof60, Some(just), true).unwrap();
 		// Wait for state sync.
 		block_on(futures::future::poll_fn::<(), _>(|cx| {
 			net.poll(cx);

substrate/client/rpc/src/chain/tests.rs

@@ -198,6 +198,7 @@ async fn should_return_finalized_hash() {
 
 	// import new block
 	let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
+	let block_hash = block.hash();
 	client.import(BlockOrigin::Own, block).await.unwrap();
 
 	// no finalization yet
@@ -205,9 +206,9 @@ async fn should_return_finalized_hash() {
 	assert_eq!(res, client.genesis_hash());
 
 	// finalize
-	client.finalize_block(BlockId::number(1), None).unwrap();
+	client.finalize_block(&block_hash, None).unwrap();
 	let res: H256 = api.call("chain_getFinalizedHead", EmptyParams::new()).await.unwrap();
-	assert_eq!(res, client.block_hash(1).unwrap().unwrap());
+	assert_eq!(res, block_hash);
 }
 
 #[tokio::test]
@@ -232,8 +233,9 @@ async fn test_head_subscription(method: &str) {
 		let api = new_full(client.clone(), test_executor()).into_rpc();
 		let sub = api.subscribe(method, EmptyParams::new()).await.unwrap();
 		let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
+		let block_hash = block.hash();
 		client.import(BlockOrigin::Own, block).await.unwrap();
-		client.finalize_block(BlockId::number(1), None).unwrap();
+		client.finalize_block(&block_hash, None).unwrap();
 		sub
 	};
 

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

@@ -1845,29 +1845,22 @@ where
 	fn apply_finality(
 		&self,
 		operation: &mut ClientImportOperation<Block, B>,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> sp_blockchain::Result<()> {
 		let last_best = self.backend.blockchain().info().best_hash;
-		let to_finalize_hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		self.apply_finality_with_block_hash(
-			operation,
-			to_finalize_hash,
-			justification,
-			last_best,
-			notify,
-		)
+		self.apply_finality_with_block_hash(operation, *hash, justification, last_best, notify)
 	}
 
 	fn finalize_block(
 		&self,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> sp_blockchain::Result<()> {
 		self.lock_import_and_run(|operation| {
-			self.apply_finality(operation, id, justification, notify)
+			self.apply_finality(operation, hash, justification, notify)
 		})
 	}
 }
@@ -1881,20 +1874,20 @@ where
 	fn apply_finality(
 		&self,
 		operation: &mut ClientImportOperation<Block, B>,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> sp_blockchain::Result<()> {
-		(**self).apply_finality(operation, id, justification, notify)
+		(**self).apply_finality(operation, hash, justification, notify)
 	}
 
 	fn finalize_block(
 		&self,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 		justification: Option<Justification>,
 		notify: bool,
 	) -> sp_blockchain::Result<()> {
-		(**self).finalize_block(id, justification, notify)
+		(**self).finalize_block(hash, justification, notify)
 	}
 }
 

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

@@ -867,7 +867,7 @@ fn import_with_justification() {
 		.unwrap()
 		.block;
 	block_on(client.import(BlockOrigin::Own, a2.clone())).unwrap();
-	client.finalize_block(BlockId::hash(a2.hash()), None).unwrap();
+	client.finalize_block(&a2.hash(), None).unwrap();
 
 	// A2 -> A3
 	let justification = Justifications::from((TEST_ENGINE_ID, vec![1, 2, 3]));
@@ -996,7 +996,7 @@ fn finalizing_diverged_block_should_trigger_reorg() {
 
 	// we finalize block B1 which is on a different branch from current best
 	// which should trigger a re-org.
-	ClientExt::finalize_block(&client, BlockId::Hash(b1.hash()), None).unwrap();
+	ClientExt::finalize_block(&client, &b1.hash(), None).unwrap();
 
 	// B1 should now be the latest finalized
 	assert_eq!(client.chain_info().finalized_hash, b1.hash());
@@ -1020,7 +1020,7 @@ fn finalizing_diverged_block_should_trigger_reorg() {
 
 	assert_eq!(client.chain_info().best_hash, b3.hash());
 
-	ClientExt::finalize_block(&client, BlockId::Hash(b3.hash()), None).unwrap();
+	ClientExt::finalize_block(&client, &b3.hash(), None).unwrap();
 
 	finality_notification_check(&mut finality_notifications, &[b1.hash()], &[]);
 	finality_notification_check(&mut finality_notifications, &[b2.hash(), b3.hash()], &[a2.hash()]);
@@ -1118,7 +1118,7 @@ fn finality_notifications_content() {
 
 	// Postpone import to test behavior of import of finalized block.
 
-	ClientExt::finalize_block(&client, BlockId::Hash(a2.hash()), None).unwrap();
+	ClientExt::finalize_block(&client, &a2.hash(), None).unwrap();
 
 	// Import and finalize D4
 	block_on(client.import_as_final(BlockOrigin::Own, d4.clone())).unwrap();
@@ -1274,7 +1274,7 @@ fn doesnt_import_blocks_that_revert_finality() {
 
 	// we will finalize A2 which should make it impossible to import a new
 	// B3 at the same height but that doesn't include it
-	ClientExt::finalize_block(&client, BlockId::Hash(a2.hash()), None).unwrap();
+	ClientExt::finalize_block(&client, &a2.hash(), None).unwrap();
 
 	let import_err = block_on(client.import(BlockOrigin::Own, b3)).err().unwrap();
 	let expected_err =
@@ -1309,7 +1309,7 @@ fn doesnt_import_blocks_that_revert_finality() {
 		.unwrap()
 		.block;
 	block_on(client.import(BlockOrigin::Own, a3.clone())).unwrap();
-	ClientExt::finalize_block(&client, BlockId::Hash(a3.hash()), None).unwrap();
+	ClientExt::finalize_block(&client, &a3.hash(), None).unwrap();
 
 	finality_notification_check(&mut finality_notifications, &[a1.hash(), a2.hash()], &[]);
 

substrate/test-utils/client/src/client_ext.rs

@@ -22,14 +22,14 @@ use sc_client_api::{backend::Finalizer, client::BlockBackend};
 use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy};
 use sc_service::client::Client;
 use sp_consensus::{BlockOrigin, Error as ConsensusError};
-use sp_runtime::{generic::BlockId, traits::Block as BlockT, Justification, Justifications};
+use sp_runtime::{traits::Block as BlockT, Justification, Justifications};
 
 /// Extension trait for a test client.
 pub trait ClientExt<Block: BlockT>: Sized {
 	/// Finalize a block.
 	fn finalize_block(
 		&self,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 		justification: Option<Justification>,
 	) -> sp_blockchain::Result<()>;
 
@@ -75,10 +75,10 @@ where
 {
 	fn finalize_block(
 		&self,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 		justification: Option<Justification>,
 	) -> sp_blockchain::Result<()> {
-		Finalizer::finalize_block(self, id, justification, true)
+		Finalizer::finalize_block(self, hash, justification, true)
 	}
 
 	fn genesis_hash(&self) -> <Block as BlockT>::Hash {