Cleanup of the state-machine crate (#3524)

* Start refactoring state-machine crate

* More improvement to state-machine

* Fix tests compilation on master and remove warnings

* Fix compilation

* Apply suggestions from code review

Co-Authored-By: Sergei Pepyakin <sergei@parity.io>

* Update core/state-machine/src/basic.rs

Co-Authored-By: DemiMarie-parity <48690212+DemiMarie-parity@users.noreply.github.com>

* Line width

* Update core/primitives/src/storage.rs

Co-Authored-By: Benjamin Kampmann <ben.kampmann@googlemail.com>

* Update core/state-machine/src/error.rs

Co-Authored-By: Benjamin Kampmann <ben.kampmann@googlemail.com>

* Review feedback

substrate/core/client/src/backend.rs

@@ -144,7 +144,7 @@ pub trait Finalizer<Block: BlockT, H: Hasher<Out=Block::Hash>, B: Backend<Block,
 		notify: bool,
 	) -> error::Result<()>;
 
-		
+
 	/// Finalize a block. This will implicitly finalize all blocks up to it and
 	/// fire finality notifications.
 	///

substrate/core/client/src/call_executor.rs

@@ -20,13 +20,15 @@ use sr_primitives::{
 	generic::BlockId, traits::Block as BlockT, traits::NumberFor,
 };
 use state_machine::{
-	self, OverlayedChanges, Ext, CodeExecutor, ExecutionManager,
-	ExecutionStrategy, NeverOffchainExt, backend::Backend as _,
-	ChangesTrieTransaction,
+	self, OverlayedChanges, Ext, ExecutionManager, StateMachine, ExecutionStrategy,
+	backend::Backend as _, ChangesTrieTransaction,
 };
 use executor::{RuntimeVersion, RuntimeInfo, NativeVersion};
 use hash_db::Hasher;
-use primitives::{offchain, H256, Blake2Hasher, NativeOrEncoded, NeverNativeValue};
+use primitives::{
+	offchain::{self, NeverOffchainExt}, H256, Blake2Hasher, NativeOrEncoded, NeverNativeValue,
+	traits::CodeExecutor,
+};
 
 use crate::runtime_api::{ProofRecorder, InitializeBlock};
 use crate::backend;
@@ -204,7 +206,7 @@ where
 	) -> error::Result<Vec<u8>> {
 		let mut changes = OverlayedChanges::default();
 		let state = self.backend.state_at(*id)?;
-		let return_data = state_machine::new(
+		let return_data = StateMachine::new(
 			&state,
 			self.backend.changes_trie_storage(),
 			side_effects_handler,
@@ -277,7 +279,7 @@ where
 					recorder.clone()
 				);
 
-				state_machine::new(
+				StateMachine::new(
 					&backend,
 					self.backend.changes_trie_storage(),
 					side_effects_handler,
@@ -295,7 +297,7 @@ where
 				.map(|(result, _, _)| result)
 				.map_err(Into::into)
 			}
-			None => state_machine::new(
+			None => StateMachine::new(
 				&state,
 				self.backend.changes_trie_storage(),
 				side_effects_handler,
@@ -354,7 +356,7 @@ where
 		(S::Transaction, <Blake2Hasher as Hasher>::Out),
 		Option<ChangesTrieTransaction<Blake2Hasher, NumberFor<Block>>>,
 	)> {
-		state_machine::new(
+		StateMachine::new(
 			state,
 			self.backend.changes_trie_storage(),
 			side_effects_handler,

substrate/core/client/src/client.rs

@@ -26,10 +26,9 @@ use parking_lot::{Mutex, RwLock};
 use codec::{Encode, Decode};
 use hash_db::{Hasher, Prefix};
 use primitives::{
-	Blake2Hasher, H256, ChangesTrieConfiguration, convert_hash,
-	NeverNativeValue, ExecutionContext, NativeOrEncoded,
-	storage::{StorageKey, StorageData, well_known_keys},
-	offchain,
+	Blake2Hasher, H256, ChangesTrieConfiguration, convert_hash, NeverNativeValue, ExecutionContext,
+	NativeOrEncoded, storage::{StorageKey, StorageData, well_known_keys},
+	offchain::{NeverOffchainExt, self}, traits::CodeExecutor,
 };
 use substrate_telemetry::{telemetry, SUBSTRATE_INFO};
 use sr_primitives::{
@@ -41,11 +40,10 @@ use sr_primitives::{
 	},
 };
 use state_machine::{
-	DBValue, Backend as StateBackend, CodeExecutor, ChangesTrieAnchorBlockId,
-	ExecutionStrategy, ExecutionManager, prove_read, prove_child_read,
-	ChangesTrieRootsStorage, ChangesTrieStorage,
-	ChangesTrieTransaction, ChangesTrieConfigurationRange,
-	key_changes, key_changes_proof, OverlayedChanges, NeverOffchainExt,
+	DBValue, Backend as StateBackend, ChangesTrieAnchorBlockId, ExecutionStrategy, ExecutionManager,
+	prove_read, prove_child_read, ChangesTrieRootsStorage, ChangesTrieStorage,
+	ChangesTrieTransaction, ChangesTrieConfigurationRange, key_changes, key_changes_proof,
+	OverlayedChanges,
 };
 use executor::{RuntimeVersion, RuntimeInfo};
 use consensus::{

substrate/core/client/src/error.rs

@@ -131,9 +131,7 @@ impl Error {
 	}
 
 	/// Chain a state error.
-	pub fn from_state(e: Box<dyn state_machine::Error + Send>) -> Self {
+	pub fn from_state(e: Box<dyn state_machine::Error>) -> Self {
 		Error::Execution(e)
 	}
 }
-
-impl state_machine::Error for Error {}

substrate/core/client/src/genesis.rs

@@ -24,7 +24,10 @@ pub fn construct_genesis_block<
 > (
 	state_root: Block::Hash
 ) -> Block {
-	let extrinsics_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(::std::iter::empty::<(&[u8], &[u8])>());
+	let extrinsics_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
+		std::iter::empty::<(&[u8], &[u8])>(),
+	);
+
 	Block::new(
 		<<Block as BlockT>::Header as HeaderT>::new(
 			Zero::zero(),
@@ -41,14 +44,16 @@ pub fn construct_genesis_block<
 mod tests {
 	use codec::{Encode, Decode, Joiner};
 	use executor::native_executor_instance;
-	use state_machine::{self, OverlayedChanges, ExecutionStrategy, InMemoryChangesTrieStorage};
+	use state_machine::{
+		StateMachine, OverlayedChanges, ExecutionStrategy, InMemoryChangesTrieStorage,
+	};
 	use state_machine::backend::InMemory;
 	use test_client::{
 		runtime::genesismap::{GenesisConfig, insert_genesis_block},
 		runtime::{Hash, Transfer, Block, BlockNumber, Header, Digest},
 		AccountKeyring, Sr25519Keyring,
 	};
-	use primitives::{Blake2Hasher, map};
+	use primitives::{Blake2Hasher, map, offchain::NeverOffchainExt};
 	use hex::*;
 
 	native_executor_instance!(
@@ -85,10 +90,10 @@ mod tests {
 		let hash = header.hash();
 		let mut overlay = OverlayedChanges::default();
 
-		state_machine::new(
+		StateMachine::new(
 			backend,
 			Some(&InMemoryChangesTrieStorage::<_, u64>::new()),
-			state_machine::NeverOffchainExt::new(),
+			NeverOffchainExt::new(),
 			&mut overlay,
 			&executor(),
 			"Core_initialize_block",
@@ -99,10 +104,10 @@ mod tests {
 		).unwrap();
 
 		for tx in transactions.iter() {
-			state_machine::new(
+			StateMachine::new(
 				backend,
 				Some(&InMemoryChangesTrieStorage::<_, u64>::new()),
-				state_machine::NeverOffchainExt::new(),
+				NeverOffchainExt::new(),
 				&mut overlay,
 				&executor(),
 				"BlockBuilder_apply_extrinsic",
@@ -113,10 +118,10 @@ mod tests {
 			).unwrap();
 		}
 
-		let (ret_data, _, _) = state_machine::new(
+		let (ret_data, _, _) = StateMachine::new(
 			backend,
 			Some(&InMemoryChangesTrieStorage::<_, u64>::new()),
-			state_machine::NeverOffchainExt::new(),
+			NeverOffchainExt::new(),
 			&mut overlay,
 			&executor(),
 			"BlockBuilder_finalize_block",
@@ -161,10 +166,10 @@ mod tests {
 		let (b1data, _b1hash) = block1(genesis_hash, &backend);
 
 		let mut overlay = OverlayedChanges::default();
-		let _ = state_machine::new(
+		let _ = StateMachine::new(
 			&backend,
 			Some(&InMemoryChangesTrieStorage::<_, u64>::new()),
-			state_machine::NeverOffchainExt::new(),
+			NeverOffchainExt::new(),
 			&mut overlay,
 			&executor(),
 			"Core_execute_block",
@@ -191,10 +196,10 @@ mod tests {
 		let (b1data, _b1hash) = block1(genesis_hash, &backend);
 
 		let mut overlay = OverlayedChanges::default();
-		let _ = state_machine::new(
+		let _ = StateMachine::new(
 			&backend,
 			Some(&InMemoryChangesTrieStorage::<_, u64>::new()),
-			state_machine::NeverOffchainExt::new(),
+			NeverOffchainExt::new(),
 			&mut overlay,
 			&executor(),
 			"Core_execute_block",
@@ -221,10 +226,10 @@ mod tests {
 		let (b1data, _b1hash) = block1(genesis_hash, &backend);
 
 		let mut overlay = OverlayedChanges::default();
-		let r = state_machine::new(
+		let r = StateMachine::new(
 			&backend,
 			Some(&InMemoryChangesTrieStorage::<_, u64>::new()),
-			state_machine::NeverOffchainExt::new(),
+			NeverOffchainExt::new(),
 			&mut overlay,
 			&executor(),
 			"Core_execute_block",

substrate/core/client/src/lib.rs

@@ -121,7 +121,7 @@ pub use crate::client::{
 #[cfg(feature = "std")]
 pub use crate::notifications::{StorageEventStream, StorageChangeSet};
 #[cfg(feature = "std")]
-pub use state_machine::{ExecutionStrategy, NeverOffchainExt};
+pub use state_machine::ExecutionStrategy;
 #[cfg(feature = "std")]
 pub use crate::leaves::LeafSet;
 

substrate/core/client/src/light/call_executor.rs

@@ -23,13 +23,15 @@ use std::{
 };
 
 use codec::{Encode, Decode};
-use primitives::{offchain, H256, Blake2Hasher, convert_hash, NativeOrEncoded};
+use primitives::{
+	offchain::{self, NeverOffchainExt}, H256, Blake2Hasher, convert_hash, NativeOrEncoded,
+	traits::CodeExecutor,
+};
 use sr_primitives::generic::BlockId;
 use sr_primitives::traits::{One, Block as BlockT, Header as HeaderT, NumberFor};
 use state_machine::{
-	self, Backend as StateBackend, CodeExecutor, OverlayedChanges,
-	ExecutionStrategy, ChangesTrieTransaction, create_proof_check_backend,
-	execution_proof_check_on_trie_backend, ExecutionManager, NeverOffchainExt
+	self, Backend as StateBackend, OverlayedChanges, ExecutionStrategy, create_proof_check_backend,
+	execution_proof_check_on_trie_backend, ExecutionManager, ChangesTrieTransaction,
 };
 use hash_db::Hasher;
 
@@ -451,7 +453,7 @@ pub fn prove_execution<Block, S, E>(
 pub fn check_execution_proof<Header, E, H>(
 	executor: &E,
 	request: &RemoteCallRequest<Header>,
-	remote_proof: Vec<Vec<u8>>
+	remote_proof: Vec<Vec<u8>>,
 ) -> ClientResult<Vec<u8>>
 	where
 		Header: HeaderT,
@@ -482,16 +484,14 @@ pub fn check_execution_proof<Header, E, H>(
 	)?;
 
 	// execute method
-	let local_result = execution_proof_check_on_trie_backend::<H, _>(
+	execution_proof_check_on_trie_backend::<H, _>(
 		&trie_backend,
 		&mut changes,
 		executor,
 		&request.method,
 		&request.call_data,
 		None,
-	)?;
-
-	Ok(local_result)
+	).map_err(Into::into)
 }
 
 #[cfg(test)]
@@ -568,8 +568,14 @@ mod tests {
 		backend.blockchain().insert(hash0, header0, None, None, NewBlockState::Final).unwrap();
 		backend.blockchain().insert(hash1, header1, None, None, NewBlockState::Final).unwrap();
 
-		let local_executor = RemoteCallExecutor::new(Arc::new(backend.blockchain().clone()), Arc::new(OkCallFetcher::new(vec![1])));
-		let remote_executor = RemoteCallExecutor::new(Arc::new(backend.blockchain().clone()), Arc::new(OkCallFetcher::new(vec![2])));
+		let local_executor = RemoteCallExecutor::new(
+			Arc::new(backend.blockchain().clone()),
+			Arc::new(OkCallFetcher::new(vec![1])),
+		);
+		let remote_executor = RemoteCallExecutor::new(
+			Arc::new(backend.blockchain().clone()),
+			Arc::new(OkCallFetcher::new(vec![2])),
+		);
 		let remote_or_local = RemoteOrLocalCallExecutor::new(backend, remote_executor, local_executor);
 		assert_eq!(
 			remote_or_local.call(

substrate/core/client/src/light/fetcher.rs

@@ -23,16 +23,15 @@ use std::future::Future;
 
 use hash_db::{HashDB, Hasher, EMPTY_PREFIX};
 use codec::{Decode, Encode};
-use primitives::{ChangesTrieConfiguration, convert_hash};
+use primitives::{ChangesTrieConfiguration, convert_hash, traits::CodeExecutor};
 use sr_primitives::traits::{
 	Block as BlockT, Header as HeaderT, Hash, HashFor, NumberFor,
 	SimpleArithmetic, CheckedConversion, Zero,
 };
 use state_machine::{
-	CodeExecutor, ChangesTrieRootsStorage,
-	ChangesTrieAnchorBlockId, ChangesTrieConfigurationRange,
-	TrieBackend, read_proof_check, key_changes_proof_check,
-	create_proof_check_backend_storage, read_child_proof_check,
+	ChangesTrieRootsStorage, ChangesTrieAnchorBlockId, ChangesTrieConfigurationRange,
+	TrieBackend, read_proof_check, key_changes_proof_check, create_proof_check_backend_storage,
+	read_child_proof_check,
 };
 
 use crate::cht;

substrate/core/client/src/light/mod.rs

@@ -24,10 +24,9 @@ pub mod fetcher;
 use std::sync::Arc;
 
 use executor::RuntimeInfo;
-use primitives::{H256, Blake2Hasher};
+use primitives::{H256, Blake2Hasher, traits::CodeExecutor};
 use sr_primitives::BuildStorage;
 use sr_primitives::traits::Block as BlockT;
-use state_machine::CodeExecutor;
 
 use crate::call_executor::LocalCallExecutor;
 use crate::client::Client;