PVF: Incorporate wasmtime version in worker version checks

polkadot/build.rs

@@ -16,6 +16,7 @@
 
 fn main() {
 	substrate_build_script_utils::generate_cargo_keys();
+	substrate_build_script_utils::generate_wasmtime_version();
 	// For the node/worker version check, make sure we always rebuild the node and binary workers
 	// when the version changes.
 	substrate_build_script_utils::rerun_if_git_head_changed();

polkadot/cli/src/command.rs

@@ -184,6 +184,7 @@ pub fn run_node(
 fn run_node_inner<F>(
 	cli: Cli,
 	overseer_gen: impl service::OverseerGen,
+	node_version: Option<String>,
 	maybe_malus_finality_delay: Option<u32>,
 	logger_hook: F,
 ) -> Result<()>
@@ -235,8 +236,7 @@ where
 		None
 	};
 
-	let node_version =
-		if cli.run.disable_worker_version_check { None } else { Some(NODE_VERSION.to_string()) };
+	let node_version = if cli.run.disable_worker_version_check { None } else { node_version };
 
 	let secure_validator_mode = cli.run.base.validator && !cli.run.insecure_validator;
 
@@ -282,7 +282,7 @@ where
 }
 
 /// Parses polkadot specific CLI arguments and run the service.
-pub fn run() -> Result<()> {
+pub fn run(node_version: String) -> Result<()> {
 	let cli: Cli = Cli::from_args();
 
 	#[cfg(feature = "pyroscope")]
@@ -313,6 +313,7 @@ pub fn run() -> Result<()> {
 		None => run_node_inner(
 			cli,
 			service::RealOverseerGen,
+			Some(node_version),
 			None,
 			polkadot_node_metrics::logger_hook(),
 		),

polkadot/node/core/pvf/Cargo.toml

@@ -31,7 +31,6 @@ polkadot-parachain-primitives = { path = "../../../parachain" }
 polkadot-core-primitives = { path = "../../../core-primitives" }
 polkadot-node-core-pvf-common = { path = "common" }
 polkadot-node-metrics = { path = "../../metrics" }
-polkadot-node-primitives = { path = "../../primitives" }
 polkadot-node-subsystem = { path = "../../subsystem" }
 polkadot-primitives = { path = "../../../primitives" }
 
@@ -60,6 +59,9 @@ procfs = "0.16.0"
 rusty-fork = "0.3.0"
 sc-sysinfo = { path = "../../../../substrate/client/sysinfo" }
 
+[build-dependencies]
+substrate-build-script-utils = { path = "../../../../substrate/utils/build-script-utils" }
+
 [[bench]]
 name = "host_prepare_rococo_runtime"
 harness = false

polkadot/node/core/pvf/build.rs

@@ -15,6 +15,8 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 fn main() {
+	substrate_build_script_utils::generate_wasmtime_version();
+
 	if let Ok(profile) = std::env::var("PROFILE") {
 		println!(r#"cargo:rustc-cfg=build_type="{}""#, profile);
 	}

polkadot/node/core/pvf/common/Cargo.toml

@@ -19,6 +19,7 @@ thiserror = "1.0.31"
 
 parity-scale-codec = { version = "3.6.1", default-features = false, features = ["derive"] }
 
+polkadot-node-primitives = { path = "../../../primitives" }
 polkadot-parachain-primitives = { path = "../../../../parachain" }
 polkadot-primitives = { path = "../../../../primitives" }
 
@@ -39,9 +40,6 @@ seccompiler = "0.4.0"
 assert_matches = "1.4.0"
 tempfile = "3.3.0"
 
-[build-dependencies]
-substrate-build-script-utils = { path = "../../../../../substrate/utils/build-script-utils" }
-
 [features]
 # This feature is used to export test code to other crates without putting it in the production build.
 test-utils = []

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

@@ -31,9 +31,8 @@ pub use sp_tracing;
 
 const LOG_TARGET: &str = "parachain::pvf-common";
 
-pub const RUNTIME_VERSION: &str = env!("SUBSTRATE_WASMTIME_VERSION");
-
 use parity_scale_codec::{Decode, Encode};
+use polkadot_node_primitives::NODE_VERSION;
 use std::{
 	io::{self, Read, Write},
 	mem,
@@ -47,6 +46,26 @@ pub mod tests {
 	pub const TEST_PREPARATION_TIMEOUT: Duration = Duration::from_secs(30);
 }
 
+const RUNTIME_PREFIX: &str = "wasmtime_v";
+const NODE_PREFIX: &str = "polkadot_v";
+
+/// Returns the logical PVF version. This is the version used when version-checking PVF artifacts
+/// and in the node-worker version check.
+///
+/// NOTE: This should only ever be called in a top-level crate, to accurately get the versions of
+/// the node and workers. The version should then be passed down to the crates that need it. If this
+/// were called in the same place it was used, then the version would be compiled-in at that calling
+/// crate. But that crate may be stale by the time that the top-level binary is compiled, which can
+/// result in an old version being compiled-in and ultimately a version mismatch.
+///
+/// # Parameters
+///
+/// - runtime_version: This should come from an env var, which must be compiled-in at the call site
+///   and passed in, not compiled-in here. See above.
+pub fn logical_node_version(runtime_version: &str) -> String {
+	format!("{}{}_{}{}", RUNTIME_PREFIX, runtime_version, NODE_PREFIX, NODE_VERSION)
+}
+
 /// Status of security features on the current system.
 #[derive(Debug, Clone, Default, PartialEq, Eq, Encode, Decode)]
 pub struct SecurityStatus {

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

@@ -35,13 +35,13 @@ use std::{
 /// spawning the desired worker.
 #[macro_export]
 macro_rules! decl_worker_main {
-	($expected_command:expr, $entrypoint:expr, $worker_version:expr, $worker_version_hash:expr $(,)*) => {
+	($entrypoint:expr, $worker_version:expr, $worker_version_hash:expr $(,)*) => {
 		fn get_full_version() -> String {
 			format!("{}-{}", $worker_version, $worker_version_hash)
 		}
 
-		fn print_help(expected_command: &str) {
-			println!("{} {}", expected_command, $worker_version);
+		fn print_help(binary_name: &str) {
+			println!("{} {}", binary_name, $worker_version);
 			println!("commit: {}", $worker_version_hash);
 			println!();
 			println!("PVF worker that is called by polkadot.");
@@ -54,16 +54,22 @@ macro_rules! decl_worker_main {
 			$crate::sp_tracing::try_init_simple();
 
 			let worker_pid = std::process::id();
+			let current_exe = std::env::current_exe().expect("this is an exe");
+			let binary_name = current_exe
+				.file_name()
+				.expect("the exe has a file name")
+				.to_str()
+				.expect("the worker file names are valid Unicode");
 
 			let args = std::env::args().collect::<Vec<_>>();
 			if args.len() == 1 {
-				print_help($expected_command);
+				print_help(binary_name);
 				return
 			}
 
 			match args[1].as_ref() {
 				"--help" | "-h" => {
-					print_help($expected_command);
+					print_help(binary_name);
 					return
 				},
 				"--version" | "-v" => {
@@ -125,22 +131,14 @@ macro_rules! decl_worker_main {
 					return
 				},
 
-				subcommand => {
-					// Must be passed for compatibility with the single-binary test workers.
-					if subcommand != $expected_command {
-						panic!(
-							"trying to run {} binary with the {} subcommand",
-							$expected_command, subcommand
-						)
-					}
-				},
+				_ => (),
 			}
 
 			let mut socket_path = None;
 			let mut worker_dir_path = None;
 			let mut node_version = None;
 
-			let mut i = 2;
+			let mut i = 1;
 			while i < args.len() {
 				match args[i].as_ref() {
 					"--socket-path" => {
@@ -166,7 +164,7 @@ macro_rules! decl_worker_main {
 			let socket_path = std::path::Path::new(socket_path).to_owned();
 			let worker_dir_path = std::path::Path::new(worker_dir_path).to_owned();
 
-			$entrypoint(socket_path, worker_dir_path, node_version, Some($worker_version));
+			$entrypoint(socket_path, worker_dir_path, node_version, Some(&$worker_version));
 		}
 	};
 }
@@ -203,7 +201,7 @@ pub struct WorkerInfo {
 }
 
 // NOTE: The worker version must be passed in so that we accurately get the version of the worker,
-// and not the version that this crate was compiled with.
+// and not the version that this crate was compiled with. See [`crate::logical_node_version`].
 //
 // NOTE: This must not spawn any threads due to safety requirements in `event_loop` and to avoid
 // errors in [`security::change_root::try_restrict`].