sc-block-builder: Remove BlockBuilderProvider (paritytech#2099)

The `BlockBuilderProvider` was a trait that was defined in
`sc-block-builder`. The trait was implemented for `Client`. This
basically meant that you needed to import `sc-block-builder` any way to
have access to the block builder. So, this trait was not providing any
real value. This pull request is removing the said trait. Instead of the
trait it introduces a builder for creating a `BlockBuilder`. The builder
currently has the quite fabulous name `BlockBuilderBuilder` (I'm open to
any better name 😅). The rest of the pull request is about
replacing the old trait with the new builder.

# Downstream code changes

If you used `new_block` or `new_block_at` before you now need to switch
it over to the new `BlockBuilderBuilder` pattern:

```rust
// `new` requires a type that implements `CallApiAt`. 
let mut block_builder = BlockBuilderBuilder::new(client)
                // Then you need to specify the hash of the parent block the block will be build on top of
		.on_parent_block(at)
                // The block builder also needs the block number of the parent block. 
                // Here it is fetched from the given `client` using the `HeaderBackend`
                // However, there also exists `with_parent_block_number` for directly passing the number
		.fetch_parent_block_number(client)
		.unwrap()
                // Enable proof recording if required. This call is optional.
		.enable_proof_recording()
                // Pass the digests. This call is optional.
                .with_inherent_digests(digests)
		.build()
		.expect("Creates new block builder");
```

---------

Co-authored-by: Sebastian Kunert <[email protected]>
Co-authored-by: command-bot <>

substrate/bin/node/cli/benches/block_production.rs

@@ -20,7 +20,7 @@ use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughpu
 
 use kitchensink_runtime::{constants::currency::*, BalancesCall};
 use node_cli::service::{create_extrinsic, FullClient};
-use sc_block_builder::{BlockBuilderProvider, BuiltBlock, RecordProof};
+use sc_block_builder::{BlockBuilderBuilder, BuiltBlock};
 use sc_consensus::{
 	block_import::{BlockImportParams, ForkChoiceStrategy},
 	BlockImport, StateAction,
@@ -127,7 +127,11 @@ fn prepare_benchmark(client: &FullClient) -> (usize, Vec<OpaqueExtrinsic>) {
 
 	let mut max_transfer_count = 0;
 	let mut extrinsics = Vec::new();
-	let mut block_builder = client.new_block(Default::default()).unwrap();
+	let mut block_builder = BlockBuilderBuilder::new(client)
+		.on_parent_block(client.chain_info().best_hash)
+		.with_parent_block_number(client.chain_info().best_number)
+		.build()
+		.unwrap();
 
 	// Every block needs one timestamp extrinsic.
 	let extrinsic_set_time = extrinsic_set_time(1 + MINIMUM_PERIOD_FOR_BLOCKS);
@@ -174,7 +178,11 @@ fn block_production(c: &mut Criterion) {
 
 	// Buliding the very first block is around ~30x slower than any subsequent one,
 	// so let's make sure it's built and imported before we benchmark anything.
-	let mut block_builder = client.new_block(Default::default()).unwrap();
+	let mut block_builder = BlockBuilderBuilder::new(client)
+		.on_parent_block(client.chain_info().best_hash)
+		.with_parent_block_number(client.chain_info().best_number)
+		.build()
+		.unwrap();
 	block_builder.push(extrinsic_set_time(1)).unwrap();
 	import_block(client, block_builder.build().unwrap());
 
@@ -186,14 +194,19 @@ fn block_production(c: &mut Criterion) {
 	group.sample_size(10);
 	group.throughput(Throughput::Elements(max_transfer_count as u64));
 
-	let best_hash = client.chain_info().best_hash;
+	let chain = client.chain_info();
+	let best_hash = chain.best_hash;
+	let best_number = chain.best_number;
 
 	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(best_hash, Default::default(), RecordProof::No).unwrap();
+				let mut block_builder = BlockBuilderBuilder::new(client)
+					.on_parent_block(best_hash)
+					.with_parent_block_number(best_number)
+					.build()
+					.unwrap();
 				for extrinsic in extrinsics {
 					block_builder.push(extrinsic).unwrap();
 				}
@@ -207,8 +220,11 @@ fn block_production(c: &mut Criterion) {
 		b.iter_batched(
 			|| extrinsics.clone(),
 			|extrinsics| {
-				let mut block_builder =
-					client.new_block_at(best_hash, Default::default(), RecordProof::Yes).unwrap();
+				let mut block_builder = BlockBuilderBuilder::new(client)
+					.on_parent_block(best_hash)
+					.with_parent_block_number(best_number)
+					.build()
+					.unwrap();
 				for extrinsic in extrinsics {
 					block_builder.push(extrinsic).unwrap();
 				}

substrate/bin/node/testing/src/bench.rs

@@ -39,8 +39,8 @@ use kitchensink_runtime::{
 	RuntimeCall, Signature, SystemCall, UncheckedExtrinsic,
 };
 use node_primitives::Block;
-use sc_block_builder::BlockBuilderProvider;
-use sc_client_api::execution_extensions::ExecutionExtensions;
+use sc_block_builder::BlockBuilderBuilder;
+use sc_client_api::{execution_extensions::ExecutionExtensions, UsageProvider};
 use sc_client_db::PruningMode;
 use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy, ImportResult, ImportedAux};
 use sc_executor::{NativeElseWasmExecutor, WasmExecutionMethod, WasmtimeInstantiationStrategy};
@@ -455,8 +455,13 @@ impl BenchDb {
 	/// Generate new block using this database.
 	pub fn generate_block(&mut self, content: BlockContent) -> Block {
 		let client = self.client();
+		let chain = client.usage_info().chain;
 
-		let mut block = client.new_block(Default::default()).expect("Block creation failed");
+		let mut block = BlockBuilderBuilder::new(&client)
+			.on_parent_block(chain.best_hash)
+			.with_parent_block_number(chain.best_number)
+			.build()
+			.expect("Failed to create block builder.");
 
 		for extrinsic in self.generate_inherents(&client) {
 			block.push(extrinsic).expect("Push inherent failed");

substrate/client/basic-authorship/Cargo.toml

@@ -19,7 +19,6 @@ futures-timer = "3.0.1"
 log = "0.4.17"
 prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus" }
 sc-block-builder = { path = "../block-builder" }
-sc-client-api = { path = "../api" }
 sc-proposer-metrics = { path = "../proposer-metrics" }
 sc-telemetry = { path = "../telemetry" }
 sc-transaction-pool-api = { path = "../transaction-pool/api" }
@@ -32,5 +31,6 @@ sp-runtime = { path = "../../primitives/runtime" }
 
 [dev-dependencies]
 parking_lot = "0.12.1"
+sc-client-api = { path = "../api" }
 sc-transaction-pool = { path = "../transaction-pool" }
 substrate-test-runtime-client = { path = "../../test-utils/runtime/client" }

substrate/client/basic-authorship/src/basic_authorship.rs

@@ -28,11 +28,10 @@ use futures::{
 	select,
 };
 use log::{debug, error, info, trace, warn};
-use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider};
-use sc_client_api::backend;
+use sc_block_builder::{BlockBuilderApi, BlockBuilderBuilder};
 use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_INFO};
 use sc_transaction_pool_api::{InPoolTransaction, TransactionPool};
-use sp_api::{ApiExt, ProvideRuntimeApi};
+use sp_api::{ApiExt, CallApiAt, ProvideRuntimeApi};
 use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed, HeaderBackend};
 use sp_consensus::{DisableProofRecording, EnableProofRecording, ProofRecording, Proposal};
 use sp_core::traits::SpawnNamed;
@@ -60,7 +59,7 @@ const DEFAULT_SOFT_DEADLINE_PERCENT: Percent = Percent::from_percent(50);
 const LOG_TARGET: &'static str = "basic-authorship";
 
 /// [`Proposer`] factory.
-pub struct ProposerFactory<A, B, C, PR> {
+pub struct ProposerFactory<A, C, PR> {
 	spawn_handle: Box<dyn SpawnNamed>,
 	/// The client instance.
 	client: Arc<C>,
@@ -84,11 +83,11 @@ pub struct ProposerFactory<A, B, C, PR> {
 	telemetry: Option<TelemetryHandle>,
 	/// When estimating the block size, should the proof be included?
 	include_proof_in_block_size_estimation: bool,
-	/// phantom member to pin the `Backend`/`ProofRecording` type.
-	_phantom: PhantomData<(B, PR)>,
+	/// phantom member to pin the `ProofRecording` type.
+	_phantom: PhantomData<PR>,
 }
 
-impl<A, B, C> ProposerFactory<A, B, C, DisableProofRecording> {
+impl<A, C> ProposerFactory<A, C, DisableProofRecording> {
 	/// Create a new proposer factory.
 	///
 	/// Proof recording will be disabled when using proposers built by this instance to build
@@ -114,7 +113,7 @@ impl<A, B, C> ProposerFactory<A, B, C, DisableProofRecording> {
 	}
 }
 
-impl<A, B, C> ProposerFactory<A, B, C, EnableProofRecording> {
+impl<A, C> ProposerFactory<A, C, EnableProofRecording> {
 	/// Create a new proposer factory with proof recording enabled.
 	///
 	/// Each proposer created by this instance will record a proof while building a block.
@@ -147,7 +146,7 @@ impl<A, B, C> ProposerFactory<A, B, C, EnableProofRecording> {
 	}
 }
 
-impl<A, B, C, PR> ProposerFactory<A, B, C, PR> {
+impl<A, C, PR> ProposerFactory<A, C, PR> {
 	/// Set the default block size limit in bytes.
 	///
 	/// The default value for the block size limit is:
@@ -176,29 +175,23 @@ impl<A, B, C, PR> ProposerFactory<A, B, C, PR> {
 	}
 }
 
-impl<B, Block, C, A, PR> ProposerFactory<A, B, C, PR>
+impl<Block, C, A, PR> ProposerFactory<A, C, PR>
 where
 	A: TransactionPool<Block = Block> + 'static,
-	B: backend::Backend<Block> + Send + Sync + 'static,
 	Block: BlockT,
-	C: BlockBuilderProvider<B, Block, C>
-		+ HeaderBackend<Block>
-		+ ProvideRuntimeApi<Block>
-		+ Send
-		+ Sync
-		+ 'static,
+	C: HeaderBackend<Block> + ProvideRuntimeApi<Block> + Send + Sync + 'static,
 	C::Api: ApiExt<Block> + BlockBuilderApi<Block>,
 {
 	fn init_with_now(
 		&mut self,
 		parent_header: &<Block as BlockT>::Header,
 		now: Box<dyn Fn() -> time::Instant + Send + Sync>,
-	) -> Proposer<B, Block, C, A, PR> {
+	) -> Proposer<Block, C, A, PR> {
 		let parent_hash = parent_header.hash();
 
 		info!("🙌 Starting consensus session on top of parent {:?}", parent_hash);
 
-		let proposer = Proposer::<_, _, _, _, PR> {
+		let proposer = Proposer::<_, _, _, PR> {
 			spawn_handle: self.spawn_handle.clone(),
 			client: self.client.clone(),
 			parent_hash,
@@ -217,22 +210,16 @@ where
 	}
 }
 
-impl<A, B, Block, C, PR> sp_consensus::Environment<Block> for ProposerFactory<A, B, C, PR>
+impl<A, Block, C, PR> sp_consensus::Environment<Block> for ProposerFactory<A, C, PR>
 where
 	A: TransactionPool<Block = Block> + 'static,
-	B: backend::Backend<Block> + Send + Sync + 'static,
 	Block: BlockT,
-	C: BlockBuilderProvider<B, Block, C>
-		+ HeaderBackend<Block>
-		+ ProvideRuntimeApi<Block>
-		+ Send
-		+ Sync
-		+ 'static,
+	C: HeaderBackend<Block> + ProvideRuntimeApi<Block> + CallApiAt<Block> + Send + Sync + 'static,
 	C::Api: ApiExt<Block> + BlockBuilderApi<Block>,
 	PR: ProofRecording,
 {
 	type CreateProposer = future::Ready<Result<Self::Proposer, Self::Error>>;
-	type Proposer = Proposer<B, Block, C, A, PR>;
+	type Proposer = Proposer<Block, C, A, PR>;
 	type Error = sp_blockchain::Error;
 
 	fn init(&mut self, parent_header: &<Block as BlockT>::Header) -> Self::CreateProposer {
@@ -241,7 +228,7 @@ where
 }
 
 /// The proposer logic.
-pub struct Proposer<B, Block: BlockT, C, A: TransactionPool, PR> {
+pub struct Proposer<Block: BlockT, C, A: TransactionPool, PR> {
 	spawn_handle: Box<dyn SpawnNamed>,
 	client: Arc<C>,
 	parent_hash: Block::Hash,
@@ -253,20 +240,14 @@ pub struct Proposer<B, Block: BlockT, C, A: TransactionPool, PR> {
 	include_proof_in_block_size_estimation: bool,
 	soft_deadline_percent: Percent,
 	telemetry: Option<TelemetryHandle>,
-	_phantom: PhantomData<(B, PR)>,
+	_phantom: PhantomData<PR>,
 }
 
-impl<A, B, Block, C, PR> sp_consensus::Proposer<Block> for Proposer<B, Block, C, A, PR>
+impl<A, Block, C, PR> sp_consensus::Proposer<Block> for Proposer<Block, C, A, PR>
 where
 	A: TransactionPool<Block = Block> + 'static,
-	B: backend::Backend<Block> + Send + Sync + 'static,
 	Block: BlockT,
-	C: BlockBuilderProvider<B, Block, C>
-		+ HeaderBackend<Block>
-		+ ProvideRuntimeApi<Block>
-		+ Send
-		+ Sync
-		+ 'static,
+	C: HeaderBackend<Block> + ProvideRuntimeApi<Block> + CallApiAt<Block> + Send + Sync + 'static,
 	C::Api: ApiExt<Block> + BlockBuilderApi<Block>,
 	PR: ProofRecording,
 {
@@ -313,17 +294,11 @@ where
 /// It allows us to increase block utilization.
 const MAX_SKIPPED_TRANSACTIONS: usize = 8;
 
-impl<A, B, Block, C, PR> Proposer<B, Block, C, A, PR>
+impl<A, Block, C, PR> Proposer<Block, C, A, PR>
 where
 	A: TransactionPool<Block = Block>,
-	B: backend::Backend<Block> + Send + Sync + 'static,
 	Block: BlockT,
-	C: BlockBuilderProvider<B, Block, C>
-		+ HeaderBackend<Block>
-		+ ProvideRuntimeApi<Block>
-		+ Send
-		+ Sync
-		+ 'static,
+	C: HeaderBackend<Block> + ProvideRuntimeApi<Block> + CallApiAt<Block> + Send + Sync + 'static,
 	C::Api: ApiExt<Block> + BlockBuilderApi<Block>,
 	PR: ProofRecording,
 {
@@ -335,8 +310,12 @@ where
 		block_size_limit: Option<usize>,
 	) -> Result<Proposal<Block, PR::Proof>, sp_blockchain::Error> {
 		let block_timer = time::Instant::now();
-		let mut block_builder =
-			self.client.new_block_at(self.parent_hash, inherent_digests, PR::ENABLED)?;
+		let mut block_builder = BlockBuilderBuilder::new(&*self.client)
+			.on_parent_block(self.parent_hash)
+			.with_parent_block_number(self.parent_number)
+			.with_proof_recording(PR::ENABLED)
+			.with_inherent_digests(inherent_digests)
+			.build()?;
 
 		self.apply_inherents(&mut block_builder, inherent_data)?;
 
@@ -358,7 +337,7 @@ where
 	/// Apply all inherents to the block.
 	fn apply_inherents(
 		&self,
-		block_builder: &mut sc_block_builder::BlockBuilder<'_, Block, C, B>,
+		block_builder: &mut sc_block_builder::BlockBuilder<'_, Block, C>,
 		inherent_data: InherentData,
 	) -> Result<(), sp_blockchain::Error> {
 		let create_inherents_start = time::Instant::now();
@@ -402,7 +381,7 @@ where
 	/// Apply as many extrinsics as possible to the block.
 	async fn apply_extrinsics(
 		&self,
-		block_builder: &mut sc_block_builder::BlockBuilder<'_, Block, C, B>,
+		block_builder: &mut sc_block_builder::BlockBuilder<'_, Block, C>,
 		deadline: time::Instant,
 		block_size_limit: Option<usize>,
 	) -> Result<EndProposingReason, sp_blockchain::Error> {
@@ -976,8 +955,12 @@ mod tests {
 		// Exact block_limit, which includes:
 		// 99 (header_size) + 718 (proof@initialize_block) + 246 (one Transfer extrinsic)
 		let block_limit = {
-			let builder =
-				client.new_block_at(genesis_header.hash(), Default::default(), true).unwrap();
+			let builder = BlockBuilderBuilder::new(&*client)
+				.on_parent_block(genesis_header.hash())
+				.with_parent_block_number(0)
+				.enable_proof_recording()
+				.build()
+				.unwrap();
 			builder.estimate_block_size(true) + extrinsics[0].encoded_size()
 		};
 		let block = block_on(proposer.propose(

substrate/client/block-builder/Cargo.toml

@@ -16,7 +16,6 @@ targets = ["x86_64-unknown-linux-gnu"]
 codec = { package = "parity-scale-codec", version = "3.6.1", features = [
 	"derive",
 ] }
-sc-client-api = { path = "../api" }
 sp-api = { path = "../../primitives/api" }
 sp-block-builder = { path = "../../primitives/block-builder" }
 sp-blockchain = { path = "../../primitives/blockchain" }