From cc8e1fdeff1bbed12963bc3b1017ca72e85f8240 Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Tue, 1 Aug 2023 19:16:04 +0200 Subject: [PATCH 01/14] Move PVF worker CLI binaries to separate packages --- Cargo.lock | 24 +++++++++++++++---- Cargo.toml | 20 +++++++--------- node/core/pvf/execute-worker/cli/Cargo.toml | 16 +++++++++++++ node/core/pvf/execute-worker/cli/build.rs | 22 +++++++++++++++++ .../core/pvf/execute-worker/cli/src/main.rs | 0 node/core/pvf/prepare-worker/Cargo.toml | 3 +++ node/core/pvf/prepare-worker/build.rs | 22 +++++++++++++++++ node/core/pvf/prepare-worker/cli/Cargo.toml | 16 +++++++++++++ node/core/pvf/prepare-worker/cli/build.rs | 22 +++++++++++++++++ .../core/pvf/prepare-worker/cli/src/main.rs | 0 node/malus/Cargo.toml | 18 -------------- 11 files changed, 128 insertions(+), 35 deletions(-) create mode 100644 node/core/pvf/execute-worker/cli/Cargo.toml create mode 100644 node/core/pvf/execute-worker/cli/build.rs rename src/bin/execute-worker.rs => node/core/pvf/execute-worker/cli/src/main.rs (100%) create mode 100644 node/core/pvf/prepare-worker/build.rs create mode 100644 node/core/pvf/prepare-worker/cli/Cargo.toml create mode 100644 node/core/pvf/prepare-worker/cli/build.rs rename src/bin/prepare-worker.rs => node/core/pvf/prepare-worker/cli/src/main.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index ed59fa2039af..c3f282d69e4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6436,8 +6436,6 @@ dependencies = [ "polkadot-cli", "polkadot-core-primitives", "polkadot-node-core-pvf", - "polkadot-node-core-pvf-common", - "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker", "polkadot-overseer", "substrate-build-script-utils", @@ -7089,6 +7087,15 @@ dependencies = [ "tracing-gum", ] +[[package]] +name = "polkadot-node-core-pvf-execute-worker-cli" +version = "0.9.43" +dependencies = [ + "polkadot-node-core-pvf-common", + "polkadot-node-core-pvf-execute-worker", + "substrate-build-script-utils", +] + [[package]] name = "polkadot-node-core-pvf-prepare-worker" version = "0.9.43" @@ -7106,11 +7113,21 @@ dependencies = [ "sp-io", "sp-maybe-compressed-blob", "sp-tracing", + "substrate-build-script-utils", "tikv-jemalloc-ctl", "tokio", "tracing-gum", ] +[[package]] +name = "polkadot-node-core-pvf-prepare-worker-cli" +version = "0.9.43" +dependencies = [ + "polkadot-node-core-pvf-common", + "polkadot-node-core-pvf-prepare-worker", + "substrate-build-script-utils", +] + [[package]] name = "polkadot-node-core-runtime-api" version = "0.9.43" @@ -7883,9 +7900,6 @@ dependencies = [ "polkadot-node-core-backing", "polkadot-node-core-candidate-validation", "polkadot-node-core-dispute-coordinator", - "polkadot-node-core-pvf-common", - "polkadot-node-core-pvf-execute-worker", - "polkadot-node-core-pvf-prepare-worker", "polkadot-node-primitives", "polkadot-node-subsystem", "polkadot-node-subsystem-test-helpers", diff --git a/Cargo.toml b/Cargo.toml index 760c6ce39533..31fdc2a84b10 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,14 +2,6 @@ name = "polkadot" path = "src/main.rs" -[[bin]] -name = "polkadot-execute-worker" -path = "src/bin/execute-worker.rs" - -[[bin]] -name = "polkadot-prepare-worker" -path = "src/bin/prepare-worker.rs" - [package] name = "polkadot" description = "Implementation of a `https://polkadot.network` node in Rust based on the Substrate framework." @@ -37,10 +29,6 @@ polkadot-node-core-pvf = { path = "node/core/pvf" } polkadot-node-core-pvf-prepare-worker = { path = "node/core/pvf/prepare-worker" } polkadot-overseer = { path = "node/overseer" } -# Needed for worker binaries. -polkadot-node-core-pvf-common = { path = "node/core/pvf/common" } -polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker" } - [dev-dependencies] assert_cmd = "2.0.4" nix = { version = "0.26.1", features = ["signal"] } @@ -99,7 +87,9 @@ members = [ "node/core/pvf", "node/core/pvf/common", "node/core/pvf/execute-worker", + "node/core/pvf/execute-worker/cli", "node/core/pvf/prepare-worker", + "node/core/pvf/prepare-worker/cli", "node/core/pvf-checker", "node/core/runtime-api", "node/network/approval-distribution", @@ -141,6 +131,12 @@ members = [ "utils/generate-bags", ] +default-members = [ + ".", + "node/core/pvf/execute-worker/cli", + "node/core/pvf/prepare-worker/cli", +] + [badges] maintenance = { status = "actively-developed" } diff --git a/node/core/pvf/execute-worker/cli/Cargo.toml b/node/core/pvf/execute-worker/cli/Cargo.toml new file mode 100644 index 000000000000..ef0c1b7d6857 --- /dev/null +++ b/node/core/pvf/execute-worker/cli/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "polkadot-node-core-pvf-execute-worker-cli" +version.workspace = true +authors.workspace = true +edition.workspace = true + +[[bin]] +name = "polkadot-execute-worker" +path = "src/main.rs" + +[dependencies] +polkadot-node-core-pvf-common = { path = "../../common" } +polkadot-node-core-pvf-execute-worker = { path = "../" } + +[build-dependencies] +substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/pvf/execute-worker/cli/build.rs b/node/core/pvf/execute-worker/cli/build.rs new file mode 100644 index 000000000000..84fe22e23ed6 --- /dev/null +++ b/node/core/pvf/execute-worker/cli/build.rs @@ -0,0 +1,22 @@ +// 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 . + +fn main() { + substrate_build_script_utils::generate_cargo_keys(); + // 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(); +} diff --git a/src/bin/execute-worker.rs b/node/core/pvf/execute-worker/cli/src/main.rs similarity index 100% rename from src/bin/execute-worker.rs rename to node/core/pvf/execute-worker/cli/src/main.rs diff --git a/node/core/pvf/prepare-worker/Cargo.toml b/node/core/pvf/prepare-worker/Cargo.toml index 3bd1fd43b673..92444668f16a 100644 --- a/node/core/pvf/prepare-worker/Cargo.toml +++ b/node/core/pvf/prepare-worker/Cargo.toml @@ -25,6 +25,9 @@ sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-maybe-compressed-blob = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" } +[build-dependencies] +substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } + [target.'cfg(target_os = "linux")'.dependencies] tikv-jemalloc-ctl = "0.5.0" diff --git a/node/core/pvf/prepare-worker/build.rs b/node/core/pvf/prepare-worker/build.rs new file mode 100644 index 000000000000..84fe22e23ed6 --- /dev/null +++ b/node/core/pvf/prepare-worker/build.rs @@ -0,0 +1,22 @@ +// 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 . + +fn main() { + substrate_build_script_utils::generate_cargo_keys(); + // 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(); +} diff --git a/node/core/pvf/prepare-worker/cli/Cargo.toml b/node/core/pvf/prepare-worker/cli/Cargo.toml new file mode 100644 index 000000000000..53d2bab54d9e --- /dev/null +++ b/node/core/pvf/prepare-worker/cli/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "polkadot-node-core-pvf-prepare-worker-cli" +version.workspace = true +authors.workspace = true +edition.workspace = true + +[[bin]] +name = "polkadot-prepare-worker" +path = "src/main.rs" + +[dependencies] +polkadot-node-core-pvf-common = { path = "../../common" } +polkadot-node-core-pvf-prepare-worker = { path = "../" } + +[build-dependencies] +substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/pvf/prepare-worker/cli/build.rs b/node/core/pvf/prepare-worker/cli/build.rs new file mode 100644 index 000000000000..84fe22e23ed6 --- /dev/null +++ b/node/core/pvf/prepare-worker/cli/build.rs @@ -0,0 +1,22 @@ +// 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 . + +fn main() { + substrate_build_script_utils::generate_cargo_keys(); + // 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(); +} diff --git a/src/bin/prepare-worker.rs b/node/core/pvf/prepare-worker/cli/src/main.rs similarity index 100% rename from src/bin/prepare-worker.rs rename to node/core/pvf/prepare-worker/cli/src/main.rs diff --git a/node/malus/Cargo.toml b/node/malus/Cargo.toml index 7e0bf0d8dd08..8b00a308fb47 100644 --- a/node/malus/Cargo.toml +++ b/node/malus/Cargo.toml @@ -12,19 +12,6 @@ publish = false name = "malus" path = "src/malus.rs" -# Use artifact dependencies once stable. -# See https://github.com/rust-lang/cargo/issues/9096. -[[bin]] -name = "polkadot-execute-worker" -path = "../../src/bin/execute-worker.rs" -# Prevent rustdoc error. Already documented from top-level Cargo.toml. -doc = false -[[bin]] -name = "polkadot-prepare-worker" -path = "../../src/bin/prepare-worker.rs" -# Prevent rustdoc error. Already documented from top-level Cargo.toml. -doc = false - [dependencies] polkadot-cli = { path = "../../cli", features = [ "malus", "rococo-native", "kusama-native", "westend-native", "polkadot-native" ] } polkadot-node-subsystem = { path = "../subsystem" } @@ -47,11 +34,6 @@ gum = { package = "tracing-gum", path = "../gum/" } erasure = { package = "polkadot-erasure-coding", path = "../../erasure-coding" } rand = "0.8.5" -# Required for worker binaries to build. -polkadot-node-core-pvf-common = { path = "../core/pvf/common" } -polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker" } -polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker" } - [dev-dependencies] polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } From 3c43f007c44b7598c0f7ec78352dc657197b31dd Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Wed, 2 Aug 2023 11:15:00 +0200 Subject: [PATCH 02/14] Move worker CLIs closer to the root package --- Cargo.lock | 36 +++++++++---------- Cargo.toml | 8 ++--- .../cli => bin/execute-worker-cli}/Cargo.toml | 6 ++-- .../cli => bin/execute-worker-cli}/build.rs | 0 .../execute-worker-cli}/src/main.rs | 0 .../cli => bin/prepare-worker-cli}/Cargo.toml | 6 ++-- .../prepare-worker-cli}/build.rs | 0 .../prepare-worker-cli}/src/main.rs | 0 node/core/pvf/prepare-worker/cli/build.rs | 22 ------------ 9 files changed, 28 insertions(+), 50 deletions(-) rename {node/core/pvf/execute-worker/cli => bin/execute-worker-cli}/Cargo.toml (59%) rename {node/core/pvf/execute-worker/cli => bin/execute-worker-cli}/build.rs (100%) rename {node/core/pvf/execute-worker/cli => bin/execute-worker-cli}/src/main.rs (100%) rename {node/core/pvf/prepare-worker/cli => bin/prepare-worker-cli}/Cargo.toml (59%) rename {node/core/pvf/prepare-worker => bin/prepare-worker-cli}/build.rs (100%) rename {node/core/pvf/prepare-worker/cli => bin/prepare-worker-cli}/src/main.rs (100%) delete mode 100644 node/core/pvf/prepare-worker/cli/build.rs diff --git a/Cargo.lock b/Cargo.lock index c3f282d69e4d..2e24e75b0cb8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6671,6 +6671,15 @@ dependencies = [ "thiserror", ] +[[package]] +name = "polkadot-execute-worker-cli" +version = "0.9.43" +dependencies = [ + "polkadot-node-core-pvf-common", + "polkadot-node-core-pvf-execute-worker", + "substrate-build-script-utils", +] + [[package]] name = "polkadot-gossip-support" version = "0.9.43" @@ -7087,15 +7096,6 @@ dependencies = [ "tracing-gum", ] -[[package]] -name = "polkadot-node-core-pvf-execute-worker-cli" -version = "0.9.43" -dependencies = [ - "polkadot-node-core-pvf-common", - "polkadot-node-core-pvf-execute-worker", - "substrate-build-script-utils", -] - [[package]] name = "polkadot-node-core-pvf-prepare-worker" version = "0.9.43" @@ -7119,15 +7119,6 @@ dependencies = [ "tracing-gum", ] -[[package]] -name = "polkadot-node-core-pvf-prepare-worker-cli" -version = "0.9.43" -dependencies = [ - "polkadot-node-core-pvf-common", - "polkadot-node-core-pvf-prepare-worker", - "substrate-build-script-utils", -] - [[package]] name = "polkadot-node-core-runtime-api" version = "0.9.43" @@ -7387,6 +7378,15 @@ dependencies = [ "thiserror", ] +[[package]] +name = "polkadot-prepare-worker-cli" +version = "0.9.43" +dependencies = [ + "polkadot-node-core-pvf-common", + "polkadot-node-core-pvf-prepare-worker", + "substrate-build-script-utils", +] + [[package]] name = "polkadot-primitives" version = "0.9.43" diff --git a/Cargo.toml b/Cargo.toml index 31fdc2a84b10..a94801a8fbcb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,8 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" [workspace] members = [ "cli", + "bin/execute-worker-cli", + "bin/prepare-worker-cli", "core-primitives", "erasure-coding", "erasure-coding/fuzzer", @@ -87,9 +89,7 @@ members = [ "node/core/pvf", "node/core/pvf/common", "node/core/pvf/execute-worker", - "node/core/pvf/execute-worker/cli", "node/core/pvf/prepare-worker", - "node/core/pvf/prepare-worker/cli", "node/core/pvf-checker", "node/core/runtime-api", "node/network/approval-distribution", @@ -133,8 +133,8 @@ members = [ default-members = [ ".", - "node/core/pvf/execute-worker/cli", - "node/core/pvf/prepare-worker/cli", + "bin/execute-worker-cli", + "bin/prepare-worker-cli", ] [badges] diff --git a/node/core/pvf/execute-worker/cli/Cargo.toml b/bin/execute-worker-cli/Cargo.toml similarity index 59% rename from node/core/pvf/execute-worker/cli/Cargo.toml rename to bin/execute-worker-cli/Cargo.toml index ef0c1b7d6857..7b55f79e1609 100644 --- a/node/core/pvf/execute-worker/cli/Cargo.toml +++ b/bin/execute-worker-cli/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "polkadot-node-core-pvf-execute-worker-cli" +name = "polkadot-execute-worker-cli" version.workspace = true authors.workspace = true edition.workspace = true @@ -9,8 +9,8 @@ name = "polkadot-execute-worker" path = "src/main.rs" [dependencies] -polkadot-node-core-pvf-common = { path = "../../common" } -polkadot-node-core-pvf-execute-worker = { path = "../" } +polkadot-node-core-pvf-common = { path = "../../node/core/pvf/common" } +polkadot-node-core-pvf-execute-worker = { path = "../../node/core/pvf/execute-worker" } [build-dependencies] substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/pvf/execute-worker/cli/build.rs b/bin/execute-worker-cli/build.rs similarity index 100% rename from node/core/pvf/execute-worker/cli/build.rs rename to bin/execute-worker-cli/build.rs diff --git a/node/core/pvf/execute-worker/cli/src/main.rs b/bin/execute-worker-cli/src/main.rs similarity index 100% rename from node/core/pvf/execute-worker/cli/src/main.rs rename to bin/execute-worker-cli/src/main.rs diff --git a/node/core/pvf/prepare-worker/cli/Cargo.toml b/bin/prepare-worker-cli/Cargo.toml similarity index 59% rename from node/core/pvf/prepare-worker/cli/Cargo.toml rename to bin/prepare-worker-cli/Cargo.toml index 53d2bab54d9e..c8055d5f2467 100644 --- a/node/core/pvf/prepare-worker/cli/Cargo.toml +++ b/bin/prepare-worker-cli/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "polkadot-node-core-pvf-prepare-worker-cli" +name = "polkadot-prepare-worker-cli" version.workspace = true authors.workspace = true edition.workspace = true @@ -9,8 +9,8 @@ name = "polkadot-prepare-worker" path = "src/main.rs" [dependencies] -polkadot-node-core-pvf-common = { path = "../../common" } -polkadot-node-core-pvf-prepare-worker = { path = "../" } +polkadot-node-core-pvf-common = { path = "../../node/core/pvf/common" } +polkadot-node-core-pvf-prepare-worker = { path = "../../node/core/pvf/prepare-worker" } [build-dependencies] substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/pvf/prepare-worker/build.rs b/bin/prepare-worker-cli/build.rs similarity index 100% rename from node/core/pvf/prepare-worker/build.rs rename to bin/prepare-worker-cli/build.rs diff --git a/node/core/pvf/prepare-worker/cli/src/main.rs b/bin/prepare-worker-cli/src/main.rs similarity index 100% rename from node/core/pvf/prepare-worker/cli/src/main.rs rename to bin/prepare-worker-cli/src/main.rs diff --git a/node/core/pvf/prepare-worker/cli/build.rs b/node/core/pvf/prepare-worker/cli/build.rs deleted file mode 100644 index 84fe22e23ed6..000000000000 --- a/node/core/pvf/prepare-worker/cli/build.rs +++ /dev/null @@ -1,22 +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 . - -fn main() { - substrate_build_script_utils::generate_cargo_keys(); - // 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(); -} From 25f950ceb56f2e139c4def4e7b8f5cc67871f83a Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Wed, 2 Aug 2023 17:15:42 +0200 Subject: [PATCH 03/14] Rename crates --- Cargo.lock | 4 ++-- Cargo.toml | 8 ++++---- bin/{execute-worker-cli => execute-worker}/Cargo.toml | 2 +- bin/{execute-worker-cli => execute-worker}/build.rs | 0 bin/{execute-worker-cli => execute-worker}/src/main.rs | 0 bin/{prepare-worker-cli => prepare-worker}/Cargo.toml | 2 +- bin/{prepare-worker-cli => prepare-worker}/build.rs | 0 bin/{prepare-worker-cli => prepare-worker}/src/main.rs | 0 8 files changed, 8 insertions(+), 8 deletions(-) rename bin/{execute-worker-cli => execute-worker}/Cargo.toml (92%) rename bin/{execute-worker-cli => execute-worker}/build.rs (100%) rename bin/{execute-worker-cli => execute-worker}/src/main.rs (100%) rename bin/{prepare-worker-cli => prepare-worker}/Cargo.toml (92%) rename bin/{prepare-worker-cli => prepare-worker}/build.rs (100%) rename bin/{prepare-worker-cli => prepare-worker}/src/main.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 2e24e75b0cb8..5d7ad6df8f9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6672,7 +6672,7 @@ dependencies = [ ] [[package]] -name = "polkadot-execute-worker-cli" +name = "polkadot-execute-worker" version = "0.9.43" dependencies = [ "polkadot-node-core-pvf-common", @@ -7379,7 +7379,7 @@ dependencies = [ ] [[package]] -name = "polkadot-prepare-worker-cli" +name = "polkadot-prepare-worker" version = "0.9.43" dependencies = [ "polkadot-node-core-pvf-common", diff --git a/Cargo.toml b/Cargo.toml index a94801a8fbcb..f352450fbb54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,8 +43,8 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" [workspace] members = [ "cli", - "bin/execute-worker-cli", - "bin/prepare-worker-cli", + "bin/execute-worker", + "bin/prepare-worker", "core-primitives", "erasure-coding", "erasure-coding/fuzzer", @@ -133,8 +133,8 @@ members = [ default-members = [ ".", - "bin/execute-worker-cli", - "bin/prepare-worker-cli", + "bin/execute-worker", + "bin/prepare-worker", ] [badges] diff --git a/bin/execute-worker-cli/Cargo.toml b/bin/execute-worker/Cargo.toml similarity index 92% rename from bin/execute-worker-cli/Cargo.toml rename to bin/execute-worker/Cargo.toml index 7b55f79e1609..73820df432d8 100644 --- a/bin/execute-worker-cli/Cargo.toml +++ b/bin/execute-worker/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "polkadot-execute-worker-cli" +name = "polkadot-execute-worker" version.workspace = true authors.workspace = true edition.workspace = true diff --git a/bin/execute-worker-cli/build.rs b/bin/execute-worker/build.rs similarity index 100% rename from bin/execute-worker-cli/build.rs rename to bin/execute-worker/build.rs diff --git a/bin/execute-worker-cli/src/main.rs b/bin/execute-worker/src/main.rs similarity index 100% rename from bin/execute-worker-cli/src/main.rs rename to bin/execute-worker/src/main.rs diff --git a/bin/prepare-worker-cli/Cargo.toml b/bin/prepare-worker/Cargo.toml similarity index 92% rename from bin/prepare-worker-cli/Cargo.toml rename to bin/prepare-worker/Cargo.toml index c8055d5f2467..50597c91ab2c 100644 --- a/bin/prepare-worker-cli/Cargo.toml +++ b/bin/prepare-worker/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "polkadot-prepare-worker-cli" +name = "polkadot-prepare-worker" version.workspace = true authors.workspace = true edition.workspace = true diff --git a/bin/prepare-worker-cli/build.rs b/bin/prepare-worker/build.rs similarity index 100% rename from bin/prepare-worker-cli/build.rs rename to bin/prepare-worker/build.rs diff --git a/bin/prepare-worker-cli/src/main.rs b/bin/prepare-worker/src/main.rs similarity index 100% rename from bin/prepare-worker-cli/src/main.rs rename to bin/prepare-worker/src/main.rs From 53d035dbc9ee94a83547ff45eafdc9cae5b9c496 Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Wed, 2 Aug 2023 19:24:09 +0200 Subject: [PATCH 04/14] Fix malus --- scripts/ci/gitlab/pipeline/build.yml | 2 +- tests/workers.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/ci/gitlab/pipeline/build.yml b/scripts/ci/gitlab/pipeline/build.yml index dafca393cd4f..5b6fe261a690 100644 --- a/scripts/ci/gitlab/pipeline/build.yml +++ b/scripts/ci/gitlab/pipeline/build.yml @@ -79,7 +79,7 @@ build-malus: - .compiler-info - .collect-artifacts script: - - time cargo build --locked --profile testnet --verbose -p polkadot-test-malus + - time cargo build --locked --profile testnet --verbose -p polkadot-test-malus -p polkadot-execute-worker -p polkadot-prepare-worker # pack artifacts - mkdir -p ./artifacts - mv ./target/testnet/malus ./artifacts/. diff --git a/tests/workers.rs b/tests/workers.rs index 2872a1298dcd..c6a35a13bac9 100644 --- a/tests/workers.rs +++ b/tests/workers.rs @@ -17,8 +17,8 @@ use polkadot_cli::NODE_VERSION; use std::process::Command; -const PREPARE_WORKER_EXE: &str = env!("CARGO_BIN_EXE_polkadot-prepare-worker"); -const EXECUTE_WORKER_EXE: &str = env!("CARGO_BIN_EXE_polkadot-execute-worker"); +const PREPARE_WORKER_EXE: &str = "polkadot-prepare-worker"; +const EXECUTE_WORKER_EXE: &str = "polkadot-execute-worker"; #[test] fn worker_binaries_have_same_version_as_node() { From 5ae83f3633de20e382fd3f5319660bf1310e03f1 Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Tue, 8 Aug 2023 16:47:53 +0200 Subject: [PATCH 05/14] Fix tests --- Cargo.lock | 9 +++++++++ Cargo.toml | 2 ++ bin/execute-worker/src/lib.rs | 18 ++++++++++++++++++ bin/execute-worker/src/main.rs | 10 ++++++++++ bin/execute-worker/tests/intergation.rs | 23 +++++++++++++++++++++++ bin/prepare-worker/src/lib.rs | 18 ++++++++++++++++++ bin/prepare-worker/src/main.rs | 10 ++++++++++ bin/prepare-worker/tests/integration.rs | 23 +++++++++++++++++++++++ tests/workers.rs | 16 +++++++++++----- 9 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 bin/execute-worker/src/lib.rs create mode 100644 bin/execute-worker/tests/intergation.rs create mode 100644 bin/prepare-worker/src/lib.rs create mode 100644 bin/prepare-worker/tests/integration.rs diff --git a/Cargo.lock b/Cargo.lock index 5d7ad6df8f9c..5426986dc411 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6435,9 +6435,11 @@ dependencies = [ "nix 0.26.2", "polkadot-cli", "polkadot-core-primitives", + "polkadot-execute-worker", "polkadot-node-core-pvf", "polkadot-node-core-pvf-prepare-worker", "polkadot-overseer", + "polkadot-prepare-worker", "substrate-build-script-utils", "substrate-rpc-client", "tempfile", @@ -6677,7 +6679,14 @@ version = "0.9.43" dependencies = [ "polkadot-node-core-pvf-common", "polkadot-node-core-pvf-execute-worker", + "polkadot-primitives", + "polkadot-test-service", + "sc-cli", + "sc-service", + "sp-keyring", "substrate-build-script-utils", + "substrate-test-utils", + "tokio", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index f352450fbb54..1edbeb4373c5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,8 @@ tempfile = "3.2.0" tokio = "1.24.2" substrate-rpc-client = { git = "https://github.com/paritytech/substrate", branch = "master" } polkadot-core-primitives = { path = "core-primitives" } +polkadot-execute-worker = { path = "bin/execute-worker" } +polkadot-prepare-worker = { path = "bin/prepare-worker" } [build-dependencies] substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/bin/execute-worker/src/lib.rs b/bin/execute-worker/src/lib.rs new file mode 100644 index 000000000000..0d5591d45852 --- /dev/null +++ b/bin/execute-worker/src/lib.rs @@ -0,0 +1,18 @@ +// 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 . + +/// Execute worker binary name +pub const BINARY_NAME: &str = "polkadot-execute-worker"; diff --git a/bin/execute-worker/src/main.rs b/bin/execute-worker/src/main.rs index 72cab799d753..32d3cbc1d131 100644 --- a/bin/execute-worker/src/main.rs +++ b/bin/execute-worker/src/main.rs @@ -16,8 +16,18 @@ //! Execute worker. +#[cfg(test)] +use polkadot_execute_worker::BINARY_NAME; + polkadot_node_core_pvf_common::decl_worker_main!( "execute-worker", polkadot_node_core_pvf_execute_worker::worker_entrypoint, env!("SUBSTRATE_CLI_IMPL_VERSION") ); + +#[test] +fn name_test() { + // If binary name ever needs to change, it must be changed in the package manifest + // as well as in the constant value + assert_eq!(env!("CARGO_BIN_NAME"), BINARY_NAME); +} diff --git a/bin/execute-worker/tests/intergation.rs b/bin/execute-worker/tests/intergation.rs new file mode 100644 index 000000000000..a3b6e2647784 --- /dev/null +++ b/bin/execute-worker/tests/intergation.rs @@ -0,0 +1,23 @@ +// 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 . + +//! Dummy integration test needed for the binary to get build when other intergation +//! tests are run + +#[test] +fn dummy_test() { + assert!(true); +} diff --git a/bin/prepare-worker/src/lib.rs b/bin/prepare-worker/src/lib.rs new file mode 100644 index 000000000000..058ace9d5f6d --- /dev/null +++ b/bin/prepare-worker/src/lib.rs @@ -0,0 +1,18 @@ +// 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 . + +/// Prepare worker binary name +pub const BINARY_NAME: &str = "polkadot-prepare-worker"; diff --git a/bin/prepare-worker/src/main.rs b/bin/prepare-worker/src/main.rs index 695f66cc7b7d..e3cd31fd26f1 100644 --- a/bin/prepare-worker/src/main.rs +++ b/bin/prepare-worker/src/main.rs @@ -16,8 +16,18 @@ //! Prepare worker. +#[cfg(test)] +use polkadot_prepare_worker::BINARY_NAME; + polkadot_node_core_pvf_common::decl_worker_main!( "prepare-worker", polkadot_node_core_pvf_prepare_worker::worker_entrypoint, env!("SUBSTRATE_CLI_IMPL_VERSION") ); + +#[test] +fn name_test() { + // If binary name ever needs to change, it must be changed in the package manifest + // as well as in the constant value + assert_eq!(env!("CARGO_BIN_NAME"), BINARY_NAME); +} diff --git a/bin/prepare-worker/tests/integration.rs b/bin/prepare-worker/tests/integration.rs new file mode 100644 index 000000000000..a3b6e2647784 --- /dev/null +++ b/bin/prepare-worker/tests/integration.rs @@ -0,0 +1,23 @@ +// 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 . + +//! Dummy integration test needed for the binary to get build when other intergation +//! tests are run + +#[test] +fn dummy_test() { + assert!(true); +} diff --git a/tests/workers.rs b/tests/workers.rs index c6a35a13bac9..f79158ba9323 100644 --- a/tests/workers.rs +++ b/tests/workers.rs @@ -15,22 +15,28 @@ // along with Polkadot. If not, see . use polkadot_cli::NODE_VERSION; +use polkadot_execute_worker::BINARY_NAME as EXECUTE_WORKER_EXE; +use polkadot_prepare_worker::BINARY_NAME as PREPARE_WORKER_EXE; use std::process::Command; -const PREPARE_WORKER_EXE: &str = "polkadot-prepare-worker"; -const EXECUTE_WORKER_EXE: &str = "polkadot-execute-worker"; - #[test] fn worker_binaries_have_same_version_as_node() { + // We're in `deps/` directory, target directory is one level up + let mut path = std::env::current_exe().unwrap(); + path.pop(); + path.pop(); + path.push(&PREPARE_WORKER_EXE); let prep_worker_version = - Command::new(&PREPARE_WORKER_EXE).args(["--version"]).output().unwrap().stdout; + Command::new(path.clone()).args(["--version"]).output().unwrap().stdout; let prep_worker_version = std::str::from_utf8(&prep_worker_version) .expect("version is printed as a string; qed") .trim(); assert_eq!(prep_worker_version, NODE_VERSION); + path.pop(); + path.push(&EXECUTE_WORKER_EXE); let exec_worker_version = - Command::new(&EXECUTE_WORKER_EXE).args(["--version"]).output().unwrap().stdout; + Command::new(path).args(["--version"]).output().unwrap().stdout; let exec_worker_version = std::str::from_utf8(&exec_worker_version) .expect("version is printed as a string; qed") .trim(); From c9329ca7b28f96858ce5a0c55596a30238bdde4a Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Tue, 8 Aug 2023 17:18:57 +0200 Subject: [PATCH 06/14] `Cargo.lock` & `cargo fmt` --- Cargo.lock | 7 ------- tests/workers.rs | 3 +-- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5426986dc411..3b6e84e154ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6679,14 +6679,7 @@ version = "0.9.43" dependencies = [ "polkadot-node-core-pvf-common", "polkadot-node-core-pvf-execute-worker", - "polkadot-primitives", - "polkadot-test-service", - "sc-cli", - "sc-service", - "sp-keyring", "substrate-build-script-utils", - "substrate-test-utils", - "tokio", ] [[package]] diff --git a/tests/workers.rs b/tests/workers.rs index f79158ba9323..07dad41f1492 100644 --- a/tests/workers.rs +++ b/tests/workers.rs @@ -35,8 +35,7 @@ fn worker_binaries_have_same_version_as_node() { path.pop(); path.push(&EXECUTE_WORKER_EXE); - let exec_worker_version = - Command::new(path).args(["--version"]).output().unwrap().stdout; + let exec_worker_version = Command::new(path).args(["--version"]).output().unwrap().stdout; let exec_worker_version = std::str::from_utf8(&exec_worker_version) .expect("version is printed as a string; qed") .trim(); From 8d50a4cece2cff15ce9a68d07d82eb0df381fcfc Mon Sep 17 00:00:00 2001 From: Marcin S Date: Wed, 9 Aug 2023 16:46:08 +0200 Subject: [PATCH 07/14] Try to fix `cargo run` not building worker binaries --- Cargo.toml | 6 ------ build.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 65c6ff63ea1a..29a9de5d3835 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -134,12 +134,6 @@ members = [ "utils/generate-bags", ] -default-members = [ - ".", - "bin/execute-worker", - "bin/prepare-worker", -] - [badges] maintenance = { status = "actively-developed" } diff --git a/build.rs b/build.rs index 84fe22e23ed6..f39ae23672a2 100644 --- a/build.rs +++ b/build.rs @@ -14,7 +14,59 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Build script to ensure that PVF worker binaries are always built whenever polkadot is built. +//! +//! This is needed because `default-members` does the same thing, but only for `cargo build` -- it +//! does not work for `cargo run`. + +use std::{env::var, path::Path, process::Command}; + +use polkadot_node_core_pvf::{EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME}; + +// TODO: unskip +#[rustfmt::skip] fn main() { + // Build additional artifacts if a single package build is explicitly requested. + { + let cargo = dbg!(var("CARGO").expect("`CARGO` env variable is always set by cargo")); + let target = dbg!(var("TARGET").expect("`TARGET` env variable is always set by cargo")); + let profile = dbg!(var("PROFILE").expect("`PROFILE` env variable is always set by cargo")); + let out_dir = dbg!(var("OUT_DIR").expect("`OUT_DIR` env variable is always set by cargo")); + let target_dir = format!("{}/workers", out_dir); + + // assert!(false); + + // TODO: opt-level, debug, features, etc. + let mut args = vec![ + "build", + "-p", + EXECUTE_BINARY_NAME, + "-p", + PREPARE_BINARY_NAME, + "--target", + &target, + "--target-dir", + &target_dir, + ]; + if profile != "debug" { + args.push("--profile"); + args.push(&profile); + } + + Command::new(cargo).args(&args).status().unwrap(); + std::fs::rename( + Path::new(&format!("{target_dir}/{target}/{profile}/{EXECUTE_BINARY_NAME}")), + Path::new(&format!("{target_dir}/../../../../{EXECUTE_BINARY_NAME}")), + ) + .unwrap(); + std::fs::rename( + Path::new(&format!("{target_dir}/{target}/{profile}/{PREPARE_BINARY_NAME}")), + Path::new(&format!("{target_dir}/../../../../{PREPARE_BINARY_NAME}")), + ) + .unwrap(); + } + + // TODO: is this needed here? substrate_build_script_utils::generate_cargo_keys(); // For the node/worker version check, make sure we always rebuild the node and binary workers // when the version changes. From 3ab565761e08afa2979be7ad86c8878b9db5ae9d Mon Sep 17 00:00:00 2001 From: Marcin S Date: Wed, 9 Aug 2023 19:17:30 +0200 Subject: [PATCH 08/14] Resolve some TODOs, make output nicer I looked into whether profile settings are passed along, and they are! So this script is basically good-to-go, modulo a couple of small TODOs. --- Cargo.lock | 1 + Cargo.toml | 1 + build.rs | 61 ++++++++++++++++++++++++++++++++++++++---------------- 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7ff3f480b28b..fe43584b1eaf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6432,6 +6432,7 @@ checksum = "e3d7ddaed09e0eb771a79ab0fd64609ba0afb0a8366421957936ad14cbd13630" name = "polkadot" version = "0.9.43" dependencies = [ + "ansi_term", "assert_cmd", "color-eyre", "nix 0.26.2", diff --git a/Cargo.toml b/Cargo.toml index 29a9de5d3835..3c2dce09a312 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ polkadot-execute-worker = { path = "bin/execute-worker" } polkadot-prepare-worker = { path = "bin/prepare-worker" } [build-dependencies] +ansi_term = "0.12.1" substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } [workspace] diff --git a/build.rs b/build.rs index f39ae23672a2..056ddc121425 100644 --- a/build.rs +++ b/build.rs @@ -14,29 +14,33 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -//! Build script to ensure that PVF worker binaries are always built whenever polkadot is built. -//! -//! This is needed because `default-members` does the same thing, but only for `cargo build` -- it -//! does not work for `cargo run`. +//! Build script to ensure that node/worker versions stay in sync and PVF worker binaries are always +//! built whenever polkadot is built. use std::{env::var, path::Path, process::Command}; -use polkadot_node_core_pvf::{EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME}; +// TODO: fix +// use polkadot_node_core_pvf::{PREPARE_BINARY_NAME, EXECUTE_BINARY_NAME}; +const PREPARE_BINARY_NAME: &str = "polkadot-prepare-worker"; +const EXECUTE_BINARY_NAME: &str = "polkadot-execute-worker"; -// TODO: unskip -#[rustfmt::skip] fn main() { - // Build additional artifacts if a single package build is explicitly requested. + // Always build PVF worker binaries whenever polkadot is built. + // + // This is needed because `default-members` does the same thing, but only for `cargo build` -- + // it does not work for `cargo run`. + // + // TODO: Test with `cargo + ...` { - let cargo = dbg!(var("CARGO").expect("`CARGO` env variable is always set by cargo")); - let target = dbg!(var("TARGET").expect("`TARGET` env variable is always set by cargo")); - let profile = dbg!(var("PROFILE").expect("`PROFILE` env variable is always set by cargo")); - let out_dir = dbg!(var("OUT_DIR").expect("`OUT_DIR` env variable is always set by cargo")); + let cargo = var("CARGO").expect("`CARGO` env variable is always set by cargo"); + let target = var("TARGET").expect("`TARGET` env variable is always set by cargo"); + let profile = var("PROFILE").expect("`PROFILE` env variable is always set by cargo"); + let out_dir = var("OUT_DIR").expect("`OUT_DIR` env variable is always set by cargo"); let target_dir = format!("{}/workers", out_dir); - // assert!(false); - - // TODO: opt-level, debug, features, etc. + // Settings like overflow-checks, opt-level, lto, etc. are correctly passed to cargo + // subcommand through env vars, e.g. `CARGO_CFG_OVERFLOW_CHECKS`, which the subcommand + // inherits. We don't pass along features because the workers don't use any right now. let mut args = vec![ "build", "-p", @@ -52,18 +56,32 @@ fn main() { args.push("--profile"); args.push(&profile); } + let mut build_cmd = Command::new(cargo); + build_cmd.args(&args); + + println!( + "{}", + colorize_info_message("Information that should be included in a bug report.") + ); + println!("{} {:?}", colorize_info_message("Executing build command:"), build_cmd); + // println!("{} {}", colorize_info_message("Using rustc version:"), build_cmd.rustc_version()); + + match build_cmd.status().map(|s| s.success()) { + Ok(true) => (), + // Use `process.exit(1)` to have a clean error output. + _ => std::process::exit(1), + } - Command::new(cargo).args(&args).status().unwrap(); std::fs::rename( Path::new(&format!("{target_dir}/{target}/{profile}/{EXECUTE_BINARY_NAME}")), Path::new(&format!("{target_dir}/../../../../{EXECUTE_BINARY_NAME}")), ) - .unwrap(); + .unwrap(); std::fs::rename( Path::new(&format!("{target_dir}/{target}/{profile}/{PREPARE_BINARY_NAME}")), Path::new(&format!("{target_dir}/../../../../{PREPARE_BINARY_NAME}")), ) - .unwrap(); + .unwrap(); } // TODO: is this needed here? @@ -72,3 +90,10 @@ fn main() { // when the version changes. substrate_build_script_utils::rerun_if_git_head_changed(); } + +/// Colorize an info message. +/// +/// Returns the colorized message. +fn colorize_info_message(message: &str) -> String { + ansi_term::Color::Yellow.bold().paint(message).to_string() +} From 96d2a90e38e6c4d5f8ca5badfe68df724742a5ff Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Thu, 10 Aug 2023 11:34:20 +0200 Subject: [PATCH 09/14] Fix filename typo --- bin/execute-worker/tests/{intergation.rs => integration.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename bin/execute-worker/tests/{intergation.rs => integration.rs} (100%) diff --git a/bin/execute-worker/tests/intergation.rs b/bin/execute-worker/tests/integration.rs similarity index 100% rename from bin/execute-worker/tests/intergation.rs rename to bin/execute-worker/tests/integration.rs From 63ee51520ff0fae185e7bffe2352caea013e23ba Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Thu, 10 Aug 2023 12:14:39 +0200 Subject: [PATCH 10/14] Remove duplicated constants --- Cargo.lock | 2 ++ node/core/pvf/Cargo.toml | 2 ++ node/core/pvf/src/host.rs | 6 ------ node/core/pvf/src/lib.rs | 4 +++- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe43584b1eaf..74ef8817685d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7014,12 +7014,14 @@ dependencies = [ "parity-scale-codec", "pin-project", "polkadot-core-primitives", + "polkadot-execute-worker", "polkadot-node-core-pvf-common", "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker", "polkadot-node-metrics", "polkadot-node-primitives", "polkadot-parachain", + "polkadot-prepare-worker", "polkadot-primitives", "rand 0.8.5", "slotmap", diff --git a/node/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml index 02a56ed9d2df..9fe072561872 100644 --- a/node/core/pvf/Cargo.toml +++ b/node/core/pvf/Cargo.toml @@ -32,6 +32,8 @@ polkadot-node-core-pvf-prepare-worker = { path = "prepare-worker" } polkadot-node-metrics = { path = "../../metrics" } polkadot-node-primitives = { path = "../../primitives" } polkadot-primitives = { path = "../../../primitives" } +polkadot-prepare-worker = { path = "../../../bin/prepare-worker" } +polkadot-execute-worker = { path = "../../../bin/execute-worker" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-wasm-interface = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/core/pvf/src/host.rs b/node/core/pvf/src/host.rs index a5772e34e16e..5e51f9a2dfc4 100644 --- a/node/core/pvf/src/host.rs +++ b/node/core/pvf/src/host.rs @@ -52,12 +52,6 @@ pub const PREPARE_FAILURE_COOLDOWN: Duration = Duration::from_millis(200); /// The amount of times we will retry failed prepare jobs. pub const NUM_PREPARE_RETRIES: u32 = 5; -/// The name of binary spawned to prepare a PVF artifact -pub const PREPARE_BINARY_NAME: &str = "polkadot-prepare-worker"; - -/// The name of binary spawned to execute a PVF -pub const EXECUTE_BINARY_NAME: &str = "polkadot-execute-worker"; - /// An alias to not spell the type for the oneshot sender for the PVF execution result. pub(crate) type ResultSender = oneshot::Sender>; diff --git a/node/core/pvf/src/lib.rs b/node/core/pvf/src/lib.rs index 2ed3f5242ded..02785b437de3 100644 --- a/node/core/pvf/src/lib.rs +++ b/node/core/pvf/src/lib.rs @@ -105,10 +105,12 @@ pub mod testing; pub use sp_tracing; pub use error::{InvalidCandidate, ValidationError}; -pub use host::{start, Config, ValidationHost, EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME}; +pub use host::{start, Config, ValidationHost}; pub use metrics::Metrics; pub use priority::Priority; pub use worker_intf::{framed_recv, framed_send, JOB_TIMEOUT_WALL_CLOCK_FACTOR}; +pub use polkadot_prepare_worker::BINARY_NAME as PREPARE_BINARY_NAME; +pub use polkadot_execute_worker::BINARY_NAME as EXECUTE_BINARY_NAME; // Re-export some common types. pub use polkadot_node_core_pvf_common::{ From c172aabd701af4564c45ed9183f1f9712d61c29c Mon Sep 17 00:00:00 2001 From: Dmitry Sinyavin Date: Thu, 10 Aug 2023 12:15:13 +0200 Subject: [PATCH 11/14] `cargo fmt` --- node/core/pvf/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/core/pvf/src/lib.rs b/node/core/pvf/src/lib.rs index 02785b437de3..50011cf360eb 100644 --- a/node/core/pvf/src/lib.rs +++ b/node/core/pvf/src/lib.rs @@ -107,10 +107,10 @@ pub use sp_tracing; pub use error::{InvalidCandidate, ValidationError}; pub use host::{start, Config, ValidationHost}; pub use metrics::Metrics; +pub use polkadot_execute_worker::BINARY_NAME as EXECUTE_BINARY_NAME; +pub use polkadot_prepare_worker::BINARY_NAME as PREPARE_BINARY_NAME; pub use priority::Priority; pub use worker_intf::{framed_recv, framed_send, JOB_TIMEOUT_WALL_CLOCK_FACTOR}; -pub use polkadot_prepare_worker::BINARY_NAME as PREPARE_BINARY_NAME; -pub use polkadot_execute_worker::BINARY_NAME as EXECUTE_BINARY_NAME; // Re-export some common types. pub use polkadot_node_core_pvf_common::{ From 85f0a50f5e5765142f293ee735db1eb8ef443ed0 Mon Sep 17 00:00:00 2001 From: Marcin S Date: Fri, 11 Aug 2023 14:44:20 +0200 Subject: [PATCH 12/14] Fix build script - [ ] Fix profile - [ ] Use eprintln! - [ ] Add test steps - [ ] Remove some TODOs - [ ] Add some comments --- Cargo.lock | 53 ++++++++++++++++++++++++++++++++++++++++------------- Cargo.toml | 12 ++++++++++++ build.rs | 46 +++++++++++++++++++++++++++++++--------------- 3 files changed, 83 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 74ef8817685d..255f0d160f2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -135,6 +135,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "aho-corasick" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86b8f9420f797f2d9e935edf629310eb938a0d839f984e25327f3c7eed22300c" +dependencies = [ + "memchr", +] + [[package]] name = "allocator-api2" version = "0.2.16" @@ -620,7 +629,7 @@ checksum = "ba3569f383e8f1598449f1a423e72e99569137b47740b1da11ef19af3d5c3223" dependencies = [ "lazy_static", "memchr", - "regex-automata", + "regex-automata 0.1.10", ] [[package]] @@ -2845,7 +2854,7 @@ version = "0.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "10463d9ff00a2a068db14231982f5132edebad0d7660cd956a1c30292dbcbfbd" dependencies = [ - "aho-corasick", + "aho-corasick 0.7.18", "bstr", "fnv", "log", @@ -4449,7 +4458,7 @@ version = "0.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f099785f7595cc4b4553a174ce30dd7589ef93391ff414dbb67f62392b9e0ce1" dependencies = [ - "regex-automata", + "regex-automata 0.1.10", ] [[package]] @@ -4458,7 +4467,7 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8263075bb86c5a1b1427b5ae862e8889656f126e9f77c484496e8b47cf5c5558" dependencies = [ - "regex-automata", + "regex-automata 0.1.10", ] [[package]] @@ -4478,9 +4487,9 @@ dependencies = [ [[package]] name = "memchr" -version = "2.4.1" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "308cc39be01b73d0d18f82a0e7b2a3df85245f84af96fdddc5d202d27e47b86a" +checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" [[package]] name = "memfd" @@ -6443,6 +6452,8 @@ dependencies = [ "polkadot-node-core-pvf-prepare-worker", "polkadot-overseer", "polkadot-prepare-worker", + "regex", + "rustc_version", "substrate-build-script-utils", "substrate-rpc-client", "tempfile", @@ -7014,14 +7025,12 @@ dependencies = [ "parity-scale-codec", "pin-project", "polkadot-core-primitives", - "polkadot-execute-worker", "polkadot-node-core-pvf-common", "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker", "polkadot-node-metrics", "polkadot-node-primitives", "polkadot-parachain", - "polkadot-prepare-worker", "polkadot-primitives", "rand 0.8.5", "slotmap", @@ -8670,13 +8679,14 @@ dependencies = [ [[package]] name = "regex" -version = "1.6.0" +version = "1.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c4eb3267174b8c6c2f654116623910a0fef09c4753f8dd83db29c48a0df988b" +checksum = "81bc1d4caf89fac26a70747fe603c130093b53c773888797a6329091246d651a" dependencies = [ - "aho-corasick", + "aho-corasick 1.0.3", "memchr", - "regex-syntax", + "regex-automata 0.3.6", + "regex-syntax 0.7.4", ] [[package]] @@ -8685,7 +8695,18 @@ version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" dependencies = [ - "regex-syntax", + "regex-syntax 0.6.27", +] + +[[package]] +name = "regex-automata" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fed1ceff11a1dddaee50c9dc8e4938bd106e9d89ae372f192311e7da498e3b69" +dependencies = [ + "aho-corasick 1.0.3", + "memchr", + "regex-syntax 0.7.4", ] [[package]] @@ -8694,6 +8715,12 @@ version = "0.6.27" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a3f87b73ce11b1619a3c6332f45341e0047173771e8b8b73f87bfeefb7b56244" +[[package]] +name = "regex-syntax" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5ea92a5b6195c6ef2a0295ea818b312502c6fc94dde986c5553242e18fd4ce2" + [[package]] name = "remote-ext-tests-bags-list" version = "0.9.43" diff --git a/Cargo.toml b/Cargo.toml index 3c2dce09a312..f787ebc1a3a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,8 +42,14 @@ polkadot-prepare-worker = { path = "bin/prepare-worker" } [build-dependencies] ansi_term = "0.12.1" +regex = "1.9.3" +rustc_version = "0.4.0" substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } +# To get the binary names. +polkadot-execute-worker = { path = "bin/execute-worker" } +polkadot-prepare-worker = { path = "bin/prepare-worker" } + [workspace] members = [ "cli", @@ -135,6 +141,12 @@ members = [ "utils/generate-bags", ] +default-members = [ + ".", + "bin/execute-worker", + "bin/prepare-worker", +] + [badges] maintenance = { status = "actively-developed" } diff --git a/build.rs b/build.rs index 056ddc121425..84ebaf1397f2 100644 --- a/build.rs +++ b/build.rs @@ -16,31 +16,47 @@ //! Build script to ensure that node/worker versions stay in sync and PVF worker binaries are always //! built whenever polkadot is built. +//! +//! NOTE: The crates for the workers should still be specified in `default-members` in order for +//! `cargo install` to work. +//! +//! The following scenarios have been tested. Run these tests after changes to this script. +//! +//! - [x] `cargo run` +//! - [ ] `cargo run`, commit, `cargo run` again (workers should be rebuilt) +//! - [x] `cargo build` +//! - [x] `cargo + run` +//! - [ ] TODO: `cargo clean` then `cargo install` +//! - [ ] TODO: `cargo run` then `cargo install` with binaries already having been built +//! - [x] `cargo run` with a profile, like `--profile testnet`. use std::{env::var, path::Path, process::Command}; -// TODO: fix -// use polkadot_node_core_pvf::{PREPARE_BINARY_NAME, EXECUTE_BINARY_NAME}; -const PREPARE_BINARY_NAME: &str = "polkadot-prepare-worker"; -const EXECUTE_BINARY_NAME: &str = "polkadot-execute-worker"; +use polkadot_execute_worker::BINARY_NAME as EXECUTE_BINARY_NAME; +use polkadot_prepare_worker::BINARY_NAME as PREPARE_BINARY_NAME; fn main() { // Always build PVF worker binaries whenever polkadot is built. // // This is needed because `default-members` does the same thing, but only for `cargo build` -- // it does not work for `cargo run`. - // - // TODO: Test with `cargo + ...` { let cargo = var("CARGO").expect("`CARGO` env variable is always set by cargo"); let target = var("TARGET").expect("`TARGET` env variable is always set by cargo"); - let profile = var("PROFILE").expect("`PROFILE` env variable is always set by cargo"); let out_dir = var("OUT_DIR").expect("`OUT_DIR` env variable is always set by cargo"); let target_dir = format!("{}/workers", out_dir); - // Settings like overflow-checks, opt-level, lto, etc. are correctly passed to cargo - // subcommand through env vars, e.g. `CARGO_CFG_OVERFLOW_CHECKS`, which the subcommand - // inherits. We don't pass along features because the workers don't use any right now. + // Get the profile from OUT_DIR instead of the PROFILE env var. If we are using e.g. testnet + // which inherits from release, then PROFILE will be release. The only way to get the actual + // profile (testnet) is with this hacky parsing code. 🙈 We need to get the actual profile + // to pass along settings like LTO (which is not even available to this build script). + let re = regex::Regex::new(r".*/target/(?\w+)/build/.*").unwrap(); + let caps = re.captures(&out_dir).expect("regex broke, please contact your local regex repair facility."); + let profile = &caps["profile"]; + + // Construct the `cargo build ...` command. + // + // We don't pass along features because the workers don't use any right now. let mut args = vec![ "build", "-p", @@ -52,6 +68,7 @@ fn main() { "--target-dir", &target_dir, ]; + // Needed to prevent an error. if profile != "debug" { args.push("--profile"); args.push(&profile); @@ -59,12 +76,12 @@ fn main() { let mut build_cmd = Command::new(cargo); build_cmd.args(&args); - println!( + eprintln!( "{}", colorize_info_message("Information that should be included in a bug report.") ); - println!("{} {:?}", colorize_info_message("Executing build command:"), build_cmd); - // println!("{} {}", colorize_info_message("Using rustc version:"), build_cmd.rustc_version()); + eprintln!("{} {:?}", colorize_info_message("Executing build command:"), build_cmd); + eprintln!("{} {:?}", colorize_info_message("Using rustc version:"), rustc_version::version()); match build_cmd.status().map(|s| s.success()) { Ok(true) => (), @@ -72,6 +89,7 @@ fn main() { _ => std::process::exit(1), } + // Move the built worker binaries into the same location as the `polkadot` binary. std::fs::rename( Path::new(&format!("{target_dir}/{target}/{profile}/{EXECUTE_BINARY_NAME}")), Path::new(&format!("{target_dir}/../../../../{EXECUTE_BINARY_NAME}")), @@ -84,8 +102,6 @@ fn main() { .unwrap(); } - // TODO: is this needed here? - substrate_build_script_utils::generate_cargo_keys(); // 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(); From c4d88440df7d31f03d0d9afd8bad10410714f210 Mon Sep 17 00:00:00 2001 From: Marcin S Date: Fri, 11 Aug 2023 16:00:13 +0200 Subject: [PATCH 13/14] Update build script --- Cargo.lock | 2 ++ build.rs | 37 ++++++++++++++++++++++++++----------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 255f0d160f2e..b77bef5d972a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7025,12 +7025,14 @@ dependencies = [ "parity-scale-codec", "pin-project", "polkadot-core-primitives", + "polkadot-execute-worker", "polkadot-node-core-pvf-common", "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker", "polkadot-node-metrics", "polkadot-node-primitives", "polkadot-parachain", + "polkadot-prepare-worker", "polkadot-primitives", "rand 0.8.5", "slotmap", diff --git a/build.rs b/build.rs index 84ebaf1397f2..c6f09158f155 100644 --- a/build.rs +++ b/build.rs @@ -20,15 +20,24 @@ //! NOTE: The crates for the workers should still be specified in `default-members` in order for //! `cargo install` to work. //! +//! # Testing +//! //! The following scenarios have been tested. Run these tests after changes to this script. //! -//! - [x] `cargo run` -//! - [ ] `cargo run`, commit, `cargo run` again (workers should be rebuilt) -//! - [x] `cargo build` -//! - [x] `cargo + run` -//! - [ ] TODO: `cargo clean` then `cargo install` -//! - [ ] TODO: `cargo run` then `cargo install` with binaries already having been built -//! - [x] `cargo run` with a profile, like `--profile testnet`. +//! - [x] `cargo clean` then `cargo run` (workers should be built) +//! +//! - [x] `cargo run`, commit, `cargo run` again (workers should be rebuilt the second time) +//! +//! - [x] `cargo clean` then `cargo build` (workers should be built) +//! +//! - [x] `cargo + run` (same as `cargo run`) +//! +//! - [ ] TODO: `cargo clean` then `cargo install` (latest workers should be installed) +//! +//! - [ ] TODO: `cargo clean`, `cargo run`, then `cargo install` with binaries already having been +//! built. +//! +//! - [x] `cargo run` with a profile, like `--profile testnet` (same as `cargo run`) use std::{env::var, path::Path, process::Command}; @@ -39,7 +48,7 @@ fn main() { // Always build PVF worker binaries whenever polkadot is built. // // This is needed because `default-members` does the same thing, but only for `cargo build` -- - // it does not work for `cargo run`. + // by itself, `default-members` does not work for `cargo run`. { let cargo = var("CARGO").expect("`CARGO` env variable is always set by cargo"); let target = var("TARGET").expect("`TARGET` env variable is always set by cargo"); @@ -51,12 +60,14 @@ fn main() { // profile (testnet) is with this hacky parsing code. 🙈 We need to get the actual profile // to pass along settings like LTO (which is not even available to this build script). let re = regex::Regex::new(r".*/target/(?\w+)/build/.*").unwrap(); - let caps = re.captures(&out_dir).expect("regex broke, please contact your local regex repair facility."); + let caps = re + .captures(&out_dir) + .expect("regex broke, please contact your local regex repair facility."); let profile = &caps["profile"]; // Construct the `cargo build ...` command. // - // We don't pass along features because the workers don't use any right now. + // NOTE: We don't pass along features because the workers don't use any right now. let mut args = vec![ "build", "-p", @@ -81,7 +92,11 @@ fn main() { colorize_info_message("Information that should be included in a bug report.") ); eprintln!("{} {:?}", colorize_info_message("Executing build command:"), build_cmd); - eprintln!("{} {:?}", colorize_info_message("Using rustc version:"), rustc_version::version()); + eprintln!( + "{} {:?}", + colorize_info_message("Using rustc version:"), + rustc_version::version() + ); match build_cmd.status().map(|s| s.success()) { Ok(true) => (), From a8c98c17840cbe0013bb71d0f95050bdc01d81e8 Mon Sep 17 00:00:00 2001 From: Marcin S Date: Sun, 13 Aug 2023 15:50:12 +0200 Subject: [PATCH 14/14] Update comment --- build.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/build.rs b/build.rs index c6f09158f155..001728e5b421 100644 --- a/build.rs +++ b/build.rs @@ -55,10 +55,11 @@ fn main() { let out_dir = var("OUT_DIR").expect("`OUT_DIR` env variable is always set by cargo"); let target_dir = format!("{}/workers", out_dir); - // Get the profile from OUT_DIR instead of the PROFILE env var. If we are using e.g. testnet - // which inherits from release, then PROFILE will be release. The only way to get the actual - // profile (testnet) is with this hacky parsing code. 🙈 We need to get the actual profile - // to pass along settings like LTO (which is not even available to this build script). + // HACK: Get the profile from OUT_DIR instead of the PROFILE env var. If we are using e.g. + // testnet which inherits from release, then PROFILE will be release. The only way to get + // the actual profile (e.g. testnet) is with this hacky parsing code. 🙈 We need to get the + // actual profile to pass along settings like LTO (which is not even available to this build + // script) and build the binaries as expected. let re = regex::Regex::new(r".*/target/(?\w+)/build/.*").unwrap(); let caps = re .captures(&out_dir)