Merge branch 'master' into child-trie-soft-min and

state prefix and kvdb prefix confusion fix:
- revert change on state prefix
- add default db prefix on all keyvalue (break db exept if using legacy feature, but then child trie are not usable).
- child prefix got a first byte to differ from default

core/client/db/src/lib.rs

@@ -440,7 +440,7 @@ where Block: BlockT<Hash=H256>,
 		children: ChildrenStorageOverlay
 	) -> Result<H256, client::error::Error> {
 
-		if top.iter().any(|(k, _)| well_known_keys::is_protected_storage_content(k)) {
+		if top.iter().any(|(k, _)| well_known_keys::is_child_storage_key(k)) {
 			return Err(client::error::Error::GenesisInvalid.into());
 		}
 
@@ -1404,7 +1404,7 @@ where Block: BlockT<Hash=H256> {}
 
 #[cfg(test)]
 mod tests {
-	use hash_db::HashDB;
+	use hash_db::{HashDB, EMPTY_PREFIX};
 	use super::*;
 	use crate::columns;
 	use client::backend::Backend as BTrait;
@@ -1640,7 +1640,10 @@ mod tests {
 
 			backend.commit_operation(op).unwrap();
 
-			assert_eq!(backend.storage.db.get(columns::STATE, key.as_bytes()).unwrap().unwrap(), &b"hello"[..]);
+			assert_eq!(backend.storage.db.get(
+				columns::STATE,
+				&trie::prefixed_key::<Blake2Hasher>(&key, EMPTY_PREFIX)
+			).unwrap().unwrap(), &b"hello"[..]);
 			hash
 		};
 
@@ -1674,8 +1677,10 @@ mod tests {
 			).unwrap();
 
 			backend.commit_operation(op).unwrap();
-
-			assert_eq!(backend.storage.db.get(columns::STATE, key.as_bytes()).unwrap().unwrap(), &b"hello"[..]);
+			assert_eq!(backend.storage.db.get(
+				columns::STATE,
+				&trie::prefixed_key::<Blake2Hasher>(&key, EMPTY_PREFIX)
+			).unwrap().unwrap(), &b"hello"[..]);
 			hash
 		};
 
@@ -1708,8 +1713,10 @@ mod tests {
 			).unwrap();
 
 			backend.commit_operation(op).unwrap();
-
-			assert!(backend.storage.db.get(columns::STATE, key.as_bytes()).unwrap().is_some());
+			assert!(backend.storage.db.get(
+				columns::STATE,
+				&trie::prefixed_key::<Blake2Hasher>(&key, EMPTY_PREFIX)
+			).unwrap().is_some());
 			hash
 		};
 
@@ -1740,14 +1747,19 @@ mod tests {
 			).unwrap();
 
 			backend.commit_operation(op).unwrap();
-
-			assert!(backend.storage.db.get(columns::STATE, key.as_bytes()).unwrap().is_none());
+			assert!(backend.storage.db.get(
+				columns::STATE,
+				&trie::prefixed_key::<Blake2Hasher>(&key, EMPTY_PREFIX)
+			).unwrap().is_none())
 		}
 
 		backend.finalize_block(BlockId::Number(1), None).unwrap();
 		backend.finalize_block(BlockId::Number(2), None).unwrap();
 		backend.finalize_block(BlockId::Number(3), None).unwrap();
-		assert!(backend.storage.db.get(columns::STATE, key.as_bytes()).unwrap().is_none());
+		assert!(backend.storage.db.get(
+			columns::STATE,
+			&trie::prefixed_key::<Blake2Hasher>(&key, EMPTY_PREFIX)
+		).unwrap().is_none());
 	}
 
 	#[test]

core/client/src/in_mem.rs

@@ -757,12 +757,12 @@ impl<Block, H> state_machine::ChangesTrieStorage<H, NumberFor<Block>> for Change
 
 /// Check that genesis storage is valid.
 pub fn check_genesis_storage(top: &StorageOverlay, children: &ChildrenStorageOverlay) -> error::Result<()> {
-	if top.iter().any(|(k, _)| well_known_keys::is_protected_storage_content(k)) {
+	if top.iter().any(|(k, _)| well_known_keys::is_child_storage_key(k)) {
 		return Err(error::Error::GenesisInvalid.into());
 	}
 
 	debug_assert!(!children.iter()
-		.any(|(_, (_, subtrie))| !well_known_keys::is_protected_storage_content(&subtrie.parent_trie()[..])));
+		.any(|(_, (_, subtrie))| !well_known_keys::is_child_storage_key(&subtrie.parent_trie()[..])));
 
 	Ok(())
 }

core/primitives/src/child_trie.rs

@@ -20,7 +20,6 @@ use parity_codec::{Encode, Decode};
 use rstd::prelude::*;
 use rstd::ptr;
 use crate::storage::well_known_keys::CHILD_STORAGE_KEY_PREFIX;
-use crate::storage::well_known_keys::CHILD_STORAGE_CONTENT_PREFIX;
 #[cfg(feature = "std")]
 pub use impl_serde::serialize as bytes;
 
@@ -31,38 +30,55 @@ pub use impl_serde::serialize as bytes;
 /// this child trie.
 /// The `KeySpace` of a child trie must be unique for the canonical chain
 /// in order to avoid key collision at a key value database level.
-/// This is currently build by using the `ParentTrie` path of the child trie
-/// at its creation and the block number at its creation (a child trie
-/// when new if moved must therefore update its keyspace).
+/// Child trie variant (start with 1u8), is currently build by using the `ParentTrie`
+/// path of the child trie at its creation and the block number at its creation
+/// (only child trie in their creation block need to update this value when moved).
+/// No keyspace variant is only 0u8.
 /// This id is unique as for a block number state we cannot have
 /// two created child trie with the same `ParentTrie`.
 pub type KeySpace = Vec<u8>;
 
-/// Parent trie origin. This type contains all information
-/// needed to access a parent trie.
-/// Currently only a single depth is supported for child trie,
-/// so it only contains the top trie key to the child trie.
-/// Internally this contains a full path key (with
-/// `well_known_keys::CHILD_STORAGE_KEY_PREFIX`).
-pub type ParentTrie = Vec<u8>;
+
+/// Keyspace to use for the parent trie key.
+pub const NO_CHILD_KEYSPACE: [u8;1] = [0u8];
+const CHILD_KEYSPACE_HEAD: u8 = 1;
+
+// see FIXME #2741 for removal of this allocation on every operation.
+// (the PrefixedDB one is enough). Method could be to pass mutable
+// vector to `KeyFunction` and have an new key function to estimate
+// required additional allocation size.
+/// Generate a new keyspace for a child trie.
+pub fn generate_keyspace<N: Encode>(block_nb: &N, parent_trie: &ParentTrie) -> Vec<u8> {
+	// using 9 for block number and additional encoding targeting ~u64
+	let mut result = Vec::with_capacity(parent_trie.len() + 9);
+	result.push(CHILD_KEYSPACE_HEAD); // 1 first value if there is a keyspace (
+	parity_codec::Encode::encode_to(ChildTrie::parent_key_slice(parent_trie), &mut result);
+	parity_codec::Encode::encode_to(block_nb, &mut result);
+	result
+}
 
 /// Utility function used for merging `KeySpace` data and `prefix` data
 /// before calling key value database primitives.
 /// Note that it currently does a costy append operation resulting in bigger
 /// key length but possibly allowing prefix related operation at lower level.
-/// Note that we also append a common prefix to avoid general state key to
-/// conflict (if the KeySpace gets build from a strong cryptographic hash
-/// in the future, or if general state get written with empty keyspace,
-/// this would be useless).
 pub fn keyspace_as_prefix_alloc(ks: &KeySpace, prefix: &[u8]) -> Vec<u8> {
-	let pre = CHILD_STORAGE_CONTENT_PREFIX;
-	let mut res = rstd::vec![0; pre.len() + ks.len() + prefix.len()];
-	res[..pre.len()].copy_from_slice(&pre);
-	res[pre.len()..pre.len() + ks.len()].copy_from_slice(&ks);
-	res[pre.len() + ks.len()..].copy_from_slice(prefix);
+	let n_pre_len = NO_CHILD_KEYSPACE.len();
+	// only use on prefixed db where NO_CHILD_KEYSPACE is added.
+	debug_assert!(prefix.len() >= n_pre_len);
+	let mut res = rstd::vec![0; ks.len() + prefix.len() - n_pre_len];
+	res[..ks.len()].copy_from_slice(&ks);
+	res[ks.len()..].copy_from_slice(&prefix[n_pre_len..]);
 	res
 }
 
+/// Parent trie origin. This type contains all information
+/// needed to access a parent trie.
+/// Currently only a single depth is supported for child trie,
+/// so it only contains the top trie key to the child trie.
+/// Internally this contains a full path key (with
+/// `well_known_keys::CHILD_STORAGE_KEY_PREFIX`).
+pub type ParentTrie = Vec<u8>;
+
 /// `ChildTrieReadRef` contains a reference to information
 /// needed to access a child trie content.
 /// Generally this should not be build directly but accessed
@@ -339,12 +355,3 @@ impl AsRef<ChildTrie> for ChildTrie {
 		self
 	}
 }
-
-/// generate a new keyspace
-pub fn generate_keyspace<N: Encode>(block_nb: &N, parent_trie: &ParentTrie) -> Vec<u8> {
-	// using 8 for block number and additianal encoding targeting ~u64
-	let mut result = Vec::with_capacity(parent_trie.len() + 8);
-	parity_codec::Encode::encode_to(ChildTrie::parent_key_slice(parent_trie), &mut result);
-	parity_codec::Encode::encode_to(block_nb, &mut result);
-	result
-}

core/primitives/src/storage.rs

@@ -64,25 +64,16 @@ pub mod well_known_keys {
 	/// Changes trie configuration is stored under this key.
 	pub const CHANGES_TRIE_CONFIG: &'static [u8] = b":changes_trie";
 
-	/// Prefix of child storage content.
-	const PROTECTED_STORAGE_PREFIX: &'static [u8] = b":child_storage:";
-
-	/// Prefix of child storage keys.
-	/// This contains the `PROTECTED_STORAGE_PREFIX`. Then utf8 encoded 0.
-	pub const CHILD_STORAGE_KEY_PREFIX: &'static [u8] = b":child_storage:0";
-
 	/// Prefix of child storage keys.
-	/// This contains the `PROTECTED_STORAGE_PREFIX`. Then utf8 encoded 1.
-	pub const CHILD_STORAGE_CONTENT_PREFIX: &'static [u8] = b":child_storage:1";
-
+	pub const CHILD_STORAGE_KEY_PREFIX: &'static [u8] = b":child_storage:";
 
-	/// Whether a key is a protected content.
+	/// Whether a key is a child storage key.
 	///
 	/// This is convenience function which basically checks if the given `key` starts
-	/// with `PROTECTED_STORAGE_PREFIX` and doesn't do anything apart from that.
-	pub fn is_protected_storage_content(key: &[u8]) -> bool {
+	/// with `CHILD_STORAGE_KEY_PREFIX` and doesn't do anything apart from that.
+	pub fn is_child_storage_key(key: &[u8]) -> bool {
 		// Other code might depend on this, so be careful changing this.
-		key.starts_with(PROTECTED_STORAGE_PREFIX)
+		key.starts_with(CHILD_STORAGE_KEY_PREFIX)
 	}
 
 }

core/rpc/src/state/tests.rs

@@ -56,7 +56,7 @@ fn should_return_child_storage() {
 		.add_child_storage("test", "key", vec![42_u8])
 		.build());
 	let genesis_hash = client.genesis_hash();
-	let client = State::new(client, Subscriptions::new(core.executor()));
+	let client = State::new(client, Subscriptions::new(Arc::new(core.executor())));
 	let child_key = StorageKey(b"test".to_vec());
 	let key = StorageKey(b"key".to_vec());
 

core/state-machine/src/ext.rs

@@ -23,7 +23,7 @@ use crate::changes_trie::{Storage as ChangesTrieStorage, compute_changes_trie_ro
 use crate::{Externalities, OverlayedChanges};
 use hash_db::Hasher;
 use primitives::offchain;
-use primitives::storage::well_known_keys::is_protected_storage_content;
+use primitives::storage::well_known_keys::is_child_storage_key;
 use primitives::child_trie::ChildTrie;
 use primitives::child_trie::ChildTrieReadRef;
 use trie::{MemoryDB, TrieDBMut, TrieMut, default_child_trie_root};
@@ -226,7 +226,7 @@ where
 
 	fn place_storage(&mut self, key: Vec<u8>, value: Option<Vec<u8>>) {
 		let _guard = panic_handler::AbortGuard::new(true);
-		if is_protected_storage_content(&key) {
+		if is_child_storage_key(&key) {
 			warn!(target: "trie", "Refuse to directly set child storage key");
 			return;
 		}
@@ -275,7 +275,7 @@ where
 
 	fn clear_prefix(&mut self, prefix: &[u8]) {
 		let _guard = panic_handler::AbortGuard::new(true);
-		if is_protected_storage_content(prefix) {
+		if is_child_storage_key(prefix) {
 			warn!(target: "trie", "Refuse to directly clear prefix that is part of child storage key");
 			return;
 		}

core/state-machine/src/lib.rs

@@ -1281,13 +1281,13 @@ mod tests {
 		};
 
 		let mut set1 = HashSet::new();
-		tr1.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
+		tr1.0.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
 			set1.remove(&i);
 		} else {
 			set1.insert(i);
 		});
 		let mut set2 = HashSet::new();
-		tr2.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
+		tr2.0.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
 			set2.remove(&i);
 		} else {
 			set2.insert(i);
@@ -1335,13 +1335,13 @@ mod tests {
 			ext.transaction().0
 		};
 		let mut set1 = HashSet::new();
-		tr1.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
+		tr1.0.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
 			set1.remove(&i);
 		} else {
 			set1.insert(i);
 		});
 		let mut set2 = HashSet::new();
-		tr2.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
+		tr2.0.drain().into_iter().for_each(|(i, (_,rc))| if rc == -1i32 {
 			set2.remove(&i);
 		} else {
 			set2.insert(i);

core/state-machine/src/testing.rs

@@ -20,7 +20,7 @@ use std::collections::{HashMap, BTreeMap};
 use std::iter::FromIterator;
 use hash_db::Hasher;
 use crate::backend::{InMemory, Backend};
-use primitives::storage::well_known_keys::is_protected_storage_content;
+use primitives::storage::well_known_keys::is_child_storage_key;
 use crate::changes_trie::{
 	compute_changes_trie_root, InMemoryStorage as ChangesTrieInMemoryStorage,
 	BlockNumber as ChangesTrieBlockNumber,
@@ -180,7 +180,7 @@ impl<H, N> Externalities<H> for TestExternalities<H, N>
 	}
 
 	fn place_storage(&mut self, key: Vec<u8>, maybe_value: Option<Vec<u8>>) {
-		if is_protected_storage_content(&key) {
+		if is_child_storage_key(&key) {
 			panic!("Refuse to directly set child storage key");
 		}
 
@@ -207,7 +207,7 @@ impl<H, N> Externalities<H> for TestExternalities<H, N>
 	}
 
 	fn clear_prefix(&mut self, prefix: &[u8]) {
-		if is_protected_storage_content(prefix) {
+		if is_child_storage_key(prefix) {
 			panic!("Refuse to directly clear prefix that is part of child storage key");
 		}
 

core/trie/Cargo.toml

@@ -29,6 +29,9 @@ hex-literal = "0.2.0"
 
 [features]
 default = ["std"]
+# use legacy trie implementation
+# not compatible with child trie
+legacy-trie = []
 std = [
 	"rstd/std",
 	"codec/std",