Cache the RuntimeVersion for code blocks, too (#283)

* Refactor to provide performance improvements for RuntimeVersion lookup, too.

Provides a new runtime_version function which, given a specific , returns
the runtime version for that block, either cached or by executing the block
and caching the result. Exposes that function through all clients to make
most use of that performance improvements.

substrate/substrate/client/src/call_executor.rs

@@ -17,7 +17,8 @@
 use std::sync::Arc;
 use runtime_primitives::generic::BlockId;
 use runtime_primitives::traits::Block as BlockT;
-use state_machine::{self, OverlayedChanges, Backend as StateBackend, CodeExecutor};
+use state_machine::{self, OverlayedChanges, Ext, Backend as StateBackend, CodeExecutor};
+use runtime_io::Externalities;
 use executor::{RuntimeVersion, RuntimeInfo};
 
 use backend;
@@ -42,6 +43,11 @@ pub trait CallExecutor<B: BlockT> {
 	/// No changes are made.
 	fn call(&self, id: &BlockId<B>, method: &str, call_data: &[u8]) -> Result<CallResult, error::Error>;
 
+	/// Extract RuntimeVersion of given block
+	///
+	/// No changes are made.
+	fn runtime_version(&self, id: &BlockId<B>) -> Result<RuntimeVersion, error::Error>;
+
 	/// Execute a call to a contract on top of given state.
 	///
 	/// No changes are made.
@@ -94,6 +100,17 @@ impl<B, E, Block> CallExecutor<Block> for LocalCallExecutor<B, E>
 		Ok(CallResult{ return_data, changes })
 	}
 
+	fn runtime_version(&self, id: &BlockId<Block>) -> error::Result<RuntimeVersion> {
+		let mut overlay = OverlayedChanges::default();
+		let state = self.backend.state_at(*id)?;
+		let mut externalities = Ext::new(&mut overlay, &state);
+		let code = externalities.storage(b":code").ok_or(error::ErrorKind::VersionInvalid)?
+			.to_vec();
+
+		self.executor.runtime_version(&mut externalities, &code)
+			.ok_or(error::ErrorKind::VersionInvalid.into())
+	}
+
 	fn call_at_state<S: state_machine::Backend>(&self, state: &S, changes: &mut OverlayedChanges, method: &str, call_data: &[u8]) -> error::Result<(Vec<u8>, S::Transaction)> {
 		state_machine::execute(
 			state,

substrate/substrate/client/src/client.rs

@@ -216,13 +216,10 @@ impl<B, E, Block> Client<B, E, Block> where
 				.ok_or(error::ErrorKind::AuthLenInvalid.into()))
 	}
 
-	/// Get the set of authorities at a given block.
+	/// Get the RuntimeVersion at a given block.
 	pub fn runtime_version_at(&self, id: &BlockId<Block>) -> error::Result<RuntimeVersion> {
 		// TODO: Post Poc-2 return an error if version is missing
-		Ok(self.executor.call(id, "version", &[])
-			.and_then(|r| RuntimeVersion::decode(&mut &r.return_data[..])
-				.ok_or(error::ErrorKind::VersionInvalid.into()))
-			.unwrap_or_default())
+		self.executor.runtime_version(id)
 	}
 
 	/// Get call executor reference.

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

@@ -29,6 +29,7 @@ use call_executor::{CallExecutor, CallResult};
 use error::{Error as ClientError, ErrorKind as ClientErrorKind, Result as ClientResult};
 use light::fetcher::{Fetcher, RemoteCallRequest};
 use executor::RuntimeVersion;
+use codec::Slicable;
 
 /// Call executor that executes methods on remote node, querying execution proof
 /// and checking proof by re-executing locally.
@@ -66,6 +67,12 @@ impl<B, F, Block> CallExecutor<Block> for RemoteCallExecutor<B, F>
 		}).into_future().wait()
 	}
 
+	fn runtime_version(&self, id: &BlockId<Block>) -> ClientResult<RuntimeVersion> {
+		let call_result = self.call(id, "version", &[])?;
+		RuntimeVersion::decode(&mut call_result.return_data.as_slice())
+			.ok_or_else(|| ClientErrorKind::VersionInvalid.into())
+	}
+
 	fn call_at_state<S: StateBackend>(&self, _state: &S, _changes: &mut OverlayedChanges, _method: &str, _call_data: &[u8]) -> ClientResult<(Vec<u8>, S::Transaction)> {
 		Err(ClientErrorKind::NotAvailableOnLightClient.into())
 	}

substrate/substrate/executor/src/error.rs

@@ -39,6 +39,12 @@ error_chain! {
 			display("Invalid Code: {:?}", c),
 		}
 
+		/// Cound not get runtime version.
+		VersionInvalid {
+			description("Runtime version error"),
+			display("On-chain runtime does not specify version"),
+		}
+
 		/// Externalities have failed.
 		Externalities {
 			description("externalities failure"),

substrate/substrate/executor/src/lib.rs

@@ -75,4 +75,11 @@ pub use codec::Slicable;
 pub trait RuntimeInfo {
 	/// Native runtime information if any.
 	const NATIVE_VERSION: Option<RuntimeVersion>;
+
+	/// Extract RuntimeVersion of given :code block
+	fn runtime_version<E: Externalities> (
+		&self,
+		ext: &mut E,
+		code: &[u8]
+	) -> Option<RuntimeVersion>;
 }

substrate/substrate/executor/src/native_executor.rs

@@ -23,17 +23,21 @@ use std::collections::HashMap;
 use codec::Slicable;
 use twox_hash::XxHash;
 use std::hash::Hasher;
-use parking_lot::Mutex;
+use parking_lot::{Mutex, MutexGuard};
 use RuntimeInfo;
 
-// For the internal Runtime Cache
+// For the internal Runtime Cache:
+// Do we run this natively or use the given WasmModule
 enum RunWith {
-	NativeRuntime,
-	WasmRuntime(WasmModule)
+	InvalidVersion(WasmModule),
+	NativeRuntime(RuntimeVersion),
+	WasmRuntime(RuntimeVersion, WasmModule)
 }
 
+type CacheType = HashMap<u64, RunWith>;
+
 lazy_static! {
-	static ref RUNTIMES_CACHE: Mutex<HashMap<u64, RunWith>> = Mutex::new(HashMap::new());
+	static ref RUNTIMES_CACHE: Mutex<CacheType> = Mutex::new(HashMap::new());
 }
 
 // helper function to generate low-over-head caching_keys
@@ -46,6 +50,34 @@ fn gen_cache_key(code: &[u8]) -> u64 {
 	h.finish()
 }
 
+/// fetch a runtime version from the cache or if there is no cached version yet, create
+/// the runtime version entry for `code`, determines whether `RunWith::NativeRuntime`
+/// can be used by by comparing returned RuntimeVersion to `ref_version`
+fn fetch_cached_runtime_version<'a, E: Externalities>(
+	cache: &'a mut MutexGuard<CacheType>,
+	ext: &mut E,
+	code: &[u8],
+	ref_version: RuntimeVersion
+) -> &'a RunWith {
+	cache.entry(gen_cache_key(code))
+		.or_insert_with(|| {
+			let module = WasmModule::from_buffer(code).expect("all modules compiled with rustc are valid wasm code; qed");
+			let version = WasmExecutor.call_in_wasm_module(ext, &module, "version", &[]).ok()
+				.and_then(|v| RuntimeVersion::decode(&mut v.as_slice()));
+
+
+			if let Some(v) = version {
+				if ref_version.can_call_with(&v) {
+					RunWith::NativeRuntime(v)
+				} else {
+					RunWith::WasmRuntime(v, module)
+				}
+			} else {
+				RunWith::InvalidVersion(module)
+			}
+	})
+}
+
 fn safe_call<F, U>(f: F) -> Result<U>
 	where F: ::std::panic::UnwindSafe + FnOnce() -> U
 {
@@ -88,7 +120,7 @@ impl<D: NativeExecutionDispatch + Sync + Send> NativeExecutor<D> {
 		// FIXME: set this entry at compile time
 		RUNTIMES_CACHE.lock().insert(
 			gen_cache_key(D::native_equivalent()),
-			RunWith::NativeRuntime);
+			RunWith::NativeRuntime(D::VERSION));
 
 		NativeExecutor {
 			_dummy: Default::default(),
@@ -106,6 +138,18 @@ impl<D: NativeExecutionDispatch + Sync + Send> Clone for NativeExecutor<D> {
 
 impl<D: NativeExecutionDispatch + Sync + Send> RuntimeInfo for NativeExecutor<D> {
 	const NATIVE_VERSION: Option<RuntimeVersion> = Some(D::VERSION);
+
+	fn runtime_version<E: Externalities>(
+		&self,
+		ext: &mut E,
+		code: &[u8],
+	) -> Option<RuntimeVersion> {
+		let mut c = RUNTIMES_CACHE.lock();
+		match fetch_cached_runtime_version(&mut c, ext, code, D::VERSION) {
+			RunWith::NativeRuntime(v) | RunWith::WasmRuntime(v, _) => Some(v.clone()),
+			RunWith::InvalidVersion(_m) => None
+		}
+	}
 }
 
 impl<D: NativeExecutionDispatch + Sync + Send> CodeExecutor for NativeExecutor<D> {
@@ -118,26 +162,10 @@ impl<D: NativeExecutionDispatch + Sync + Send> CodeExecutor for NativeExecutor<D
 		method: &str,
 		data: &[u8],
 	) -> Result<Vec<u8>> {
-		let mut cache = RUNTIMES_CACHE.lock();
-		let r = cache.entry(gen_cache_key(code))
-			.or_insert_with(|| {
-				let wasm_module = WasmModule::from_buffer(code)
-					.expect("all modules compiled with rustc are valid wasm code; qed");
-
-				// Missing version export is allowed in Poc-2 for Poc-1 compatibility.
-				// TODO: return an error on missing version.
-				if WasmExecutor.call_in_wasm_module(ext, &wasm_module, "version", &[]).ok()
-					.and_then(|version| RuntimeVersion::decode(&mut version.as_slice()))
-					.map_or(false, |v| D::VERSION.can_call_with(&v))
-				{
-					RunWith::NativeRuntime
-				} else {
-					RunWith::WasmRuntime(wasm_module)
-				}
-			});
-		match r {
-			RunWith::NativeRuntime => D::dispatch(ext, method, data),
-			RunWith::WasmRuntime(module) => WasmExecutor.call_in_wasm_module(ext, &module, method, data)
+		let mut c = RUNTIMES_CACHE.lock();
+		match fetch_cached_runtime_version(&mut c, ext, code, D::VERSION) {
+			RunWith::NativeRuntime(_v) => D::dispatch(ext, method, data),
+			RunWith::WasmRuntime(_, m) | RunWith::InvalidVersion(m) => WasmExecutor.call_in_wasm_module(ext, m, method, data)
 		}
 	}
 }