BlockId removal: refactor: Backend::state_at (#12488)

* Minor naming improved

* BlockId removal refactor: Backend::state_at

* formatting

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

@@ -147,17 +147,14 @@ where
 		extensions: Option<Extensions>,
 	) -> sp_blockchain::Result<Vec<u8>> {
 		let mut changes = OverlayedChanges::default();
-		let state = self.backend.state_at(*at)?;
+		let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?;
+		let state = self.backend.state_at(&at_hash)?;
 		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
 		let runtime_code =
 			state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?;
 
 		let runtime_code = self.check_override(runtime_code, at)?;
 
-		let at_hash = self.backend.blockchain().block_hash_from_id(at)?.ok_or_else(|| {
-			sp_blockchain::Error::UnknownBlock(format!("Could not find block hash for {:?}", at))
-		})?;
-
 		let mut sm = StateMachine::new(
 			&state,
 			&mut changes,
@@ -195,14 +192,11 @@ where
 	{
 		let mut storage_transaction_cache = storage_transaction_cache.map(|c| c.borrow_mut());
 
-		let state = self.backend.state_at(*at)?;
+		let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?;
+		let state = self.backend.state_at(&at_hash)?;
 
 		let changes = &mut *changes.borrow_mut();
 
-		let at_hash = self.backend.blockchain().block_hash_from_id(at)?.ok_or_else(|| {
-			sp_blockchain::Error::UnknownBlock(format!("Could not find block hash for {:?}", at))
-		})?;
-
 		// It is important to extract the runtime code here before we create the proof
 		// recorder to not record it. We also need to fetch the runtime code from `state` to
 		// make sure we use the caching layers.
@@ -255,7 +249,9 @@ where
 
 	fn runtime_version(&self, id: &BlockId<Block>) -> sp_blockchain::Result<RuntimeVersion> {
 		let mut overlay = OverlayedChanges::default();
-		let state = self.backend.state_at(*id)?;
+
+		let at_hash = self.backend.blockchain().expect_block_hash_from_id(id)?;
+		let state = self.backend.state_at(&at_hash)?;
 		let mut cache = StorageTransactionCache::<Block, B::State>::default();
 		let mut ext = Ext::new(&mut overlay, &mut cache, &state, None);
 		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
@@ -272,7 +268,8 @@ where
 		method: &str,
 		call_data: &[u8],
 	) -> sp_blockchain::Result<(Vec<u8>, StorageProof)> {
-		let state = self.backend.state_at(*at)?;
+		let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?;
+		let state = self.backend.state_at(&at_hash)?;
 
 		let trie_backend = state.as_trie_backend();
 

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

@@ -414,8 +414,8 @@ where
 	}
 
 	/// Get a reference to the state at a given block.
-	pub fn state_at(&self, block: &BlockId<Block>) -> sp_blockchain::Result<B::State> {
-		self.backend.state_at(*block)
+	pub fn state_at(&self, hash: &Block::Hash) -> sp_blockchain::Result<B::State> {
+		self.backend.state_at(hash)
 	}
 
 	/// Get the code at a given block.
@@ -813,7 +813,7 @@ where
 					Block::new(import_block.header.clone(), body.clone()),
 				)?;
 
-				let state = self.backend.state_at(at)?;
+				let state = self.backend.state_at(parent_hash)?;
 				let gen_storage_changes = runtime_api
 					.into_storage_changes(&state, *parent_hash)
 					.map_err(sp_blockchain::Error::Storage)?;
@@ -1154,7 +1154,9 @@ where
 		id: &BlockId<Block>,
 		keys: &mut dyn Iterator<Item = &[u8]>,
 	) -> sp_blockchain::Result<StorageProof> {
-		self.state_at(id).and_then(|state| prove_read(state, keys).map_err(Into::into))
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		self.state_at(&hash)
+			.and_then(|state| prove_read(state, keys).map_err(Into::into))
 	}
 
 	fn read_child_proof(
@@ -1163,7 +1165,8 @@ where
 		child_info: &ChildInfo,
 		keys: &mut dyn Iterator<Item = &[u8]>,
 	) -> sp_blockchain::Result<StorageProof> {
-		self.state_at(id)
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		self.state_at(&hash)
 			.and_then(|state| prove_child_read(state, child_info, keys).map_err(Into::into))
 	}
 
@@ -1182,7 +1185,8 @@ where
 		start_key: &[Vec<u8>],
 		size_limit: usize,
 	) -> sp_blockchain::Result<(CompactProof, u32)> {
-		let state = self.state_at(id)?;
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		let state = self.state_at(&hash)?;
 		// this is a read proof, using version V0 or V1 is equivalent.
 		let root = state.storage_root(std::iter::empty(), StateVersion::V0).0;
 
@@ -1204,7 +1208,8 @@ where
 		if start_key.len() > MAX_NESTED_TRIE_DEPTH {
 			return Err(Error::Backend("Invalid start key.".to_string()))
 		}
-		let state = self.state_at(id)?;
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		let state = self.state_at(&hash)?;
 		let child_info = |storage_key: &Vec<u8>| -> sp_blockchain::Result<ChildInfo> {
 			let storage_key = PrefixedStorageKey::new_ref(storage_key);
 			match ChildType::from_prefixed_key(storage_key) {
@@ -1400,7 +1405,8 @@ where
 		id: &BlockId<Block>,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<StorageKey>> {
-		let keys = self.state_at(id)?.keys(&key_prefix.0).into_iter().map(StorageKey).collect();
+		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();
 		Ok(keys)
 	}
 
@@ -1409,7 +1415,8 @@ where
 		id: &BlockId<Block>,
 		key_prefix: &StorageKey,
 	) -> sp_blockchain::Result<Vec<(StorageKey, StorageData)>> {
-		let state = self.state_at(id)?;
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		let state = self.state_at(&hash)?;
 		let keys = state
 			.keys(&key_prefix.0)
 			.into_iter()
@@ -1427,7 +1434,8 @@ where
 		prefix: Option<&'a StorageKey>,
 		start_key: Option<&StorageKey>,
 	) -> sp_blockchain::Result<KeyIterator<'a, B::State, Block>> {
-		let state = self.state_at(id)?;
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		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))
 	}
@@ -1439,7 +1447,8 @@ where
 		prefix: Option<&'a StorageKey>,
 		start_key: Option<&StorageKey>,
 	) -> sp_blockchain::Result<KeyIterator<'a, B::State, Block>> {
-		let state = self.state_at(id)?;
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		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))
 	}
@@ -1449,8 +1458,9 @@ where
 		id: &BlockId<Block>,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<StorageData>> {
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
 		Ok(self
-			.state_at(id)?
+			.state_at(&hash)?
 			.storage(&key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?
 			.map(StorageData))
@@ -1461,7 +1471,8 @@ where
 		id: &BlockId<Block>,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
-		self.state_at(id)?
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		self.state_at(&hash)?
 			.storage_hash(&key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))
 	}
@@ -1472,8 +1483,9 @@ where
 		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(id)?
+			.state_at(&hash)?
 			.child_keys(child_info, &key_prefix.0)
 			.into_iter()
 			.map(StorageKey)
@@ -1487,8 +1499,9 @@ where
 		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(id)?
+			.state_at(&hash)?
 			.child_storage(child_info, &key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?
 			.map(StorageData))
@@ -1500,7 +1513,8 @@ where
 		child_info: &ChildInfo,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
-		self.state_at(id)?
+		let hash = self.backend.blockchain().expect_block_hash_from_id(&id)?;
+		self.state_at(&hash)?
 			.child_storage_hash(child_info, &key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))
 	}
@@ -1681,7 +1695,8 @@ where
 	}
 
 	fn state_at(&self, at: &BlockId<Block>) -> Result<Self::StateBackend, sp_api::ApiError> {
-		self.state_at(at).map_err(Into::into)
+		let hash = self.backend.blockchain().expect_block_hash_from_id(at)?;
+		self.state_at(&hash).map_err(Into::into)
 	}
 }
 

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

@@ -20,7 +20,7 @@ use futures::executor::block_on;
 use parity_scale_codec::{Decode, Encode, Joiner};
 use sc_block_builder::BlockBuilderProvider;
 use sc_client_api::{
-	in_mem, BlockBackend, BlockchainEvents, FinalityNotifications, StorageProvider,
+	in_mem, BlockBackend, BlockchainEvents, FinalityNotifications, HeaderBackend, StorageProvider,
 };
 use sc_client_db::{Backend, BlocksPruning, DatabaseSettings, DatabaseSource, PruningMode};
 use sc_consensus::{
@@ -338,11 +338,15 @@ fn block_builder_works_with_transactions() {
 	let block = builder.build().unwrap().block;
 	block_on(client.import(BlockOrigin::Own, block)).unwrap();
 
+	let hash0 = client
+		.expect_block_hash_from_id(&BlockId::Number(0))
+		.expect("block 0 was just imported. qed");
+	let hash1 = client
+		.expect_block_hash_from_id(&BlockId::Number(1))
+		.expect("block 1 was just imported. qed");
+
 	assert_eq!(client.chain_info().best_number, 1);
-	assert_ne!(
-		client.state_at(&BlockId::Number(1)).unwrap().pairs(),
-		client.state_at(&BlockId::Number(0)).unwrap().pairs()
-	);
+	assert_ne!(client.state_at(&hash1).unwrap().pairs(), client.state_at(&hash0).unwrap().pairs());
 	assert_eq!(
 		client
 			.runtime_api()
@@ -392,11 +396,15 @@ fn block_builder_does_not_include_invalid() {
 	let block = builder.build().unwrap().block;
 	block_on(client.import(BlockOrigin::Own, block)).unwrap();
 
+	let hash0 = client
+		.expect_block_hash_from_id(&BlockId::Number(0))
+		.expect("block 0 was just imported. qed");
+	let hash1 = client
+		.expect_block_hash_from_id(&BlockId::Number(1))
+		.expect("block 1 was just imported. qed");
+
 	assert_eq!(client.chain_info().best_number, 1);
-	assert_ne!(
-		client.state_at(&BlockId::Number(1)).unwrap().pairs(),
-		client.state_at(&BlockId::Number(0)).unwrap().pairs()
-	);
+	assert_ne!(client.state_at(&hash1).unwrap().pairs(), client.state_at(&hash0).unwrap().pairs());
 	assert_eq!(client.body(&BlockId::Number(1)).unwrap().unwrap().len(), 1)
 }