From 5ee019610b9a163f10d97a8b04645db0a8b892ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 25 May 2023 14:53:21 +0200 Subject: [PATCH 1/4] wasm-builder: Enforce `runtime_version` wasm section This pr changes the `wasm-builder` to enforce the `runtime_version` wasm section. This wasm section is being created by the `sp_version::runtime_version` attribute macro. This attribute macro now exists since quite some time and `runtime_version` also is the only way for parachains to support reading the `RuntimeVersion` from the runtime. \# Disabling the check By default the `WasmBuilder` will now check for this wasm section and if not found, exit with an error. However, there are situations where you may want to disable this check (like for tests). In this case there exists the `disable_runtime_version_section_check` function. ``` WasmBuilder::new() ... ... ... .disable_runtime_version_section_check() .build() ``` By using this method you get back the old behavior. --- Cargo.lock | 1 + client/executor/runtime-test/build.rs | 2 ++ .../test-wasm-deprecated/build.rs | 1 + .../runtime-interface/test-wasm/build.rs | 1 + utils/wasm-builder/Cargo.toml | 3 +- utils/wasm-builder/src/builder.rs | 24 +++++++++++++-- utils/wasm-builder/src/wasm_project.rs | 29 +++++++++++++++++++ 7 files changed, 58 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7bffd9e169159..b89d46b31ec14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11860,6 +11860,7 @@ dependencies = [ "build-helper", "cargo_metadata", "filetime", + "parity-wasm", "sp-maybe-compressed-blob", "strum", "tempfile", diff --git a/client/executor/runtime-test/build.rs b/client/executor/runtime-test/build.rs index 745742123a8f6..088c93110d855 100644 --- a/client/executor/runtime-test/build.rs +++ b/client/executor/runtime-test/build.rs @@ -24,6 +24,7 @@ fn main() { .with_current_project() .export_heap_base() .import_memory() + .disable_runtime_version_section_check() .build(); } @@ -36,6 +37,7 @@ fn main() { .import_memory() .set_file_name("wasm_binary_with_tracing.rs") .append_to_rust_flags(r#"--cfg feature="with-tracing""#) + .disable_runtime_version_section_check() .build(); } } diff --git a/primitives/runtime-interface/test-wasm-deprecated/build.rs b/primitives/runtime-interface/test-wasm-deprecated/build.rs index b7676a70dfe84..651f57388e0d0 100644 --- a/primitives/runtime-interface/test-wasm-deprecated/build.rs +++ b/primitives/runtime-interface/test-wasm-deprecated/build.rs @@ -22,6 +22,7 @@ fn main() { .with_current_project() .export_heap_base() .import_memory() + .disable_runtime_version_section_check() .build(); } } diff --git a/primitives/runtime-interface/test-wasm/build.rs b/primitives/runtime-interface/test-wasm/build.rs index b7676a70dfe84..651f57388e0d0 100644 --- a/primitives/runtime-interface/test-wasm/build.rs +++ b/primitives/runtime-interface/test-wasm/build.rs @@ -22,6 +22,7 @@ fn main() { .with_current_project() .export_heap_base() .import_memory() + .disable_runtime_version_section_check() .build(); } } diff --git a/utils/wasm-builder/Cargo.toml b/utils/wasm-builder/Cargo.toml index 5228f26d57c6d..79ff4d19ac675 100644 --- a/utils/wasm-builder/Cargo.toml +++ b/utils/wasm-builder/Cargo.toml @@ -22,4 +22,5 @@ toml = "0.7.3" walkdir = "2.3.2" sp-maybe-compressed-blob = { version = "4.1.0-dev", path = "../../primitives/maybe-compressed-blob" } filetime = "0.2.16" -wasm-opt = "0.112" \ No newline at end of file +wasm-opt = "0.112" +parity-wasm = "0.45" diff --git a/utils/wasm-builder/src/builder.rs b/utils/wasm-builder/src/builder.rs index 72d32445e8da5..6cc4bcec4af2b 100644 --- a/utils/wasm-builder/src/builder.rs +++ b/utils/wasm-builder/src/builder.rs @@ -48,6 +48,7 @@ impl WasmBuilderSelectProject { file_name: None, project_cargo_toml: get_manifest_dir().join("Cargo.toml"), features_to_enable: Vec::new(), + check_for_runtime_version_section: true, } } @@ -63,6 +64,7 @@ impl WasmBuilderSelectProject { file_name: None, project_cargo_toml: path, features_to_enable: Vec::new(), + check_for_runtime_version_section: true, }) } else { Err("Project path must point to the `Cargo.toml` of the project") @@ -93,6 +95,8 @@ pub struct WasmBuilder { project_cargo_toml: PathBuf, /// Features that should be enabled when building the wasm binary. features_to_enable: Vec, + /// Should the builder check that the `runtime_version` section exists in the wasm binary? + check_for_runtime_version_section: bool, } impl WasmBuilder { @@ -143,6 +147,17 @@ impl WasmBuilder { self } + /// Disable the check for the `runtime_version` wasm section. + /// + /// By default the `wasm-builder` will ensure that the `runtime_version` section will + /// exists in the build wasm binary. This `runtime_version` section is used to get the + /// `RuntimeVersion` without needing to call into the wasm binary. However, for some + /// use cases (like tests) you may want to disable this check. + pub fn disable_runtime_version_section_check(mut self) -> Self { + self.check_for_runtime_version_section = false; + self + } + /// Build the WASM binary. pub fn build(self) { let out_dir = PathBuf::from(env::var("OUT_DIR").expect("`OUT_DIR` is set by cargo!")); @@ -165,6 +180,7 @@ impl WasmBuilder { self.rust_flags.into_iter().map(|f| format!("{} ", f)).collect(), self.features_to_enable, self.file_name, + self.check_for_runtime_version_section, ); // As last step we need to generate our `rerun-if-changed` stuff. If a build fails, we don't @@ -215,7 +231,7 @@ fn generate_rerun_if_changed_instructions() { /// The current project is determined by using the `CARGO_MANIFEST_DIR` environment variable. /// /// `file_name` - The name + path of the file being generated. The file contains the -/// constant `WASM_BINARY`, which contains the built WASM binary. +/// constant `WASM_BINARY`, which contains the built wasm binary. /// /// `project_cargo_toml` - The path to the `Cargo.toml` of the project that should be built. /// @@ -224,14 +240,17 @@ fn generate_rerun_if_changed_instructions() { /// `features_to_enable` - Features that should be enabled for the project. /// /// `wasm_binary_name` - The optional wasm binary name that is extended with -/// /// `.compact.compressed.wasm`. If `None`, the project name will be used. +/// +/// `check_for_runtime_version_section` - Should the wasm binary be checked for the +/// `runtime_version` section? fn build_project( file_name: PathBuf, project_cargo_toml: PathBuf, default_rustflags: String, features_to_enable: Vec, wasm_binary_name: Option, + check_for_runtime_version_section: bool, ) { let cargo_cmd = match crate::prerequisites::check() { Ok(cmd) => cmd, @@ -247,6 +266,7 @@ fn build_project( cargo_cmd, features_to_enable, wasm_binary_name, + check_for_runtime_version_section, ); let (wasm_binary, wasm_binary_bloaty) = if let Some(wasm_binary) = wasm_binary { diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index c45a40a6b9202..849af853c6da4 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -19,6 +19,7 @@ use crate::{write_file_if_changed, CargoCommandVersioned, OFFLINE}; use build_helper::rerun_if_changed; use cargo_metadata::{CargoOpt, Metadata, MetadataCommand}; +use parity_wasm::elements::{deserialize_buffer, Module}; use std::{ borrow::ToOwned, collections::HashSet, @@ -116,6 +117,7 @@ pub(crate) fn create_and_compile( cargo_cmd: CargoCommandVersioned, features_to_enable: Vec, wasm_binary_name: Option, + check_for_runtime_version_section: bool, ) -> (Option, WasmBinaryBloaty) { let wasm_workspace_root = get_wasm_workspace_root(); let wasm_workspace = wasm_workspace_root.join("wbuild"); @@ -134,6 +136,10 @@ pub(crate) fn create_and_compile( let (wasm_binary, wasm_binary_compressed, bloaty) = compact_wasm_file(&project, profile, project_cargo_toml, wasm_binary_name); + if check_for_runtime_version_section { + ensure_runtime_version_wasm_section_exists(bloaty.wasm_binary_bloaty_path()); + } + wasm_binary .as_ref() .map(|wasm_binary| copy_wasm_to_target_directory(project_cargo_toml, wasm_binary)); @@ -159,6 +165,29 @@ pub(crate) fn create_and_compile( (final_wasm_binary, bloaty) } +/// Ensures that the `runtime_version` wasm section exists in the given wasm file. +/// +/// If the section can not be found, it will print an error and exit the builder. +fn ensure_runtime_version_wasm_section_exists(wasm: &Path) { + let wasm_blob = fs::read(wasm).expect("`{wasm}` was just written and should exist; qed"); + + let module: Module = match deserialize_buffer(&wasm_blob) { + Ok(m) => m, + Err(e) => { + println!("Failed to deserialize `{}`: {e:?}", wasm.display()); + process::exit(1); + }, + }; + + if !module.custom_sections().any(|cs| cs.name() == "runtime_version") { + println!( + "Couldn't find the `runtime_version` wasm section. \ + Please ensure that you are using the `sp_version::runtime_version` attribute macro!" + ); + process::exit(1); + } +} + /// Adjust the mtime of the bloaty and compressed/compact wasm files. /// /// We add the bloaty and the compressed/compact wasm file to the `rerun-if-changed` files. From 03c01e096b4f139ed1b7de35cb07c26371a05674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 26 May 2023 11:44:20 +0200 Subject: [PATCH 2/4] Review comment --- utils/wasm-builder/src/builder.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/utils/wasm-builder/src/builder.rs b/utils/wasm-builder/src/builder.rs index 6cc4bcec4af2b..a444a4787759a 100644 --- a/utils/wasm-builder/src/builder.rs +++ b/utils/wasm-builder/src/builder.rs @@ -48,7 +48,7 @@ impl WasmBuilderSelectProject { file_name: None, project_cargo_toml: get_manifest_dir().join("Cargo.toml"), features_to_enable: Vec::new(), - check_for_runtime_version_section: true, + disable_runtime_version_section_check: fale, } } @@ -64,7 +64,7 @@ impl WasmBuilderSelectProject { file_name: None, project_cargo_toml: path, features_to_enable: Vec::new(), - check_for_runtime_version_section: true, + disable_runtime_version_section_check: false, }) } else { Err("Project path must point to the `Cargo.toml` of the project") @@ -95,8 +95,8 @@ pub struct WasmBuilder { project_cargo_toml: PathBuf, /// Features that should be enabled when building the wasm binary. features_to_enable: Vec, - /// Should the builder check that the `runtime_version` section exists in the wasm binary? - check_for_runtime_version_section: bool, + /// Should the builder not check that the `runtime_version` section exists in the wasm binary? + disable_runtime_version_section_check: bool, } impl WasmBuilder { @@ -154,7 +154,7 @@ impl WasmBuilder { /// `RuntimeVersion` without needing to call into the wasm binary. However, for some /// use cases (like tests) you may want to disable this check. pub fn disable_runtime_version_section_check(mut self) -> Self { - self.check_for_runtime_version_section = false; + self.disable_runtime_version_section_check = true; self } @@ -180,7 +180,7 @@ impl WasmBuilder { self.rust_flags.into_iter().map(|f| format!("{} ", f)).collect(), self.features_to_enable, self.file_name, - self.check_for_runtime_version_section, + !self.disable_runtime_version_section_check, ); // As last step we need to generate our `rerun-if-changed` stuff. If a build fails, we don't From 1a4d26d8fbc3f792976993f9e5f5aaeccbcb5443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 28 May 2023 13:03:01 +0200 Subject: [PATCH 3/4] Fix --- utils/wasm-builder/src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/wasm-builder/src/builder.rs b/utils/wasm-builder/src/builder.rs index a444a4787759a..d0bed8d22a036 100644 --- a/utils/wasm-builder/src/builder.rs +++ b/utils/wasm-builder/src/builder.rs @@ -48,7 +48,7 @@ impl WasmBuilderSelectProject { file_name: None, project_cargo_toml: get_manifest_dir().join("Cargo.toml"), features_to_enable: Vec::new(), - disable_runtime_version_section_check: fale, + disable_runtime_version_section_check: false, } } From 6c514c4dbdc70098ca13e40b3975174b064f5580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 29 May 2023 01:03:52 +0200 Subject: [PATCH 4/4] Fix issue with `enum-as-inner` --- Cargo.lock | 1 + bin/node-template/node/Cargo.toml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index b89d46b31ec14..54e5613bcab8d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5421,6 +5421,7 @@ name = "node-template" version = "4.0.0-dev" dependencies = [ "clap 4.2.7", + "enum-as-inner", "frame-benchmarking", "frame-benchmarking-cli", "frame-system", diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 1801caad676ab..ceac4fa8db1cd 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -66,6 +66,9 @@ node-template-runtime = { version = "4.0.0-dev", path = "../runtime" } # CLI-specific dependencies try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } +# Workaround until https://github.com/bluejekyll/enum-as-inner/issues/98 is fixed. +enum-as-inner = "=0.5.1" + [build-dependencies] substrate-build-script-utils = { version = "3.0.0", path = "../../../utils/build-script-utils" }