From e39253c022bfed42ba241113c7c9b545b554f098 Mon Sep 17 00:00:00 2001
From: Marcin S <marcin@realemail.net>
Date: Tue, 24 Oct 2023 16:22:15 +0200
Subject: [PATCH] PVF: Add worker check during tests and benches (#1771)
---
Cargo.lock | 5 +-
polkadot/cli/src/command.rs | 2 +-
polkadot/node/core/pvf/Cargo.toml | 13 +-
.../benches/host_prepare_rococo_runtime.rs | 130 ++++++++++++++++++
polkadot/node/core/pvf/bin/puppet_worker.rs | 17 ---
polkadot/node/core/pvf/common/Cargo.toml | 1 -
.../node/core/pvf/common/src/worker/mod.rs | 12 +-
polkadot/node/core/pvf/src/artifacts.rs | 1 +
polkadot/node/core/pvf/src/host.rs | 10 +-
polkadot/node/core/pvf/src/lib.rs | 13 ++
polkadot/node/core/pvf/src/testing.rs | 58 +++++++-
polkadot/node/core/pvf/src/worker_intf.rs | 1 +
polkadot/node/core/pvf/tests/it/main.rs | 36 ++---
.../node/core/pvf/tests/it/worker_common.rs | 22 ++-
polkadot/node/service/Cargo.toml | 8 +-
polkadot/node/service/src/workers.rs | 61 ++++----
polkadot/src/bin/execute-worker.rs | 1 +
polkadot/src/bin/prepare-worker.rs | 1 +
.../utils/build-script-utils/src/version.rs | 6 +-
19 files changed, 285 insertions(+), 113 deletions(-)
create mode 100644 polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs
delete mode 100644 polkadot/node/core/pvf/bin/puppet_worker.rs
diff --git a/Cargo.lock b/Cargo.lock
index a8d679c6ce8..d1a1b1877a9 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -12184,9 +12184,11 @@ dependencies = [
"always-assert",
"assert_matches",
"cfg-if",
+ "criterion 0.4.0",
"futures",
"futures-timer",
"hex-literal",
+ "is_executable",
"libc",
"parity-scale-codec",
"pin-project",
@@ -12200,11 +12202,11 @@ dependencies = [
"polkadot-parachain-primitives",
"polkadot-primitives",
"rand 0.8.5",
+ "rococo-runtime",
"slotmap",
"sp-core",
"sp-maybe-compressed-blob",
"sp-wasm-interface",
- "substrate-build-script-utils",
"tempfile",
"test-parachain-adder",
"test-parachain-halt",
@@ -12912,6 +12914,7 @@ dependencies = [
"sc-telemetry",
"sc-transaction-pool",
"sc-transaction-pool-api",
+ "schnellru",
"serde",
"serde_json",
"serial_test",
diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs
index 3da6e54b812..2dcf5e0e8d7 100644
--- a/polkadot/cli/src/command.rs
+++ b/polkadot/cli/src/command.rs
@@ -50,7 +50,7 @@ impl SubstrateCli for Cli {
fn impl_version() -> String {
let commit_hash = env!("SUBSTRATE_CLI_COMMIT_HASH");
- format!("{NODE_VERSION}-{commit_hash}")
+ format!("{}-{commit_hash}", NODE_VERSION)
}
fn description() -> String {
diff --git a/polkadot/node/core/pvf/Cargo.toml b/polkadot/node/core/pvf/Cargo.toml
index 27f4df117e5..bfd70c6fbd4 100644
--- a/polkadot/node/core/pvf/Cargo.toml
+++ b/polkadot/node/core/pvf/Cargo.toml
@@ -12,6 +12,7 @@ cfg-if = "1.0"
futures = "0.3.21"
futures-timer = "3.0.2"
gum = { package = "tracing-gum", path = "../../gum" }
+is_executable = "1.0.1"
libc = "0.2.139"
pin-project = "1.0.9"
rand = "0.8.5"
@@ -34,19 +35,23 @@ sp-maybe-compressed-blob = { path = "../../../../substrate/primitives/maybe-comp
polkadot-node-core-pvf-prepare-worker = { path = "prepare-worker", optional = true }
polkadot-node-core-pvf-execute-worker = { path = "execute-worker", optional = true }
-[build-dependencies]
-substrate-build-script-utils = { path = "../../../../substrate/utils/build-script-utils" }
-
[dev-dependencies]
assert_matches = "1.4.0"
+criterion = { version = "0.4.0", default-features = false, features = ["cargo_bench_support", "async_tokio"] }
hex-literal = "0.4.1"
polkadot-node-core-pvf-common = { path = "common", features = ["test-utils"] }
-# For the puppet worker, depend on ourselves with the test-utils feature.
+# For benches and integration tests, depend on ourselves with the test-utils
+# feature.
polkadot-node-core-pvf = { path = ".", features = ["test-utils"] }
+rococo-runtime = { path = "../../../runtime/rococo" }
adder = { package = "test-parachain-adder", path = "../../../parachain/test-parachains/adder" }
halt = { package = "test-parachain-halt", path = "../../../parachain/test-parachains/halt" }
+[[bench]]
+name = "host_prepare_rococo_runtime"
+harness = false
+
[features]
ci-only-tests = []
jemalloc-allocator = [ "polkadot-node-core-pvf-common/jemalloc-allocator" ]
diff --git a/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs b/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs
new file mode 100644
index 00000000000..3069fa2b194
--- /dev/null
+++ b/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs
@@ -0,0 +1,130 @@
+// Copyright (C) Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
+
+//! Benchmarks for preparation through the host. We use a real PVF to get realistic results.
+
+use criterion::{criterion_group, criterion_main, BatchSize, Criterion, SamplingMode};
+use parity_scale_codec::Encode;
+use polkadot_node_core_pvf::{
+ start, testing, Config, Metrics, PrepareError, PrepareJobKind, PrepareStats, PvfPrepData,
+ ValidationError, ValidationHost,
+};
+use polkadot_parachain_primitives::primitives::{BlockData, ValidationParams, ValidationResult};
+use polkadot_primitives::ExecutorParams;
+use rococo_runtime::WASM_BINARY;
+use std::time::Duration;
+use tokio::{runtime::Handle, sync::Mutex};
+
+const TEST_EXECUTION_TIMEOUT: Duration = Duration::from_secs(3);
+const TEST_PREPARATION_TIMEOUT: Duration = Duration::from_secs(30);
+
+struct TestHost {
+ host: Mutex<ValidationHost>,
+}
+
+impl TestHost {
+ fn new_with_config<F>(handle: &Handle, f: F) -> Self
+ where
+ F: FnOnce(&mut Config),
+ {
+ let (prepare_worker_path, execute_worker_path) = testing::get_and_check_worker_paths();
+
+ let cache_dir = tempfile::tempdir().unwrap();
+ 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 _ = handle.spawn(task);
+ Self { host: Mutex::new(host) }
+ }
+
+ async fn precheck_pvf(
+ &self,
+ code: &[u8],
+ executor_params: ExecutorParams,
+ ) -> Result<PrepareStats, PrepareError> {
+ let (result_tx, result_rx) = futures::channel::oneshot::channel();
+
+ let code = sp_maybe_compressed_blob::decompress(code, 16 * 1024 * 1024)
+ .expect("Compression works");
+
+ self.host
+ .lock()
+ .await
+ .precheck_pvf(
+ PvfPrepData::from_code(
+ code.into(),
+ executor_params,
+ TEST_PREPARATION_TIMEOUT,
+ PrepareJobKind::Prechecking,
+ ),
+ result_tx,
+ )
+ .await
+ .unwrap();
+ result_rx.await.unwrap()
+ }
+}
+
+fn host_prepare_rococo_runtime(c: &mut Criterion) {
+ polkadot_node_core_pvf_common::sp_tracing::try_init_simple();
+
+ let rt = tokio::runtime::Runtime::new().unwrap();
+
+ let blob = WASM_BINARY.expect("You need to build the WASM binaries to run the tests!");
+ let pvf = match sp_maybe_compressed_blob::decompress(&blob, 64 * 1024 * 1024) {
+ Ok(code) => PvfPrepData::from_code(
+ code.into_owned(),
+ ExecutorParams::default(),
+ Duration::from_secs(360),
+ PrepareJobKind::Compilation,
+ ),
+ Err(e) => {
+ panic!("Cannot decompress blob: {:?}", e);
+ },
+ };
+
+ let mut group = c.benchmark_group("prepare rococo");
+ group.sampling_mode(SamplingMode::Flat);
+ group.sample_size(20);
+ group.measurement_time(Duration::from_secs(240));
+ group.bench_function("host: prepare Rococo runtime", |b| {
+ b.to_async(&rt).iter_batched(
+ || {
+ (
+ TestHost::new_with_config(rt.handle(), |cfg| {
+ cfg.prepare_workers_hard_max_num = 1;
+ }),
+ pvf.clone().code(),
+ )
+ },
+ |(host, pvf_code)| async move {
+ // `PvfPrepData` is designed to be cheap to clone, so cloning shouldn't affect the
+ // benchmark accuracy.
+ let _stats = host.precheck_pvf(&pvf_code, Default::default()).await.unwrap();
+ },
+ BatchSize::SmallInput,
+ )
+ });
+ group.finish();
+}
+
+criterion_group!(prepare, host_prepare_rococo_runtime);
+criterion_main!(prepare);
diff --git a/polkadot/node/core/pvf/bin/puppet_worker.rs b/polkadot/node/core/pvf/bin/puppet_worker.rs
deleted file mode 100644
index 7f93519d845..00000000000
--- a/polkadot/node/core/pvf/bin/puppet_worker.rs
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright (C) Parity Technologies (UK) Ltd.
-// This file is part of Polkadot.
-
-// Polkadot is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-
-// Polkadot is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License
-// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
-
-polkadot_node_core_pvf::decl_puppet_worker_main!();
diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml
index 4bdacca72f4..5fe2c6b6845 100644
--- a/polkadot/node/core/pvf/common/Cargo.toml
+++ b/polkadot/node/core/pvf/common/Cargo.toml
@@ -37,6 +37,5 @@ tempfile = "3.3.0"
[features]
# This feature is used to export test code to other crates without putting it in the production build.
-# Also used for building the puppet worker.
test-utils = []
jemalloc-allocator = []
diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs
index d0bd5b6bd7c..e7b996ccdc3 100644
--- a/polkadot/node/core/pvf/common/src/worker/mod.rs
+++ b/polkadot/node/core/pvf/common/src/worker/mod.rs
@@ -35,9 +35,14 @@ use tokio::{io, runtime::Runtime};
/// spawning the desired worker.
#[macro_export]
macro_rules! decl_worker_main {
- ($expected_command:expr, $entrypoint:expr, $worker_version:expr $(,)*) => {
+ ($expected_command: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);
+ println!("commit: {}", $worker_version_hash);
println!();
println!("PVF worker that is called by polkadot.");
}
@@ -67,6 +72,11 @@ macro_rules! decl_worker_main {
println!("{}", $worker_version);
return
},
+ // Useful for debugging. --version is used for version checks.
+ "--full-version" => {
+ println!("{}", get_full_version());
+ return
+ },
"--check-can-enable-landlock" => {
#[cfg(target_os = "linux")]
diff --git a/polkadot/node/core/pvf/src/artifacts.rs b/polkadot/node/core/pvf/src/artifacts.rs
index d7b15ece7b2..dd83f76494e 100644
--- a/polkadot/node/core/pvf/src/artifacts.rs
+++ b/polkadot/node/core/pvf/src/artifacts.rs
@@ -141,6 +141,7 @@ impl ArtifactPathId {
}
}
+#[derive(Debug)]
pub enum ArtifactState {
/// The artifact is ready to be used by the executor.
///
diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs
index 81695829122..6c9606bb2f3 100644
--- a/polkadot/node/core/pvf/src/host.rs
+++ b/polkadot/node/core/pvf/src/host.rs
@@ -446,7 +446,8 @@ async fn handle_to_host(
/// This tries to prepare the PVF by compiling the WASM blob within a timeout set in
/// `PvfPrepData`.
///
-/// If the prepare job failed previously, we may retry it under certain conditions.
+/// We don't retry artifacts that previously failed preparation. We don't expect multiple
+/// pre-checking requests.
async fn handle_precheck_pvf(
artifacts: &mut Artifacts,
prepare_queue: &mut mpsc::Sender<prepare::ToQueue>,
@@ -464,8 +465,7 @@ async fn handle_precheck_pvf(
ArtifactState::Preparing { waiting_for_response, num_failures: _ } =>
waiting_for_response.push(result_sender),
ArtifactState::FailedToProcess { error, .. } => {
- // Do not retry failed preparation if another pre-check request comes in. We do not
- // retry pre-checking, anyway.
+ // Do not retry an artifact that previously failed preparation.
let _ = result_sender.send(PrepareResult::Err(error.clone()));
},
}
@@ -764,7 +764,7 @@ async fn handle_prepare_done(
let last_time_failed = SystemTime::now();
let num_failures = *num_failures + 1;
- gum::warn!(
+ gum::error!(
target: LOG_TARGET,
?artifact_id,
time_failed = ?last_time_failed,
@@ -846,7 +846,7 @@ async fn sweeper_task(mut sweeper_rx: mpsc::Receiver<PathBuf>) {
gum::trace!(
target: LOG_TARGET,
?result,
- "Sweeping the artifact file {}",
+ "Sweeped the artifact file {}",
condemned.display(),
);
},
diff --git a/polkadot/node/core/pvf/src/lib.rs b/polkadot/node/core/pvf/src/lib.rs
index e3c6da9c4c6..27630af40c2 100644
--- a/polkadot/node/core/pvf/src/lib.rs
+++ b/polkadot/node/core/pvf/src/lib.rs
@@ -116,5 +116,18 @@ pub use polkadot_node_core_pvf_common::{
SecurityStatus,
};
+use std::{path::Path, process::Command};
+
/// The log target for this crate.
pub const LOG_TARGET: &str = "parachain::pvf";
+
+/// Utility to get the version of a worker, used for version checks.
+///
+/// The worker's existence at the given path must be checked separately.
+pub fn get_worker_version(worker_path: &Path) -> std::io::Result<String> {
+ let worker_version = Command::new(worker_path).args(["--version"]).output()?.stdout;
+ Ok(std::str::from_utf8(&worker_version)
+ .expect("version is printed as a string; qed")
+ .trim()
+ .to_string())
+}
diff --git a/polkadot/node/core/pvf/src/testing.rs b/polkadot/node/core/pvf/src/testing.rs
index ca69ef9e4d0..31bcfe7fadb 100644
--- a/polkadot/node/core/pvf/src/testing.rs
+++ b/polkadot/node/core/pvf/src/testing.rs
@@ -15,13 +15,20 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
//! Various things for testing other crates.
-//!
-//! N.B. This is not guarded with some feature flag. Overexposing items here may affect the final
-//! artifact even for production builds.
-pub use crate::worker_intf::{spawn_with_program_path, SpawnErr};
+pub use crate::{
+ host::{EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME},
+ worker_intf::{spawn_with_program_path, SpawnErr},
+};
+use crate::get_worker_version;
+use is_executable::IsExecutable;
+use polkadot_node_primitives::NODE_VERSION;
use polkadot_primitives::ExecutorParams;
+use std::{
+ path::PathBuf,
+ sync::{Mutex, OnceLock},
+};
/// A function that emulates the stitches together behaviors of the preparation and the execution
/// worker in a single synchronous function.
@@ -47,3 +54,46 @@ pub fn validate_candidate(
Ok(result)
}
+
+/// Retrieves the worker paths, checks that they exist and does a version check.
+///
+/// NOTE: This should only be called in dev code (tests, benchmarks) as it relies on the relative
+/// paths of the built workers.
+pub fn get_and_check_worker_paths() -> (PathBuf, PathBuf) {
+ // Only needs to be called once for the current process.
+ static WORKER_PATHS: OnceLock<Mutex<(PathBuf, PathBuf)>> = OnceLock::new();
+ let mutex = WORKER_PATHS.get_or_init(|| {
+ 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(PREPARE_BINARY_NAME);
+ let mut execute_worker_path = workers_path.clone();
+ execute_worker_path.push(EXECUTE_BINARY_NAME);
+
+ // Check that the workers are valid.
+ if !prepare_worker_path.is_executable() || !execute_worker_path.is_executable() {
+ panic!("ERROR: Workers do not exist or are not executable. Workers directory: {:?}", workers_path);
+ }
+
+ let worker_version =
+ get_worker_version(&prepare_worker_path).expect("checked for worker existence");
+ if worker_version != NODE_VERSION {
+ panic!("ERROR: Prepare worker version {worker_version} does not match node version {NODE_VERSION}; worker path: {prepare_worker_path:?}");
+ }
+ let worker_version =
+ get_worker_version(&execute_worker_path).expect("checked for worker existence");
+ if worker_version != NODE_VERSION {
+ panic!("ERROR: Execute worker version {worker_version} does not match node version {NODE_VERSION}; worker path: {execute_worker_path:?}");
+ }
+
+ // We don't want to check against the commit hash because we'd have to always rebuild
+ // the calling crate on every commit.
+ eprintln!("WARNING: Workers match the node version, but may have changed in recent commits. Please rebuild them if anything funny happens. Workers path: {workers_path:?}");
+
+ Mutex::new((prepare_worker_path, execute_worker_path))
+ });
+
+ let guard = mutex.lock().unwrap();
+ (guard.0.clone(), guard.1.clone())
+}
diff --git a/polkadot/node/core/pvf/src/worker_intf.rs b/polkadot/node/core/pvf/src/worker_intf.rs
index bd85d84055c..e9382b66bf7 100644
--- a/polkadot/node/core/pvf/src/worker_intf.rs
+++ b/polkadot/node/core/pvf/src/worker_intf.rs
@@ -283,6 +283,7 @@ impl WorkerHandle {
if let Ok(value) = std::env::var("RUST_LOG") {
command.env("RUST_LOG", value);
}
+
let mut child = command
.args(extra_args)
.arg("--socket-path")
diff --git a/polkadot/node/core/pvf/tests/it/main.rs b/polkadot/node/core/pvf/tests/it/main.rs
index f699b5840d8..cdf8d6eb82d 100644
--- a/polkadot/node/core/pvf/tests/it/main.rs
+++ b/polkadot/node/core/pvf/tests/it/main.rs
@@ -18,8 +18,9 @@
use assert_matches::assert_matches;
use parity_scale_codec::Encode as _;
use polkadot_node_core_pvf::{
- start, Config, InvalidCandidate, Metrics, PrepareError, PrepareJobKind, PrepareStats,
- PvfPrepData, ValidationError, ValidationHost, JOB_TIMEOUT_WALL_CLOCK_FACTOR,
+ start, testing::get_and_check_worker_paths, Config, InvalidCandidate, Metrics, PrepareError,
+ PrepareJobKind, PrepareStats, PvfPrepData, ValidationError, ValidationHost,
+ JOB_TIMEOUT_WALL_CLOCK_FACTOR,
};
use polkadot_parachain_primitives::primitives::{BlockData, ValidationParams, ValidationResult};
use polkadot_primitives::ExecutorParams;
@@ -50,13 +51,8 @@ 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 (prepare_worker_path, execute_worker_path) = get_and_check_worker_paths();
+
let cache_dir = tempfile::tempdir().unwrap();
let mut config = Config::new(
cache_dir.path().to_owned(),
@@ -296,25 +292,9 @@ async fn deleting_prepared_artifact_does_not_dispute() {
let host = TestHost::new();
let cache_dir = host.cache_dir.path();
- let result = host
- .validate_candidate(
- halt::wasm_binary_unwrap(),
- ValidationParams {
- block_data: BlockData(Vec::new()),
- parent_head: Default::default(),
- relay_parent_number: 1,
- relay_parent_storage_root: Default::default(),
- },
- Default::default(),
- )
- .await;
-
- match result {
- Err(ValidationError::InvalidCandidate(InvalidCandidate::HardTimeout)) => {},
- r => panic!("{:?}", r),
- }
+ let _stats = host.precheck_pvf(halt::wasm_binary_unwrap(), Default::default()).await.unwrap();
- // Delete the prepared artifact.
+ // Manually delete the prepared artifact from disk. The in-memory artifacts table won't change.
{
// Get the artifact path (asserting it exists).
let mut cache_dir: Vec<_> = std::fs::read_dir(cache_dir).unwrap().collect();
@@ -329,7 +309,7 @@ async fn deleting_prepared_artifact_does_not_dispute() {
std::fs::remove_file(artifact_path.path()).unwrap();
}
- // Try to validate again, artifact should get recreated.
+ // Try to validate, artifact should get recreated.
let result = host
.validate_candidate(
halt::wasm_binary_unwrap(),
diff --git a/polkadot/node/core/pvf/tests/it/worker_common.rs b/polkadot/node/core/pvf/tests/it/worker_common.rs
index 5379d29556c..df64980dc80 100644
--- a/polkadot/node/core/pvf/tests/it/worker_common.rs
+++ b/polkadot/node/core/pvf/tests/it/worker_common.rs
@@ -15,27 +15,21 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
use polkadot_node_core_pvf::{
- testing::{spawn_with_program_path, SpawnErr},
+ testing::{get_and_check_worker_paths, spawn_with_program_path, SpawnErr},
SecurityStatus,
};
use std::{env, time::Duration};
-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 (prepare_worker_path, _) = get_and_check_worker_paths();
+
// 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"),
+ prepare_worker_path,
&env::temp_dir(),
&["exit"],
Duration::from_secs(2),
@@ -47,9 +41,11 @@ async fn spawn_immediate_exit() {
#[tokio::test]
async fn spawn_timeout() {
+ let (_, execute_worker_path) = get_and_check_worker_paths();
+
let result = spawn_with_program_path(
"integration-test",
- worker_path("polkadot-execute-worker"),
+ execute_worker_path,
&env::temp_dir(),
&["test-sleep"],
Duration::from_secs(2),
@@ -61,9 +57,11 @@ async fn spawn_timeout() {
#[tokio::test]
async fn should_connect() {
+ let (prepare_worker_path, _) = get_and_check_worker_paths();
+
let _ = spawn_with_program_path(
"integration-test",
- worker_path("polkadot-prepare-worker"),
+ prepare_worker_path,
&env::temp_dir(),
&["prepare-worker"],
Duration::from_secs(2),
diff --git a/polkadot/node/service/Cargo.toml b/polkadot/node/service/Cargo.toml
index 899a95dd3a7..3429f4e0a3a 100644
--- a/polkadot/node/service/Cargo.toml
+++ b/polkadot/node/service/Cargo.toml
@@ -74,9 +74,13 @@ frame-benchmarking-cli = { path = "../../../substrate/utils/frame/benchmarking-c
frame-benchmarking = { path = "../../../substrate/frame/benchmarking" }
# External Crates
+async-trait = "0.1.57"
futures = "0.3.21"
hex-literal = "0.4.1"
+is_executable = "1.0.1"
gum = { package = "tracing-gum", path = "../gum" }
+log = "0.4.17"
+schnellru = "0.2.1"
serde = { version = "1.0.188", features = ["derive"] }
serde_json = "1.0.107"
thiserror = "1.0.48"
@@ -85,10 +89,6 @@ kvdb-rocksdb = { version = "0.19.0", optional = true }
parity-db = { version = "0.4.8", optional = true }
codec = { package = "parity-scale-codec", version = "3.6.1" }
-async-trait = "0.1.57"
-log = "0.4.17"
-is_executable = "1.0.1"
-
# Polkadot
polkadot-core-primitives = { path = "../../core-primitives" }
polkadot-node-core-parachains-inherent = { path = "../core/parachains-inherent" }
diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs
index 5f7cc1c2ed4..b35bb8302fd 100644
--- a/polkadot/node/service/src/workers.rs
+++ b/polkadot/node/service/src/workers.rs
@@ -18,7 +18,7 @@
use super::Error;
use is_executable::IsExecutable;
-use std::{path::PathBuf, process::Command};
+use std::path::PathBuf;
#[cfg(test)]
use std::sync::{Mutex, OnceLock};
@@ -75,11 +75,7 @@ pub fn determine_workers_paths(
// Do the version check.
if let Some(node_version) = node_version {
- let worker_version = Command::new(&prep_worker_path).args(["--version"]).output()?.stdout;
- let worker_version = std::str::from_utf8(&worker_version)
- .expect("version is printed as a string; qed")
- .trim()
- .to_string();
+ let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?;
if worker_version != node_version {
return Err(Error::WorkerBinaryVersionMismatch {
worker_version,
@@ -87,11 +83,8 @@ pub fn determine_workers_paths(
worker_path: prep_worker_path,
})
}
- let worker_version = Command::new(&exec_worker_path).args(["--version"]).output()?.stdout;
- let worker_version = std::str::from_utf8(&worker_version)
- .expect("version is printed as a string; qed")
- .trim()
- .to_string();
+
+ let worker_version = polkadot_node_core_pvf::get_worker_version(&exec_worker_path)?;
if worker_version != node_version {
return Err(Error::WorkerBinaryVersionMismatch {
worker_version,
@@ -215,11 +208,11 @@ mod tests {
use serial_test::serial;
use std::{env::temp_dir, fs, os::unix::fs::PermissionsExt, path::Path};
- const NODE_VERSION: &'static str = "v0.1.2";
+ const TEST_NODE_VERSION: &'static str = "v0.1.2";
/// Write a dummy executable to the path which satisfies the version check.
fn write_worker_exe(path: impl AsRef<Path>) -> Result<(), Box<dyn std::error::Error>> {
- let program = get_program(NODE_VERSION);
+ let program = get_program(TEST_NODE_VERSION);
fs::write(&path, program)?;
Ok(fs::set_permissions(&path, fs::Permissions::from_mode(0o744))?)
}
@@ -287,7 +280,7 @@ echo {}
// Try with provided workers path that has missing binaries.
assert_matches!(
- determine_workers_paths(Some(given_workers_path.clone()), None, Some(NODE_VERSION.into())),
+ determine_workers_paths(Some(given_workers_path.clone()), None, Some(TEST_NODE_VERSION.into())),
Err(Error::MissingWorkerBinaries { given_workers_path: Some(p1), current_exe_path: p2, workers_names: None }) if p1 == given_workers_path && p2 == exe_path
);
@@ -299,7 +292,7 @@ echo {}
write_worker_exe(&execute_worker_path)?;
fs::set_permissions(&execute_worker_path, fs::Permissions::from_mode(0o644))?;
assert_matches!(
- determine_workers_paths(Some(given_workers_path.clone()), None, Some(NODE_VERSION.into())),
+ determine_workers_paths(Some(given_workers_path.clone()), None, Some(TEST_NODE_VERSION.into())),
Err(Error::InvalidWorkerBinaries { prep_worker_path: p1, exec_worker_path: p2 }) if p1 == prepare_worker_path && p2 == execute_worker_path
);
@@ -307,15 +300,15 @@ echo {}
fs::set_permissions(&prepare_worker_path, fs::Permissions::from_mode(0o744))?;
fs::set_permissions(&execute_worker_path, fs::Permissions::from_mode(0o744))?;
assert_matches!(
- determine_workers_paths(Some(given_workers_path), None, Some(NODE_VERSION.into())),
+ determine_workers_paths(Some(given_workers_path), None, Some(TEST_NODE_VERSION.into())),
Ok((p1, p2)) if p1 == prepare_worker_path && p2 == execute_worker_path
);
// Try with valid provided workers path that is a binary file.
- let given_workers_path = tempdir.join("usr/local/bin/puppet-worker");
+ let given_workers_path = tempdir.join("usr/local/bin/test-worker");
write_worker_exe(&given_workers_path)?;
assert_matches!(
- determine_workers_paths(Some(given_workers_path.clone()), None, Some(NODE_VERSION.into())),
+ determine_workers_paths(Some(given_workers_path.clone()), None, Some(TEST_NODE_VERSION.into())),
Ok((p1, p2)) if p1 == given_workers_path && p2 == given_workers_path
);
@@ -330,7 +323,7 @@ echo {}
with_temp_dir_structure(|tempdir, exe_path| {
// Try with both binaries missing.
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Err(Error::MissingWorkerBinaries { given_workers_path: None, current_exe_path: p, workers_names: None }) if p == exe_path
);
@@ -338,7 +331,7 @@ echo {}
let prepare_worker_path = tempdir.join("usr/bin/polkadot-prepare-worker");
write_worker_exe(&prepare_worker_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Err(Error::MissingWorkerBinaries { given_workers_path: None, current_exe_path: p, workers_names: None }) if p == exe_path
);
@@ -347,7 +340,7 @@ echo {}
let execute_worker_path = tempdir.join("usr/bin/polkadot-execute-worker");
write_worker_exe(&execute_worker_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Err(Error::MissingWorkerBinaries { given_workers_path: None, current_exe_path: p, workers_names: None }) if p == exe_path
);
@@ -356,7 +349,7 @@ echo {}
let prepare_worker_path = tempdir.join("usr/lib/polkadot/polkadot-prepare-worker");
write_worker_exe(&prepare_worker_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Err(Error::MissingWorkerBinaries { given_workers_path: None, current_exe_path: p, workers_names: None }) if p == exe_path
);
@@ -365,7 +358,7 @@ echo {}
let execute_worker_path = tempdir.join("usr/lib/polkadot/polkadot-execute-worker");
write_worker_exe(execute_worker_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Err(Error::MissingWorkerBinaries { given_workers_path: None, current_exe_path: p, workers_names: None }) if p == exe_path
);
@@ -440,8 +433,8 @@ echo {}
write_worker_exe_invalid_version(&prepare_worker_bin_path, bad_version)?;
write_worker_exe(&execute_worker_bin_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
- Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == NODE_VERSION && p == prepare_worker_bin_path
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
+ Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == TEST_NODE_VERSION && p == prepare_worker_bin_path
);
// Workers at lib location return bad version.
@@ -452,8 +445,8 @@ echo {}
write_worker_exe(&prepare_worker_lib_path)?;
write_worker_exe_invalid_version(&execute_worker_lib_path, bad_version)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
- Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == NODE_VERSION && p == execute_worker_lib_path
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
+ Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == TEST_NODE_VERSION && p == execute_worker_lib_path
);
// Workers at provided workers location return bad version.
@@ -463,16 +456,16 @@ echo {}
write_worker_exe_invalid_version(&prepare_worker_path, bad_version)?;
write_worker_exe_invalid_version(&execute_worker_path, bad_version)?;
assert_matches!(
- determine_workers_paths(Some(given_workers_path), None, Some(NODE_VERSION.into())),
- Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == NODE_VERSION && p == prepare_worker_path
+ determine_workers_paths(Some(given_workers_path), None, Some(TEST_NODE_VERSION.into())),
+ Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == TEST_NODE_VERSION && p == prepare_worker_path
);
// Given worker binary returns bad version.
- let given_workers_path = tempdir.join("usr/local/bin/puppet-worker");
+ let given_workers_path = tempdir.join("usr/local/bin/test-worker");
write_worker_exe_invalid_version(&given_workers_path, bad_version)?;
assert_matches!(
- determine_workers_paths(Some(given_workers_path.clone()), None, Some(NODE_VERSION.into())),
- Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == NODE_VERSION && p == given_workers_path
+ determine_workers_paths(Some(given_workers_path.clone()), None, Some(TEST_NODE_VERSION.into())),
+ Err(Error::WorkerBinaryVersionMismatch { worker_version: v1, node_version: v2, worker_path: p }) if v1 == bad_version && v2 == TEST_NODE_VERSION && p == given_workers_path
);
Ok(())
@@ -492,7 +485,7 @@ echo {}
write_worker_exe(&execute_worker_bin_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Ok((p1, p2)) if p1 == prepare_worker_bin_path && p2 == execute_worker_bin_path
);
@@ -509,7 +502,7 @@ echo {}
write_worker_exe(&execute_worker_lib_path)?;
assert_matches!(
- determine_workers_paths(None, None, Some(NODE_VERSION.into())),
+ determine_workers_paths(None, None, Some(TEST_NODE_VERSION.into())),
Ok((p1, p2)) if p1 == prepare_worker_lib_path && p2 == execute_worker_lib_path
);
diff --git a/polkadot/src/bin/execute-worker.rs b/polkadot/src/bin/execute-worker.rs
index 1deb3658098..c39a5a3c89d 100644
--- a/polkadot/src/bin/execute-worker.rs
+++ b/polkadot/src/bin/execute-worker.rs
@@ -20,4 +20,5 @@ polkadot_node_core_pvf_common::decl_worker_main!(
"execute-worker",
polkadot_node_core_pvf_execute_worker::worker_entrypoint,
polkadot_cli::NODE_VERSION,
+ env!("SUBSTRATE_CLI_COMMIT_HASH"),
);
diff --git a/polkadot/src/bin/prepare-worker.rs b/polkadot/src/bin/prepare-worker.rs
index d731f8a30d0..3b42991f18b 100644
--- a/polkadot/src/bin/prepare-worker.rs
+++ b/polkadot/src/bin/prepare-worker.rs
@@ -20,4 +20,5 @@ polkadot_node_core_pvf_common::decl_worker_main!(
"prepare-worker",
polkadot_node_core_pvf_prepare_worker::worker_entrypoint,
polkadot_cli::NODE_VERSION,
+ env!("SUBSTRATE_CLI_COMMIT_HASH"),
);
diff --git a/substrate/utils/build-script-utils/src/version.rs b/substrate/utils/build-script-utils/src/version.rs
index 309c6d71d77..f6a9ff9554a 100644
--- a/substrate/utils/build-script-utils/src/version.rs
+++ b/substrate/utils/build-script-utils/src/version.rs
@@ -31,7 +31,11 @@ pub fn generate_cargo_keys() {
Cow::from(sha)
},
Ok(o) => {
- println!("cargo:warning=Git command failed with status: {}", o.status);
+ let stderr = String::from_utf8_lossy(&o.stderr).trim().to_owned();
+ println!(
+ "cargo:warning=Git command failed with status '{}' with message: '{}'",
+ o.status, stderr,
+ );
Cow::from("unknown")
},
Err(err) => {
--
GitLab