avoid key collision on child trie and proof on child trie

core/primitives/src/child_trie.rs

@@ -19,8 +19,6 @@
 use parity_codec::{Encode, Decode, Compact};
 use rstd::prelude::*;
 use rstd::ptr;
-use rstd::ops::Sub;
-use rstd::convert::{TryInto, TryFrom};
 use crate::storage::well_known_keys::CHILD_STORAGE_KEY_PREFIX;
 #[cfg(feature = "std")]
 pub use impl_serde::serialize as bytes;
@@ -33,55 +31,23 @@ use hash_db::Hasher;
 /// 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.
-/// 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`.
+/// This id is unique is currently build from a simple counter in state.
 pub type KeySpace = Vec<u8>;
 
 
 #[cfg(not(feature = "legacy-trie"))]
 /// Keyspace to use for the parent trie key.
-/// Block number 0 and no path.
-pub const NO_CHILD_KEYSPACE: [u8;2] = [0, 0];
+pub const NO_CHILD_KEYSPACE: [u8;1] = [0];
 #[cfg(feature = "legacy-trie")]
 // Keyspace to use for the parent trie key.
 const NO_CHILD_KEYSPACE: [u8;0] = [];
 
 
 /// Generate a new keyspace for a child trie.
-pub fn generate_keyspace<N>(block_nb: &N, parent_trie: &ParentTrie) -> Vec<u8>
-	where
-		N: TryInto<u128>,
-		N: Sub<N, Output = N>,
-		N: TryFrom<u128>,
-		N: Clone,
-{
-	// using 12 for block number and additional encoding targeting ~u64
-	let mut result = Vec::with_capacity(parent_trie.len() + 12);
-	parity_codec::Encode::encode_to(ChildTrie::parent_key_slice(parent_trie), &mut result);
-
-	let mut block_nb = block_nb.clone();
-	// compact number of child.
-	result.push(1);
-	// Note that this algo only work if conversion failure are related to out of bound and
-	// implemented when possible.
-	loop {
-		if let Ok(v) = block_nb.clone().try_into() {
-			if v < u128::max_value() {
-				parity_codec::Encode::encode_to(&Compact(v), &mut result);
-				break;
-			}
-		}
-		parity_codec::Encode::encode_to(&Compact(u128::max_value()), &mut result);
-		if let Ok(max) = N::try_from(u128::max_value()) {
-			block_nb = block_nb - max;
-		} else {
-			unreachable!("Previously fail with bigger value; qed");
-		}
-	}
+pub fn generate_keyspace(child_counter: u128) -> Vec<u8> {
+	// using 8 for block number and additional encoding targeting ~u64
+	let mut result = Vec::with_capacity(8);
+	parity_codec::Encode::encode_to(&Compact(child_counter), &mut result);
 	result
 }
 
@@ -227,23 +193,17 @@ impl ChildTrie {
 	/// or for performance purpose (later write).
 	///
 	/// We also provide an encodable value specific to the creation state (block number).
-	pub fn fetch_or_new<N>(
+	pub fn fetch_or_new(
 		parent_fetcher: impl FnOnce(&[u8]) -> Option<Self>,
 		child_trie_update: impl FnOnce(ChildTrie),
 		parent: &[u8],
-		block_nb: &N,
-	) -> Self
-		where
-			N: TryInto<u128>,
-			N: Sub<N, Output = N>,
-			N: TryFrom<u128>,
-			N: Clone,
-	{
+		child_trie_counter: u128,
+	) -> Self {
 		parent_fetcher(parent)
 			.unwrap_or_else(|| {
 			let parent = Self::prefix_parent_key(parent);
 			let ct = ChildTrie {
-				keyspace: generate_keyspace(block_nb, &parent),
+				keyspace: generate_keyspace(child_trie_counter),
 				root: Default::default(),
 				parent,
 				extension: Default::default(),
@@ -405,3 +365,11 @@ impl AsRef<ChildTrie> for ChildTrie {
 		self
 	}
 }
+
+#[test]
+fn encode_empty_prefix() {
+	let empt = generate_keyspace(0);
+
+	// this ensure root trie can be move to be a child trie
+	assert_eq!(&NO_CHILD_KEYSPACE[..], &empt[..]);
+}

core/primitives/src/storage.rs

@@ -67,6 +67,9 @@ pub mod well_known_keys {
 	/// Prefix of child storage keys.
 	pub const CHILD_STORAGE_KEY_PREFIX: &'static [u8] = b":child_storage:";
 
+	/// Counter for child trie keyspace.
+	pub const CHILD_STORAGE_KEYSPACE_COUNTER: &'static [u8] = b":child_keyspace_counter:";
+
 	/// Whether a key is a child storage key.
 	///
 	/// This is convenience function which basically checks if the given `key` starts

core/state-machine/src/lib.rs

@@ -1153,7 +1153,7 @@ mod tests {
 			|_| None,
 			|_| (),
 			b"testchild",
-			&0u64, // block number
+			1u128, // child trie counter
 		);
 		ext.set_child_storage(&child_trie, b"abc".to_vec(), b"def".to_vec());
 		assert_eq!(ext.child_storage(child_trie.node_ref(), b"abc"), Some(b"def".to_vec()));
@@ -1266,9 +1266,8 @@ mod tests {
 		use crate::trie_backend::tests::test_trie;
 		use std::collections::HashSet;
 
-		let block_number = 0u64;
-		let child_trie1 = ChildTrie::fetch_or_new(|_| None, |_| (), &[0x01], &block_number);
-		let child_trie2 = ChildTrie::fetch_or_new(|_| None, |_| (), &[0x23], &block_number);
+		let child_trie1 = ChildTrie::fetch_or_new(|_| None, |_| (), &[0x01], 1u128);
+		let child_trie2 = ChildTrie::fetch_or_new(|_| None, |_| (), &[0x23], 2u128);
 		let mut tr1 = {
 			let mut ttrie = test_trie();
 			let backend = ttrie.as_trie_backend().unwrap();

core/state-machine/src/proving_backend.rs

@@ -288,8 +288,8 @@ mod tests {
 
 	#[test]
 	fn proof_recorded_and_checked_with_child() {
-		let child_trie1 = ChildTrie::fetch_or_new(|_| None, |_| (), b"sub1", &0u64);
-		let child_trie2 = ChildTrie::fetch_or_new(|_| None, |_| (), b"sub2", &0u64);
+		let child_trie1 = ChildTrie::fetch_or_new(|_| None, |_| (), b"sub1", 1u128);
+		let child_trie2 = ChildTrie::fetch_or_new(|_| None, |_| (), b"sub2", 2u128);
 		let contents = (0..64).map(|i| (None, vec![i], Some(vec![i])))
 			.chain((28..65).map(|i| (Some(child_trie1.clone()), vec![i], Some(vec![i]))))
 			.chain((10..15).map(|i| (Some(child_trie2.clone()), vec![i], Some(vec![i]))))

core/state-machine/src/trie_backend.rs

@@ -198,7 +198,7 @@ pub mod tests {
 		let mut root = H256::default();
 		let mut mdb = PrefixedMemoryDB::<Blake2Hasher>::default();
 
-		let child_trie1 = ChildTrie::fetch_or_new(|_| None, |_| (), &b"sub1"[..], &0u64);
+		let child_trie1 = ChildTrie::fetch_or_new(|_| None, |_| (), &b"sub1"[..], 1u128);
 		let mut sub_root = H256::default();
 		{
 			let mut mdb = KeySpacedDBMut::new(&mut mdb, Some(child_trie1.keyspace()));

core/test-client/src/lib.rs

@@ -178,7 +178,7 @@ impl<Executor, Backend, G: GenesisInit> TestClientBuilder<
 					|_| (),
 					&b"test"[..],
 					// block 0
-					&0u64,
+					1u128,
 				);
 				storage.1.insert(
 					child_trie.keyspace().clone(),

srml/contracts/src/account_db.rs

@@ -151,10 +151,7 @@ impl<T: Trait> AccountDb<T> for DirectAccountDb {
 				}
 				let p_key = prefixed_trie_id(&new_info.trie_id);
 				// see issue FIXME #2744 to avoid this fetch
-				let child_trie = child::fetch_or_new(
-					p_key.as_ref(),
-					&block_number,
-				);
+				let child_trie = child::fetch_or_new(p_key.as_ref());
 
 				if !changed.storage.is_empty() {
 					new_info.last_write = Some(block_number);

srml/support/src/storage/mod.rs

@@ -20,6 +20,7 @@ use crate::rstd::prelude::*;
 use crate::rstd::borrow::Borrow;
 use substrate_primitives::child_trie::ChildTrie;
 use substrate_primitives::child_trie::ChildTrieReadRef;
+use substrate_primitives::storage::well_known_keys;
 use codec::{Codec, Encode, Decode, KeyedVec, Input, EncodeAppend};
 use hashed::generator::{HashedStorage, StorageHasher};
 use unhashed::generator::UnhashedStorage;
@@ -446,22 +447,23 @@ where
 /// Note that `storage_key` must be unique and strong (strong in the sense of being long enough to
 /// avoid collision from a resistant hash function (which unique implies)).
 pub mod child {
-	use sr_std::ops::Sub;
-	use sr_std::convert::{TryInto, TryFrom};
 	use super::{runtime_io, Codec, Decode, Vec, IncrementalChildInput, ChildTrie,
-		ChildTrieReadRef};
+		ChildTrieReadRef, well_known_keys};
 
 	/// Method for fetching or initiating a new child trie.
-	pub fn fetch_or_new<N>(
+	pub fn next_keyspace() -> u128 {
+		let key = well_known_keys::CHILD_STORAGE_KEYSPACE_COUNTER;
+		// do start at 1 (0 is reserved for top trie)
+		let previous = super::unhashed::get(key).unwrap_or(0u128);
+		let new = previous + 1;
+		super::unhashed::put(key, &new);
+		new
+	}
+
+	/// Method for fetching or initiating a new child trie.
+	pub fn fetch_or_new(
 		parent: &[u8],
-		block_nb: &N,
-	) -> ChildTrie
-		where
-			N: TryInto<u128>,
-			N: Sub<N, Output = N>,
-			N: TryFrom<u128>,
-			N: Clone,
-	{
+	) -> ChildTrie {
 		ChildTrie::fetch_or_new(
 			|pk| { child_trie(pk) },
 			|ct| {
@@ -472,7 +474,7 @@ pub mod child {
 				debug_assert!(updated);
 			},
 			parent,
-			block_nb,
+			next_keyspace(),
 		)
 	}