[stable2409] Backport #5655

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

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);
+}

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,);
 }