From f273cacb5ebab88fba69bbb69e7cdeaf03954aa4 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 3 Sep 2018 12:05:01 +0300 Subject: [PATCH 1/2] fixed Ext calls from outside of runtime --- substrate/state-machine/src/ext.rs | 5 +- substrate/state-machine/src/lib.rs | 53 +++++++++++++++---- .../state-machine/src/proving_backend.rs | 4 +- substrate/state-machine/src/trie_backend.rs | 26 ++++----- 4 files changed, 62 insertions(+), 26 deletions(-) diff --git a/substrate/state-machine/src/ext.rs b/substrate/state-machine/src/ext.rs index 8504f42b59392..af3631c231880 100644 --- a/substrate/state-machine/src/ext.rs +++ b/substrate/state-machine/src/ext.rs @@ -126,8 +126,9 @@ where H::Out: Ord + Encodable { fn storage(&self, key: &[u8]) -> Option> { - self.overlay.storage(key).map(|x| x.map(|x| x.to_vec())).unwrap_or_else(|| - self.backend.storage(key).expect("Externalities not allowed to fail within runtime")) + use {try_read_overlay_value}; + try_read_overlay_value(self.overlay, self.backend, key) + .expect("Externalities not allowed to fail within runtime") } fn exists_storage(&self, key: &[u8]) -> bool { diff --git a/substrate/state-machine/src/lib.rs b/substrate/state-machine/src/lib.rs index 4b4bc4fc4c687..996332ec7993a 100644 --- a/substrate/state-machine/src/lib.rs +++ b/substrate/state-machine/src/lib.rs @@ -153,6 +153,8 @@ impl Error for ExecutionError {} /// and as a transition away from the pre-existing framework. #[derive(Debug, Eq, PartialEq)] pub enum ExecutionError { + /// Backend error. + Backend(String), /// The entry `:code` doesn't exist in storage so there's no way we can execute anything. CodeEntryDoesNotExist, /// Backend is incompatible with execution proof generation process. @@ -324,11 +326,13 @@ where let strategy: ExecutionStrategy = (&manager).into(); // make a copy. - let code = ext::Ext::new(overlay, backend).storage(b":code") + let code = try_read_overlay_value(overlay, backend, b":code") + .map_err(|err| Box::new(ExecutionError::Backend(format!("{}", err))) as Box)? .ok_or_else(|| Box::new(ExecutionError::CodeEntryDoesNotExist) as Box)? .to_vec(); - let heap_pages = ext::Ext::new(overlay, backend).storage(b":heappages") + let heap_pages = try_read_overlay_value(overlay, backend, b":heappages") + .map_err(|err| Box::new(ExecutionError::Backend(format!("{}", err))) as Box)? .and_then(|v| u64::decode(&mut &v[..])).unwrap_or(8) as usize; let result = { @@ -435,10 +439,10 @@ pub fn execution_proof_check( call_data: &[u8], ) -> Result<(Vec, memorydb::MemoryDB), Box> where -H: Hasher, -C: NodeCodec, -Exec: CodeExecutor, -H::Out: Ord + Encodable + HeapSizeOf, + H: Hasher, + C: NodeCodec, + Exec: CodeExecutor, + H::Out: Ord + Encodable + HeapSizeOf, { let backend = proving_backend::create_proof_check_backend::(root.into(), proof)?; execute::(&backend, overlay, exec, method, call_data, ExecutionStrategy::NativeWhenPossible) @@ -477,6 +481,19 @@ where backend.storage(key).map_err(|e| Box::new(e) as Box) } +/// Reads storage value from overlay or from the backend. +pub(crate) fn try_read_overlay_value(overlay: &OverlayedChanges, backend: &B, key: &[u8]) -> Result>, B::Error> + where + H: Hasher, + C: NodeCodec, + B: Backend, +{ + match overlay.storage(key).map(|x| x.map(|x| x.to_vec())) { + Some(value) => Ok(value), + None => backend.storage(key), + } +} + #[cfg(test)] mod tests { use super::*; @@ -578,7 +595,7 @@ mod tests { #[test] fn execute_works() { assert_eq!(execute( - &trie_backend::tests::test_trie(), + &trie_backend::tests::test_trie(true), &mut Default::default(), &DummyCodeExecutor { native_available: true, @@ -591,11 +608,27 @@ mod tests { ).unwrap().0, vec![66]); } + #[test] + fn execute_should_not_panic_when_there_is_no_code_entry() { + assert!(execute( + &trie_backend::tests::test_trie(false), + &mut Default::default(), + &DummyCodeExecutor { + native_available: true, + native_succeeds: true, + fallback_succeeds: true, + }, + "test", + &[], + ExecutionStrategy::NativeWhenPossible + ).is_err()); + } + #[test] fn dual_execution_strategy_detects_consensus_failure() { let mut consensus_failed = false; assert!(execute_using_consensus_failure_handler( - &trie_backend::tests::test_trie(), + &trie_backend::tests::test_trie(true), &mut Default::default(), &DummyCodeExecutor { native_available: true, @@ -622,7 +655,7 @@ mod tests { }; // fetch execution proof from 'remote' full node - let remote_backend = trie_backend::tests::test_trie(); + let remote_backend = trie_backend::tests::test_trie(true); let remote_root = remote_backend.storage_root(::std::iter::empty()).0; let (remote_result, remote_proof, _) = prove_execution(remote_backend, &mut Default::default(), &executor, "test", &[]).unwrap(); @@ -679,7 +712,7 @@ mod tests { #[test] fn prove_read_and_proof_check_works() { // fetch read proof from 'remote' full node - let remote_backend = trie_backend::tests::test_trie(); + let remote_backend = trie_backend::tests::test_trie(true); let remote_root = remote_backend.storage_root(::std::iter::empty()).0; let remote_proof = prove_read(remote_backend, b"value2").unwrap().1; // check proof locally diff --git a/substrate/state-machine/src/proving_backend.rs b/substrate/state-machine/src/proving_backend.rs index db8d142452eba..d84b5e1dcf6b9 100644 --- a/substrate/state-machine/src/proving_backend.rs +++ b/substrate/state-machine/src/proving_backend.rs @@ -126,7 +126,7 @@ mod tests { use primitives::{KeccakHasher, RlpCodec}; fn test_proving() -> ProvingBackend { - ProvingBackend::new(test_trie()) + ProvingBackend::new(test_trie(true)) } #[test] @@ -148,7 +148,7 @@ mod tests { #[test] fn passes_throgh_backend_calls() { - let trie_backend = test_trie(); + let trie_backend = test_trie(true); let proving_backend = test_proving(); assert_eq!(trie_backend.storage(b"key").unwrap(), proving_backend.storage(b"key").unwrap()); assert_eq!(trie_backend.pairs(), proving_backend.pairs()); diff --git a/substrate/state-machine/src/trie_backend.rs b/substrate/state-machine/src/trie_backend.rs index 066f74b77d06a..29bef2c7cdedc 100644 --- a/substrate/state-machine/src/trie_backend.rs +++ b/substrate/state-machine/src/trie_backend.rs @@ -282,7 +282,7 @@ pub mod tests { use std::collections::HashSet; use primitives::{KeccakHasher, RlpCodec, H256}; - fn test_db() -> (MemoryDB, H256) { + fn test_db(has_code: bool) -> (MemoryDB, H256) { let mut root = H256::default(); let mut mdb = MemoryDB::::new(); { @@ -290,7 +290,9 @@ pub mod tests { trie.insert(b"key", b"value").expect("insert failed"); trie.insert(b"value1", &[42]).expect("insert failed"); trie.insert(b"value2", &[24]).expect("insert failed"); - trie.insert(b":code", b"return 42").expect("insert failed"); + if has_code { + trie.insert(b":code", b"return 42").expect("insert failed"); + } for i in 128u8..255u8 { trie.insert(&[i], &[i]).unwrap(); } @@ -298,24 +300,24 @@ pub mod tests { (mdb, root) } - pub(crate) fn test_trie() -> TrieBackend { - let (mdb, root) = test_db(); + pub(crate) fn test_trie(has_code: bool) -> TrieBackend { + let (mdb, root) = test_db(has_code); TrieBackend::with_memorydb(mdb, root) } #[test] fn read_from_storage_returns_some() { - assert_eq!(test_trie().storage(b"key").unwrap(), Some(b"value".to_vec())); + assert_eq!(test_trie(true).storage(b"key").unwrap(), Some(b"value".to_vec())); } #[test] fn read_from_storage_returns_none() { - assert_eq!(test_trie().storage(b"non-existing-key").unwrap(), None); + assert_eq!(test_trie(true).storage(b"non-existing-key").unwrap(), None); } #[test] fn pairs_are_not_empty_on_non_empty_storage() { - assert!(!test_trie().pairs().is_empty()); + assert!(!test_trie(true).pairs().is_empty()); } #[test] @@ -329,24 +331,24 @@ pub mod tests { #[test] fn storage_root_is_non_default() { - assert!(test_trie().storage_root(::std::iter::empty()).0 != H256([0; 32])); + assert!(test_trie(true).storage_root(::std::iter::empty()).0 != H256([0; 32])); } #[test] fn storage_root_transaction_is_empty() { - assert!(test_trie().storage_root(::std::iter::empty()).1.drain().is_empty()); + assert!(test_trie(true).storage_root(::std::iter::empty()).1.drain().is_empty()); } #[test] fn storage_root_transaction_is_non_empty() { - let (new_root, mut tx) = test_trie().storage_root(vec![(b"new-key".to_vec(), Some(b"new-value".to_vec()))]); + let (new_root, mut tx) = test_trie(true).storage_root(vec![(b"new-key".to_vec(), Some(b"new-value".to_vec()))]); assert!(!tx.drain().is_empty()); - assert!(new_root != test_trie().storage_root(::std::iter::empty()).0); + assert!(new_root != test_trie(true).storage_root(::std::iter::empty()).0); } #[test] fn prefix_walking_works() { - let trie = test_trie(); + let trie = test_trie(true); let mut seen = HashSet::new(); trie.for_keys_with_prefix(b"value", |key| { From fc07dc0a9c8c1d94f3336c816b32f3b429159b7b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 3 Sep 2018 13:00:47 +0300 Subject: [PATCH 2/2] remove brackets --- substrate/state-machine/src/ext.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/state-machine/src/ext.rs b/substrate/state-machine/src/ext.rs index af3631c231880..8a20398f82731 100644 --- a/substrate/state-machine/src/ext.rs +++ b/substrate/state-machine/src/ext.rs @@ -126,7 +126,7 @@ where H::Out: Ord + Encodable { fn storage(&self, key: &[u8]) -> Option> { - use {try_read_overlay_value}; + use try_read_overlay_value; try_read_overlay_value(self.overlay, self.backend, key) .expect("Externalities not allowed to fail within runtime") }