Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use generic hash for runtime wasm in resolve_state_version_from_wasm #3447

Merged
merged 4 commits into from
Feb 24, 2024
Merged

Use generic hash for runtime wasm in resolve_state_version_from_wasm #3447

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ff7060
Use blake2_256 hash for runtime wasm
tmpolaczyk Feb 22, 2024
7a787ac
Make it generic over hash
tmpolaczyk Feb 23, 2024
4db5c7c
Add prdoc
tmpolaczyk Feb 23, 2024
4b86b42
Apply suggestions from code review
tmpolaczyk Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions substrate/client/chain-spec/src/genesis_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

//! Tool for creating the genesis block.

use std::{collections::hash_map::DefaultHasher, marker::PhantomData, sync::Arc};
use std::{marker::PhantomData, sync::Arc};

use sc_client_api::{backend::Backend, BlockImportOperation};
tmpolaczyk marked this conversation as resolved.
Show resolved Hide resolved
use sc_executor::RuntimeVersionOf;
Expand All @@ -43,12 +43,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: sp_core::blake2_256(wasm).to_vec(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct, you need to get the hasher used by the Block type. For most chains this is blake256, but not for all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean "not correct" not being really the right term here, but if we want to fix this, we should fix it properly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polkadot-sdk/substrate/primitives/state-machine/src/backend.rs

Lines 427 to 433 in 36b7037

let hash = self
.backend
.storage_hash(sp_core::storage::well_known_keys::CODE)
.ok()
.flatten()
.ok_or("`:code` hash not found")?
.encode();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to do that, and there are many places where this hash is already hardcoded to be blake2_256. How about we merge this and create an issue to make the hasher depend on the Block?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the function to take H: Hash and then you can pass HashingOf<Block> on the calling side of this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it was easier than I expected.

};
let runtime_version = RuntimeVersionOf::runtime_version(executor, &mut ext, &runtime_code)
.map_err(|e| sp_blockchain::Error::VersionInvalid(e.to_string()))?;
Expand Down
Loading