Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

BlockId removal: refactor of runtime API #2190

Merged
merged 8 commits into from
Feb 21, 2023
15 changes: 5 additions & 10 deletions client/collator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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!(
Expand All @@ -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))
Expand Down Expand Up @@ -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(),
);
Expand Down
26 changes: 11 additions & 15 deletions client/consensus/common/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ 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};
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,
Expand Down Expand Up @@ -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()),
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
};

Expand Down Expand Up @@ -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
};

Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
11 changes: 2 additions & 9 deletions client/consensus/relay-chain/src/import_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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() {
Expand Down
20 changes: 8 additions & 12 deletions client/relay-chain-inprocess-interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
)?)
Expand All @@ -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,
)?)
Expand All @@ -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,
)?)
Expand All @@ -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(
Expand Down
8 changes: 2 additions & 6 deletions polkadot-parachain/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions primitives/timestamp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -128,7 +125,7 @@ mod tests {

let block = client
.init_block_builder_with_timestamp(
&BlockId::Hash(hash),
hash,
Some(validation_data),
sproof_builder,
timestamp,
Expand Down
18 changes: 7 additions & 11 deletions test/client/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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> {
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion test/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down