From 042e4c38fdc820c152031646e8f7d879e6b84cbc Mon Sep 17 00:00:00 2001 From: Oak <5263301+d-roak@users.noreply.github.com> Date: Sat, 30 Sep 2023 20:08:19 +0900 Subject: [PATCH] fix: use official repo to dowload configs; fix clap (#1159) --- CHANGELOG.md | 3 + crates/node/src/cli.rs | 24 +++----- crates/node/src/command.rs | 115 ++++++++++++++++++----------------- crates/node/src/constants.rs | 2 +- 4 files changed, 71 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 994c4f0fd9..7093332952 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Next release +- fix: fix clap for run command +- fix: add `madara_path` flag for setup command +- fix: add official references to configs files - refactor: exported chain id constant in mp-chain-id crate and added one for SN_MAIN - fix: cargo update and `main` branch prettier fix diff --git a/crates/node/src/cli.rs b/crates/node/src/cli.rs index cd31058cc4..732c818768 100644 --- a/crates/node/src/cli.rs +++ b/crates/node/src/cli.rs @@ -32,11 +32,15 @@ pub struct Cli { #[command(subcommand)] pub subcommand: Option, - #[clap(flatten)] - pub run: ExtendedRunCmd, + /// Path to the folder where all configuration files and data are stored + /// base_path will always be overwritten by madara_path + /// in the case you use the --tmp, the base_path will be changed during the runtime + #[clap(global = true, long, default_value = get_default_madara_path())] + pub madara_path: Option, - #[clap(flatten)] - pub setup: SetupCmd, + /// Choose sealing method. + #[clap(global = true, long, value_enum, ignore_case = true)] + pub sealing: Option, } #[derive(Clone, Debug, clap::Args)] @@ -53,16 +57,6 @@ pub struct ExtendedRunCmd { #[clap(long, conflicts_with = "testnet")] pub fetch_chain_spec: Option, - /// Path to the folder where all configuration files and data are stored - /// base_path will always be overwritten by madara_path - /// in the case you use the --tmp, the base_path will be changed during the runtime - #[clap(long, default_value = get_default_madara_path())] - pub madara_path: Option, - - /// Choose sealing method. - #[arg(long, value_enum, ignore_case = true)] - pub sealing: Option, - /// Choose a supported testnet chain which will load some default values /// The testnets will allways be fetched when this flag is passed to search for updates #[clap(long, conflicts_with = "fetch_chain_spec", conflicts_with = "chain")] @@ -73,7 +67,7 @@ pub struct ExtendedRunCmd { pub struct SetupCmd { /// Load a index.json file for downloading assets /// The index.json must follow the format of the official index.json - /// (https://github.com/d-roak/madara/blob/feat/configs-index/configs/index.json) + /// (https://github.com/keep-starknet-strange/madara/blob/main/configs/index.json) /// Where the `md5` and `url` fields are optional #[clap(long, default_value = constants::DEFAULT_CONFIGS_URL)] pub fetch_madara_configs: Option, diff --git a/crates/node/src/command.rs b/crates/node/src/command.rs index dcc38ddc72..5bf981e9cc 100644 --- a/crates/node/src/command.rs +++ b/crates/node/src/command.rs @@ -7,7 +7,7 @@ use pallet_starknet::utils; use sc_cli::{ChainSpec, RpcMethods, RuntimeVersion, SubstrateCli}; use crate::benchmarking::{inherent_benchmark_data, RemarkBuilder}; -use crate::cli::{Cli, Subcommand, Testnet}; +use crate::cli::{Cli, ExtendedRunCmd, SetupCmd, Subcommand, Testnet}; use crate::{chain_spec, configs, constants, service}; impl SubstrateCli for Cli { fn impl_name() -> String { @@ -37,14 +37,14 @@ impl SubstrateCli for Cli { fn load_spec(&self, id: &str) -> Result, String> { Ok(match id { "dev" => { - let enable_manual_seal = self.run.sealing.map(|_| true); + let enable_manual_seal = self.sealing.map(|_| true); Box::new(chain_spec::development_config( enable_manual_seal, - self.run.madara_path.clone().expect("`madara_path` expected to be set with clap default value"), + self.madara_path.clone().expect("`madara_path` expected to be set with clap default value"), )?) } "" | "local" | "madara-local" => Box::new(chain_spec::local_testnet_config( - self.run.madara_path.clone().expect("`madara_path` expected to be set with clap default value"), + self.madara_path.clone().expect("`madara_path` expected to be set with clap default value"), )?), path => Box::new(chain_spec::ChainSpec::from_json_file(std::path::PathBuf::from(path))?), }) @@ -55,9 +55,8 @@ impl SubstrateCli for Cli { } } -fn get_madara_path_string(cli: &Cli) -> String { - cli.run - .madara_path +fn get_madara_path_string(madara_path: &Option) -> String { + madara_path .clone() .expect("`madara_path` expected to be set with clap default value") .into_os_string() @@ -65,64 +64,63 @@ fn get_madara_path_string(cli: &Cli) -> String { .expect("Failed to convert `madara_path` to string") } -fn set_dev_environment(cli: &mut Cli) { +fn set_dev_environment(cmd: &mut ExtendedRunCmd) { // create a reproducible dev environment - cli.run.run_cmd.shared_params.dev = false; - cli.run.run_cmd.shared_params.chain = Some("dev".to_string()); + cmd.run_cmd.shared_params.dev = false; + cmd.run_cmd.shared_params.chain = Some("dev".to_string()); - cli.run.run_cmd.force_authoring = true; - cli.run.run_cmd.alice = true; + cmd.run_cmd.force_authoring = true; + cmd.run_cmd.alice = true; // we can't set `--rpc-cors=all`, so it needs to be set manually if we want to connect with external // hosts - cli.run.run_cmd.rpc_external = true; - cli.run.run_cmd.rpc_methods = RpcMethods::Unsafe; + cmd.run_cmd.rpc_external = true; + cmd.run_cmd.rpc_methods = RpcMethods::Unsafe; } -fn try_set_testnet(cli: &mut Cli) -> Result<(), String> { +fn try_set_testnet(madara_path: &Option, cmd: &mut ExtendedRunCmd) -> Result<(), String> { // checks if it should retrieve and enable a specific chain-spec - let madara_path = get_madara_path_string(cli); + let madara_path = get_madara_path_string(madara_path); let local_path = utils::get_project_path(); - if cli.run.testnet == Some(Testnet::Sharingan) { + if cmd.testnet == Some(Testnet::Sharingan) { if let Ok(ref src_path) = local_path { let src_path = src_path.clone() + "/configs/chain-specs/testnet-sharingan-raw.json"; utils::copy_from_filesystem(src_path, madara_path.clone() + "/chain-specs")?; - cli.run.run_cmd.shared_params.chain = Some(madara_path + "/chain-specs/testnet-sharingan-raw.json"); + cmd.run_cmd.shared_params.chain = Some(madara_path + "/chain-specs/testnet-sharingan-raw.json"); } else { utils::fetch_from_url( constants::SHARINGAN_CHAIN_SPEC_URL.to_string(), madara_path.clone() + "/configs/chain-specs/", )?; - cli.run.run_cmd.shared_params.chain = Some(madara_path + "/chain-specs/testnet-sharingan-raw.json"); + cmd.run_cmd.shared_params.chain = Some(madara_path + "/chain-specs/testnet-sharingan-raw.json"); } } - if cli.run.run_cmd.shared_params.chain.is_some() { - cli.run.run_cmd.rpc_external = true; - cli.run.run_cmd.rpc_methods = RpcMethods::Unsafe; + if cmd.run_cmd.shared_params.chain.is_some() { + cmd.run_cmd.rpc_external = true; + cmd.run_cmd.rpc_methods = RpcMethods::Unsafe; } Ok(()) } -fn set_chain_spec(cli: &mut Cli) -> Result<(), String> { - let madara_path = get_madara_path_string(cli); - let chain_spec_url = cli - .run +fn set_chain_spec(madara_path: &Option, cmd: &mut ExtendedRunCmd) -> Result<(), String> { + let madara_path = get_madara_path_string(madara_path); + let chain_spec_url = cmd .fetch_chain_spec .clone() .expect("`chain_spec_url` expected to be set because the function is called upon verification"); utils::fetch_from_url(chain_spec_url.clone(), madara_path.clone() + "/chain-specs")?; let chain_spec = chain_spec_url.split('/').last().expect("Failed to get chain spec file name from `chain_spec_url`"); - cli.run.run_cmd.shared_params.chain = Some(madara_path + "/chain-specs/" + chain_spec); + cmd.run_cmd.shared_params.chain = Some(madara_path + "/chain-specs/" + chain_spec); Ok(()) } -fn fetch_madara_configs(cli: &Cli) -> Result<(), String> { - let madara_path = get_madara_path_string(cli); +fn fetch_madara_configs(madara_path: &Option, cmd: &SetupCmd) -> Result<(), String> { + let madara_path = get_madara_path_string(madara_path); let local_path = utils::get_project_path(); if let Ok(ref src_path) = local_path { @@ -136,7 +134,7 @@ fn fetch_madara_configs(cli: &Cli) -> Result<(), String> { let src_path = src_path.clone() + "/configs/genesis-assets/" + &asset.name; utils::copy_from_filesystem(src_path, madara_path.clone() + "/configs/genesis-assets")?; } - } else if let Some(configs_url) = &cli.setup.fetch_madara_configs { + } else if let Some(configs_url) = &cmd.fetch_madara_configs { utils::fetch_from_url(configs_url.to_string(), madara_path.clone() + "/configs")?; let madara_configs: configs::Configs = @@ -159,47 +157,45 @@ fn fetch_madara_configs(cli: &Cli) -> Result<(), String> { pub fn run() -> sc_cli::Result<()> { let mut cli = Cli::from_args(); - cli.run.run_cmd.shared_params.base_path = cli.run.madara_path.clone(); - - match &cli.subcommand { - Some(Subcommand::Key(cmd)) => cmd.run(&cli), - Some(Subcommand::BuildSpec(cmd)) => { + match cli.subcommand { + Some(Subcommand::Key(ref cmd)) => cmd.run(&cli), + Some(Subcommand::BuildSpec(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.sync_run(|config| cmd.run(config.chain_spec, config.network)) } - Some(Subcommand::CheckBlock(cmd)) => { + Some(Subcommand::CheckBlock(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { let (client, _, import_queue, task_manager, _) = service::new_chain_ops(&mut config)?; Ok((cmd.run(client, import_queue), task_manager)) }) } - Some(Subcommand::ExportBlocks(cmd)) => { + Some(Subcommand::ExportBlocks(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { let (client, _, _, task_manager, _) = service::new_chain_ops(&mut config)?; Ok((cmd.run(client, config.database), task_manager)) }) } - Some(Subcommand::ExportState(cmd)) => { + Some(Subcommand::ExportState(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { let (client, _, _, task_manager, _) = service::new_chain_ops(&mut config)?; Ok((cmd.run(client, config.chain_spec), task_manager)) }) } - Some(Subcommand::ImportBlocks(cmd)) => { + Some(Subcommand::ImportBlocks(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { let (client, _, import_queue, task_manager, _) = service::new_chain_ops(&mut config)?; Ok((cmd.run(client, import_queue), task_manager)) }) } - Some(Subcommand::PurgeChain(cmd)) => { + Some(Subcommand::PurgeChain(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.sync_run(|config| cmd.run(config.database)) } - Some(Subcommand::Revert(cmd)) => { + Some(Subcommand::Revert(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { let (client, backend, _, task_manager, _) = service::new_chain_ops(&mut config)?; @@ -210,7 +206,7 @@ pub fn run() -> sc_cli::Result<()> { Ok((cmd.run(client, backend, Some(aux_revert)), task_manager)) }) } - Some(Subcommand::Benchmark(cmd)) => { + Some(Subcommand::Benchmark(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.sync_run(|mut config| { @@ -275,32 +271,33 @@ pub fn run() -> sc_cli::Result<()> { Some(Subcommand::TryRuntime) => Err("TryRuntime wasn't enabled when building the node. You can enable it \ with `--features try-runtime`." .into()), - Some(Subcommand::ChainInfo(cmd)) => { + Some(Subcommand::ChainInfo(ref cmd)) => { let runner = cli.create_runner(cmd)?; runner.sync_run(|config| cmd.run::(&config)) } - Some(Subcommand::Run(cmd)) => { - let madara_path = get_madara_path_string(&cli); + Some(Subcommand::Run(ref mut cmd)) => { + cmd.run_cmd.shared_params.base_path = cli.madara_path.clone(); + let madara_path = get_madara_path_string(&cli.madara_path); // Set the node_key_file for substrate in the case that it was not manually setted if cmd.run_cmd.network_params.node_key_params.node_key_file.is_none() { - cli.run.run_cmd.network_params.node_key_params.node_key_file = + cmd.run_cmd.network_params.node_key_params.node_key_file = Some((madara_path.clone() + "/p2p-key.ed25519").into()); } if cmd.run_cmd.shared_params.dev { - set_dev_environment(&mut cli); + set_dev_environment(cmd); } - if cli.run.fetch_chain_spec.is_some() { - set_chain_spec(&mut cli)?; + if cmd.fetch_chain_spec.is_some() { + set_chain_spec(&cli.madara_path, cmd)?; } - if cli.run.testnet.is_some() { - try_set_testnet(&mut cli)?; + if cmd.testnet.is_some() { + try_set_testnet(&cli.madara_path, cmd)?; } - let da_config: Option<(DaLayer, PathBuf)> = match cli.run.da_layer { + let da_config: Option<(DaLayer, PathBuf)> = match cmd.da_layer { Some(da_layer) => { let da_path = std::path::PathBuf::from(madara_path.clone() + "/da-config.json"); if !da_path.exists() { @@ -311,18 +308,22 @@ pub fn run() -> sc_cli::Result<()> { Some((da_layer, da_path)) } None => { - log::info!("madara initialized w/o da layer"); + log::info!("Madara initialized w/o DA layer"); None } }; - let runner = cli.create_runner(&cli.run.run_cmd)?; + // pre assign variables because of cmd mutable borrow + let run_cmd: sc_cli::RunCmd = cmd.run_cmd.clone(); + let sealing = cli.sealing; + + let runner = cli.create_runner(&run_cmd)?; runner.run_node_until_exit(|config| async move { - service::new_full(config, cli.run.sealing, da_config).map_err(sc_cli::Error::Service) + service::new_full(config, sealing, da_config).map_err(sc_cli::Error::Service) }) } - Some(Subcommand::Setup(_)) => { - fetch_madara_configs(&cli)?; + Some(Subcommand::Setup(cmd)) => { + fetch_madara_configs(&cli.madara_path, &cmd)?; Ok(()) } _ => Err("You need to specify some subcommand. E.g. `madara run`".into()), diff --git a/crates/node/src/constants.rs b/crates/node/src/constants.rs index d1536721c6..96b7255325 100644 --- a/crates/node/src/constants.rs +++ b/crates/node/src/constants.rs @@ -1,4 +1,4 @@ pub const DEFAULT_CONFIGS_URL: &str = - "https://raw.githubusercontent.com/d-roak/madara/feat/configs-index/configs/index.json"; + "https://raw.githubusercontent.com/keep-starknet-strange/madara/main/configs/index.json"; pub const SHARINGAN_CHAIN_SPEC_URL: &str = "https://raw.githubusercontent.com/keep-starknet-strange/madara/main/configs/chain-specs/testnet-sharingan-raw.json";