Retire puppet workers (#1449)

Closes #583

After the separation of PVF worker binaries, dedicated puppet workers
are not needed for tests anymore. The production workers can be used
instead, avoiding some code duplication and decreasing complexity.

The changes also make it possible to further refactor the code to
isolate workers completely.

polkadot/node/core/pvf/Cargo.toml

@@ -6,11 +6,6 @@ authors.workspace = true
 edition.workspace = true
 license.workspace = true
 
-[[bin]]
-name = "puppet_worker"
-path = "bin/puppet_worker.rs"
-required-features = ["test-utils"]
-
 [dependencies]
 always-assert = "0.1"
 futures = "0.3.21"
@@ -35,7 +30,6 @@ polkadot-primitives = { path = "../../../primitives" }
 sp-core = { path = "../../../../substrate/primitives/core" }
 sp-wasm-interface = { path = "../../../../substrate/primitives/wasm-interface" }
 sp-maybe-compressed-blob = { path = "../../../../substrate/primitives/maybe-compressed-blob" }
-sp-tracing = { path = "../../../../substrate/primitives/tracing", optional = true }
 polkadot-node-core-pvf-prepare-worker = { path = "prepare-worker", optional = true }
 polkadot-node-core-pvf-execute-worker = { path = "execute-worker", optional = true }
 
@@ -56,9 +50,7 @@ halt = { package = "test-parachain-halt", path = "../../../parachain/test-parach
 ci-only-tests = []
 jemalloc-allocator = [ "polkadot-node-core-pvf-common/jemalloc-allocator" ]
 # This feature is used to export test code to other crates without putting it in the production build.
-# This is also used by the `puppet_worker` binary.
 test-utils = [
 	"polkadot-node-core-pvf-execute-worker",
 	"polkadot-node-core-pvf-prepare-worker",
-	"sp-tracing",
 ]

polkadot/node/core/pvf/common/src/worker/mod.rs

@@ -60,6 +60,10 @@ macro_rules! decl_worker_main {
 					println!("{}", $worker_version);
 					return
 				},
+				"test-sleep" => {
+					std::thread::sleep(std::time::Duration::from_secs(5));
+					return
+				},
 				subcommand => {
 					// Must be passed for compatibility with the single-binary test workers.
 					if subcommand != $expected_command {

polkadot/node/core/pvf/src/lib.rs

@@ -100,10 +100,6 @@ mod worker_intf;
 #[cfg(feature = "test-utils")]
 pub mod testing;
 
-// Used by `decl_puppet_worker_main!`.
-#[cfg(feature = "test-utils")]
-pub use sp_tracing;
-
 pub use error::{InvalidCandidate, ValidationError};
 pub use host::{start, Config, ValidationHost, EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME};
 pub use metrics::Metrics;
@@ -117,11 +113,5 @@ pub use polkadot_node_core_pvf_common::{
 	pvf::PvfPrepData,
 };
 
-// Re-export worker entrypoints.
-#[cfg(feature = "test-utils")]
-pub use polkadot_node_core_pvf_execute_worker::worker_entrypoint as execute_worker_entrypoint;
-#[cfg(feature = "test-utils")]
-pub use polkadot_node_core_pvf_prepare_worker::worker_entrypoint as prepare_worker_entrypoint;
-
 /// The log target for this crate.
 pub const LOG_TARGET: &str = "parachain::pvf";

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

@@ -47,45 +47,3 @@ pub fn validate_candidate(
 
 	Ok(result)
 }
-
-/// Use this macro to declare a `fn main() {}` that will check the arguments and dispatch them to
-/// the appropriate worker, making the executable that can be used for spawning workers.
-#[macro_export]
-macro_rules! decl_puppet_worker_main {
-	() => {
-		fn main() {
-			$crate::sp_tracing::try_init_simple();
-
-			let args = std::env::args().collect::<Vec<_>>();
-			if args.len() == 1 {
-				panic!("wrong number of arguments");
-			}
-
-			let entrypoint = match args[1].as_ref() {
-				"exit" => {
-					std::process::exit(1);
-				},
-				"sleep" => {
-					std::thread::sleep(std::time::Duration::from_secs(5));
-					return
-				},
-				"prepare-worker" => $crate::prepare_worker_entrypoint,
-				"execute-worker" => $crate::execute_worker_entrypoint,
-				other => panic!("unknown subcommand: {}", other),
-			};
-
-			let mut node_version = None;
-			let mut socket_path: &str = "";
-
-			for i in (2..args.len()).step_by(2) {
-				match args[i].as_ref() {
-					"--socket-path" => socket_path = args[i + 1].as_str(),
-					"--node-impl-version" => node_version = Some(args[i + 1].as_str()),
-					arg => panic!("Unexpected argument found: {}", arg),
-				}
-			}
-
-			entrypoint(&socket_path, node_version, None);
-		}
-	};
-}

polkadot/node/core/pvf/tests/README.md

@@ -0,0 +1,9 @@
+# PVF host integration tests
+
+## Testing
+
+Before running these tests, make sure the worker binaries are built first. This can be done with:
+
+```sh
+cargo build --bin polkadot-execute-worker --bin polkadot-prepare-worker
+```

polkadot/node/core/pvf/tests/it/main.rs

@@ -33,7 +33,6 @@ use tokio::sync::Mutex;
 mod adder;
 mod worker_common;
 
-const PUPPET_EXE: &str = env!("CARGO_BIN_EXE_puppet_worker");
 const TEST_EXECUTION_TIMEOUT: Duration = Duration::from_secs(3);
 const TEST_PREPARATION_TIMEOUT: Duration = Duration::from_secs(3);
 
@@ -51,10 +50,20 @@ impl TestHost {
 	where
 		F: FnOnce(&mut Config),
 	{
+		let mut workers_path = std::env::current_exe().unwrap();
+		workers_path.pop();
+		workers_path.pop();
+		let mut prepare_worker_path = workers_path.clone();
+		prepare_worker_path.push("polkadot-prepare-worker");
+		let mut execute_worker_path = workers_path.clone();
+		execute_worker_path.push("polkadot-execute-worker");
 		let cache_dir = tempfile::tempdir().unwrap();
-		let program_path = std::path::PathBuf::from(PUPPET_EXE);
-		let mut config =
-			Config::new(cache_dir.path().to_owned(), None, program_path.clone(), program_path);
+		let mut config = Config::new(
+			cache_dir.path().to_owned(),
+			None,
+			prepare_worker_path,
+			execute_worker_path,
+		);
 		f(&mut config);
 		let (host, task) = start(config, Metrics::default());
 		let _ = tokio::task::spawn(task);

polkadot/node/core/pvf/tests/it/worker_common.rs

@@ -14,26 +14,41 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
+use polkadot_node_core_pvf::testing::{spawn_with_program_path, SpawnErr};
 use std::time::Duration;
 
-use polkadot_node_core_pvf::testing::{spawn_with_program_path, SpawnErr};
-
-use crate::PUPPET_EXE;
+fn worker_path(name: &str) -> std::path::PathBuf {
+	let mut worker_path = std::env::current_exe().unwrap();
+	worker_path.pop();
+	worker_path.pop();
+	worker_path.push(name);
+	worker_path
+}
 
 // Test spawning a program that immediately exits with a failure code.
 #[tokio::test]
 async fn spawn_immediate_exit() {
-	let result =
-		spawn_with_program_path("integration-test", PUPPET_EXE, &["exit"], Duration::from_secs(2))
-			.await;
+	// There's no explicit `exit` subcommand in the worker; it will panic on an unknown
+	// subcommand anyway
+	let result = spawn_with_program_path(
+		"integration-test",
+		worker_path("polkadot-prepare-worker"),
+		&["exit"],
+		Duration::from_secs(2),
+	)
+	.await;
 	assert!(matches!(result, Err(SpawnErr::AcceptTimeout)));
 }
 
 #[tokio::test]
 async fn spawn_timeout() {
-	let result =
-		spawn_with_program_path("integration-test", PUPPET_EXE, &["sleep"], Duration::from_secs(2))
-			.await;
+	let result = spawn_with_program_path(
+		"integration-test",
+		worker_path("polkadot-execute-worker"),
+		&["test-sleep"],
+		Duration::from_secs(2),
+	)
+	.await;
 	assert!(matches!(result, Err(SpawnErr::AcceptTimeout)));
 }
 
@@ -41,7 +56,7 @@ async fn spawn_timeout() {
 async fn should_connect() {
 	let _ = spawn_with_program_path(
 		"integration-test",
-		PUPPET_EXE,
+		worker_path("polkadot-prepare-worker"),
 		&["prepare-worker"],
 		Duration::from_secs(2),
 	)