Skip to content

Commit

Permalink
Use generic hash for runtime wasm in resolve_state_version_from_wasm (p…
Browse files Browse the repository at this point in the history
…aritytech#3447)

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>
  • Loading branch information
2 people authored and RomarQ committed Mar 21, 2024
1 parent b1a8ee8 commit aec7a1f
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
13 changes: 13 additions & 0 deletions prdoc/pr_3447.prdoc
Original file line number Diff line number Diff line change
@@ -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
15 changes: 11 additions & 4 deletions substrate/client/chain-spec/src/genesis_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use std::{marker::PhantomData, sync::Arc};

use codec::Encode;
use sc_client_api::{backend::Backend, BlockImportOperation};
use sc_executor::RuntimeVersionOf;
use sp_core::{
Expand All @@ -28,17 +29,18 @@ use sp_core::{
};

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.
Expand All @@ -49,7 +51,11 @@ where
heap_pages: None,
hash: {
use sp_core::Hasher;
Blake2Hasher::hash(&wasm.to_vec()).as_ref().to_vec()
let hash = <H as HashT>::hash(wasm).encode();
log::debug!("hash1: {:?}", hash.clone());
log::debug!("hash2: {:?}", Blake2Hasher::hash(&wasm.to_vec()).as_ref().to_vec());

hash
},
};
let runtime_version = RuntimeVersionOf::runtime_version(executor, &mut ext, &runtime_code)
Expand Down Expand Up @@ -131,7 +137,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)?;
Expand Down
6 changes: 4 additions & 2 deletions substrate/client/service/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,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
Expand Down

0 comments on commit aec7a1f

Please sign in to comment.