From 043222af0b0eed2290d062bc1fe48a8d50cf68cf Mon Sep 17 00:00:00 2001 From: Marcin S Date: Mon, 28 Aug 2023 12:56:48 +0200 Subject: [PATCH 1/4] PVF: Take back a stolen right After a recent [worker revolt](https://github.com/paritytech/polkadot/pull/7538), they managed to gain a small amount of freedom from the landowner host. But after the failed [proletariat revolution](https://github.com/paritytech/polkadot/pull/7570), there were mass migrations and we ended up in a new repo. The capitalists took advantage of the confusion to secretly take away a workers' right. We must undo this before it goes too far. --- polkadot/node/core/pvf/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/core/pvf/Cargo.toml b/polkadot/node/core/pvf/Cargo.toml index 872ab0107cb8..5f6e3b26d904 100644 --- a/polkadot/node/core/pvf/Cargo.toml +++ b/polkadot/node/core/pvf/Cargo.toml @@ -47,7 +47,7 @@ assert_matches = "1.4.0" hex-literal = "0.3.4" polkadot-node-core-pvf-common = { path = "common", features = ["test-utils"] } # For the puppet worker, depend on ourselves with the test-utils feature. -polkadot-node-core-pvf = { path = "", features = ["test-utils"] } +polkadot-node-core-pvf = { path = ".", features = ["test-utils"] } adder = { package = "test-parachain-adder", path = "../../../parachain/test-parachains/adder" } halt = { package = "test-parachain-halt", path = "../../../parachain/test-parachains/halt" } From 1ebe09276835e26fd58961f54dcfdc26a11919fa Mon Sep 17 00:00:00 2001 From: Marcin S Date: Mon, 28 Aug 2023 15:04:02 +0200 Subject: [PATCH 2/4] Add missing `optional = true` for `sp-tracing` See polkadot repo. --- polkadot/node/core/pvf/common/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml index f9f900a0fec0..25c6069c8ba3 100644 --- a/polkadot/node/core/pvf/common/Cargo.toml +++ b/polkadot/node/core/pvf/common/Cargo.toml @@ -25,7 +25,7 @@ sc-executor-wasmtime = { path = "../../../../../substrate/client/executor/wasmti sp-core = { path = "../../../../../substrate/primitives/core" } sp-externalities = { path = "../../../../../substrate/primitives/externalities" } sp-io = { path = "../../../../../substrate/primitives/io" } -sp-tracing = { path = "../../../../../substrate/primitives/tracing" } +sp-tracing = { path = "../../../../../substrate/primitives/tracing", optional = true } [target.'cfg(target_os = "linux")'.dependencies] landlock = "0.2.0" @@ -37,4 +37,4 @@ 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 = [] +test-utils = ["sp-tracing"] From 43423527402e6122b40781dd0f65c4e9c6837d2c Mon Sep 17 00:00:00 2001 From: Marcin S Date: Mon, 28 Aug 2023 15:51:32 +0200 Subject: [PATCH 3/4] Abolish private ownership --- polkadot/node/core/pvf/common/src/lib.rs | 8 +++----- polkadot/node/core/pvf/common/src/worker/mod.rs | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/polkadot/node/core/pvf/common/src/lib.rs b/polkadot/node/core/pvf/common/src/lib.rs index 2cc9c72e182c..8ff9757a07a0 100644 --- a/polkadot/node/core/pvf/common/src/lib.rs +++ b/polkadot/node/core/pvf/common/src/lib.rs @@ -25,11 +25,9 @@ pub mod worker; pub use cpu_time::ProcessTime; -/// DO NOT USE - internal for macros only. -#[doc(hidden)] -pub mod __private { - pub use sp_tracing::try_init_simple; -} +// Used by `decl_worker_main!`. +#[cfg(feature = "test-utils")] +pub use sp_tracing; const LOG_TARGET: &str = "parachain::pvf-common"; diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs index 4ea0e5aa1a9a..8b41cb82f73b 100644 --- a/polkadot/node/core/pvf/common/src/worker/mod.rs +++ b/polkadot/node/core/pvf/common/src/worker/mod.rs @@ -41,7 +41,7 @@ macro_rules! decl_worker_main { } fn main() { - $crate::__private::try_init_simple(); + $crate::sp_tracing::try_init_simple(); let args = std::env::args().collect::>(); if args.len() == 1 { From 5cc22a9413034f69e70ec37ff176f5d92c70cd90 Mon Sep 17 00:00:00 2001 From: Marcin S Date: Wed, 30 Aug 2023 16:24:51 +0200 Subject: [PATCH 4/4] Remove capitalistic restriction on workers' right to trace Workers always should have tracing, including in production, it should not be gated behind the `test-utils` feature. Added a note to eventually remove it with a link to the issue. --- polkadot/node/core/pvf/common/Cargo.toml | 4 ++-- polkadot/node/core/pvf/common/src/lib.rs | 1 - polkadot/node/core/pvf/common/src/worker/mod.rs | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml index 5a1e634a568b..ded8f0dc63be 100644 --- a/polkadot/node/core/pvf/common/Cargo.toml +++ b/polkadot/node/core/pvf/common/Cargo.toml @@ -25,7 +25,7 @@ sc-executor-wasmtime = { path = "../../../../../substrate/client/executor/wasmti sp-core = { path = "../../../../../substrate/primitives/core" } sp-externalities = { path = "../../../../../substrate/primitives/externalities" } sp-io = { path = "../../../../../substrate/primitives/io" } -sp-tracing = { path = "../../../../../substrate/primitives/tracing", optional = true } +sp-tracing = { path = "../../../../../substrate/primitives/tracing" } [target.'cfg(target_os = "linux")'.dependencies] landlock = "0.2.0" @@ -37,4 +37,4 @@ 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 = ["sp-tracing"] +test-utils = [] diff --git a/polkadot/node/core/pvf/common/src/lib.rs b/polkadot/node/core/pvf/common/src/lib.rs index 8ff9757a07a0..c358ad6e134d 100644 --- a/polkadot/node/core/pvf/common/src/lib.rs +++ b/polkadot/node/core/pvf/common/src/lib.rs @@ -26,7 +26,6 @@ pub mod worker; pub use cpu_time::ProcessTime; // Used by `decl_worker_main!`. -#[cfg(feature = "test-utils")] pub use sp_tracing; const LOG_TARGET: &str = "parachain::pvf-common"; diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs index 8b41cb82f73b..40e540bb3f7e 100644 --- a/polkadot/node/core/pvf/common/src/worker/mod.rs +++ b/polkadot/node/core/pvf/common/src/worker/mod.rs @@ -41,6 +41,8 @@ macro_rules! decl_worker_main { } fn main() { + // TODO: Remove this dependency, and `pub use sp_tracing` in `lib.rs`. + // See . $crate::sp_tracing::try_init_simple(); let args = std::env::args().collect::>();