fixed grumbles

substrate/client/src/blockchain.rs

@@ -17,22 +17,27 @@
 //! Polkadot blockchain trait
 
 use primitives::AuthorityId;
-use runtime_primitives::traits::{Block as BlockT, Header as HeaderT};
+use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, NumberFor};
 use runtime_primitives::generic::BlockId;
 use runtime_primitives::bft::Justification;
 
-use error::Result;
+use error::{ErrorKind, Result};
 
 /// Blockchain database header backend. Does not perform any validation.
 pub trait HeaderBackend<Block: BlockT>: Send + Sync {
 	/// Get block header. Returns `None` if block is not found.
-	fn header(&self, id: BlockId<Block>) -> Result<Option<<Block as BlockT>::Header>>;
+	fn header(&self, id: BlockId<Block>) -> Result<Option<Block::Header>>;
 	/// Get blockchain info.
 	fn info(&self) -> Result<Info<Block>>;
 	/// Get block status.
 	fn status(&self, id: BlockId<Block>) -> Result<BlockStatus>;
 	/// Get block hash by number. Returns `None` if the header is not in the chain.
-	fn hash(&self, number: <<Block as BlockT>::Header as HeaderT>::Number) -> Result<Option<<<Block as BlockT>::Header as HeaderT>::Hash>>;
+	fn hash(&self, number: NumberFor<Block>) -> Result<Option<Block::Hash>>;
+
+	/// Get block header. Returns `UnknownBlock` error if block is not found.
+	fn expect_header(&self, id: BlockId<Block>) -> Result<Block::Header> {
+		self.header(id)?.ok_or_else(|| ErrorKind::UnknownBlock(format!("{}", id)).into())
+	}
 }
 
 /// Blockchain database backend. Does not perform any validation.

substrate/client/src/light/backend.rs

@@ -19,6 +19,7 @@
 
 use std::sync::{Arc, Weak};
 use futures::{Future, IntoFuture};
+use parking_lot::RwLock;
 
 use primitives::AuthorityId;
 use runtime_primitives::{bft::Justification, generic::BlockId};
@@ -38,17 +39,19 @@ pub struct Backend<S, F> {
 }
 
 /// Light block (header and justification) import operation.
-pub struct ImportOperation<Block: BlockT, F> {
+pub struct ImportOperation<Block: BlockT, S, F> {
 	is_new_best: bool,
 	header: Option<Block::Header>,
 	authorities: Option<Vec<AuthorityId>>,
-	_phantom: ::std::marker::PhantomData<F>,
+	_phantom: ::std::marker::PhantomData<(S, F)>,
 }
 
 /// On-demand state.
-pub struct OnDemandState<Block: BlockT, F> {
+pub struct OnDemandState<Block: BlockT, S, F> {
 	fetcher: Weak<F>,
+	blockchain: Weak<Blockchain<S, F>>,
 	block: Block::Hash,
+	cached_header: RwLock<Option<Block::Header>>,
 }
 
 impl<S, F> Backend<S, F> {
@@ -64,9 +67,9 @@ impl<S, F> Backend<S, F> {
 }
 
 impl<S, F, Block> ClientBackend<Block> for Backend<S, F> where Block: BlockT, S: BlockchainStorage<Block>, F: Fetcher<Block> {
-	type BlockImportOperation = ImportOperation<Block, F>;
+	type BlockImportOperation = ImportOperation<Block, S, F>;
 	type Blockchain = Blockchain<S, F>;
-	type State = OnDemandState<Block, F>;
+	type State = OnDemandState<Block, S, F>;
 
 	fn begin_operation(&self, _block: BlockId<Block>) -> ClientResult<Self::BlockImportOperation> {
 		Ok(ImportOperation {
@@ -93,8 +96,10 @@ impl<S, F, Block> ClientBackend<Block> for Backend<S, F> where Block: BlockT, S:
 		};
 
 		Ok(OnDemandState {
-			block: block_hash.ok_or_else(|| ClientErrorKind::UnknownBlock(format!("{}", block)))?,
 			fetcher: self.blockchain.fetcher(),
+			blockchain: Arc::downgrade(&self.blockchain),
+			block: block_hash.ok_or_else(|| ClientErrorKind::UnknownBlock(format!("{}", block)))?,
+			cached_header: RwLock::new(None),
 		})
 	}
 
@@ -105,8 +110,13 @@ impl<S, F, Block> ClientBackend<Block> for Backend<S, F> where Block: BlockT, S:
 
 impl<S, F, Block> RemoteBackend<Block> for Backend<S, F> where Block: BlockT, S: BlockchainStorage<Block>, F: Fetcher<Block> {}
 
-impl<F, Block> BlockImportOperation<Block> for ImportOperation<Block, F> where Block: BlockT, F: Fetcher<Block> {
-	type State = OnDemandState<Block, F>;
+impl<S, F, Block> BlockImportOperation<Block> for ImportOperation<Block, S, F>
+	where
+		Block: BlockT,
+		S: BlockchainStorage<Block>,
+		F: Fetcher<Block>,
+{
+	type State = OnDemandState<Block, S, F>;
 
 	fn state(&self) -> ClientResult<Option<&Self::State>> {
 		// None means 'locally-stateless' backend
@@ -140,23 +150,29 @@ impl<F, Block> BlockImportOperation<Block> for ImportOperation<Block, F> where B
 	}
 }
 
-impl<Block: BlockT, F> Clone for OnDemandState<Block, F> {
-	fn clone(&self) -> Self {
-		OnDemandState {
-			fetcher: self.fetcher.clone(),
-			block: self.block,
-		}
-	}
-}
-
-impl<Block, F> StateBackend for OnDemandState<Block, F> where Block: BlockT, F: Fetcher<Block> {
+impl<Block, S, F> StateBackend for OnDemandState<Block, S, F>
+	where
+		Block: BlockT,
+		S: BlockchainStorage<Block>,
+		F: Fetcher<Block>,
+{
 	type Error = ClientError;
 	type Transaction = ();
 
 	fn storage(&self, key: &[u8]) -> ClientResult<Option<Vec<u8>>> {
+		let mut header = self.cached_header.read().clone();
+		if header.is_none() {
+			let cached_header = self.blockchain.upgrade()
+				.ok_or_else(|| ClientErrorKind::UnknownBlock(format!("{}", self.block)).into())
+				.and_then(|blockchain| blockchain.expect_header(BlockId::Hash(self.block)))?;
+			header = Some(cached_header.clone());
+			*self.cached_header.write() = Some(cached_header);
+		}
+
 		self.fetcher.upgrade().ok_or(ClientErrorKind::NotAvailableOnLightClient)?
 			.remote_read(RemoteReadRequest {
 				block: self.block,
+				header: header.expect("if block above guarantees that header is_some(); qed"),
 				key: key.to_vec(),
 			})
 			.into_future().wait()
@@ -177,7 +193,7 @@ impl<Block, F> StateBackend for OnDemandState<Block, F> where Block: BlockT, F:
 	}
 }
 
-impl<Block, F> TryIntoStateTrieBackend for OnDemandState<Block, F> where Block: BlockT, F: Fetcher<Block> {
+impl<Block, S, F> TryIntoStateTrieBackend for OnDemandState<Block, S, F> where Block: BlockT, F: Fetcher<Block> {
 	fn try_into_trie_backend(self) -> Option<StateTrieBackend> {
 		None
 	}
@@ -188,12 +204,11 @@ pub mod tests {
 	use futures::future::{ok, err, FutureResult};
 	use parking_lot::Mutex;
 	use call_executor::CallResult;
-	use executor::{self, NativeExecutionDispatch};
+	use executor::NativeExecutionDispatch;
 	use error::Error as ClientError;
-	use test_client::{self, runtime::{Hash, Block}};
-	use in_mem::{Blockchain as InMemoryBlockchain};
-	use light::{new_fetch_checker, new_light_blockchain};
-	use light::fetcher::{Fetcher, FetchChecker, LightDataChecker, RemoteCallRequest};
+	use test_client::{self, runtime::{Header, Block}};
+	use light::new_fetch_checker;
+	use light::fetcher::{Fetcher, FetchChecker, RemoteCallRequest};
 	use super::*;
 
 	pub type OkCallFetcher = Mutex<CallResult>;
@@ -202,11 +217,11 @@ pub mod tests {
 		type RemoteReadResult = FutureResult<Option<Vec<u8>>, ClientError>;
 		type RemoteCallResult = FutureResult<CallResult, ClientError>;
 
-		fn remote_read(&self, _request: RemoteReadRequest<Hash>) -> Self::RemoteReadResult {
+		fn remote_read(&self, _request: RemoteReadRequest<Header>) -> Self::RemoteReadResult {
 			err("Not implemented on test node".into())
 		}
 
-		fn remote_call(&self, _request: RemoteCallRequest<Hash>) -> Self::RemoteCallResult {
+		fn remote_call(&self, _request: RemoteCallRequest<Header>) -> Self::RemoteCallResult {
 			ok((*self.lock()).clone())
 		}
 	}
@@ -218,24 +233,23 @@ pub mod tests {
 		let remote_block_id = BlockId::Number(0);
 		let remote_block_hash = remote_client.block_hash(0).unwrap().unwrap();
 		let mut remote_block_header = remote_client.header(&remote_block_id).unwrap().unwrap();
-		remote_block_header.state_root = remote_client.state_at(&remote_block_id).unwrap().storage_root(::std::iter::empty()).0.into();
+		remote_block_header.state_root = remote_client.state_at(&remote_block_id)
+			.unwrap().storage_root(::std::iter::empty()).0.into();
 
 		// 'fetch' read proof from remote node
 		let authorities_len = remote_client.authorities_at(&remote_block_id).unwrap().len();
 		let remote_read_proof = remote_client.read_proof(&remote_block_id, b":auth:len").unwrap();
 
 		// check remote read proof locally
-		let local_storage = InMemoryBlockchain::<Block>::new();
-		local_storage.insert(remote_block_hash, remote_block_header, None, None, true);
 		let local_executor = test_client::LocalExecutor::with_heap_pages(8, 8);
-		let local_checker: LightDataChecker<
-				InMemoryBlockchain<Block>,
-				executor::NativeExecutor<test_client::LocalExecutor>,
-				OkCallFetcher
-			> = new_fetch_checker(new_light_blockchain(local_storage), local_executor);
-		assert_eq!(local_checker.check_read_proof(&RemoteReadRequest {
+		let local_checker = new_fetch_checker(local_executor);
+		let request = RemoteReadRequest {
 			block: remote_block_hash,
+			header: remote_block_header,
 			key: b":auth:len".to_vec(),
-		}, remote_read_proof).unwrap().unwrap()[0], authorities_len as u8);
+		};
+		assert_eq!((&local_checker as &FetchChecker<Block>).check_read_proof(
+			&request,
+			remote_read_proof).unwrap().unwrap()[0], authorities_len as u8);
 	}
 }

substrate/client/src/light/call_executor.rs

@@ -60,9 +60,11 @@ impl<B, F, Block> CallExecutor<Block> for RemoteCallExecutor<B, F>
 			BlockId::Number(number) => self.blockchain.hash(number)?
 				.ok_or_else(|| ClientErrorKind::UnknownBlock(format!("{}", number)))?,
 		};
+		let block_header = self.blockchain.expect_header(id.clone())?;
 
 		self.fetcher.remote_call(RemoteCallRequest {
-			block: block_hash.clone(),
+			block: block_hash,
+			header: block_header,
 			method: method.into(),
 			call_data: call_data.to_vec(),
 		}).into_future().wait()
@@ -97,34 +99,17 @@ impl<B, F, Block> CallExecutor<Block> for RemoteCallExecutor<B, F>
 }
 
 /// Check remote execution proof using given backend.
-pub fn check_execution_proof<Block, B, E>(
-	blockchain: &B,
+pub fn check_execution_proof<Header, E>(
 	executor: &E,
-	request: &RemoteCallRequest<Block::Hash>,
+	request: &RemoteCallRequest<Header>,
 	remote_proof: Vec<Vec<u8>>
 ) -> ClientResult<CallResult>
 	where
-		Block: BlockT,
-		B: ChainBackend<Block>,
+		Header: HeaderT,
 		E: CodeExecutor,
 {
-	let local_header = blockchain.header(BlockId::Hash(request.block))?;
-	let local_header = local_header.ok_or_else(|| ClientErrorKind::UnknownBlock(format!("{}", request.block)))?;
-	let local_state_root = *local_header.state_root();
-	do_check_execution_proof(local_state_root.into(), executor, request, remote_proof)
-}
+	let local_state_root = request.header.state_root();
 
-/// Check remote execution proof using given state root.
-fn do_check_execution_proof<Hash, E>(
-	local_state_root: Hash,
-	executor: &E,
-	request: &RemoteCallRequest<Hash>,
-	remote_proof: Vec<Vec<u8>>,
-) -> ClientResult<CallResult>
-	where
-		Hash: ::std::fmt::Display + ::std::convert::AsRef<[u8]>,
-		E: CodeExecutor,
-{
 	let mut changes = OverlayedChanges::default();
 	let (local_result, _) = execution_proof_check(
 		TrieH256::from_slice(local_state_root.as_ref()).into(),
@@ -156,8 +141,15 @@ mod tests {
 
 		// check remote execution proof locally
 		let local_executor = test_client::LocalExecutor::with_heap_pages(8, 8);
-		do_check_execution_proof(remote_block_storage_root.into(), &local_executor, &RemoteCallRequest {
+		check_execution_proof(&local_executor, &RemoteCallRequest {
 			block: test_client::runtime::Hash::default(),
+			header: test_client::runtime::Header {
+				state_root: remote_block_storage_root.into(),
+				parent_hash: Default::default(),
+				number: 0,
+				extrinsics_root: Default::default(),
+				digest: Default::default(),
+			},
 			method: "authorities".into(),
 			call_data: vec![],
 		}, remote_execution_proof).unwrap();