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

BlockId removal: BlockBuilderProvider::new_block_at #13401

Merged
merged 6 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions bin/node/cli/benches/block_production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed}
use sp_consensus::BlockOrigin;
use sp_keyring::Sr25519Keyring;
use sp_runtime::{
generic::BlockId,
transaction_validity::{InvalidTransaction, TransactionValidityError},
AccountId32, MultiAddress, OpaqueExtrinsic,
};
Expand Down Expand Up @@ -206,14 +205,14 @@ fn block_production(c: &mut Criterion) {
group.sample_size(10);
group.throughput(Throughput::Elements(max_transfer_count as u64));

let block_id = BlockId::Hash(client.chain_info().best_hash);
let best_hash = client.chain_info().best_hash;

group.bench_function(format!("{} transfers (no proof)", max_transfer_count), |b| {
b.iter_batched(
|| extrinsics.clone(),
|extrinsics| {
let mut block_builder =
client.new_block_at(&block_id, Default::default(), RecordProof::No).unwrap();
client.new_block_at(best_hash, Default::default(), RecordProof::No).unwrap();
for extrinsic in extrinsics {
block_builder.push(extrinsic).unwrap();
}
Expand All @@ -228,7 +227,7 @@ fn block_production(c: &mut Criterion) {
|| extrinsics.clone(),
|extrinsics| {
let mut block_builder =
client.new_block_at(&block_id, Default::default(), RecordProof::Yes).unwrap();
client.new_block_at(best_hash, Default::default(), RecordProof::Yes).unwrap();
for extrinsic in extrinsics {
block_builder.push(extrinsic).unwrap();
}
Expand Down
11 changes: 4 additions & 7 deletions client/basic-authorship/src/basic_authorship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use sp_consensus::{DisableProofRecording, EnableProofRecording, ProofRecording,
use sp_core::traits::SpawnNamed;
use sp_inherents::InherentData;
use sp_runtime::{
generic::BlockId,
traits::{BlakeTwo256, Block as BlockT, Hash as HashT, Header as HeaderT},
Digest, Percent, SaturatedConversion,
};
Expand Down Expand Up @@ -196,14 +195,12 @@ where
) -> Proposer<B, Block, C, A, PR> {
let parent_hash = parent_header.hash();

let id = BlockId::hash(parent_hash);

info!("🙌 Starting consensus session on top of parent {:?}", parent_hash);

let proposer = Proposer::<_, _, _, _, PR> {
spawn_handle: self.spawn_handle.clone(),
client: self.client.clone(),
parent_id: id,
parent_hash,
parent_number: *parent_header.number(),
transaction_pool: self.transaction_pool.clone(),
now,
Expand Down Expand Up @@ -247,7 +244,7 @@ where
pub struct Proposer<B, Block: BlockT, C, A: TransactionPool, PR> {
spawn_handle: Box<dyn SpawnNamed>,
client: Arc<C>,
parent_id: BlockId<Block>,
parent_hash: Block::Hash,
parent_number: <<Block as BlockT>::Header as HeaderT>::Number,
transaction_pool: Arc<A>,
now: Box<dyn Fn() -> time::Instant + Send + Sync>,
Expand Down Expand Up @@ -344,7 +341,7 @@ where
{
let propose_with_start = time::Instant::now();
let mut block_builder =
self.client.new_block_at(&self.parent_id, inherent_digests, PR::ENABLED)?;
self.client.new_block_at(self.parent_hash, inherent_digests, PR::ENABLED)?;

let create_inherents_start = time::Instant::now();
let inherents = block_builder.create_inherents(inherent_data)?;
Expand Down Expand Up @@ -559,7 +556,7 @@ mod tests {
use sp_blockchain::HeaderBackend;
use sp_consensus::{BlockOrigin, Environment, Proposer};
use sp_core::Pair;
use sp_runtime::traits::NumberFor;
use sp_runtime::{generic::BlockId, traits::NumberFor};
use substrate_test_runtime_client::{
prelude::*,
runtime::{Extrinsic, Transfer},
Expand Down
15 changes: 6 additions & 9 deletions client/beefy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ use sp_keystore::{testing::KeyStore as TestKeystore, SyncCryptoStore, SyncCrypto
use sp_mmr_primitives::{Error as MmrError, MmrApi};
use sp_runtime::{
codec::Encode,
generic::BlockId,
traits::{Header as HeaderT, NumberFor},
BuildStorage, DigestItem, EncodedJustification, Justifications, Storage,
};
Expand Down Expand Up @@ -753,8 +752,9 @@ async fn beefy_importing_justifications() {
.and_then(|j| j.get(BEEFY_ENGINE_ID).cloned())
};

let parent_id = BlockId::Number(0);
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let builder = full_client
.new_block_at(full_client.chain_info().genesis_hash, Default::default(), false)
.unwrap();
let block = builder.build().unwrap().block;
let hashof1 = block.header.hash();

Expand All @@ -772,9 +772,8 @@ async fn beefy_importing_justifications() {
);

// Import block 2 with "valid" justification (beefy pallet genesis block not yet reached).
let parent_id = BlockId::Number(1);
let block_num = 2;
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let builder = full_client.new_block_at(hashof1, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof2 = block.header.hash();

Expand Down Expand Up @@ -805,9 +804,8 @@ async fn beefy_importing_justifications() {
}

// Import block 3 with valid justification.
let parent_id = BlockId::Number(2);
let block_num = 3;
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let builder = full_client.new_block_at(hashof2, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof3 = block.header.hash();
let proof = crate::justification::tests::new_finality_proof(block_num, &good_set, keys);
Expand Down Expand Up @@ -840,9 +838,8 @@ async fn beefy_importing_justifications() {
}

// Import block 4 with invalid justification (incorrect validator set).
let parent_id = BlockId::Number(3);
let block_num = 4;
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let builder = full_client.new_block_at(hashof3, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof4 = block.header.hash();
let keys = &[BeefyKeyring::Alice];
Expand Down
3 changes: 1 addition & 2 deletions client/block-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use sp_api::{
use sp_blockchain::{ApplyExtrinsicFailed, Error};
use sp_core::ExecutionContext;
use sp_runtime::{
generic::BlockId,
legacy,
traits::{Block as BlockT, Hash, HashFor, Header as HeaderT, NumberFor, One},
Digest,
Expand Down Expand Up @@ -120,7 +119,7 @@ where
/// output of this block builder without having access to the full storage.
fn new_block_at<R: Into<RecordProof>>(
&self,
parent: &BlockId<Block>,
parent: Block::Hash,
inherent_digests: Digest,
record_proof: R,
) -> sp_blockchain::Result<BlockBuilder<Block, RA, B>>;
Expand Down
9 changes: 3 additions & 6 deletions client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use sp_keystore::{
SyncCryptoStore,
};
use sp_runtime::{
generic::{BlockId, Digest, DigestItem},
generic::{Digest, DigestItem},
traits::Block as BlockT,
};
use sp_timestamp::Timestamp;
Expand Down Expand Up @@ -123,11 +123,8 @@ impl DummyProposer {
Error,
>,
> {
let block_builder = self
.factory
.client
.new_block_at(&BlockId::Hash(self.parent_hash), pre_digests, false)
.unwrap();
let block_builder =
self.factory.client.new_block_at(self.parent_hash, pre_digests, false).unwrap();

let mut block = match block_builder.build().map_err(|e| e.into()) {
Ok(b) => b.block,
Expand Down
6 changes: 3 additions & 3 deletions client/finality-grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ async fn allows_reimporting_change_blocks() {

let full_client = client.as_client();
let builder = full_client
.new_block_at(&BlockId::Number(0), Default::default(), false)
.new_block_at(full_client.chain_info().genesis_hash, Default::default(), false)
.unwrap();
let mut block = builder.build().unwrap().block;
add_scheduled_change(
Expand Down Expand Up @@ -922,7 +922,7 @@ async fn test_bad_justification() {

let full_client = client.as_client();
let builder = full_client
.new_block_at(&BlockId::Number(0), Default::default(), false)
.new_block_at(full_client.chain_info().genesis_hash, Default::default(), false)
.unwrap();
let mut block = builder.build().unwrap().block;

Expand Down Expand Up @@ -1854,7 +1854,7 @@ async fn imports_justification_for_regular_blocks_on_import() {

let full_client = client.as_client();
let builder = full_client
.new_block_at(&BlockId::Number(0), Default::default(), false)
.new_block_at(full_client.chain_info().genesis_hash, Default::default(), false)
.unwrap();
let block = builder.build().unwrap().block;

Expand Down
3 changes: 2 additions & 1 deletion client/merkle-mountain-range/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ impl MockClient {
) -> MmrBlock {
let mut client = self.client.lock();

let mut block_builder = client.new_block_at(at, Default::default(), false).unwrap();
let hash = client.expect_block_hash_from_id(&at).unwrap();
let mut block_builder = client.new_block_at(hash, Default::default(), false).unwrap();
// Make sure the block has a different hash than its siblings
block_builder
.push_storage_change(b"name".to_vec(), Some(name.to_vec()))
Expand Down
7 changes: 2 additions & 5 deletions client/network/src/service/tests/chain_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ use sc_network_common::{
};
use sc_network_sync::{mock::MockChainSync, service::mock::MockChainSyncInterface, ChainSync};
use sp_core::H256;
use sp_runtime::{
generic::BlockId,
traits::{Block as BlockT, Header as _},
};
use sp_runtime::traits::{Block as BlockT, Header as _};
use std::{
sync::{Arc, RwLock},
task::Poll,
Expand Down Expand Up @@ -188,7 +185,7 @@ async fn on_block_finalized() {

let at = client.header(client.info().best_hash).unwrap().unwrap().hash();
let block = client
.new_block_at(&BlockId::Hash(at), Default::default(), false)
.new_block_at(at, Default::default(), false)
.unwrap()
.build()
.unwrap()
Expand Down
7 changes: 2 additions & 5 deletions client/network/sync/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3212,7 +3212,6 @@ mod test {
};
use sp_blockchain::HeaderBackend;
use sp_consensus::block_validation::DefaultBlockAnnounceValidator;
use sp_runtime::generic::BlockId;
use substrate_test_runtime_client::{
runtime::{Block, Hash, Header},
BlockBuilderExt, ClientBlockImportExt, ClientExt, DefaultTestClientBuilderExt, TestClient,
Expand Down Expand Up @@ -3449,8 +3448,7 @@ mod test {
fn build_block(client: &mut Arc<TestClient>, at: Option<Hash>, fork: bool) -> Block {
let at = at.unwrap_or_else(|| client.info().best_hash);

let mut block_builder =
client.new_block_at(&BlockId::Hash(at), Default::default(), false).unwrap();
let mut block_builder = client.new_block_at(at, Default::default(), false).unwrap();

if fork {
block_builder.push_storage_change(vec![1, 2, 3], Some(vec![4, 5, 6])).unwrap();
Expand Down Expand Up @@ -3503,8 +3501,7 @@ mod test {

let mut client2 = client.clone();
let mut build_block_at = |at, import| {
let mut block_builder =
client2.new_block_at(&BlockId::Hash(at), Default::default(), false).unwrap();
let mut block_builder = client2.new_block_at(at, Default::default(), false).unwrap();
// Make sure we generate a different block as fork
block_builder.push_storage_change(vec![1, 2, 3], Some(vec![4, 5, 6])).unwrap();

Expand Down
3 changes: 1 addition & 2 deletions client/network/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,7 @@ where
let full_client = self.client.as_client();
let mut at = full_client.block_hash_from_id(&at).unwrap().unwrap();
for _ in 0..count {
let builder =
full_client.new_block_at(&BlockId::Hash(at), Default::default(), false).unwrap();
let builder = full_client.new_block_at(at, Default::default(), false).unwrap();
let block = edit_block(builder);
let hash = block.header.hash();
trace!(
Expand Down
10 changes: 3 additions & 7 deletions client/rpc-spec-v2/src/chain_head/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use jsonrpsee::{
};
use sc_block_builder::BlockBuilderProvider;
use sc_client_api::ChildInfo;
use sp_api::BlockId;
use sp_blockchain::HeaderBackend;
use sp_consensus::BlockOrigin;
use sp_core::{
Expand Down Expand Up @@ -639,9 +638,8 @@ async fn follow_generates_initial_blocks() {
let block_2_hash = block_2.header.hash();
client.import(BlockOrigin::Own, block_2.clone()).await.unwrap();

let mut block_builder = client
.new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false)
.unwrap();
let mut block_builder =
client.new_block_at(block_1.header.hash(), Default::default(), false).unwrap();
// This push is required as otherwise block 3 has the same hash as block 2 and won't get
// imported
block_builder
Expand Down Expand Up @@ -921,9 +919,7 @@ async fn follow_prune_best_block() {
client.import(BlockOrigin::Own, block_4.clone()).await.unwrap();

// Import block 2 as best on the fork.
let mut block_builder = client
.new_block_at(&BlockId::Hash(block_1.header.hash()), Default::default(), false)
.unwrap();
let mut block_builder = client.new_block_at(block_1_hash, Default::default(), false).unwrap();
// This push is required as otherwise block 3 has the same hash as block 2 and won't get
// imported
block_builder
Expand Down
6 changes: 3 additions & 3 deletions client/service/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1408,14 +1408,14 @@ where
{
fn new_block_at<R: Into<RecordProof>>(
&self,
parent: &BlockId<Block>,
parent: Block::Hash,
inherent_digests: Digest,
record_proof: R,
) -> sp_blockchain::Result<sc_block_builder::BlockBuilder<Block, Self, B>> {
sc_block_builder::BlockBuilder::new(
self,
self.expect_block_hash_from_id(parent)?,
self.expect_block_number_from_id(parent)?,
parent,
self.expect_block_number_from_id(&BlockId::Hash(parent))?,
record_proof.into(),
inherent_digests,
&self.backend,
Expand Down
Loading