Child trie api changes BREAKING (#4857)

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

substrate/client/src/client.rs

@@ -25,8 +25,8 @@ use parking_lot::{Mutex, RwLock};
 use codec::{Encode, Decode};
 use hash_db::Prefix;
 use sp_core::{
-	ChangesTrieConfiguration, convert_hash, traits::CodeExecutor,
-	NativeOrEncoded, storage::{StorageKey, StorageData, well_known_keys, ChildInfo},
+	ChangesTrieConfiguration, convert_hash, traits::CodeExecutor, NativeOrEncoded,
+	storage::{StorageKey, PrefixedStorageKey, StorageData, well_known_keys, ChildInfo},
 };
 use sc_telemetry::{telemetry, SUBSTRATE_INFO};
 use sp_runtime::{
@@ -344,7 +344,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		last: Block::Hash,
 		min: Block::Hash,
 		max: Block::Hash,
-		storage_key: Option<&StorageKey>,
+		storage_key: Option<&PrefixedStorageKey>,
 		key: &StorageKey,
 		cht_size: NumberFor<Block>,
 	) -> sp_blockchain::Result<ChangesProof<Block::Header>> {
@@ -393,7 +393,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			fn with_cached_changed_keys(
 				&self,
 				root: &Block::Hash,
-				functor: &mut dyn FnMut(&HashMap<Option<Vec<u8>>, HashSet<Vec<u8>>>),
+				functor: &mut dyn FnMut(&HashMap<Option<PrefixedStorageKey>, HashSet<Vec<u8>>>),
 			) -> bool {
 				self.storage.with_cached_changed_keys(root, functor)
 			}
@@ -438,7 +438,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 					number: last_number,
 				},
 				max_number,
-				storage_key.as_ref().map(|x| &x.0[..]),
+				storage_key,
 				&key.0,
 			)
 			.map_err(|err| sp_blockchain::Error::ChangesTrieAccessFailed(err))?;
@@ -1109,12 +1109,11 @@ impl<B, E, Block, RA> ProofProvider<Block> for Client<B, E, Block, RA> where
 	fn read_child_proof(
 		&self,
 		id: &BlockId<Block>,
-		storage_key: &[u8],
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		keys: &mut dyn Iterator<Item=&[u8]>,
 	) -> sp_blockchain::Result<StorageProof> {
 		self.state_at(id)
-			.and_then(|state| prove_child_read(state, storage_key, child_info, keys)
+			.and_then(|state| prove_child_read(state, child_info, keys)
 				.map_err(Into::into))
 	}
 
@@ -1156,7 +1155,7 @@ impl<B, E, Block, RA> ProofProvider<Block> for Client<B, E, Block, RA> where
 		last: Block::Hash,
 		min: Block::Hash,
 		max: Block::Hash,
-		storage_key: Option<&StorageKey>,
+		storage_key: Option<&PrefixedStorageKey>,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<ChangesProof<Block::Header>> {
 		self.key_changes_proof_with_cht_size(
@@ -1286,46 +1285,40 @@ impl<B, E, Block, RA> StorageProvider<Block, B> for Client<B, E, Block, RA> wher
 		)
 	}
 
-
 	fn child_storage_keys(
 		&self,
 		id: &BlockId<Block>,
-		child_storage_key: &StorageKey,
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		key_prefix: &StorageKey
 	) -> sp_blockchain::Result<Vec<StorageKey>> {
 		let keys = self.state_at(id)?
-			.child_keys(&child_storage_key.0, child_info, &key_prefix.0)
+			.child_keys(child_info, &key_prefix.0)
 			.into_iter()
 			.map(StorageKey)
 			.collect();
 		Ok(keys)
 	}
 
-
 	fn child_storage(
 		&self,
 		id: &BlockId<Block>,
-		storage_key: &StorageKey,
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		key: &StorageKey
 	) -> sp_blockchain::Result<Option<StorageData>> {
 		Ok(self.state_at(id)?
-			.child_storage(&storage_key.0, child_info, &key.0)
+			.child_storage(child_info, &key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?
 			.map(StorageData))
 	}
 
-
 	fn child_storage_hash(
 		&self,
 		id: &BlockId<Block>,
-		storage_key: &StorageKey,
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		key: &StorageKey
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
 		Ok(self.state_at(id)?
-			.child_storage_hash(&storage_key.0, child_info, &key.0)
+			.child_storage_hash(child_info, &key.0)
 			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?
 		)
 	}
@@ -1361,7 +1354,7 @@ impl<B, E, Block, RA> StorageProvider<Block, B> for Client<B, E, Block, RA> wher
 		&self,
 		first: NumberFor<Block>,
 		last: BlockId<Block>,
-		storage_key: Option<&StorageKey>,
+		storage_key: Option<&PrefixedStorageKey>,
 		key: &StorageKey
 	) -> sp_blockchain::Result<Vec<(NumberFor<Block>, u32)>> {
 		let last_number = self.backend.blockchain().expect_block_number_from_id(&last)?;
@@ -1392,7 +1385,7 @@ impl<B, E, Block, RA> StorageProvider<Block, B> for Client<B, E, Block, RA> wher
 				range_first,
 				&range_anchor,
 				best_number,
-				storage_key.as_ref().map(|x| &x.0[..]),
+				storage_key,
 				&key.0)
 				.and_then(|r| r.map(|r| r.map(|(block, tx)| (block, tx))).collect::<Result<_, _>>())
 				.map_err(|err| sp_blockchain::Error::ChangesTrieAccessFailed(err))?;

substrate/client/src/in_mem.rs

@@ -516,9 +516,9 @@ impl<Block: BlockT> backend::BlockImportOperation<Block> for BlockImportOperatio
 	fn reset_storage(&mut self, storage: Storage) -> sp_blockchain::Result<Block::Hash> {
 		check_genesis_storage(&storage)?;
 
-		let child_delta = storage.children.into_iter()
-			.map(|(storage_key, child_content)|
-				(storage_key, child_content.data.into_iter().map(|(k, v)| (k, Some(v))), child_content.child_info));
+		let child_delta = storage.children_default.into_iter()
+			.map(|(_storage_key, child_content)|
+				(child_content.child_info, child_content.data.into_iter().map(|(k, v)| (k, Some(v)))));
 
 		let (root, transaction) = self.old_state.full_storage_root(
 			storage.top.into_iter().map(|(k, v)| (k, Some(v))),
@@ -725,8 +725,9 @@ pub fn check_genesis_storage(storage: &Storage) -> sp_blockchain::Result<()> {
 		return Err(sp_blockchain::Error::GenesisInvalid.into());
 	}
 
-	if storage.children.keys().any(|child_key| !well_known_keys::is_child_storage_key(&child_key)) {
-		return Err(sp_blockchain::Error::GenesisInvalid.into());
+	if storage.children_default.keys()
+		.any(|child_key| !well_known_keys::is_child_storage_key(&child_key)) {
+			return Err(sp_blockchain::Error::GenesisInvalid.into());
 	}
 
 	Ok(())

substrate/client/src/light/backend.rs

@@ -24,7 +24,7 @@ use parking_lot::RwLock;
 use codec::{Decode, Encode};
 
 use sp_core::ChangesTrieConfiguration;
-use sp_core::storage::{well_known_keys, ChildInfo, OwnedChildInfo};
+use sp_core::storage::{well_known_keys, ChildInfo};
 use sp_core::offchain::storage::InMemOffchainStorage;
 use sp_state_machine::{
 	Backend as StateBackend, TrieBackend, InMemoryBackend, ChangesTrieTransaction,
@@ -312,17 +312,17 @@ impl<S, Block> BlockImportOperation<Block> for ImportOperation<Block, S>
 		self.changes_trie_config_update = Some(changes_trie_config);
 
 		// this is only called when genesis block is imported => shouldn't be performance bottleneck
-		let mut storage: HashMap<Option<(Vec<u8>, OwnedChildInfo)>, _> = HashMap::new();
+		let mut storage: HashMap<Option<ChildInfo>, _> = HashMap::new();
 		storage.insert(None, input.top);
 
 		// create a list of children keys to re-compute roots for
-		let child_delta = input.children.iter()
-			.map(|(storage_key, storage_child)| (storage_key.clone(), None, storage_child.child_info.clone()))
+		let child_delta = input.children_default.iter()
+			.map(|(_storage_key, storage_child)| (storage_child.child_info.clone(), None))
 			.collect::<Vec<_>>();
 
 		// make sure to persist the child storage
-		for (child_key, storage_child) in input.children {
-			storage.insert(Some((child_key, storage_child.child_info)), storage_child.data);
+		for (_child_key, storage_child) in input.children_default {
+			storage.insert(Some(storage_child.child_info), storage_child.data);
 		}
 
 		let storage_update = InMemoryBackend::from(storage);
@@ -386,13 +386,12 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 
 	fn child_storage(
 		&self,
-		storage_key: &[u8],
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		key: &[u8],
 	) -> ClientResult<Option<Vec<u8>>> {
 		match *self {
 			GenesisOrUnavailableState::Genesis(ref state) =>
-				Ok(state.child_storage(storage_key, child_info, key).expect(IN_MEMORY_EXPECT_PROOF)),
+				Ok(state.child_storage(child_info, key).expect(IN_MEMORY_EXPECT_PROOF)),
 			GenesisOrUnavailableState::Unavailable => Err(ClientError::NotAvailableOnLightClient),
 		}
 	}
@@ -407,13 +406,12 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 
 	fn next_child_storage_key(
 		&self,
-		storage_key: &[u8],
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		key: &[u8],
 	) -> Result<Option<Vec<u8>>, Self::Error> {
 		match *self {
 			GenesisOrUnavailableState::Genesis(ref state) => Ok(
-				state.next_child_storage_key(storage_key, child_info, key)
+				state.next_child_storage_key(child_info, key)
 					.expect(IN_MEMORY_EXPECT_PROOF)
 			),
 			GenesisOrUnavailableState::Unavailable => Err(ClientError::NotAvailableOnLightClient),
@@ -436,27 +434,25 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 
 	fn for_keys_in_child_storage<A: FnMut(&[u8])>(
 		&self,
-		storage_key: &[u8],
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		action: A,
 	) {
 		match *self {
 			GenesisOrUnavailableState::Genesis(ref state) =>
-				state.for_keys_in_child_storage(storage_key, child_info, action),
+				state.for_keys_in_child_storage(child_info, action),
 			GenesisOrUnavailableState::Unavailable => (),
 		}
 	}
 
 	fn for_child_keys_with_prefix<A: FnMut(&[u8])>(
 		&self,
-		storage_key: &[u8],
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		prefix: &[u8],
 		action: A,
 	) {
 		match *self {
 			GenesisOrUnavailableState::Genesis(ref state) =>
-				state.for_child_keys_with_prefix(storage_key, child_info, prefix, action),
+				state.for_child_keys_with_prefix(child_info, prefix, action),
 			GenesisOrUnavailableState::Unavailable => (),
 		}
 	}
@@ -474,8 +470,7 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 
 	fn child_storage_root<I>(
 		&self,
-		storage_key: &[u8],
-		child_info: ChildInfo,
+		child_info: &ChildInfo,
 		delta: I,
 	) -> (H::Out, bool, Self::Transaction)
 	where
@@ -483,7 +478,7 @@ impl<H: Hasher> StateBackend<H> for GenesisOrUnavailableState<H>
 	{
 		match *self {
 			GenesisOrUnavailableState::Genesis(ref state) => {
-				let (root, is_equal, _) = state.child_storage_root(storage_key, child_info, delta);
+				let (root, is_equal, _) = state.child_storage_root(child_info, delta);
 				(root, is_equal, Default::default())
 			},
 			GenesisOrUnavailableState::Unavailable =>

substrate/client/src/light/fetcher.rs

@@ -23,6 +23,7 @@ use std::marker::PhantomData;
 use hash_db::{HashDB, Hasher, EMPTY_PREFIX};
 use codec::{Decode, Encode};
 use sp_core::{convert_hash, traits::CodeExecutor};
+use sp_core::storage::{ChildInfo, ChildType};
 use sp_runtime::traits::{
 	Block as BlockT, Header as HeaderT, Hash, HashFor, NumberFor,
 	AtLeast32Bit, CheckedConversion,
@@ -135,7 +136,7 @@ impl<E, H, B: BlockT, S: BlockchainStorage<B>> LightDataChecker<E, H, B, S> {
 					number: request.last_block.0,
 				},
 				remote_max_block,
-				request.storage_key.as_ref().map(Vec::as_slice),
+				request.storage_key.as_ref(),
 				&request.key)
 			.map_err(|err| ClientError::ChangesTrieAccessFailed(err))?;
 			result.extend(result_range);
@@ -242,10 +243,14 @@ impl<E, Block, H, S> FetchChecker<Block> for LightDataChecker<E, H, Block, S>
 		request: &RemoteReadChildRequest<Block::Header>,
 		remote_proof: StorageProof,
 	) -> ClientResult<HashMap<Vec<u8>, Option<Vec<u8>>>> {
+		let child_info = match ChildType::from_prefixed_key(&request.storage_key) {
+			Some((ChildType::ParentKeyId, storage_key)) => ChildInfo::new_default(storage_key),
+			None => return Err("Invalid child type".into()),
+		};
 		read_child_proof_check::<H, _>(
 			convert_hash(request.header.state_root()),
 			remote_proof,
-			&request.storage_key,
+			&child_info,
 			request.keys.iter(),
 		).map_err(Into::into)
 	}
@@ -360,8 +365,6 @@ pub mod tests {
 	use sc_client_api::{StorageProvider, ProofProvider};
 	use sc_block_builder::BlockBuilderProvider;
 
-	const CHILD_INFO_1: ChildInfo<'static> = ChildInfo::new_default(b"unique_id_1");
-
 	type TestChecker = LightDataChecker<
 		NativeExecutor<substrate_test_runtime_client::LocalExecutor>,
 		BlakeTwo256,
@@ -411,11 +414,12 @@ pub mod tests {
 	fn prepare_for_read_child_proof_check() -> (TestChecker, Header, StorageProof, Vec<u8>) {
 		use substrate_test_runtime_client::DefaultTestClientBuilderExt;
 		use substrate_test_runtime_client::TestClientBuilderExt;
+		let child_info = ChildInfo::new_default(b"child1");
+		let child_info = &child_info;
 		// prepare remote client
 		let remote_client = substrate_test_runtime_client::TestClientBuilder::new()
 			.add_extra_child_storage(
-				b":child_storage:default:child1".to_vec(),
-				CHILD_INFO_1,
+				child_info,
 				b"key1".to_vec(),
 				b"value1".to_vec(),
 			).build();
@@ -428,15 +432,13 @@ pub mod tests {
 		// 'fetch' child read proof from remote node
 		let child_value = remote_client.child_storage(
 			&remote_block_id,
-			&StorageKey(b":child_storage:default:child1".to_vec()),
-			CHILD_INFO_1,
+			child_info,
 			&StorageKey(b"key1".to_vec()),
 		).unwrap().unwrap().0;
 		assert_eq!(b"value1"[..], child_value[..]);
 		let remote_read_proof = remote_client.read_child_proof(
 			&remote_block_id,
-			b":child_storage:default:child1",
-			CHILD_INFO_1,
+			child_info,
 			&mut std::iter::once("key1".as_bytes()),
 		).unwrap();
 
@@ -510,20 +512,18 @@ pub mod tests {
 
 	#[test]
 	fn storage_child_read_proof_is_generated_and_checked() {
+		let child_info = ChildInfo::new_default(&b"child1"[..]);
 		let (
 			local_checker,
 			remote_block_header,
 			remote_read_proof,
 			result,
 		) = prepare_for_read_child_proof_check();
-		let child_infos = CHILD_INFO_1.info();
 		assert_eq!((&local_checker as &dyn FetchChecker<Block>).check_read_child_proof(
 			&RemoteReadChildRequest::<Header> {
 				block: remote_block_header.hash(),
 				header: remote_block_header,
-				storage_key: b":child_storage:default:child1".to_vec(),
-				child_info: child_infos.0.to_vec(),
-				child_type: child_infos.1,
+				storage_key: child_info.prefixed_storage_key(),
 				keys: vec![b"key1".to_vec()],
 				retry_count: None,
 			},