Cleanup service (#9265)

* Renames, move the RPC builder to own field, cleanup

* Rename rpsee_builder to rpc_builder

* Formatting

bin/node-template/node/src/service.rs

@@ -114,6 +114,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
 		keystore_container,
 		select_chain,
 		transaction_pool,
+		rpc_builder: Box::new(|_, _| RpcModule::new(())),
 		other: (grandpa_block_import, grandpa_link, telemetry),
 	})
 }
@@ -135,6 +136,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
 		mut keystore_container,
 		select_chain,
 		transaction_pool,
+		rpc_builder: _rpc_builder,
 		other: (block_import, grandpa_link, mut telemetry),
 	} = new_partial(&config)?;
 
@@ -182,7 +184,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
 			task_manager: &mut task_manager,
 			transaction_pool: transaction_pool.clone(),
 			// TODO: (dp) implement
-			rpsee_builder: Box::new(|_, _| { RpcModule::new(()) }),
+			rpc_builder: Box::new(|_, _| { RpcModule::new(()) }),
 			on_demand: None,
 			remote_blockchain: None,
 			backend,
@@ -401,7 +403,7 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError>
 		task_manager: &mut task_manager,
 		on_demand: Some(on_demand),
 		// TODO: (dp) implement
-		rpsee_builder: Box::new(|_, _| RpcModule::new(())),
+		rpc_builder: Box::new(|_, _| RpcModule::new(())),
 		config,
 		client,
 		keystore: keystore_container.sync_keystore(),

bin/node/cli/src/service.rs

@@ -60,17 +60,12 @@ pub fn new_partial(
 	sp_consensus::DefaultImportQueue<Block, FullClient>,
 	sc_transaction_pool::FullPool<Block, FullClient>,
 	(
-		// rpc_extensions_builder (jsonrpc, old,  TODO: (dp) remove)
-		impl Fn(node_rpc::DenyUnsafe, sc_rpc::SubscriptionTaskExecutor) -> node_rpc::IoHandler,
-		// rpc setup (jsonrpsee)
-		impl FnOnce(node_rpc::DenyUnsafe, Arc<sc_rpc::SubscriptionTaskExecutor>) -> RpcModule<()>,
-		// import setup
+		// Block import setup.
 		(
 			sc_consensus_babe::BabeBlockImport<Block, FullClient, FullGrandpaBlockImport>,
 			grandpa::LinkHalf<Block, FullClient, FullSelectChain>,
 			sc_consensus_babe::BabeLink<Block>,
 		),
-		grandpa::SharedVoterState,
 		Option<Telemetry>,
 	)
 >, ServiceError> {
@@ -149,7 +144,6 @@ pub fn new_partial(
 		telemetry.as_ref().map(|x| x.handle()),
 	)?;
 
-	// TODO: (dp) cleanup all of this crap when removing the jsonrpc stuff below.
 	// Grandpa stuff
 	let shared_authority_set = grandpa_link.shared_authority_set().clone();
 	let justification_stream = grandpa_link.justification_stream().clone();
@@ -164,63 +158,60 @@ pub fn new_partial(
 	let shared_epoch_changes = babe_link.epoch_changes().clone();
 	// System
 	let transaction_pool2 = transaction_pool.clone();
-	let rpsee_builder = move |deny_unsafe, executor| -> RpcModule<()> {
-		let grandpa_rpc = GrandpaRpc::new(
-			executor,
-			shared_authority_set.clone(),
-			grandpa::SharedVoterState::empty(),
-			justification_stream,
-			grandpa::FinalityProofProvider::new_for_service(
-				backend2,
-				Some(shared_authority_set.clone()),
-			),
-		).into_rpc_module().expect("TODO: error handling");
-
-		let babe_rpc = BabeRpc::new(
-			client2.clone(),
-			babe_link.epoch_changes().clone(),
-			sync_keystore,
-			babe_link.config().clone(),
-			select_chain2,
-			deny_unsafe,
-		).into_rpc_module().expect("TODO: error handling");
-		let sync_state_rpc = SyncStateRpc::new(
-			chain_spec,
-			client2.clone(),
-			shared_authority_set.clone(),
-			shared_epoch_changes,
-			deny_unsafe,
-		).into_rpc_module().expect("TODO: error handling");
-		let transaction_payment_rpc = TransactionPaymentRpc::new(client2.clone()).into_rpc_module().expect("TODO: error handling");
-		let system_rpc_backend = SystemRpcBackendFull::new(client2.clone(), transaction_pool2.clone(), deny_unsafe);
-		let system_rpc = SystemRpc::new(Box::new(system_rpc_backend)).into_rpc_module().expect("TODO: error handling");
-
-		let mmr_rpc = MmrRpc::new(client2.clone()).into_rpc_module().expect("TODO: error handling");
-		let contracts_rpc = ContractsRpc::new(client2.clone()).into_rpc_module().expect("TODO: error handling");
-
-		let mut module = RpcModule::new(());
-		module.merge(grandpa_rpc).expect("TODO: error handling");
-		module.merge(babe_rpc).expect("TODO: error handling");
-		module.merge(sync_state_rpc).expect("TODO: error handling");
-		module.merge(transaction_payment_rpc).expect("TODO: error handling");
-		module.merge(system_rpc).expect("TODO: error handling");
-		module.merge(mmr_rpc).expect("TODO: error handling");
-		module.merge(contracts_rpc).expect("TODO: error handling");
-		module
-	};
+	let rpc_builder = Box::new(move |deny_unsafe, executor| -> RpcModule<()> {
+			let grandpa_rpc = GrandpaRpc::new(
+				executor,
+				shared_authority_set.clone(),
+				grandpa::SharedVoterState::empty(),
+				justification_stream,
+				grandpa::FinalityProofProvider::new_for_service(
+					backend2,
+					Some(shared_authority_set.clone()),
+				),
+			).into_rpc_module().expect("TODO: error handling");
+
+			let babe_rpc = BabeRpc::new(
+				client2.clone(),
+				babe_link.epoch_changes().clone(),
+				sync_keystore,
+				babe_link.config().clone(),
+				select_chain2,
+				deny_unsafe,
+			).into_rpc_module().expect("TODO: error handling");
+			let sync_state_rpc = SyncStateRpc::new(
+				chain_spec,
+				client2.clone(),
+				shared_authority_set.clone(),
+				shared_epoch_changes,
+				deny_unsafe,
+			).into_rpc_module().expect("TODO: error handling");
+			let transaction_payment_rpc = TransactionPaymentRpc::new(
+				client2.clone()
+			).into_rpc_module().expect("TODO: error handling");
+			let system_rpc_backend = SystemRpcBackendFull::new(client2.clone(), transaction_pool2.clone(), deny_unsafe);
+			let system_rpc = SystemRpc::new(
+				Box::new(system_rpc_backend)
+			).into_rpc_module().expect("TODO: error handling");
+			let mmr_rpc = MmrRpc::new(
+				client2.clone()
+			).into_rpc_module().expect("TODO: error handling");
+			let contracts_rpc = ContractsRpc::new(
+				client2.clone()
+			).into_rpc_module().expect("TODO: error handling");
+
+			let mut module = RpcModule::new(());
+			module.merge(grandpa_rpc).expect("TODO: error handling");
+			module.merge(babe_rpc).expect("TODO: error handling");
+			module.merge(sync_state_rpc).expect("TODO: error handling");
+			module.merge(transaction_payment_rpc).expect("TODO: error handling");
+			module.merge(system_rpc).expect("TODO: error handling");
+			module.merge(mmr_rpc).expect("TODO: error handling");
+			module.merge(contracts_rpc).expect("TODO: error handling");
+			module
+		});
 
 	let import_setup = (block_import, grandpa_link, babe_link2);
 
-	// TODO: (dp) remove this when all APIs are ported.
-	let (rpc_extensions_builder, rpc_setup) = {
-		let rpc_setup = grandpa::SharedVoterState::empty();
-		let rpc_extensions_builder = move |_deny_unsafe, _subscription_executor| {
-			node_rpc::create_full()
-		};
-
-		(rpc_extensions_builder, rpc_setup)
-	};
-
 	Ok(sc_service::PartialComponents {
 		client,
 		backend,
@@ -229,8 +220,8 @@ pub fn new_partial(
 		select_chain,
 		import_queue,
 		transaction_pool,
-		// TODO: (dp) `rpc_setup` is a copy of `shared_voter_state`, but why?
-		other: (rpc_extensions_builder, Box::new(rpsee_builder), import_setup, rpc_setup, telemetry),
+		rpc_builder,
+		other: (import_setup, telemetry),
 	})
 }
 
@@ -257,16 +248,13 @@ pub fn new_full_base(
 		keystore_container,
 		select_chain,
 		transaction_pool,
+		rpc_builder,
 		other: (
-			_rpc_extensions_builder,
-			rpsee_builder,
 			import_setup,
-			rpc_setup,
 			mut telemetry
 		),
 	} = new_partial(&config)?;
 
-	let shared_voter_state = rpc_setup;
 	let auth_disc_publish_non_global_ips = config.network.allow_non_globals_in_dht;
 
 	config.network.extra_sets.push(grandpa::grandpa_peers_set_config());
@@ -313,7 +301,7 @@ pub fn new_full_base(
 			client: client.clone(),
 			keystore: keystore_container.sync_keystore(),
 			network: network.clone(),
-			rpsee_builder: Box::new(rpsee_builder),
+			rpc_builder: Box::new(rpc_builder),
 			transaction_pool: transaction_pool.clone(),
 			task_manager: &mut task_manager,
 			on_demand: None,
@@ -439,7 +427,7 @@ pub fn new_full_base(
 			telemetry: telemetry.as_ref().map(|x| x.handle()),
 			voting_rule: grandpa::VotingRulesBuilder::default().build(),
 			prometheus_registry,
-			shared_voter_state,
+			shared_voter_state: grandpa::SharedVoterState::empty(),
 		};
 
 		// the GRANDPA voter task is considered infallible, i.e.
@@ -600,7 +588,7 @@ pub fn new_light_base(mut config: Configuration) -> Result<(
 			on_demand: Some(on_demand),
 			remote_blockchain: Some(backend.remote_blockchain()),
 			// TODO(niklasad1): implement.
-			rpsee_builder: Box::new(|_, _| RpcModule::new(())),
+			rpc_builder: Box::new(|_, _| RpcModule::new(())),
 			client: client.clone(),
 			transaction_pool: transaction_pool.clone(),
 			keystore: keystore_container.sync_keystore(),

client/service/src/builder.rs

@@ -32,7 +32,7 @@ use sp_consensus::{
 	block_validation::{BlockAnnounceValidator, DefaultBlockAnnounceValidator, Chain},
 	import_queue::ImportQueue,
 };
-use sc_rpc::SubscriptionTaskExecutor;
+use sc_rpc::{DenyUnsafe, SubscriptionTaskExecutor};
 use futures::{
 	FutureExt, StreamExt,
 	future::ready,
@@ -455,7 +455,7 @@ pub struct SpawnTasksParams<'a, TBl: BlockT, TCl, TExPool, Backend> {
 	/// A shared transaction pool.
 	pub transaction_pool: Arc<TExPool>,
 	/// Builds additional [`RpcModule`]s that should be added to the server
-	pub rpsee_builder: Box<dyn FnOnce(sc_rpc::DenyUnsafe, Arc<SubscriptionTaskExecutor>) -> RpcModule<()>>,
+	pub rpc_builder: Box<dyn FnOnce(DenyUnsafe, Arc<SubscriptionTaskExecutor>) -> RpcModule<()>>,
 	/// An optional, shared remote blockchain instance. Used for light clients.
 	pub remote_blockchain: Option<Arc<dyn RemoteBlockchain<TBl>>>,
 	/// A shared network instance.
@@ -525,7 +525,7 @@ pub fn spawn_tasks<TBl, TBackend, TExPool, TCl>(
 		backend,
 		keystore,
 		transaction_pool,
-		rpsee_builder,
+		rpc_builder,
 		remote_blockchain,
 		network,
 		system_rpc_tx,
@@ -596,7 +596,7 @@ pub fn spawn_tasks<TBl, TBackend, TExPool, TCl>(
 	);
 
 	// jsonrpsee RPC
-	let gen_rpc_module = |deny_unsafe: sc_rpc::DenyUnsafe| {
+	let gen_rpc_module = |deny_unsafe: DenyUnsafe| {
 		gen_rpc_module(
 			deny_unsafe,
 			task_manager.spawn_handle(),
@@ -608,7 +608,7 @@ pub fn spawn_tasks<TBl, TBackend, TExPool, TCl>(
 			system_rpc_tx.clone(),
 			&config,
 			backend.offchain_storage(),
-			rpsee_builder,
+			rpc_builder,
 		)
 	};
 
@@ -695,7 +695,7 @@ fn init_telemetry<TBl: BlockT, TCl: BlockBackend<TBl>>(
 // Maciej: This is very WIP, mocking the original `gen_handler`. All of the `jsonrpsee`
 // specific logic should be merged back to `gen_handler` down the road.
 fn gen_rpc_module<TBl, TBackend, TCl, TExPool>(
-	_deny_unsafe: sc_rpc::DenyUnsafe,
+	_deny_unsafe: DenyUnsafe,
 	spawn_handle: SpawnTaskHandle,
 	client: Arc<TCl>,
 	on_demand: Option<Arc<OnDemand<TBl>>>,
@@ -705,7 +705,7 @@ fn gen_rpc_module<TBl, TBackend, TCl, TExPool>(
 	system_rpc_tx: TracingUnboundedSender<sc_rpc::system::Request<TBl>>,
 	config: &Configuration,
 	offchain_storage: Option<<TBackend as sc_client_api::backend::Backend<TBl>>::OffchainStorage>,
-	rpsee_builder: Box<dyn FnOnce(sc_rpc::DenyUnsafe, Arc<SubscriptionTaskExecutor>) -> RpcModule<()>>,
+	rpc_builder: Box<dyn FnOnce(DenyUnsafe, Arc<SubscriptionTaskExecutor>) -> RpcModule<()>>,
 ) -> RpcModule<()>
 	where
 		TBl: BlockT,
@@ -722,7 +722,7 @@ fn gen_rpc_module<TBl, TBackend, TCl, TExPool>(
 	const UNIQUE_METHOD_NAMES_PROOF: &str = "Method names are unique; qed";
 
 	// TODO(niklasad1): expose CORS to jsonrpsee to handle this propely.
-	let deny_unsafe = sc_rpc::DenyUnsafe::No;
+	let deny_unsafe = DenyUnsafe::No;
 
 	let system_info = sc_rpc::system::SystemInfo {
 		chain_name: config.chain_spec.name().into(),
@@ -800,7 +800,7 @@ fn gen_rpc_module<TBl, TBackend, TCl, TExPool>(
 	rpc_api.merge(state).expect(UNIQUE_METHOD_NAMES_PROOF);
 	rpc_api.merge(child_state).expect(UNIQUE_METHOD_NAMES_PROOF);
 	// Additional [`RpcModule`]s defined in the node to fit the specific blockchain
-	let extra_rpcs = rpsee_builder(deny_unsafe, task_executor.clone());
+	let extra_rpcs = rpc_builder(deny_unsafe, task_executor.clone());
 	rpc_api.merge(extra_rpcs).expect(UNIQUE_METHOD_NAMES_PROOF);
 
 	rpc_api

client/service/src/lib.rs

@@ -47,6 +47,7 @@ use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 use parity_util_mem::MallocSizeOf;
 use sp_utils::mpsc::TracingUnboundedReceiver;
 use jsonrpsee::RpcModule;
+use sc_rpc::{DenyUnsafe, SubscriptionTaskExecutor};
 
 pub use self::error::Error;
 pub use self::builder::{
@@ -129,6 +130,8 @@ pub struct PartialComponents<Client, Backend, SelectChain, ImportQueue, Transact
 	pub import_queue: ImportQueue,
 	/// A shared transaction pool.
 	pub transaction_pool: Arc<TransactionPool>,
+	/// RPC module builder.
+	pub rpc_builder: Box<dyn FnOnce(DenyUnsafe, Arc<SubscriptionTaskExecutor>) -> RpcModule<()>>,
 	/// Everything else that needs to be passed into the main build function.
 	pub other: Other,
 }

frame/contracts/rpc/src/lib.rs

@@ -153,8 +153,7 @@ where
 		module.register_method(
 			"contracts_call",
 			|params, contracts| -> Result<ContractExecResult, CallError> {
-				let (call_request, at): (CallRequest<AccountId>, Option<<Block as BlockT>::Hash>) =
-					params.parse()?;
+				let (call_request, at): (CallRequest<AccountId>, Option<<Block as BlockT>::Hash>) = params.parse()?;
 				let api = contracts.client.runtime_api();
 				let at = BlockId::hash(at.unwrap_or_else(|| contracts.client.info().best_hash));
 
@@ -186,9 +185,7 @@ where
 		// This method is useful for UIs to dry-run contract instantiations.
 		module.register_method(
 			"contracts_instantiate",
-			|params,
-			 contracts|
-			 -> Result<
+			|params, contracts| -> Result<
 				ContractInstantiateResult<
 					AccountId,
 					<<Block as BlockT>::Header as HeaderT>::Number,

test-utils/test-runner/Cargo.toml

@@ -49,6 +49,6 @@ futures = { package = "futures", version = "0.3", features = ["compat"] }
 tokio = { version = "1", features = ["full"] }
 
 # Calling RPC
+
 jsonrpc-core = "15.1"
-jsonrpsee-ws-server = { git = "https://github.com/paritytech/jsonrpsee", branch = "master" }
-jsonrpsee-types = { git = "https://github.com/paritytech/jsonrpsee", branch = "master" }
+jsonrpsee = { git = "https://github.com/paritytech/jsonrpsee", branch = "master", features = ["server"] }

test-utils/test-runner/src/node.rs

@@ -18,6 +18,7 @@
 
 use std::sync::Arc;
 
+use jsonrpsee::RpcModule;
 use futures::{FutureExt, SinkExt, channel::{mpsc, oneshot}};
 use manual_seal::{run_manual_seal, EngineCommand, ManualSealParams};
 use sc_cli::build_runtime;
@@ -180,7 +181,7 @@ impl<T: ChainInfo> Node<T> {
 				keystore,
 				on_demand: None,
 				transaction_pool: transaction_pool.clone(),
-				rpsee_builder: Box::new(|_, _| jsonrpsee_ws_server::RpcModule::new(())),
+				rpc_builder: Box::new(|_, _| RpcModule::new(())),
 				remote_blockchain: None,
 				network,
 				system_rpc_tx,