BlockId removal: refactor: StorageProvider (#12510)

* BlockId removal: refactor: StorageProvider

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

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

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update client/api/src/backend.rs

Co-authored-by: Dmitrii Markin <dmitry@markin.tech>

* GrandpaBlockImport::current_set_id reworked

* ExportStateCmd reworked

* trigger CI job

* trigger CI job

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Dmitrii Markin <dmitry@markin.tech>

substrate/client/api/src/backend.rs

@@ -357,77 +357,77 @@ where
 
 /// Provides acess to storage primitives
 pub trait StorageProvider<Block: BlockT, B: Backend<Block>> {
-	/// Given a `BlockId` and a key, return the value under the key in that block.
+	/// Given a block's `Hash` and a key, return the value under the key in that block.
 	fn storage(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<StorageData>>;
 
-	/// Given a `BlockId` and a key prefix, return the matching storage keys in that block.
+	/// Given a block's `Hash` and a key prefix, return the matching storage keys in that block.
 	fn storage_keys(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<StorageKey>>;
 
-	/// Given a `BlockId` and a key, return the value under the hash in that block.
+	/// Given a block's `Hash` and a key, return the value under the hash in that block.
 	fn storage_hash(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>>;
 
-	/// Given a `BlockId` and a key prefix, return the matching child storage keys and values in
-	/// that block.
+	/// Given a block's `Hash` and a key prefix, return the matching child storage keys and values
+	/// in that block.
 	fn storage_pairs(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<(StorageKey, StorageData)>>;
 
-	/// Given a `BlockId` and a key prefix, return a `KeyIterator` iterates matching storage keys in
-	/// that block.
+	/// Given a block's `Hash` and a key prefix, return a `KeyIterator` iterates matching storage
+	/// keys in that block.
 	fn storage_keys_iter<'a>(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		prefix: Option<&'a StorageKey>,
 		start_key: Option<&StorageKey>,
 	) -> sp_blockchain::Result<KeyIterator<'a, B::State, Block>>;
 
-	/// Given a `BlockId`, a key and a child storage key, return the value under the key in that
-	/// block.
+	/// Given a block's `Hash`, a key and a child storage key, return the value under the key in
+	/// that block.
 	fn child_storage(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		child_info: &ChildInfo,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<StorageData>>;
 
-	/// Given a `BlockId`, a key prefix, and a child storage key, return the matching child storage
-	/// keys.
+	/// Given a block's `Hash`, a key prefix, and a child storage key, return the matching child
+	/// storage keys.
 	fn child_storage_keys(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		child_info: &ChildInfo,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<StorageKey>>;
 
-	/// Given a `BlockId` and a key `prefix` and a child storage key,
+	/// Given a block's `Hash` and a key `prefix` and a child storage key,
 	/// return a `KeyIterator` that iterates matching storage keys in that block.
 	fn child_storage_keys_iter<'a>(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		child_info: ChildInfo,
 		prefix: Option<&'a StorageKey>,
 		start_key: Option<&StorageKey>,
 	) -> sp_blockchain::Result<KeyIterator<'a, B::State, Block>>;
 
-	/// Given a `BlockId`, a key and a child storage key, return the hash under the key in that
+	/// Given a block's `Hash`, a key and a child storage key, return the hash under the key in that
 	/// block.
 	fn child_storage_hash(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		child_info: &ChildInfo,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>>;

substrate/client/cli/src/commands/export_state_cmd.rs

@@ -23,7 +23,7 @@ use crate::{
 };
 use clap::Parser;
 use log::info;
-use sc_client_api::{StorageProvider, UsageProvider};
+use sc_client_api::{HeaderBackend, StorageProvider, UsageProvider};
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 use std::{fmt::Debug, io::Write, str::FromStr, sync::Arc};
 
@@ -57,7 +57,7 @@ impl ExportStateCmd {
 	) -> error::Result<()>
 	where
 		B: BlockT,
-		C: UsageProvider<B> + StorageProvider<B, BA>,
+		C: UsageProvider<B> + StorageProvider<B, BA> + HeaderBackend<B>,
 		BA: sc_client_api::backend::Backend<B>,
 		B::Hash: FromStr,
 		<B::Hash as FromStr>::Err: Debug,
@@ -65,7 +65,11 @@ impl ExportStateCmd {
 	{
 		info!("Exporting raw state...");
 		let block_id = self.input.as_ref().map(|b| b.parse()).transpose()?;
-		let raw_state = sc_service::chain_ops::export_raw_state(client, block_id)?;
+		let hash = match block_id {
+			Some(id) => client.expect_block_hash_from_id(&id)?,
+			None => client.usage_info().chain.best_hash,
+		};
+		let raw_state = sc_service::chain_ops::export_raw_state(client, &hash)?;
 		input_spec.set_storage(raw_state);
 
 		info!("Generating new chain spec...");

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

@@ -424,13 +424,15 @@ where
 	}
 
 	/// Read current set id form a given state.
-	fn current_set_id(&self, id: &BlockId<Block>) -> Result<SetId, ConsensusError> {
+	fn current_set_id(&self, hash: &Block::Hash) -> Result<SetId, ConsensusError> {
+		let id = &BlockId::hash(*hash);
 		let runtime_version = self.inner.runtime_api().version(id).map_err(|e| {
 			ConsensusError::ClientImport(format!(
 				"Unable to retrieve current runtime version. {}",
 				e
 			))
 		})?;
+
 		if runtime_version
 			.api_version(&<dyn GrandpaApi<Block>>::ID)
 			.map_or(false, |v| v < 3)
@@ -439,7 +441,8 @@ where
 			// This code may be removed once warp sync to an old runtime is no longer needed.
 			for prefix in ["GrandpaFinality", "Grandpa"] {
 				let k = [twox_128(prefix.as_bytes()), twox_128(b"CurrentSetId")].concat();
-				if let Ok(Some(id)) = self.inner.storage(id, &sc_client_api::StorageKey(k.to_vec()))
+				if let Ok(Some(id)) =
+					self.inner.storage(hash, &sc_client_api::StorageKey(k.to_vec()))
 				{
 					if let Ok(id) = SetId::decode(&mut id.0.as_ref()) {
 						return Ok(id)
@@ -472,13 +475,12 @@ where
 				// finality proofs and that the state is correct and final.
 				// So we can read the authority list and set id from the state.
 				self.authority_set_hard_forks.clear();
-				let block_id = BlockId::hash(hash);
 				let authorities = self
 					.inner
 					.runtime_api()
-					.grandpa_authorities(&block_id)
+					.grandpa_authorities(&BlockId::hash(hash))
 					.map_err(|e| ConsensusError::ClientImport(e.to_string()))?;
-				let set_id = self.current_set_id(&block_id)?;
+				let set_id = self.current_set_id(&hash)?;
 				let authority_set = AuthoritySet::new(
 					authorities.clone(),
 					set_id,

substrate/client/rpc/src/state/state_full.rs

@@ -145,10 +145,9 @@ where
 	) -> Result<()> {
 		for block_hash in &range.hashes {
 			let mut block_changes = StorageChangeSet { block: *block_hash, changes: Vec::new() };
-			let id = BlockId::hash(*block_hash);
 			for key in keys {
 				let (has_changed, data) = {
-					let curr_data = self.client.storage(&id, key).map_err(client_err)?;
+					let curr_data = self.client.storage(block_hash, key).map_err(client_err)?;
 					match last_values.get(key) {
 						Some(prev_data) => (curr_data != *prev_data, curr_data),
 						None => (true, curr_data),
@@ -214,7 +213,7 @@ where
 		prefix: StorageKey,
 	) -> std::result::Result<Vec<StorageKey>, Error> {
 		self.block_or_best(block)
-			.and_then(|block| self.client.storage_keys(&BlockId::Hash(block), &prefix))
+			.and_then(|block| self.client.storage_keys(&block, &prefix))
 			.map_err(client_err)
 	}
 
@@ -224,7 +223,7 @@ where
 		prefix: StorageKey,
 	) -> std::result::Result<Vec<(StorageKey, StorageData)>, Error> {
 		self.block_or_best(block)
-			.and_then(|block| self.client.storage_pairs(&BlockId::Hash(block), &prefix))
+			.and_then(|block| self.client.storage_pairs(&block, &prefix))
 			.map_err(client_err)
 	}
 
@@ -237,11 +236,7 @@ where
 	) -> std::result::Result<Vec<StorageKey>, Error> {
 		self.block_or_best(block)
 			.and_then(|block| {
-				self.client.storage_keys_iter(
-					&BlockId::Hash(block),
-					prefix.as_ref(),
-					start_key.as_ref(),
-				)
+				self.client.storage_keys_iter(&block, prefix.as_ref(), start_key.as_ref())
 			})
 			.map(|iter| iter.take(count as usize).collect())
 			.map_err(client_err)
@@ -253,7 +248,7 @@ where
 		key: StorageKey,
 	) -> std::result::Result<Option<StorageData>, Error> {
 		self.block_or_best(block)
-			.and_then(|block| self.client.storage(&BlockId::Hash(block), &key))
+			.and_then(|block| self.client.storage(&block, &key))
 			.map_err(client_err)
 	}
 
@@ -267,14 +262,14 @@ where
 			Err(e) => return Err(client_err(e)),
 		};
 
-		match self.client.storage(&BlockId::Hash(block), &key) {
+		match self.client.storage(&block, &key) {
 			Ok(Some(d)) => return Ok(Some(d.0.len() as u64)),
 			Err(e) => return Err(client_err(e)),
 			Ok(None) => {},
 		}
 
 		self.client
-			.storage_pairs(&BlockId::Hash(block), &key)
+			.storage_pairs(&block, &key)
 			.map(|kv| {
 				let item_sum = kv.iter().map(|(_, v)| v.0.len() as u64).sum::<u64>();
 				if item_sum > 0 {
@@ -292,7 +287,7 @@ where
 		key: StorageKey,
 	) -> std::result::Result<Option<Block::Hash>, Error> {
 		self.block_or_best(block)
-			.and_then(|block| self.client.storage_hash(&BlockId::Hash(block), &key))
+			.and_then(|block| self.client.storage_hash(&block, &key))
 			.map_err(client_err)
 	}
 
@@ -418,7 +413,7 @@ where
 			let changes = keys
 				.into_iter()
 				.map(|key| {
-					let v = self.client.storage(&BlockId::Hash(block), &key).ok().flatten();
+					let v = self.client.storage(&block, &key).ok().flatten();
 					(key, v)
 				})
 				.collect();
@@ -522,7 +517,7 @@ where
 						ChildInfo::new_default(storage_key),
 					None => return Err(sp_blockchain::Error::InvalidChildStorageKey),
 				};
-				self.client.child_storage_keys(&BlockId::Hash(block), &child_info, &prefix)
+				self.client.child_storage_keys(&block, &child_info, &prefix)
 			})
 			.map_err(client_err)
 	}
@@ -543,7 +538,7 @@ where
 					None => return Err(sp_blockchain::Error::InvalidChildStorageKey),
 				};
 				self.client.child_storage_keys_iter(
-					&BlockId::Hash(block),
+					&block,
 					child_info,
 					prefix.as_ref(),
 					start_key.as_ref(),
@@ -566,7 +561,7 @@ where
 						ChildInfo::new_default(storage_key),
 					None => return Err(sp_blockchain::Error::InvalidChildStorageKey),
 				};
-				self.client.child_storage(&BlockId::Hash(block), &child_info, &key)
+				self.client.child_storage(&block, &child_info, &key)
 			})
 			.map_err(client_err)
 	}
@@ -589,10 +584,7 @@ where
 
 		keys.into_iter()
 			.map(move |key| {
-				client
-					.clone()
-					.child_storage(&BlockId::Hash(block), &child_info, &key)
-					.map_err(client_err)
+				client.clone().child_storage(&block, &child_info, &key).map_err(client_err)
 			})
 			.collect()
 	}
@@ -610,7 +602,7 @@ where
 						ChildInfo::new_default(storage_key),
 					None => return Err(sp_blockchain::Error::InvalidChildStorageKey),
 				};
-				self.client.child_storage_hash(&BlockId::Hash(block), &child_info, &key)
+				self.client.child_storage_hash(&block, &child_info, &key)
 			})
 			.map_err(client_err)
 	}

substrate/client/service/src/chain_ops/export_raw_state.rs

@@ -19,25 +19,20 @@
 use crate::error::Error;
 use sc_client_api::{StorageProvider, UsageProvider};
 use sp_core::storage::{well_known_keys, ChildInfo, Storage, StorageChild, StorageKey, StorageMap};
-use sp_runtime::{generic::BlockId, traits::Block as BlockT};
+use sp_runtime::traits::Block as BlockT;
 
 use std::{collections::HashMap, sync::Arc};
 
 /// Export the raw state at the given `block`. If `block` is `None`, the
 /// best block will be used.
-pub fn export_raw_state<B, BA, C>(
-	client: Arc<C>,
-	block: Option<BlockId<B>>,
-) -> Result<Storage, Error>
+pub fn export_raw_state<B, BA, C>(client: Arc<C>, hash: &B::Hash) -> Result<Storage, Error>
 where
 	C: UsageProvider<B> + StorageProvider<B, BA>,
 	B: BlockT,
 	BA: sc_client_api::backend::Backend<B>,
 {
-	let block = block.unwrap_or_else(|| BlockId::Hash(client.usage_info().chain.best_hash));
-
 	let empty_key = StorageKey(Vec::new());
-	let mut top_storage = client.storage_pairs(&block, &empty_key)?;
+	let mut top_storage = client.storage_pairs(hash, &empty_key)?;
 	let mut children_default = HashMap::new();
 
 	// Remove all default child storage roots from the top storage and collect the child storage
@@ -52,10 +47,10 @@ where
 			StorageKey(key.0[well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX.len()..].to_vec());
 		let child_info = ChildInfo::new_default(&key.0);
 
-		let keys = client.child_storage_keys(&block, &child_info, &empty_key)?;
+		let keys = client.child_storage_keys(hash, &child_info, &empty_key)?;
 		let mut pairs = StorageMap::new();
 		keys.into_iter().try_for_each(|k| {
-			if let Some(value) = client.child_storage(&block, &child_info, &k)? {
+			if let Some(value) = client.child_storage(hash, &child_info, &k)? {
 				pairs.insert(k.0, value.0);
 			}
 

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

@@ -420,7 +420,8 @@ where
 
 	/// Get the code at a given block.
 	pub fn code_at(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Vec<u8>> {
-		Ok(StorageProvider::storage(self, id, &StorageKey(well_known_keys::CODE.to_vec()))?
+		let hash = self.backend.blockchain().expect_block_hash_from_id(id)?;
+		Ok(StorageProvider::storage(self, &hash, &StorageKey(well_known_keys::CODE.to_vec()))?
 			.expect(
 				"None is returned if there's no value stored for the given key;\
 				':code' key is always defined; qed",
@@ -1402,21 +1403,19 @@ where
 {
 	fn storage_keys(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<StorageKey>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		let keys = self.state_at(&hash)?.keys(&key_prefix.0).into_iter().map(StorageKey).collect();
+		let keys = self.state_at(hash)?.keys(&key_prefix.0).into_iter().map(StorageKey).collect();
 		Ok(keys)
 	}
 
 	fn storage_pairs(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<(StorageKey, StorageData)>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		let state = self.state_at(&hash)?;
+		let state = self.state_at(hash)?;
 		let keys = state
 			.keys(&key_prefix.0)
 			.into_iter()
@@ -1430,37 +1429,34 @@ where
 
 	fn storage_keys_iter<'a>(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		prefix: Option<&'a StorageKey>,
 		start_key: Option<&StorageKey>,
 	) -> sp_blockchain::Result<KeyIterator<'a, B::State, Block>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		let state = self.state_at(&hash)?;
+		let state = self.state_at(hash)?;
 		let start_key = start_key.or(prefix).map(|key| key.0.clone()).unwrap_or_else(Vec::new);
 		Ok(KeyIterator::new(state, prefix, start_key))
 	}
 
 	fn child_storage_keys_iter<'a>(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		child_info: ChildInfo,
 		prefix: Option<&'a StorageKey>,
 		start_key: Option<&StorageKey>,
 	) -> sp_blockchain::Result<KeyIterator<'a, B::State, Block>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		let state = self.state_at(&hash)?;
+		let state = self.state_at(hash)?;
 		let start_key = start_key.or(prefix).map(|key| key.0.clone()).unwrap_or_else(Vec::new);
 		Ok(KeyIterator::new_child(state, child_info, prefix, start_key))
 	}
 
 	fn storage(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<StorageData>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
 		Ok(self
-			.state_at(&hash)?
+			.state_at(hash)?
 			.storage(&key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?
 			.map(StorageData))
@@ -1468,24 +1464,22 @@ where
 
 	fn storage_hash(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		self.state_at(&hash)?
+		self.state_at(hash)?
 			.storage_hash(&key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))
 	}
 
 	fn child_storage_keys(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		child_info: &ChildInfo,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<StorageKey>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
 		let keys = self
-			.state_at(&hash)?
+			.state_at(hash)?
 			.child_keys(child_info, &key_prefix.0)
 			.into_iter()
 			.map(StorageKey)
@@ -1495,13 +1489,12 @@ where
 
 	fn child_storage(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		child_info: &ChildInfo,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<StorageData>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
 		Ok(self
-			.state_at(&hash)?
+			.state_at(hash)?
 			.child_storage(child_info, &key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?
 			.map(StorageData))
@@ -1509,12 +1502,11 @@ where
 
 	fn child_storage_hash(
 		&self,
-		id: &BlockId<Block>,
+		hash: &<Block as BlockT>::Hash,
 		child_info: &ChildInfo,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
-		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
-		self.state_at(&hash)?
+		self.state_at(hash)?
 			.child_storage_hash(child_info, &key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))
 	}

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

@@ -1602,12 +1602,14 @@ fn storage_keys_iter_prefix_and_start_key_works() {
 		.add_extra_child_storage(&child_info, b"third".to_vec(), vec![0u8; 32])
 		.build();
 
+	let block_hash = client.info().best_hash;
+
 	let child_root = b":child_storage:default:child".to_vec();
 	let prefix = StorageKey(array_bytes::hex2bytes_unchecked("3a"));
 	let child_prefix = StorageKey(b"sec".to_vec());
 
 	let res: Vec<_> = client
-		.storage_keys_iter(&BlockId::Number(0), Some(&prefix), None)
+		.storage_keys_iter(&block_hash, Some(&prefix), None)
 		.unwrap()
 		.map(|x| x.0)
 		.collect();
@@ -1622,7 +1624,7 @@ fn storage_keys_iter_prefix_and_start_key_works() {
 
 	let res: Vec<_> = client
 		.storage_keys_iter(
-			&BlockId::Number(0),
+			&block_hash,
 			Some(&prefix),
 			Some(&StorageKey(array_bytes::hex2bytes_unchecked("3a636f6465"))),
 		)
@@ -1633,7 +1635,7 @@ fn storage_keys_iter_prefix_and_start_key_works() {
 
 	let res: Vec<_> = client
 		.storage_keys_iter(
-			&BlockId::Number(0),
+			&block_hash,
 			Some(&prefix),
 			Some(&StorageKey(array_bytes::hex2bytes_unchecked("3a686561707061676573"))),
 		)
@@ -1643,7 +1645,7 @@ fn storage_keys_iter_prefix_and_start_key_works() {
 	assert_eq!(res, Vec::<Vec<u8>>::new());
 
 	let res: Vec<_> = client
-		.child_storage_keys_iter(&BlockId::Number(0), child_info.clone(), Some(&child_prefix), None)
+		.child_storage_keys_iter(&block_hash, child_info.clone(), Some(&child_prefix), None)
 		.unwrap()
 		.map(|x| x.0)
 		.collect();
@@ -1651,7 +1653,7 @@ fn storage_keys_iter_prefix_and_start_key_works() {
 
 	let res: Vec<_> = client
 		.child_storage_keys_iter(
-			&BlockId::Number(0),
+			&block_hash,
 			child_info,
 			None,
 			Some(&StorageKey(b"second".to_vec())),
@@ -1666,10 +1668,12 @@ fn storage_keys_iter_prefix_and_start_key_works() {
 fn storage_keys_iter_works() {
 	let client = substrate_test_runtime_client::new();
 
+	let block_hash = client.info().best_hash;
+
 	let prefix = StorageKey(array_bytes::hex2bytes_unchecked(""));
 
 	let res: Vec<_> = client
-		.storage_keys_iter(&BlockId::Number(0), Some(&prefix), None)
+		.storage_keys_iter(&block_hash, Some(&prefix), None)
 		.unwrap()
 		.take(9)
 		.map(|x| array_bytes::bytes2hex("", &x.0))
@@ -1691,7 +1695,7 @@ fn storage_keys_iter_works() {
 
 	let res: Vec<_> = client
 		.storage_keys_iter(
-			&BlockId::Number(0),
+			&block_hash,
 			Some(&prefix),
 			Some(&StorageKey(array_bytes::hex2bytes_unchecked("3a636f6465"))),
 		)
@@ -1714,7 +1718,7 @@ fn storage_keys_iter_works() {
 
 	let res: Vec<_> = client
 		.storage_keys_iter(
-			&BlockId::Number(0),
+			&block_hash,
 			Some(&prefix),
 			Some(&StorageKey(array_bytes::hex2bytes_unchecked(
 				"7d5007603a7f5dd729d51d93cf695d6465789443bb967c0d1fe270e388c96eaa",

substrate/utils/frame/benchmarking-cli/src/block/bench.rs

@@ -22,7 +22,9 @@ use frame_support::weights::constants::WEIGHT_PER_NANOS;
 use frame_system::ConsumedWeight;
 use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider};
 use sc_cli::{Error, Result};
-use sc_client_api::{Backend as ClientBackend, BlockBackend, StorageProvider, UsageProvider};
+use sc_client_api::{
+	Backend as ClientBackend, BlockBackend, HeaderBackend, StorageProvider, UsageProvider,
+};
 use sp_api::{ApiExt, Core, HeaderT, ProvideRuntimeApi};
 use sp_blockchain::Error::RuntimeApiError;
 use sp_runtime::{generic::BlockId, traits::Block as BlockT, DigestItem, OpaqueExtrinsic};
@@ -73,7 +75,8 @@ where
 		+ ProvideRuntimeApi<Block>
 		+ StorageProvider<Block, BA>
 		+ UsageProvider<Block>
-		+ BlockBackend<Block>,
+		+ BlockBackend<Block>
+		+ HeaderBackend<Block>,
 	C::Api: ApiExt<Block, StateBackend = BA::State> + BlockBuilderApi<Block>,
 {
 	/// Returns a new [`Self`] from the arguments.
@@ -136,9 +139,10 @@ where
 		)?;
 		let key = StorageKey(hash);
 
+		let block_hash = self.client.expect_block_hash_from_id(block)?;
 		let mut raw_weight = &self
 			.client
-			.storage(&block, &key)?
+			.storage(&block_hash, &key)?
 			.ok_or(format!("Could not find System::BlockWeight for block: {}", block))?
 			.0[..];
 

substrate/utils/frame/benchmarking-cli/src/block/cmd.rs

@@ -22,6 +22,7 @@ use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider};
 use sc_cli::{CliConfiguration, ImportParams, Result, SharedParams};
 use sc_client_api::{Backend as ClientBackend, BlockBackend, StorageProvider, UsageProvider};
 use sp_api::{ApiExt, ProvideRuntimeApi};
+use sp_blockchain::HeaderBackend;
 use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic};
 
 use clap::Parser;
@@ -87,7 +88,8 @@ impl BlockCmd {
 			+ BlockBackend<Block>
 			+ ProvideRuntimeApi<Block>
 			+ StorageProvider<Block, BA>
-			+ UsageProvider<Block>,
+			+ UsageProvider<Block>
+			+ HeaderBackend<Block>,
 		C::Api: ApiExt<Block, StateBackend = BA::State> + BlockBuilderApi<Block>,
 	{
 		// Put everything in the benchmark type to have the generic types handy.

substrate/utils/frame/benchmarking-cli/src/storage/cmd.rs

@@ -191,9 +191,9 @@ impl StorageCmd {
 		B: BlockT + Debug,
 		BA: ClientBackend<B>,
 	{
-		let block = BlockId::Hash(client.usage_info().chain.best_hash);
+		let hash = client.usage_info().chain.best_hash;
 		let empty_prefix = StorageKey(Vec::new());
-		let mut keys = client.storage_keys(&block, &empty_prefix)?;
+		let mut keys = client.storage_keys(&hash, &empty_prefix)?;
 		let (mut rng, _) = new_rng(None);
 		keys.shuffle(&mut rng);
 
@@ -201,7 +201,7 @@ impl StorageCmd {
 			info!("Warmup round {}/{}", i + 1, self.params.warmups);
 			for key in keys.as_slice() {
 				let _ = client
-					.storage(&block, &key)
+					.storage(&hash, &key)
 					.expect("Checked above to exist")
 					.ok_or("Value unexpectedly empty");
 			}

substrate/utils/frame/benchmarking-cli/src/storage/read.rs

@@ -18,10 +18,7 @@
 use sc_cli::Result;
 use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider};
 use sp_core::storage::StorageKey;
-use sp_runtime::{
-	generic::BlockId,
-	traits::{Block as BlockT, Header as HeaderT},
-};
+use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 
 use log::info;
 use rand::prelude::*;
@@ -41,12 +38,12 @@ impl StorageCmd {
 		<<B as BlockT>::Header as HeaderT>::Number: From<u32>,
 	{
 		let mut record = BenchRecord::default();
-		let block = BlockId::Hash(client.usage_info().chain.best_hash);
+		let best_hash = client.usage_info().chain.best_hash;
 
-		info!("Preparing keys from block {}", block);
+		info!("Preparing keys from block {}", best_hash);
 		// Load all keys and randomly shuffle them.
 		let empty_prefix = StorageKey(Vec::new());
-		let mut keys = client.storage_keys(&block, &empty_prefix)?;
+		let mut keys = client.storage_keys(&best_hash, &empty_prefix)?;
 		let (mut rng, _) = new_rng(None);
 		keys.shuffle(&mut rng);
 
@@ -58,7 +55,7 @@ impl StorageCmd {
 			match (self.params.include_child_trees, self.is_child_key(key.clone().0)) {
 				(true, Some(info)) => {
 					// child tree key
-					let child_keys = client.child_storage_keys(&block, &info, &empty_prefix)?;
+					let child_keys = client.child_storage_keys(&best_hash, &info, &empty_prefix)?;
 					for ck in child_keys {
 						child_nodes.push((ck.clone(), info.clone()));
 					}
@@ -67,7 +64,7 @@ impl StorageCmd {
 					// regular key
 					let start = Instant::now();
 					let v = client
-						.storage(&block, &key)
+						.storage(&best_hash, &key)
 						.expect("Checked above to exist")
 						.ok_or("Value unexpectedly empty")?;
 					record.append(v.0.len(), start.elapsed())?;
@@ -82,7 +79,7 @@ impl StorageCmd {
 			for (key, info) in child_nodes.as_slice() {
 				let start = Instant::now();
 				let v = client
-					.child_storage(&block, info, key)
+					.child_storage(&best_hash, info, key)
 					.expect("Checked above to exist")
 					.ok_or("Value unexpectedly empty")?;
 				record.append(v.0.len(), start.elapsed())?;

substrate/utils/frame/benchmarking-cli/src/storage/write.rs

@@ -57,12 +57,12 @@ impl StorageCmd {
 		// Store the time that it took to write each value.
 		let mut record = BenchRecord::default();
 
-		let block = BlockId::Hash(client.usage_info().chain.best_hash);
-		let header = client.header(block)?.ok_or("Header not found")?;
+		let best_hash = client.usage_info().chain.best_hash;
+		let header = client.header(BlockId::Hash(best_hash))?.ok_or("Header not found")?;
 		let original_root = *header.state_root();
 		let trie = DbStateBuilder::<Block>::new(storage.clone(), original_root).build();
 
-		info!("Preparing keys from block {}", block);
+		info!("Preparing keys from block {}", best_hash);
 		// Load all KV pairs and randomly shuffle them.
 		let mut kvs = trie.pairs();
 		let (mut rng, _) = new_rng(None);
@@ -77,7 +77,7 @@ impl StorageCmd {
 			match (self.params.include_child_trees, self.is_child_key(k.to_vec())) {
 				(true, Some(info)) => {
 					let child_keys =
-						client.child_storage_keys_iter(&block, info.clone(), None, None)?;
+						client.child_storage_keys_iter(&best_hash, info.clone(), None, None)?;
 					for ck in child_keys {
 						child_nodes.push((ck.clone(), info.clone()));
 					}
@@ -124,7 +124,7 @@ impl StorageCmd {
 
 			for (key, info) in child_nodes {
 				if let Some(original_v) = client
-					.child_storage(&block, &info.clone(), &key)
+					.child_storage(&best_hash, &info.clone(), &key)
 					.expect("Checked above to exist")
 				{
 					let mut new_v = vec![0; original_v.0.len()];