Service various cleanups (#3238)

* Remove generic from sign()

* Remove mandatory RuntimeGenesis trait req

* Remove requirement from Configuration

* Relax trait bounds of core/cli

* Move method

* The config field is no longer public

* Remove Components from bounds of functions

* Implement DerefMut for LightComponents

* Implement Executor for Full/LightComponents

* Fix bad merge

* Fix forgotten config()

* Fix build
This commit is contained in:
Pierre Krieger
2019-08-08 16:14:30 +02:00
committed by Gavin Wood
parent fe18b4055d
commit bafc7202ca
8 changed files with 133 additions and 88 deletions
+38 -34
View File
@@ -27,6 +27,7 @@ pub mod informant;
use client::ExecutionStrategies;
use service::{
config::Configuration,
ServiceFactory, FactoryFullConfiguration, RuntimeGenesis,
FactoryGenesis, PruningMode, ChainSpec,
};
@@ -220,17 +221,17 @@ where
fdlimit::raise_fd_limit();
match cli_args {
params::CoreParams::Run(params) => run_node::<F, _, _, _, _>(
params::CoreParams::Run(params) => run_node(
params, spec_factory, exit, run_service, impl_name, version,
).map(|_| None),
params::CoreParams::BuildSpec(params) =>
build_spec::<F, _>(params, spec_factory, version).map(|_| None),
build_spec(params, spec_factory, version).map(|_| None),
params::CoreParams::ExportBlocks(params) =>
export_blocks::<F, _, _>(params, spec_factory, exit, version).map(|_| None),
params::CoreParams::ImportBlocks(params) =>
import_blocks::<F, _, _>(params, spec_factory, exit, version).map(|_| None),
params::CoreParams::PurgeChain(params) =>
purge_chain::<F, _>(params, spec_factory, version).map(|_| None),
purge_chain(params, spec_factory, version).map(|_| None),
params::CoreParams::Revert(params) =>
revert_chain::<F, _>(params, spec_factory, version).map(|_| None),
params::CoreParams::Custom(params) => Ok(Some(params)),
@@ -292,8 +293,8 @@ fn parse_ed25519_secret(hex: &String) -> error::Result<network::config::Ed25519S
}
/// Fill the given `PoolConfiguration` by looking at the cli parameters.
fn fill_transaction_pool_configuration<F: ServiceFactory>(
options: &mut FactoryFullConfiguration<F>,
fn fill_transaction_pool_configuration<C, G>(
options: &mut Configuration<C, G>,
params: TransactionPoolParams,
) -> error::Result<()> {
// ready queue
@@ -384,12 +385,13 @@ fn fill_config_keystore_password<C, G>(
Ok(())
}
fn create_run_node_config<F, S>(
fn create_run_node_config<C, G, S>(
cli: RunCmd, spec_factory: S, impl_name: &'static str, version: &VersionInfo
) -> error::Result<FactoryFullConfiguration<F>>
) -> error::Result<Configuration<C, G>>
where
F: ServiceFactory,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
C: Default,
G: RuntimeGenesis,
S: FnOnce(&str) -> Result<Option<ChainSpec<G>>, String>,
{
let spec = load_spec(&cli.shared_params, spec_factory)?;
let mut config = service::Configuration::default_with_spec(spec.clone());
@@ -471,7 +473,7 @@ where
is_dev,
)?;
fill_transaction_pool_configuration::<F>(&mut config, cli.pool_config)?;
fill_transaction_pool_configuration(&mut config, cli.pool_config)?;
config.dev_key_seed = cli.keyring.account
.map(|a| format!("//{}", a)).or_else(|| {
@@ -516,7 +518,7 @@ where
Ok(config)
}
fn run_node<F, S, RS, E, RP>(
fn run_node<C, G, S, RS, E, RP>(
cli: MergeParameters<RunCmd, RP>,
spec_factory: S,
exit: E,
@@ -526,12 +528,13 @@ fn run_node<F, S, RS, E, RP>(
) -> error::Result<()>
where
RP: StructOpt + Clone,
F: ServiceFactory,
C: Default,
G: RuntimeGenesis,
E: IntoExit,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
RS: FnOnce(E, RunCmd, RP, FactoryFullConfiguration<F>) -> Result<(), String>,
S: FnOnce(&str) -> Result<Option<ChainSpec<G>>, String>,
RS: FnOnce(E, RunCmd, RP, Configuration<C, G>) -> Result<(), String>,
{
let config = create_run_node_config::<F, _>(cli.left.clone(), spec_factory, impl_name, version)?;
let config = create_run_node_config(cli.left.clone(), spec_factory, impl_name, version)?;
run_service(exit, cli.left, cli.right, config).map_err(Into::into)
}
@@ -544,13 +547,13 @@ where
// 9803-9874 Unassigned
// 9926-9949 Unassigned
fn with_default_boot_node<F>(
spec: &mut ChainSpec<FactoryGenesis<F>>,
fn with_default_boot_node<G>(
spec: &mut ChainSpec<G>,
cli: BuildSpecCmd,
version: &VersionInfo,
) -> error::Result<()>
where
F: ServiceFactory
G: RuntimeGenesis
{
if spec.boot_nodes().is_empty() {
let base_path = base_path(&cli.shared_params, version);
@@ -568,20 +571,20 @@ where
Ok(())
}
fn build_spec<F, S>(
fn build_spec<G, S>(
cli: BuildSpecCmd,
spec_factory: S,
version: &VersionInfo,
) -> error::Result<()>
where
F: ServiceFactory,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
G: RuntimeGenesis,
S: FnOnce(&str) -> Result<Option<ChainSpec<G>>, String>,
{
info!("Building chain spec");
let raw_output = cli.raw;
let mut spec = load_spec(&cli.shared_params, spec_factory)?;
with_default_boot_node::<F>(&mut spec, cli, version)?;
let json = service::chain_ops::build_spec::<FactoryGenesis<F>>(spec, raw_output)?;
with_default_boot_node(&mut spec, cli, version)?;
let json = service::chain_ops::build_spec(spec, raw_output)?;
print!("{}", json);
@@ -589,12 +592,13 @@ where
}
/// Creates a configuration including the database path.
pub fn create_config_with_db_path<F, S>(
pub fn create_config_with_db_path<C, G, S>(
spec_factory: S, cli: &SharedParams, version: &VersionInfo,
) -> error::Result<FactoryFullConfiguration<F>>
) -> error::Result<Configuration<C, G>>
where
F: ServiceFactory,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
C: Default,
G: RuntimeGenesis,
S: FnOnce(&str) -> Result<Option<ChainSpec<G>>, String>,
{
let spec = load_spec(cli, spec_factory)?;
let base_path = base_path(cli, version);
@@ -616,7 +620,7 @@ where
E: IntoExit,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
{
let config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
let config = create_config_with_db_path(spec_factory, &cli.shared_params, version)?;
info!("DB path: {}", config.database_path.display());
let from = cli.from.unwrap_or(1);
@@ -649,7 +653,7 @@ where
E: IntoExit,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
{
let mut config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
let mut config = create_config_with_db_path(spec_factory, &cli.shared_params, version)?;
config.execution_strategies = ExecutionStrategies {
importing: cli.execution.into(),
other: cli.execution.into(),
@@ -679,21 +683,21 @@ where
F: ServiceFactory,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
{
let config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
let config = create_config_with_db_path(spec_factory, &cli.shared_params, version)?;
let blocks = cli.num;
Ok(service::chain_ops::revert_chain::<F>(config, blocks.into())?)
}
fn purge_chain<F, S>(
fn purge_chain<G, S>(
cli: PurgeChainCmd,
spec_factory: S,
version: &VersionInfo,
) -> error::Result<()>
where
F: ServiceFactory,
S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
G: RuntimeGenesis,
S: FnOnce(&str) -> Result<Option<ChainSpec<G>>, String>,
{
let config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
let config = create_config_with_db_path::<(), _, _>(spec_factory, &cli.shared_params, version)?;
let db_path = config.database_path;
if cli.yes == false {
+3 -1
View File
@@ -127,7 +127,7 @@ impl<G> Clone for ChainSpec<G> {
}
}
impl<G: RuntimeGenesis> ChainSpec<G> {
impl<G> ChainSpec<G> {
/// A list of bootnode addresses.
pub fn boot_nodes(&self) -> &[String] {
&self.spec.boot_nodes
@@ -215,7 +215,9 @@ impl<G: RuntimeGenesis> ChainSpec<G> {
genesis: GenesisSource::Factory(constructor),
}
}
}
impl<G: RuntimeGenesis> ChainSpec<G> {
/// Dump to json string.
pub fn to_json(self, raw: bool) -> Result<String, String> {
#[derive(Serialize, Deserialize)]
+26
View File
@@ -506,6 +506,16 @@ impl<Factory: ServiceFactory> Future for FullComponents<Factory> {
}
}
impl<Factory: ServiceFactory> Executor<Box<dyn Future<Item = (), Error = ()> + Send>>
for FullComponents<Factory> {
fn execute(
&self,
future: Box<dyn Future<Item = (), Error = ()> + Send>
) -> Result<(), futures::future::ExecuteError<Box<dyn Future<Item = (), Error = ()> + Send>>> {
self.service.execute(future)
}
}
impl<Factory: ServiceFactory> Components for FullComponents<Factory> {
type Factory = Factory;
type Executor = FullExecutor<Factory>;
@@ -606,6 +616,12 @@ impl<Factory: ServiceFactory> Deref for LightComponents<Factory> {
}
}
impl<Factory: ServiceFactory> DerefMut for LightComponents<Factory> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.service
}
}
impl<Factory: ServiceFactory> Future for LightComponents<Factory> {
type Item = ();
type Error = ();
@@ -615,6 +631,16 @@ impl<Factory: ServiceFactory> Future for LightComponents<Factory> {
}
}
impl<Factory: ServiceFactory> Executor<Box<dyn Future<Item = (), Error = ()> + Send>>
for LightComponents<Factory> {
fn execute(
&self,
future: Box<dyn Future<Item = (), Error = ()> + Send>
) -> Result<(), futures::future::ExecuteError<Box<dyn Future<Item = (), Error = ()> + Send>>> {
self.service.execute(future)
}
}
impl<Factory: ServiceFactory> Components for LightComponents<Factory> {
type Factory = Factory;
type Executor = LightExecutor<Factory>;
+52 -37
View File
@@ -93,7 +93,7 @@ pub struct Service<Components: components::Components> {
/// The elements must then be polled manually.
to_poll: Vec<Box<dyn Future<Item = (), Error = ()> + Send>>,
/// Configuration of this Service
pub config: FactoryFullConfiguration<Components::Factory>,
config: FactoryFullConfiguration<Components::Factory>,
rpc_handlers: rpc::RpcHandler,
_rpc: Box<dyn std::any::Any + Send + Sync>,
_telemetry: Option<tel::Telemetry>,
@@ -149,13 +149,6 @@ pub struct TelemetryOnConnect {
}
impl<Components: components::Components> Service<Components> {
/// Get event stream for telemetry connection established events.
pub fn telemetry_on_connect_stream(&self) -> TelemetryOnConnectNotifications {
let (sink, stream) = mpsc::unbounded();
self._telemetry_on_connect_sinks.lock().push(sink);
stream
}
/// Creates a new service.
pub fn new(
mut config: FactoryFullConfiguration<Components::Factory>,
@@ -200,7 +193,7 @@ impl<Components: components::Components> Service<Components> {
let transaction_pool = Arc::new(
Components::build_transaction_pool(config.transaction_pool.clone(), client.clone())?
);
let transaction_pool_adapter = Arc::new(TransactionPoolAdapter::<Components> {
let transaction_pool_adapter = Arc::new(TransactionPoolAdapter {
imports_external_transactions: !config.roles.is_light(),
pool: transaction_pool.clone(),
client: client.clone(),
@@ -383,9 +376,9 @@ impl<Components: components::Components> Service<Components> {
)
};
let rpc_handlers = gen_handler();
let rpc = start_rpc_servers::<Components::Factory, _>(&config, gen_handler)?;
let rpc = start_rpc_servers(&config, gen_handler)?;
let _ = to_spawn_tx.unbounded_send(Box::new(build_network_future::<Components, _, _>(
let _ = to_spawn_tx.unbounded_send(Box::new(build_network_future(
network_mut,
client.clone(),
network_status_sinks.clone(),
@@ -460,6 +453,27 @@ impl<Components: components::Components> Service<Components> {
})
}
/// Returns a reference to the config passed at initialization.
pub fn config(&self) -> &FactoryFullConfiguration<Components::Factory> {
&self.config
}
/// Returns a reference to the config passed at initialization.
///
/// > **Note**: This method is currently necessary because we extract some elements from the
/// > configuration at the end of the service initialization. It is intended to be
/// > removed.
pub fn config_mut(&mut self) -> &mut FactoryFullConfiguration<Components::Factory> {
&mut self.config
}
/// Get event stream for telemetry connection established events.
pub fn telemetry_on_connect_stream(&self) -> TelemetryOnConnectNotifications {
let (sink, stream) = mpsc::unbounded();
self._telemetry_on_connect_sinks.lock().push(sink);
stream
}
/// Return a shared instance of Telemetry (if enabled)
pub fn telemetry(&self) -> Option<tel::Telemetry> {
self._telemetry.as_ref().map(|t| t.clone())
@@ -577,14 +591,15 @@ impl<Components> Executor<Box<dyn Future<Item = (), Error = ()> + Send>>
///
/// The `status_sink` contain a list of senders to send a periodic network status to.
fn build_network_future<
Components: components::Components,
S: network::specialization::NetworkSpecialization<ComponentBlock<Components>>,
B: BlockT,
C: client::BlockchainEvents<B>,
S: network::specialization::NetworkSpecialization<B>,
H: network::ExHashT
> (
mut network: network::NetworkWorker<ComponentBlock<Components>, S, H>,
client: Arc<ComponentClient<Components>>,
status_sinks: Arc<Mutex<Vec<mpsc::UnboundedSender<(NetworkStatus<ComponentBlock<Components>>, NetworkState)>>>>,
rpc_rx: futures03::channel::mpsc::UnboundedReceiver<rpc::apis::system::Request<ComponentBlock<Components>>>,
mut network: network::NetworkWorker<B, S, H>,
client: Arc<C>,
status_sinks: Arc<Mutex<Vec<mpsc::UnboundedSender<(NetworkStatus<B>, NetworkState)>>>>,
rpc_rx: futures03::channel::mpsc::UnboundedReceiver<rpc::apis::system::Request<B>>,
should_have_peers: bool,
) -> impl Future<Item = (), Error = ()> {
// Compatibility shim while we're transitionning to stable Futures.
@@ -710,8 +725,8 @@ impl<Components> Drop for Service<Components> where Components: components::Comp
/// Starts RPC servers that run in their own thread, and returns an opaque object that keeps them alive.
#[cfg(not(target_os = "unknown"))]
fn start_rpc_servers<F: ServiceFactory, H: FnMut() -> rpc::RpcHandler>(
config: &FactoryFullConfiguration<F>,
fn start_rpc_servers<C, G, H: FnMut() -> rpc::RpcHandler>(
config: &Configuration<C, G>,
mut gen_handler: H
) -> Result<Box<dyn std::any::Any + Send + Sync>, error::Error> {
fn maybe_start_server<T, F>(address: Option<SocketAddr>, mut start: F) -> Result<Option<T>, io::Error>
@@ -751,8 +766,8 @@ fn start_rpc_servers<F: ServiceFactory, H: FnMut() -> rpc::RpcHandler>(
/// Starts RPC servers that run in their own thread, and returns an opaque object that keeps them alive.
#[cfg(target_os = "unknown")]
fn start_rpc_servers<F: ServiceFactory, H: FnMut() -> rpc::RpcHandler>(
_: &FactoryFullConfiguration<F>,
fn start_rpc_servers<C, G, H: FnMut() -> rpc::RpcHandler>(
_: &Configuration<C, G>,
_: H
) -> Result<Box<std::any::Any + Send + Sync>, error::Error> {
Ok(Box::new(()))
@@ -779,16 +794,10 @@ impl RpcSession {
}
/// Transaction pool adapter.
pub struct TransactionPoolAdapter<C: Components> {
pub struct TransactionPoolAdapter<C, P> {
imports_external_transactions: bool,
pool: Arc<TransactionPool<C::TransactionPoolApi>>,
client: Arc<ComponentClient<C>>,
}
impl<C: Components> TransactionPoolAdapter<C> {
fn best_block_id(&self) -> Option<BlockId<ComponentBlock<C>>> {
Some(BlockId::hash(self.client.info().chain.best_hash))
}
pool: Arc<P>,
client: Arc<C>,
}
/// Get transactions for propagation.
@@ -812,14 +821,20 @@ where
.collect()
}
impl<C: Components> network::TransactionPool<ComponentExHash<C>, ComponentBlock<C>> for
TransactionPoolAdapter<C> where <C as components::Components>::RuntimeApi: Send + Sync
impl<B, H, C, PoolApi, E> network::TransactionPool<H, B> for
TransactionPoolAdapter<C, TransactionPool<PoolApi>>
where
C: network::ClientHandle<B> + Send + Sync,
PoolApi: ChainApi<Block=B, Hash=H, Error=E>,
B: BlockT,
H: std::hash::Hash + Eq + sr_primitives::traits::Member + serde::Serialize,
E: txpool::error::IntoPoolError + From<txpool::error::Error>,
{
fn transactions(&self) -> Vec<(ComponentExHash<C>, ComponentExtrinsic<C>)> {
fn transactions(&self) -> Vec<(H, <B as BlockT>::Extrinsic)> {
transactions_to_propagate(&self.pool)
}
fn import(&self, transaction: &ComponentExtrinsic<C>) -> Option<ComponentExHash<C>> {
fn import(&self, transaction: &<B as BlockT>::Extrinsic) -> Option<H> {
if !self.imports_external_transactions {
debug!("Transaction rejected");
return None;
@@ -828,12 +843,12 @@ impl<C: Components> network::TransactionPool<ComponentExHash<C>, ComponentBlock<
let encoded = transaction.encode();
match Decode::decode(&mut &encoded[..]) {
Ok(uxt) => {
let best_block_id = self.best_block_id()?;
let best_block_id = BlockId::hash(self.client.info().chain.best_hash);
match self.pool.submit_one(&best_block_id, uxt) {
Ok(hash) => Some(hash),
Err(e) => match e.into_pool_error() {
Ok(txpool::error::Error::AlreadyImported(hash)) => {
hash.downcast::<ComponentExHash<C>>().ok()
hash.downcast::<H>().ok()
.map(|x| x.as_ref().clone())
},
Ok(e) => {
@@ -854,7 +869,7 @@ impl<C: Components> network::TransactionPool<ComponentExHash<C>, ComponentBlock<
}
}
fn on_broadcasted(&self, propagations: HashMap<ComponentExHash<C>, Vec<String>>) {
fn on_broadcasted(&self, propagations: HashMap<H, Vec<String>>) {
self.pool.on_broadcasted(propagations)
}
}
+2 -2
View File
@@ -83,8 +83,8 @@ construct_service_factory! {
client,
proposer,
service.network(),
service.config.custom.inherent_data_providers.clone(),
service.config.force_authoring,
service.config().custom.inherent_data_providers.clone(),
service.config().force_authoring,
)?;
service.spawn_task(Box::new(aura.select(service.on_exit()).then(|_| Ok(()))));
}
+2 -4
View File
@@ -26,10 +26,8 @@ use keyring::sr25519::Keyring;
use node_runtime::{Call, CheckedExtrinsic, UncheckedExtrinsic, SignedExtra, BalancesCall, ExistentialDeposit};
use primitives::{sr25519, crypto::Pair};
use sr_primitives::{generic::Era, traits::{Block as BlockT, Header as HeaderT, SignedExtension}};
use substrate_service::ServiceFactory;
use transaction_factory::RuntimeAdapter;
use transaction_factory::modes::Mode;
use crate::service;
use inherents::InherentData;
use timestamp;
use finality_tracker;
@@ -140,7 +138,7 @@ impl RuntimeAdapter for FactoryState<Number> {
let index = self.extract_index(&sender, prior_block_hash);
let phase = self.extract_phase(*prior_block_hash);
sign::<service::Factory, Self>(CheckedExtrinsic {
sign::<Self>(CheckedExtrinsic {
signed: Some((sender.clone(), Self::build_extra(index, phase))),
function: Call::Balances(
BalancesCall::transfer(
@@ -233,7 +231,7 @@ fn gen_seed_bytes(seed: u64) -> [u8; 32] {
/// Creates an `UncheckedExtrinsic` containing the appropriate signature for
/// a `CheckedExtrinsics`.
fn sign<F: ServiceFactory, RA: RuntimeAdapter>(
fn sign<RA: RuntimeAdapter>(
xt: CheckedExtrinsic,
key: &sr25519::Pair,
additional_signed: <SignedExtra as SignedExtension>::AdditionalSigned,
+1 -1
View File
@@ -186,7 +186,7 @@ pub fn run<I, T, E>(args: I, exit: E, version: cli::VersionInfo) -> error::Resul
match &ret {
Ok(Some(CustomSubcommands::Factory(cli_args))) => {
let mut config = cli::create_config_with_db_path::<service::Factory, _>(
let mut config = cli::create_config_with_db_path(
load_spec,
&cli_args.shared_params,
&version,
+9 -9
View File
@@ -99,11 +99,11 @@ construct_service_factory! {
FullComponents::<Factory>::new(config) },
AuthoritySetup = {
|mut service: Self::FullService| {
let (block_import, link_half, babe_link) = service.config.custom.import_setup.take()
let (block_import, link_half, babe_link) = service.config_mut().custom.import_setup.take()
.expect("Link Half and Block Import are present for Full Services or setup failed before. qed");
// spawn any futures that were created in the previous setup steps
if let Some(tasks) = service.config.custom.tasks_to_spawn.take() {
if let Some(tasks) = service.config_mut().custom.tasks_to_spawn.take() {
for task in tasks {
service.spawn_task(
task.select(service.on_exit())
@@ -129,8 +129,8 @@ construct_service_factory! {
block_import,
env: proposer,
sync_oracle: service.network(),
inherent_data_providers: service.config.custom.inherent_data_providers.clone(),
force_authoring: service.config.force_authoring,
inherent_data_providers: service.config().custom.inherent_data_providers.clone(),
force_authoring: service.config().force_authoring,
time_source: babe_link,
};
@@ -142,12 +142,12 @@ construct_service_factory! {
// FIXME #1578 make this available through chainspec
gossip_duration: Duration::from_millis(333),
justification_period: 4096,
name: Some(service.config.name.clone()),
name: Some(service.config().name.clone()),
keystore: Some(service.keystore()),
};
if !service.config.disable_grandpa {
if service.config.roles.is_authority() {
if !service.config().disable_grandpa {
if service.config().roles.is_authority() {
let telemetry_on_connect = TelemetryOnConnect {
telemetry_connection_sinks: service.telemetry_on_connect_stream(),
};
@@ -155,7 +155,7 @@ construct_service_factory! {
config: config,
link: link_half,
network: service.network(),
inherent_data_providers: service.config.custom.inherent_data_providers.clone(),
inherent_data_providers: service.config().custom.inherent_data_providers.clone(),
on_exit: service.on_exit(),
telemetry_on_connect: Some(telemetry_on_connect),
};
@@ -331,7 +331,7 @@ mod tests {
let block_factory = |service: &SyncService<<Factory as ServiceFactory>::FullService>| {
let service = service.get();
let mut inherent_data = service
.config
.config()
.custom
.inherent_data_providers
.create_inherent_data()