Merge branch 'master' into ao-past-session-slashing-runtime

* master:
  PVF: instantiate runtime from bytes (#7270)

node/core/pvf/worker/Cargo.toml

@@ -43,7 +43,6 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate"
 [dev-dependencies]
 adder = { package = "test-parachain-adder", path = "../../../../parachain/test-parachains/adder" }
 halt = { package = "test-parachain-halt", path = "../../../../parachain/test-parachains/halt" }
-tempfile = "3.3.0"
 
 [features]
 jemalloc-allocator = ["dep:tikv-jemalloc-ctl"]

node/core/pvf/worker/src/execute.rs

@@ -31,7 +31,7 @@ use polkadot_node_core_pvf::{
 };
 use polkadot_parachain::primitives::ValidationResult;
 use std::{
-	path::{Path, PathBuf},
+	path::PathBuf,
 	sync::{mpsc::channel, Arc},
 	time::Duration,
 };
@@ -97,6 +97,20 @@ pub fn worker_entrypoint(socket_path: &str, node_version: Option<&str>) {
 				artifact_path.display(),
 			);
 
+			// Get the artifact bytes.
+			//
+			// We do this outside the thread so that we can lock down filesystem access there.
+			let compiled_artifact_blob = match std::fs::read(artifact_path) {
+				Ok(bytes) => bytes,
+				Err(err) => {
+					let response = Response::InternalError(
+						InternalValidationError::CouldNotOpenFile(err.to_string()),
+					);
+					send_response(&mut stream, response).await?;
+					continue
+				},
+			};
+
 			// Conditional variable to notify us when a thread is done.
 			let condvar = thread::get_condvar();
 
@@ -116,7 +130,12 @@ pub fn worker_entrypoint(socket_path: &str, node_version: Option<&str>) {
 			let execute_thread = thread::spawn_worker_thread_with_stack_size(
 				"execute thread",
 				move || {
-					validate_using_artifact(&artifact_path, &params, executor_2, cpu_time_start)
+					validate_using_artifact(
+						&compiled_artifact_blob,
+						&params,
+						executor_2,
+						cpu_time_start,
+					)
 				},
 				Arc::clone(&condvar),
 				WaitOutcome::Finished,
@@ -167,23 +186,16 @@ pub fn worker_entrypoint(socket_path: &str, node_version: Option<&str>) {
 }
 
 fn validate_using_artifact(
-	artifact_path: &Path,
+	compiled_artifact_blob: &[u8],
 	params: &[u8],
 	executor: Executor,
 	cpu_time_start: ProcessTime,
 ) -> Response {
-	// Check here if the file exists, because the error from Substrate is not match-able.
-	// TODO: Re-evaluate after <https://github.com/paritytech/substrate/issues/13860>.
-	let file_metadata = std::fs::metadata(artifact_path);
-	if let Err(err) = file_metadata {
-		return Response::InternalError(InternalValidationError::CouldNotOpenFile(err.to_string()))
-	}
-
 	let descriptor_bytes = match unsafe {
 		// SAFETY: this should be safe since the compiled artifact passed here comes from the
 		//         file created by the prepare workers. These files are obtained by calling
 		//         [`executor_intf::prepare`].
-		executor.execute(artifact_path.as_ref(), params)
+		executor.execute(compiled_artifact_blob, params)
 	} {
 		Err(err) => return Response::format_invalid("execute", &err),
 		Ok(d) => d,

node/core/pvf/worker/src/executor_intf.rs

@@ -18,16 +18,14 @@
 
 use polkadot_primitives::{ExecutorParam, ExecutorParams};
 use sc_executor_common::{
+	error::WasmError,
 	runtime_blob::RuntimeBlob,
 	wasm_runtime::{HeapAllocStrategy, InvokeMethod, WasmModule as _},
 };
-use sc_executor_wasmtime::{Config, DeterministicStackLimit, Semantics};
+use sc_executor_wasmtime::{Config, DeterministicStackLimit, Semantics, WasmtimeRuntime};
 use sp_core::storage::{ChildInfo, TrackedStorageKey};
 use sp_externalities::MultiRemovalResults;
-use std::{
-	any::{Any, TypeId},
-	path::Path,
-};
+use std::any::{Any, TypeId};
 
 // Wasmtime powers the Substrate Executor. It compiles the wasm bytecode into native code.
 // That native code does not create any stacks and just reuses the stack of the thread that
@@ -206,7 +204,7 @@ impl Executor {
 	/// Failure to adhere to these requirements might lead to crashes and arbitrary code execution.
 	pub unsafe fn execute(
 		&self,
-		compiled_artifact_path: &Path,
+		compiled_artifact_blob: &[u8],
 		params: &[u8],
 	) -> Result<Vec<u8>, String> {
 		let mut extensions = sp_externalities::Extensions::new();
@@ -216,17 +214,33 @@ impl Executor {
 		let mut ext = ValidationExternalities(extensions);
 
 		match sc_executor::with_externalities_safe(&mut ext, || {
-			let runtime = sc_executor_wasmtime::create_runtime_from_artifact::<HostFunctions>(
-				compiled_artifact_path,
-				self.config.clone(),
-			)?;
+			let runtime = self.create_runtime_from_bytes(compiled_artifact_blob)?;
 			runtime.new_instance()?.call(InvokeMethod::Export("validate_block"), params)
 		}) {
 			Ok(Ok(ok)) => Ok(ok),
 			Ok(Err(err)) | Err(err) => Err(err),
 		}
 		.map_err(|err| format!("execute error: {:?}", err))
 	}
+
+	/// Constructs the runtime for the given PVF, given the artifact bytes.
+	///
+	/// # Safety
+	///
+	/// The caller must ensure that the compiled artifact passed here was:
+	///   1) produced by [`prepare`],
+	///   2) was not modified,
+	///
+	/// Failure to adhere to these requirements might lead to crashes and arbitrary code execution.
+	pub unsafe fn create_runtime_from_bytes(
+		&self,
+		compiled_artifact_blob: &[u8],
+	) -> Result<WasmtimeRuntime, WasmError> {
+		sc_executor_wasmtime::create_runtime_from_artifact_bytes::<HostFunctions>(
+			compiled_artifact_blob,
+			self.config.clone(),
+		)
+	}
 }
 
 type HostFunctions = (

node/core/pvf/worker/src/testing.rs

@@ -33,16 +33,13 @@ pub fn validate_candidate(
 		.expect("Decompressing code failed");
 
 	let blob = prevalidate(&code)?;
-	let artifact = prepare(blob, &ExecutorParams::default())?;
-	let tmpdir = tempfile::tempdir()?;
-	let artifact_path = tmpdir.path().join("blob");
-	std::fs::write(&artifact_path, &artifact)?;
+	let compiled_artifact_blob = prepare(blob, &ExecutorParams::default())?;
 
 	let executor = Executor::new(ExecutorParams::default())?;
 	let result = unsafe {
 		// SAFETY: This is trivially safe since the artifact is obtained by calling `prepare`
 		//         and is written into a temporary directory in an unmodified state.
-		executor.execute(&artifact_path, params)?
+		executor.execute(&compiled_artifact_blob, params)?
 	};
 
 	Ok(result)