diff --git a/Cargo.lock b/Cargo.lock index 5b09589be93ea..59028fbc61bd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4630,6 +4630,7 @@ dependencies = [ "substrate-executor 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", + "substrate-panic-handler 2.0.0", "substrate-primitives 2.0.0", "substrate-state-machine 2.0.0", "substrate-telemetry 2.0.0", diff --git a/core/client/Cargo.toml b/core/client/Cargo.toml index 76daf1dc60dbe..3fc7407a7c50e 100644 --- a/core/client/Cargo.toml +++ b/core/client/Cargo.toml @@ -33,6 +33,7 @@ env_logger = "0.6" tempfile = "3.1" test-client = { package = "substrate-test-runtime-client", path = "../test-runtime/client" } kvdb-memorydb = { git = "https://github.com/paritytech/parity-common", rev="b0317f649ab2c665b7987b8475878fc4d2e1f81d" } +panic-handler = { package = "substrate-panic-handler", path = "../panic-handler" } [features] default = ["std"] diff --git a/core/client/src/client.rs b/core/client/src/client.rs index c7df30d8ae575..63d30b45e3b97 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -43,7 +43,7 @@ use state_machine::{ DBValue, Backend as StateBackend, ChangesTrieAnchorBlockId, ExecutionStrategy, ExecutionManager, prove_read, prove_child_read, ChangesTrieRootsStorage, ChangesTrieStorage, ChangesTrieTransaction, ChangesTrieConfigurationRange, key_changes, key_changes_proof, - OverlayedChanges, + OverlayedChanges, BackendTrustLevel, }; use executor::{RuntimeVersion, RuntimeInfo}; use consensus::{ @@ -1043,7 +1043,7 @@ impl Client where let get_execution_manager = |execution_strategy: ExecutionStrategy| { match execution_strategy { ExecutionStrategy::NativeElseWasm => ExecutionManager::NativeElseWasm, - ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm, + ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm(BackendTrustLevel::Trusted), ExecutionStrategy::NativeWhenPossible => ExecutionManager::NativeWhenPossible, ExecutionStrategy::Both => ExecutionManager::Both(|wasm_result, native_result| { let header = import_headers.post(); diff --git a/core/client/src/light/call_executor.rs b/core/client/src/light/call_executor.rs index 370b6054e6090..ab7b5b0a108b0 100644 --- a/core/client/src/light/call_executor.rs +++ b/core/client/src/light/call_executor.rs @@ -460,6 +460,32 @@ pub fn check_execution_proof( E: CodeExecutor, H: Hasher, H::Out: Ord + 'static, +{ + check_execution_proof_with_make_header( + executor, + request, + remote_proof, + |header|
::new( + *header.number() + One::one(), + Default::default(), + Default::default(), + header.hash(), + Default::default(), + ), + ) +} + +fn check_execution_proof_with_make_header Header>( + executor: &E, + request: &RemoteCallRequest
, + remote_proof: Vec>, + make_next_header: MakeNextHeader, +) -> ClientResult> + where + Header: HeaderT, + E: CodeExecutor, + H: Hasher, + H::Out: Ord + 'static, { let local_state_root = request.header.state_root(); let root: H::Out = convert_hash(&local_state_root); @@ -467,19 +493,13 @@ pub fn check_execution_proof( // prepare execution environment + check preparation proof let mut changes = OverlayedChanges::default(); let trie_backend = create_proof_check_backend(root, remote_proof)?; - let next_block =
::new( - *request.header.number() + One::one(), - Default::default(), - Default::default(), - request.header.hash(), - Default::default(), - ); + let next_header = make_next_header(&request.header); execution_proof_check_on_trie_backend::( &trie_backend, &mut changes, executor, "Core_initialize_block", - &next_block.encode(), + &next_header.encode(), None, )?; @@ -530,6 +550,43 @@ mod tests { (remote_result, local_result) } + fn execute_with_proof_failure(remote_client: &TestClient, at: u64, method: &'static str) { + let remote_block_id = BlockId::Number(at); + let remote_header = remote_client.header(&remote_block_id).unwrap().unwrap(); + + // 'fetch' execution proof from remote node + let (_, remote_execution_proof) = remote_client.execution_proof( + &remote_block_id, + method, + &[] + ).unwrap(); + + // check remote execution proof locally + let local_executor = NativeExecutor::::new(None); + let execution_result = check_execution_proof_with_make_header( + &local_executor, + &RemoteCallRequest { + block: test_client::runtime::Hash::default(), + header: remote_header, + method: method.into(), + call_data: vec![], + retry_count: None, + }, + remote_execution_proof, + |header|
::new( + at + 1, + Default::default(), + Default::default(), + header.hash(), + header.digest().clone(), // this makes next header wrong + ), + ); + match execution_result { + Err(crate::error::Error::Execution(_)) => (), + _ => panic!("Unexpected execution result: {:?}", execution_result), + } + } + // prepare remote client let remote_client = test_client::new(); for i in 1u32..3u32 { @@ -546,15 +603,24 @@ mod tests { let (remote, local) = execute(&remote_client, 0, "Core_version"); assert_eq!(remote, local); + let (remote, local) = execute(&remote_client, 2, "Core_version"); + assert_eq!(remote, local); + // check method that requires environment let (_, block) = execute(&remote_client, 0, "BlockBuilder_finalize_block"); let local_block: Header = Decode::decode(&mut &block[..]).unwrap(); assert_eq!(local_block.number, 1); - // check method that requires environment let (_, block) = execute(&remote_client, 2, "BlockBuilder_finalize_block"); let local_block: Header = Decode::decode(&mut &block[..]).unwrap(); assert_eq!(local_block.number, 3); + + // check that proof check doesn't panic even if proof is incorrect AND no panic handler is set + execute_with_proof_failure(&remote_client, 2, "Core_version"); + + // check that proof check doesn't panic even if proof is incorrect AND panic handler is set + panic_handler::set("TEST"); + execute_with_proof_failure(&remote_client, 2, "Core_version"); } #[test] diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index a2cf9e83bd86c..fd0ab30c9a7b1 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -806,7 +806,7 @@ mod tests { keystore.write().insert_ephemeral_from_seed::(&key.to_seed()) .expect("Creates authority key"); keystore_paths.push(keystore_path); - + let environ = DummyFactory(client.clone()); import_notifications.push( client.import_notification_stream() diff --git a/core/consensus/babe/src/tests.rs b/core/consensus/babe/src/tests.rs index 2410cadbd59ca..7274cc1230418 100644 --- a/core/consensus/babe/src/tests.rs +++ b/core/consensus/babe/src/tests.rs @@ -207,7 +207,7 @@ fn run_one_test() { let peer = net.peer(*peer_id); let client = peer.client().as_full().expect("Only full clients are used in tests").clone(); let select_chain = peer.select_chain().expect("Full client has select_chain"); - + let keystore_path = tempfile::tempdir().expect("Creates keystore path"); let keystore = keystore::Store::open(keystore_path.path(), None).expect("Creates keystore"); keystore.write().insert_ephemeral_from_seed::(seed).expect("Generates authority key"); diff --git a/core/executor/src/wasm_executor.rs b/core/executor/src/wasm_executor.rs index 4baa547ab8723..1cef4b7cb1ade 100644 --- a/core/executor/src/wasm_executor.rs +++ b/core/executor/src/wasm_executor.rs @@ -19,7 +19,7 @@ //! This module defines and implements the wasm part of Substrate Host Interface and provides //! an interface for calling into the wasm runtime. -use std::{convert::TryFrom, str}; +use std::{convert::TryFrom, str, panic}; use tiny_keccak; use secp256k1; @@ -31,8 +31,8 @@ use crate::error::{Error, Result}; use codec::{Encode, Decode}; use primitives::{ blake2_128, blake2_256, twox_64, twox_128, twox_256, ed25519, sr25519, Pair, crypto::KeyTypeId, - offchain, hexdisplay::HexDisplay, sandbox as sandbox_primitives, H256, Blake2Hasher, - traits::Externalities, child_storage_key::ChildStorageKey, + offchain, hexdisplay::HexDisplay, sandbox as sandbox_primitives, Blake2Hasher, + traits::Externalities, }; use trie::{TrieConfiguration, trie_types::Layout}; use crate::sandbox; @@ -447,7 +447,9 @@ impl_wasm_host_interface! { .map_err(|_| "Invalid attempt to determine key in ext_set_storage")?; let value = context.read_memory(value_data, value_len) .map_err(|_| "Invalid attempt to determine value in ext_set_storage")?; - runtime_io::set_storage(&key, &value); + with_external_storage(move || + Ok(runtime_io::set_storage(&key, &value)) + )?; Ok(()) } @@ -466,7 +468,9 @@ impl_wasm_host_interface! { let value = context.read_memory(value_data, value_len) .map_err(|_| "Invalid attempt to determine value in ext_set_child_storage")?; - runtime_io::set_child_storage(&storage_key, &key, &value); + with_external_storage(move || + Ok(runtime_io::set_child_storage(&storage_key, &key, &value)) + )?; Ok(()) } @@ -481,21 +485,27 @@ impl_wasm_host_interface! { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to determine key in ext_clear_child_storage")?; - runtime_io::clear_child_storage(&storage_key, &key); + with_external_storage(move || + Ok(runtime_io::clear_child_storage(&storage_key, &key)) + )?; Ok(()) } ext_clear_storage(key_data: Pointer, key_len: WordSize) { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to determine key in ext_clear_storage")?; - runtime_io::clear_storage(&key); + with_external_storage(move || + Ok(runtime_io::clear_storage(&key)) + )?; Ok(()) } ext_exists_storage(key_data: Pointer, key_len: WordSize) -> u32 { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to determine key in ext_exists_storage")?; - Ok(if runtime_io::exists_storage(&key) { 1 } else { 0 }) + with_external_storage(move || + Ok(if runtime_io::exists_storage(&key) { 1 } else { 0 }) + ) } ext_exists_child_storage( @@ -509,13 +519,17 @@ impl_wasm_host_interface! { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to determine key in ext_exists_child_storage")?; - Ok(if runtime_io::exists_child_storage(&storage_key, &key) { 1 } else { 0 }) + with_external_storage(move || + Ok(if runtime_io::exists_child_storage(&storage_key, &key) { 1 } else { 0 }) + ) } ext_clear_prefix(prefix_data: Pointer, prefix_len: WordSize) { let prefix = context.read_memory(prefix_data, prefix_len) .map_err(|_| "Invalid attempt to determine prefix in ext_clear_prefix")?; - runtime_io::clear_prefix(&prefix); + with_external_storage(move || + Ok(runtime_io::clear_prefix(&prefix)) + )?; Ok(()) } @@ -529,7 +543,9 @@ impl_wasm_host_interface! { .map_err(|_| "Invalid attempt to determine storage_key in ext_clear_child_prefix")?; let prefix = context.read_memory(prefix_data, prefix_len) .map_err(|_| "Invalid attempt to determine prefix in ext_clear_child_prefix")?; - runtime_io::clear_child_prefix(&storage_key, &prefix); + with_external_storage(move || + Ok(runtime_io::clear_child_prefix(&storage_key, &prefix)) + )?; Ok(()) } @@ -537,7 +553,9 @@ impl_wasm_host_interface! { ext_kill_child_storage(storage_key_data: Pointer, storage_key_len: WordSize) { let storage_key = context.read_memory(storage_key_data, storage_key_len) .map_err(|_| "Invalid attempt to determine storage_key in ext_kill_child_storage")?; - runtime_io::kill_child_storage(&storage_key); + with_external_storage(move || + Ok(runtime_io::kill_child_storage(&storage_key)) + )?; Ok(()) } @@ -549,7 +567,9 @@ impl_wasm_host_interface! { ) -> Pointer { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to determine key in ext_get_allocated_storage")?; - let maybe_value = runtime_io::storage(&key); + let maybe_value = with_external_storage(move || + Ok(runtime_io::storage(&key)) + )?; if let Some(value) = maybe_value { let offset = context.allocate_memory(value.len() as u32)?; @@ -577,7 +597,9 @@ impl_wasm_host_interface! { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to determine key in ext_get_allocated_child_storage")?; - let maybe_value = runtime_io::child_storage(&storage_key, &key); + let maybe_value = with_external_storage(move || + Ok(runtime_io::child_storage(&storage_key, &key)) + )?; if let Some(value) = maybe_value { let offset = context.allocate_memory(value.len() as u32)?; @@ -602,7 +624,9 @@ impl_wasm_host_interface! { ) -> WordSize { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to get key in ext_get_storage_into")?; - let maybe_value = runtime_io::storage(&key); + let maybe_value = with_external_storage(move || + Ok(runtime_io::storage(&key)) + )?; if let Some(value) = maybe_value { let value = &value[value_offset as usize..]; @@ -629,7 +653,9 @@ impl_wasm_host_interface! { let key = context.read_memory(key_data, key_len) .map_err(|_| "Invalid attempt to get key in ext_get_child_storage_into")?; - let maybe_value = runtime_io::child_storage(&storage_key, &key); + let maybe_value = with_external_storage(move || + Ok(runtime_io::child_storage(&storage_key, &key)) + )?; if let Some(value) = maybe_value { let value = &value[value_offset as usize..]; @@ -643,7 +669,9 @@ impl_wasm_host_interface! { } ext_storage_root(result: Pointer) { - let r = runtime_io::storage_root(); + let r = with_external_storage(move || + Ok(runtime_io::storage_root()) + )?; context.write_memory(result, r.as_ref()) .map_err(|_| "Invalid attempt to set memory in ext_storage_root")?; Ok(()) @@ -656,7 +684,9 @@ impl_wasm_host_interface! { ) -> Pointer { let storage_key = context.read_memory(storage_key_data, storage_key_len) .map_err(|_| "Invalid attempt to determine storage_key in ext_child_storage_root")?; - let value = runtime_io::child_storage_root(&storage_key); + let value = with_external_storage(move || + Ok(runtime_io::child_storage_root(&storage_key)) + )?; let offset = context.allocate_memory(value.len() as u32)?; context.write_memory(offset, &value) @@ -674,7 +704,9 @@ impl_wasm_host_interface! { let mut parent_hash = [0u8; 32]; context.read_memory_into(parent_hash_data, &mut parent_hash[..]) .map_err(|_| "Invalid attempt to get parent_hash in ext_storage_changes_root")?; - let r = runtime_io::storage_changes_root(parent_hash); + let r = with_external_storage(move || + Ok(runtime_io::storage_changes_root(parent_hash)) + )?; if let Some(r) = r { context.write_memory(result, &r[..]) @@ -1331,6 +1363,25 @@ impl_wasm_host_interface! { } } +/// Execute closure that access external storage. +/// +/// All panics that happen within closure are captured and transformed into +/// runtime error. This requires special panic handler mode to be enabled +/// during the call (see `panic_handler::AbortGuard::never_abort`). +/// If this mode isn't enabled, then all panics within externalities are +/// leading to process abort. +fn with_external_storage(f: F) -> std::result::Result + where + F: panic::UnwindSafe + FnOnce() -> Result +{ + // it is safe beause basic methods of StorageExternalities are guaranteed to touch only + // its internal state + we should discard it on error + panic::catch_unwind(move || f()) + .map_err(|_| Error::Runtime) + .and_then(|result| result) + .map_err(|err| format!("{}", err)) +} + /// Wasm rust executor for contracts. /// /// Executes the provided code in a sandboxed wasm runtime. diff --git a/core/network/src/config.rs b/core/network/src/config.rs index 46bb8aeff4ddd..7392a4aaf7b4e 100644 --- a/core/network/src/config.rs +++ b/core/network/src/config.rs @@ -205,23 +205,23 @@ pub enum ParseErr { } impl fmt::Display for ParseErr { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ParseErr::MultiaddrParse(err) => write!(f, "{}", err), - ParseErr::InvalidPeerId => write!(f, "Peer id at the end of the address is invalid"), - ParseErr::PeerIdMissing => write!(f, "Peer id is missing from the address"), - } - } + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ParseErr::MultiaddrParse(err) => write!(f, "{}", err), + ParseErr::InvalidPeerId => write!(f, "Peer id at the end of the address is invalid"), + ParseErr::PeerIdMissing => write!(f, "Peer id is missing from the address"), + } + } } impl std::error::Error for ParseErr { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - ParseErr::MultiaddrParse(err) => Some(err), - ParseErr::InvalidPeerId => None, - ParseErr::PeerIdMissing => None, - } - } + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ParseErr::MultiaddrParse(err) => Some(err), + ParseErr::InvalidPeerId => None, + ParseErr::PeerIdMissing => None, + } + } } impl From for ParseErr { diff --git a/core/network/src/test/mod.rs b/core/network/src/test/mod.rs index a025bd663a119..f6c866d76d806 100644 --- a/core/network/src/test/mod.rs +++ b/core/network/src/test/mod.rs @@ -348,7 +348,7 @@ impl> Peer { /// Test helper to compare the blockchain state of multiple (networked) /// clients. /// Potentially costly, as it creates in-memory copies of both blockchains in order - /// to compare them. If you have easier/softer checks that are sufficient, e.g. + /// to compare them. If you have easier/softer checks that are sufficient, e.g. /// by using .info(), you should probably use it instead of this. pub fn blockchain_canon_equals(&self, other: &Self) -> bool { if let (Some(mine), Some(others)) = (self.backend.clone(), other.backend.clone()) { diff --git a/core/panic-handler/src/lib.rs b/core/panic-handler/src/lib.rs index 287ff72a6aa1c..2c04700e96950 100644 --- a/core/panic-handler/src/lib.rs +++ b/core/panic-handler/src/lib.rs @@ -31,7 +31,18 @@ use std::cell::Cell; use std::thread; thread_local! { - static ABORT: Cell = Cell::new(true); + static ON_PANIC: Cell = Cell::new(OnPanic::Abort); +} + +/// Panic action. +#[derive(Debug, Clone, Copy, PartialEq)] +enum OnPanic { + /// Abort when panic occurs. + Abort, + /// Unwind when panic occurs. + Unwind, + /// Always unwind even if someone changes strategy to Abort afterwards. + NeverAbort, } /// Set the panic hook. @@ -52,10 +63,13 @@ This is a bug. Please report it at: ")} /// Set aborting flag. Returns previous value of the flag. -fn set_abort(enabled: bool) -> bool { - ABORT.with(|flag| { - let prev = flag.get(); - flag.set(enabled); +fn set_abort(on_panic: OnPanic) -> OnPanic { + ON_PANIC.with(|val| { + let prev = val.get(); + match prev { + OnPanic::Abort | OnPanic::Unwind => val.set(on_panic), + OnPanic::NeverAbort => (), + } prev }) } @@ -69,7 +83,7 @@ fn set_abort(enabled: bool) -> bool { /// > the `AbortGuard` on the stack and let it destroy itself naturally. pub struct AbortGuard { /// Value that was in `ABORT` before we created this guard. - previous_val: bool, + previous_val: OnPanic, /// Marker so that `AbortGuard` doesn't implement `Send`. _not_send: PhantomData> } @@ -79,7 +93,7 @@ impl AbortGuard { /// unwind the stack (unless another guard is created afterwards). pub fn force_unwind() -> AbortGuard { AbortGuard { - previous_val: set_abort(false), + previous_val: set_abort(OnPanic::Unwind), _not_send: PhantomData } } @@ -88,7 +102,16 @@ impl AbortGuard { /// abort the process (unless another guard is created afterwards). pub fn force_abort() -> AbortGuard { AbortGuard { - previous_val: set_abort(true), + previous_val: set_abort(OnPanic::Abort), + _not_send: PhantomData + } + } + + /// Create a new guard. While the guard is alive, panics that happen in the current thread will + /// **never** abort the process (even if `AbortGuard::force_abort()` guard will be created afterwards). + pub fn never_abort() -> AbortGuard { + AbortGuard { + previous_val: set_abort(OnPanic::NeverAbort), _not_send: PhantomData } } @@ -133,8 +156,8 @@ fn panic_hook(info: &PanicInfo, report_url: &'static str) { ); let _ = writeln!(stderr, ABOUT_PANIC!(), report_url); - ABORT.with(|flag| { - if flag.get() { + ON_PANIC.with(|val| { + if val.get() == OnPanic::Abort { ::std::process::exit(1); } }) @@ -150,4 +173,12 @@ mod tests { let _guard = AbortGuard::force_unwind(); ::std::panic::catch_unwind(|| panic!()).ok(); } + + #[test] + fn does_not_abort_after_never_abort() { + set("test"); + let _guard = AbortGuard::never_abort(); + let _guard = AbortGuard::force_abort(); + std::panic::catch_unwind(|| panic!()).ok(); + } } diff --git a/core/rpc/api/src/author/mod.rs b/core/rpc/api/src/author/mod.rs index e8314c5c5f71f..5cde56995aad9 100644 --- a/core/rpc/api/src/author/mod.rs +++ b/core/rpc/api/src/author/mod.rs @@ -84,4 +84,3 @@ pub trait AuthorApi { id: SubscriptionId ) -> Result; } - diff --git a/core/state-machine/src/changes_trie/build.rs b/core/state-machine/src/changes_trie/build.rs index 824bdd3542d58..10c38a41e2650 100644 --- a/core/state-machine/src/changes_trie/build.rs +++ b/core/state-machine/src/changes_trie/build.rs @@ -111,7 +111,7 @@ fn prepare_extrinsics_input<'a, B, H, Number>( block: block.clone(), storage_key: storage_key.clone(), }; - + let iter = prepare_extrinsics_input_inner(backend, block, changes, Some(storage_key))?; children_result.insert(child_index, iter); } @@ -120,7 +120,7 @@ fn prepare_extrinsics_input<'a, B, H, Number>( Ok((top, children_result)) } - + fn prepare_extrinsics_input_inner<'a, B, H, Number>( backend: &'a B, block: &Number, diff --git a/core/state-machine/src/changes_trie/storage.rs b/core/state-machine/src/changes_trie/storage.rs index f8c556a46c61b..a82477bdc3933 100644 --- a/core/state-machine/src/changes_trie/storage.rs +++ b/core/state-machine/src/changes_trie/storage.rs @@ -100,7 +100,7 @@ impl InMemoryStorage { for (storage_key, child_input) in children_inputs { for (block, pairs) in child_input { let root = insert_into_memory_db::(&mut mdb, pairs.into_iter().map(Into::into)); - + if let Some(root) = root { let ix = if let Some(ix) = top_inputs.iter().position(|v| v.0 == block) { ix diff --git a/core/state-machine/src/ext.rs b/core/state-machine/src/ext.rs index 129ed96ab9516..6ce5b120857b8 100644 --- a/core/state-machine/src/ext.rs +++ b/core/state-machine/src/ext.rs @@ -174,13 +174,12 @@ where } impl<'a, B, T, H, N, O> Externalities for Ext<'a, H, N, B, T, O> -where - H: Hasher, +where H: Hasher, B: 'a + Backend, T: 'a + ChangesTrieStorage, - O: 'a + offchain::Externalities, H::Out: Ord + 'static, N: crate::changes_trie::BlockNumber, + O: 'a + offchain::Externalities, { fn storage(&self, key: &[u8]) -> Option> { let _guard = panic_handler::AbortGuard::force_abort(); @@ -371,6 +370,7 @@ where fn keystore(&self) -> Option { self.keystore.clone() } + } #[cfg(test)] diff --git a/core/state-machine/src/lib.rs b/core/state-machine/src/lib.rs index 41d9b092bd53d..4008ec7c23d5f 100644 --- a/core/state-machine/src/lib.rs +++ b/core/state-machine/src/lib.rs @@ -89,13 +89,26 @@ pub enum ExecutionStrategy { NativeElseWasm, } +/// Storage backend trust level. +#[derive(Debug, Clone)] +pub enum BackendTrustLevel { + /// Panics from trusted backends are considered justified, and never caught. + Trusted, + /// Panics from untrusted backend are caught and interpreted as runtime error. + /// Untrusted backend may be missing some parts of the trie, so panics are not considered + /// fatal. + Untrusted, +} + /// Like `ExecutionStrategy` only it also stores a handler in case of consensus failure. #[derive(Clone)] pub enum ExecutionManager { /// Execute with the native equivalent if it is compatible with the given wasm module; otherwise fall back to the wasm. NativeWhenPossible, - /// Use the given wasm module. - AlwaysWasm, + /// Use the given wasm module. The backend on which code is executed code could be + /// trusted to provide all storage or not (i.e. the light client cannot be trusted to provide + /// for all storage queries since the storage entries it has come from an external node). + AlwaysWasm(BackendTrustLevel), /// Run with both the wasm and the native variant (if compatible). Call `F` in the case of any discrepency. Both(F), /// First native, then if that fails or is not possible, wasm. @@ -106,7 +119,7 @@ impl<'a, F> From<&'a ExecutionManager> for ExecutionStrategy { fn from(s: &'a ExecutionManager) -> Self { match *s { ExecutionManager::NativeWhenPossible => ExecutionStrategy::NativeWhenPossible, - ExecutionManager::AlwaysWasm => ExecutionStrategy::AlwaysWasm, + ExecutionManager::AlwaysWasm(_) => ExecutionStrategy::AlwaysWasm, ExecutionManager::NativeElseWasm => ExecutionStrategy::NativeElseWasm, ExecutionManager::Both(_) => ExecutionStrategy::Both, } @@ -119,7 +132,7 @@ impl ExecutionStrategy { self, ) -> ExecutionManager> { match self { - ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm, + ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm(BackendTrustLevel::Trusted), ExecutionStrategy::NativeWhenPossible => ExecutionManager::NativeWhenPossible, ExecutionStrategy::NativeElseWasm => ExecutionManager::NativeElseWasm, ExecutionStrategy::Both => ExecutionManager::Both(|wasm_result, native_result| { @@ -139,6 +152,16 @@ pub fn native_else_wasm() -> ExecutionManager ExecutionManager::NativeElseWasm } +/// Evaluate to ExecutionManager::AlwaysWasm with trusted backend, without having to figure out the type. +fn always_wasm() -> ExecutionManager> { + ExecutionManager::AlwaysWasm(BackendTrustLevel::Trusted) +} + +/// Evaluate ExecutionManager::AlwaysWasm with untrusted backend, without having to figure out the type. +fn always_untrusted_wasm() -> ExecutionManager> { + ExecutionManager::AlwaysWasm(BackendTrustLevel::Untrusted) +} + /// The substrate state machine. pub struct StateMachine<'a, B, H, N, T, O, Exec> { backend: B, @@ -375,7 +398,11 @@ impl<'a, B, H, N, T, O, Exec> StateMachine<'a, B, H, N, T, O, Exec> where orig_prospective, ) }, - ExecutionManager::AlwaysWasm => { + ExecutionManager::AlwaysWasm(trust_level) => { + let _abort_guard = match trust_level { + BackendTrustLevel::Trusted => None, + BackendTrustLevel::Untrusted => Some(panic_handler::AbortGuard::never_abort()), + }; let res = self.execute_aux(compute_tx, false, native_call); (res.0, res.2, res.3) }, @@ -444,7 +471,7 @@ where ); let (result, _, _) = sm.execute_using_consensus_failure_handler::<_, NeverNativeValue, fn() -> _>( - native_else_wasm(), + always_wasm(), false, None, )?; @@ -490,7 +517,7 @@ where ); sm.execute_using_consensus_failure_handler::<_, NeverNativeValue, fn() -> _>( - native_else_wasm(), + always_untrusted_wasm(), false, None, ).map(|(result, _, _)| result.into_encoded()) diff --git a/core/state-machine/src/testing.rs b/core/state-machine/src/testing.rs index 2ee2268c0ee21..160f7d2a47ccb 100644 --- a/core/state-machine/src/testing.rs +++ b/core/state-machine/src/testing.rs @@ -142,11 +142,10 @@ impl From for TestExternalit } } -impl Externalities for TestExternalities - where - H: Hasher, - N: ChangesTrieBlockNumber, - H::Out: Ord + 'static +impl Externalities for TestExternalities where + H: Hasher, + N: ChangesTrieBlockNumber, + H::Out: Ord + 'static, { fn storage(&self, key: &[u8]) -> Option> { self.overlay.storage(key).map(|x| x.map(|x| x.to_vec())).unwrap_or_else(|| diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 1fef4bba7ad06..41d7b834b05b1 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -45,7 +45,8 @@ mod tests { }; use state_machine::TestExternalities as CoreTestExternalities; use primitives::{ - Blake2Hasher, NeverNativeValue, NativeOrEncoded, map, traits::{CodeExecutor, Externalities}, + Blake2Hasher, NeverNativeValue, NativeOrEncoded, map, + traits::{CodeExecutor, Externalities}, }; use sr_primitives::{ traits::{Header as HeaderT, Hash as HashT, Convert}, ApplyOutcome, ApplyResult, diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 9d7ae5d9df85d..16a4b551cb886 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -83,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. spec_version: 155, - impl_version: 155, + impl_version: 156, apis: RUNTIME_API_VERSIONS, };