substrate: return Option in all storage related RPC methods (#510)

* substrate: return Option in all storage related RPC methods

* substrate: remove unused imports

* substrate: remove unused NoValueForKey error variant

* substrate: don't return Option on code_at since code is always defined

substrate/substrate/client/src/client.rs

@@ -214,16 +214,16 @@ impl<B, E, Block> Client<B, E, Block> where
 	}
 
 	/// Return single storage entry of contract under given address in state in a block of given hash.
-	pub fn storage(&self, id: &BlockId<Block>, key: &StorageKey) -> error::Result<StorageData> {
-		Ok(StorageData(self.state_at(id)?
+	pub fn storage(&self, id: &BlockId<Block>, key: &StorageKey) -> error::Result<Option<StorageData>> {
+		Ok(self.state_at(id)?
 			.storage(&key.0).map_err(|e| error::Error::from_state(Box::new(e)))?
-			.ok_or_else(|| error::ErrorKind::NoValueForKey(key.0.clone()))?
-			.to_vec()))
+			.map(StorageData))
 	}
 
 	/// Get the code at a given block.
 	pub fn code_at(&self, id: &BlockId<Block>) -> error::Result<Vec<u8>> {
-		self.storage(id, &StorageKey(b":code".to_vec())).map(|data| data.0)
+		Ok(self.storage(id, &StorageKey(b":code".to_vec()))?
+			.expect("None is returned if there's no value stored for the given key; ':code' key is always defined; qed").0)
 	}
 
 	/// Get the set of authorities at a given block.
@@ -580,13 +580,12 @@ impl<B, E, Block> BlockBody<Block> for Client<B, E, Block>
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use codec::Encode;
 	use keyring::Keyring;
 	use test_client::{self, TestClient};
 	use test_client::client::BlockOrigin;
 	use test_client::client::backend::Backend as TestBackend;
 	use test_client::{runtime as test_runtime, BlockBuilderExt};
-	use test_client::runtime::{Transfer, Extrinsic};
+	use test_client::runtime::Transfer;
 
 	#[test]
 	fn client_initialises_from_genesis_ok() {

substrate/substrate/client/src/error.rs

@@ -18,7 +18,6 @@
 
 use std;
 use state_machine;
-use primitives::hexdisplay::HexDisplay;
 use runtime_primitives::ApplyError;
 
 error_chain! {
@@ -53,12 +52,6 @@ error_chain! {
 			display("Blockchain: {}", e),
 		}
 
-		/// Attempt to read storage yet nothing set for that key.
-		NoValueForKey(key: Vec<u8>) {
-			description("storage doesn't contain key"),
-			display("Storage does not contain the key entry: {}", HexDisplay::from(key)),
-		}
-
 		/// Invalid state data.
 		AuthLenEmpty {
 			description("authority count state error"),

substrate/substrate/rpc/src/state/mod.rs

@@ -45,7 +45,7 @@ build_rpc_trait! {
 
 		/// Returns a storage entry at a specific block's state.
 		#[rpc(name = "state_getStorageAt")]
-		fn storage_at(&self, StorageKey, Hash) -> Result<StorageData>;
+		fn storage_at(&self, StorageKey, Hash) -> Result<Option<StorageData>>;
 
 		/// Call a contract at a block's state.
 		#[rpc(name = "state_callAt")]
@@ -53,23 +53,23 @@ build_rpc_trait! {
 
 		/// Returns the hash of a storage entry at a block's state.
 		#[rpc(name = "state_getStorageHashAt")]
-		fn storage_hash_at(&self, StorageKey, Hash) -> Result<Hash>;
+		fn storage_hash_at(&self, StorageKey, Hash) -> Result<Option<Hash>>;
 
 		/// Returns the size of a storage entry at a block's state.
 		#[rpc(name = "state_getStorageSizeAt")]
-		fn storage_size_at(&self, StorageKey, Hash) -> Result<u64>;
+		fn storage_size_at(&self, StorageKey, Hash) -> Result<Option<u64>>;
 
 		/// Returns the hash of a storage entry at the best block.
 		#[rpc(name = "state_getStorageHash")]
-		fn storage_hash(&self, StorageKey) -> Result<Hash>;
+		fn storage_hash(&self, StorageKey) -> Result<Option<Hash>>;
 
 		/// Returns the size of a storage entry at the best block.
 		#[rpc(name = "state_getStorageSize")]
-		fn storage_size(&self, StorageKey) -> Result<u64>;
+		fn storage_size(&self, StorageKey) -> Result<Option<u64>>;
 
 		/// Returns a storage entry at the best block.
 		#[rpc(name = "state_getStorage")]
-		fn storage(&self, StorageKey) -> Result<StorageData>;
+		fn storage(&self, StorageKey) -> Result<Option<StorageData>>;
 
 		/// Call a contract at the best block.
 		#[rpc(name = "state_call")]
@@ -112,7 +112,7 @@ impl<B, E, Block> StateApi<Block::Hash> for State<B, E, Block> where
 {
 	type Metadata = ::metadata::Metadata;
 
-	fn storage_at(&self, key: StorageKey, block: Block::Hash) -> Result<StorageData> {
+	fn storage_at(&self, key: StorageKey, block: Block::Hash) -> Result<Option<StorageData>> {
 		trace!(target: "rpc", "Querying storage at {:?} for key {}", block, HexDisplay::from(&key.0));
 		Ok(self.client.storage(&BlockId::Hash(block), &key)?)
 	}
@@ -122,24 +122,24 @@ impl<B, E, Block> StateApi<Block::Hash> for State<B, E, Block> where
 		Ok(self.client.executor().call(&BlockId::Hash(block), &method, &data)?.return_data)
 	}
 
-	fn storage_hash_at(&self, key: StorageKey, block: Block::Hash) -> Result<Block::Hash> {
+	fn storage_hash_at(&self, key: StorageKey, block: Block::Hash) -> Result<Option<Block::Hash>> {
 		use runtime_primitives::traits::{Hash, Header as HeaderT};
-		self.storage_at(key, block).map(|x| <Block::Header as HeaderT>::Hashing::hash(&x.0))
+		Ok(self.storage_at(key, block)?.map(|x| <Block::Header as HeaderT>::Hashing::hash(&x.0)))
 	}
 
-	fn storage_size_at(&self, key: StorageKey, block: Block::Hash) -> Result<u64> {
-		self.storage_at(key, block).map(|x| x.0.len() as u64)
+	fn storage_size_at(&self, key: StorageKey, block: Block::Hash) -> Result<Option<u64>> {
+		Ok(self.storage_at(key, block)?.map(|x| x.0.len() as u64))
 	}
 
-	fn storage_hash(&self, key: StorageKey) -> Result<Block::Hash> {
+	fn storage_hash(&self, key: StorageKey) -> Result<Option<Block::Hash>> {
 		self.storage_hash_at(key, self.client.info()?.chain.best_hash)
 	}
 
-	fn storage_size(&self, key: StorageKey) -> Result<u64> {
+	fn storage_size(&self, key: StorageKey) -> Result<Option<u64>> {
 		self.storage_size_at(key, self.client.info()?.chain.best_hash)
 	}
 
-	fn storage(&self, key: StorageKey) -> Result<StorageData> {
+	fn storage(&self, key: StorageKey) -> Result<Option<StorageData>> {
 		self.storage_at(key, self.client.info()?.chain.best_hash)
 
 	}
@@ -169,7 +169,7 @@ impl<B, E, Block> StateApi<Block::Hash> for State<B, E, Block> where
 				let changes = keys
 					.into_iter()
 					.map(|key| self.storage(key.clone())
-						.map(|val| (key.clone(), Some(val)))
+						.map(|val| (key.clone(), val))
 						.unwrap_or_else(|_| (key, None))
 					)
 					.collect();

substrate/substrate/rpc/src/state/tests.rs

@@ -32,7 +32,7 @@ fn should_return_storage() {
 
 	assert_matches!(
 		client.storage_at(StorageKey(vec![10]), genesis_hash),
-		Err(Error(ErrorKind::Client(client::error::ErrorKind::NoValueForKey(ref k)), _)) if *k == vec![10]
+		Ok(None)
 	)
 }