From de6d02591b57d03f70ed8db0c674f045ad2ea029 Mon Sep 17 00:00:00 2001
From: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com>
Date: Sat, 24 Feb 2024 11:34:05 +0100
Subject: [PATCH] Use generic hash for runtime wasm in
 resolve_state_version_from_wasm (#3447)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Changes the runtime hash algorithm used in
`resolve_state_version_from_wasm` from `DefaultHasher` to a
caller-provided one (usually `HashingFor<Block>`), to match the one used
elsewhere.

This fixes an issue where the runtime wasm is compiled 3 times when
starting the `tanssi-node` with `--dev`. With this fix, the runtime wasm
is only compiled 2 times. The other redundant compilation is caused by
the `GenesisConfigBuilderRuntimeCaller` struct, which ignores the
runtime cache.

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
---
 prdoc/pr_3447.prdoc                            | 13 +++++++++++++
 .../client/chain-spec/src/genesis_block.rs     | 18 ++++++++----------
 substrate/client/service/src/client/client.rs  |  6 ++++--
 3 files changed, 25 insertions(+), 12 deletions(-)
 create mode 100644 prdoc/pr_3447.prdoc

diff --git a/prdoc/pr_3447.prdoc b/prdoc/pr_3447.prdoc
new file mode 100644
index 0000000000..1d8d4f409f
--- /dev/null
+++ b/prdoc/pr_3447.prdoc
@@ -0,0 +1,13 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Use generic hash for runtime wasm in resolve_state_version_from_wasm
+
+doc:
+  - audience: Node Dev
+    description: |
+      Changes the runtime hash algorithm used in resolve_state_version_from_wasm from DefaultHasher to a caller-provided
+      one (usually HashingFor<Block>). Fixes a bug where the runtime wasm was being compiled again when it was not
+      needed, because the hash did not match
+crates:
+  - name: sc-chain-spec
diff --git a/substrate/client/chain-spec/src/genesis_block.rs b/substrate/client/chain-spec/src/genesis_block.rs
index 6aa156a620..3c7b9f64dc 100644
--- a/substrate/client/chain-spec/src/genesis_block.rs
+++ b/substrate/client/chain-spec/src/genesis_block.rs
@@ -18,23 +18,25 @@
 
 //! Tool for creating the genesis block.
 
-use std::{collections::hash_map::DefaultHasher, marker::PhantomData, sync::Arc};
+use std::{marker::PhantomData, sync::Arc};
 
+use codec::Encode;
 use sc_client_api::{backend::Backend, BlockImportOperation};
 use sc_executor::RuntimeVersionOf;
 use sp_core::storage::{well_known_keys, StateVersion, Storage};
 use sp_runtime::{
-	traits::{Block as BlockT, Hash as HashT, Header as HeaderT, Zero},
+	traits::{Block as BlockT, Hash as HashT, HashingFor, Header as HeaderT, Zero},
 	BuildStorage,
 };
 
 /// Return the state version given the genesis storage and executor.
-pub fn resolve_state_version_from_wasm<E>(
+pub fn resolve_state_version_from_wasm<E, H>(
 	storage: &Storage,
 	executor: &E,
 ) -> sp_blockchain::Result<StateVersion>
 where
 	E: RuntimeVersionOf,
+	H: HashT,
 {
 	if let Some(wasm) = storage.top.get(well_known_keys::CODE) {
 		let mut ext = sp_state_machine::BasicExternalities::new_empty(); // just to read runtime version.
@@ -43,12 +45,7 @@ where
 		let runtime_code = sp_core::traits::RuntimeCode {
 			code_fetcher: &code_fetcher,
 			heap_pages: None,
-			hash: {
-				use std::hash::{Hash, Hasher};
-				let mut state = DefaultHasher::new();
-				wasm.hash(&mut state);
-				state.finish().to_le_bytes().to_vec()
-			},
+			hash: <H as HashT>::hash(wasm).encode(),
 		};
 		let runtime_version = RuntimeVersionOf::runtime_version(executor, &mut ext, &runtime_code)
 			.map_err(|e| sp_blockchain::Error::VersionInvalid(e.to_string()))?;
@@ -129,7 +126,8 @@ impl<Block: BlockT, B: Backend<Block>, E: RuntimeVersionOf> BuildGenesisBlock<Bl
 	fn build_genesis_block(self) -> sp_blockchain::Result<(Block, Self::BlockImportOperation)> {
 		let Self { genesis_storage, commit_genesis_state, backend, executor, _phantom } = self;
 
-		let genesis_state_version = resolve_state_version_from_wasm(&genesis_storage, &executor)?;
+		let genesis_state_version =
+			resolve_state_version_from_wasm::<_, HashingFor<Block>>(&genesis_storage, &executor)?;
 		let mut op = backend.begin_operation()?;
 		let state_root =
 			op.set_genesis_state(genesis_storage, commit_genesis_state, genesis_state_version)?;
diff --git a/substrate/client/service/src/client/client.rs b/substrate/client/service/src/client/client.rs
index aa9c1b80a2..108c258595 100644
--- a/substrate/client/service/src/client/client.rs
+++ b/substrate/client/service/src/client/client.rs
@@ -675,8 +675,10 @@ where
 
 						// This is use by fast sync for runtime version to be resolvable from
 						// changes.
-						let state_version =
-							resolve_state_version_from_wasm(&storage, &self.executor)?;
+						let state_version = resolve_state_version_from_wasm::<_, HashingFor<Block>>(
+							&storage,
+							&self.executor,
+						)?;
 						let state_root = operation.op.reset_storage(storage, state_version)?;
 						if state_root != *import_headers.post().state_root() {
 							// State root mismatch when importing state. This should not happen in