The storage runtime interface should not enforce a hash type (#4231)

* The storage runtime interface should not enforce a hash type

Currently the runtime interface enforces `H256` as hash type, but in the
future people could use whatever they want as hash type. The hash type
always needs to match between the runtime and the node, but that is
already required.

* Update primitives/externalities/src/lib.rs

Co-Authored-By: thiolliere <gui.thiolliere@gmail.com>

bin/node/executor/src/lib.rs

@@ -913,7 +913,7 @@ mod tests {
 			None,
 		).0.unwrap();
 
-		assert!(t.ext().storage_changes_root(GENESIS_HASH.into()).unwrap().is_some());
+		assert!(t.ext().storage_changes_root(&GENESIS_HASH.encode()).unwrap().is_some());
 	}
 
 	#[test]
@@ -929,7 +929,7 @@ mod tests {
 			None,
 		).0.unwrap();
 
-		assert!(t.ext().storage_changes_root(GENESIS_HASH.into()).unwrap().is_some());
+		assert!(t.ext().storage_changes_root(&GENESIS_HASH.encode()).unwrap().is_some());
 	}
 
 	#[test]

client/db/src/lib.rs

@@ -174,7 +174,7 @@ impl<B: BlockT> StateBackend<Blake2Hasher> for RefTrackingState<B> {
 		self.state.storage_root(delta)
 	}
 
-	fn child_storage_root<I>(&self, storage_key: &[u8], delta: I) -> (Vec<u8>, bool, Self::Transaction)
+	fn child_storage_root<I>(&self, storage_key: &[u8], delta: I) -> (H256, bool, Self::Transaction)
 		where
 			I: IntoIterator<Item=(Vec<u8>, Option<Vec<u8>>)>,
 	{
@@ -193,7 +193,9 @@ impl<B: BlockT> StateBackend<Blake2Hasher> for RefTrackingState<B> {
 		self.state.child_keys(child_key, prefix)
 	}
 
-	fn as_trie_backend(&mut self) -> Option<&state_machine::TrieBackend<Self::TrieBackendStorage, Blake2Hasher>> {
+	fn as_trie_backend(
+		&mut self,
+	) -> Option<&state_machine::TrieBackend<Self::TrieBackendStorage, Blake2Hasher>> {
 		self.state.as_trie_backend()
 	}
 }

client/db/src/storage_cache.rs

@@ -568,7 +568,7 @@ impl<H: Hasher, S: StateBackend<H>, B: BlockT> StateBackend<H> for CachingState<
 		self.state.storage_root(delta)
 	}
 
-	fn child_storage_root<I>(&self, storage_key: &[u8], delta: I) -> (Vec<u8>, bool, Self::Transaction)
+	fn child_storage_root<I>(&self, storage_key: &[u8], delta: I) -> (H::Out, bool, Self::Transaction)
 		where
 			I: IntoIterator<Item=(Vec<u8>, Option<Vec<u8>>)>,
 			H::Out: Ord

client/executor/src/deprecated_host_interface.rs

@@ -425,7 +425,7 @@ impl_wasm_host_interface! {
 			context.read_memory_into(parent_hash_data, &mut parent_hash[..])
 				.map_err(|_| "Invalid attempt to get parent_hash in ext_storage_changes_root")?;
 
-			if let Some(r) = runtime_io::storage::changes_root(parent_hash) {
+			if let Some(r) = runtime_io::storage::changes_root(&parent_hash) {
 				context.write_memory(result, &r[..])
 					.map_err(|_| "Invalid attempt to set memory in ext_storage_changes_root")?;
 				Ok(1)

client/executor/src/wasm_runtime.rs

@@ -26,7 +26,7 @@ use log::{trace, warn};
 
 use codec::Decode;
 
-use primitives::{storage::well_known_keys, traits::Externalities, H256};
+use primitives::{storage::well_known_keys, traits::Externalities};
 
 use runtime_version::RuntimeVersion;
 use std::{collections::hash_map::{Entry, HashMap}, panic::AssertUnwindSafe};
@@ -82,7 +82,7 @@ pub struct RuntimesCache {
 	/// A cache of runtime instances along with metadata, ready to be reused.
 	///
 	/// Instances are keyed by the Wasm execution method and the hash of their code.
-	instances: HashMap<(WasmExecutionMethod, [u8; 32]), Result<VersionedRuntime, WasmError>>,
+	instances: HashMap<(WasmExecutionMethod, Vec<u8>), Result<VersionedRuntime, WasmError>>,
 }
 
 impl RuntimesCache {
@@ -128,7 +128,7 @@ impl RuntimesCache {
 		wasm_method: WasmExecutionMethod,
 		default_heap_pages: u64,
 		host_functions: &[&'static dyn Function],
-	) -> Result<(&mut (dyn WasmRuntime + 'static), &RuntimeVersion, H256), Error> {
+	) -> Result<(&mut (dyn WasmRuntime + 'static), &RuntimeVersion, Vec<u8>), Error> {
 		let code_hash = ext
 			.original_storage_hash(well_known_keys::CODE)
 			.ok_or(Error::InvalidCode("`CODE` not found in storage.".into()))?;
@@ -138,7 +138,7 @@ impl RuntimesCache {
 			.and_then(|pages| u64::decode(&mut &pages[..]).ok())
 			.unwrap_or(default_heap_pages);
 
-		let result = match self.instances.entry((wasm_method, code_hash.into())) {
+		let result = match self.instances.entry((wasm_method, code_hash.clone())) {
 			Entry::Occupied(o) => {
 				let result = o.into_mut();
 				if let Ok(ref mut cached_runtime) = result {
@@ -198,10 +198,10 @@ impl RuntimesCache {
 	pub fn invalidate_runtime(
 		&mut self,
 		wasm_method: WasmExecutionMethod,
-		code_hash: H256,
+		code_hash: Vec<u8>,
 	) {
 		// Just remove the instance, it will be re-created the next time it is requested.
-		self.instances.remove(&(wasm_method, code_hash.into()));
+		self.instances.remove(&(wasm_method, code_hash));
 	}
 }
 

client/src/cht.rs

@@ -92,7 +92,7 @@ pub fn build_proof<Header, Hasher, BlocksI, HashesI>(
 	where
 		Header: HeaderT,
 		Hasher: hash_db::Hasher,
-		Hasher::Out: Ord,
+		Hasher::Out: Ord + codec::Codec,
 		BlocksI: IntoIterator<Item=Header::Number>,
 		HashesI: IntoIterator<Item=ClientResult<Option<Header::Hash>>>,
 {
@@ -119,7 +119,7 @@ pub fn check_proof<Header, Hasher>(
 	where
 		Header: HeaderT,
 		Hasher: hash_db::Hasher,
-		Hasher::Out: Ord,
+		Hasher::Out: Ord + codec::Codec,
 {
 	do_check_proof::<Header, Hasher, _>(
 		local_root,
@@ -148,7 +148,7 @@ pub fn check_proof_on_proving_backend<Header, Hasher>(
 	where
 		Header: HeaderT,
 		Hasher: hash_db::Hasher,
-		Hasher::Out: Ord,
+		Hasher::Out: Ord + codec::Codec,
 {
 	do_check_proof::<Header, Hasher, _>(
 		local_root,

client/src/light/backend.rs

@@ -21,7 +21,9 @@ use std::collections::HashMap;
 use std::sync::Arc;
 use parking_lot::RwLock;
 
-use state_machine::{Backend as StateBackend, TrieBackend, backend::InMemory as InMemoryState, ChangesTrieTransaction};
+use state_machine::{
+	Backend as StateBackend, TrieBackend, backend::InMemory as InMemoryState, ChangesTrieTransaction
+};
 use primitives::offchain::storage::InMemOffchainStorage;
 use sr_primitives::{generic::BlockId, Justification, StorageOverlay, ChildrenStorageOverlay};
 use sr_primitives::traits::{Block as BlockT, NumberFor, Zero, Header};
@@ -341,7 +343,7 @@ impl<H: Hasher> std::fmt::Debug for GenesisOrUnavailableState<H> {
 
 impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 	where
-		H::Out: Ord,
+		H::Out: Ord + codec::Codec,
 {
 	type Error = ClientError;
 	type Transaction = ();
@@ -409,7 +411,7 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 		}
 	}
 
-	fn child_storage_root<I>(&self, key: &[u8], delta: I) -> (Vec<u8>, bool, Self::Transaction)
+	fn child_storage_root<I>(&self, key: &[u8], delta: I) -> (H::Out, bool, Self::Transaction)
 	where
 		I: IntoIterator<Item=(Vec<u8>, Option<Vec<u8>>)>
 	{
@@ -418,7 +420,7 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 				let (root, is_equal, _) = state.child_storage_root(key, delta);
 				(root, is_equal, ())
 			},
-			GenesisOrUnavailableState::Unavailable => (H::Out::default().as_ref().to_vec(), true, ()),
+			GenesisOrUnavailableState::Unavailable => (H::Out::default(), true, ()),
 		}
 	}
 

client/src/light/fetcher.rs

@@ -70,7 +70,7 @@ impl<E, H, B: BlockT, S: BlockchainStorage<B>> LightDataChecker<E, H, B, S> {
 	) -> ClientResult<Vec<(NumberFor<B>, u32)>>
 		where
 			H: Hasher,
-			H::Out: Ord,
+			H::Out: Ord + codec::Codec,
 	{
 		// since we need roots of all changes tries for the range begin..max
 		// => remote node can't use max block greater that one that we have passed
@@ -148,7 +148,7 @@ impl<E, H, B: BlockT, S: BlockchainStorage<B>> LightDataChecker<E, H, B, S> {
 	) -> ClientResult<()>
 		where
 			H: Hasher,
-			H::Out: Ord,
+			H::Out: Ord + codec::Codec,
 	{
 		// all the checks are sharing the same storage
 		let storage = create_proof_check_backend_storage(remote_roots_proof);

frame/system/src/lib.rs

@@ -668,13 +668,17 @@ impl<T: Trait> Module<T> {
 			}
 		}
 
-		let storage_root = T::Hashing::storage_root();
-		let storage_changes_root = T::Hashing::storage_changes_root(parent_hash);
+		let storage_root = T::Hash::decode(&mut &runtime_io::storage::root()[..])
+			.expect("Node is configured to use the same hash; qed");
+		let storage_changes_root = runtime_io::storage::changes_root(&parent_hash.encode());
 
 		// we can't compute changes trie root earlier && put it to the Digest
 		// because it will include all currently existing temporaries.
 		if let Some(storage_changes_root) = storage_changes_root {
-			let item = generic::DigestItem::ChangesTrieRoot(storage_changes_root);
+			let item = generic::DigestItem::ChangesTrieRoot(
+				T::Hash::decode(&mut &storage_changes_root[..])
+					.expect("Node is configured to use the same hash; qed")
+			);
 			digest.push(item);
 		}
 

primitives/externalities/Cargo.toml

@@ -6,7 +6,6 @@ authors = ["Parity Technologies <admin@parity.io>"]
 edition = "2018"
 
 [dependencies]
-primitive-types = { version = "0.6", features = ["codec"] }
 primitives-storage = { package = "substrate-primitives-storage", path = "../core/storage" }
 rstd = { package = "sr-std", path = "../sr-std" }
 environmental = { version = "1.0.2" }

primitives/externalities/src/lib.rs

@@ -22,8 +22,6 @@
 //!
 //! This crate exposes the main [`Externalities`] trait.
 
-use primitive_types::H256;
-
 use std::any::{Any, TypeId};
 
 use primitives_storage::ChildStorageKey;
@@ -42,26 +40,40 @@ pub trait Externalities: ExtensionStore {
 	fn storage(&self, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Get storage value hash. This may be optimized for large values.
-	fn storage_hash(&self, key: &[u8]) -> Option<H256>;
+	fn storage_hash(&self, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Get child storage value hash. This may be optimized for large values.
-	fn child_storage_hash(&self, storage_key: ChildStorageKey, key: &[u8]) -> Option<H256>;
+	///
+	/// Returns an `Option` that holds the SCALE encoded hash.
+	fn child_storage_hash(&self, storage_key: ChildStorageKey, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Read original runtime storage, ignoring any overlayed changes.
 	fn original_storage(&self, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Read original runtime child storage, ignoring any overlayed changes.
+	///
+	/// Returns an `Option` that holds the SCALE encoded hash.
 	fn original_child_storage(&self, storage_key: ChildStorageKey, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Get original storage value hash, ignoring any overlayed changes.
 	/// This may be optimized for large values.
-	fn original_storage_hash(&self, key: &[u8]) -> Option<H256>;
+	///
+	/// Returns an `Option` that holds the SCALE encoded hash.
+	fn original_storage_hash(&self, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Get original child storage value hash, ignoring any overlayed changes.
 	/// This may be optimized for large values.
-	fn original_child_storage_hash(&self, storage_key: ChildStorageKey, key: &[u8]) -> Option<H256>;
+	///
+	/// Returns an `Option` that holds the SCALE encoded hash.
+	fn original_child_storage_hash(
+		&self,
+		storage_key: ChildStorageKey,
+		key: &[u8],
+	) -> Option<Vec<u8>>;
 
 	/// Read child runtime storage.
+	///
+	/// Returns an `Option` that holds the SCALE encoded hash.
 	fn child_storage(&self, storage_key: ChildStorageKey, key: &[u8]) -> Option<Vec<u8>>;
 
 	/// Set storage entry `key` of current contract being called (effective immediately).
@@ -107,14 +119,23 @@ pub trait Externalities: ExtensionStore {
 	fn place_storage(&mut self, key: Vec<u8>, value: Option<Vec<u8>>);
 
 	/// Set or clear a child storage entry. Return whether the operation succeeds.
-	fn place_child_storage(&mut self, storage_key: ChildStorageKey, key: Vec<u8>, value: Option<Vec<u8>>);
+	fn place_child_storage(
+		&mut self,
+		storage_key: ChildStorageKey,
+		key: Vec<u8>,
+		value: Option<Vec<u8>>,
+	);
 
 	/// Get the identity of the chain.
 	fn chain_id(&self) -> u64;
 
 	/// Get the trie root of the current storage map. This will also update all child storage keys
 	/// in the top-level storage map.
-	fn storage_root(&mut self) -> H256;
+	///
+	/// The hash is defined by the `Block`.
+	///
+	/// Returns the SCALE encoded hash.
+	fn storage_root(&mut self) -> Vec<u8>;
 
 	/// Get the trie root of a child storage map. This will also update the value of the child
 	/// storage keys in the top-level storage map.
@@ -123,7 +144,12 @@ pub trait Externalities: ExtensionStore {
 	fn child_storage_root(&mut self, storage_key: ChildStorageKey) -> Vec<u8>;
 
 	/// Get the change trie root of the current storage overlay at a block with given parent.
-	fn storage_changes_root(&mut self, parent: H256) -> Result<Option<H256>, ()>;
+	/// `parent` is expects a SCALE endcoded hash.
+	///
+	/// The hash is defined by the `Block`.
+	///
+	/// Returns the SCALE encoded hash.
+	fn storage_changes_root(&mut self, parent: &[u8]) -> Result<Option<Vec<u8>>, ()>;
 }
 
 /// Extension for the [`Externalities`] trait.

primitives/sr-io/src/lib.rs

@@ -50,7 +50,7 @@ use primitives::{
 };
 
 #[cfg(feature = "std")]
-use trie::{TrieConfiguration, trie_types::Layout};
+use ::trie::{TrieConfiguration, trie_types::Layout};
 
 use runtime_interface::{runtime_interface, Pointer};
 
@@ -186,28 +186,45 @@ pub trait Storage {
 	}
 
 	/// "Commit" all existing operations and compute the resulting storage root.
-	fn root(&mut self) -> H256 {
+	///
+	/// The hashing algorithm is defined by the `Block`.
+	///
+	/// Returns the SCALE encoded hash.
+	fn root(&mut self) -> Vec<u8> {
 		self.storage_root()
 	}
 
 	/// "Commit" all existing operations and compute the resulting child storage root.
+	///
+	/// The hashing algorithm is defined by the `Block`.
+	///
+	/// Returns the SCALE encoded hash.
 	fn child_root(&mut self, child_storage_key: &[u8]) -> Vec<u8> {
 		let storage_key = child_storage_key_or_panic(child_storage_key);
 		self.child_storage_root(storage_key)
 	}
 
 	/// "Commit" all existing operations and get the resulting storage change root.
-	fn changes_root(&mut self, parent_hash: [u8; 32]) -> Option<H256> {
-		self.storage_changes_root(parent_hash.into()).ok().and_then(|h| h)
+	/// `parent_hash` is a SCALE encoded hash.
+	///
+	/// The hashing algorithm is defined by the `Block`.
+	///
+	/// Returns an `Option` that holds the SCALE encoded hash.
+	fn changes_root(&mut self, parent_hash: &[u8]) -> Option<Vec<u8>> {
+		self.storage_changes_root(parent_hash).ok().and_then(|h| h)
 	}
+}
 
+/// Interface that provides trie related functionality.
+#[runtime_interface]
+pub trait Trie {
 	/// A trie root formed from the iterated items.
-	fn blake2_256_trie_root(input: Vec<(Vec<u8>, Vec<u8>)>) -> H256 {
+	fn blake2_256_root(input: Vec<(Vec<u8>, Vec<u8>)>) -> H256 {
 		Layout::<primitives::Blake2Hasher>::trie_root(input)
 	}
 
 	/// A trie root formed from the enumerated items.
-	fn blake2_256_ordered_trie_root(input: Vec<Vec<u8>>) -> H256 {
+	fn blake2_256_ordered_root(input: Vec<Vec<u8>>) -> H256 {
 		Layout::<primitives::Blake2Hasher>::ordered_trie_root(input)
 	}
 }
@@ -779,6 +796,7 @@ pub type SubstrateHostFunctions = (
 	allocator::HostFunctions,
 	logging::HostFunctions,
 	sandbox::HostFunctions,
+	crate::trie::HostFunctions,
 );
 
 #[cfg(test)]