add block hashes to the randomness used by hashmaps and friends in validation context (#9127)

#8606
paritytech/trie#221 replaced the usage of
BTreeMap with HashMaps in validation context. The keys are already
derived with a cryptographic hash function from user data, so users
should not be able to manipulate it.

To be on safe side this PR also modifies the TrieCache, TrieRecorder and
MemoryDB to use a hasher that on top of the default generated randomness
also adds randomness generated from the hash of the relaychain and that
of the parachain blocks, which is not something users can control or
guess ahead of time.

---------

Signed-off-by: Alexandru Gheorghe <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 7058819)

Author: 3 people

Cargo.lock

@@ -801,6 +801,7 @@ first-pallet = { package = "polkadot-sdk-docs-first-pallet", path = "docs/sdk/pa
 first-runtime = { package = "polkadot-sdk-docs-first-runtime", path = "docs/sdk/packages/guides/first-runtime", default-features = false }
 flate2 = { version = "1.0" }
 fnv = { version = "1.0.6" }
+foldhash = { version = "0.1.5", default-features = false }
 fork-tree = { path = "substrate/utils/fork-tree", default-features = false }
 forwarded-header-value = { version = "0.1.1" }
 fraction = { version = "0.13.1" }
@@ -895,7 +896,7 @@ log = { version = "0.4.22", default-features = false }
 macro_magic = { version = "0.5.1" }
 maplit = { version = "1.0.2" }
 memmap2 = { version = "0.9.3" }
-memory-db = { version = "0.33.0", default-features = false }
+memory-db = { version = "0.34.0", default-features = false }
 merkleized-metadata = { version = "0.5.0" }
 merlin = { version = "3.0", default-features = false }
 messages-relay = { path = "bridges/relays/messages" }
@@ -1425,7 +1426,7 @@ tracing-futures = { version = "0.2.4" }
 tracing-log = { version = "0.2.0" }
 tracing-subscriber = { version = "0.3.18" }
 tracking-allocator = { path = "polkadot/node/tracking-allocator", default-features = false, package = "staging-tracking-allocator" }
-trie-bench = { version = "0.41.0" }
+trie-bench = { version = "0.42.0" }
 trie-db = { version = "0.30.0", default-features = false }
 trie-root = { version = "0.18.0", default-features = false }
 trie-standardmap = { version = "0.16.0" }

Cargo.toml

@@ -36,10 +36,11 @@ use frame_support::{
 };
 use sp_core::storage::{ChildInfo, StateVersion};
 use sp_externalities::{set_and_run_with_externalities, Externalities};
-use sp_io::KillStorageResult;
+use sp_io::{hashing::blake2_128, KillStorageResult};
 use sp_runtime::traits::{
 	Block as BlockT, ExtrinsicCall, ExtrinsicLike, HashingFor, Header as HeaderT,
 };
+
 use sp_state_machine::OverlayedChanges;
 use sp_trie::ProofSizeProvider;
 use trie_recorder::SizeOnlyRecorderProvider;
@@ -142,6 +143,12 @@ where
 	let block_data = codec::decode_from_bytes::<ParachainBlockData<B>>(block_data)
 		.expect("Invalid parachain block data");
 
+	// Initialize hashmaps randomness.
+	sp_trie::add_extra_randomness(build_seed_from_head_data(
+		&block_data,
+		relay_parent_storage_root,
+	));
+
 	let mut parent_header =
 		codec::decode_from_bytes::<B::Header>(parachain_head.clone()).expect("Invalid parent head");
 
@@ -393,6 +400,27 @@ fn validate_validation_data(
 	);
 }
 
+/// Build a seed from the head data of the parachain block.
+///
+/// Uses both the relay parent storage root and the hash of the blocks
+/// in the block data, to make sure the seed changes every block and that
+/// the user cannot find about it ahead of time.
+fn build_seed_from_head_data<B: BlockT>(
+	block_data: &ParachainBlockData<B>,
+	relay_parent_storage_root: crate::relay_chain::Hash,
+) -> [u8; 16] {
+	let mut bytes_to_hash = Vec::with_capacity(
+		block_data.blocks().len() * size_of::<B::Hash>() + size_of::<crate::relay_chain::Hash>(),
+	);
+
+	bytes_to_hash.extend_from_slice(relay_parent_storage_root.as_ref());
+	block_data.blocks().iter().for_each(|block| {
+		bytes_to_hash.extend_from_slice(block.header().hash().as_ref());
+	});
+
+	blake2_128(&bytes_to_hash)
+}
+
 /// Run the given closure with the externalities and recorder set.
 fn run_with_externalities_and_recorder<Block: BlockT, R, F: FnOnce() -> R>(
 	backend: &impl sp_state_machine::Backend<HashingFor<Block>>,

cumulus/pallets/parachain-system/src/validate_block/implementation.rs

@@ -20,15 +20,15 @@ use alloc::boxed::Box;
 use core::cell::{RefCell, RefMut};
 use hashbrown::{hash_map::Entry, HashMap};
 use sp_state_machine::TrieCacheProvider;
-use sp_trie::NodeCodec;
+use sp_trie::{NodeCodec, RandomState};
 use trie_db::{node::NodeOwned, Hasher};
 
 /// Special purpose trie cache implementation that is able to cache an unlimited number
 /// of values. To be used in `validate_block` to serve values and nodes that
 /// have already been loaded and decoded from the storage proof.
 pub struct TrieCache<'a, H: Hasher> {
-	node_cache: RefMut<'a, HashMap<H::Out, NodeOwned<H::Out>>>,
-	value_cache: Option<RefMut<'a, HashMap<Box<[u8]>, trie_db::CachedValue<H::Out>>>>,
+	node_cache: RefMut<'a, HashMap<H::Out, NodeOwned<H::Out>, RandomState>>,
+	value_cache: Option<RefMut<'a, HashMap<Box<[u8]>, trie_db::CachedValue<H::Out>, RandomState>>>,
 }
 
 impl<'a, H: Hasher> trie_db::TrieCache<NodeCodec<H>> for TrieCache<'a, H> {
@@ -65,14 +65,16 @@ impl<'a, H: Hasher> trie_db::TrieCache<NodeCodec<H>> for TrieCache<'a, H> {
 
 /// Provider of [`TrieCache`] instances.
 pub struct CacheProvider<H: Hasher> {
-	node_cache: RefCell<HashMap<H::Out, NodeOwned<H::Out>>>,
+	node_cache: RefCell<HashMap<H::Out, NodeOwned<H::Out>, RandomState>>,
 	/// Cache: `storage_root` => `storage_key` => `value`.
 	///
 	/// One `block` can for example use multiple tries (child tries) and we need to distinguish the
 	/// cached (`storage_key`, `value`) between them. For this we are using the `storage_root` to
 	/// distinguish them (even if the storage root is the same for two child tries, it just means
 	/// that both are exactly the same trie and there would happen no collision).
-	value_cache: RefCell<HashMap<H::Out, HashMap<Box<[u8]>, trie_db::CachedValue<H::Out>>>>,
+	value_cache: RefCell<
+		HashMap<H::Out, HashMap<Box<[u8]>, trie_db::CachedValue<H::Out>, RandomState>, RandomState>,
+	>,
 }
 
 impl<H: Hasher> CacheProvider<H> {

cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs

@@ -26,17 +26,17 @@ use alloc::rc::Rc;
 
 use core::cell::{RefCell, RefMut};
 use hashbrown::{HashMap, HashSet};
-use sp_trie::{NodeCodec, ProofSizeProvider, StorageProof};
+use sp_trie::{NodeCodec, ProofSizeProvider, RandomState, StorageProof};
 use trie_db::{Hasher, RecordedForKey, TrieAccess};
 
 /// A trie recorder that only keeps track of the proof size.
 ///
 /// The internal size counting logic should align
 /// with ['sp_trie::recorder::Recorder'].
 pub struct SizeOnlyRecorder<'a, H: Hasher> {
-	seen_nodes: RefMut<'a, HashSet<H::Out>>,
+	seen_nodes: RefMut<'a, HashSet<H::Out, RandomState>>,
 	encoded_size: RefMut<'a, usize>,
-	recorded_keys: RefMut<'a, HashMap<Rc<[u8]>, RecordedForKey>>,
+	recorded_keys: RefMut<'a, HashMap<Rc<[u8]>, RecordedForKey, RandomState>>,
 }
 
 impl<'a, H: trie_db::Hasher> trie_db::TrieRecorder<H::Out> for SizeOnlyRecorder<'a, H> {
@@ -90,9 +90,9 @@ impl<'a, H: trie_db::Hasher> trie_db::TrieRecorder<H::Out> for SizeOnlyRecorder<
 
 #[derive(Clone)]
 pub struct SizeOnlyRecorderProvider<H: Hasher> {
-	seen_nodes: Rc<RefCell<HashSet<H::Out>>>,
+	seen_nodes: Rc<RefCell<HashSet<H::Out, RandomState>>>,
 	encoded_size: Rc<RefCell<usize>>,
-	recorded_keys: Rc<RefCell<HashMap<Rc<[u8]>, RecordedForKey>>>,
+	recorded_keys: Rc<RefCell<HashMap<Rc<[u8]>, RecordedForKey, RandomState>>>,
 }
 
 impl<H: Hasher> Default for SizeOnlyRecorderProvider<H> {

cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs

@@ -0,0 +1,25 @@
+title: add block hashes to the randomness used by hashmaps and friends in validation
+  context
+doc:
+- audience: Node Dev
+  description: |-
+    https://github.com/paritytech/polkadot-sdk/pull/8606 https://github.com/paritytech/trie/pull/221 replaced the usage of BTreeMap with HashMaps in validation context. The keys are already derived with a cryptographic hash function from user data, so users should not be able to manipulate it.
+
+    To be on safe side this PR also modifies the TrieCache, TrieRecorder and MemoryDB to use a hasher that on top of the default generated randomness also adds randomness generated from the hash of the relaychain and that of the parachain blocks, which is not something users can control or guess ahead of time.
+crates:
+- name: bridge-runtime-common
+  bump: minor
+- name: pallet-bridge-messages
+  bump: minor
+- name: bp-test-utils
+  bump: minor
+- name: cumulus-pallet-parachain-system
+  bump: minor
+- name: sp-state-machine
+  bump: minor
+- name: sp-trie
+  bump: minor
+- name: pallet-session
+  bump: minor
+- name: sp-runtime
+  bump: minor

prdoc/pr_9127.prdoc

@@ -41,7 +41,7 @@ use sp_session::{MembershipProof, ValidatorCount};
 use sp_staking::SessionIndex;
 use sp_trie::{
 	trie_types::{TrieDBBuilder, TrieDBMutBuilderV0},
-	LayoutV0, MemoryDB, Recorder, StorageProof, Trie, TrieMut, TrieRecorder,
+	LayoutV0, MemoryDB, RandomState, Recorder, StorageProof, Trie, TrieMut, TrieRecorder,
 };
 
 use frame_support::{
@@ -264,7 +264,7 @@ impl<T: Config> ProvingTrie<T> {
 	where
 		I: IntoIterator<Item = (T::ValidatorId, T::FullIdentification)>,
 	{
-		let mut db = MemoryDB::default();
+		let mut db = MemoryDB::with_hasher(RandomState::default());
 		let mut root = Default::default();
 
 		{

substrate/frame/session/src/historical/mod.rs

@@ -30,7 +30,7 @@ use alloc::vec::Vec;
 use codec::MaxEncodedLen;
 use sp_trie::{
 	trie_types::{TrieDBBuilder, TrieDBMutBuilderV1},
-	LayoutV1, MemoryDB, Trie, TrieMut,
+	LayoutV1, MemoryDB, RandomState, Trie, TrieMut,
 };
 
 /// A helper structure for building a basic base-16 merkle trie and creating compact proofs for that
@@ -77,7 +77,7 @@ where
 	where
 		I: IntoIterator<Item = (Key, Value)>,
 	{
-		let mut db = MemoryDB::default();
+		let mut db = MemoryDB::with_hasher(RandomState::default());
 		let mut root = Default::default();
 
 		{

substrate/primitives/runtime/src/proving_trie/base16.rs

@@ -30,7 +30,7 @@ use hash_db::Hasher;
 use sp_core::storage::{ChildInfo, StateVersion, TrackedStorageKey};
 #[cfg(feature = "std")]
 use sp_core::traits::RuntimeCode;
-use sp_trie::{MerkleValue, PrefixedMemoryDB};
+use sp_trie::{MerkleValue, PrefixedMemoryDB, RandomState};
 
 /// A struct containing arguments for iterating over the storage.
 #[derive(Default)]
@@ -301,7 +301,7 @@ pub trait Backend<H: Hasher>: core::fmt::Debug {
 	where
 		H::Out: Ord + Encode,
 	{
-		let mut txs = BackendTransaction::default();
+		let mut txs = BackendTransaction::with_hasher(RandomState::default());
 		let mut child_roots: Vec<_> = Default::default();
 		// child first
 		for (child_info, child_delta) in child_deltas {

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

@@ -25,7 +25,7 @@ use alloc::{collections::BTreeMap, vec::Vec};
 use codec::Codec;
 use hash_db::Hasher;
 use sp_core::storage::{ChildInfo, StateVersion, Storage};
-use sp_trie::{empty_trie_root, LayoutV1, PrefixedMemoryDB};
+use sp_trie::{empty_trie_root, LayoutV1, PrefixedMemoryDB, RandomState};
 
 #[cfg(feature = "std")]
 use std::collections::HashMap as MapType;
@@ -40,7 +40,11 @@ where
 	H::Out: Codec + Ord,
 {
 	// V1 is same as V0 for an empty trie.
-	TrieBackendBuilder::new(Default::default(), empty_trie_root::<LayoutV1<H>>()).build()
+	TrieBackendBuilder::new(
+		PrefixedMemoryDB::with_hasher(RandomState::default()),
+		empty_trie_root::<LayoutV1<H>>(),
+	)
+	.build()
 }
 
 impl<H: Hasher> TrieBackend<PrefixedMemoryDB<H>, H>