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

Make --collator reusable and imply --validator #380

Merged
164 changes: 164 additions & 0 deletions client/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@
#![warn(missing_docs)]

use sc_cli;
use sc_service::{
BasePath,
config::{TelemetryEndpoints, PrometheusConfig},
TransactionPoolOptions,
};
use std::{
fs,
io::{self, Write},
};
use structopt::StructOpt;
use std::net::SocketAddr;

/// The `purge-chain` command used to remove the whole chain: the parachain and the relaychain.
#[derive(Debug, StructOpt)]
Expand Down Expand Up @@ -112,3 +118,161 @@ impl sc_cli::CliConfiguration for PurgeChainCmd {
Some(&self.base.database_params)
}
}

/// The `run` command used to run a node.
#[derive(Debug, StructOpt)]
pub struct RunCmd {
/// The cumulus RunCmd inherents from sc_cli's
#[structopt(flatten)]
pub base: sc_cli::RunCmd,

/// Id of the parachain this collator collates for.
#[structopt(long)]
pub parachain_id: Option<u32>,

/// Run node as collator.
///
/// Note that this is the same as running with `--validator`.
#[structopt(long, conflicts_with = "validator")]
pub collator: bool,
}

/// A non-redundant version of the `RunCmd` that sets the `validator` field when the
/// original `RunCmd` had the `colaltor` field.
///Basically this is how we make `--collator` imply `--validator`.
pub struct NormalizedRunCmd {
/// The cumulus RunCmd inherents from sc_cli's
pub base: sc_cli::RunCmd,
/// Id of the parachain this collator collates for.
pub parachain_id: Option<u32>,
}


// In this approach I do not consume the original RunCmd. I'm stuck here because I don't know how to make
// a copy of the original sc_cli
impl RunCmd {
/// Transform this RunCmd into a NormalizedRunCmd which does not have a redundant `collator` field.
pub fn normalize(&self) -> NormalizedRunCmd {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Transform this RunCmd into a NormalizedRunCmd which does not have a redundant `collator` field.
pub fn normalize(&self) -> NormalizedRunCmd {
/// Transform this RunCmd into a NormalizedRunCmd which does not have a redundant `collator` field.
pub fn normalize(&self) -> NormalizedRunCmd {

Do you maybe have a better idea for the function name?

And the docs should also be updated

Copy link
Contributor Author

@JoshOrndorff JoshOrndorff May 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you not like normalize? I like it because it indicates that we aren't really changing any of the data, just writing it in a more standard form. standardize would also be fine. I like both of those better than the original transform.

I'm fine with any, of them, but I can't spend too long thinking about it because I have to paint a bike shed, and haven't decided what color it should be yet ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the docs as you requested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine

let mut new_base = self.base.clone();

new_base.validator = self.base.validator || self.collator;

NormalizedRunCmd {
base: new_base,
parachain_id: self.parachain_id,
}
}
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved
}

// In this approach I do consume the original RunCmd. I'm stuck here because I don't know how to create the runner
// error[E0505]: cannot move out of `cli.run` because it is borrowed
// --> rococo-parachains/src/command.rs:265:36
// |
// 265 | let runner = cli.create_runner(&cli.run.normalize())?;
// | --- ------------- ^^^^^^^ move out of `cli.run` occurs here
// | | |
// | | borrow later used by call
// | borrow of `cli` occurs here

// error[E0382]: use of partially moved value: `cli`
impl RunCmd {
/// Find better name ;P
fn normalize(mut self) -> NormalizedRunCmd {

self.base.validator = self.base.validator || self.collator;

NormalizedRunCmd {
base: self.base,
parachain_id: self.parachain_id,
}
}
}

impl sc_cli::CliConfiguration for NormalizedRunCmd {
fn shared_params(&self) -> &sc_cli::SharedParams {
self.base.shared_params()
}

fn import_params(&self) -> Option<&sc_cli::ImportParams> {
self.base.import_params()
}

fn network_params(&self) -> Option<&sc_cli::NetworkParams> {
self.base.network_params()
}

fn keystore_params(&self) -> Option<&sc_cli::KeystoreParams> {
self.base.keystore_params()
}

fn offchain_worker_params(&self) -> Option<&sc_cli::OffchainWorkerParams> {
self.base.offchain_worker_params()
}

fn node_name(&self) -> sc_cli::Result<String> {
self.base.node_name()
}

fn dev_key_seed(&self, is_dev: bool) -> sc_cli::Result<Option<String>> {
self.base.dev_key_seed(is_dev)
}

fn telemetry_endpoints(
&self,
chain_spec: &Box<dyn sc_cli::ChainSpec>,
) -> sc_cli::Result<Option<TelemetryEndpoints>> {
self.base.telemetry_endpoints(chain_spec)
}

fn role(&self, is_dev: bool) -> sc_cli::Result<sc_cli::Role> {
self.base.role(is_dev)
}

fn force_authoring(&self) -> sc_cli::Result<bool> {
self.base.force_authoring()
}

fn prometheus_config(&self, default_listen_port: u16) -> sc_cli::Result<Option<PrometheusConfig>> {
self.base.prometheus_config(default_listen_port)
}

fn disable_grandpa(&self) -> sc_cli::Result<bool> {
self.base.disable_grandpa()
}

fn rpc_ws_max_connections(&self) -> sc_cli::Result<Option<usize>> {
self.base.rpc_ws_max_connections()
}

fn rpc_cors(&self, is_dev: bool) -> sc_cli::Result<Option<Vec<String>>> {
self.base.rpc_cors(is_dev)
}

fn rpc_http(&self, default_listen_port: u16) -> sc_cli::Result<Option<SocketAddr>> {
self.base.rpc_http(default_listen_port)
}

fn rpc_ipc(&self) -> sc_cli::Result<Option<String>> {
self.base.rpc_ipc()
}

fn rpc_ws(&self, default_listen_port: u16) -> sc_cli::Result<Option<SocketAddr>> {
self.base.rpc_ws(default_listen_port)
}

fn rpc_methods(&self) -> sc_cli::Result<sc_service::config::RpcMethods> {
self.base.rpc_methods()
}

fn transaction_pool(&self) -> sc_cli::Result<TransactionPoolOptions> {
self.base.transaction_pool()
}

fn max_runtime_instances(&self) -> sc_cli::Result<Option<usize>> {
self.base.max_runtime_instances()
}

fn base_path(&self) -> sc_cli::Result<Option<BasePath>> {
self.base.base_path()
}
}
26 changes: 1 addition & 25 deletions rococo-parachains/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,24 +90,6 @@ pub struct ExportGenesisWasmCommand {
pub chain: Option<String>,
}

#[derive(Debug, StructOpt)]
pub struct RunCmd {
#[structopt(flatten)]
pub base: sc_cli::RunCmd,

/// Id of the parachain this collator collates for.
#[structopt(long)]
pub parachain_id: Option<u32>,
}

impl std::ops::Deref for RunCmd {
type Target = sc_cli::RunCmd;

fn deref(&self) -> &Self::Target {
&self.base
}
}
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Debug, StructOpt)]
#[structopt(settings = &[
structopt::clap::AppSettings::GlobalVersion,
Expand All @@ -119,13 +101,7 @@ pub struct Cli {
pub subcommand: Option<Subcommand>,

#[structopt(flatten)]
pub run: RunCmd,

/// Run node as collator.
///
/// Note that this is the same as running with `--validator`.
#[structopt(long, conflicts_with = "validator")]
pub collator: bool,
pub run: cumulus_client_cli::RunCmd,

/// Relaychain arguments
#[structopt(raw = true)]
Expand Down
9 changes: 4 additions & 5 deletions rococo-parachains/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ pub fn run() -> Result<()> {
Ok(())
}
None => {
let runner = cli.create_runner(&*cli.run)?;
let runner = cli.create_runner(&cli.run)?;
let use_shell = use_shell_runtime(&runner.config().chain_spec);

runner.run_node_until_exit(|config| async move {
Expand Down Expand Up @@ -295,20 +295,19 @@ pub fn run() -> Result<()> {
task_executor,
)
.map_err(|err| format!("Relay chain argument error: {}", err))?;
let collator = cli.run.base.validator || cli.collator;

info!("Parachain id: {:?}", id);
info!("Parachain Account: {}", parachain_account);
info!("Parachain genesis state: {}", genesis_state);
info!("Is collating: {}", if collator { "yes" } else { "no" });
info!("Is collating: {}", if config.role.is_authority() { "yes" } else { "no" });

if use_shell {
crate::service::start_shell_node(config, key, polkadot_config, id, collator)
crate::service::start_shell_node(config, key, polkadot_config, id)
.await
.map(|r| r.0)
.map_err(Into::into)
} else {
crate::service::start_node(config, key, polkadot_config, id, collator)
crate::service::start_node(config, key, polkadot_config, id)
.await
.map(|r| r.0)
.map_err(Into::into)
Expand Down
6 changes: 1 addition & 5 deletions rococo-parachains/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ async fn start_node_impl<RuntimeApi, Executor, RB>(
collator_key: CollatorPair,
polkadot_config: Configuration,
id: ParaId,
validator: bool,
rpc_ext_builder: RB,
) -> sc_service::error::Result<(TaskManager, Arc<TFullClient<Block, RuntimeApi, Executor>>)>
where
Expand Down Expand Up @@ -206,6 +205,7 @@ where
polkadot_full_node.backend.clone(),
);

let validator = parachain_config.role.is_authority();
let prometheus_registry = parachain_config.prometheus_registry().cloned();
let transaction_pool = params.transaction_pool.clone();
let mut task_manager = params.task_manager;
Expand Down Expand Up @@ -301,7 +301,6 @@ pub async fn start_node(
collator_key: CollatorPair,
polkadot_config: Configuration,
id: ParaId,
validator: bool,
) -> sc_service::error::Result<
(TaskManager, Arc<TFullClient<Block, parachain_runtime::RuntimeApi, RuntimeExecutor>>)
> {
Expand All @@ -310,7 +309,6 @@ pub async fn start_node(
collator_key,
polkadot_config,
id,
validator,
|_| Default::default(),
).await
}
Expand All @@ -321,7 +319,6 @@ pub async fn start_shell_node(
collator_key: CollatorPair,
polkadot_config: Configuration,
id: ParaId,
validator: bool,
) -> sc_service::error::Result<
(TaskManager, Arc<TFullClient<Block, shell_runtime::RuntimeApi, ShellRuntimeExecutor>>)
> {
Expand All @@ -330,7 +327,6 @@ pub async fn start_shell_node(
collator_key,
polkadot_config,
id,
validator,
|_| Default::default(),
).await
}