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

Commit

Permalink
BlockId removal: BlockBuilderProvider::new_block_at (#13401)
Browse files Browse the repository at this point in the history
* `BlockId` removal: `BlockBuilderProvider::new_block_at`

It changes the arguments of `BlockBuilderProvider::new_block_at` from:
`BlockId<Block>` to: `Block::Hash`

* fmt

* fix

* more fixes
  • Loading branch information
michalkucharczyk authored Feb 21, 2023
1 parent 0e8daf0 commit 2d6e555
Show file tree
Hide file tree
Showing 15 changed files with 175 additions and 196 deletions.
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

0 comments on commit 2d6e555

Please sign in to comment.