From 81d265034770ffbce42780dc35ce8370bb8036ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 7 Feb 2023 14:31:04 +0100 Subject: [PATCH] Disable `hostperfcheck` by default (#6640) This feature should only be activated by the polkadot binary. Otherwise parachains may accidentally activate this feature. --- Cargo.toml | 4 ++-- cli/Cargo.toml | 2 +- cli/src/command.rs | 28 ++++++++++++---------- cli/src/error.rs | 4 ++++ node/service/src/lib.rs | 34 ++++++++++++++++++++------- node/test/performance-test/src/lib.rs | 3 --- 6 files changed, 49 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8c8f1435abad..6ce31d1e938b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ repository = "https://github.com/paritytech/polkadot.git" version = "0.9.37" [dependencies] -polkadot-cli = { path = "cli", features = [ "kusama-native", "westend-native", "rococo-native" ] } +polkadot-cli = { path = "cli", features = [ "kusama-native", "westend-native", "rococo-native", "hostperfcheck" ] } color-eyre = { version = "0.6.1", default-features = false } tikv-jemallocator = "0.5.0" @@ -30,7 +30,7 @@ tempfile = "3.2.0" tokio = "1.24.2" substrate-rpc-client = { git = "https://github.com/paritytech/substrate", branch = "master" } polkadot-core-primitives = { path = "core-primitives" } - + [workspace] members = [ "cli", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index feced4c148a1..01247bbc996f 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -42,7 +42,7 @@ sc-storage-monitor = { git = "https://github.com/paritytech/substrate", branch = substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } [features] -default = ["db", "cli", "hostperfcheck", "full-node", "polkadot-native"] +default = ["db", "cli", "full-node", "polkadot-native"] db = ["service/db"] cli = [ "clap", diff --git a/cli/src/command.rs b/cli/src/command.rs index b110054957eb..05e330aa4d27 100644 --- a/cli/src/command.rs +++ b/cli/src/command.rs @@ -28,6 +28,7 @@ use sp_keyring::Sr25519Keyring; use std::net::ToSocketAddrs; pub use crate::{error::Error, service::BlockId}; +#[cfg(feature = "hostperfcheck")] pub use polkadot_performance_test::PerfCheckError; impl From for Error { @@ -238,7 +239,11 @@ macro_rules! unwrap_client { #[cfg(feature = "rococo-native")] polkadot_client::Client::Rococo($client) => $code, #[allow(unreachable_patterns)] - _ => Err(Error::CommandNotImplemented), + _ => { + let _ = $client; + + Err(Error::CommandNotImplemented) + }, } }; } @@ -246,21 +251,20 @@ macro_rules! unwrap_client { /// Runs performance checks. /// Should only be used in release build since the check would take too much time otherwise. fn host_perf_check() -> Result<()> { - #[cfg(not(build_type = "release"))] + #[cfg(not(feature = "hostperfcheck"))] + { + return Err(Error::FeatureNotEnabled { feature: "hostperfcheck" }.into()) + } + + #[cfg(all(not(build_type = "release"), feature = "hostperfcheck"))] { return Err(PerfCheckError::WrongBuildType.into()) } - #[cfg(build_type = "release")] + + #[cfg(all(feature = "hostperfcheck", build_type = "release"))] { - #[cfg(not(feature = "hostperfcheck"))] - { - return Err(PerfCheckError::FeatureNotEnabled { feature: "hostperfcheck" }.into()) - } - #[cfg(feature = "hostperfcheck")] - { - crate::host_perf_check::host_perf_check()?; - return Ok(()) - } + crate::host_perf_check::host_perf_check()?; + return Ok(()) } } diff --git a/cli/src/error.rs b/cli/src/error.rs index 37ecb53d8aca..e125998c476f 100644 --- a/cli/src/error.rs +++ b/cli/src/error.rs @@ -29,6 +29,7 @@ pub enum Error { SubstrateTracing(#[from] sc_tracing::logging::Error), #[error(transparent)] + #[cfg(feature = "hostperfcheck")] PerfCheck(#[from] polkadot_performance_test::PerfCheckError), #[cfg(not(feature = "pyroscope"))] @@ -53,4 +54,7 @@ pub enum Error { #[error("Other: {0}")] Other(String), + + #[error("This subcommand is only available when compiled with `{feature}`")] + FeatureNotEnabled { feature: &'static str }, } diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 5760cb6542a0..344f31390fc1 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -1355,32 +1355,36 @@ pub fn new_chain_ops( > { config.keystore = service::config::KeystoreConfig::InMemory; - let telemetry_worker_handle = None; - #[cfg(feature = "rococo-native")] if config.chain_spec.is_rococo() || config.chain_spec.is_wococo() || config.chain_spec.is_versi() { - return chain_ops!(config, jaeger_agent, telemetry_worker_handle; rococo_runtime, RococoExecutorDispatch, Rococo) + return chain_ops!(config, jaeger_agent, None; rococo_runtime, RococoExecutorDispatch, Rococo) } #[cfg(feature = "kusama-native")] if config.chain_spec.is_kusama() { - return chain_ops!(config, jaeger_agent, telemetry_worker_handle; kusama_runtime, KusamaExecutorDispatch, Kusama) + return chain_ops!(config, jaeger_agent, None; kusama_runtime, KusamaExecutorDispatch, Kusama) } #[cfg(feature = "westend-native")] if config.chain_spec.is_westend() { - return chain_ops!(config, jaeger_agent, telemetry_worker_handle; westend_runtime, WestendExecutorDispatch, Westend) + return chain_ops!(config, jaeger_agent, None; westend_runtime, WestendExecutorDispatch, Westend) } #[cfg(feature = "polkadot-native")] { - return chain_ops!(config, jaeger_agent, telemetry_worker_handle; polkadot_runtime, PolkadotExecutorDispatch, Polkadot) + return chain_ops!(config, jaeger_agent, None; polkadot_runtime, PolkadotExecutorDispatch, Polkadot) } + #[cfg(not(feature = "polkadot-native"))] - Err(Error::NoRuntime) + { + let _ = config; + let _ = jaeger_agent; + + Err(Error::NoRuntime) + } } /// Build a full node. @@ -1488,7 +1492,21 @@ pub fn build_full( } #[cfg(not(feature = "polkadot-native"))] - Err(Error::NoRuntime) + { + let _ = config; + let _ = is_collator; + let _ = grandpa_pause; + let _ = enable_beefy; + let _ = jaeger_agent; + let _ = telemetry_worker_handle; + let _ = overseer_enable_anyways; + let _ = overseer_gen; + let _ = overseer_message_channel_override; + let _ = malus_finality_delay; + let _ = hwbench; + + Err(Error::NoRuntime) + } } /// Reverts the node state down to at most the last finalized block. diff --git a/node/test/performance-test/src/lib.rs b/node/test/performance-test/src/lib.rs index 762c530e5d20..e80b5e7589f2 100644 --- a/node/test/performance-test/src/lib.rs +++ b/node/test/performance-test/src/lib.rs @@ -36,9 +36,6 @@ pub enum PerfCheckError { #[error("This subcommand is only available in release mode")] WrongBuildType, - #[error("This subcommand is only available when compiled with `{feature}`")] - FeatureNotEnabled { feature: &'static str }, - #[error("No wasm code found for running the performance test")] WasmBinaryMissing,