Clean up genesis initialization in benchmarking-cli

[skunert]

With the introduction of the frame-omni-bencher binary we have the option to initialize genesis state with multiple methods.

The underlying implementation in benchmarking-cli provides options:

1. We can provide a runtime wasm blob with --runtime
   a. If provided without --genesis-builder or with --genesis-builder=runtime, it will use the development preset of the WASM blob. This case breaks when used from polkadot-parachain, because in the CLI there we expect that a chain-spec is provided.
   b. If provided with --genesis-builder=none, it will initialize with empty genesis
   c. If provided with --genesis-builder=spec it will crash because no spec will be provided
2. We can provide a chain-spec with --chain
   a. If provided without --genesis-builder=runtime, it will use the development preset of the WASM code that is included in the chainspec. This case is currently broken as the state we try to read from is not properly initialized here:

   polkadot-sdk/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

   Lines 601 to 606 in a34cc8d

   	let state = BenchmarkingState::<H>::new(
   	Default::default(),
   	Some(self.database_cache_size as usize),
   	false,
   	false,
   	)?;

   
   b. If provided with --genesis-builder=none, it will initialize with empty genesis
   c. If provided with --genesis-builder=spec it will use the storage included in the chain-spec

There is also currently a difference between initialization form chain spec vs runtime preset. For runtime presets, we return OverlayedChanges, for chain-specs, we return Storage here:

polkadot-sdk/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

Lines 570 to 579 in a34cc8d

	/// Produce a genesis storage and genesis changes.
	///
	/// 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>(
	&self,
	chain_spec: &Option<Box<dyn ChainSpec>>,
	) -> Result<(sp_storage::Storage, OverlayedChanges<H>)> {

The comment there says that this should be refactored to always return OverlayedChanges, but I propose we do the opposite and use Storage always. If we use OverlayedChanges read values will be invisible to the proof recorder, which will change the measured storage proof values. The real-world effect of this should be minimal since the measured values only come into effect when someone provides a custom --default-pov-mode, but I am not sure if anyone does that and what the use-case is. Still always using Storage should be the better solution.

TLDR: 1a and 2a are problematic and should be fixed. For the unification I already have some code prepared.

cc @ggwpez

[bkontur]

Based on my investigations and testing so far here, the OverlayedChanges and BenchmarkingState seem to cause problems. Therefore, I am in favor of not using them. However, there could still be a hidden bug related to OverlayedChanges, BenchmarkingState, or StateMachine.