diff --git a/cumulus/polkadot-omni-node/lib/src/command.rs b/cumulus/polkadot-omni-node/lib/src/command.rs index 78ac50834083c..fd8925529b65a 100644 --- a/cumulus/polkadot-omni-node/lib/src/command.rs +++ b/cumulus/polkadot-omni-node/lib/src/command.rs @@ -227,38 +227,61 @@ where }) }, Some(Subcommand::Benchmark(cmd)) => { - let runner = cli.create_runner(cmd)?; - // Switch on the concrete benchmark sub-command- match cmd { #[cfg(feature = "runtime-benchmarks")] - BenchmarkCmd::Pallet(cmd) => runner.sync_run(|config| { - cmd.run_with_spec::>, ReclaimHostFunctions>(Some( - config.chain_spec, - )) - }), - BenchmarkCmd::Block(cmd) => runner.sync_run(|config| { - let node = new_node_spec( - &config, - &cmd_config.runtime_resolver, - &cli.node_extra_args(), - )?; - node.run_benchmark_block_cmd(config, cmd) - }), + BenchmarkCmd::Pallet(cmd) => { + let chain = cmd + .shared_params + .chain + .as_ref() + .map(|chain| cli.load_spec(&chain)) + .transpose()?; + cmd.run_with_spec::>, ReclaimHostFunctions>(chain) + }, + BenchmarkCmd::Block(cmd) => { + // The command needs the full node configuration because it uses the node + // client and the database source, which in its turn has a dependency on the + // chain spec, given via the `--chain` flag. + let runner = cli.create_runner(cmd)?; + runner.sync_run(|config| { + let node = new_node_spec( + &config, + &cmd_config.runtime_resolver, + &cli.node_extra_args(), + )?; + node.run_benchmark_block_cmd(config, cmd) + }) + }, #[cfg(feature = "runtime-benchmarks")] - BenchmarkCmd::Storage(cmd) => runner.sync_run(|config| { - let node = new_node_spec( - &config, - &cmd_config.runtime_resolver, - &cli.node_extra_args(), - )?; - node.run_benchmark_storage_cmd(config, cmd) - }), - BenchmarkCmd::Machine(cmd) => - runner.sync_run(|config| cmd.run(&config, SUBSTRATE_REFERENCE_HARDWARE.clone())), + BenchmarkCmd::Storage(cmd) => { + // The command needs the full node configuration because it uses the node + // client and the database API, storage and shared_trie_cache. It requires + // the `--chain` flag to be passed. + let runner = cli.create_runner(cmd)?; + runner.sync_run(|config| { + let node = new_node_spec( + &config, + &cmd_config.runtime_resolver, + &cli.node_extra_args(), + )?; + node.run_benchmark_storage_cmd(config, cmd) + }) + }, + BenchmarkCmd::Machine(cmd) => { + // The command needs the full node configuration, and implicitly a chain + // spec to be passed, even if it doesn't use it directly. The `--chain` flag is + // relevant in determining the database path, which is used for the disk + // benchmark. + // + // TODO: change `machine` subcommand to take instead a disk path we want to + // benchmark?. + let runner = cli.create_runner(cmd)?; + runner.sync_run(|config| cmd.run(&config, SUBSTRATE_REFERENCE_HARDWARE.clone())) + }, #[allow(unreachable_patterns)] _ => Err("Benchmarking sub-command unsupported or compilation feature missing. \ - Make sure to compile with --features=runtime-benchmarks \ + Make sure to compile omni-node with --features=runtime-benchmarks \ to enable all supported benchmarks." .into()), } diff --git a/prdoc/pr_8594.prdoc b/prdoc/pr_8594.prdoc new file mode 100644 index 0000000000000..6511fcfc878a0 --- /dev/null +++ b/prdoc/pr_8594.prdoc @@ -0,0 +1,19 @@ +title: 'omni-node: fix `benchmark pallet` to work with `--runtime`' +doc: +- audience: Node Dev + description: | + `polkadot-omni-node benchmark pallet` can use one of `--runtime` or `--chain` now, like `frame-omni-bencher` does. + +crates: +- name: polkadot-omni-node-lib + bump: patch +- name: frame-benchmarking-cli + bump: major +- name: frame-omni-bencher + bump: patch +- name: polkadot-sdk + bump: patch +- name: polkadot-omni-node + bump: patch +- name: polkadot-parachain-bin + bump: patch diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 39a2070b7b4ef..50d7432a634d6 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -159,21 +159,6 @@ This could mean that you either did not build the node correctly with the \ not created by a node that was compiled with the flag"; impl PalletCmd { - /// Runs the command and benchmarks a pallet. - #[deprecated( - note = "`run` will be removed after December 2024. Use `run_with_spec` instead or \ - completely remove the code and use the `frame-benchmarking-cli` instead (see \ - https://github.com/paritytech/polkadot-sdk/pull/3512)." - )] - pub fn run(&self, config: sc_service::Configuration) -> Result<()> - where - Hasher: Hash, - ::Output: DecodeWithMemTracking, - ExtraHostFunctions: HostFunctions, - { - self.run_with_spec::(Some(config.chain_spec)) - } - fn state_handler_from_cli( &self, chain_spec_from_api: Option>, diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index f5796d05e339d..112af864990d0 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -134,14 +134,6 @@ impl V1SubCommand { match self { V1SubCommand::Benchmark(V1BenchmarkCommand { sub }) => match sub { BenchmarkCmd::Pallet(pallet) => { - if let Some(spec) = pallet.shared_params.chain { - return Err(format!( - "Chain specs are not supported. Please remove `--chain={spec}` and use \ - `--runtime=` instead" - ) - .into()); - } - pallet.run_with_spec::(None) }, BenchmarkCmd::Overhead(overhead_cmd) =>