BlockId removal: refactor of runtime API (#2190)

* BlockId removal: refactor of runtime API

It changes the first argument of all generated runtime API calls from: `BlockId<Block>` to: `Block::Hash`

* fmt

* cargo update -p polkadot-primitives

* Revert "cargo update -p polkadot-primitives"

This reverts commit 58506ecd31e1e5e42b78c2db3982acbc4d8edc70.

* update lockfile for {"substrate", "polkadot"}

---------

Co-authored-by: parity-processbot <>

client/collator/src/lib.rs

@@ -26,10 +26,7 @@ use sc_client_api::BlockBackend;
 use sp_api::{ApiExt, ProvideRuntimeApi};
 use sp_consensus::BlockStatus;
 use sp_core::traits::SpawnNamed;
-use sp_runtime::{
-	generic::BlockId,
-	traits::{Block as BlockT, HashFor, Header as HeaderT, Zero},
-};
+use sp_runtime::traits::{Block as BlockT, HashFor, Header as HeaderT, Zero};
 
 use cumulus_client_consensus_common::ParachainConsensus;
 use polkadot_node_primitives::{
@@ -154,10 +151,9 @@ where
 		header: &Block::Header,
 	) -> Result<Option<CollationInfo>, sp_api::ApiError> {
 		let runtime_api = self.runtime_api.runtime_api();
-		let block_id = BlockId::Hash(block_hash);
 
 		let api_version =
-			match runtime_api.api_version::<dyn CollectCollationInfo<Block>>(&block_id)? {
+			match runtime_api.api_version::<dyn CollectCollationInfo<Block>>(block_hash)? {
 				Some(version) => version,
 				None => {
 					tracing::error!(
@@ -171,10 +167,10 @@ where
 		let collation_info = if api_version < 2 {
 			#[allow(deprecated)]
 			runtime_api
-				.collect_collation_info_before_version_2(&block_id)?
+				.collect_collation_info_before_version_2(block_hash)?
 				.into_latest(header.encode().into())
 		} else {
-			runtime_api.collect_collation_info(&block_id, header)?
+			runtime_api.collect_collation_info(block_hash, header)?
 		};
 
 		Ok(Some(collation_info))
@@ -419,9 +415,8 @@ mod tests {
 			_: PHash,
 			validation_data: &PersistedValidationData,
 		) -> Option<ParachainCandidate<Block>> {
-			let block_id = BlockId::Hash(parent.hash());
 			let builder = self.client.init_block_builder_at(
-				&block_id,
+				parent.hash(),
 				Some(validation_data.clone()),
 				Default::default(),
 			);

client/consensus/common/src/tests.rs

@@ -25,7 +25,7 @@ use cumulus_relay_chain_interface::{
 	RelayChainInterface, RelayChainResult, SessionIndex, StorageValue, ValidatorId,
 };
 use cumulus_test_client::{
-	runtime::{Block, Header},
+	runtime::{Block, Hash, Header},
 	Backend, Client, InitBlockBuilder, TestClientBuilder, TestClientBuilderExt,
 };
 use futures::{channel::mpsc, executor::block_on, select, FutureExt, Stream, StreamExt};
@@ -33,7 +33,6 @@ use futures_timer::Delay;
 use sc_client_api::{blockchain::Backend as _, Backend as _, UsageProvider};
 use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy};
 use sp_consensus::{BlockOrigin, BlockStatus};
-use sp_runtime::generic::BlockId;
 use std::{
 	collections::{BTreeMap, HashMap},
 	pin::Pin,
@@ -212,14 +211,13 @@ impl RelayChainInterface for Relaychain {
 
 fn build_block<B: InitBlockBuilder>(
 	builder: &B,
-	at: Option<BlockId<Block>>,
+	at: Option<Hash>,
 	timestamp: Option<u64>,
 ) -> Block {
 	let builder = match at {
 		Some(at) => match timestamp {
-			Some(ts) =>
-				builder.init_block_builder_with_timestamp(&at, None, Default::default(), ts),
-			None => builder.init_block_builder_at(&at, None, Default::default()),
+			Some(ts) => builder.init_block_builder_with_timestamp(at, None, Default::default(), ts),
+			None => builder.init_block_builder_at(at, None, Default::default()),
 		},
 		None => builder.init_block_builder(None, Default::default()),
 	};
@@ -267,7 +265,7 @@ fn build_and_import_block_ext<B: InitBlockBuilder, I: BlockImport<Block>>(
 	origin: BlockOrigin,
 	import_as_best: bool,
 	importer: &mut I,
-	at: Option<BlockId<Block>>,
+	at: Option<Hash>,
 	timestamp: Option<u64>,
 ) -> Block {
 	let block = build_block(builder, at, timestamp);
@@ -425,8 +423,7 @@ fn follow_finalized_does_not_stop_on_unknown_block() {
 	let block = build_and_import_block(client.clone(), false);
 
 	let unknown_block = {
-		let block_builder =
-			client.init_block_builder_at(&BlockId::Hash(block.hash()), None, Default::default());
+		let block_builder = client.init_block_builder_at(block.hash(), None, Default::default());
 		block_builder.build().unwrap().block
 	};
 
@@ -475,8 +472,7 @@ fn follow_new_best_sets_best_after_it_is_imported() {
 	let block = build_and_import_block(client.clone(), false);
 
 	let unknown_block = {
-		let block_builder =
-			client.init_block_builder_at(&BlockId::Hash(block.hash()), None, Default::default());
+		let block_builder = client.init_block_builder_at(block.hash(), None, Default::default());
 		block_builder.build().unwrap().block
 	};
 
@@ -608,7 +604,7 @@ fn prune_blocks_on_level_overflow() {
 		None,
 		None,
 	);
-	let id0 = BlockId::Hash(block0.header.hash());
+	let id0 = block0.header.hash();
 
 	let blocks1 = (0..LEVEL_LIMIT)
 		.into_iter()
@@ -623,7 +619,7 @@ fn prune_blocks_on_level_overflow() {
 			)
 		})
 		.collect::<Vec<_>>();
-	let id10 = BlockId::Hash(blocks1[0].header.hash());
+	let id10 = blocks1[0].header.hash();
 
 	let blocks2 = (0..2)
 		.into_iter()
@@ -721,7 +717,7 @@ fn restore_limit_monitor() {
 		None,
 		None,
 	);
-	let id00 = BlockId::Hash(block00.header.hash());
+	let id00 = block00.header.hash();
 
 	let blocks1 = (0..LEVEL_LIMIT + 1)
 		.into_iter()
@@ -736,7 +732,7 @@ fn restore_limit_monitor() {
 			)
 		})
 		.collect::<Vec<_>>();
-	let id10 = BlockId::Hash(blocks1[0].header.hash());
+	let id10 = blocks1[0].header.hash();
 
 	let _ = (0..LEVEL_LIMIT)
 		.into_iter()

client/consensus/relay-chain/src/import_queue.rs

@@ -27,10 +27,7 @@ use sp_block_builder::BlockBuilder as BlockBuilderApi;
 use sp_blockchain::Result as ClientResult;
 use sp_consensus::{error::Error as ConsensusError, CacheKeyId};
 use sp_inherents::{CreateInherentDataProviders, InherentDataProvider};
-use sp_runtime::{
-	generic::BlockId,
-	traits::{Block as BlockT, Header as HeaderT},
-};
+use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 
 /// A verifier that just checks the inherents.
 pub struct Verifier<Client, Block, CIDP> {
@@ -83,11 +80,7 @@ where
 			let inherent_res = self
 				.client
 				.runtime_api()
-				.check_inherents(
-					&BlockId::Hash(*block.header().parent_hash()),
-					block.clone(),
-					inherent_data,
-				)
+				.check_inherents(*block.header().parent_hash(), block.clone(), inherent_data)
 				.map_err(|e| format!("{:?}", e))?;
 
 			if !inherent_res.ok() {

client/relay-chain-inprocess-interface/src/lib.rs

@@ -19,9 +19,8 @@ use std::{pin::Pin, sync::Arc, time::Duration};
 use async_trait::async_trait;
 use cumulus_primitives_core::{
 	relay_chain::{
-		runtime_api::ParachainHost, Block as PBlock, BlockId, CommittedCandidateReceipt,
-		Hash as PHash, Header as PHeader, InboundHrmpMessage, OccupiedCoreAssumption, SessionIndex,
-		ValidatorId,
+		runtime_api::ParachainHost, Block as PBlock, CommittedCandidateReceipt, Hash as PHash,
+		Header as PHeader, InboundHrmpMessage, OccupiedCoreAssumption, SessionIndex, ValidatorId,
 	},
 	InboundDownwardMessage, ParaId, PersistedValidationData,
 };
@@ -93,7 +92,7 @@ where
 		relay_parent: PHash,
 	) -> RelayChainResult<Vec<InboundDownwardMessage>> {
 		Ok(self.full_client.runtime_api().dmq_contents_with_context(
-			&BlockId::hash(relay_parent),
+			relay_parent,
 			sp_core::ExecutionContext::Importing,
 			para_id,
 		)?)
@@ -105,7 +104,7 @@ where
 		relay_parent: PHash,
 	) -> RelayChainResult<BTreeMap<ParaId, Vec<InboundHrmpMessage>>> {
 		Ok(self.full_client.runtime_api().inbound_hrmp_channels_contents_with_context(
-			&BlockId::hash(relay_parent),
+			relay_parent,
 			sp_core::ExecutionContext::Importing,
 			para_id,
 		)?)
@@ -118,7 +117,7 @@ where
 		occupied_core_assumption: OccupiedCoreAssumption,
 	) -> RelayChainResult<Option<PersistedValidationData>> {
 		Ok(self.full_client.runtime_api().persisted_validation_data(
-			&BlockId::Hash(hash),
+			hash,
 			para_id,
 			occupied_core_assumption,
 		)?)
@@ -129,18 +128,15 @@ where
 		hash: PHash,
 		para_id: ParaId,
 	) -> RelayChainResult<Option<CommittedCandidateReceipt>> {
-		Ok(self
-			.full_client
-			.runtime_api()
-			.candidate_pending_availability(&BlockId::Hash(hash), para_id)?)
+		Ok(self.full_client.runtime_api().candidate_pending_availability(hash, para_id)?)
 	}
 
 	async fn session_index_for_child(&self, hash: PHash) -> RelayChainResult<SessionIndex> {
-		Ok(self.full_client.runtime_api().session_index_for_child(&BlockId::Hash(hash))?)
+		Ok(self.full_client.runtime_api().session_index_for_child(hash)?)
 	}
 
 	async fn validators(&self, hash: PHash) -> RelayChainResult<Vec<ValidatorId>> {
-		Ok(self.full_client.runtime_api().validators(&BlockId::Hash(hash))?)
+		Ok(self.full_client.runtime_api().validators(hash)?)
 	}
 
 	async fn import_notification_stream(

polkadot-parachain/src/service.rs

@@ -53,7 +53,6 @@ use sp_consensus_aura::AuraApi;
 use sp_keystore::SyncCryptoStorePtr;
 use sp_runtime::{
 	app_crypto::AppKey,
-	generic::BlockId,
 	traits::{BlakeTwo256, Header as HeaderT},
 };
 use std::{marker::PhantomData, sync::Arc, time::Duration};
@@ -994,11 +993,10 @@ where
 		relay_parent: PHash,
 		validation_data: &PersistedValidationData,
 	) -> Option<ParachainCandidate<Block>> {
-		let block_id = BlockId::hash(parent.hash());
 		if self
 			.client
 			.runtime_api()
-			.has_api::<dyn AuraApi<Block, AuraId>>(&block_id)
+			.has_api::<dyn AuraApi<Block, AuraId>>(parent.hash())
 			.unwrap_or(false)
 		{
 			self.aura_consensus
@@ -1035,12 +1033,10 @@ where
 		&mut self,
 		block_import: BlockImportParams<Block, ()>,
 	) -> Result<(BlockImportParams<Block, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> {
-		let block_id = BlockId::hash(*block_import.header.parent_hash());
-
 		if self
 			.client
 			.runtime_api()
-			.has_api::<dyn AuraApi<Block, AuraId>>(&block_id)
+			.has_api::<dyn AuraApi<Block, AuraId>>(*block_import.header.parent_hash())
 			.unwrap_or(false)
 		{
 			self.aura_verifier.get_mut().verify(block_import).await

primitives/timestamp/src/lib.rs

@@ -82,10 +82,7 @@ mod tests {
 		ValidationParams,
 	};
 	use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder;
-	use sp_runtime::{
-		generic::BlockId,
-		traits::{Block as BlockT, Header as HeaderT},
-	};
+	use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 	use std::{env, process::Command, str::FromStr};
 
 	const SLOT_DURATION: u64 = 6000;
@@ -128,7 +125,7 @@ mod tests {
 
 		let block = client
 			.init_block_builder_with_timestamp(
-				&BlockId::Hash(hash),
+				hash,
 				Some(validation_data),
 				sproof_builder,
 				timestamp,

test/client/src/block_builder.rs

@@ -48,7 +48,7 @@ pub trait InitBlockBuilder {
 	/// [`BlockId`] to say which should be the parent block of the block that is being build.
 	fn init_block_builder_at(
 		&self,
-		at: &BlockId<Block>,
+		at: Hash,
 		validation_data: Option<PersistedValidationData<PHash, PBlockNumber>>,
 		relay_sproof_builder: RelayStateSproofBuilder,
 	) -> sc_block_builder::BlockBuilder<Block, Client, Backend>;
@@ -60,7 +60,7 @@ pub trait InitBlockBuilder {
 	/// it will use the given `timestamp` as input for the timestamp inherent.
 	fn init_block_builder_with_timestamp(
 		&self,
-		at: &BlockId<Block>,
+		at: Hash,
 		validation_data: Option<PersistedValidationData<PHash, PBlockNumber>>,
 		relay_sproof_builder: RelayStateSproofBuilder,
 		timestamp: u64,
@@ -69,13 +69,13 @@ pub trait InitBlockBuilder {
 
 fn init_block_builder<'a>(
 	client: &'a Client,
-	at: &BlockId<Block>,
+	at: Hash,
 	validation_data: Option<PersistedValidationData<PHash, PBlockNumber>>,
 	relay_sproof_builder: RelayStateSproofBuilder,
 	timestamp: u64,
 ) -> BlockBuilder<'a, Block, Client, Backend> {
 	let mut block_builder = client
-		.new_block_at(at, Default::default(), true)
+		.new_block_at(&BlockId::Hash(at), Default::default(), true)
 		.expect("Creates new block builder for test runtime");
 
 	let mut inherent_data = sp_inherents::InherentData::new();
@@ -123,16 +123,12 @@ impl InitBlockBuilder for Client {
 		relay_sproof_builder: RelayStateSproofBuilder,
 	) -> BlockBuilder<Block, Client, Backend> {
 		let chain_info = self.chain_info();
-		self.init_block_builder_at(
-			&BlockId::Hash(chain_info.best_hash),
-			validation_data,
-			relay_sproof_builder,
-		)
+		self.init_block_builder_at(chain_info.best_hash, validation_data, relay_sproof_builder)
 	}
 
 	fn init_block_builder_at(
 		&self,
-		at: &BlockId<Block>,
+		at: Hash,
 		validation_data: Option<PersistedValidationData<PHash, PBlockNumber>>,
 		relay_sproof_builder: RelayStateSproofBuilder,
 	) -> BlockBuilder<Block, Client, Backend> {
@@ -145,7 +141,7 @@ impl InitBlockBuilder for Client {
 
 	fn init_block_builder_with_timestamp(
 		&self,
-		at: &BlockId<Block>,
+		at: Hash,
 		validation_data: Option<PersistedValidationData<PHash, PBlockNumber>>,
 		relay_sproof_builder: RelayStateSproofBuilder,
 		timestamp: u64,

test/service/src/lib.rs

@@ -806,7 +806,7 @@ pub fn fetch_nonce(client: &Client, account: sp_core::sr25519::Public) -> u32 {
 	let best_hash = client.chain_info().best_hash;
 	client
 		.runtime_api()
-		.account_nonce(&generic::BlockId::Hash(best_hash), account.into())
+		.account_nonce(best_hash, account.into())
 		.expect("Fetching account nonce works; qed")
 }