Some changes tries optimizations (#2840)

* changes tries initial optimizations

* line width

substrate/core/state-machine/src/changes_trie/build.rs

@@ -16,7 +16,8 @@
 
 //! Structures and functions required to build changes trie for given block.
 
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeMap;
+use std::collections::btree_map::Entry;
 use parity_codec::Decode;
 use hash_db::Hasher;
 use num_traits::One;
@@ -31,75 +32,89 @@ use crate::changes_trie::{AnchorBlockId, Configuration, Storage, BlockNumber};
 ///
 /// Returns Err if storage error has occurred OR if storage haven't returned
 /// required data.
-/// Returns Ok(None) data required to prepare input pairs is not collected
-/// or storage is not provided.
 pub fn prepare_input<'a, B, S, H, Number>(
-	backend: &B,
+	backend: &'a B,
 	storage: &'a S,
 	config: &'a Configuration,
-	changes: &OverlayedChanges,
+	changes: &'a OverlayedChanges,
 	parent: &'a AnchorBlockId<H::Out, Number>,
-) -> Result<Option<Vec<InputPair<Number>>>, String>
+) -> Result<impl Iterator<Item=InputPair<Number>> + 'a, String>
 	where
 		B: Backend<H>,
 		S: Storage<H, Number>,
-		H: Hasher,
+		H: Hasher + 'a,
 		Number: BlockNumber,
 {
-	let mut input = Vec::new();
-	input.extend(prepare_extrinsics_input(
+	let number = parent.number.clone() + One::one();
+	let extrinsics_input = prepare_extrinsics_input(
 		backend,
-		parent.number.clone() + 1.into(),
-		changes)?);
-	input.extend(prepare_digest_input::<_, H, Number>(
+		&number,
+		changes)?;
+	let digest_input = prepare_digest_input::<_, H, Number>(
 		parent,
 		config,
-		storage)?);
-
-	Ok(Some(input))
+		number,
+		storage)?;
+	Ok(extrinsics_input.chain(digest_input))
 }
 
 /// Prepare ExtrinsicIndex input pairs.
-fn prepare_extrinsics_input<B, H, Number>(
-	backend: &B,
-	block: Number,
-	changes: &OverlayedChanges,
-) -> Result<impl Iterator<Item=InputPair<Number>>, String>
+fn prepare_extrinsics_input<'a, B, H, Number>(
+	backend: &'a B,
+	block: &Number,
+	changes: &'a OverlayedChanges,
+) -> Result<impl Iterator<Item=InputPair<Number>> + 'a, String>
 	where
 		B: Backend<H>,
 		H: Hasher,
 		Number: BlockNumber,
 {
-	let mut extrinsic_map = BTreeMap::<Vec<u8>, BTreeSet<u32>>::new();
-	for (key, val) in changes.prospective.top.iter().chain(changes.committed.top.iter()) {
-		let extrinsics = match val.extrinsics {
-			Some(ref extrinsics) => extrinsics,
-			None => continue,
-		};
+	changes.committed.top.iter()
+		.chain(changes.prospective.top.iter())
+		.filter(|( _, v)| v.extrinsics.is_some())
+		.try_fold(BTreeMap::new(), |mut map: BTreeMap<&[u8], (ExtrinsicIndex<Number>, Vec<u32>)>, (k, v)| {
+			match map.entry(k) {
+				Entry::Vacant(entry) => {
+					// ignore temporary values (values that have null value at the end of operation
+					// AND are not in storage at the beginning of operation
+					if !changes.storage(k).map(|v| v.is_some()).unwrap_or_default() {
+						if !backend.exists_storage(k).map_err(|e| format!("{}", e))? {
+							return Ok(map);
+						}
+					}
 
-		// ignore values that have null value at the end of operation AND are not in storage
-		// at the beginning of operation
-		if !changes.storage(key).map(|v| v.is_some()).unwrap_or_default() {
-			if !backend.exists_storage(key).map_err(|e| format!("{}", e))? {
-				continue;
+					let extrinsics = v.extrinsics.as_ref()
+						.expect("filtered by filter() call above; qed")
+						.iter().cloned().collect();
+					entry.insert((ExtrinsicIndex {
+						block: block.clone(),
+						key: k.to_vec(),
+					}, extrinsics));
+				},
+				Entry::Occupied(mut entry) => {
+					// we do not need to check for temporary values here, because entry is Occupied
+					// AND we are checking it before insertion
+					let extrinsics = &mut entry.get_mut().1;
+					extrinsics.extend(
+						v.extrinsics.as_ref()
+							.expect("filtered by filter() call above; qed")
+							.iter()
+							.cloned()
+					);
+					extrinsics.sort_unstable();
+				},
 			}
-		}
 
-		extrinsic_map.entry(key.clone()).or_default()
-			.extend(extrinsics.iter().cloned());
-	}
-
-	Ok(extrinsic_map.into_iter()
-		.map(move |(key, extrinsics)| InputPair::ExtrinsicIndex(ExtrinsicIndex {
-			block: block.clone(),
-			key,
-		}, extrinsics.iter().cloned().collect())))
+			Ok(map)
+		})
+		.map(|pairs| pairs.into_iter().map(|(_, (k, v))| InputPair::ExtrinsicIndex(k, v)))
 }
 
 /// Prepare DigestIndex input pairs.
 fn prepare_digest_input<'a, S, H, Number>(
 	parent: &'a AnchorBlockId<H::Out, Number>,
 	config: &Configuration,
+	block: Number,
 	storage: &'a S
 ) -> Result<impl Iterator<Item=InputPair<Number>> + 'a, String>
 	where
@@ -108,35 +123,52 @@ fn prepare_digest_input<'a, S, H, Number>(
 		H::Out: 'a,
 		Number: BlockNumber,
 {
-	let mut digest_map = BTreeMap::<Vec<u8>, BTreeSet<Number>>::new();
-	for digest_build_block in digest_build_iterator(config, parent.number.clone() + One::one()) {
-		let trie_root = storage.root(parent, digest_build_block.clone())?;
-		let trie_root = trie_root.ok_or_else(|| format!("No changes trie root for block {}", digest_build_block.clone()))?;
-		let trie_storage = TrieBackendEssence::<_, H>::new(
-			crate::changes_trie::TrieBackendStorageAdapter(storage),
-			trie_root,
-		);
+	digest_build_iterator(config, block.clone())
+		.try_fold(BTreeMap::new(), move |mut map, digest_build_block| {
+			let trie_root = storage.root(parent, digest_build_block.clone())?;
+			let trie_root = trie_root.ok_or_else(|| format!("No changes trie root for block {}", digest_build_block.clone()))?;
+			let trie_storage = TrieBackendEssence::<_, H>::new(
+				crate::changes_trie::TrieBackendStorageAdapter(storage),
+				trie_root,
+			);
 
-		let extrinsic_prefix = ExtrinsicIndex::key_neutral_prefix(digest_build_block.clone());
-		trie_storage.for_keys_with_prefix(&extrinsic_prefix, |key|
-			if let Some(InputKey::ExtrinsicIndex::<Number>(trie_key)) = Decode::decode(&mut &key[..]) {
-				digest_map.entry(trie_key.key).or_default()
-					.insert(digest_build_block.clone());
-			});
+			let mut insert_to_map = |key: Vec<u8>| {
+				match map.entry(key.clone()) {
+					Entry::Vacant(entry) => {
+						entry.insert((DigestIndex {
+							block: block.clone(),
+							key,
+						}, vec![digest_build_block.clone()]));
+					},
+					Entry::Occupied(mut entry) => {
+						// DigestIndexValue must be sorted. Here we are relying on the fact that digest_build_iterator()
+						// returns blocks in ascending order => we only need to check for duplicates
+						//
+						// is_dup_block could be true when key has been changed in both digest block
+						// AND other blocks that it covers
+						let is_dup_block = entry.get().1.last() == Some(&digest_build_block);
+						if !is_dup_block {
+							entry.get_mut().1.push(digest_build_block.clone());
+						}
+					},
+				}
+			};
 
-		let digest_prefix = DigestIndex::key_neutral_prefix(digest_build_block.clone());
-		trie_storage.for_keys_with_prefix(&digest_prefix, |key|
-			if let Some(InputKey::DigestIndex::<Number>(trie_key)) = Decode::decode(&mut &key[..]) {
-				digest_map.entry(trie_key.key).or_default()
-					.insert(digest_build_block.clone());
-			});
-	}
+			let extrinsic_prefix = ExtrinsicIndex::key_neutral_prefix(digest_build_block.clone());
+			trie_storage.for_keys_with_prefix(&extrinsic_prefix, |key|
+				if let Some(InputKey::ExtrinsicIndex::<Number>(trie_key)) = Decode::decode(&mut &key[..]) {
+					insert_to_map(trie_key.key);
+				});
 
-	Ok(digest_map.into_iter()
-		.map(move |(key, set)| InputPair::DigestIndex(DigestIndex {
-			block: parent.number.clone() + One::one(),
-			key
-		}, set.into_iter().collect())))
+			let digest_prefix = DigestIndex::key_neutral_prefix(digest_build_block.clone());
+			trie_storage.for_keys_with_prefix(&digest_prefix, |key|
+				if let Some(InputKey::DigestIndex::<Number>(trie_key)) = Decode::decode(&mut &key[..]) {
+					insert_to_map(trie_key.key);
+				});
+
+			Ok(map)
+		})
+		.map(|pairs| pairs.into_iter().map(|(_, (k, v))| InputPair::DigestIndex(k, v)))
 }
 
 #[cfg(test)]
@@ -227,32 +259,34 @@ mod test {
 	fn build_changes_trie_nodes_on_non_digest_block() {
 		let (backend, storage, changes) = prepare_for_build();
 		let config = changes.changes_trie_config.as_ref().unwrap();
+		let parent = AnchorBlockId { hash: Default::default(), number: 4 };
 		let changes_trie_nodes = prepare_input(
 			&backend,
 			&storage,
 			config,
 			&changes,
-			&AnchorBlockId { hash: Default::default(), number: 4 },
+			&parent,
 		).unwrap();
-		assert_eq!(changes_trie_nodes, Some(vec![
+		assert_eq!(changes_trie_nodes.collect::<Vec<InputPair<u64>>>(), vec![
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 5, key: vec![100] }, vec![0, 2, 3]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 5, key: vec![101] }, vec![1]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 5, key: vec![103] }, vec![0, 1]),
-		]));
+		]);
 	}
 
 	#[test]
 	fn build_changes_trie_nodes_on_digest_block_l1() {
 		let (backend, storage, changes) = prepare_for_build();
 		let config = changes.changes_trie_config.as_ref().unwrap();
+		let parent = AnchorBlockId { hash: Default::default(), number: 3 };
 		let changes_trie_nodes = prepare_input(
 			&backend,
 			&storage,
 			config,
 			&changes,
-			&AnchorBlockId { hash: Default::default(), number: 3 },
+			&parent,
 		).unwrap();
-		assert_eq!(changes_trie_nodes, Some(vec![
+		assert_eq!(changes_trie_nodes.collect::<Vec<InputPair<u64>>>(), vec![
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 4, key: vec![100] }, vec![0, 2, 3]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 4, key: vec![101] }, vec![1]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 4, key: vec![103] }, vec![0, 1]),
@@ -261,21 +295,22 @@ mod test {
 			InputPair::DigestIndex(DigestIndex { block: 4, key: vec![101] }, vec![1]),
 			InputPair::DigestIndex(DigestIndex { block: 4, key: vec![102] }, vec![2]),
 			InputPair::DigestIndex(DigestIndex { block: 4, key: vec![105] }, vec![1, 3]),
-		]));
+		]);
 	}
 
 	#[test]
 	fn build_changes_trie_nodes_on_digest_block_l2() {
 		let (backend, storage, changes) = prepare_for_build();
 		let config = changes.changes_trie_config.as_ref().unwrap();
+		let parent = AnchorBlockId { hash: Default::default(), number: 15 };
 		let changes_trie_nodes = prepare_input(
 			&backend,
 			&storage,
 			config,
 			&changes,
-			&AnchorBlockId { hash: Default::default(), number: 15 },
+			&parent,
 		).unwrap();
-		assert_eq!(changes_trie_nodes, Some(vec![
+		assert_eq!(changes_trie_nodes.collect::<Vec<InputPair<u64>>>(), vec![
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 16, key: vec![100] }, vec![0, 2, 3]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 16, key: vec![101] }, vec![1]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 16, key: vec![103] }, vec![0, 1]),
@@ -285,7 +320,7 @@ mod test {
 			InputPair::DigestIndex(DigestIndex { block: 16, key: vec![102] }, vec![4]),
 			InputPair::DigestIndex(DigestIndex { block: 16, key: vec![103] }, vec![4]),
 			InputPair::DigestIndex(DigestIndex { block: 16, key: vec![105] }, vec![4, 8]),
-		]));
+		]);
 	}
 
 	#[test]
@@ -299,14 +334,15 @@ mod test {
 		});
 
 		let config = changes.changes_trie_config.as_ref().unwrap();
+		let parent = AnchorBlockId { hash: Default::default(), number: 3 };
 		let changes_trie_nodes = prepare_input(
 			&backend,
 			&storage,
 			config,
 			&changes,
-			&AnchorBlockId { hash: Default::default(), number: 3 },
+			&parent,
 		).unwrap();
-		assert_eq!(changes_trie_nodes, Some(vec![
+		assert_eq!(changes_trie_nodes.collect::<Vec<InputPair<u64>>>(), vec![
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 4, key: vec![100] }, vec![0, 2, 3]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 4, key: vec![101] }, vec![1]),
 			InputPair::ExtrinsicIndex(ExtrinsicIndex { block: 4, key: vec![103] }, vec![0, 1]),
@@ -315,6 +351,6 @@ mod test {
 			InputPair::DigestIndex(DigestIndex { block: 4, key: vec![101] }, vec![1]),
 			InputPair::DigestIndex(DigestIndex { block: 4, key: vec![102] }, vec![2]),
 			InputPair::DigestIndex(DigestIndex { block: 4, key: vec![105] }, vec![1, 3]),
-		]));
+		]);
 	}
 }

substrate/core/state-machine/src/changes_trie/build_iterator.rs

@@ -20,7 +20,8 @@
 use crate::changes_trie::{Configuration, BlockNumber};
 
 /// Returns iterator of OTHER blocks that are required for inclusion into
-/// changes trie of given block.
+/// changes trie of given block. Blocks are guaranteed to be returned in
+/// ascending order.
 pub fn digest_build_iterator<Number: BlockNumber>(
 	config: &Configuration,
 	block: Number,
@@ -47,6 +48,8 @@ pub struct DigestBuildIterator<Number: BlockNumber> {
 	max_step: u32,
 	/// Step of current blocks range.
 	current_step: u32,
+	/// Reverse step of current blocks range.
+	current_step_reverse: u32,
 	/// Current blocks range.
 	current_range: Option<BlocksRange<Number>>,
 }
@@ -58,7 +61,8 @@ impl<Number: BlockNumber> DigestBuildIterator<Number> {
 			block,
 			digest_interval,
 			max_step,
-			current_step: 0,
+			current_step: max_step,
+			current_step_reverse: 0,
 			current_range: None,
 		}
 	}
@@ -81,18 +85,27 @@ impl<Number: BlockNumber> Iterator for DigestBuildIterator<Number> {
 		// DigestBuildIterator is created only by internal function that is checking
 		// that all multiplications/subtractions are safe within max_step limit
 
-		let next_step = if self.current_step == 0 { 1 } else { self.current_step * self.digest_interval };
-		if next_step > self.max_step {
+		let next_step_reverse = if self.current_step_reverse == 0 {
+			1
+		} else {
+			self.current_step_reverse * self.digest_interval
+		};
+		if next_step_reverse > self.max_step {
 			return None;
 		}
 
-		self.current_step = next_step;
+		self.current_step_reverse = next_step_reverse;
 		self.current_range = Some(BlocksRange::new(
 			self.block.clone() - (self.current_step * self.digest_interval - self.current_step).into(),
 			self.block.clone(),
 			self.current_step.into(),
 		));
 
+		self.current_step = self.current_step / self.digest_interval;
+		if self.current_step == 0 {
+			self.current_step = 1;
+		}
+
 		Some(self.current_range.as_mut()
 			.expect("assigned one line above; qed")
 			.next()
@@ -203,18 +216,18 @@ mod tests {
 	fn digest_iterator_returns_level1_and_level2_blocks() {
 		assert_eq!(digest_build_iterator_blocks(16, 2, 256),
 			vec![
-				// level2 is a level1 digest of 16-1 previous blocks:
-				241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255,
 				// level2 points to previous 16-1 level1 digests:
 				16, 32, 48, 64, 80, 96, 112, 128, 144, 160, 176, 192, 208, 224, 240,
+				// level2 is a level1 digest of 16-1 previous blocks:
+				241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255,
 			],
 		);
 		assert_eq!(digest_build_iterator_blocks(16, 2, 4096),
 			vec![
-				// level2 is a level1 digest of 16-1 previous blocks:
-				4081, 4082, 4083, 4084, 4085, 4086, 4087, 4088, 4089, 4090, 4091, 4092, 4093, 4094, 4095,
 				// level2 points to previous 16-1 level1 digests:
 				3856, 3872, 3888, 3904, 3920, 3936, 3952, 3968, 3984, 4000, 4016, 4032, 4048, 4064, 4080,
+				// level2 is a level1 digest of 16-1 previous blocks:
+				4081, 4082, 4083, 4084, 4085, 4086, 4087, 4088, 4089, 4090, 4091, 4092, 4093, 4094, 4095,
 			],
 		);
 	}
@@ -223,12 +236,12 @@ mod tests {
 	fn digest_iterator_returns_level1_and_level2_and_level3_blocks() {
 		assert_eq!(digest_build_iterator_blocks(16, 3, 4096),
 			vec![
-				// level3 is a level1 digest of 16-1 previous blocks:
-				4081, 4082, 4083, 4084, 4085, 4086, 4087, 4088, 4089, 4090, 4091, 4092, 4093, 4094, 4095,
-				// level3 points to previous 16-1 level1 digests:
-				3856, 3872, 3888, 3904, 3920, 3936, 3952, 3968, 3984, 4000, 4016, 4032, 4048, 4064, 4080,
 				// level3 points to previous 16-1 level2 digests:
 				256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840,
+				// level3 points to previous 16-1 level1 digests:
+				3856, 3872, 3888, 3904, 3920, 3936, 3952, 3968, 3984, 4000, 4016, 4032, 4048, 4064, 4080,
+				// level3 is a level1 digest of 16-1 previous blocks:
+				4081, 4082, 4083, 4084, 4085, 4086, 4087, 4088, 4089, 4090, 4091, 4092, 4093, 4094, 4095,
 			],
 		);
 	}

substrate/core/state-machine/src/changes_trie/mod.rs

@@ -53,7 +53,7 @@ use parity_codec::{Decode, Encode};
 use primitives;
 use crate::changes_trie::build::prepare_input;
 use crate::overlayed_changes::OverlayedChanges;
-use trie::{DBValue, trie_root};
+use trie::{MemoryDB, TrieDBMut, TrieMut, DBValue};
 
 /// Changes that are made outside of extrinsics are marked with this index;
 pub const NO_EXTRINSIC_INDEX: u32 = 0xffffffff;
@@ -128,13 +128,13 @@ pub type Configuration = primitives::ChangesTrieConfiguration;
 /// Compute the changes trie root and transaction for given block.
 /// Returns Err(()) if unknown `parent_hash` has been passed.
 /// Returns Ok(None) if there's no data to perform computation.
-/// Panics if background storage returns an error.
-pub fn compute_changes_trie_root<'a, B: Backend<H>, S: Storage<H, Number>, H: Hasher, Number: BlockNumber>(
+/// Panics if background storage returns an error OR if insert to MemoryDB fails.
+pub fn build_changes_trie<'a, B: Backend<H>, S: Storage<H, Number>, H: Hasher, Number: BlockNumber>(
 	backend: &B,
 	storage: Option<&'a S>,
 	changes: &OverlayedChanges,
 	parent_hash: H::Out,
-) -> Result<Option<(H::Out, Vec<(Vec<u8>, Vec<u8>)>)>, ()>
+) -> Result<Option<(MemoryDB<H>, H::Out)>, ()>
 	where
 		H::Out: Ord + 'static,
 {
@@ -148,16 +148,16 @@ pub fn compute_changes_trie_root<'a, B: Backend<H>, S: Storage<H, Number>, H: Ha
 
 	// storage errors are considered fatal (similar to situations when runtime fetches values from storage)
 	let input_pairs = prepare_input::<B, S, H, Number>(backend, storage, config, changes, &parent)
-		.expect("storage is not allowed to fail within runtime");
-	match input_pairs {
-		Some(input_pairs) => {
-			let transaction = input_pairs.into_iter()
-				.map(Into::into)
-				.collect::<Vec<_>>();
-			let root = trie_root::<H, _, _, _>(transaction.iter().map(|(k, v)| (&*k, &*v)));
-
-			Ok(Some((root, transaction)))
-		},
-		None => Ok(None),
+		.expect("changes trie: storage access is not allowed to fail within runtime");
+	let mut root = Default::default();
+	let mut mdb = MemoryDB::default();
+	{
+		let mut trie = TrieDBMut::<H>::new(&mut mdb, &mut root);
+		for (key, value) in input_pairs.map(Into::into) {
+			trie.insert(&key, &value)
+				.expect("changes trie: insertion to trie is not allowed to fail within runtime");
+		}
 	}
+
+	Ok(Some((mdb, root)))
 }

substrate/core/state-machine/src/ext.rs

@@ -19,12 +19,12 @@
 use std::{error, fmt, cmp::Ord};
 use log::warn;
 use crate::backend::Backend;
-use crate::changes_trie::{Storage as ChangesTrieStorage, compute_changes_trie_root};
+use crate::changes_trie::{Storage as ChangesTrieStorage, build_changes_trie};
 use crate::{Externalities, OverlayedChanges, ChildStorageKey};
 use hash_db::Hasher;
 use primitives::offchain;
 use primitives::storage::well_known_keys::is_child_storage_key;
-use trie::{MemoryDB, TrieDBMut, TrieMut, default_child_trie_root};
+use trie::{MemoryDB, default_child_trie_root};
 
 const EXT_NOT_ALLOWED_TO_FAIL: &str = "Externalities not allowed to fail within runtime";
 
@@ -320,27 +320,13 @@ where
 
 	fn storage_changes_root(&mut self, parent_hash: H::Out) -> Result<Option<H::Out>, ()> {
 		let _guard = panic_handler::AbortGuard::new(true);
-		let root_and_tx = compute_changes_trie_root::<_, T, H, N>(
+		self.changes_trie_transaction = build_changes_trie::<_, T, H, N>(
 			self.backend,
 			self.changes_trie_storage.clone(),
 			self.overlay,
 			parent_hash,
 		)?;
-		let root_and_tx = root_and_tx.map(|(root, changes)| {
-			let mut calculated_root = Default::default();
-			let mut mdb = MemoryDB::default();
-			{
-				let mut trie = TrieDBMut::<H>::new(&mut mdb, &mut calculated_root);
-				for (key, value) in changes {
-					trie.insert(&key, &value).expect(EXT_NOT_ALLOWED_TO_FAIL);
-				}
-			}
-
-			(mdb, root)
-		});
-		let root = root_and_tx.as_ref().map(|(_, root)| root.clone());
-		self.changes_trie_transaction = root_and_tx;
-		Ok(root)
+		Ok(self.changes_trie_transaction.as_ref().map(|(_, root)| root.clone()))
 	}
 
 	fn offchain(&mut self) -> Option<&mut dyn offchain::Externalities> {

substrate/core/state-machine/src/overlayed_changes.rs

@@ -17,7 +17,7 @@
 //! The overlayed changes to state.
 
 #[cfg(test)] use std::iter::FromIterator;
-use std::collections::{HashMap, HashSet};
+use std::collections::{HashMap, BTreeSet};
 use parity_codec::Decode;
 use crate::changes_trie::{NO_EXTRINSIC_INDEX, Configuration as ChangesTrieConfig};
 use primitives::storage::well_known_keys::EXTRINSIC_INDEX;
@@ -45,7 +45,7 @@ pub struct OverlayedValue {
 	pub value: Option<Vec<u8>>,
 	/// The set of extinsic indices where the values has been changed.
 	/// Is filled only if runtime has announced changes trie support.
-	pub extrinsics: Option<HashSet<u32>>,
+	pub extrinsics: Option<BTreeSet<u32>>,
 }
 
 /// Prospective or committed overlayed change set.
@@ -55,7 +55,7 @@ pub struct OverlayedChangeSet {
 	/// Top level storage changes.
 	pub top: HashMap<Vec<u8>, OverlayedValue>,
 	/// Child storage changes.
-	pub children: HashMap<Vec<u8>, (Option<HashSet<u32>>, HashMap<Vec<u8>, Option<Vec<u8>>>)>,
+	pub children: HashMap<Vec<u8>, (Option<BTreeSet<u32>>, HashMap<Vec<u8>, Option<Vec<u8>>>)>,
 }
 
 #[cfg(test)]

substrate/core/state-machine/src/testing.rs

@@ -22,7 +22,7 @@ use hash_db::Hasher;
 use crate::backend::{InMemory, Backend};
 use primitives::storage::well_known_keys::is_child_storage_key;
 use crate::changes_trie::{
-	compute_changes_trie_root, InMemoryStorage as ChangesTrieInMemoryStorage,
+	build_changes_trie, InMemoryStorage as ChangesTrieInMemoryStorage,
 	BlockNumber as ChangesTrieBlockNumber,
 };
 use primitives::offchain;
@@ -250,12 +250,12 @@ impl<H, N> Externalities<H> for TestExternalities<H, N>
 	}
 
 	fn storage_changes_root(&mut self, parent: H::Out) -> Result<Option<H::Out>, ()> {
-		Ok(compute_changes_trie_root::<_, _, H, N>(
+		Ok(build_changes_trie::<_, _, H, N>(
 			&self.backend,
 			Some(&self.changes_trie_storage),
 			&self.overlay,
 			parent,
-		)?.map(|(root, _)| root.clone()))
+		)?.map(|(_, root)| root))
 	}
 
 	fn offchain(&mut self) -> Option<&mut dyn offchain::Externalities> {