From 9e1447042b648149d515d53ccdef3bd3e4e37ef6 Mon Sep 17 00:00:00 2001
From: Julian Eager <eagr@tutanota.com>
Date: Sun, 15 Oct 2023 16:39:03 +0800
Subject: [PATCH] Include polkadot version in artifact path (#1828)

closes #695

Could potentially be helpful to preserving caches when applicable, as
discussed in #685

kusama address: FvpsvV1GQAAbwqX6oyRjemgdKV11QU5bXsMg9xsonD1FLGK
---
 Cargo.lock                              |  1 +
 polkadot/cli/Cargo.toml                 |  1 +
 polkadot/cli/src/cli.rs                 | 12 +-----
 polkadot/node/core/pvf/src/artifacts.rs | 55 ++++++++++++++++++-------
 polkadot/node/primitives/src/lib.rs     |  7 ++++
 5 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index ed5e0a29f71..855700f3c20 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -11635,6 +11635,7 @@ dependencies = [
  "futures",
  "log",
  "polkadot-node-metrics",
+ "polkadot-node-primitives",
  "polkadot-service",
  "pyroscope",
  "pyroscope_pprofrs",
diff --git a/polkadot/cli/Cargo.toml b/polkadot/cli/Cargo.toml
index 8057342aaea..9cad7e88dfe 100644
--- a/polkadot/cli/Cargo.toml
+++ b/polkadot/cli/Cargo.toml
@@ -33,6 +33,7 @@ try-runtime-cli = { path = "../../substrate/utils/frame/try-runtime/cli", option
 sc-cli = { path = "../../substrate/client/cli", optional = true }
 sc-service = { path = "../../substrate/client/service", optional = true }
 polkadot-node-metrics = { path = "../node/metrics" }
+polkadot-node-primitives = { path = "../node/primitives" }
 sc-tracing = { path = "../../substrate/client/tracing", optional = true }
 sc-sysinfo = { path = "../../substrate/client/sysinfo" }
 sc-executor = { path = "../../substrate/client/executor" }
diff --git a/polkadot/cli/src/cli.rs b/polkadot/cli/src/cli.rs
index faca2b25e23..bc060c21fba 100644
--- a/polkadot/cli/src/cli.rs
+++ b/polkadot/cli/src/cli.rs
@@ -16,19 +16,11 @@
 
 //! Polkadot CLI library.
 
+pub use polkadot_node_primitives::NODE_VERSION;
+
 use clap::Parser;
 use std::path::PathBuf;
 
-/// The version of the node.
-///
-/// This is the version that is used for versioning this node binary.
-/// By default the `minor` version is bumped in every release. `Major` or `patch` releases are only
-/// expected in very rare cases.
-///
-/// The worker binaries associated to the node binary should ensure that they are using the same
-/// version as the main node that started them.
-pub const NODE_VERSION: &'static str = "1.1.0";
-
 #[allow(missing_docs)]
 #[derive(Debug, Parser)]
 pub enum Subcommand {
diff --git a/polkadot/node/core/pvf/src/artifacts.rs b/polkadot/node/core/pvf/src/artifacts.rs
index 5a1767af75b..d7b15ece7b2 100644
--- a/polkadot/node/core/pvf/src/artifacts.rs
+++ b/polkadot/node/core/pvf/src/artifacts.rs
@@ -58,6 +58,7 @@
 use crate::host::PrepareResultSender;
 use always_assert::always;
 use polkadot_node_core_pvf_common::{error::PrepareError, prepare::PrepareStats, pvf::PvfPrepData};
+use polkadot_node_primitives::NODE_VERSION;
 use polkadot_parachain_primitives::primitives::ValidationCodeHash;
 use polkadot_primitives::ExecutorParamsHash;
 use std::{
@@ -75,6 +76,7 @@ pub struct ArtifactId {
 
 impl ArtifactId {
 	const PREFIX: &'static str = "wasmtime_";
+	const NODE_VERSION_PREFIX: &'static str = "polkadot_v";
 
 	/// Creates a new artifact ID with the given hash.
 	pub fn new(code_hash: ValidationCodeHash, executor_params_hash: ExecutorParamsHash) -> Self {
@@ -92,8 +94,13 @@ impl ArtifactId {
 		use polkadot_core_primitives::Hash;
 		use std::str::FromStr as _;
 
-		let file_name = file_name.strip_prefix(Self::PREFIX)?;
-		let (code_hash_str, executor_params_hash_str) = file_name.split_once('_')?;
+		let file_name =
+			file_name.strip_prefix(Self::PREFIX)?.strip_prefix(Self::NODE_VERSION_PREFIX)?;
+
+		// [ node version | code hash | param hash ]
+		let parts: Vec<&str> = file_name.split('_').collect();
+		let (_node_ver, code_hash_str, executor_params_hash_str) = (parts[0], parts[1], parts[2]);
+
 		let code_hash = Hash::from_str(code_hash_str).ok()?.into();
 		let executor_params_hash =
 			ExecutorParamsHash::from_hash(Hash::from_str(executor_params_hash_str).ok()?);
@@ -103,8 +110,14 @@ impl ArtifactId {
 
 	/// Returns the expected path to this artifact given the root of the cache.
 	pub fn path(&self, cache_path: &Path) -> PathBuf {
-		let file_name =
-			format!("{}{:#x}_{:#x}", Self::PREFIX, self.code_hash, self.executor_params_hash);
+		let file_name = format!(
+			"{}{}{}_{:#x}_{:#x}",
+			Self::PREFIX,
+			Self::NODE_VERSION_PREFIX,
+			NODE_VERSION,
+			self.code_hash,
+			self.executor_params_hash
+		);
 		cache_path.join(file_name)
 	}
 }
@@ -253,20 +266,27 @@ impl Artifacts {
 
 #[cfg(test)]
 mod tests {
-	use super::{ArtifactId, Artifacts};
+	use super::{ArtifactId, Artifacts, NODE_VERSION};
 	use polkadot_primitives::ExecutorParamsHash;
 	use sp_core::H256;
 	use std::{path::Path, str::FromStr};
 
+	fn file_name(code_hash: &str, param_hash: &str) -> String {
+		format!("wasmtime_polkadot_v{}_0x{}_0x{}", NODE_VERSION, code_hash, param_hash)
+	}
+
 	#[test]
 	fn from_file_name() {
 		assert!(ArtifactId::from_file_name("").is_none());
 		assert!(ArtifactId::from_file_name("junk").is_none());
 
+		let file_name = file_name(
+			"0022800000000000000000000000000000000000000000000000000000000000",
+			"0033900000000000000000000000000000000000000000000000000000000000",
+		);
+
 		assert_eq!(
-			ArtifactId::from_file_name(
-				"wasmtime_0x0022800000000000000000000000000000000000000000000000000000000000_0x0033900000000000000000000000000000000000000000000000000000000000"
-			),
+			ArtifactId::from_file_name(&file_name),
 			Some(ArtifactId::new(
 				hex_literal::hex![
 					"0022800000000000000000000000000000000000000000000000000000000000"
@@ -281,16 +301,19 @@ mod tests {
 
 	#[test]
 	fn path() {
-		let path = Path::new("/test");
-		let hash =
-			H256::from_str("1234567890123456789012345678901234567890123456789012345678901234")
-				.unwrap();
+		let dir = Path::new("/test");
+		let code_hash = "1234567890123456789012345678901234567890123456789012345678901234";
+		let params_hash = "4321098765432109876543210987654321098765432109876543210987654321";
+		let file_name = file_name(code_hash, params_hash);
+
+		let code_hash = H256::from_str(code_hash).unwrap();
+		let params_hash = H256::from_str(params_hash).unwrap();
 
 		assert_eq!(
-			ArtifactId::new(hash.into(), ExecutorParamsHash::from_hash(hash)).path(path).to_str(),
-			Some(
-				"/test/wasmtime_0x1234567890123456789012345678901234567890123456789012345678901234_0x1234567890123456789012345678901234567890123456789012345678901234"
-			),
+			ArtifactId::new(code_hash.into(), ExecutorParamsHash::from_hash(params_hash))
+				.path(dir)
+				.to_str(),
+			Some(format!("/test/{}", file_name).as_str()),
 		);
 	}
 
diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs
index 463c7f960ba..dab72bb2a5e 100644
--- a/polkadot/node/primitives/src/lib.rs
+++ b/polkadot/node/primitives/src/lib.rs
@@ -53,6 +53,13 @@ pub use disputes::{
 	ValidDisputeVote, ACTIVE_DURATION_SECS,
 };
 
+/// The current node version, which takes the basic SemVer form `<major>.<minor>.<patch>`.
+/// In general, minor should be bumped on every release while major or patch releases are
+/// relatively rare.
+///
+/// The associated worker binaries should use the same version as the node that spawns them.
+pub const NODE_VERSION: &'static str = "1.1.0";
+
 // For a 16-ary Merkle Prefix Trie, we can expect at most 16 32-byte hashes per node
 // plus some overhead:
 // header 1 + bitmap 2 + max partial_key 8 + children 16 * (32 + len 1) + value 32 + value len 1
-- 
GitLab