BlockId removal: refactor: HeaderBackend::expect_header

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

client/basic-authorship/src/basic_authorship.rs

@@ -614,16 +614,20 @@ mod tests {
 		block_on(txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0), extrinsic(1)]))
 			.unwrap();
 
-		block_on(txpool.maintain(chain_event(
-			client.expect_header(BlockId::Number(0u64)).expect("there should be header"),
-		)));
+		block_on(
+			txpool.maintain(chain_event(
+				client
+					.expect_header(client.info().genesis_hash)
+					.expect("there should be header"),
+			)),
+		);
 
 		let mut proposer_factory =
 			ProposerFactory::new(spawner.clone(), client.clone(), txpool.clone(), None, None);
 
 		let cell = Mutex::new((false, time::Instant::now()));
 		let proposer = proposer_factory.init_with_now(
-			&client.expect_header(BlockId::number(0)).unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				if !value.0 {
@@ -667,7 +671,7 @@ mod tests {
 
 		let cell = Mutex::new((false, time::Instant::now()));
 		let proposer = proposer_factory.init_with_now(
-			&client.expect_header(BlockId::number(0)).unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				if !value.0 {
@@ -703,9 +707,13 @@ mod tests {
 
 		block_on(txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0)])).unwrap();
 
-		block_on(txpool.maintain(chain_event(
-			client.expect_header(BlockId::Number(0u64)).expect("there should be header"),
-		)));
+		block_on(
+			txpool.maintain(chain_event(
+				client
+					.expect_header(client.info().genesis_hash)
+					.expect("there should be header"),
+			)),
+		);
 
 		let mut proposer_factory =
 			ProposerFactory::new(spawner.clone(), client.clone(), txpool.clone(), None, None);
@@ -779,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.expect_header(BlockId::number(number)).unwrap(),
+				&client.expect_header(hash).unwrap(),
 				Box::new(move || time::Instant::now()),
 			);
 
@@ -799,18 +808,25 @@ mod tests {
 			block
 		};
 
-		block_on(txpool.maintain(chain_event(
-			client.expect_header(BlockId::Number(0u64)).expect("there should be header"),
-		)));
+		block_on(
+			txpool.maintain(chain_event(
+				client
+					.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.expect_header(BlockId::Number(1)).expect("there should be header"),
-		)));
+		block_on(
+			txpool.maintain(chain_event(
+				client.expect_header(hashof1).expect("there should be header"),
+			)),
+		);
 		assert_eq!(txpool.ready().count(), 5);
 
 		// now let's make sure that we can still make some progress
@@ -829,8 +845,9 @@ mod tests {
 			spawner.clone(),
 			client.clone(),
 		);
-		let genesis_header =
-			client.expect_header(BlockId::Number(0u64)).expect("there should be header");
+		let genesis_header = client
+			.expect_header(client.info().genesis_hash)
+			.expect("there should be header");
 
 		let extrinsics_num = 5;
 		let extrinsics = std::iter::once(
@@ -940,17 +957,21 @@ mod tests {
 		)
 		.unwrap();
 
-		block_on(txpool.maintain(chain_event(
-			client.expect_header(BlockId::Number(0u64)).expect("there should be header"),
-		)));
+		block_on(
+			txpool.maintain(chain_event(
+				client
+					.expect_header(client.info().genesis_hash)
+					.expect("there should be header"),
+			)),
+		);
 		assert_eq!(txpool.ready().count(), MAX_SKIPPED_TRANSACTIONS * 3);
 
 		let mut proposer_factory =
 			ProposerFactory::new(spawner.clone(), client.clone(), txpool.clone(), None, None);
 
 		let cell = Mutex::new(time::Instant::now());
 		let proposer = proposer_factory.init_with_now(
-			&client.expect_header(BlockId::number(0)).unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				let old = *value;
@@ -998,9 +1019,13 @@ mod tests {
 		)
 		.unwrap();
 
-		block_on(txpool.maintain(chain_event(
-			client.expect_header(BlockId::Number(0u64)).expect("there should be header"),
-		)));
+		block_on(
+			txpool.maintain(chain_event(
+				client
+					.expect_header(client.info().genesis_hash)
+					.expect("there should be header"),
+			)),
+		);
 		assert_eq!(txpool.ready().count(), MAX_SKIPPED_TRANSACTIONS * 2 + 2);
 
 		let mut proposer_factory =
@@ -1010,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.expect_header(BlockId::number(0)).unwrap(),
+			&client.expect_header(client.info().genesis_hash).unwrap(),
 			Box::new(move || {
 				let mut value = cell.lock();
 				let (called, old) = *value;

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 @@ 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,15 +718,26 @@ 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();
@@ -1051,8 +1062,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

@@ -969,7 +969,7 @@ mod tests {
 			compatibility_mode: Default::default(),
 		};
 
-		let head = client.expect_header(BlockId::Number(0)).unwrap();
+		let head = client.expect_header(client.info().genesis_hash).unwrap();
 
 		let res = runtime
 			.block_on(worker.on_slot(SlotInfo {

client/db/src/lib.rs

@@ -1372,7 +1372,7 @@ impl<Block: BlockT> Backend<Block> {
 		};
 
 		for (block_hash, justification) in operation.finalized_blocks {
-			let block_header = self.blockchain.expect_header(BlockId::Hash(block_hash))?;
+			let block_header = self.blockchain.expect_header(block_hash)?;
 			meta_updates.push(self.finalize_block_with_transaction(
 				&mut transaction,
 				block_hash,
@@ -2010,7 +2010,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
 		justification: Option<Justification>,
 	) -> ClientResult<()> {
 		let mut transaction = Transaction::new();
-		let header = self.blockchain.expect_header(BlockId::Hash(hash))?;
+		let header = self.blockchain.expect_header(hash)?;
 		let mut displaced = None;
 
 		let m = self.finalize_block_with_transaction(
@@ -2032,7 +2032,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
 		justification: Justification,
 	) -> ClientResult<()> {
 		let mut transaction: Transaction<DbHash> = Transaction::new();
-		let header = self.blockchain.expect_header(BlockId::Hash(hash))?;
+		let header = self.blockchain.expect_header(hash)?;
 		let number = *header.number();
 
 		// Check if the block is finalized first.

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

client/finality-grandpa/src/tests.rs

@@ -1564,9 +1564,12 @@ fn justification_with_equivocation() {
 	net.peer(0).push_blocks(3, false);
 
 	let client = net.peer(0).client().as_client().clone();
-	let block1 = client.expect_header(BlockId::Number(1)).unwrap();
-	let block2 = client.expect_header(BlockId::Number(2)).unwrap();
-	let block3 = client.expect_header(BlockId::Number(3)).unwrap();
+	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 = {

client/finality-grandpa/src/warp_proof.rs

@@ -116,13 +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 hash = blockchain.hash(*last_block)?.expect(
-				"header number comes from previously applied set changes; corresponding hash 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 for given hash obtained successfully from db exists in db; qed.");
+				.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

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

@@ -1141,13 +1141,22 @@ fn get_block_by_bad_block_hash_returns_none() {
 }
 
 #[test]
-fn get_header_by_block_number_doesnt_panic() {
+fn expect_block_hash_by_block_number_doesnt_panic() {
 	let client = substrate_test_runtime_client::new();
 
 	// backend uses u32 for block numbers, make sure we don't panic when
 	// trying to convert
 	let id = BlockId::<Block>::Number(72340207214430721);
-	client.expect_header(id).expect_err("invalid block number overflows u32");
+	client.block_hash_from_id(&id).expect_err("invalid block number overflows u32");
+}
+
+#[test]
+fn get_hash_by_block_number_doesnt_panic() {
+	let client = substrate_test_runtime_client::new();
+
+	// backend uses u32 for block numbers, make sure we don't panic when
+	// trying to convert
+	client.hash(72340207214430721).expect_err("invalid block number overflows u32");
 }
 
 #[test]

primitives/blockchain/src/backend.rs

@@ -63,9 +63,9 @@ pub trait HeaderBackend<Block: BlockT>: Send + Sync {
 	}
 
 	/// Get block header. Returns `UnknownBlock` error if block is not found.
-	fn expect_header(&self, id: BlockId<Block>) -> Result<Block::Header> {
-		self.header(self.expect_block_hash_from_id(&id)?)?
-			.ok_or_else(|| Error::UnknownBlock(format!("Expect header: {}", id)))
+	fn expect_header(&self, hash: Block::Hash) -> Result<Block::Header> {
+		self.header(hash)?
+			.ok_or_else(|| Error::UnknownBlock(format!("Expect header: {}", hash)))
 	}
 
 	/// Convert an arbitrary block ID into a block number. Returns `UnknownBlock` error if block is