From 2e4e5bf2fd0ae19fa38951c7e5f495dd1453b2bb Mon Sep 17 00:00:00 2001
From: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Date: Mon, 23 Sep 2024 00:28:38 +0200
Subject: [PATCH] [benchmarking] Reset to genesis storage after each run
 (#5655)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The genesis state is currently partially provided via
`OverlayedChanges`, but these changes are reset by the runtime after the
first repetition, causing the second repitition to use an invalid
genesis state.

Changes:
- Provide the genesis state as a `Storage` without any
`OverlayedChanges` to make it work correctly with repetitions.
- Add `--genesis-builder-preset` option to use different genesis preset
names.
- Improve error messages.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: ggwpez <ggwpez@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
---
 prdoc/pr_5655.prdoc                           |  15 ++
 .../node/cli/tests/benchmark_pallet_works.rs  |  35 +++
 .../benchmarking/pov/src/benchmarking.rs      |  31 +++
 .../benchmarking-cli/src/pallet/command.rs    | 228 +++++++-----------
 .../frame/benchmarking-cli/src/pallet/mod.rs  |  16 +-
 .../benchmarking-cli/src/pallet/types.rs      |  22 +-
 .../benchmarking-cli/src/pallet/writer.rs     |   7 +-
 7 files changed, 197 insertions(+), 157 deletions(-)
 create mode 100644 prdoc/pr_5655.prdoc

diff --git a/prdoc/pr_5655.prdoc b/prdoc/pr_5655.prdoc
new file mode 100644
index 00000000000..bfa2e295d15
--- /dev/null
+++ b/prdoc/pr_5655.prdoc
@@ -0,0 +1,15 @@
+title: '[benchmarking] Reset to genesis storage after each run'
+doc:
+- audience: Runtime Dev
+  description: |-
+    The genesis state is currently partially provided via `OverlayedChanges`, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state.
+
+    Changes:
+    - Provide the genesis state as a `Storage` without any `OverlayedChanges` to make it work correctly with repetitions.
+    - Add `--genesis-builder-preset` option to use different genesis preset names.
+    - Improve error messages.
+crates:
+- name: frame-benchmarking-cli
+  bump: major
+- name: frame-benchmarking-pallet-pov
+  bump: patch
diff --git a/substrate/bin/node/cli/tests/benchmark_pallet_works.rs b/substrate/bin/node/cli/tests/benchmark_pallet_works.rs
index 8441333429b..d913228881a 100644
--- a/substrate/bin/node/cli/tests/benchmark_pallet_works.rs
+++ b/substrate/bin/node/cli/tests/benchmark_pallet_works.rs
@@ -33,6 +33,31 @@ fn benchmark_pallet_works() {
 	benchmark_pallet(20, 50, true);
 }
 
+#[test]
+fn benchmark_pallet_args_work() {
+	benchmark_pallet_args(&["--list", "--pallet=pallet_balances"], true);
+	benchmark_pallet_args(&["--list", "--pallet=pallet_balances"], true);
+	benchmark_pallet_args(
+		&["--list", "--pallet=pallet_balances", "--genesis-builder=spec-genesis"],
+		true,
+	);
+	benchmark_pallet_args(
+		&["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=spec-genesis"],
+		true,
+	);
+
+	// Error because the genesis runtime does not have any presets in it:
+	benchmark_pallet_args(
+		&["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=spec-runtime"],
+		false,
+	);
+	// Error because no runtime is provided:
+	benchmark_pallet_args(
+		&["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=runtime"],
+		false,
+	);
+}
+
 fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) {
 	let status = Command::new(cargo_bin("substrate-node"))
 		.args(["benchmark", "pallet", "--dev"])
@@ -51,3 +76,13 @@ fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) {
 
 	assert_eq!(status.success(), should_work);
 }
+
+fn benchmark_pallet_args(args: &[&str], should_work: bool) {
+	let status = Command::new(cargo_bin("substrate-node"))
+		.args(["benchmark", "pallet"])
+		.args(args)
+		.status()
+		.unwrap();
+
+	assert_eq!(status.success(), should_work);
+}
diff --git a/substrate/frame/benchmarking/pov/src/benchmarking.rs b/substrate/frame/benchmarking/pov/src/benchmarking.rs
index bf3d406d0b2..d52fcc2689c 100644
--- a/substrate/frame/benchmarking/pov/src/benchmarking.rs
+++ b/substrate/frame/benchmarking/pov/src/benchmarking.rs
@@ -26,6 +26,11 @@ use frame_support::traits::UnfilteredDispatchable;
 use frame_system::{Pallet as System, RawOrigin};
 use sp_runtime::traits::Hash;
 
+#[cfg(feature = "std")]
+frame_support::parameter_types! {
+	pub static StorageRootHash: Option<alloc::vec::Vec<u8>> = None;
+}
+
 #[benchmarks]
 mod benchmarks {
 	use super::*;
@@ -392,6 +397,32 @@ mod benchmarks {
 		}
 	}
 
+	#[benchmark]
+	fn storage_root_is_the_same_every_time(i: Linear<0, 10>) {
+		#[cfg(feature = "std")]
+		let root = sp_io::storage::root(sp_runtime::StateVersion::V1);
+
+		#[cfg(feature = "std")]
+		match (i, StorageRootHash::get()) {
+			(0, Some(_)) => panic!("StorageRootHash should be None initially"),
+			(0, None) => StorageRootHash::set(Some(root)),
+			(_, Some(r)) if r == root => {},
+			(_, Some(r)) =>
+				panic!("StorageRootHash should be the same every time: {:?} vs {:?}", r, root),
+			(_, None) => panic!("StorageRootHash should be Some after the first iteration"),
+		}
+
+		// Also test that everything is reset correctly:
+		sp_io::storage::set(b"key1", b"value");
+
+		#[block]
+		{
+			sp_io::storage::set(b"key2", b"value");
+		}
+
+		sp_io::storage::set(b"key3", b"value");
+	}
+
 	impl_benchmark_test_suite!(Pallet, super::mock::new_test_ext(), super::mock::Test,);
 }
 
diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
index 47191981520..f1499f41c2e 100644
--- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
+++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
@@ -19,7 +19,7 @@ use super::{
 	types::{ComponentRange, ComponentRangeMap},
 	writer, ListOutput, PalletCmd,
 };
-use crate::pallet::{types::FetchedCode, GenesisBuilder};
+use crate::pallet::{types::FetchedCode, GenesisBuilderPolicy};
 use codec::{Decode, Encode};
 use frame_benchmarking::{
 	Analysis, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, BenchmarkParameter,
@@ -27,7 +27,7 @@ use frame_benchmarking::{
 };
 use frame_support::traits::StorageInfo;
 use linked_hash_map::LinkedHashMap;
-use sc_chain_spec::json_patch::merge as json_merge;
+use sc_chain_spec::GenesisConfigBuilderRuntimeCaller;
 use sc_cli::{execution_method_from_cli, ChainSpec, CliConfiguration, Result, SharedParams};
 use sc_client_db::BenchmarkingState;
 use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY};
@@ -36,18 +36,17 @@ use sp_core::{
 		testing::{TestOffchainExt, TestTransactionPoolExt},
 		OffchainDbExt, OffchainWorkerExt, TransactionPoolExt,
 	},
-	traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt, WrappedRuntimeCode},
+	traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt},
 	Hasher,
 };
 use sp_externalities::Extensions;
-use sp_genesis_builder::{PresetId, Result as GenesisBuildResult};
 use sp_keystore::{testing::MemoryKeystore, KeystoreExt};
 use sp_runtime::traits::Hash;
-use sp_state_machine::{OverlayedChanges, StateMachine};
+use sp_state_machine::StateMachine;
+use sp_storage::{well_known_keys::CODE, Storage};
 use sp_trie::{proof_size_extension::ProofSizeExt, recorder::Recorder};
 use sp_wasm_interface::HostFunctions;
 use std::{
-	borrow::Cow,
 	collections::{BTreeMap, BTreeSet, HashMap},
 	fmt::Debug,
 	fs,
@@ -162,9 +161,6 @@ generate the genesis state is deprecated. Please remove the `--chain`/`--dev`/`-
 point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. This warning may \
 become a hard error any time after December 2024.";
 
-/// The preset that we expect to find in the GenesisBuilder runtime API.
-const GENESIS_PRESET: &str = "development";
-
 impl PalletCmd {
 	/// Runs the command and benchmarks a pallet.
 	#[deprecated(
@@ -214,9 +210,7 @@ impl PalletCmd {
 			return self.output_from_results(&batches)
 		}
 
-		let (genesis_storage, genesis_changes) =
-			self.genesis_storage::<Hasher, ExtraHostFunctions>(&chain_spec)?;
-		let mut changes = genesis_changes.clone();
+		let genesis_storage = self.genesis_storage::<ExtraHostFunctions>(&chain_spec)?;
 
 		let cache_size = Some(self.database_cache_size as usize);
 		let state_with_tracking = BenchmarkingState::<Hasher>::new(
@@ -262,7 +256,7 @@ impl PalletCmd {
 			Self::exec_state_machine(
 				StateMachine::new(
 					state,
-					&mut changes,
+					&mut Default::default(),
 					&executor,
 					"Benchmark_benchmark_metadata",
 					&(self.extra).encode(),
@@ -347,7 +341,6 @@ impl PalletCmd {
 			for (s, selected_components) in all_components.iter().enumerate() {
 				// First we run a verification
 				if !self.no_verify {
-					let mut changes = genesis_changes.clone();
 					let state = &state_without_tracking;
 					// Don't use these results since verification code will add overhead.
 					let _batch: Vec<BenchmarkBatch> = match Self::exec_state_machine::<
@@ -357,7 +350,7 @@ impl PalletCmd {
 					>(
 						StateMachine::new(
 							state,
-							&mut changes,
+							&mut Default::default(),
 							&executor,
 							"Benchmark_dispatch_benchmark",
 							&(
@@ -389,7 +382,6 @@ impl PalletCmd {
 				}
 				// Do one loop of DB tracking.
 				{
-					let mut changes = genesis_changes.clone();
 					let state = &state_with_tracking;
 					let batch: Vec<BenchmarkBatch> = match Self::exec_state_machine::<
 						std::result::Result<Vec<BenchmarkBatch>, String>,
@@ -398,7 +390,7 @@ impl PalletCmd {
 					>(
 						StateMachine::new(
 							state, // todo remove tracking
-							&mut changes,
+							&mut Default::default(),
 							&executor,
 							"Benchmark_dispatch_benchmark",
 							&(
@@ -432,7 +424,6 @@ impl PalletCmd {
 				}
 				// Finally run a bunch of loops to get extrinsic timing information.
 				for r in 0..self.external_repeat {
-					let mut changes = genesis_changes.clone();
 					let state = &state_without_tracking;
 					let batch = match Self::exec_state_machine::<
 						std::result::Result<Vec<BenchmarkBatch>, String>,
@@ -441,7 +432,7 @@ impl PalletCmd {
 					>(
 						StateMachine::new(
 							state, // todo remove tracking
-							&mut changes,
+							&mut Default::default(),
 							&executor,
 							"Benchmark_dispatch_benchmark",
 							&(
@@ -567,19 +558,19 @@ impl PalletCmd {
 		Ok(benchmarks_to_run)
 	}
 
-	/// Produce a genesis storage and genesis changes.
+	/// Build the genesis storage by either the Genesis Builder API, chain spec or nothing.
 	///
-	/// It would be easier to only return one type, but there is no easy way to convert them.
-	// TODO: Re-write `BenchmarkingState` to not be such a clusterfuck and only accept
-	// `OverlayedChanges` instead of a mix between `OverlayedChanges` and `State`. But this can only
-	// be done once we deprecated and removed the legacy interface :(
-	fn genesis_storage<H: Hash, F: HostFunctions>(
+	/// Behaviour can be controlled by the `--genesis-builder` flag.
+	fn genesis_storage<F: HostFunctions>(
 		&self,
 		chain_spec: &Option<Box<dyn ChainSpec>>,
-	) -> Result<(sp_storage::Storage, OverlayedChanges<H>)> {
-		Ok(match (self.genesis_builder, self.runtime.is_some()) {
-			(Some(GenesisBuilder::None), _) => Default::default(),
-			(Some(GenesisBuilder::Spec), _) | (None, false) => {
+	) -> Result<sp_storage::Storage> {
+		Ok(match (self.genesis_builder, self.runtime.as_ref()) {
+			(Some(GenesisBuilderPolicy::None), Some(_)) => return Err("Cannot use `--genesis-builder=none` with `--runtime` since the runtime would be ignored.".into()),
+			(Some(GenesisBuilderPolicy::None), None) => Storage::default(),
+			(Some(GenesisBuilderPolicy::SpecGenesis | GenesisBuilderPolicy::Spec), Some(_)) =>
+					return Err("Cannot use `--genesis-builder=spec-genesis` with `--runtime` since the runtime would be ignored.".into()),
+			(Some(GenesisBuilderPolicy::SpecGenesis | GenesisBuilderPolicy::Spec), None) | (None, None) => {
 				log::warn!("{WARN_SPEC_GENESIS_CTOR}");
 				let Some(chain_spec) = chain_spec else {
 					return Err("No chain spec specified to generate the genesis state".into());
@@ -589,111 +580,73 @@ impl PalletCmd {
 					.build_storage()
 					.map_err(|e| format!("{ERROR_CANNOT_BUILD_GENESIS}\nError: {e}"))?;
 
-				(storage, Default::default())
+				storage
 			},
-			(Some(GenesisBuilder::Runtime), _) | (None, true) =>
-				(Default::default(), self.genesis_from_runtime::<H, F>()?),
-		})
-	}
+			(Some(GenesisBuilderPolicy::SpecRuntime), Some(_)) =>
+				return Err("Cannot use `--genesis-builder=spec` with `--runtime` since the runtime would be ignored.".into()),
+			(Some(GenesisBuilderPolicy::SpecRuntime), None) => {
+				let Some(chain_spec) = chain_spec else {
+					return Err("No chain spec specified to generate the genesis state".into());
+				};
 
-	/// Generate the genesis changeset by the runtime API.
-	fn genesis_from_runtime<H: Hash, F: HostFunctions>(&self) -> Result<OverlayedChanges<H>> {
-		let state = BenchmarkingState::<H>::new(
-			Default::default(),
-			Some(self.database_cache_size as usize),
-			false,
-			false,
-		)?;
+				self.genesis_from_spec_runtime::<F>(chain_spec.as_ref())?
+			},
+			(Some(GenesisBuilderPolicy::Runtime), None) => return Err("Cannot use `--genesis-builder=runtime` without `--runtime`".into()),
+			(Some(GenesisBuilderPolicy::Runtime), Some(runtime)) | (None, Some(runtime)) => {
+				log::info!("Loading WASM from {}", runtime.display());
+
+				let code = fs::read(&runtime).map_err(|e| {
+					format!(
+						"Could not load runtime file from path: {}, error: {}",
+						runtime.display(),
+						e
+					)
+				})?;
 
-		// Create a dummy WasmExecutor just to build the genesis storage.
-		let method =
-			execution_method_from_cli(self.wasm_method, self.wasmtime_instantiation_strategy);
-		let executor = WasmExecutor::<(
-			sp_io::SubstrateHostFunctions,
-			frame_benchmarking::benchmarking::HostFunctions,
-			F,
-		)>::builder()
-		.with_execution_method(method)
-		.with_allow_missing_host_functions(self.allow_missing_host_functions)
-		.build();
+				self.genesis_from_code::<F>(&code)?
+			}
+		})
+	}
 
-		let runtime = self.runtime_blob(&state)?;
-		let runtime_code = runtime.code()?;
+	/// Setup the genesis state by calling the runtime APIs of the chain-specs genesis runtime.
+	fn genesis_from_spec_runtime<EHF: HostFunctions>(
+		&self,
+		chain_spec: &dyn ChainSpec,
+	) -> Result<Storage> {
+		log::info!("Building genesis state from chain spec runtime");
+		let storage = chain_spec
+			.build_storage()
+			.map_err(|e| format!("{ERROR_CANNOT_BUILD_GENESIS}\nError: {e}"))?;
 
-		// We cannot use the `GenesisConfigBuilderRuntimeCaller` here since it returns the changes
-		// as `Storage` item, but we need it as `OverlayedChanges`.
-		let genesis_json: Option<Vec<u8>> = Self::exec_state_machine(
-			StateMachine::new(
-				&state,
-				&mut Default::default(),
-				&executor,
-				"GenesisBuilder_get_preset",
-				&None::<PresetId>.encode(), // Use the default preset
-				&mut Self::build_extensions(executor.clone(), state.recorder()),
-				&runtime_code,
-				CallContext::Offchain,
-			),
-			"build the genesis spec",
-		)?;
+		let code: &Vec<u8> =
+			storage.top.get(CODE).ok_or("No runtime code in the genesis storage")?;
 
-		let Some(base_genesis_json) = genesis_json else {
-			return Err("GenesisBuilder::get_preset returned no data".into())
-		};
-
-		let base_genesis_json = serde_json::from_slice::<serde_json::Value>(&base_genesis_json)
-			.map_err(|e| format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e))?;
-
-		let dev_genesis_json: Option<Vec<u8>> = Self::exec_state_machine(
-			StateMachine::new(
-				&state,
-				&mut Default::default(),
-				&executor,
-				"GenesisBuilder_get_preset",
-				&Some::<PresetId>(GENESIS_PRESET.into()).encode(), // Use the default preset
-				&mut Self::build_extensions(executor.clone(), state.recorder()),
-				&runtime_code,
-				CallContext::Offchain,
-			),
-			"build the genesis spec",
-		)?;
+		self.genesis_from_code::<EHF>(code)
+	}
 
-		let mut genesis_json = serde_json::Value::default();
-		json_merge(&mut genesis_json, base_genesis_json);
+	fn genesis_from_code<EHF: HostFunctions>(&self, code: &[u8]) -> Result<Storage> {
+		let genesis_config_caller = GenesisConfigBuilderRuntimeCaller::<(
+			sp_io::SubstrateHostFunctions,
+			frame_benchmarking::benchmarking::HostFunctions,
+			EHF,
+		)>::new(code);
+		let preset = Some(&self.genesis_builder_preset);
 
-		if let Some(dev) = dev_genesis_json {
-			let dev: serde_json::Value = serde_json::from_slice(&dev).map_err(|e| {
-				format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e)
+		let mut storage =
+			genesis_config_caller.get_storage_for_named_preset(preset).inspect_err(|e| {
+				let presets = genesis_config_caller.preset_names().unwrap_or_default();
+				log::error!(
+					"Please pick one of the available presets with \
+			`--genesis-builder-preset=<PRESET>`. Available presets ({}): {:?}. Error: {:?}",
+					presets.len(),
+					presets,
+					e
+				);
 			})?;
-			json_merge(&mut genesis_json, dev);
-		} else {
-			log::warn!(
-				"Could not find genesis preset '{GENESIS_PRESET}'. Falling back to default."
-			);
-		}
-
-		let json_pretty_str = serde_json::to_string_pretty(&genesis_json)
-			.map_err(|e| format!("json to string failed: {e}"))?;
-
-		let mut changes = Default::default();
-		let build_res: GenesisBuildResult = Self::exec_state_machine(
-			StateMachine::new(
-				&state,
-				&mut changes,
-				&executor,
-				"GenesisBuilder_build_state",
-				&json_pretty_str.encode(),
-				&mut Extensions::default(),
-				&runtime_code,
-				CallContext::Offchain,
-			),
-			"populate the genesis state",
-		)?;
 
-		if let Err(e) = build_res {
-			return Err(format!("GenesisBuilder::build_state failed: {}", e).into())
-		}
+		storage.top.insert(CODE.into(), code.into());
 
-		Ok(changes)
+		Ok(storage)
 	}
 
 	/// Execute a state machine and decode its return value as `R`.
@@ -737,19 +690,10 @@ impl PalletCmd {
 		&self,
 		state: &'a BenchmarkingState<H>,
 	) -> Result<FetchedCode<'a, BenchmarkingState<H>, H>> {
-		if let Some(runtime) = &self.runtime {
-			log::info!("Loading WASM from {}", runtime.display());
-			let code = fs::read(runtime)?;
-			let hash = sp_core::blake2_256(&code).to_vec();
-			let wrapped_code = WrappedRuntimeCode(Cow::Owned(code));
-
-			Ok(FetchedCode::FromFile { wrapped_code, heap_pages: self.heap_pages, hash })
-		} else {
-			log::info!("Loading WASM from genesis state");
-			let state = sp_state_machine::backend::BackendRuntimeCode::new(state);
-
-			Ok(FetchedCode::FromGenesis { state })
-		}
+		log::info!("Loading WASM from state");
+		let state = sp_state_machine::backend::BackendRuntimeCode::new(state);
+
+		Ok(FetchedCode { state })
 	}
 
 	/// Allocation strategy for pallet benchmarking.
@@ -990,19 +934,25 @@ impl PalletCmd {
 
 		if let Some(output_path) = &self.output {
 			if !output_path.is_dir() && output_path.file_name().is_none() {
-				return Err("Output file or path is invalid!".into())
+				return Err(format!(
+					"Output path is neither a directory nor a file: {output_path:?}"
+				)
+				.into())
 			}
 		}
 
 		if let Some(header_file) = &self.header {
 			if !header_file.is_file() {
-				return Err("Header file is invalid!".into())
+				return Err(format!("Header file could not be found: {header_file:?}").into())
 			};
 		}
 
 		if let Some(handlebars_template_file) = &self.template {
 			if !handlebars_template_file.is_file() {
-				return Err("Handlebars template file is invalid!".into())
+				return Err(format!(
+					"Handlebars template file could not be found: {handlebars_template_file:?}"
+				)
+				.into())
 			};
 		}
 
diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs
index ebf737be1db..a507c1d1f54 100644
--- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs
+++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs
@@ -19,7 +19,7 @@ mod command;
 mod types;
 mod writer;
 
-use crate::{pallet::types::GenesisBuilder, shared::HostInfoParams};
+use crate::{pallet::types::GenesisBuilderPolicy, shared::HostInfoParams};
 use clap::ValueEnum;
 use sc_cli::{
 	WasmExecutionMethod, WasmtimeInstantiationStrategy, DEFAULT_WASMTIME_INSTANTIATION_STRATEGY,
@@ -177,9 +177,17 @@ pub struct PalletCmd {
 
 	/// How to construct the genesis state.
 	///
-	/// Uses `GenesisBuilder::Spec` by default and  `GenesisBuilder::Runtime` if `runtime` is set.
-	#[arg(long, value_enum)]
-	pub genesis_builder: Option<GenesisBuilder>,
+	/// Uses `GenesisBuilderPolicy::Spec` by default and  `GenesisBuilderPolicy::Runtime` if
+	/// `runtime` is set.
+	#[arg(long, value_enum, alias = "genesis-builder-policy")]
+	pub genesis_builder: Option<GenesisBuilderPolicy>,
+
+	/// The preset that we expect to find in the GenesisBuilder runtime API.
+	///
+	/// This can be useful when a runtime has a dedicated benchmarking preset instead of using the
+	/// default one.
+	#[arg(long, default_value = sp_genesis_builder::DEV_RUNTIME_PRESET)]
+	pub genesis_builder_preset: String,
 
 	/// DEPRECATED: This argument has no effect.
 	#[arg(long = "execution")]
diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs
index 2bb00d66560..ce37be455e8 100644
--- a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs
+++ b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs
@@ -18,13 +18,13 @@
 //! Various types used by this crate.
 
 use sc_cli::Result;
-use sp_core::traits::{RuntimeCode, WrappedRuntimeCode};
+use sp_core::traits::RuntimeCode;
 use sp_runtime::traits::Hash;
 
 /// How the genesis state for benchmarking should be build.
 #[derive(clap::ValueEnum, Debug, Eq, PartialEq, Clone, Copy)]
 #[clap(rename_all = "kebab-case")]
-pub enum GenesisBuilder {
+pub enum GenesisBuilderPolicy {
 	/// Do not provide any genesis state.
 	///
 	/// Benchmarks are advised to function with this, since they should setup their own required
@@ -32,16 +32,19 @@ pub enum GenesisBuilder {
 	None,
 	/// Let the runtime build the genesis state through its `BuildGenesisConfig` runtime API.
 	Runtime,
+	// Use the runtime from the Spec file to build the genesis state.
+	SpecRuntime,
 	/// Use the spec file to build the genesis state. This fails when there is no spec.
+	SpecGenesis,
+	/// Same as `SpecGenesis` - only here for backwards compatibility.
 	Spec,
 }
 
 /// A runtime blob that was either fetched from genesis storage or loaded from a file.
 // NOTE: This enum is only needed for the annoying lifetime bounds on `RuntimeCode`. Otherwise we
 // could just directly return the blob.
-pub enum FetchedCode<'a, B, H> {
-	FromGenesis { state: sp_state_machine::backend::BackendRuntimeCode<'a, B, H> },
-	FromFile { wrapped_code: WrappedRuntimeCode<'a>, heap_pages: Option<u64>, hash: Vec<u8> },
+pub struct FetchedCode<'a, B, H> {
+	pub state: sp_state_machine::backend::BackendRuntimeCode<'a, B, H>,
 }
 
 impl<'a, B, H> FetchedCode<'a, B, H>
@@ -51,14 +54,7 @@ where
 {
 	/// The runtime blob.
 	pub fn code(&'a self) -> Result<RuntimeCode<'a>> {
-		match self {
-			Self::FromGenesis { state } => state.runtime_code().map_err(Into::into),
-			Self::FromFile { wrapped_code, heap_pages, hash } => Ok(RuntimeCode {
-				code_fetcher: wrapped_code,
-				heap_pages: *heap_pages,
-				hash: hash.clone(),
-			}),
-		}
+		self.state.runtime_code().map_err(Into::into)
 	}
 }
 
diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs
index df7d81b2822..34f31734da6 100644
--- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs
+++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs
@@ -484,7 +484,12 @@ pub(crate) fn write_results(
 			benchmarks: results.clone(),
 		};
 
-		let mut output_file = fs::File::create(&file_path)?;
+		let file_path = fs::canonicalize(&file_path).map_err(|e| {
+			format!("Could not get absolute path for: {:?}. Error: {:?}", &file_path, e)
+		})?;
+		let mut output_file = fs::File::create(&file_path).map_err(|e| {
+			format!("Could not write weight file to: {:?}. Error: {:?}", &file_path, e)
+		})?;
 		handlebars
 			.render_template_to_write(&template, &hbs_data, &mut output_file)
 			.map_err(|e| io_error(&e.to_string()))?;
-- 
GitLab