Add cli option --wasmtime-precompiled and subcommand precompile-wasm
#1641
Conversation
|
What's your motivation to add this? Is this only supposed to be used for tests? I'd be fine to have this to speed up tests, but I'm not convinced we should support this for running normal nodes, as it would add extra complexity and be yet another place where something can go wrong for not much benefit. There's also the issue that for PVFs in Polkadot we already have caching implemented, so this would essentially duplicate that functionality in an incompatible way. (Although the requirements are different, as for PVFs we need sandboxing, so it's not really obvious how you would unify those.) How about we fundamentally simplify this and do something like this instead?
This would be, I think, simpler, need less code, be more maintainable, be more foolproof, and be easier to use. |
|
Yeah I was proposing to go this route. They are running a lot of tests in their CI and that got really slow with the removal of I'm not sure that a env variable is the way to go here either. No one will find out about this env variable besides the three of us :P I would propose that you extend |
Yeah, that also sounds good to me. |
|
The command should output the name of the file also IMO |
|
The expected wasm and the generated one don't have the same hash: |
|
The wasm is actually loaded at multiple places with different hashes: The other one is initiated by grandpa where it retrieve the runtime and hash in the call_executor, computed in the state-machine using the This mean that for the same code, we have 2 different hashes and so we won't load the precompiled wasm |
|
The CI pipeline was cancelled due to failure one of the required jobs. |
|
Something to verify. I replaced the hasher of the genesis code to use the Blake2Hasher to be compatible with the precompiled and on-chain wasm calculated hash: After that change and marking allow_missing_func_imports to false, I was able on Moonbeam to boot the node without any compilation. Something to note also, by default the "precompile-wasm" command will use the default base-path if none is provided, making this command: |
|
The |
This isn't a problem specific to moonbeam, it concerns all substrate chains that need to spawn a lot of nodes for their end2end tests. For example, Tanssi also has many end2end typescrit tests.
This is linked to the fact that the command doesn't know if you want to retrieve the runtime code from the DB or not, so it has to be initialized. Adding an explicit flag to retrieve from chain spec should solve the problem. |
|
Any update on this? as @librelois this would be heplful for other ecosystem projects as well (like Tanssi in this case) |
This cherry-pick should match this pull request: paritytech#1641
This cherry-pick should match this pull request: paritytech#1641
This cherry-pick should match this pull request: paritytech#1641
This cherry-pick should match this pull request: paritytech#1641
This cherry-pick should match this pull request: paritytech#1641
|
@bkchr Do you think this could be merged ? (We are maintaining it in our cherry-picks but it would be ideal to have it merged) |
This cherry-pick should match this pull request: paritytech#1641
There was a problem hiding this comment.
@librelois please apply my comments and merge master and then we can go ahead merging this.
| ); | ||
| let mut buffer = Vec::new(); | ||
| buffer.extend_from_slice(code_hash); | ||
| buffer.extend_from_slice(sp_wasm_interface::VERSION.as_bytes()); |
There was a problem hiding this comment.
This should use the wasmtime version.
| /// wasmtime modules. | ||
| /// | ||
| /// By default there is no `wasmtime_precompiled_path` given. | ||
| pub fn with_wasmtime_precompiled_path( |
There was a problem hiding this comment.
You should also one with_optional_wasmtime_precompiled_path function. Then we don't need the if let Some code when setting up the builder.
| @@ -234,6 +251,8 @@ pub struct WasmExecutor<H> { | |||
| cache_path: Option<PathBuf>, | |||
| /// Ignore missing function imports. | |||
| allow_missing_host_functions: bool, | |||
| /// TODO | |||
| let mut file = std::fs::File::create( | ||
| wasmtime_precompiled_path.join(format!("precompiled_wasm_{}", &artifact_version)), | ||
| ) | ||
| .map_err(|e| { | ||
| WasmError::Other(format!( | ||
| "Fail to create file 'precompiled_wasm_0x{}', I/O Error: {}", | ||
| &artifact_version, e | ||
| )) | ||
| })?; | ||
| file.write_all(&serialized_precompiled_wasm).map_err(|e| { | ||
| WasmError::Other(format!("Fail to write precompiled artifact, I/O Error: {}", e)) | ||
| })?; |
There was a problem hiding this comment.
| let mut file = std::fs::File::create( | |
| wasmtime_precompiled_path.join(format!("precompiled_wasm_{}", &artifact_version)), | |
| ) | |
| .map_err(|e| { | |
| WasmError::Other(format!( | |
| "Fail to create file 'precompiled_wasm_0x{}', I/O Error: {}", | |
| &artifact_version, e | |
| )) | |
| })?; | |
| file.write_all(&serialized_precompiled_wasm).map_err(|e| { | |
| WasmError::Other(format!("Fail to write precompiled artifact, I/O Error: {}", e)) | |
| })?; | |
| std::fs::write(wasmtime_precompiled_path.join(format!("precompiled_wasm_{artifact_version}")), &serialized_precompiled_wasm).map_err(|e| { | |
| WasmError::Other(format!("Fail to write precompiled artifact, I/O Error: {}", e)) | |
| })?; |
| code_fetcher: &sp_core::traits::WrappedRuntimeCode( | ||
| wasm_bytecode.as_slice().into(), | ||
| ), | ||
| hash: sp_core::blake2_256(&wasm_bytecode).to_vec(), |
There was a problem hiding this comment.
| hash: sp_core::blake2_256(&wasm_bytecode).to_vec(), | |
| hash: B::Hasher::hash(&wasm_bytecode).to_vec(), |
The hash depends on the configured hasher.
| for entry in std::fs::read_dir(wasmtime_precompiled_dir).map_err(handle_err)? { | ||
| let entry = entry.map_err(handle_err)?; | ||
| if let Some(file_name) = entry.file_name().to_str() { | ||
| // We check that the artifact was generated for this specific artifact | ||
| // version and with the same wasm interface version and configuration. | ||
| if file_name.contains(&artifact_version.clone()) { | ||
| log::info!( | ||
| target: "wasmtime-runtime", | ||
| "Found precompiled wasm: {}", | ||
| file_name | ||
| ); | ||
| // We change the version check strategy to make sure that the file | ||
| // content was serialized with the exact same config as well | ||
| maybe_compiled_artifact = Some(( | ||
| entry.path(), | ||
| sc_executor_wasmtime::ModuleVersionStrategy::Custom( | ||
| artifact_version.clone(), | ||
| ), | ||
| )); | ||
| } | ||
| } else { | ||
| return Err(WasmError::Instantiation( | ||
| "wasmtime precompiled folder contain a file with invalid utf8 name" | ||
| .to_owned(), | ||
| )) | ||
| } | ||
| } |
There was a problem hiding this comment.
We don't need to iterate? We know the artifact version and thus, can just generate the file name and check if the file exists?
| use sp_state_machine::backend::BackendRuntimeCode; | ||
| use std::{fmt::Debug, path::PathBuf, sync::Arc}; | ||
|
|
||
| /// The `precompile-wasm` command used to serialize a precompiled WASM module. |
There was a problem hiding this comment.
Please also mention which WASM is actually pre-compiled.
| // `prepare_runtime_artifact` and with the same wasmtime | ||
| // version and configuration. | ||
| unsafe { | ||
| sc_executor_wasmtime::create_runtime_from_artifact::<H>( |
There was a problem hiding this comment.
| sc_executor_wasmtime::create_runtime_from_artifact::<H>( | |
| return sc_executor_wasmtime::create_runtime_from_artifact::<H>( |
If you make this an early return, below we can move the create_runtime out of the if to only have it once.
This cherry-pick should match this pull request: paritytech#1641
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <git@kchr.de>
Description
What does this PR do?
This PR adds the possibility of starting the node with a precompiled WASM runtime.
This eliminates the need to compile the runtime at startup, making it possible to launch the node much more quickly.
Why are these changes needed?
A practical example of where this feature is useful is when you need to run a large number of end2end tests quickly: you precompile the runtime once in the same environment, then use the precompiled artifact every time you need to spawn the node (potentially hundreds of times).
How were these changes implemented and what do they affect?
As a security measure, a hash of the wasmtime configuration is inserted into the precompiled artifact and checked by wasmtime at startup, to ensure that, to ensure that the precompiled artifact has been generated under the same conditions.
This check is delegated to wasmtime directly thanks to the module_version function.
How to use
precompile-wasm:This will precompile the on-chain wasm at the last finalized block and save the precompiled artifact in
path/to/artifacts/folder.--wasmtime-precompiled path/to/artifacts/folder