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 <>

bin/node/bench/src/construct.rs

@@ -143,7 +143,7 @@ impl core::Benchmark for ConstructionBenchmark {
 			proposer_factory.init(
 				&context
 					.client
-					.header(&BlockId::number(0))
+					.header(context.client.chain_info().genesis_hash)
 					.expect("Database error querying block #0")
 					.expect("Block #0 should exist"),
 			),

bin/node/cli/src/service.rs

@@ -640,9 +640,8 @@ mod tests {
 				Ok((node, setup_handles.unwrap()))
 			},
 			|service, &mut (ref mut block_import, ref babe_link)| {
-				let parent_id = BlockId::number(service.client().chain_info().best_number);
-				let parent_header = service.client().header(&parent_id).unwrap().unwrap();
-				let parent_hash = parent_header.hash();
+				let parent_hash = service.client().chain_info().best_hash;
+				let parent_header = service.client().header(parent_hash).unwrap().unwrap();
 				let parent_number = *parent_header.number();
 
 				futures::executor::block_on(service.transaction_pool().maintain(

bin/node/inspect/src/lib.rs

@@ -147,18 +147,17 @@ impl<TBlock: Block, TPrinter: PrettyPrinter<TBlock>> Inspector<TBlock, TPrinter>
 					.block_body(hash)?
 					.ok_or_else(|| Error::NotFound(not_found.clone()))?;
 				let header =
-					self.chain.header(id)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
+					self.chain.header(hash)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
 				TBlock::new(header, body)
 			},
 			BlockAddress::Hash(hash) => {
-				let id = BlockId::hash(hash);
-				let not_found = format!("Could not find block {:?}", id);
+				let not_found = format!("Could not find block {:?}", BlockId::<TBlock>::Hash(hash));
 				let body = self
 					.chain
 					.block_body(hash)?
 					.ok_or_else(|| Error::NotFound(not_found.clone()))?;
 				let header =
-					self.chain.header(id)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
+					self.chain.header(hash)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
 				TBlock::new(header, body)
 			},
 		})

client/api/src/in_mem.rs

@@ -225,7 +225,7 @@ impl<Block: BlockT> Blockchain<Block> {
 	/// Set an existing block as head.
 	pub fn set_head(&self, hash: Block::Hash) -> sp_blockchain::Result<()> {
 		let header = self
-			.header(BlockId::Hash(hash))?
+			.header(hash)?
 			.ok_or_else(|| sp_blockchain::Error::UnknownBlock(format!("{}", hash)))?;
 
 		self.apply_head(&header)
@@ -336,11 +336,9 @@ impl<Block: BlockT> Blockchain<Block> {
 impl<Block: BlockT> HeaderBackend<Block> for Blockchain<Block> {
 	fn header(
 		&self,
-		id: BlockId<Block>,
+		hash: Block::Hash,
 	) -> sp_blockchain::Result<Option<<Block as BlockT>::Header>> {
-		Ok(self
-			.id(id)
-			.and_then(|hash| self.storage.read().blocks.get(&hash).map(|b| b.header().clone())))
+		Ok(self.storage.read().blocks.get(&hash).map(|b| b.header().clone()))
 	}
 
 	fn info(&self) -> blockchain::Info<Block> {
@@ -387,7 +385,7 @@ impl<Block: BlockT> HeaderMetadata<Block> for Blockchain<Block> {
 		&self,
 		hash: Block::Hash,
 	) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
-		self.header(BlockId::hash(hash))?
+		self.header(hash)?
 			.map(|header| CachedHeaderMetadata::from(&header))
 			.ok_or_else(|| {
 				sp_blockchain::Error::UnknownBlock(format!("header not found: {}", hash))

client/authority-discovery/src/worker/tests.rs

@@ -58,7 +58,7 @@ impl ProvideRuntimeApi<Block> for TestApi {
 impl<Block: BlockT> HeaderBackend<Block> for TestApi {
 	fn header(
 		&self,
-		_id: BlockId<Block>,
+		_hash: Block::Hash,
 	) -> std::result::Result<Option<Block::Header>, sp_blockchain::Error> {
 		Ok(None)
 	}

client/basic-authorship/README.md

@@ -8,7 +8,7 @@ let mut proposer_factory = ProposerFactory::new(client.clone(), txpool.clone(),
 
 // From this factory, we create a `Proposer`.
 let proposer = proposer_factory.init(
-	&client.header(&BlockId::number(0)).unwrap().unwrap(),
+	&client.header(client.chain_info().genesis_hash).unwrap().unwrap(),
 );
 
 // The proposer is created asynchronously.

client/basic-authorship/src/basic_authorship.rs

@@ -617,8 +617,7 @@ mod tests {
 		block_on(
 			txpool.maintain(chain_event(
 				client
-					.header(&BlockId::Number(0u64))
-					.expect("header get error")
+					.expect_header(client.info().genesis_hash)
 					.expect("there should be header"),
 			)),
 		);
@@ -628,7 +627,7 @@ mod tests {
 
 		let cell = Mutex::new((false, time::Instant::now()));
 		let proposer = proposer_factory.init_with_now(
-			&client.header(&BlockId::number(0)).unwrap().unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				if !value.0 {
@@ -672,7 +671,7 @@ mod tests {
 
 		let cell = Mutex::new((false, time::Instant::now()));
 		let proposer = proposer_factory.init_with_now(
-			&client.header(&BlockId::number(0)).unwrap().unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				if !value.0 {
@@ -705,15 +704,13 @@ mod tests {
 		);
 
 		let genesis_hash = client.info().best_hash;
-		let block_id = BlockId::Hash(genesis_hash);
 
 		block_on(txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0)])).unwrap();
 
 		block_on(
 			txpool.maintain(chain_event(
 				client
-					.header(&BlockId::Number(0u64))
-					.expect("header get error")
+					.expect_header(client.info().genesis_hash)
 					.expect("there should be header"),
 			)),
 		);
@@ -722,7 +719,7 @@ mod tests {
 			ProposerFactory::new(spawner.clone(), client.clone(), txpool.clone(), None, None);
 
 		let proposer = proposer_factory.init_with_now(
-			&client.header(&block_id).unwrap().unwrap(),
+			&client.header(genesis_hash).unwrap().unwrap(),
 			Box::new(move || time::Instant::now()),
 		);
 
@@ -734,7 +731,7 @@ mod tests {
 		assert_eq!(proposal.block.extrinsics().len(), 1);
 
 		let api = client.runtime_api();
-		api.execute_block(&block_id, proposal.block).unwrap();
+		api.execute_block(&BlockId::Hash(genesis_hash), proposal.block).unwrap();
 
 		let state = backend.state_at(genesis_hash).unwrap();
 
@@ -790,8 +787,9 @@ mod tests {
 		                         number,
 		                         expected_block_extrinsics,
 		                         expected_pool_transactions| {
+			let hash = client.expect_block_hash_from_id(&BlockId::Number(number)).unwrap();
 			let proposer = proposer_factory.init_with_now(
-				&client.header(&BlockId::number(number)).unwrap().unwrap(),
+				&client.expect_header(hash).unwrap(),
 				Box::new(move || time::Instant::now()),
 			);
 
@@ -813,23 +811,20 @@ mod tests {
 		block_on(
 			txpool.maintain(chain_event(
 				client
-					.header(&BlockId::Number(0u64))
-					.expect("header get error")
+					.expect_header(client.info().genesis_hash)
 					.expect("there should be header"),
 			)),
 		);
 		assert_eq!(txpool.ready().count(), 7);
 
 		// let's create one block and import it
 		let block = propose_block(&client, 0, 2, 7);
+		let hashof1 = block.hash();
 		block_on(client.import(BlockOrigin::Own, block)).unwrap();
 
 		block_on(
 			txpool.maintain(chain_event(
-				client
-					.header(&BlockId::Number(1))
-					.expect("header get error")
-					.expect("there should be header"),
+				client.expect_header(hashof1).expect("there should be header"),
 			)),
 		);
 		assert_eq!(txpool.ready().count(), 5);
@@ -851,8 +846,7 @@ mod tests {
 			client.clone(),
 		);
 		let genesis_header = client
-			.header(&BlockId::Number(0u64))
-			.expect("header get error")
+			.expect_header(client.info().genesis_hash)
 			.expect("there should be header");
 
 		let extrinsics_num = 5;
@@ -966,8 +960,7 @@ mod tests {
 		block_on(
 			txpool.maintain(chain_event(
 				client
-					.header(&BlockId::Number(0u64))
-					.expect("header get error")
+					.expect_header(client.info().genesis_hash)
 					.expect("there should be header"),
 			)),
 		);
@@ -978,7 +971,7 @@ mod tests {
 
 		let cell = Mutex::new(time::Instant::now());
 		let proposer = proposer_factory.init_with_now(
-			&client.header(&BlockId::number(0)).unwrap().unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				let old = *value;
@@ -1029,8 +1022,7 @@ mod tests {
 		block_on(
 			txpool.maintain(chain_event(
 				client
-					.header(&BlockId::Number(0u64))
-					.expect("header get error")
+					.expect_header(client.info().genesis_hash)
 					.expect("there should be header"),
 			)),
 		);
@@ -1043,7 +1035,7 @@ mod tests {
 		let cell = Arc::new(Mutex::new((0, time::Instant::now())));
 		let cell2 = cell.clone();
 		let proposer = proposer_factory.init_with_now(
-			&client.header(&BlockId::number(0)).unwrap().unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				let (called, old) = *value;

client/basic-authorship/src/lib.rs

@@ -50,7 +50,7 @@
 //!
 //! // From this factory, we create a `Proposer`.
 //! let proposer = proposer_factory.init(
-//! 	&client.header(&BlockId::number(0)).unwrap().unwrap(),
+//! 	&client.header(client.chain_info().genesis_hash).unwrap().unwrap(),
 //! );
 //!
 //! // The proposer is created asynchronously.

client/beefy/src/lib.rs

@@ -412,7 +412,7 @@ where
 			})?;
 
 		// Move up the chain.
-		header = blockchain.expect_header(BlockId::Hash(parent_hash))?;
+		header = blockchain.expect_header(parent_hash)?;
 	};
 
 	aux_schema::write_current_version(backend)?;

client/beefy/src/tests.rs

@@ -448,9 +448,8 @@ async fn wait_for_best_beefy_blocks(
 		wait_for.push(Box::pin(stream.take(len).for_each(move |best_beefy_hash| {
 			let expected = expected.next();
 			async move {
-				let block_id = BlockId::hash(best_beefy_hash);
 				let header =
-					net.lock().peer(i).client().as_client().expect_header(block_id).unwrap();
+					net.lock().peer(i).client().as_client().expect_header(best_beefy_hash).unwrap();
 				let best_beefy = *header.number();
 
 				assert_eq!(expected, Some(best_beefy).as_ref());

client/beefy/src/worker.rs

@@ -465,7 +465,7 @@ where
 				.map(|hash| {
 					backend
 						.blockchain()
-						.expect_header(BlockId::hash(*hash))
+						.expect_header(*hash)
 						.expect("just finalized block should be available; qed.")
 				})
 				.chain(std::iter::once(header.clone()))
@@ -718,16 +718,25 @@ where
 		let target_header = if target_number == self.best_grandpa_block() {
 			self.persisted_state.best_grandpa_block_header.clone()
 		} else {
-			self.backend
+			let hash = self
+				.backend
 				.blockchain()
-				.expect_header(BlockId::Number(target_number))
+				.expect_block_hash_from_id(&BlockId::Number(target_number))
 				.map_err(|err| {
 					let err_msg = format!(
-						"Couldn't get header for block #{:?} (error: {:?}), skipping vote..",
+						"Couldn't get hash for block #{:?} (error: {:?}), skipping vote..",
 						target_number, err
 					);
 					Error::Backend(err_msg)
-				})?
+				})?;
+
+			self.backend.blockchain().expect_header(hash).map_err(|err| {
+				let err_msg = format!(
+					"Couldn't get header for block #{:?} ({:?}) (error: {:?}), skipping vote..",
+					target_number, hash, err
+				);
+				Error::Backend(err_msg)
+			})?
 		};
 		let target_hash = target_header.hash();
 
@@ -1050,8 +1059,10 @@ pub(crate) mod tests {
 			"/beefy/justifs/1".into(),
 			known_peers,
 		);
-		let at = BlockId::number(Zero::zero());
-		let genesis_header = backend.blockchain().expect_header(at).unwrap();
+		let genesis_header = backend
+			.blockchain()
+			.expect_header(backend.blockchain().info().genesis_hash)
+			.unwrap();
 		let persisted_state = PersistedState::checked_new(
 			genesis_header,
 			Zero::zero(),

client/consensus/aura/src/lib.rs

@@ -957,7 +957,7 @@ mod tests {
 			compatibility_mode: Default::default(),
 		};
 
-		let head = client.header(&BlockId::Number(0)).unwrap().unwrap();
+		let head = client.expect_header(client.info().genesis_hash).unwrap();
 
 		let res = worker
 			.on_slot(SlotInfo {
@@ -972,6 +972,6 @@ mod tests {
 			.unwrap();
 
 		// The returned block should be imported and we should be able to get its header by now.
-		assert!(client.header(&BlockId::Hash(res.block.hash())).unwrap().is_some());
+		assert!(client.header(res.block.hash()).unwrap().is_some());
 	}
 }

client/consensus/babe/src/lib.rs

@@ -1425,7 +1425,7 @@ where
 		let parent_hash = *block.header.parent_hash();
 		let parent_header = self
 			.client
-			.header(BlockId::Hash(parent_hash))
+			.header(parent_hash)
 			.map_err(|e| ConsensusError::ChainLookup(e.to_string()))?
 			.ok_or_else(|| {
 				ConsensusError::ChainLookup(
@@ -1664,7 +1664,7 @@ where
 
 	let finalized_slot = {
 		let finalized_header = client
-			.header(BlockId::Hash(info.finalized_hash))
+			.header(info.finalized_hash)
 			.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
 			.expect(
 				"best finalized hash was given by client; finalized headers must exist in db; qed",