Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge subspace-executor executable into subspace-node #366

Merged
merged 7 commits into from
Apr 17, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/subspace-node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ include = [
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
cirrus-node = { version = "0.1.0", path = "../../cumulus/node" }
Copy link
Member

Choose a reason for hiding this comment

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

Should we add it to runtime-benchmarks feature below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should, but runtime-benchmarks is actually unused at present. It also reminds me that we might also want to actually integrate try-runtime feature for convenient runtime upgrade testing.

clap = { version = "3.1.8", features = ["derive"] }
cumulus-client-cli = { version = "0.1.0", path = "../../cumulus/client/cli" }
frame-benchmarking-cli = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d" }
frame-support = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d" }
futures = "0.3.21"
Expand All @@ -31,6 +33,7 @@ sc-consensus = { version = "0.10.0-dev", git = "https://github.com/paritytech/su
sc-executor = { version = "0.10.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d", features = ["wasmtime"] }
sc-service = { version = "0.10.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d", features = ["wasmtime"] }
sc-telemetry = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d" }
sc-tracing = { version = "4.0.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d" }
serde = "1.0.136"
sp-consensus = { version = "0.10.0-dev", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d" }
sp-core = { version = "6.0.0", git = "https://github.com/paritytech/substrate", rev = "c4f3d028621edb293d2c423516221aa396f76a2d" }
Expand Down
50 changes: 47 additions & 3 deletions crates/subspace-node/src/bin/subspace-node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@

use frame_benchmarking_cli::BenchmarkCmd;
use futures::future::TryFutureExt;
use sc_cli::{ChainSpec, SubstrateCli};
use sc_service::PartialComponents;
use sc_cli::{ChainSpec, CliConfiguration, SubstrateCli};
use sc_service::{PartialComponents, Role};
use sp_core::crypto::Ss58AddressFormat;
use subspace_node::{Cli, ExecutorDispatch, Subcommand};
use subspace_node::{Cli, ExecutorDispatch, SecondaryChainCli, Subcommand};
use subspace_runtime::{Block, RuntimeApi};

/// Subspace node error.
Expand Down Expand Up @@ -73,6 +73,47 @@ fn set_default_ss58_version<C: AsRef<dyn ChainSpec>>(chain_spec: C) {
fn main() -> std::result::Result<(), Error> {
let cli = Cli::from_args();

if !cli.secondarychain_args.is_empty() {
let secondary_chain_cli =
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why is this all before match statement, shouldn't it be in None => { branch os match statement instead?

Also Subcommand::PurgeChain command wasn't updated the way it behaved in subspace-executor accordingly and probably some others that I don't recall immediately.

I was expecting that primary chain would start just like before and in its .map(|full| { we'd check for cli.secondarychain_args and run it before calling full.network_starter.start_network();. Right now it doesn't seem like networking is started at all.

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 don't understand why is this all before match statement, shouldn't it be in None => { branch os match statement instead?

That's reasonable, updated.

Also Subcommand::PurgeChain command wasn't updated the way it behaved in subspace-executor accordingly and probably some others that I don't recall immediately.

All executor-related sub-commands are TODO for now.

I was expecting that primary chain would start just like before and in its .map(|full| { we'd check for cli.secondarychain_args and run it before calling full.network_starter.start_network();. Right now it doesn't seem like networking is started at all.

When running an executor, the runner has to be created using SecondaryChainCli in which the primary node will be started instead of creating a runner using the primary node arguments.

Primary networking is started here:
https://github.com/subspace/subspace/blob/fba21ad1355e475a0ddf63fb747464cc04a524be/cumulus/node/src/service.rs#L308

Copy link
Member

Choose a reason for hiding this comment

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

I see. In future we'll probably want to have it the other way around and start networking outside of that.

SecondaryChainCli::new(cli.run.base.base_path()?, cli.secondarychain_args.iter());

let runner = SubstrateCli::create_runner(&secondary_chain_cli, &secondary_chain_cli)?;
set_default_ss58_version(&runner.config().chain_spec);
runner.run_node_until_exit(|config| async move {
let primary_chain_full_node = {
let span = sc_tracing::tracing::info_span!(
sc_tracing::logging::PREFIX_LOG_SPAN,
name = "Primarychain"
);
let _enter = span.enter();

let mut primary_config = cli
.create_configuration(&cli.run.base, config.tokio_handle.clone())
.map_err(|_| {
sc_service::Error::Other("Failed to create subspace configuration".into())
})?;

// The embedded primary full node must be an authority node for the new slots
// notification.
primary_config.role = Role::Authority;

subspace_service::new_full::<subspace_runtime::RuntimeApi, ExecutorDispatch>(
primary_config,
false,
)
.map_err(|_| {
sc_service::Error::Other("Failed to build a full subspace node".into())
})?
};

cirrus_node::service::start_parachain_node(config, primary_chain_full_node)
.await
.map(|r| r.0)
})?;

return Ok(());
}

match &cli.subcommand {
Some(Subcommand::Key(cmd)) => cmd.run(&cli)?,
Some(Subcommand::BuildSpec(cmd)) => {
Expand Down Expand Up @@ -222,6 +263,9 @@ fn main() -> std::result::Result<(), Error> {
}
})?;
}
Some(Subcommand::Executor(_cmd)) => {
unimplemented!("Executor subcommand");
}
None => {
let runner = cli.create_runner(&cli.run.base)?;
set_default_ss58_version(&runner.config().chain_spec);
Expand Down
20 changes: 20 additions & 0 deletions crates/subspace-node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

mod chain_spec;
mod import_blocks_from_dsn;
mod secondary_chain_cli;

pub use crate::import_blocks_from_dsn::ImportBlocksFromDsnCmd;
pub use crate::secondary_chain_cli::SecondaryChainCli;
use clap::Parser;
use sc_cli::SubstrateCli;
use sc_executor::{NativeExecutionDispatch, RuntimeVersion};
Expand Down Expand Up @@ -79,6 +81,10 @@ pub enum Subcommand {
/// Revert the chain to a previous state.
Revert(sc_cli::RevertCmd),

/// Run executor sub-commands.
#[clap(subcommand)]
Executor(cirrus_node::cli::Subcommand),

/// Sub-commands concerned with benchmarking.
#[clap(subcommand)]
Benchmark(frame_benchmarking_cli::BenchmarkCmd),
Expand All @@ -94,6 +100,11 @@ pub struct RunCmd {

/// Subspace Cli.
#[derive(Debug, Parser)]
#[clap(
propagate_version = true,
args_conflicts_with_subcommands = true,
subcommand_negates_reqs = true
)]
pub struct Cli {
/// Various utility commands.
#[clap(subcommand)]
Expand All @@ -102,6 +113,15 @@ pub struct Cli {
/// Run a node.
#[clap(flatten)]
pub run: RunCmd,

/// Secondarychain arguments
///
/// The command-line arguments provided first will be passed to the embedded primary node,
/// while the arguments provided after -- will be passed to the executor node.
///
/// subspace-node [primarychain-args] -- [secondarychain-args]
#[clap(raw = true)]
pub secondarychain_args: Vec<String>,
}

impl SubstrateCli for Cli {
Expand Down
189 changes: 189 additions & 0 deletions crates/subspace-node/src/secondary_chain_cli.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
use clap::Parser;
use sc_cli::{
ChainSpec, CliConfiguration, DefaultConfigurationValues, ImportParams, KeystoreParams,
NetworkParams, Result, RuntimeVersion, SharedParams, SubstrateCli,
};
use sc_service::{config::PrometheusConfig, BasePath};
use std::{net::SocketAddr, path::PathBuf};

#[derive(Debug)]
pub struct SecondaryChainCli {
/// The actual secondary chain cli object.
pub base: cumulus_client_cli::RunCmd,

/// The base path that should be used by the secondary chain.
pub base_path: Option<PathBuf>,
}

impl SecondaryChainCli {
/// Constructs a new instance of [`SecondaryChainCli`].
///
/// If no explicit base path for the secondary chain, the default value will be `primary_base_path/executor`.
pub fn new<'a>(
primary_base_path: Option<BasePath>,
relay_chain_args: impl Iterator<Item = &'a String>,
) -> Self {
let base_path = primary_base_path.map(|x| x.path().join("executor"));
Self {
base_path,
base: cumulus_client_cli::RunCmd::parse_from(relay_chain_args),
}
}
}

impl SubstrateCli for SecondaryChainCli {
fn impl_name() -> String {
"Subspace Executor".into()
}

fn impl_version() -> String {
env!("SUBSTRATE_CLI_IMPL_VERSION").into()
}

fn description() -> String {
"Subspace Executor".into()
}

fn author() -> String {
env!("CARGO_PKG_AUTHORS").into()
}

fn support_url() -> String {
"https://github.com/subspace/subspace/issues/new".into()
}

fn copyright_start_year() -> i32 {
2022
}

fn load_spec(&self, id: &str) -> std::result::Result<Box<dyn ChainSpec>, String> {
<cirrus_node::cli::Cli as SubstrateCli>::from_iter(
[SecondaryChainCli::executable_name()].iter(),
)
.load_spec(id)
}

fn native_runtime_version(chain_spec: &Box<dyn ChainSpec>) -> &'static RuntimeVersion {
cirrus_node::cli::Cli::native_runtime_version(chain_spec)
}
}

impl DefaultConfigurationValues for SecondaryChainCli {
fn p2p_listen_port() -> u16 {
30334
}

fn rpc_ws_listen_port() -> u16 {
9945
}

fn rpc_http_listen_port() -> u16 {
9934
}

fn prometheus_listen_port() -> u16 {
9616
}
}

impl CliConfiguration<Self> for SecondaryChainCli {
fn shared_params(&self) -> &SharedParams {
self.base.base.shared_params()
}

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

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

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

fn base_path(&self) -> Result<Option<BasePath>> {
Ok(self
.shared_params()
.base_path()
.or_else(|| self.base_path.clone().map(Into::into)))
}

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

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

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

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

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

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

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

fn state_cache_child_ratio(&self) -> Result<Option<usize>> {
self.base.base.state_cache_child_ratio()
}

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

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

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

fn default_heap_pages(&self) -> Result<Option<u64>> {
self.base.base.default_heap_pages()
}

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

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

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

fn announce_block(&self) -> Result<bool> {
self.base.base.announce_block()
}

fn telemetry_endpoints(
&self,
chain_spec: &Box<dyn ChainSpec>,
) -> Result<Option<sc_telemetry::TelemetryEndpoints>> {
self.base.base.telemetry_endpoints(chain_spec)
}
}
2 changes: 0 additions & 2 deletions cumulus/node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ cirrus-client-service = { path = "../client/cirrus-service" }
cirrus-primitives = { path = "../primitives" }

# Subspace dependencies
subspace-node = { path = "../../crates/subspace-node" }
subspace-runtime = { path = "../../crates/subspace-runtime" }
subspace-runtime-primitives = { path = "../../crates/subspace-runtime-primitives" }
subspace-service = { path = "../../crates/subspace-service" }
Expand All @@ -81,5 +80,4 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate"
[features]
runtime-benchmarks = [
"cirrus-runtime/runtime-benchmarks",
"subspace-node/runtime-benchmarks",
]
Loading