Eradicate native_equivalent (#3494)

* Add ability to supply extra storage in test-client

* Don't use native_equivalent in tests.

* Get rid of native_equivalent

* Try to make fields private

* Apply Basti's suggestions

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

substrate/Cargo.lock

@@ -4988,7 +4988,6 @@ dependencies = [
  "sr-primitives 2.0.0",
  "sr-version 2.0.0",
  "substrate-client 2.0.0",
- "substrate-executor 2.0.0",
  "substrate-keystore 2.0.0",
  "substrate-network 2.0.0",
  "substrate-primitives 2.0.0",

substrate/core/client/src/genesis.rs

@@ -54,8 +54,7 @@ mod tests {
 	native_executor_instance!(
 		Executor,
 		test_client::runtime::api::dispatch,
-		test_client::runtime::native_version,
-		test_client::runtime::WASM_BINARY
+		test_client::runtime::native_version
 	);
 
 	fn executor() -> executor::NativeExecutor<Executor> {
@@ -153,6 +152,7 @@ mod tests {
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			1000,
 			None,
+			vec![],
 		).genesis_map();
 		let genesis_hash = insert_genesis_block(&mut storage);
 
@@ -181,6 +181,7 @@ mod tests {
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			1000,
 			None,
+			vec![],
 		).genesis_map();
 		let genesis_hash = insert_genesis_block(&mut storage);
 
@@ -209,6 +210,7 @@ mod tests {
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			68,
 			None,
+			vec![],
 		).genesis_map();
 		let genesis_hash = insert_genesis_block(&mut storage);
 

substrate/core/executor/src/native_executor.rs

@@ -49,9 +49,6 @@ pub fn with_native_environment<F, U>(ext: &mut dyn Externalities<Blake2Hasher>,
 
 /// Delegate for dispatching a CodeExecutor call to native code.
 pub trait NativeExecutionDispatch: Send + Sync {
-	/// Get the wasm code that the native dispatch will be equivalent to.
-	fn native_equivalent() -> &'static [u8];
-
 	/// Dispatch a method and input data to be executed natively.
 	fn dispatch(ext: &mut dyn Externalities<Blake2Hasher>, method: &str, data: &[u8]) -> Result<Vec<u8>>;
 
@@ -211,19 +208,13 @@ impl<D: NativeExecutionDispatch> CodeExecutor<Blake2Hasher> for NativeExecutor<D
 /// Implements a `NativeExecutionDispatch` for provided parameters.
 #[macro_export]
 macro_rules! native_executor_instance {
-	( $pub:vis $name:ident, $dispatcher:path, $version:path, $code:expr) => {
+	( $pub:vis $name:ident, $dispatcher:path, $version:path $(,)?) => {
 		/// A unit struct which implements `NativeExecutionDispatch` feeding in the hard-coded runtime.
 		$pub struct $name;
-		$crate::native_executor_instance!(IMPL $name, $dispatcher, $version, $code);
+		$crate::native_executor_instance!(IMPL $name, $dispatcher, $version);
 	};
-	(IMPL $name:ident, $dispatcher:path, $version:path, $code:expr) => {
+	(IMPL $name:ident, $dispatcher:path, $version:path) => {
 		impl $crate::NativeExecutionDispatch for $name {
-			fn native_equivalent() -> &'static [u8] {
-				// WARNING!!! This assumes that the runtime was built *before* the main project. Until we
-				// get a proper build script, this must be strictly adhered to or things will go wrong.
-				$code
-			}
-
 			fn dispatch(
 				ext: &mut $crate::Externalities<$crate::Blake2Hasher>,
 				method: &str,

substrate/core/rpc/Cargo.toml

@@ -18,7 +18,6 @@ codec = { package = "parity-scale-codec", version = "1.0.0" }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 client = { package = "substrate-client", path = "../client" }
-substrate-executor = { path = "../executor" }
 network = { package = "substrate-network", path = "../network" }
 primitives = { package = "substrate-primitives", path = "../primitives" }
 session = { package = "substrate-session", path = "../session" }

substrate/core/rpc/src/state/tests.rs

@@ -25,20 +25,24 @@ use test_client::{
 	consensus::BlockOrigin,
 	runtime,
 };
-use substrate_executor::NativeExecutionDispatch;
 
 #[test]
 fn should_return_storage() {
+	const KEY: &[u8] = b":mock";
+	const VALUE: &[u8] = b"hello world";
+
 	let core = tokio::runtime::Runtime::new().unwrap();
-	let client = Arc::new(test_client::new());
+	let client = TestClientBuilder::new()
+		.add_extra_storage(KEY.to_vec(), VALUE.to_vec())
+		.build();
 	let genesis_hash = client.genesis_hash();
-	let client = State::new(client, Subscriptions::new(Arc::new(core.executor())));
-	let key = StorageKey(b":code".to_vec());
+	let client = State::new(Arc::new(client), Subscriptions::new(Arc::new(core.executor())));
+	let key = StorageKey(KEY.to_vec());
 
 	assert_eq!(
 		client.storage(key.clone(), Some(genesis_hash).into())
 			.map(|x| x.map(|x| x.0.len())).unwrap().unwrap() as usize,
-		LocalExecutor::native_equivalent().len(),
+		VALUE.len(),
 	);
 	assert_matches!(
 		client.storage_hash(key.clone(), Some(genesis_hash).into()).map(|x| x.is_some()),
@@ -46,7 +50,7 @@ fn should_return_storage() {
 	);
 	assert_eq!(
 		client.storage_size(key.clone(), None).unwrap().unwrap() as usize,
-		LocalExecutor::native_equivalent().len(),
+		VALUE.len(),
 	);
 }
 

substrate/core/test-runtime/client/src/lib.rs

@@ -51,8 +51,7 @@ mod local_executor {
 	native_executor_instance!(
 		pub LocalExecutor,
 		runtime::api::dispatch,
-		runtime::native_version,
-		runtime::WASM_BINARY
+		runtime::native_version
 	);
 }
 
@@ -97,12 +96,34 @@ pub type LightExecutor = client::light::call_executor::RemoteOrLocalCallExecutor
 pub struct GenesisParameters {
 	support_changes_trie: bool,
 	heap_pages_override: Option<u64>,
+	extra_storage: Vec<(Vec<u8>, Vec<u8>)>,
+}
+
+impl GenesisParameters {
+	fn genesis_config(&self) -> GenesisConfig {
+		GenesisConfig::new(
+			self.support_changes_trie,
+			vec![
+				sr25519::Public::from(Sr25519Keyring::Alice).into(),
+				sr25519::Public::from(Sr25519Keyring::Bob).into(),
+				sr25519::Public::from(Sr25519Keyring::Charlie).into(),
+			],
+			vec![
+				AccountKeyring::Alice.into(),
+				AccountKeyring::Bob.into(),
+				AccountKeyring::Charlie.into(),
+			],
+			1000,
+			self.heap_pages_override,
+			self.extra_storage.clone(),
+		)
+	}
 }
 
 impl generic_test_client::GenesisInit for GenesisParameters {
 	fn genesis_storage(&self) -> (StorageOverlay, ChildrenStorageOverlay) {
 		use codec::Encode;
-		let mut storage = genesis_config(self.support_changes_trie, self.heap_pages_override).genesis_map();
+		let mut storage = self.genesis_config().genesis_map();
 
 		let child_roots = storage.1.iter().map(|(sk, child_map)| {
 			let state_root = <<<runtime::Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
@@ -157,6 +178,13 @@ pub trait TestClientBuilderExt<B>: Sized {
 	/// Override the default value for Wasm heap pages.
 	fn set_heap_pages(self, heap_pages: u64) -> Self;
 
+	/// Add an extra value into the genesis storage.
+	///
+	/// # Panics
+	///
+	/// Panics if the key is empty.
+	fn add_extra_storage<K: Into<Vec<u8>>, V: Into<Vec<u8>>>(self, key: K, value: V) -> Self;
+
 	/// Build the test client.
 	fn build(self) -> Client<B> {
 		self.build_with_longest_chain().0
@@ -182,28 +210,18 @@ impl<B> TestClientBuilderExt<B> for TestClientBuilder<
 		self
 	}
 
+	fn add_extra_storage<K: Into<Vec<u8>>, V: Into<Vec<u8>>>(mut self, key: K, value: V) -> Self {
+		let key = key.into();
+		assert!(!key.is_empty());
+		self.genesis_init_mut().extra_storage.push((key, value.into()));
+		self
+	}
+
 	fn build_with_longest_chain(self) -> (Client<B>, client::LongestChain<B, runtime::Block>) {
 		self.build_with_native_executor(None)
 	}
 }
 
-fn genesis_config(support_changes_trie: bool, heap_pages_override: Option<u64>) -> GenesisConfig {
-	GenesisConfig::new(
-		support_changes_trie,
-		vec![
-			sr25519::Public::from(Sr25519Keyring::Alice).into(),
-			sr25519::Public::from(Sr25519Keyring::Bob).into(),
-			sr25519::Public::from(Sr25519Keyring::Charlie).into(),
-		], vec![
-			AccountKeyring::Alice.into(),
-			AccountKeyring::Bob.into(),
-			AccountKeyring::Charlie.into(),
-		],
-		1000,
-		heap_pages_override,
-	)
-}
-
 /// Creates new client instance used for tests.
 pub fn new() -> Client<Backend> {
 	TestClientBuilder::new().build()

substrate/core/test-runtime/src/genesismap.rs

@@ -25,10 +25,12 @@ use sr_primitives::traits::{Block as BlockT, Hash as HashT, Header as HeaderT};
 
 /// Configuration of a general Substrate test genesis block.
 pub struct GenesisConfig {
-	pub changes_trie_config: Option<ChangesTrieConfiguration>,
-	pub authorities: Vec<AuthorityId>,
-	pub balances: Vec<(AccountId, u64)>,
-	pub heap_pages_override: Option<u64>,
+	changes_trie_config: Option<ChangesTrieConfiguration>,
+	authorities: Vec<AuthorityId>,
+	balances: Vec<(AccountId, u64)>,
+	heap_pages_override: Option<u64>,
+	/// Additional storage key pairs that will be added to the genesis map.
+	extra_storage: Vec<(Vec<u8>, Vec<u8>)>,
 }
 
 impl GenesisConfig {
@@ -38,6 +40,7 @@ impl GenesisConfig {
 		endowed_accounts: Vec<AccountId>,
 		balance: u64,
 		heap_pages_override: Option<u64>,
+		extra_storage: Vec<(Vec<u8>, Vec<u8>)>,
 	) -> Self {
 		GenesisConfig {
 			changes_trie_config: match support_changes_trie {
@@ -47,6 +50,7 @@ impl GenesisConfig {
 			authorities: authorities.clone(),
 			balances: endowed_accounts.into_iter().map(|a| (a, balance)).collect(),
 			heap_pages_override,
+			extra_storage,
 		}
 	}
 
@@ -70,6 +74,10 @@ impl GenesisConfig {
 			map.insert(well_known_keys::CHANGES_TRIE_CONFIG.to_vec(), changes_trie_config.encode());
 		}
 		map.insert(twox_128(&b"sys:auth"[..])[..].to_vec(), self.authorities.encode());
+		// Finally, add the extra storage entries.
+		for (key, value) in self.extra_storage.iter().cloned() {
+			map.insert(key, value);
+		}
 		(map, Default::default())
 	}
 }

substrate/node-template/src/service.rs

@@ -18,8 +18,7 @@ pub use substrate_executor::NativeExecutor;
 native_executor_instance!(
 	pub Executor,
 	node_template_runtime::api::dispatch,
-	node_template_runtime::native_version,
-	WASM_BINARY
+	node_template_runtime::native_version
 );
 
 construct_simple_protocol! {
@@ -72,7 +71,7 @@ macro_rules! new_full_start {
 
 				Ok(import_queue)
 			})?;
-		
+
 		(builder, import_setup, inherent_data_providers, tasks_to_spawn)
 	}}
 }

substrate/node/executor/src/lib.rs

@@ -30,8 +30,7 @@ use substrate_executor::native_executor_instance;
 native_executor_instance!(
 	pub Executor,
 	node_runtime::api::dispatch,
-	node_runtime::native_version,
-	node_runtime::WASM_BINARY
+	node_runtime::native_version
 );
 
 #[cfg(test)]