From 72f3d9a9eb82a6df6f17f469fbfea05b63be6fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 9 Aug 2018 15:32:07 +0200 Subject: [PATCH 1/2] Query storage changes from-to block. --- substrate/rpc/src/state/error.rs | 6 +++ substrate/rpc/src/state/mod.rs | 58 ++++++++++++++++++++- substrate/runtime/primitives/src/generic.rs | 44 ++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/substrate/rpc/src/state/error.rs b/substrate/rpc/src/state/error.rs index 24adeb29d386a..acd55e3897219 100644 --- a/substrate/rpc/src/state/error.rs +++ b/substrate/rpc/src/state/error.rs @@ -25,6 +25,12 @@ error_chain! { } errors { + /// Provided block range couldn't be resolved to a list of blocks. + InvalidBlockRange(from: Option, to: Option) { + description("Invalid block range"), + display("Cannot resolve a block range ['{:?}' ... '{:?}]. \ + Perhaps requested blocks are not in the canonical chain?", from, to), + } /// Not implemented yet Unimplemented { description("not implemented yet"), diff --git a/substrate/rpc/src/state/mod.rs b/substrate/rpc/src/state/mod.rs index 25f985c206cdc..3875ca7d9fa2f 100644 --- a/substrate/rpc/src/state/mod.rs +++ b/substrate/rpc/src/state/mod.rs @@ -16,7 +16,10 @@ //! Polkadot state API. -use std::sync::Arc; +use std::{ + collections::HashMap, + sync::Arc, +}; use client::{self, Client, CallExecutor, BlockchainEvents}; use jsonrpc_macros::Trailing; @@ -27,6 +30,7 @@ use primitives::storage::{StorageKey, StorageData, StorageChangeSet}; use rpc::Result as RpcResult; use rpc::futures::{stream, Future, Sink, Stream}; use runtime_primitives::generic::BlockId; +use runtime_primitives::generic::RangeIterator; use runtime_primitives::traits::Block as BlockT; use tokio::runtime::TaskExecutor; @@ -59,6 +63,13 @@ build_rpc_trait! { #[rpc(name = "state_getStorageSize", alias = ["state_getStorageSizeAt", ])] fn storage_size(&self, StorageKey, Trailing) -> Result>; + /// Query historical storage entries (by key) starting from a block given as the second parameter. + /// + /// NOTE This first returned result contains the initial state of storage for all keys. + /// Subsequent values in the vector represent changes to the previous state (diffs). + #[rpc(name = "state_queryStorage")] + fn query_storage(&self, Vec, Hash, Trailing) -> Result>>; + #[pubsub(name = "state_storage")] { /// New storage subscription #[rpc(name = "state_subscribeStorage")] @@ -130,6 +141,51 @@ impl StateApi for State where Ok(self.storage(key, block)?.map(|x| x.0.len() as u64)) } + fn query_storage(&self, keys: Vec, from: Block::Hash, to: Trailing) -> Result>> { + let to = self.unwrap_or_best(to)?; + + let from = self.client.block_number_from_id(&BlockId::Hash(from))?; + let to = self.client.block_number_from_id(&BlockId::Hash(to))?; + + match (from, to) { + (Some(from), Some(to)) if from <= to => { + // fetch all blocks and compute storage diffs for all keys + let mut result = Vec::new(); + let mut last_state: HashMap<_, Option<_>> = Default::default(); + for block in RangeIterator::new(from, to) { + let mut changes = vec![]; + + for key in &keys { + let (has_changed, data) = { + let curr_data = self.client.storage(&BlockId::number(block), key)?; + let prev_data = last_state.get(key).and_then(|x| x.as_ref()); + + (curr_data.as_ref() != prev_data, curr_data) + }; + + if has_changed { + changes.push((key.clone(), data.clone())); + } + + last_state.insert(key.clone(), data); + } + + result.push(StorageChangeSet { + block: self.client + .block_hash_from_id(&BlockId::number(block))? + .expect("`from` and `to` is in the chain; all blocks in between are in the chain; qed"), + changes, + }); + } + Ok(result) + }, + (from, to) => Err(error::ErrorKind::InvalidBlockRange( + from.map(|x| format!("{}", x)), + to.map(|x| format!("{}", x)), + ).into()), + } + } + fn subscribe_storage( &self, _meta: Self::Metadata, diff --git a/substrate/runtime/primitives/src/generic.rs b/substrate/runtime/primitives/src/generic.rs index f1a9448c8fafc..149f24e6874e6 100644 --- a/substrate/runtime/primitives/src/generic.rs +++ b/substrate/runtime/primitives/src/generic.rs @@ -446,6 +446,36 @@ pub struct SignedBlock { pub justification: Justification, } +/// Range iterator for types implementing `SimpleArithmetic`. +/// +/// It's a half-open range (includes lower bound, excludes higher bound). +/// A workaround for lack of `Step` trait stabilisation. +/// See: https://github.com/rust-lang/rust/issues/42168 +pub struct RangeIterator(pub T, pub T); + +impl RangeIterator { + /// Creates new range iterator. + /// + /// NOTE if `from >= to` the iterator will not panic, + /// but rather will just be empty. + pub fn new(from: T, to: T) -> Self { + RangeIterator(from, to) + } +} + +impl Iterator for RangeIterator { + type Item = T; + + fn next(&mut self) -> Option { + if self.0 >= self.1 { + return None; + } + + self.0 = self.0 + T::one(); + self.0.into() + } +} + #[cfg(test)] mod tests { use codec::{Decode, Encode}; @@ -500,4 +530,18 @@ mod tests { assert_eq!(block, decoded); } } + + #[test] + fn range_iterator() { + let mut it1 = RangeIterator::new(0, 3); + let mut it2 = RangeIterator::new(3, 3); + let mut it3 = RangeIterator::new(4, 3); + + assert!(it1.next(), Some(0)); + assert!(it1.next(), Some(1)); + assert!(it1.next(), Some(2)); + assert!(it1.next(), None); + assert!(it2.next(), None); + assert!(it3.next(), None); + } } From 818ef77600c656106dff89d5c6faac958523f5ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 10 Aug 2018 13:33:08 +0200 Subject: [PATCH 2/2] Rewrite to use hashes instead of numbers. Add tests. --- substrate/primitives/src/storage.rs | 2 +- substrate/rpc/src/state/error.rs | 5 +- substrate/rpc/src/state/mod.rs | 65 +++++++++++++++------ substrate/rpc/src/state/tests.rs | 65 ++++++++++++++++++++- substrate/runtime/primitives/src/generic.rs | 44 -------------- 5 files changed, 115 insertions(+), 66 deletions(-) diff --git a/substrate/primitives/src/storage.rs b/substrate/primitives/src/storage.rs index 25bb11fb6e549..a3330571fcec6 100644 --- a/substrate/primitives/src/storage.rs +++ b/substrate/primitives/src/storage.rs @@ -31,7 +31,7 @@ pub struct StorageKey(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec< pub struct StorageData(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec); /// Storage change set -#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug, PartialEq, Eq))] pub struct StorageChangeSet { /// Block hash pub block: Hash, diff --git a/substrate/rpc/src/state/error.rs b/substrate/rpc/src/state/error.rs index acd55e3897219..587937ea607ab 100644 --- a/substrate/rpc/src/state/error.rs +++ b/substrate/rpc/src/state/error.rs @@ -26,10 +26,9 @@ error_chain! { errors { /// Provided block range couldn't be resolved to a list of blocks. - InvalidBlockRange(from: Option, to: Option) { + InvalidBlockRange(from: String, to: String, details: String) { description("Invalid block range"), - display("Cannot resolve a block range ['{:?}' ... '{:?}]. \ - Perhaps requested blocks are not in the canonical chain?", from, to), + display("Cannot resolve a block range ['{:?}' ... '{:?}]. {}", from, to, details), } /// Not implemented yet Unimplemented { diff --git a/substrate/rpc/src/state/mod.rs b/substrate/rpc/src/state/mod.rs index 3875ca7d9fa2f..0ae462452fe1c 100644 --- a/substrate/rpc/src/state/mod.rs +++ b/substrate/rpc/src/state/mod.rs @@ -30,8 +30,7 @@ use primitives::storage::{StorageKey, StorageData, StorageChangeSet}; use rpc::Result as RpcResult; use rpc::futures::{stream, Future, Sink, Stream}; use runtime_primitives::generic::BlockId; -use runtime_primitives::generic::RangeIterator; -use runtime_primitives::traits::Block as BlockT; +use runtime_primitives::traits::{Block as BlockT, Header}; use tokio::runtime::TaskExecutor; use subscriptions::Subscriptions; @@ -144,20 +143,48 @@ impl StateApi for State where fn query_storage(&self, keys: Vec, from: Block::Hash, to: Trailing) -> Result>> { let to = self.unwrap_or_best(to)?; - let from = self.client.block_number_from_id(&BlockId::Hash(from))?; - let to = self.client.block_number_from_id(&BlockId::Hash(to))?; - - match (from, to) { - (Some(from), Some(to)) if from <= to => { - // fetch all blocks and compute storage diffs for all keys + let from_hdr = self.client.header(&BlockId::hash(from))?; + let to_hdr = self.client.header(&BlockId::hash(to))?; + + match (from_hdr, to_hdr) { + (Some(ref from), Some(ref to)) if from.number() <= to.number() => { + let from = from.clone(); + let to = to.clone(); + // check if we can get from `to` to `from` by going through parent_hashes. + let blocks = { + let mut blocks = vec![to.hash()]; + let mut last = to.clone(); + while last.number() > from.number() { + if let Some(hdr) = self.client.header(&BlockId::hash(*last.parent_hash()))? { + blocks.push(hdr.hash()); + last = hdr; + } else { + bail!(invalid_block_range( + Some(from), + Some(to), + format!("Parent of {} ({}) not found", last.number(), last.hash()), + )) + } + } + if last.hash() != from.hash() { + bail!(invalid_block_range( + Some(from), + Some(to), + format!("Expected to reach `from`, got {} ({})", last.number(), last.hash()), + )) + } + blocks.reverse(); + blocks + }; let mut result = Vec::new(); let mut last_state: HashMap<_, Option<_>> = Default::default(); - for block in RangeIterator::new(from, to) { + for block in blocks { let mut changes = vec![]; + let id = BlockId::hash(block.clone()); for key in &keys { let (has_changed, data) = { - let curr_data = self.client.storage(&BlockId::number(block), key)?; + let curr_data = self.client.storage(&id, key)?; let prev_data = last_state.get(key).and_then(|x| x.as_ref()); (curr_data.as_ref() != prev_data, curr_data) @@ -171,18 +198,13 @@ impl StateApi for State where } result.push(StorageChangeSet { - block: self.client - .block_hash_from_id(&BlockId::number(block))? - .expect("`from` and `to` is in the chain; all blocks in between are in the chain; qed"), + block, changes, }); } Ok(result) }, - (from, to) => Err(error::ErrorKind::InvalidBlockRange( - from.map(|x| format!("{}", x)), - to.map(|x| format!("{}", x)), - ).into()), + (from, to) => bail!(invalid_block_range(from, to, "Invalid range or unknown block".into())), } } @@ -235,3 +257,12 @@ impl StateApi for State where Ok(self.subscriptions.cancel(id)) } } + +fn invalid_block_range(from: Option, to: Option, reason: String) -> error::ErrorKind { + let to_string = |x: Option| match x { + None => "unknown hash".into(), + Some(h) => format!("{} ({})", h.number(), h.hash()), + }; + + error::ErrorKind::InvalidBlockRange(to_string(from), to_string(to), reason) +} diff --git a/substrate/rpc/src/state/tests.rs b/substrate/rpc/src/state/tests.rs index 1ab9d1896a1d9..bad934b8405a3 100644 --- a/substrate/rpc/src/state/tests.rs +++ b/substrate/rpc/src/state/tests.rs @@ -22,7 +22,6 @@ use jsonrpc_macros::pubsub; use rustc_hex::FromHex; use test_client::{self, runtime, keyring::Keyring, TestClient, BlockBuilderExt}; - #[test] fn should_return_storage() { let core = ::tokio::runtime::Runtime::new().unwrap(); @@ -121,3 +120,67 @@ fn should_send_initial_storage_changes_and_notifications() { // no more notifications on this channel assert_eq!(core.block_on(next.into_future()).unwrap().0, None); } + +#[test] +fn should_query_storage() { + let core = ::tokio::runtime::Runtime::new().unwrap(); + let client = Arc::new(test_client::new()); + let api = State::new(client.clone(), core.executor()); + + let add_block = |nonce| { + let mut builder = client.new_block().unwrap(); + builder.push_transfer(runtime::Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 42, + nonce, + }).unwrap(); + let block = builder.bake().unwrap(); + let hash = block.header.hash(); + client.justify_and_import(BlockOrigin::Own, block).unwrap(); + hash + }; + let block1_hash = add_block(0); + let block2_hash = add_block(1); + let genesis_hash = client.genesis_hash(); + + + let mut expected = vec![ + StorageChangeSet { + block: genesis_hash, + changes: vec![ + (StorageKey("a52da2b7c269da1366b3ed1cdb7299ce".from_hex().unwrap()), Some(StorageData(vec![232, 3, 0, 0, 0, 0, 0, 0]))), + ], + }, + StorageChangeSet { + block: block1_hash, + changes: vec![ + (StorageKey("a52da2b7c269da1366b3ed1cdb7299ce".from_hex().unwrap()), Some(StorageData(vec![190, 3, 0, 0, 0, 0, 0, 0]))), + ], + }, + ]; + + // Query changes only up to block1 + let result = api.query_storage( + vec![StorageKey("a52da2b7c269da1366b3ed1cdb7299ce".from_hex().unwrap())], + genesis_hash, + Some(block1_hash).into(), + ); + + assert_eq!(result.unwrap(), expected); + + // Query all changes + let result = api.query_storage( + vec![StorageKey("a52da2b7c269da1366b3ed1cdb7299ce".from_hex().unwrap())], + genesis_hash, + None.into(), + ); + + expected.push(StorageChangeSet { + block: block2_hash, + changes: vec![ + (StorageKey("a52da2b7c269da1366b3ed1cdb7299ce".from_hex().unwrap()), Some(StorageData(vec![148, 3, 0, 0, 0, 0, 0, 0]))), + ], + }); + assert_eq!(result.unwrap(), expected); +} diff --git a/substrate/runtime/primitives/src/generic.rs b/substrate/runtime/primitives/src/generic.rs index 149f24e6874e6..f1a9448c8fafc 100644 --- a/substrate/runtime/primitives/src/generic.rs +++ b/substrate/runtime/primitives/src/generic.rs @@ -446,36 +446,6 @@ pub struct SignedBlock { pub justification: Justification, } -/// Range iterator for types implementing `SimpleArithmetic`. -/// -/// It's a half-open range (includes lower bound, excludes higher bound). -/// A workaround for lack of `Step` trait stabilisation. -/// See: https://github.com/rust-lang/rust/issues/42168 -pub struct RangeIterator(pub T, pub T); - -impl RangeIterator { - /// Creates new range iterator. - /// - /// NOTE if `from >= to` the iterator will not panic, - /// but rather will just be empty. - pub fn new(from: T, to: T) -> Self { - RangeIterator(from, to) - } -} - -impl Iterator for RangeIterator { - type Item = T; - - fn next(&mut self) -> Option { - if self.0 >= self.1 { - return None; - } - - self.0 = self.0 + T::one(); - self.0.into() - } -} - #[cfg(test)] mod tests { use codec::{Decode, Encode}; @@ -530,18 +500,4 @@ mod tests { assert_eq!(block, decoded); } } - - #[test] - fn range_iterator() { - let mut it1 = RangeIterator::new(0, 3); - let mut it2 = RangeIterator::new(3, 3); - let mut it3 = RangeIterator::new(4, 3); - - assert!(it1.next(), Some(0)); - assert!(it1.next(), Some(1)); - assert!(it1.next(), Some(2)); - assert!(it1.next(), None); - assert!(it2.next(), None); - assert!(it3.next(), None); - } }