BlockId removal: refactor: HeaderBackend::header (#12874)

* BlockId removal: refactor: HeaderBackend::header

It changes the arguments of:
- `HeaderBackend::header`,
- `Client::header`,
- `PeersClient::header`
- `ChainApi::block_header`

methods from: `BlockId<Block>` to: `Block::Hash`

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

* non-trivial usages of haeder(block_id) refactored

This may required introduction of dedicated function:
header_for_block_num

* fmt

* fix

* doc fixed

* ".git/.scripts/fmt.sh"

* BlockId removal: refactor: HeaderBackend::expect_header

It changes the arguments of `HeaderBackend::expect_header` method from: `BlockId<Block>` to: `Block::Hash`

* ".git/.scripts/fmt.sh"

* readme updated

* ".git/.scripts/fmt.sh"

* fix

Co-authored-by: parity-processbot <>

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

@@ -525,7 +525,7 @@ where
 			Some((_, n)) if n > best_block_number => best_block_hash,
 			Some((h, _)) => {
 				// this is the header at which the new set will start
-				let header = self.client.header(BlockId::Hash(h))?.expect(
+				let header = self.client.header(h)?.expect(
 					"got block hash from registered pending change; \
 					 pending changes are only registered on block import; qed.",
 				);
@@ -1171,7 +1171,7 @@ where
 	SelectChain: SelectChainT<Block> + 'static,
 	VotingRule: VotingRuleT<Block, Client>,
 {
-	let base_header = match client.header(BlockId::Hash(block))? {
+	let base_header = match client.header(block)? {
 		Some(h) => h,
 		None => {
 			debug!(
@@ -1197,7 +1197,7 @@ where
 	let result = match select_chain.finality_target(block, None).await {
 		Ok(best_hash) => {
 			let best_header = client
-				.header(BlockId::Hash(best_hash))?
+				.header(best_hash)?
 				.expect("Header known to exist after `finality_target` call; qed");
 
 			// check if our vote is currently being limited due to a pending change
@@ -1221,7 +1221,7 @@ where
 					}
 
 					target_header = client
-						.header(BlockId::Hash(*target_header.parent_hash()))?
+						.header(*target_header.parent_hash())?
 						.expect("Header known to exist after `finality_target` call; qed");
 				}
 

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

@@ -222,7 +222,8 @@ where
 			if current > just_block || headers.len() >= MAX_UNKNOWN_HEADERS {
 				break
 			}
-			headers.push(backend.blockchain().expect_header(BlockId::Number(current))?);
+			let hash = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(current))?;
+			headers.push(backend.blockchain().expect_header(hash)?);
 			current += One::one();
 		}
 		headers

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

@@ -122,7 +122,7 @@ where
 				};
 
 				if let Ok(hash) = effective_block_hash {
-					if let Ok(Some(header)) = self.inner.header(BlockId::Hash(hash)) {
+					if let Ok(Some(header)) = self.inner.header(hash) {
 						if *header.number() == pending_change.effective_number() {
 							out.push((header.hash(), *header.number()));
 						}
@@ -364,14 +364,12 @@ where
 					// best finalized block.
 					let best_finalized_number = self.inner.info().finalized_number;
 					let canon_number = best_finalized_number.min(median_last_finalized_number);
-					let canon_hash =
-						self.inner.header(BlockId::Number(canon_number))
+					let canon_hash = self.inner.hash(canon_number)
 							.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
 							.expect(
 								"the given block number is less or equal than the current best finalized number; \
 								 current best finalized number must exist in chain; qed."
-							)
-							.hash();
+							);
 
 					NewAuthoritySet {
 						canon_number,

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

@@ -26,10 +26,7 @@ use finality_grandpa::{voter_set::VoterSet, Error as GrandpaError};
 use parity_scale_codec::{Decode, Encode};
 use sp_blockchain::{Error as ClientError, HeaderBackend};
 use sp_finality_grandpa::AuthorityId;
-use sp_runtime::{
-	generic::BlockId,
-	traits::{Block as BlockT, Header as HeaderT, NumberFor},
-};
+use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor};
 
 use crate::{AuthorityList, Commit, Error};
 
@@ -104,7 +101,7 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 					break
 				}
 
-				match client.header(BlockId::Hash(current_hash))? {
+				match client.header(current_hash)? {
 					Some(current_header) => {
 						// NOTE: this should never happen as we pick the lowest block
 						// as base and only traverse backwards from the other blocks

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

@@ -1530,10 +1530,13 @@ async fn justification_with_equivocation() {
 	// we create a basic chain with 3 blocks (no forks)
 	net.peer(0).push_blocks(3, false);
 
-	let client = net.peer(0).client().clone();
-	let block1 = client.header(&BlockId::Number(1)).ok().flatten().unwrap();
-	let block2 = client.header(&BlockId::Number(2)).ok().flatten().unwrap();
-	let block3 = client.header(&BlockId::Number(3)).ok().flatten().unwrap();
+	let client = net.peer(0).client().as_client().clone();
+	let hashof1 = client.expect_block_hash_from_id(&BlockId::Number(1)).unwrap();
+	let hashof2 = client.expect_block_hash_from_id(&BlockId::Number(2)).unwrap();
+	let hashof3 = client.expect_block_hash_from_id(&BlockId::Number(3)).unwrap();
+	let block1 = client.expect_header(hashof1).unwrap();
+	let block2 = client.expect_header(hashof2).unwrap();
+	let block3 = client.expect_header(hashof3).unwrap();
 
 	let set_id = 0;
 	let justification = {
@@ -1576,7 +1579,7 @@ async fn justification_with_equivocation() {
 			precommits,
 		};
 
-		GrandpaJustification::from_commit(&client.as_client(), round, commit).unwrap()
+		GrandpaJustification::from_commit(&client, round, commit).unwrap()
 	};
 
 	// the justification should include the minimal necessary vote ancestry and

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

@@ -27,10 +27,7 @@ use std::{future::Future, pin::Pin, sync::Arc};
 use dyn_clone::DynClone;
 
 use sc_client_api::blockchain::HeaderBackend;
-use sp_runtime::{
-	generic::BlockId,
-	traits::{Block as BlockT, Header, NumberFor, One, Zero},
-};
+use sp_runtime::traits::{Block as BlockT, Header, NumberFor, One, Zero};
 
 /// A future returned by a `VotingRule` to restrict a given vote, if any restriction is necessary.
 pub type VotingRuleResult<Block> =
@@ -197,7 +194,7 @@ where
 
 		target_hash = *target_header.parent_hash();
 		target_header = backend
-			.header(BlockId::Hash(target_hash))
+			.header(target_hash)
 			.ok()?
 			.expect("Header known to exist due to the existence of one of its descendents; qed");
 	}
@@ -242,7 +239,7 @@ where
 						restricted_number >= base.number() &&
 							restricted_number < restricted_target.number()
 					})
-					.and_then(|(hash, _)| backend.header(BlockId::Hash(hash)).ok())
+					.and_then(|(hash, _)| backend.header(hash).ok())
 					.and_then(std::convert::identity)
 				{
 					restricted_target = header;
@@ -371,16 +368,18 @@ mod tests {
 		let rule = VotingRulesBuilder::new().add(Subtract(50)).add(Subtract(50)).build();
 
 		let mut client = Arc::new(TestClientBuilder::new().build());
+		let mut hashes = Vec::with_capacity(200);
 
 		for _ in 0..200 {
 			let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
+			hashes.push(block.hash());
 
 			futures::executor::block_on(client.import(BlockOrigin::Own, block)).unwrap();
 		}
 
-		let genesis = client.header(&BlockId::Number(0u32.into())).unwrap().unwrap();
+		let genesis = client.header(client.info().genesis_hash).unwrap().unwrap();
 
-		let best = client.header(&BlockId::Hash(client.info().best_hash)).unwrap().unwrap();
+		let best = client.header(client.info().best_hash).unwrap().unwrap();
 
 		let (_, number) =
 			futures::executor::block_on(rule.restrict_vote(client.clone(), &genesis, &best, &best))
@@ -390,7 +389,7 @@ mod tests {
 		// which means that we should be voting for block #100
 		assert_eq!(number, 100);
 
-		let block110 = client.header(&BlockId::Number(110u32.into())).unwrap().unwrap();
+		let block110 = client.header(hashes[109]).unwrap().unwrap();
 
 		let (_, number) = futures::executor::block_on(rule.restrict_vote(
 			client.clone(),
@@ -412,17 +411,20 @@ mod tests {
 
 		let mut client = Arc::new(TestClientBuilder::new().build());
 
-		for _ in 0..5 {
+		let n = 5;
+		let mut hashes = Vec::with_capacity(n);
+		for _ in 0..n {
 			let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
+			hashes.push(block.hash());
 
 			futures::executor::block_on(client.import(BlockOrigin::Own, block)).unwrap();
 		}
 
-		let best = client.header(&BlockId::Hash(client.info().best_hash)).unwrap().unwrap();
+		let best = client.header(client.info().best_hash).unwrap().unwrap();
 		let best_number = *best.number();
 
-		for i in 0u32..5 {
-			let base = client.header(&BlockId::Number(i.into())).unwrap().unwrap();
+		for i in 0..n {
+			let base = client.header(hashes[i]).unwrap().unwrap();
 			let (_, number) = futures::executor::block_on(rule.restrict_vote(
 				client.clone(),
 				&base,

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

@@ -116,9 +116,12 @@ impl<Block: BlockT> WarpSyncProof<Block> {
 		let set_changes = set_changes.iter_from(begin_number).ok_or(Error::MissingData)?;
 
 		for (_, last_block) in set_changes {
-			let header = blockchain.header(BlockId::Number(*last_block))?.expect(
-				"header number comes from previously applied set changes; must exist in db; qed.",
-			);
+			let hash = blockchain.block_hash_from_id(&BlockId::Number(*last_block))?
+				.expect("header number comes from previously applied set changes; corresponding hash must exist in db; qed.");
+
+			let header = blockchain
+				.header(hash)?
+				.expect("header hash obtained from header number exists in db; corresponding header must exist in db too; qed.");
 
 			// the last block in a set is the one that triggers a change to the next set,
 			// therefore the block must have a digest that signals the authority set change
@@ -172,7 +175,7 @@ impl<Block: BlockT> WarpSyncProof<Block> {
 			});
 
 			if let Some(latest_justification) = latest_justification {
-				let header = blockchain.header(BlockId::Hash(latest_justification.target().1))?
+				let header = blockchain.header(latest_justification.target().1)?
 					.expect("header hash corresponds to a justification in db; must exist in db as well; qed.");
 
 				proofs.push(WarpSyncFragment { header, justification: latest_justification })