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

Commit b0efaa2

Browse files
cectonseunlanlegebkchr
authored
CLI: refactoring: remove Options from sc_service::Configuration's fields (#5271)
* WIP Forked at: 2afecf8 Parent branch: origin/master * Rename IntoConfiguration to CliConfiguration * Renamed into_configuration to create_configuration * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * Move keystore params to its own module * Use in-memory keystore even for build-spec * Enforce proper value for node name * dev_key_seed * Telemetry endpoints * rustfmt * Converted all RunCmd * rustfmt * Added export-blocks * Missed something * Removed config_path in NetworkConfiguration (not used) * Fixed warnings * public_addresses is used but never set, keeping it * Merge Configuration.node and NetworkConfiguration.node_name ...because they are the same thing * Added: import-blocks * Adding a proc_macro to help impl SubstrateCli * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * Re-export spec_factory from sc_cli * Re-added all the commands * Refactored node_key_params * Fixed previous refucktoring * Clean-up and removed full_version() * Renamed get_is_dev to not confuse with Configuration field * Fixed sc-cli-derive example * Fixing tests * Fixing tests and removing some (will re-add later) * Fixing more tests * Removes the need of type parameter * Converting bin/node and simplifying API * Converting more * Converting last command * WIP Forked at: 2afecf8 Parent branch: origin/master * Fixing tests and added default for WasmExecutionMethod * Fixing stuff * Fixed something I broke oops * Update Cargo.lock * Moving things around * Convert everything to Result * Added new macros to simplify the impl of CliConfiguration * Added a macro to generate CliConfiguration automatically for subcommands * Revert... too many macros (this one is not really useful) This reverts commit 9c516dd. * Renamed is_dev to get_is_dev Good enough for now * Fixed name roles (this is plural, not singular) * Clean-up * Re-export NodeKeyConfig and TelemetryEndpoints from sc_service * Improve styling/formatting * Added copyrights * Added doc and fixed warnings * Added myself to code owners * Yes it is needed according to the history * Revert formatting * Fixing conflict * Updated build.rs * Cargo.lock * Clean-up * Update client/cli-derive/Cargo.toml Co-Authored-By: Seun Lanlege <[email protected]> * Fail if using proc_macro and build.rs is not set properly * Dropped all get_ in front of methods * Clean-up * Fixing proc macro missing env var * Get the configuration inside the Runtime (needed for polkadot) * Clean-up * Get is_dev from argument like the others * Get chain ID instead of chain spec from shared params * &self is passed to spec_factory/load_spec * Wrong text * Fix example * Officialize macro and made a cool doc * Renamed spec_factory to load_spec (substrate_cli_configuration) * Removed not so useful ChainSpec * Renamed SubstrateCLI to SubstrateCli * Added changelog for impl_version being full now * Renamed Runtime to Runner * Update changelog to show example * Removed option on database cache size * WIP Forked at: 2afecf8 Parent branch: origin/master * Fix on removal of option * typo * Clean-up imports * Added info in Cargo.toml * typo * remarks * Moved function for build.rs to substrate-build-script-utils * Fixed example & test of cli-derive * Moved function for build.rs to substrate-build-script-utils * Renamed substrate_cli_configuration to substrate_cli oops It implements SubstrateCli not CliConfiguration! * Added documentation and wrapper macro * Removed option on database cache size * Removed option on database cache size * Clean-up * Reduce risk of errors due to typos * Removed option on database cache size * Added NOTE as suggested * Added doc as suggested * Fixed test * typo * renamed runtime to runner * Fixed weird argument * More commas * Moved client/cli-derive to client/cli/derive * Added 7 tests for the macros * Improve error message * Upgrade assert_cmd * Fixing missing stuff * Fixed unused import * Improve SubstrateCli doc * Applied suggestions * Fix and clean-up imports * Started replacing macros WIP * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * Started removing substrate_cli * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * WIP Forked at: 2afecf8 Parent branch: origin/master * fixed bug introduced while refactoring * Renamed NetworkConfigurationParams to NetworkParams for consistency sake * Fixed test * Update client/cli/src/commands/runcmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/runcmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/export_blocks_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/check_block_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update bin/node/cli/src/command.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update bin/node/cli/src/command.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/export_blocks_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Revert "Update client/cli/src/commands/export_blocks_cmd.rs" This reverts commit 5906776. * Revert "Update client/cli/src/commands/check_block_cmd.rs" This reverts commit f705f42. * Revert "Update client/cli/src/commands/export_blocks_cmd.rs" This reverts commit 8d57c05. * Revert "Update client/cli/src/commands/runcmd.rs" This reverts commit 93e74cf. * Revert "Update client/cli/src/commands/runcmd.rs" This reverts commit 11d527b. * Update client/cli/src/commands/export_blocks_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/import_blocks_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/purge_chain_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Changed ::sc_cli to $crate in the macro * fixed tests * fixed conflicts * Fixing test * Update client/cli/src/commands/purge_chain_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/params/pruning_params.rs Co-Authored-By: Bastian Köcher <[email protected]> * Remove comment as suggested * Apply suggestion * Update client/cli/src/commands/purge_chain_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/purge_chain_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/commands/purge_chain_cmd.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update utils/frame/benchmarking-cli/src/command.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/runner.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/runner.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/runner.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/params/pruning_params.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/params/node_key_params.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/params/network_params.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/lib.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update client/cli/src/config.rs Co-Authored-By: Bastian Köcher <[email protected]> * Added doc * Fixed error introduced after applying suggestion * Revert "Update client/cli/src/params/pruning_params.rs" This reverts commit 0574d06. * Print error * Apply suggestions from code review * Remove useless Results * Fixed CI failing on polkadot approval Co-authored-by: Seun Lanlege <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
1 parent 9f1bddf commit b0efaa2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+2894
-2134
lines changed

.maintain/gitlab/check_polkadot_companion_status.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ fi
8787
curl -H "${github_header}" -sS -o companion_pr_reviews.json \
8888
${github_api_polkadot_pull_url}/${pr_companion}/reviews
8989

90-
if [ "$(jq -r -e '.[].state' < companion_pr_reviews.json | uniq)" != "APPROVED" ]
90+
if [ -n "$(jq -r -e '.[].state | select(. == "CHANGES_REQUESTED")' < companion_pr_reviews.json)" ] && \
91+
[ -z "$(jq -r -e '.[].state | select(. == "APPROVED")' < companion_pr_reviews.json)" ]
9192
then
9293
boldprint "polkadot pr #${pr_companion} not APPROVED"
9394
exit 1

Cargo.lock

Lines changed: 11 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/node-template/node/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
name = "node-template"
33
version = "2.0.0-alpha.5"
44
authors = ["Anonymous"]
5+
description = "Substrate Node template"
56
edition = "2018"
67
license = "Unlicense"
78
build = "build.rs"
@@ -37,8 +38,7 @@ sc-basic-authorship = { path = "../../../client/basic-authorship", version = "0.
3738
node-template-runtime = { version = "2.0.0-alpha.5", path = "../runtime" }
3839

3940
[build-dependencies]
40-
vergen = "3.0.4"
41-
build-script-utils = { version = "2.0.0-alpha.5", package = "substrate-build-script-utils", path = "../../../utils/build-script-utils" }
41+
substrate-build-script-utils = { version = "2.0.0-alpha.5", path = "../../../utils/build-script-utils" }
4242

4343
[package.metadata.docs.rs]
4444
targets = ["x86_64-unknown-linux-gnu"]

bin/node-template/node/build.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
use vergen::{ConstantsFlags, generate_cargo_keys};
2-
3-
const ERROR_MSG: &str = "Failed to generate metadata files";
1+
use substrate_build_script_utils::{generate_cargo_keys, rerun_if_git_head_changed};
42

53
fn main() {
6-
generate_cargo_keys(ConstantsFlags::SHA_SHORT).expect(ERROR_MSG);
4+
generate_cargo_keys();
75

8-
build_script_utils::rerun_if_git_head_changed();
6+
rerun_if_git_head_changed();
97
}

bin/node-template/node/src/chain_spec.rs

Lines changed: 57 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,6 @@ use sp_runtime::traits::{Verify, IdentifyAccount};
1414
/// Specialized `ChainSpec`. This is a specialization of the general Substrate ChainSpec type.
1515
pub type ChainSpec = sc_service::GenericChainSpec<GenesisConfig>;
1616

17-
/// The chain specification option. This is expected to come in from the CLI and
18-
/// is little more than one of a number of alternatives which can easily be converted
19-
/// from a string (`--chain=...`) into a `ChainSpec`.
20-
#[derive(Clone, Debug)]
21-
pub enum Alternative {
22-
/// Whatever the current runtime is, with just Alice as an auth.
23-
Development,
24-
/// Whatever the current runtime is, with simple Alice/Bob auths.
25-
LocalTestnet,
26-
}
27-
2817
/// Helper function to generate a crypto pair from seed
2918
pub fn get_from_seed<TPublic: Public>(seed: &str) -> <TPublic::Pair as Pair>::Public {
3019
TPublic::Pair::from_string(&format!("//{}", seed), None)
@@ -42,80 +31,70 @@ pub fn get_account_id_from_seed<TPublic: Public>(seed: &str) -> AccountId where
4231
}
4332

4433
/// Helper function to generate an authority key for Aura
45-
pub fn get_authority_keys_from_seed(s: &str) -> (AuraId, GrandpaId) {
34+
pub fn authority_keys_from_seed(s: &str) -> (AuraId, GrandpaId) {
4635
(
4736
get_from_seed::<AuraId>(s),
4837
get_from_seed::<GrandpaId>(s),
4938
)
5039
}
5140

52-
impl Alternative {
53-
/// Get an actual chain config from one of the alternatives.
54-
pub(crate) fn load(self) -> Result<ChainSpec, String> {
55-
Ok(match self {
56-
Alternative::Development => ChainSpec::from_genesis(
57-
"Development",
58-
"dev",
59-
|| testnet_genesis(
60-
vec![
61-
get_authority_keys_from_seed("Alice"),
62-
],
63-
get_account_id_from_seed::<sr25519::Public>("Alice"),
64-
vec![
65-
get_account_id_from_seed::<sr25519::Public>("Alice"),
66-
get_account_id_from_seed::<sr25519::Public>("Bob"),
67-
get_account_id_from_seed::<sr25519::Public>("Alice//stash"),
68-
get_account_id_from_seed::<sr25519::Public>("Bob//stash"),
69-
],
70-
true,
71-
),
72-
vec![],
73-
None,
74-
None,
75-
None,
76-
None
77-
),
78-
Alternative::LocalTestnet => ChainSpec::from_genesis(
79-
"Local Testnet",
80-
"local_testnet",
81-
|| testnet_genesis(
82-
vec![
83-
get_authority_keys_from_seed("Alice"),
84-
get_authority_keys_from_seed("Bob"),
85-
],
86-
get_account_id_from_seed::<sr25519::Public>("Alice"),
87-
vec![
88-
get_account_id_from_seed::<sr25519::Public>("Alice"),
89-
get_account_id_from_seed::<sr25519::Public>("Bob"),
90-
get_account_id_from_seed::<sr25519::Public>("Charlie"),
91-
get_account_id_from_seed::<sr25519::Public>("Dave"),
92-
get_account_id_from_seed::<sr25519::Public>("Eve"),
93-
get_account_id_from_seed::<sr25519::Public>("Ferdie"),
94-
get_account_id_from_seed::<sr25519::Public>("Alice//stash"),
95-
get_account_id_from_seed::<sr25519::Public>("Bob//stash"),
96-
get_account_id_from_seed::<sr25519::Public>("Charlie//stash"),
97-
get_account_id_from_seed::<sr25519::Public>("Dave//stash"),
98-
get_account_id_from_seed::<sr25519::Public>("Eve//stash"),
99-
get_account_id_from_seed::<sr25519::Public>("Ferdie//stash"),
100-
],
101-
true,
102-
),
103-
vec![],
104-
None,
105-
None,
106-
None,
107-
None
108-
),
109-
})
110-
}
41+
pub fn development_config() -> ChainSpec {
42+
ChainSpec::from_genesis(
43+
"Development",
44+
"dev",
45+
|| testnet_genesis(
46+
vec![
47+
authority_keys_from_seed("Alice"),
48+
],
49+
get_account_id_from_seed::<sr25519::Public>("Alice"),
50+
vec![
51+
get_account_id_from_seed::<sr25519::Public>("Alice"),
52+
get_account_id_from_seed::<sr25519::Public>("Bob"),
53+
get_account_id_from_seed::<sr25519::Public>("Alice//stash"),
54+
get_account_id_from_seed::<sr25519::Public>("Bob//stash"),
55+
],
56+
true,
57+
),
58+
vec![],
59+
None,
60+
None,
61+
None,
62+
None,
63+
)
64+
}
11165

112-
pub(crate) fn from(s: &str) -> Option<Self> {
113-
match s {
114-
"dev" => Some(Alternative::Development),
115-
"" | "local" => Some(Alternative::LocalTestnet),
116-
_ => None,
117-
}
118-
}
66+
pub fn local_testnet_config() -> ChainSpec {
67+
ChainSpec::from_genesis(
68+
"Local Testnet",
69+
"local_testnet",
70+
|| testnet_genesis(
71+
vec![
72+
authority_keys_from_seed("Alice"),
73+
authority_keys_from_seed("Bob"),
74+
],
75+
get_account_id_from_seed::<sr25519::Public>("Alice"),
76+
vec![
77+
get_account_id_from_seed::<sr25519::Public>("Alice"),
78+
get_account_id_from_seed::<sr25519::Public>("Bob"),
79+
get_account_id_from_seed::<sr25519::Public>("Charlie"),
80+
get_account_id_from_seed::<sr25519::Public>("Dave"),
81+
get_account_id_from_seed::<sr25519::Public>("Eve"),
82+
get_account_id_from_seed::<sr25519::Public>("Ferdie"),
83+
get_account_id_from_seed::<sr25519::Public>("Alice//stash"),
84+
get_account_id_from_seed::<sr25519::Public>("Bob//stash"),
85+
get_account_id_from_seed::<sr25519::Public>("Charlie//stash"),
86+
get_account_id_from_seed::<sr25519::Public>("Dave//stash"),
87+
get_account_id_from_seed::<sr25519::Public>("Eve//stash"),
88+
get_account_id_from_seed::<sr25519::Public>("Ferdie//stash"),
89+
],
90+
true,
91+
),
92+
vec![],
93+
None,
94+
None,
95+
None,
96+
None,
97+
)
11998
}
12099

121100
fn testnet_genesis(initial_authorities: Vec<(AuraId, GrandpaId)>,
@@ -141,10 +120,3 @@ fn testnet_genesis(initial_authorities: Vec<(AuraId, GrandpaId)>,
141120
}),
142121
}
143122
}
144-
145-
pub fn load_spec(id: &str) -> Result<Box<dyn sc_service::ChainSpec>, String> {
146-
Ok(match Alternative::from(id) {
147-
Some(spec) => Box::new(spec.load()?),
148-
None => Box::new(ChainSpec::from_json_file(std::path::PathBuf::from(id))?),
149-
})
150-
}

bin/node-template/node/src/command.rs

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,64 @@
1414
// You should have received a copy of the GNU General Public License
1515
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.
1616

17-
use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
18-
use sc_cli::VersionInfo;
19-
use crate::service;
2017
use crate::chain_spec;
2118
use crate::cli::Cli;
19+
use crate::service;
20+
use sc_cli::SubstrateCli;
21+
use sp_consensus_aura::sr25519::AuthorityPair as AuraPair;
2222

23-
/// Parse and run command line arguments
24-
pub fn run(version: VersionInfo) -> sc_cli::Result<()> {
25-
let opt = sc_cli::from_args::<Cli>(&version);
23+
impl SubstrateCli for Cli {
24+
fn impl_name() -> &'static str {
25+
"Substrate Node"
26+
}
27+
28+
fn impl_version() -> &'static str {
29+
env!("SUBSTRATE_CLI_IMPL_VERSION")
30+
}
31+
32+
fn description() -> &'static str {
33+
env!("CARGO_PKG_DESCRIPTION")
34+
}
35+
36+
fn author() -> &'static str {
37+
env!("CARGO_PKG_AUTHORS")
38+
}
2639

27-
let mut config = sc_service::Configuration::from_version(&version);
40+
fn support_url() -> &'static str {
41+
"support.anonymous.an"
42+
}
43+
44+
fn copyright_start_year() -> i32 {
45+
2017
46+
}
47+
48+
fn executable_name() -> &'static str {
49+
env!("CARGO_PKG_NAME")
50+
}
51+
52+
fn load_spec(&self, id: &str) -> Result<Box<dyn sc_service::ChainSpec>, String> {
53+
Ok(match id {
54+
"dev" => Box::new(chain_spec::development_config()),
55+
"" | "local" => Box::new(chain_spec::local_testnet_config()),
56+
path => Box::new(chain_spec::ChainSpec::from_json_file(
57+
std::path::PathBuf::from(path),
58+
)?),
59+
})
60+
}
61+
}
62+
63+
/// Parse and run command line arguments
64+
pub fn run() -> sc_cli::Result<()> {
65+
let cli = Cli::from_args();
2866

29-
match opt.subcommand {
67+
match &cli.subcommand {
3068
Some(subcommand) => {
31-
subcommand.init(&version)?;
32-
subcommand.update_config(&mut config, chain_spec::load_spec, &version)?;
33-
subcommand.run(
34-
config,
35-
|config: _| Ok(new_full_start!(config).0),
36-
)
37-
},
69+
let runner = cli.create_runner(subcommand)?;
70+
runner.run_subcommand(subcommand, |config| Ok(new_full_start!(config).0))
71+
}
3872
None => {
39-
opt.run.init(&version)?;
40-
opt.run.update_config(&mut config, chain_spec::load_spec, &version)?;
41-
opt.run.run(
42-
config,
43-
service::new_light,
44-
service::new_full,
45-
&version,
46-
)
47-
},
73+
let runner = cli.create_runner(&cli.run)?;
74+
runner.run_node(service::new_light, service::new_full)
75+
}
4876
}
4977
}

0 commit comments

Comments
 (0)