Backport: Make ValidationPool accepts execution mode to run custom co…

…mmand or in process validation (#1622) (#1675)

collator/src/lib.rs

@@ -67,7 +67,7 @@ pub use sc_network::PeerId;
 pub use service::{RuntimeApiCollection, Client};
 pub use sc_cli::SubstrateCli;
 #[cfg(not(feature = "service-rewr"))]
-use polkadot_service::{FullNodeHandles, AbstractClient};
+use polkadot_service::{FullNodeHandles, AbstractClient, ClientHandle};
 #[cfg(feature = "service-rewr")]
 use polkadot_service_new::{
 	self as polkadot_service,
@@ -76,7 +76,8 @@ use polkadot_service_new::{
 use sc_service::SpawnTaskHandle;
 use sp_core::traits::SpawnNamed;
 use sp_runtime::traits::BlakeTwo256;
-use consensus_common::SyncOracle;
+pub use consensus_common::SyncOracle;
+use sc_client_api::Backend as BackendT;
 
 const COLLATION_TIMEOUT: Duration = Duration::from_secs(30);
 
@@ -117,14 +118,20 @@ pub trait BuildParachainContext {
 	type ParachainContext: self::ParachainContext;
 
 	/// Build the `ParachainContext`.
-	fn build<SP>(
+	fn build<SP, Client, Backend, PNetwork>(
 		self,
-		client: polkadot_service::Client,
+		client: Arc<Client>,
 		spawner: SP,
-		network: impl Network + SyncOracle + Clone + 'static,
+		network: PNetwork,
 	) -> Result<Self::ParachainContext, ()>
 		where
-			SP: SpawnNamed + Clone + Send + Sync + 'static;
+			SP: SpawnNamed + Clone + Send + Sync + 'static,
+			Backend: BackendT<Block>,
+			Backend::State: sp_api::StateBackend<BlakeTwo256>,
+			Client: polkadot_service::AbstractClient<Block, Backend> + 'static,
+			Client::Api: RuntimeApiCollection<StateBackend = Backend::State>,
+			PNetwork: Network + SyncOracle + Clone + 'static,
+		;
 }
 
 /// Parachain context needed for collation.
@@ -194,11 +201,12 @@ pub async fn collate<P>(
 	Some(collation)
 }
 
+/// Build a collator service based on the `ClientHandle`.
 #[cfg(feature = "service-rewr")]
-fn build_collator_service<P>(
+pub fn build_collator_service<P>(
 	spawner: SpawnTaskHandle,
 	handles: FullNodeHandles,
-	client: polkadot_service::Client,
+	client: impl ClientHandle,
 	para_id: ParaId,
 	key: Arc<CollatorPair>,
 	build_parachain_context: P,
@@ -217,7 +225,6 @@ struct BuildCollationWork<P> {
 	key: Arc<CollatorPair>,
 	build_parachain_context: P,
 	spawner: SpawnTaskHandle,
-	client: polkadot_service::Client,
 }
 
 impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
@@ -233,7 +240,7 @@ impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
 		Backend: sc_client_api::Backend<Block>,
 		Backend::State: sp_api::StateBackend<BlakeTwo256>,
 		Api: RuntimeApiCollection<StateBackend = Backend::State>,
-		Client: AbstractClient<Block, Backend, Api = Api> + 'static
+		Client: AbstractClient<Block, Backend, Api = Api> + 'static,
 	{
 		let polkadot_network = self.handles
 			.polkadot_network
@@ -246,7 +253,7 @@ impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
 			.ok_or_else(|| "Collator cannot run when validation networking has not been started")?;
 
 		let parachain_context = match self.build_parachain_context.build(
-			self.client,
+			client.clone(),
 			self.spawner.clone(),
 			polkadot_network.clone(),
 		) {
@@ -346,11 +353,12 @@ impl<P> polkadot_service::ExecuteWithClient for BuildCollationWork<P>
 	}
 }
 
+/// Build a collator service based on the `ClientHandle`.
 #[cfg(not(feature = "service-rewr"))]
-fn build_collator_service<P>(
+pub fn build_collator_service<P>(
 	spawner: SpawnTaskHandle,
 	handles: FullNodeHandles,
-	client: polkadot_service::Client,
+	client: impl ClientHandle,
 	para_id: ParaId,
 	key: Arc<CollatorPair>,
 	build_parachain_context: P,
@@ -366,7 +374,6 @@ fn build_collator_service<P>(
 		key,
 		build_parachain_context,
 		spawner,
-		client: client.clone(),
 	})
 }
 
@@ -454,12 +461,20 @@ mod tests {
 	impl BuildParachainContext for BuildDummyParachainContext {
 		type ParachainContext = DummyParachainContext;
 
-		fn build<SP>(
+		fn build<SP, Client, Backend, PNetwork>(
 			self,
-			_: polkadot_service::Client,
+			_: Arc<Client>,
 			_: SP,
-			_: impl Network + Clone + 'static,
-		) -> Result<Self::ParachainContext, ()> {
+			_: PNetwork,
+		) -> Result<Self::ParachainContext, ()>
+		where
+			SP: SpawnNamed + Clone + Send + Sync + 'static,
+			Backend: BackendT<Block>,
+			Backend::State: sp_api::StateBackend<BlakeTwo256>,
+			Client: polkadot_service::AbstractClient<Block, Backend> + 'static,
+			Client::Api: RuntimeApiCollection<StateBackend = Backend::State>,
+			PNetwork: Network + SyncOracle + Clone + 'static,
+		{
 			Ok(DummyParachainContext)
 		}
 	}

node/core/candidate-validation/src/lib.rs

@@ -33,8 +33,10 @@ use polkadot_primitives::v1::{
 	ValidationCode, OmittedValidationData, PoV, CandidateDescriptor, LocalValidationData,
 	GlobalValidationData, OccupiedCoreAssumption, Hash, validation_data_hash,
 };
-use polkadot_parachain::wasm_executor::{self, ValidationPool, ExecutionMode, ValidationError,
-	InvalidCandidate as WasmInvalidCandidate};
+use polkadot_parachain::wasm_executor::{
+	self, ValidationPool, ExecutionMode, ValidationError,
+	InvalidCandidate as WasmInvalidCandidate, ValidationExecutionMode,
+};
 use polkadot_parachain::primitives::{ValidationResult as WasmValidationResult, ValidationParams};
 
 use parity_scale_codec::Encode;
@@ -73,7 +75,7 @@ async fn run(
 )
 	-> SubsystemResult<()>
 {
-	let pool = ValidationPool::new();
+	let pool = ValidationPool::new(ValidationExecutionMode::ExternalProcessSelfHost);
 
 	loop {
 		match ctx.recv().await? {

node/test-service/src/lib.rs

@@ -26,7 +26,7 @@ use polkadot_primitives::v0::{
 	Block, Hash, CollatorId, Id as ParaId,
 };
 use polkadot_runtime_common::{parachains, registrar, BlockHashCount};
-use polkadot_service::{new_full, FullNodeHandles, AbstractClient};
+use polkadot_service::{new_full, FullNodeHandles, AbstractClient, ClientHandle, ExecuteWithClient};
 use polkadot_test_runtime::{RestrictFunctionality, Runtime, SignedExtra, SignedPayload, VERSION};
 use sc_chain_spec::ChainSpec;
 use sc_client_api::{execution_extensions::ExecutionStrategies, BlockchainEvents};
@@ -67,7 +67,7 @@ pub fn polkadot_test_new_full(
 ) -> Result<
 	(
 		TaskManager,
-		Arc<impl AbstractClient<Block, TFullBackend<Block>>>,
+		Arc<polkadot_service::FullClient<polkadot_test_runtime::RuntimeApi, PolkadotTestExecutor>>,
 		FullNodeHandles,
 		Arc<NetworkService<Block, Hash>>,
 		Arc<RpcHandlers>,
@@ -88,6 +88,15 @@ pub fn polkadot_test_new_full(
 	Ok((task_manager, client, handles, network, rpc_handlers))
 }
 
+/// A wrapper for the test client that implements `ClientHandle`.
+pub struct TestClient(pub Arc<polkadot_service::FullClient<polkadot_test_runtime::RuntimeApi, PolkadotTestExecutor>>);
+
+impl ClientHandle for TestClient {
+	fn execute_with<T: ExecuteWithClient>(&self, t: T) -> T::Output {
+		T::execute_with_client::<_, _, polkadot_service::FullBackend>(t, self.0.clone())
+	}
+}
+
 /// Create a Polkadot `Configuration`. By default an in-memory socket will be used, therefore you need to provide boot
 /// nodes if you want the future node to be connected to other nodes. The `storage_update_func` can be used to make
 /// adjustements to the runtime before the node starts.

parachain/Cargo.toml

@@ -19,7 +19,7 @@ polkadot-core-primitives = { path = "../core-primitives", default-features = fal
 derive_more = { version = "0.99.2", optional = true }
 serde = { version = "1.0.102", default-features = false, features = [ "derive" ], optional = true }
 sp-externalities = { git = "https://github.com/paritytech/substrate", branch = "rococo-branch", optional = true }
-sc-executor = { git = "https://github.com/paritytech/substrate", branch = "rococo-branch", optional = true, features = ["wasmtime"] }
+sc-executor = { git = "https://github.com/paritytech/substrate", branch = "rococo-branch", optional = true }
 sp-io = { git = "https://github.com/paritytech/substrate", branch = "rococo-branch", optional = true }
 parking_lot = { version = "0.10.0", optional = true }
 log = { version = "0.4.8", optional = true }

parachain/src/wasm_executor/mod.rs

@@ -28,7 +28,7 @@ use sp_externalities::Extensions;
 use sp_wasm_interface::HostFunctions as _;
 
 #[cfg(not(any(target_os = "android", target_os = "unknown")))]
-pub use validation_host::{run_worker, ValidationPool, EXECUTION_TIMEOUT_SEC};
+pub use validation_host::{run_worker, ValidationPool, EXECUTION_TIMEOUT_SEC, ValidationExecutionMode};
 
 mod validation_host;
 
@@ -66,8 +66,6 @@ pub enum ExecutionMode<'a> {
 	Local,
 	/// Remote execution in a spawned process.
 	Remote(&'a ValidationPool),
-	/// Remote execution in a spawned test runner.
-	RemoteTest(&'a ValidationPool),
 }
 
 #[derive(Debug, derive_more::Display, derive_more::From)]
@@ -143,11 +141,7 @@ pub fn validate_candidate(
 		},
 		#[cfg(not(any(target_os = "android", target_os = "unknown")))]
 		ExecutionMode::Remote(pool) => {
-			pool.validate_candidate(validation_code, params, false)
-		},
-		#[cfg(not(any(target_os = "android", target_os = "unknown")))]
-		ExecutionMode::RemoteTest(pool) => {
-			pool.validate_candidate(validation_code, params, true)
+			pool.validate_candidate(validation_code, params)
 		},
 		#[cfg(any(target_os = "android", target_os = "unknown"))]
 		ExecutionMode::Remote(_pool) =>
@@ -156,13 +150,6 @@ pub fn validate_candidate(
 					"Remote validator not available".to_string()
 				) as Box<_>
 			))),
-		#[cfg(any(target_os = "android", target_os = "unknown"))]
-		ExecutionMode::RemoteTest(_pool) =>
-			Err(ValidationError::Internal(InternalError::System(
-				Box::<dyn std::error::Error + Send + Sync>::from(
-					"Remote validator not available".to_string()
-				) as Box<_>
-			))),
 	}
 }