Switch to pooling copy-on-write instantiation strategy for WASM (companion for Substrate#11232)

node/core/pvf/Cargo.toml

@@ -19,6 +19,7 @@ slotmap = "1.0"
 gum = { package = "tracing-gum", path = "../../gum" }
 pin-project = "1.0.9"
 rand = "0.8.5"
+tempfile = "3.3.0"
 parity-scale-codec = { version = "3.1.2", default-features = false, features = ["derive"] }
 polkadot-parachain = { path = "../../../parachain" }
 polkadot-core-primitives = { path = "../../../core-primitives" }

node/core/pvf/src/artifacts.rs

@@ -17,15 +17,12 @@
 use crate::{error::PrepareError, host::PrepareResultSender};
 use always_assert::always;
 use async_std::path::{Path, PathBuf};
-use parity_scale_codec::{Decode, Encode};
 use polkadot_parachain::primitives::ValidationCodeHash;
 use std::{
 	collections::HashMap,
 	time::{Duration, SystemTime},
 };
 
-/// A wrapper for the compiled PVF code.
-#[derive(Encode, Decode)]
 pub struct CompiledArtifact(Vec<u8>);
 
 impl CompiledArtifact {

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

@@ -15,7 +15,7 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use crate::{
-	artifacts::{ArtifactPathId, CompiledArtifact},
+	artifacts::ArtifactPathId,
 	executor_intf::TaskExecutor,
 	worker_common::{
 		bytes_to_path, framed_recv, framed_send, path_to_bytes, spawn_with_program_path,
@@ -206,29 +206,12 @@ async fn validate_using_artifact(
 	params: &[u8],
 	spawner: &TaskExecutor,
 ) -> Response {
-	let artifact_bytes = match async_std::fs::read(artifact_path).await {
-		Err(e) =>
-			return Response::InternalError(format!(
-				"failed to read the artifact at {}: {:?}",
-				artifact_path.display(),
-				e,
-			)),
-		Ok(b) => b,
-	};
-
-	let artifact = match CompiledArtifact::decode(&mut artifact_bytes.as_slice()) {
-		Err(e) => return Response::InternalError(format!("artifact deserialization: {:?}", e)),
-		Ok(a) => a,
-	};
-
-	let compiled_artifact = artifact.as_ref();
-
 	let validation_started_at = Instant::now();
 	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`].
-		crate::executor_intf::execute(compiled_artifact, params, spawner.clone())
+		crate::executor_intf::execute(artifact_path.as_ref(), params, spawner.clone())
 	} {
 		Err(err) => return Response::format_invalid("execute", &err.to_string()),
 		Ok(d) => d,

node/core/pvf/src/executor_intf.rs

@@ -22,7 +22,10 @@ use sc_executor_common::{
 };
 use sc_executor_wasmtime::{Config, DeterministicStackLimit, Semantics};
 use sp_core::storage::{ChildInfo, TrackedStorageKey};
-use std::any::{Any, TypeId};
+use std::{
+	any::{Any, TypeId},
+	path::Path,
+};
 
 // Memory configuration
 //
@@ -40,15 +43,17 @@ const DEFAULT_HEAP_PAGES_ESTIMATE: u64 = 32;
 const EXTRA_HEAP_PAGES: u64 = 2048;
 
 const CONFIG: Config = Config {
-	// NOTE: This is specified in bytes, so we multiply by WASM page size.
-	max_memory_size: Some(((DEFAULT_HEAP_PAGES_ESTIMATE + EXTRA_HEAP_PAGES) * 65536) as usize),
-
 	allow_missing_func_imports: true,
 	cache_path: None,
 	semantics: Semantics {
 		extra_heap_pages: EXTRA_HEAP_PAGES,
 
-		fast_instance_reuse: false,
+		// NOTE: This is specified in bytes, so we multiply by WASM page size.
+		max_memory_size: Some(((DEFAULT_HEAP_PAGES_ESTIMATE + EXTRA_HEAP_PAGES) * 65536) as usize),
+
+		instantiation_strategy:
+			sc_executor_wasmtime::InstantiationStrategy::RecreateInstanceCopyOnWrite,
+
 		// Enable deterministic stack limit to pin down the exact number of items the wasmtime stack
 		// can contain before it traps with stack overflow.
 		//
@@ -87,7 +92,7 @@ pub fn prevalidate(code: &[u8]) -> Result<RuntimeBlob, sc_executor_common::error
 }
 
 /// Runs preparation on the given runtime blob. If successful, it returns a serialized compiled
-/// artifact which can then be used to pass into [`execute`].
+/// artifact which can then be used to pass into [`execute`] after writing it to the disk.
 pub fn prepare(blob: RuntimeBlob) -> Result<Vec<u8>, sc_executor_common::error::WasmError> {
 	sc_executor_wasmtime::prepare_runtime_artifact(blob, &CONFIG.semantics)
 }
@@ -97,10 +102,16 @@ pub fn prepare(blob: RuntimeBlob) -> Result<Vec<u8>, sc_executor_common::error::
 ///
 /// # Safety
 ///
-/// The compiled artifact must be produced with [`prepare`]. Not following this guidance can lead
-/// to arbitrary code execution.
+/// The caller must ensure that the compiled artifact passed here was:
+///   1) produced by [`prepare`],
+///   2) written to the disk as a file,
+///   3) was not modified,
+///   4) will not be modified while any runtime using this artifact is alive, or is being
+///      instantiated.
+///
+/// Failure to adhere to these requirements might lead to crashes and arbitrary code execution.
 pub unsafe fn execute(
-	compiled_artifact: &[u8],
+	compiled_artifact_path: &Path,
 	params: &[u8],
 	spawner: impl sp_core::traits::SpawnNamed + 'static,
 ) -> Result<Vec<u8>, sc_executor_common::error::Error> {
@@ -113,7 +124,7 @@ pub unsafe fn execute(
 
 	sc_executor::with_externalities_safe(&mut ext, || {
 		let runtime = sc_executor_wasmtime::create_runtime_from_artifact::<HostFunctions>(
-			compiled_artifact,
+			compiled_artifact_path,
 			CONFIG,
 		)?;
 		runtime.new_instance()?.call(InvokeMethod::Export("validate_block"), params)

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

@@ -265,15 +265,13 @@ pub fn worker_entrypoint(socket_path: &str) {
 					// worker is only required to send an empty `Ok` to the pool
 					// to indicate the success.
 
-					let artifact_bytes = compiled_artifact.encode();
-
 					gum::debug!(
 						target: LOG_TARGET,
 						worker_pid = %std::process::id(),
 						"worker: writing artifact to {}",
 						dest.display(),
 					);
-					async_std::fs::write(&dest, &artifact_bytes).await?;
+					async_std::fs::write(&dest, &compiled_artifact).await?;
 
 					Ok(())
 				},

node/core/pvf/src/testing.rs

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

node/test/service/src/lib.rs

@@ -42,7 +42,10 @@ use sc_network::{
 	multiaddr,
 };
 use sc_service::{
-	config::{DatabaseSource, KeystoreConfig, MultiaddrWithPeerId, WasmExecutionMethod},
+	config::{
+		DatabaseSource, KeystoreConfig, MultiaddrWithPeerId, WasmExecutionMethod,
+		WasmtimeInstantiationStrategy,
+	},
 	BasePath, Configuration, KeepBlocks, Role, RpcHandlers, TaskManager,
 };
 use sp_arithmetic::traits::SaturatedConversion;
@@ -174,7 +177,9 @@ pub fn node_config(
 		state_pruning: Default::default(),
 		keep_blocks: KeepBlocks::All,
 		chain_spec: Box::new(spec),
-		wasm_method: WasmExecutionMethod::Compiled,
+		wasm_method: WasmExecutionMethod::Compiled {
+			instantiation_strategy: WasmtimeInstantiationStrategy::PoolingCopyOnWrite,
+		},
 		wasm_runtime_overrides: Default::default(),
 		// NOTE: we enforce the use of the native runtime to make the errors more debuggable
 		execution_strategies: ExecutionStrategies {