BlockId removal: BlockBuilderProvider::new_block_at (paritytech#1…

…3401)

* `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

bin/node/cli/benches/block_production.rs

@@ -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,
 };
@@ -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();
 				}
@@ -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();
 				}

client/basic-authorship/src/basic_authorship.rs

@@ -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,
 };
@@ -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,
@@ -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>,
@@ -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)?;
@@ -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},

client/beefy/src/tests.rs

@@ -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,
 };
@@ -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();
 
@@ -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();
 
@@ -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);
@@ -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];

client/block-builder/src/lib.rs

@@ -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,
@@ -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>>;

client/consensus/babe/src/tests.rs

@@ -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;
@@ -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,

client/finality-grandpa/src/tests.rs

@@ -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(
@@ -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;
 
@@ -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;
 

client/merkle-mountain-range/src/test_utils.rs

@@ -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()))

client/network/src/service/tests/chain_sync.rs

@@ -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,
@@ -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()

client/network/sync/src/lib.rs

@@ -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,
@@ -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();
@@ -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();
 

client/network/test/src/lib.rs

@@ -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!(

client/rpc-spec-v2/src/chain_head/tests.rs

@@ -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::{
@@ -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
@@ -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

client/service/src/client/client.rs

@@ -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,