refacto: improve node cli

.github/workflows/madara-commands.yml

@@ -24,7 +24,7 @@ jobs:
           fail-on-cache-miss: true
       - name: Run setup
         run: |
-          target/release/madara setup
+          target/release/madara setup --chain local --from-remote
       - name: Create build-spec (plain)
         run: |
           target/release/madara build-spec --chain local > chain-plain.json

CHANGELOG.md

@@ -2,6 +2,24 @@
 
 ## Next release
 
+- feat(cli): `run` is the by default command when running the `madara` bin
+- refacto(cli): `run` and `setup` commands are defined in their own files
+- refacto(cli): `run.testnet` argument removed in favor of the substrate native
+  `chain` arg
+- feat(cli): `run.fetch_chain_spec` argument removed in favor of the substrate
+  native `chain` arg
+- feat(cli): `setup` require a source file, either from an url or a path on the
+  local filesystem
+- chore(cli): use `Url`, `Path` and `PathBuf` types rather than `String`
+- refacto(cli): moved the pallet/chain_spec/utils methods to the node crate
+- feat(cli): `madara_path` arg has been remove, we use the substrate native
+  `base_path` arg instead
+- feat(cli): sharingan chain specs are loaded during the compilation, not
+  downloaded from github
+- refacto(pallet/starknet): `GenesisLoader` refactored as `GenesisData` + a
+  `base_path` field
+- dev: fix rpc test failing
+
 ## v0.4.0
 
 - chore: release v0.4.0

Cargo.toml

@@ -206,3 +206,4 @@ indexmap = { git = "https://github.com/bluss/indexmap", rev = "ca5f848e10c31e80a
 num-traits = "0.2.17"
 num-bigint = "0.4.4"
 phf = { version = "0.11", default-features = false }
+url = "2.4.1"

README.md

@@ -146,7 +146,7 @@ bottlenecks of the system by running the following :
 
 ```bash
 ./target/release/madara setup
-flamegraph --root --open  -- ./target/release/madara run --dev
+flamegraph --root --open  -- ./target/release/madara --dev
 ```
 
 In parallel to that, run some transactions against your node (you can use

crates/node/Cargo.toml

@@ -94,7 +94,9 @@ mp-sequencer-address = { workspace = true, features = ["client"] }
 # CLI-specific dependencies
 try-runtime-cli = { optional = true, git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.43" }
 
+reqwest = { workspace = true }
 serde_json = { workspace = true }
+url = { workspace = true }
 
 [build-dependencies]
 substrate-build-script-utils = { workspace = true }
@@ -111,3 +113,6 @@ runtime-benchmarks = [
 # in the near future.
 try-runtime = ["madara-runtime/try-runtime", "try-runtime-cli/try-runtime"]
 disable-transaction-fee = ["madara-runtime/disable-transaction-fee"]
+# Load sharingan chain-specs during the compilation
+# This is the way to run a sharingan chain
+sharingan = []

crates/node/src/chain_spec.rs

@@ -2,16 +2,20 @@ use std::path::PathBuf;
 
 use madara_runtime::{AuraConfig, EnableManualSeal, GenesisConfig, GrandpaConfig, SystemConfig, WASM_BINARY};
 use mp_felt::Felt252Wrapper;
-use pallet_starknet::genesis_loader::{GenesisLoader, HexFelt};
-use pallet_starknet::utils;
-use sc_service::ChainType;
+use pallet_starknet::genesis_loader::{GenesisData, GenesisLoader, HexFelt};
+use sc_service::{BasePath, ChainType};
 use serde::{Deserialize, Serialize};
 use sp_consensus_aura::sr25519::AuthorityId as AuraId;
 use sp_consensus_grandpa::AuthorityId as GrandpaId;
 use sp_core::storage::Storage;
 use sp_core::{Pair, Public};
 use sp_state_machine::BasicExternalities;
 
+use crate::constants::DEV_CHAIN_ID;
+
+pub const GENESIS_ASSETS_DIR: &str = "genesis-assets/";
+pub const GENESIS_ASSETS_FILE: &str = "genesis.json";
+
 /// Specialized `ChainSpec`. This is a specialization of the general Substrate ChainSpec type.
 pub type ChainSpec = sc_service::GenericChainSpec<GenesisConfig>;
 
@@ -50,23 +54,25 @@ pub fn authority_keys_from_seed(s: &str) -> (AuraId, GrandpaId) {
     (get_from_seed::<AuraId>(s), get_from_seed::<GrandpaId>(s))
 }
 
-pub fn development_config(enable_manual_seal: Option<bool>, madara_path: PathBuf) -> Result<DevChainSpec, String> {
+pub fn development_config(enable_manual_seal: Option<bool>, base_path: BasePath) -> Result<DevChainSpec, String> {
     let wasm_binary = WASM_BINARY.ok_or_else(|| "Development wasm not available".to_string())?;
-    let genesis_loader = load_genesis(madara_path);
+    let chain_id = DEV_CHAIN_ID;
 
     Ok(DevChainSpec::from_genesis(
         // Name
         "Development",
         // ID
-        "dev",
+        chain_id,
         ChainType::Development,
         move || {
+            let genesis_loader = load_genesis(base_path.config_dir(chain_id));
+
             // Logging the development account
             print_development_accounts(&genesis_loader);
 
             DevGenesisExt {
                 genesis_config: testnet_genesis(
-                    genesis_loader.clone(),
+                    genesis_loader,
                     wasm_binary,
                     // Initial PoA authorities
                     vec![authority_keys_from_seed("Alice")],
@@ -93,10 +99,11 @@ pub fn development_config(enable_manual_seal: Option<bool>, madara_path: PathBuf
 // accounts with addresses 0x1 and 0x4 are NO VALIDATE accounts (don't require PK)
 // accounts with addresses 0x2 and 0x3 have the same PK
 pub fn print_development_accounts(genesis_loader: &GenesisLoader) {
-    let no_validate_account_address = genesis_loader.contracts[0].0.0;
-    let argent_account_address = genesis_loader.contracts[1].0.0;
-    let oz_account_address = genesis_loader.contracts[2].0.0;
-    let cairo_1_no_validate_account_address = genesis_loader.contracts[3].0.0;
+    // TODO: this is only true by luck. It's not enforced by anything
+    let no_validate_account_address = genesis_loader.data().contracts[0].0.0;
+    let argent_account_address = genesis_loader.data().contracts[1].0.0;
+    let oz_account_address = genesis_loader.data().contracts[2].0.0;
+    let cairo_1_no_validate_account_address = genesis_loader.data().contracts[3].0.0;
 
     let argent_pk: HexFelt =
         Felt252Wrapper::from_hex_be("0x00c1cf1490de1352865301bb8705143f3ef938f97fdf892f1090dcb5ac7bcd1d")
@@ -109,19 +116,20 @@ pub fn print_development_accounts(genesis_loader: &GenesisLoader) {
     log::info!("🧪 CAIRO 1 with address: {cairo_1_no_validate_account_address:#x} and no pk");
 }
 
-pub fn local_testnet_config(madara_path: PathBuf) -> Result<ChainSpec, String> {
+pub fn local_testnet_config(base_path: BasePath, chain_id: &str) -> Result<ChainSpec, String> {
     let wasm_binary = WASM_BINARY.ok_or_else(|| "Development wasm not available".to_string())?;
-    let genesis_loader = load_genesis(madara_path);
+
+    let owned_chain_id = chain_id.to_owned();
 
     Ok(ChainSpec::from_genesis(
         // Name
         "Local Testnet",
         // ID
-        "local_testnet",
+        chain_id,
         ChainType::Local,
         move || {
             testnet_genesis(
-                genesis_loader.clone(),
+                load_genesis(base_path.config_dir(&owned_chain_id)),
                 wasm_binary,
                 // Initial PoA authorities
                 // Intended to be only 2
@@ -143,14 +151,13 @@ pub fn local_testnet_config(madara_path: PathBuf) -> Result<ChainSpec, String> {
     ))
 }
 
-fn load_genesis(madara_path: PathBuf) -> GenesisLoader {
-    let madara_path = madara_path.to_str().unwrap().to_string();
-    let genesis_path = madara_path.clone() + "/configs/genesis-assets/genesis.json";
-    let genesis = utils::read_file_to_string(genesis_path)
+fn load_genesis(data_path: PathBuf) -> GenesisLoader {
+    let genesis_path = data_path.join(GENESIS_ASSETS_DIR).join(GENESIS_ASSETS_FILE);
+    let genesis_file_content = std::fs::read_to_string(genesis_path)
         .expect("Failed to read genesis file. Please run `madara setup` before opening an issue.");
-    let mut genesis_loader: GenesisLoader = serde_json::from_str(&genesis).expect("Failed loading genesis");
-    genesis_loader.set_madara_path(madara_path);
-    genesis_loader
+    let genesis_data: GenesisData = serde_json::from_str(&genesis_file_content).expect("Failed loading genesis");
+
+    GenesisLoader::new(data_path, genesis_data)
 }
 
 /// Configure initial storage state for FRAME modules.

crates/node/src/cli.rs

@@ -1,76 +1,12 @@
-use std::path::PathBuf;
-
-use mc_data_availability::DaLayer;
-use sc_cli::RunCmd;
-
-use crate::constants;
-
-/// Returns the `madara_path` default value ($HOME/.madara) as a string
-fn get_default_madara_path() -> String {
-    let home_path = std::env::var("HOME").unwrap_or(std::env::var("USERPROFILE").unwrap_or(".".into()));
-    format!("{}/.madara", home_path)
-}
-
-/// Available Sealing methods.
-#[derive(Debug, Copy, Clone, clap::ValueEnum, Default)]
-pub enum Sealing {
-    // Seal using rpc method.
-    #[default]
-    Manual,
-    // Seal when transaction is executed.
-    Instant,
-}
-
-/// Available testnets.
-#[derive(Debug, Copy, Clone, PartialEq, clap::ValueEnum)]
-pub enum Testnet {
-    Sharingan,
-}
+use crate::commands::{ExtendedRunCmd, SetupCmd};
 
 #[derive(Debug, clap::Parser)]
 pub struct Cli {
     #[command(subcommand)]
     pub subcommand: Option<Subcommand>,
 
-    /// 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<PathBuf>,
-
-    /// Choose sealing method.
-    #[clap(global = true, long, value_enum, ignore_case = true)]
-    pub sealing: Option<Sealing>,
-}
-
-#[derive(Clone, Debug, clap::Args)]
-pub struct ExtendedRunCmd {
     #[clap(flatten)]
-    pub run_cmd: RunCmd,
-
-    /// Choose a supported DA Layer
-    #[clap(long)]
-    pub da_layer: Option<DaLayer>,
-
-    /// Load a custom chain-spec from an url
-    /// If you want to load a chain spec that is present in your filesystem, use `--chain=<PATH>`
-    #[clap(long, conflicts_with = "testnet")]
-    pub fetch_chain_spec: Option<String>,
-
-    /// 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")]
-    pub testnet: Option<Testnet>,
-}
-
-#[derive(Debug, clap::Args)]
-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/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<String>,
+    pub run: ExtendedRunCmd,
 }
 
 #[allow(clippy::large_enum_variant)]
@@ -108,9 +44,6 @@ pub enum Subcommand {
     /// Revert the chain to a previous state.
     Revert(sc_cli::RevertCmd),
 
-    // Run madara node
-    Run(ExtendedRunCmd),
-
     // Setup madara node
     Setup(SetupCmd),