From 11ec4080619e7538395f9ae075e961c840169ddf Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 13 Feb 2018 19:42:06 +0100 Subject: [PATCH] Native runtime-io now supports empty storage items. --- substrate/substrate/runtime-io/with_std.rs | 55 ++++++++----------- substrate/substrate/state-machine/src/ext.rs | 10 ++-- substrate/substrate/state-machine/src/lib.rs | 13 +++-- .../substrate/state-machine/src/testing.rs | 6 +- 4 files changed, 40 insertions(+), 44 deletions(-) diff --git a/substrate/substrate/runtime-io/with_std.rs b/substrate/substrate/runtime-io/with_std.rs index 98864baa63..3c75935d64 100644 --- a/substrate/substrate/runtime-io/with_std.rs +++ b/substrate/substrate/runtime-io/with_std.rs @@ -27,36 +27,29 @@ pub extern crate substrate_codec as codec; // re-export hashing functions. pub use primitives::{blake2_256, twox_128, twox_256}; -pub use substrate_state_machine::{Externalities, ExternalitiesError, TestExternalities}; +pub use substrate_state_machine::{Externalities, TestExternalities}; use primitives::hexdisplay::HexDisplay; // TODO: use the real error, not NoError. -environmental!(ext : trait Externalities); +environmental!(ext: trait Externalities); /// Get `key` from storage and return a `Vec`, empty if there's a problem. -pub fn storage(key: &[u8]) -> Vec { - ext::with(|ext| ext.storage(key).ok().map(|s| s.to_vec())) +pub fn storage(key: &[u8]) -> Option> { + ext::with(|ext| ext.storage(key).map(|s| s.to_vec())) .expect("read_storage cannot be called outside of an Externalities-provided environment.") - .unwrap_or_else(Vec::new) } /// Get `key` from storage, placing the value into `value_out` (as much as possible) and return -/// the number of bytes that the key in storage was beyond the offset. -pub fn read_storage(key: &[u8], value_out: &mut [u8], value_offset: usize) -> usize { - ext::with(|ext| { - if let Ok(value) = ext.storage(key) { - let value = &value[value_offset..]; - let written = ::std::cmp::min(value.len(), value_out.len()); - value_out[0..written].copy_from_slice(&value[0..written]); - value.len() - } else { - // no-entry is treated as an empty vector of bytes. - // TODO: consider allowing empty-vector to exist separately to no-entry (i.e. return - // Option) - 0 - } - }).expect("read_storage cannot be called outside of an Externalities-provided environment.") +/// the number of bytes that the key in storage was beyond the offset or None if the storage entry +/// doesn't exist at all. +pub fn read_storage(key: &[u8], value_out: &mut [u8], value_offset: usize) -> Option { + ext::with(|ext| ext.storage(key).map(|value| { + let value = &value[value_offset..]; + let written = ::std::cmp::min(value.len(), value_out.len()); + value_out[0..written].copy_from_slice(&value[0..written]); + value.len() + })).expect("read_storage cannot be called outside of an Externalities-provided environment.") } /// Set the storage to some particular key. @@ -164,37 +157,37 @@ mod std_tests { #[test] fn storage_works() { - let mut t = TestExternalities { storage: map![], }; + let mut t = TestExternalities::new(); assert!(with_externalities(&mut t, || { - assert_eq!(storage(b"hello"), b"".to_vec()); + assert_eq!(storage(b"hello"), None); set_storage(b"hello", b"world"); - assert_eq!(storage(b"hello"), b"world".to_vec()); - assert_eq!(storage(b"foo"), b"".to_vec()); + assert_eq!(storage(b"hello"), Some(b"world".to_vec())); + assert_eq!(storage(b"foo"), None); set_storage(b"foo", &[1, 2, 3][..]); true })); - t.storage = map![b"foo".to_vec() => b"bar".to_vec()]; + t = map![b"foo".to_vec() => b"bar".to_vec()]; assert!(!with_externalities(&mut t, || { - assert_eq!(storage(b"hello"), b"".to_vec()); - assert_eq!(storage(b"foo"), b"bar".to_vec()); + assert_eq!(storage(b"hello"), None); + assert_eq!(storage(b"foo"), Some(b"bar".to_vec())); false })); } #[test] fn read_storage_works() { - let mut t = TestExternalities { storage: map![ + let mut t: TestExternalities = map![ b":test".to_vec() => b"\x0b\0\0\0Hello world".to_vec() - ], }; + ]; with_externalities(&mut t, || { let mut v = [0u8; 4]; - assert!(read_storage(b":test", &mut v[..], 0) >= 4); + assert!(read_storage(b":test", &mut v[..], 0).unwrap() >= 4); assert_eq!(v, [11u8, 0, 0, 0]); let mut w = [0u8; 11]; - assert!(read_storage(b":test", &mut w[..], 4) >= 11); + assert!(read_storage(b":test", &mut w[..], 4).unwrap() >= 11); assert_eq!(&w, b"Hello world"); }); } diff --git a/substrate/substrate/state-machine/src/ext.rs b/substrate/substrate/state-machine/src/ext.rs index fc04d18f5b..ac047a31c3 100644 --- a/substrate/substrate/state-machine/src/ext.rs +++ b/substrate/substrate/state-machine/src/ext.rs @@ -20,7 +20,7 @@ use std::{error, fmt}; use std::collections::HashMap; use triehash::trie_root; use backend::Backend; -use {Externalities, ExternalitiesError, OverlayedChanges}; +use {Externalities, OverlayedChanges}; /// Errors that can occur when interacting with the externalities. #[derive(Debug, Copy, Clone)] @@ -76,11 +76,9 @@ impl<'a, B: 'a + Backend> Ext<'a, B> { impl<'a, B: 'a> Externalities for Ext<'a, B> where B: Backend { - fn storage(&self, key: &[u8]) -> Result, ExternalitiesError> { - match self.overlay.storage(key) { - Some(x) => Ok(x), - None => self.backend.storage(key).map_err(|_| ExternalitiesError), - } + fn storage(&self, key: &[u8]) -> Option<&[u8]> { + self.overlay.storage(key).unwrap_or_else(|| + self.backend.storage(key).expect("Externalities not allowed to fail within runtime")) } fn place_storage(&mut self, key: Vec, value: Option>) { diff --git a/substrate/substrate/state-machine/src/lib.rs b/substrate/substrate/state-machine/src/lib.rs index 3bbe7632a8..363f293830 100644 --- a/substrate/substrate/state-machine/src/lib.rs +++ b/substrate/substrate/state-machine/src/lib.rs @@ -99,16 +99,19 @@ impl Error for E where E: 'static + fmt::Debug + fmt::Display + Send {} /// would not be executed unless externalities were available. This is included for completeness, /// and as a transition away from the pre-existing framework. #[derive(Debug, Eq, PartialEq)] -pub struct ExternalitiesError; +pub enum ExecutionError { + /// The entry `:code` doesn't exist in storage so there's no way we can execute anything. + CodeEntryDoesNotExist +} -impl fmt::Display for ExternalitiesError { +impl fmt::Display for ExecutionError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "Externalities Error") } } /// Externalities: pinned to specific active address. pub trait Externalities { /// Read storage of current contract being called. - fn storage(&self, key: &[u8]) -> Result, ExternalitiesError>; + fn storage(&self, key: &[u8]) -> Option<&[u8]>; /// Set storage entry `key` of current contract being called (effective immediately). fn set_storage(&mut self, key: Vec, value: Vec) { @@ -166,7 +169,9 @@ pub fn execute( overlay: &mut *overlay }; // make a copy. - let code = externalities.storage(b":code").map_err(|e| Box::new(e) as Box)?.unwrap_or(&[]).to_vec(); + let code = externalities.storage(b":code") + .ok_or(Box::new(ExecutionError::CodeEntryDoesNotExist) as Box)? + .to_vec(); exec.call( &mut externalities, diff --git a/substrate/substrate/state-machine/src/testing.rs b/substrate/substrate/state-machine/src/testing.rs index 28661579ec..6d991a9aca 100644 --- a/substrate/substrate/state-machine/src/testing.rs +++ b/substrate/substrate/state-machine/src/testing.rs @@ -17,15 +17,15 @@ //! Test implementation for Externalities. use std::collections::HashMap; -use super::{Externalities, ExternalitiesError}; +use super::Externalities; use triehash::trie_root; /// Simple HashMap based Externalities impl. pub type TestExternalities = HashMap, Vec>; impl Externalities for TestExternalities { - fn storage(&self, key: &[u8]) -> Result, ExternalitiesError> { - Ok(self.get(key).map(AsRef::as_ref)) + fn storage(&self, key: &[u8]) -> Option<&[u8]> { + self.get(key).map(AsRef::as_ref) } fn place_storage(&mut self, key: Vec, maybe_value: Option>) {