PVF: instantiate runtime from bytes

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)