Allow missing functions when checking the new runtime's version (#5741)

This commit is contained in:
Bastian Köcher
2020-04-23 14:54:52 +02:00
committed by GitHub
parent fd65c3807c
commit 46677555ac
13 changed files with 73 additions and 16 deletions
+2
View File
@@ -1529,6 +1529,7 @@ dependencies = [
"frame-support", "frame-support",
"parity-scale-codec", "parity-scale-codec",
"pretty_assertions", "pretty_assertions",
"rustversion",
"serde", "serde",
"sp-core", "sp-core",
"sp-inherents", "sp-inherents",
@@ -7643,6 +7644,7 @@ name = "sp-runtime-interface-test"
version = "2.0.0-dev" version = "2.0.0-dev"
dependencies = [ dependencies = [
"sc-executor", "sc-executor",
"sp-core",
"sp-io", "sp-io",
"sp-runtime", "sp-runtime",
"sp-runtime-interface", "sp-runtime-interface",
@@ -45,7 +45,6 @@ fn call_in_wasm<E: Externalities>(
execution_method, execution_method,
Some(1024), Some(1024),
HostFunctions::host_functions(), HostFunctions::host_functions(),
true,
8, 8,
); );
executor.call_in_wasm( executor.call_in_wasm(
@@ -54,6 +53,7 @@ fn call_in_wasm<E: Externalities>(
function, function,
call_data, call_data,
ext, ext,
sp_core::traits::MissingHostFunctions::Allow,
) )
} }
@@ -511,7 +511,6 @@ fn should_trap_when_heap_exhausted(wasm_method: WasmExecutionMethod) {
wasm_method, wasm_method,
Some(17), // `17` is the initial number of pages compiled into the binary. Some(17), // `17` is the initial number of pages compiled into the binary.
HostFunctions::host_functions(), HostFunctions::host_functions(),
true,
8, 8,
); );
executor.call_in_wasm( executor.call_in_wasm(
@@ -520,6 +519,7 @@ fn should_trap_when_heap_exhausted(wasm_method: WasmExecutionMethod) {
"test_exhaust_heap", "test_exhaust_heap",
&[0], &[0],
&mut ext.ext(), &mut ext.ext(),
sp_core::traits::MissingHostFunctions::Allow,
).unwrap(); ).unwrap();
} }
+1 -1
View File
@@ -77,7 +77,6 @@ mod tests {
WasmExecutionMethod::Interpreted, WasmExecutionMethod::Interpreted,
Some(8), Some(8),
sp_io::SubstrateHostFunctions::host_functions(), sp_io::SubstrateHostFunctions::host_functions(),
true,
8, 8,
); );
let res = executor.call_in_wasm( let res = executor.call_in_wasm(
@@ -86,6 +85,7 @@ mod tests {
"test_empty_return", "test_empty_return",
&[], &[],
&mut ext, &mut ext,
sp_core::traits::MissingHostFunctions::Allow,
).unwrap(); ).unwrap();
assert_eq!(res, vec![0u8; 0]); assert_eq!(res, vec![0u8; 0]);
} }
@@ -20,7 +20,9 @@ use crate::{
}; };
use sp_version::{NativeVersion, RuntimeVersion}; use sp_version::{NativeVersion, RuntimeVersion};
use codec::{Decode, Encode}; use codec::{Decode, Encode};
use sp_core::{NativeOrEncoded, traits::{CodeExecutor, Externalities, RuntimeCode}}; use sp_core::{
NativeOrEncoded, traits::{CodeExecutor, Externalities, RuntimeCode, MissingHostFunctions},
};
use log::trace; use log::trace;
use std::{result, panic::{UnwindSafe, AssertUnwindSafe}, sync::Arc}; use std::{result, panic::{UnwindSafe, AssertUnwindSafe}, sync::Arc};
use sp_wasm_interface::{HostFunctions, Function}; use sp_wasm_interface::{HostFunctions, Function};
@@ -83,8 +85,6 @@ pub struct WasmExecutor {
host_functions: Arc<Vec<&'static dyn Function>>, host_functions: Arc<Vec<&'static dyn Function>>,
/// WASM runtime cache. /// WASM runtime cache.
cache: Arc<RuntimeCache>, cache: Arc<RuntimeCache>,
/// Allow missing function imports.
allow_missing_func_imports: bool,
/// The size of the instances cache. /// The size of the instances cache.
max_runtime_instances: usize, max_runtime_instances: usize,
} }
@@ -102,7 +102,6 @@ impl WasmExecutor {
method: WasmExecutionMethod, method: WasmExecutionMethod,
default_heap_pages: Option<u64>, default_heap_pages: Option<u64>,
host_functions: Vec<&'static dyn Function>, host_functions: Vec<&'static dyn Function>,
allow_missing_func_imports: bool,
max_runtime_instances: usize, max_runtime_instances: usize,
) -> Self { ) -> Self {
WasmExecutor { WasmExecutor {
@@ -110,7 +109,6 @@ impl WasmExecutor {
default_heap_pages: default_heap_pages.unwrap_or(DEFAULT_HEAP_PAGES), default_heap_pages: default_heap_pages.unwrap_or(DEFAULT_HEAP_PAGES),
host_functions: Arc::new(host_functions), host_functions: Arc::new(host_functions),
cache: Arc::new(RuntimeCache::new(max_runtime_instances)), cache: Arc::new(RuntimeCache::new(max_runtime_instances)),
allow_missing_func_imports,
max_runtime_instances, max_runtime_instances,
} }
} }
@@ -132,6 +130,7 @@ impl WasmExecutor {
&self, &self,
runtime_code: &RuntimeCode, runtime_code: &RuntimeCode,
ext: &mut dyn Externalities, ext: &mut dyn Externalities,
allow_missing_host_functions: bool,
f: F, f: F,
) -> Result<R> ) -> Result<R>
where F: FnOnce( where F: FnOnce(
@@ -146,7 +145,7 @@ impl WasmExecutor {
self.method, self.method,
self.default_heap_pages, self.default_heap_pages,
&*self.host_functions, &*self.host_functions,
self.allow_missing_func_imports, allow_missing_host_functions,
|instance, version, ext| { |instance, version, ext| {
let instance = AssertUnwindSafe(instance); let instance = AssertUnwindSafe(instance);
let ext = AssertUnwindSafe(ext); let ext = AssertUnwindSafe(ext);
@@ -167,7 +166,10 @@ impl sp_core::traits::CallInWasm for WasmExecutor {
method: &str, method: &str,
call_data: &[u8], call_data: &[u8],
ext: &mut dyn Externalities, ext: &mut dyn Externalities,
missing_host_functions: MissingHostFunctions,
) -> std::result::Result<Vec<u8>, String> { ) -> std::result::Result<Vec<u8>, String> {
let allow_missing_host_functions = missing_host_functions.allowed();
if let Some(hash) = code_hash { if let Some(hash) = code_hash {
let code = RuntimeCode { let code = RuntimeCode {
code_fetcher: &sp_core::traits::WrappedRuntimeCode(wasm_code.into()), code_fetcher: &sp_core::traits::WrappedRuntimeCode(wasm_code.into()),
@@ -175,7 +177,7 @@ impl sp_core::traits::CallInWasm for WasmExecutor {
heap_pages: None, heap_pages: None,
}; };
self.with_instance(&code, ext, |instance, _, mut ext| { self.with_instance(&code, ext, allow_missing_host_functions, |instance, _, mut ext| {
with_externalities_safe( with_externalities_safe(
&mut **ext, &mut **ext,
move || instance.call(method, call_data), move || instance.call(method, call_data),
@@ -187,7 +189,7 @@ impl sp_core::traits::CallInWasm for WasmExecutor {
self.default_heap_pages, self.default_heap_pages,
&wasm_code, &wasm_code,
self.host_functions.to_vec(), self.host_functions.to_vec(),
self.allow_missing_func_imports, allow_missing_host_functions,
) )
.map_err(|e| format!("Failed to create module: {:?}", e))?; .map_err(|e| format!("Failed to create module: {:?}", e))?;
@@ -240,7 +242,6 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
fallback_method, fallback_method,
default_heap_pages, default_heap_pages,
host_functions, host_functions,
false,
max_runtime_instances, max_runtime_instances,
); );
@@ -265,8 +266,9 @@ impl<D: NativeExecutionDispatch> RuntimeInfo for NativeExecutor<D> {
self.wasm.with_instance( self.wasm.with_instance(
runtime_code, runtime_code,
ext, ext,
false,
|_instance, version, _ext| |_instance, version, _ext|
Ok(version.cloned().ok_or_else(|| Error::ApiError("Unknown version".into()))) Ok(version.cloned().ok_or_else(|| Error::ApiError("Unknown version".into()))),
) )
} }
} }
@@ -290,6 +292,7 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeExecutor<D> {
let result = self.wasm.with_instance( let result = self.wasm.with_instance(
runtime_code, runtime_code,
ext, ext,
false,
|instance, onchain_version, mut ext| { |instance, onchain_version, mut ext| {
let onchain_version = onchain_version.ok_or_else( let onchain_version = onchain_version.ok_or_else(
|| Error::ApiError("Unknown version".into()) || Error::ApiError("Unknown version".into())
@@ -372,8 +375,9 @@ impl<D: NativeExecutionDispatch> sp_core::traits::CallInWasm for NativeExecutor<
method: &str, method: &str,
call_data: &[u8], call_data: &[u8],
ext: &mut dyn Externalities, ext: &mut dyn Externalities,
missing_host_functions: MissingHostFunctions,
) -> std::result::Result<Vec<u8>, String> { ) -> std::result::Result<Vec<u8>, String> {
self.wasm.call_in_wasm(wasm_blob, code_hash, method, call_data, ext) self.wasm.call_in_wasm(wasm_blob, code_hash, method, call_data, ext, missing_host_functions)
} }
} }
+1
View File
@@ -22,6 +22,7 @@ sp-runtime = { version = "2.0.0-dev", default-features = false, path = "../../..
sp-core = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/core" } sp-core = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/core" }
trybuild = "1.0.17" trybuild = "1.0.17"
pretty_assertions = "0.6.1" pretty_assertions = "0.6.1"
rustversion = "1.0.0"
[features] [features]
default = ["std"] default = ["std"]
@@ -1,5 +1,22 @@
// Copyright 2019-2020 Parity Technologies (UK) Ltd.
// This file is part of Substrate.
// Substrate 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.
// Substrate 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 Substrate. If not, see <http://www.gnu.org/licenses/>.
use std::env; use std::env;
#[rustversion::attr(not(stable), ignore)]
#[test] #[test]
fn ui() { fn ui() {
// As trybuild is using `cargo check`, we don't need the real WASM binaries. // As trybuild is using `cargo check`, we don't need the real WASM binaries.
@@ -14,6 +14,7 @@
// You should have received a copy of the GNU General Public License // You should have received a copy of the GNU General Public License
// along with Substrate. If not, see <http://www.gnu.org/licenses/>. // along with Substrate. If not, see <http://www.gnu.org/licenses/>.
#[rustversion::attr(not(stable), ignore)]
#[test] #[test]
fn reserved_keyword() { fn reserved_keyword() {
// As trybuild is using `cargo check`, we don't need the real WASM binaries. // As trybuild is using `cargo check`, we don't need the real WASM binaries.
+1
View File
@@ -2108,6 +2108,7 @@ pub(crate) mod tests {
_: &str, _: &str,
_: &[u8], _: &[u8],
_: &mut dyn sp_externalities::Externalities, _: &mut dyn sp_externalities::Externalities,
_: sp_core::traits::MissingHostFunctions,
) -> Result<Vec<u8>, String> { ) -> Result<Vec<u8>, String> {
Ok(self.0.clone()) Ok(self.0.clone())
} }
+18
View File
@@ -262,6 +262,23 @@ impl std::fmt::Display for CodeNotFound {
} }
} }
/// `Allow` or `Disallow` missing host functions when instantiating a WASM blob.
#[derive(Clone, Copy, Debug)]
pub enum MissingHostFunctions {
/// Any missing host function will be replaced by a stub that returns an error when
/// being called.
Allow,
/// Any missing host function will result in an error while instantiating the WASM blob,
Disallow,
}
impl MissingHostFunctions {
/// Are missing host functions allowed?
pub fn allowed(self) -> bool {
matches!(self, Self::Allow)
}
}
/// Something that can call a method in a WASM blob. /// Something that can call a method in a WASM blob.
pub trait CallInWasm: Send + Sync { pub trait CallInWasm: Send + Sync {
/// Call the given `method` in the given `wasm_blob` using `call_data` (SCALE encoded arguments) /// Call the given `method` in the given `wasm_blob` using `call_data` (SCALE encoded arguments)
@@ -280,6 +297,7 @@ pub trait CallInWasm: Send + Sync {
method: &str, method: &str,
call_data: &[u8], call_data: &[u8],
ext: &mut dyn Externalities, ext: &mut dyn Externalities,
missing_host_functions: MissingHostFunctions,
) -> Result<Vec<u8>, String>; ) -> Result<Vec<u8>, String>;
} }
+12 -1
View File
@@ -329,7 +329,18 @@ pub trait Misc {
self.extension::<CallInWasmExt>() self.extension::<CallInWasmExt>()
.expect("No `CallInWasmExt` associated for the current context!") .expect("No `CallInWasmExt` associated for the current context!")
.call_in_wasm(wasm, None, "Core_version", &[], &mut ext) .call_in_wasm(
wasm,
None,
"Core_version",
&[],
&mut ext,
// If a runtime upgrade introduces new host functions that are not provided by
// the node, we should not fail at instantiation. Otherwise nodes that are
// updated could run this successfully and it could lead to a storage root
// mismatch when importing this block.
sp_core::traits::MissingHostFunctions::Allow,
)
.ok() .ok()
} }
} }
@@ -18,5 +18,6 @@ sp-runtime-interface-test-wasm = { version = "2.0.0-dev", path = "../test-wasm"
sp-runtime-interface-test-wasm-deprecated = { version = "2.0.0-dev", path = "../test-wasm-deprecated" } sp-runtime-interface-test-wasm-deprecated = { version = "2.0.0-dev", path = "../test-wasm-deprecated" }
sp-state-machine = { version = "0.8.0-dev", path = "../../../primitives/state-machine" } sp-state-machine = { version = "0.8.0-dev", path = "../../../primitives/state-machine" }
sp-runtime = { version = "2.0.0-dev", path = "../../runtime" } sp-runtime = { version = "2.0.0-dev", path = "../../runtime" }
sp-core = { version = "2.0.0-dev", path = "../../core" }
sp-io = { version = "2.0.0-dev", path = "../../io" } sp-io = { version = "2.0.0-dev", path = "../../io" }
tracing = "0.1.13" tracing = "0.1.13"
@@ -41,7 +41,6 @@ fn call_wasm_method<HF: HostFunctionsT>(binary: &[u8], method: &str) -> TestExte
sc_executor::WasmExecutionMethod::Interpreted, sc_executor::WasmExecutionMethod::Interpreted,
Some(8), Some(8),
host_functions, host_functions,
false,
8, 8,
); );
executor.call_in_wasm( executor.call_in_wasm(
@@ -50,6 +49,7 @@ fn call_wasm_method<HF: HostFunctionsT>(binary: &[u8], method: &str) -> TestExte
method, method,
&[], &[],
&mut ext_ext, &mut ext_ext,
sp_core::traits::MissingHostFunctions::Disallow,
).expect(&format!("Executes `{}`", method)); ).expect(&format!("Executes `{}`", method));
ext ext
@@ -811,6 +811,7 @@ mod tests {
_: &str, _: &str,
_: &[u8], _: &[u8],
_: &mut dyn Externalities, _: &mut dyn Externalities,
_: sp_core::traits::MissingHostFunctions,
) -> std::result::Result<Vec<u8>, String> { ) -> std::result::Result<Vec<u8>, String> {
unimplemented!("Not required in tests.") unimplemented!("Not required in tests.")
} }