state_machine no_std witness externalities (#6934)

* checkpoint before removing CT from change trie

* before trie backend without tx

* undo

* Started no transaction, but would need using a different root
calculation method, out of the scope of this pr, will roll back.

* Remove NoTransaction.

* partially address review.
dummy stats implementation for no_std.

* Remove ChangeTrieOverlay.

* modified function

* Remove witness_ext

* need noops changes root

* update from cumulus branch

* line break

* remove warning

* line break

* From review: renamings and stats active in no std (except time).

* include cache, exclude change trie cache with individual temporary bad looking
no_std check

* little test

* fuse imports and filter_map prepare_extrinsics_input_inner fold.

* put back ExtInner into Ext, awkward double proto for new function.

* Apply suggestions from code review

* Update primitives/state-machine/Cargo.toml

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/primitives/state-machine/src/overlayed_changes/changeset.rs

@@ -17,18 +17,22 @@
 
 //! Houses the code that implements the transactional overlay storage.
 
-use super::{StorageKey, StorageValue};
+use super::{StorageKey, StorageValue, Extrinsics};
 
-use itertools::Itertools;
-use std::collections::{HashSet, BTreeMap, BTreeSet};
+#[cfg(feature = "std")]
+use std::collections::HashSet as Set;
+#[cfg(not(feature = "std"))]
+use sp_std::collections::btree_set::BTreeSet as Set;
+
+use sp_std::collections::{btree_map::BTreeMap, btree_set::BTreeSet};
 use smallvec::SmallVec;
-use log::warn;
+use crate::warn;
 
 const PROOF_OVERLAY_NON_EMPTY: &str = "\
 	An OverlayValue is always created with at least one transaction and dropped as soon
 	as the last transaction is removed; qed";
 
-type DirtyKeysSets = SmallVec<[HashSet<StorageKey>; 5]>;
+type DirtyKeysSets = SmallVec<[Set<StorageKey>; 5]>;
 type Transactions = SmallVec<[InnerValue; 5]>;
 
 /// Error returned when trying to commit or rollback while no transaction is open or
@@ -63,7 +67,7 @@ struct InnerValue {
 	value: Option<StorageValue>,
 	/// The set of extrinsic indices where the values has been changed.
 	/// Is filled only if runtime has announced changes trie support.
-	extrinsics: BTreeSet<u32>,
+	extrinsics: Extrinsics,
 }
 
 /// An overlay that contains all versions of a value for a specific key.
@@ -105,8 +109,10 @@ impl OverlayedValue {
 	}
 
 	/// Unique list of extrinsic indices which modified the value.
-	pub fn extrinsics(&self) -> impl Iterator<Item=&u32> {
-		self.transactions.iter().flat_map(|t| t.extrinsics.iter()).unique()
+	pub fn extrinsics(&self) -> BTreeSet<u32> {
+		let mut set = BTreeSet::new();
+		self.transactions.iter().for_each(|t| t.extrinsics.copy_extrinsics_into(&mut set));
+		set
 	}
 
 	/// Mutable reference to the most recent version.
@@ -120,7 +126,7 @@ impl OverlayedValue {
 	}
 
 	/// Mutable reference to the set which holds the indices for the **current transaction only**.
-	fn transaction_extrinsics_mut(&mut self) -> &mut BTreeSet<u32> {
+	fn transaction_extrinsics_mut(&mut self) -> &mut Extrinsics {
 		&mut self.transactions.last_mut().expect(PROOF_OVERLAY_NON_EMPTY).extrinsics
 	}
 
@@ -163,9 +169,9 @@ impl OverlayedChangeSet {
 	/// This changeset might be created when there are already open transactions.
 	/// We need to catch up here so that the child is at the same transaction depth.
 	pub fn spawn_child(&self) -> Self {
-		use std::iter::repeat;
+		use sp_std::iter::repeat;
 		Self {
-			dirty_keys: repeat(HashSet::new()).take(self.transaction_depth()).collect(),
+			dirty_keys: repeat(Set::new()).take(self.transaction_depth()).collect(),
 			num_client_transactions: self.num_client_transactions,
 			execution_mode: self.execution_mode,
 			.. Default::default()
@@ -232,7 +238,7 @@ impl OverlayedChangeSet {
 		at_extrinsic: Option<u32>,
 	) {
 		for (key, val) in self.changes.iter_mut().filter(|(k, v)| predicate(k, v)) {
-			val.set(None, insert_dirty(&mut self.dirty_keys, key.to_owned()), at_extrinsic);
+			val.set(None, insert_dirty(&mut self.dirty_keys, key.clone()), at_extrinsic);
 		}
 	}
 
@@ -243,7 +249,7 @@ impl OverlayedChangeSet {
 
 	/// Get the change that is next to the supplied key.
 	pub fn next_change(&self, key: &[u8]) -> Option<(&[u8], &OverlayedValue)> {
-		use std::ops::Bound;
+		use sp_std::ops::Bound;
 		let range = (Bound::Excluded(key), Bound::Unbounded);
 		self.changes.range::<[u8], _>(range).next().map(|(k, v)| (&k[..], v))
 	}
@@ -388,7 +394,7 @@ mod test {
 
 	fn assert_changes(is: &OverlayedChangeSet, expected: &Changes) {
 		let is: Changes = is.changes().map(|(k, v)| {
-			(k.as_ref(), (v.value().map(AsRef::as_ref), v.extrinsics().cloned().collect()))
+			(k.as_ref(), (v.value().map(AsRef::as_ref), v.extrinsics().into_iter().collect()))
 		}).collect();
 		assert_eq!(&is, expected);
 	}

substrate/primitives/state-machine/src/overlayed_changes/mod.rs

@@ -20,23 +20,38 @@
 mod changeset;
 
 use crate::{
-	backend::Backend, ChangesTrieTransaction,
-	changes_trie::{
-		NO_EXTRINSIC_INDEX, BlockNumber, build_changes_trie,
-		State as ChangesTrieState,
-	},
+	backend::Backend,
 	stats::StateMachineStats,
 };
+use sp_std::vec::Vec;
 use self::changeset::OverlayedChangeSet;
 
-use std::collections::HashMap;
+#[cfg(feature = "std")]
+use crate::{
+	ChangesTrieTransaction,
+	changes_trie::{
+		build_changes_trie,
+		State as ChangesTrieState,
+	},
+};
+use crate::changes_trie::BlockNumber;
+#[cfg(feature = "std")]
+use std::collections::HashMap as Map;
+#[cfg(not(feature = "std"))]
+use sp_std::collections::btree_map::BTreeMap as Map;
+use sp_std::collections::btree_set::BTreeSet;
 use codec::{Decode, Encode};
 use sp_core::storage::{well_known_keys::EXTRINSIC_INDEX, ChildInfo};
+#[cfg(feature = "std")]
 use sp_core::offchain::storage::OffchainOverlayedChanges;
 use hash_db::Hasher;
+use crate::DefaultError;
 
 pub use self::changeset::{OverlayedValue, NoOpenTransaction, AlreadyInRuntime, NotInRuntime};
 
+/// Changes that are made outside of extrinsics are marked with this index;
+pub const NO_EXTRINSIC_INDEX: u32 = 0xffffffff;
+
 /// Storage key.
 pub type StorageKey = Vec<u8>;
 
@@ -49,6 +64,29 @@ pub type StorageCollection = Vec<(StorageKey, Option<StorageValue>)>;
 /// In memory arrays of storage values for multiple child tries.
 pub type ChildStorageCollection = Vec<(StorageKey, StorageCollection)>;
 
+/// Keep trace of extrinsics index for a modified value.
+#[derive(Debug, Default, Eq, PartialEq, Clone)]
+pub struct Extrinsics(Vec<u32>);
+
+impl Extrinsics {
+	/// Extracts extrinsics into a `BTreeSets`.
+	fn copy_extrinsics_into(&self, dest: &mut BTreeSet<u32>) {
+		dest.extend(self.0.iter())
+	}
+
+	/// Add an extrinsics.
+	fn insert(&mut self, ext: u32) {
+		if Some(&ext) != self.0.last() {
+			self.0.push(ext);
+		}
+	}
+
+	/// Extend `self` with `other`.
+	fn extend(&mut self, other: Self) {
+		self.0.extend(other.0.into_iter());
+	}
+}
+
 /// The set of changes that are overlaid onto the backend.
 ///
 /// It allows changes to be modified using nestable transactions.
@@ -57,7 +95,7 @@ pub struct OverlayedChanges {
 	/// Top level storage changes.
 	top: OverlayedChangeSet,
 	/// Child storage changes. The map key is the child storage key without the common prefix.
-	children: HashMap<StorageKey, (OverlayedChangeSet, ChildInfo)>,
+	children: Map<StorageKey, (OverlayedChangeSet, ChildInfo)>,
 	/// True if extrinsics stats must be collected.
 	collect_extrinsics: bool,
 	/// Collect statistic on this execution.
@@ -76,6 +114,7 @@ pub struct StorageChanges<Transaction, H: Hasher, N: BlockNumber> {
 	/// All changes to the child storages.
 	pub child_storage_changes: ChildStorageCollection,
 	/// Offchain state changes to write to the offchain database.
+	#[cfg(feature = "std")]
 	pub offchain_storage_changes: OffchainOverlayedChanges,
 	/// A transaction for the backend that contains all changes from
 	/// [`main_storage_changes`](StorageChanges::main_storage_changes) and from
@@ -87,9 +126,14 @@ pub struct StorageChanges<Transaction, H: Hasher, N: BlockNumber> {
 	/// Contains the transaction for the backend for the changes trie.
 	///
 	/// If changes trie is disabled the value is set to `None`.
+	#[cfg(feature = "std")]
 	pub changes_trie_transaction: Option<ChangesTrieTransaction<H, N>>,
+	/// Phantom data for block number until change trie support no_std.
+	#[cfg(not(feature = "std"))]
+	pub _ph: sp_std::marker::PhantomData<N>,
 }
 
+#[cfg(feature = "std")]
 impl<Transaction, H: Hasher, N: BlockNumber> StorageChanges<Transaction, H, N> {
 	/// Deconstruct into the inner values
 	pub fn into_inner(self) -> (
@@ -120,9 +164,14 @@ pub struct StorageTransactionCache<Transaction, H: Hasher, N: BlockNumber> {
 	/// The storage root after applying the transaction.
 	pub(crate) transaction_storage_root: Option<H::Out>,
 	/// Contains the changes trie transaction.
+	#[cfg(feature = "std")]
 	pub(crate) changes_trie_transaction: Option<Option<ChangesTrieTransaction<H, N>>>,
 	/// The storage root after applying the changes trie transaction.
+	#[cfg(feature = "std")]
 	pub(crate) changes_trie_transaction_storage_root: Option<Option<H::Out>>,
+	/// Phantom data for block number until change trie support no_std.
+	#[cfg(not(feature = "std"))]
+	pub(crate) _ph: sp_std::marker::PhantomData<N>,
 }
 
 impl<Transaction, H: Hasher, N: BlockNumber> StorageTransactionCache<Transaction, H, N> {
@@ -137,8 +186,12 @@ impl<Transaction, H: Hasher, N: BlockNumber> Default for StorageTransactionCache
 		Self {
 			transaction: None,
 			transaction_storage_root: None,
+			#[cfg(feature = "std")]
 			changes_trie_transaction: None,
+			#[cfg(feature = "std")]
 			changes_trie_transaction_storage_root: None,
+			#[cfg(not(feature = "std"))]
+			_ph: Default::default(),
 		}
 	}
 }
@@ -148,10 +201,14 @@ impl<Transaction: Default, H: Hasher, N: BlockNumber> Default for StorageChanges
 		Self {
 			main_storage_changes: Default::default(),
 			child_storage_changes: Default::default(),
+			#[cfg(feature = "std")]
 			offchain_storage_changes: Default::default(),
 			transaction: Default::default(),
 			transaction_storage_root: Default::default(),
+			#[cfg(feature = "std")]
 			changes_trie_transaction: None,
+			#[cfg(not(feature = "std"))]
+			_ph: Default::default(),
 		}
 	}
 }
@@ -190,7 +247,7 @@ impl OverlayedChanges {
 		key: &[u8],
 		init: impl Fn() -> StorageValue,
 	) -> &mut StorageValue {
-		let value = self.top.modify(key.to_owned(), init, self.extrinsic_index());
+		let value = self.top.modify(key.to_vec(), init, self.extrinsic_index());
 
 		// if the value was deleted initialise it back with an empty vec
 		value.get_or_insert_with(StorageValue::default)
@@ -235,7 +292,7 @@ impl OverlayedChanges {
 		let (changeset, info) = self.children.entry(storage_key).or_insert_with(||
 			(
 				top.spawn_child(),
-				child_info.to_owned()
+				child_info.clone()
 			)
 		);
 		let updatable = info.try_update(child_info);
@@ -256,7 +313,7 @@ impl OverlayedChanges {
 		let (changeset, info) = self.children.entry(storage_key).or_insert_with(||
 			(
 				top.spawn_child(),
-				child_info.to_owned()
+				child_info.clone()
 			)
 		);
 		let updatable = info.try_update(child_info);
@@ -285,7 +342,7 @@ impl OverlayedChanges {
 		let (changeset, info) = self.children.entry(storage_key).or_insert_with(||
 			(
 				top.spawn_child(),
-				child_info.to_owned()
+				child_info.clone()
 			)
 		);
 		let updatable = info.try_update(child_info);
@@ -322,7 +379,7 @@ impl OverlayedChanges {
 	/// there is no open transaction that can be rolled back.
 	pub fn rollback_transaction(&mut self) -> Result<(), NoOpenTransaction> {
 		self.top.rollback_transaction()?;
-		self.children.retain(|_, (changeset, _)| {
+		retain_map(&mut self.children, |_, (changeset, _)| {
 			changeset.rollback_transaction()
 				.expect("Top and children changesets are started in lockstep; qed");
 			!changeset.is_empty()
@@ -379,7 +436,7 @@ impl OverlayedChanges {
 		impl Iterator<Item=(StorageKey, Option<StorageValue>)>,
 		impl Iterator<Item=(StorageKey, (impl Iterator<Item=(StorageKey, Option<StorageValue>)>, ChildInfo))>,
 	) {
-		use std::mem::take;
+		use sp_std::mem::take;
 		(
 			take(&mut self.top).drain_commited(),
 			take(&mut self.children).into_iter()
@@ -409,6 +466,7 @@ impl OverlayedChanges {
 	}
 
 	/// Convert this instance with all changes into a [`StorageChanges`] instance.
+	#[cfg(feature = "std")]
 	pub fn into_storage_changes<
 		B: Backend<H>, H: Hasher, N: BlockNumber
 	>(
@@ -417,7 +475,8 @@ impl OverlayedChanges {
 		changes_trie_state: Option<&ChangesTrieState<H, N>>,
 		parent_hash: H::Out,
 		mut cache: StorageTransactionCache<B::Transaction, H, N>,
-	) -> Result<StorageChanges<B::Transaction, H, N>, String> where H::Out: Ord + Encode + 'static {
+	) -> Result<StorageChanges<B::Transaction, H, N>, DefaultError>
+		where H::Out: Ord + Encode + 'static {
 		self.drain_storage_changes(backend, changes_trie_state, parent_hash, &mut cache)
 	}
 
@@ -425,10 +484,12 @@ impl OverlayedChanges {
 	pub fn drain_storage_changes<B: Backend<H>, H: Hasher, N: BlockNumber>(
 		&mut self,
 		backend: &B,
+		#[cfg(feature = "std")]
 		changes_trie_state: Option<&ChangesTrieState<H, N>>,
 		parent_hash: H::Out,
 		mut cache: &mut StorageTransactionCache<B::Transaction, H, N>,
-	) -> Result<StorageChanges<B::Transaction, H, N>, String> where H::Out: Ord + Encode + 'static {
+	) -> Result<StorageChanges<B::Transaction, H, N>, DefaultError>
+		where H::Out: Ord + Encode + 'static {
 		// If the transaction does not exist, we generate it.
 		if cache.transaction.is_none() {
 			self.storage_root(backend, &mut cache);
@@ -439,6 +500,7 @@ impl OverlayedChanges {
 			.expect("Transaction was be generated as part of `storage_root`; qed");
 
 		// If the transaction does not exist, we generate it.
+		#[cfg(feature = "std")]
 		if cache.changes_trie_transaction.is_none() {
 			self.changes_trie_root(
 				backend,
@@ -449,20 +511,24 @@ impl OverlayedChanges {
 			).map_err(|_| "Failed to generate changes trie transaction")?;
 		}
 
+		#[cfg(feature = "std")]
 		let changes_trie_transaction = cache.changes_trie_transaction
 			.take()
 			.expect("Changes trie transaction was generated by `changes_trie_root`; qed");
 
-		let offchain_storage_changes = Default::default();
 		let (main_storage_changes, child_storage_changes) = self.drain_committed();
 
 		Ok(StorageChanges {
 			main_storage_changes: main_storage_changes.collect(),
 			child_storage_changes: child_storage_changes.map(|(sk, it)| (sk, it.0.collect())).collect(),
-			offchain_storage_changes,
+			#[cfg(feature = "std")]
+			offchain_storage_changes: Default::default(),
 			transaction,
 			transaction_storage_root,
+			#[cfg(feature = "std")]
 			changes_trie_transaction,
+			#[cfg(not(feature = "std"))]
+			_ph: Default::default(),
 		})
 	}
 
@@ -520,6 +586,7 @@ impl OverlayedChanges {
 	/// # Panics
 	///
 	/// Panics on storage error, when `panic_on_storage_error` is set.
+	#[cfg(feature = "std")]
 	pub fn changes_trie_root<'a, H: Hasher, N: BlockNumber, B: Backend<H>>(
 		&self,
 		backend: &B,
@@ -563,6 +630,29 @@ impl OverlayedChanges {
 	}
 }
 
+#[cfg(feature = "std")]
+fn retain_map<K, V, F>(map: &mut Map<K, V>, f: F)
+	where
+		K: std::cmp::Eq + std::hash::Hash,
+		F: FnMut(&K, &mut V) -> bool,
+{
+	map.retain(f);
+}
+ 
+#[cfg(not(feature = "std"))]
+fn retain_map<K, V, F>(map: &mut Map<K, V>, mut f: F)
+	where
+		K: Ord,
+		F: FnMut(&K, &mut V) -> bool,
+{
+	let old = sp_std::mem::replace(map, Map::default());
+	for (k, mut v) in old.into_iter() {
+		if f(&k, &mut v) {
+			map.insert(k, v);
+		}
+	}
+}
+ 
 #[cfg(test)]
 mod tests {
 	use hex_literal::hex;
@@ -578,7 +668,7 @@ mod tests {
 		expected: Vec<u32>,
 	) {
 		assert_eq!(
-			overlay.get(key.as_ref()).unwrap().extrinsics().cloned().collect::<Vec<_>>(),
+			overlay.get(key.as_ref()).unwrap().extrinsics().into_iter().collect::<Vec<_>>(),
 			expected
 		)
 	}