Create opaque struct for StorageProof. (#3834)

Passing around Vec<Vec<u8>> everywhere is gross and confusing and
breaks encapsulation.

substrate/core/client/src/block_builder/block_builder.rs

@@ -22,6 +22,7 @@ use sr_primitives::traits::{
 	Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef, DigestFor,
 };
 use primitives::{H256, ExecutionContext};
+use state_machine::StorageProof;
 use crate::blockchain::HeaderBackend;
 use crate::runtime_api::{Core, ApiExt};
 use crate::error;
@@ -140,7 +141,7 @@ where
 	///
 	/// The proof will be `Some(_)`, if proof recording was enabled while creating
 	/// the block builder.
-	pub fn bake_and_extract_proof(mut self) -> error::Result<(Block, Option<Vec<Vec<u8>>>)> {
+	pub fn bake_and_extract_proof(mut self) -> error::Result<(Block, Option<StorageProof>)> {
 		self.bake_impl()?;
 
 		let proof = self.api.extract_proof();

substrate/core/client/src/call_executor.rs

@@ -21,7 +21,7 @@ use sr_primitives::{
 };
 use state_machine::{
 	self, OverlayedChanges, Ext, ExecutionManager, StateMachine, ExecutionStrategy,
-	backend::Backend as _, ChangesTrieTransaction,
+	backend::Backend as _, ChangesTrieTransaction, StorageProof,
 };
 use executor::{RuntimeVersion, RuntimeInfo, NativeVersion};
 use hash_db::Hasher;
@@ -127,7 +127,7 @@ where
 		overlay: &mut OverlayedChanges,
 		method: &str,
 		call_data: &[u8]
-	) -> Result<(Vec<u8>, Vec<Vec<u8>>), error::Error> {
+	) -> Result<(Vec<u8>, StorageProof), error::Error> {
 		let trie_state = state.as_trie_backend()
 			.ok_or_else(||
 				Box::new(state_machine::ExecutionError::UnableToGenerateProof)
@@ -145,7 +145,7 @@ where
 		overlay: &mut OverlayedChanges,
 		method: &str,
 		call_data: &[u8]
-	) -> Result<(Vec<u8>, Vec<Vec<u8>>), error::Error>;
+	) -> Result<(Vec<u8>, StorageProof), error::Error>;
 
 	/// Get runtime version if supported.
 	fn native_runtime_version(&self) -> Option<&NativeVersion>;
@@ -377,7 +377,7 @@ where
 		overlay: &mut OverlayedChanges,
 		method: &str,
 		call_data: &[u8]
-	) -> Result<(Vec<u8>, Vec<Vec<u8>>), error::Error> {
+	) -> Result<(Vec<u8>, StorageProof), error::Error> {
 		state_machine::prove_execution_on_trie_backend(
 			trie_state,
 			overlay,

substrate/core/client/src/cht.rs

@@ -30,7 +30,7 @@ use trie;
 use primitives::{H256, convert_hash};
 use sr_primitives::traits::{Header as HeaderT, SimpleArithmetic, Zero, One};
 use state_machine::backend::InMemory as InMemoryState;
-use state_machine::{MemoryDB, TrieBackend, Backend as StateBackend,
+use state_machine::{MemoryDB, TrieBackend, Backend as StateBackend, StorageProof,
 	prove_read_on_trie_backend, read_proof_check, read_proof_check_on_proving_backend};
 
 use crate::error::{Error as ClientError, Result as ClientResult};
@@ -88,7 +88,7 @@ pub fn build_proof<Header, Hasher, BlocksI, HashesI>(
 	cht_num: Header::Number,
 	blocks: BlocksI,
 	hashes: HashesI
-) -> ClientResult<Vec<Vec<u8>>>
+) -> ClientResult<StorageProof>
 	where
 		Header: HeaderT,
 		Hasher: hash_db::Hasher,
@@ -114,7 +114,7 @@ pub fn check_proof<Header, Hasher>(
 	local_root: Header::Hash,
 	local_number: Header::Number,
 	remote_hash: Header::Hash,
-	remote_proof: Vec<Vec<u8>>
+	remote_proof: StorageProof,
 ) -> ClientResult<()>
 	where
 		Header: HeaderT,

substrate/core/client/src/client.rs

@@ -43,7 +43,7 @@ use state_machine::{
 	DBValue, Backend as StateBackend, ChangesTrieAnchorBlockId, ExecutionStrategy, ExecutionManager,
 	prove_read, prove_child_read, ChangesTrieRootsStorage, ChangesTrieStorage,
 	ChangesTrieTransaction, ChangesTrieConfigurationRange, key_changes, key_changes_proof,
-	OverlayedChanges, BackendTrustLevel,
+	OverlayedChanges, BackendTrustLevel, StorageProof, merge_storage_proofs,
 };
 use executor::{RuntimeVersion, RuntimeInfo};
 use consensus::{
@@ -421,7 +421,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	}
 
 	/// Reads storage value at a given block + key, returning read proof.
-	pub fn read_proof<I>(&self, id: &BlockId<Block>, keys: I) -> error::Result<Vec<Vec<u8>>> where
+	pub fn read_proof<I>(&self, id: &BlockId<Block>, keys: I) -> error::Result<StorageProof> where
 		I: IntoIterator,
 		I::Item: AsRef<[u8]>,
 	{
@@ -437,7 +437,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		id: &BlockId<Block>,
 		storage_key: &[u8],
 		keys: I,
-	) -> error::Result<Vec<Vec<u8>>> where
+	) -> error::Result<StorageProof> where
 		I: IntoIterator,
 		I::Item: AsRef<[u8]>,
 	{
@@ -454,14 +454,14 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		id: &BlockId<Block>,
 		method: &str,
 		call_data: &[u8]
-	) -> error::Result<(Vec<u8>, Vec<Vec<u8>>)> {
+	) -> error::Result<(Vec<u8>, StorageProof)> {
 		let state = self.state_at(id)?;
 		let header = self.prepare_environment_block(id)?;
 		prove_execution(state, header, &self.executor, method, call_data)
 	}
 
 	/// Reads given header and generates CHT-based header proof.
-	pub fn header_proof(&self, id: &BlockId<Block>) -> error::Result<(Block::Header, Vec<Vec<u8>>)> {
+	pub fn header_proof(&self, id: &BlockId<Block>) -> error::Result<(Block::Header, StorageProof)> {
 		self.header_proof_with_cht_size(id, cht::size())
 	}
 
@@ -477,7 +477,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		&self,
 		id: &BlockId<Block>,
 		cht_size: NumberFor<Block>,
-	) -> error::Result<(Block::Header, Vec<Vec<u8>>)> {
+	) -> error::Result<(Block::Header, StorageProof)> {
 		let proof_error = || error::Error::Backend(format!("Failed to generate header proof for {:?}", id));
 		let header = self.backend.blockchain().expect_header(*id)?;
 		let block_num = *header.number();
@@ -696,18 +696,18 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		&self,
 		cht_size: NumberFor<Block>,
 		blocks: I
-	) -> error::Result<Vec<Vec<u8>>> {
+	) -> error::Result<StorageProof> {
 		// most probably we have touched several changes tries that are parts of the single CHT
 		// => GroupBy changes tries by CHT number and then gather proof for the whole group at once
-		let mut proof = HashSet::new();
+		let mut proofs = Vec::new();
 
 		cht::for_each_cht_group::<Block::Header, _, _, _>(cht_size, blocks, |_, cht_num, cht_blocks| {
 			let cht_proof = self.changes_trie_roots_proof_at_cht(cht_size, cht_num, cht_blocks)?;
-			proof.extend(cht_proof);
+			proofs.push(cht_proof);
 			Ok(())
 		}, ())?;
 
-		Ok(proof.into_iter().collect())
+		Ok(merge_storage_proofs(proofs))
 	}
 
 	/// Generates CHT-based proof for roots of changes tries at given blocks (that are part of single CHT).
@@ -716,7 +716,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		cht_size: NumberFor<Block>,
 		cht_num: NumberFor<Block>,
 		blocks: Vec<NumberFor<Block>>
-	) -> error::Result<Vec<Vec<u8>>> {
+	) -> error::Result<StorageProof> {
 		let cht_start = cht::start_number(cht_size, cht_num);
 		let mut current_num = cht_start;
 		let cht_range = ::std::iter::from_fn(|| {

substrate/core/client/src/lib.rs

@@ -122,7 +122,7 @@ pub use crate::client::{
 #[cfg(feature = "std")]
 pub use crate::notifications::{StorageEventStream, StorageChangeSet};
 #[cfg(feature = "std")]
-pub use state_machine::ExecutionStrategy;
+pub use state_machine::{ExecutionStrategy, StorageProof};
 #[cfg(feature = "std")]
 pub use crate::leaves::LeafSet;
 #[cfg(feature = "std")]

substrate/core/client/src/light/call_executor.rs

@@ -16,10 +16,7 @@
 
 //! Methods that light client could use to execute runtime calls.
 
-use std::{
-	collections::HashSet, sync::Arc, panic::UnwindSafe, result,
-	cell::RefCell, rc::Rc,
-};
+use std::{sync::Arc, panic::UnwindSafe, result, cell::RefCell, rc::Rc};
 
 use codec::{Encode, Decode};
 use primitives::{
@@ -31,7 +28,8 @@ use sr_primitives::{
 };
 use state_machine::{
 	self, Backend as StateBackend, OverlayedChanges, ExecutionStrategy, create_proof_check_backend,
-	execution_proof_check_on_trie_backend, ExecutionManager, ChangesTrieTransaction,
+	execution_proof_check_on_trie_backend, ExecutionManager, ChangesTrieTransaction, StorageProof,
+	merge_storage_proofs,
 };
 use hash_db::Hasher;
 
@@ -179,7 +177,7 @@ impl<Block, B, Local> CallExecutor<Block, Blake2Hasher> for
 		_changes: &mut OverlayedChanges,
 		_method: &str,
 		_call_data: &[u8]
-	) -> ClientResult<(Vec<u8>, Vec<Vec<u8>>)> {
+	) -> ClientResult<(Vec<u8>, StorageProof)> {
 		Err(ClientError::NotAvailableOnLightClient)
 	}
 
@@ -198,7 +196,7 @@ pub fn prove_execution<Block, S, E>(
 	executor: &E,
 	method: &str,
 	call_data: &[u8],
-) -> ClientResult<(Vec<u8>, Vec<Vec<u8>>)>
+) -> ClientResult<(Vec<u8>, StorageProof)>
 	where
 		Block: BlockT<Hash=H256>,
 		S: StateBackend<Blake2Hasher>,
@@ -218,11 +216,7 @@ pub fn prove_execution<Block, S, E>(
 
 	// execute method + record execution proof
 	let (result, exec_proof) = executor.prove_at_trie_state(&trie_state, &mut changes, method, call_data)?;
-	let total_proof = init_proof.into_iter()
-		.chain(exec_proof.into_iter())
-		.collect::<HashSet<_>>()
-		.into_iter()
-		.collect();
+	let total_proof = merge_storage_proofs(vec![init_proof, exec_proof]);
 
 	Ok((result, total_proof))
 }
@@ -234,7 +228,7 @@ pub fn prove_execution<Block, S, E>(
 pub fn check_execution_proof<Header, E, H>(
 	executor: &E,
 	request: &RemoteCallRequest<Header>,
-	remote_proof: Vec<Vec<u8>>,
+	remote_proof: StorageProof,
 ) -> ClientResult<Vec<u8>>
 	where
 		Header: HeaderT,
@@ -258,7 +252,7 @@ pub fn check_execution_proof<Header, E, H>(
 fn check_execution_proof_with_make_header<Header, E, H, MakeNextHeader: Fn(&Header) -> Header>(
 	executor: &E,
 	request: &RemoteCallRequest<Header>,
-	remote_proof: Vec<Vec<u8>>,
+	remote_proof: StorageProof,
 	make_next_header: MakeNextHeader,
 ) -> ClientResult<Vec<u8>>
 	where
@@ -382,7 +376,7 @@ mod tests {
 			_overlay: &mut OverlayedChanges,
 			_method: &str,
 			_call_data: &[u8]
-		) -> Result<(Vec<u8>, Vec<Vec<u8>>), ClientError> {
+		) -> Result<(Vec<u8>, StorageProof), ClientError> {
 			unreachable!()
 		}
 

substrate/core/client/src/light/fetcher.rs

@@ -33,6 +33,7 @@ use state_machine::{
 	TrieBackend, read_proof_check, key_changes_proof_check, create_proof_check_backend_storage,
 	read_child_proof_check,
 };
+pub use state_machine::StorageProof;
 
 use crate::cht;
 use crate::error::{Error as ClientError, Result as ClientResult};
@@ -129,7 +130,7 @@ pub struct ChangesProof<Header: HeaderT> {
 	pub roots: BTreeMap<Header::Number, Header::Hash>,
 	/// The proofs for all changes tries roots that have been touched AND are
 	/// missing from the requester' node. It is a map of CHT number => proof.
-	pub roots_proof: Vec<Vec<u8>>,
+	pub roots_proof: StorageProof,
 }
 
 /// Remote block body request
@@ -186,25 +187,25 @@ pub trait FetchChecker<Block: BlockT>: Send + Sync {
 		&self,
 		request: &RemoteHeaderRequest<Block::Header>,
 		header: Option<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<Block::Header>;
 	/// Check remote storage read proof.
 	fn check_read_proof(
 		&self,
 		request: &RemoteReadRequest<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<HashMap<Vec<u8>, Option<Vec<u8>>>>;
 	/// Check remote storage read proof.
 	fn check_read_child_proof(
 		&self,
 		request: &RemoteReadChildRequest<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<HashMap<Vec<u8>, Option<Vec<u8>>>>;
 	/// Check remote method execution proof.
 	fn check_execution_proof(
 		&self,
 		request: &RemoteCallRequest<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<Vec<u8>>;
 	/// Check remote changes query proof.
 	fn check_changes_proof(
@@ -318,7 +319,7 @@ impl<E, H, B: BlockT, S: BlockchainStorage<B>> LightDataChecker<E, H, B, S> {
 		&self,
 		cht_size: NumberFor<B>,
 		remote_roots: &BTreeMap<NumberFor<B>, B::Hash>,
-		remote_roots_proof: Vec<Vec<u8>>,
+		remote_roots_proof: StorageProof,
 	) -> ClientResult<()>
 		where
 			H: Hasher,
@@ -378,7 +379,7 @@ impl<E, Block, H, S> FetchChecker<Block> for LightDataChecker<E, H, Block, S>
 		&self,
 		request: &RemoteHeaderRequest<Block::Header>,
 		remote_header: Option<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<Block::Header> {
 		let remote_header = remote_header.ok_or_else(||
 			ClientError::from(ClientError::InvalidCHTProof))?;
@@ -394,7 +395,7 @@ impl<E, Block, H, S> FetchChecker<Block> for LightDataChecker<E, H, Block, S>
 	fn check_read_proof(
 		&self,
 		request: &RemoteReadRequest<Block::Header>,
-		remote_proof: Vec<Vec<u8>>,
+		remote_proof: StorageProof,
 	) -> ClientResult<HashMap<Vec<u8>, Option<Vec<u8>>>> {
 		read_proof_check::<H, _>(
 			convert_hash(request.header.state_root()),
@@ -406,7 +407,7 @@ impl<E, Block, H, S> FetchChecker<Block> for LightDataChecker<E, H, Block, S>
 	fn check_read_child_proof(
 		&self,
 		request: &RemoteReadChildRequest<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<HashMap<Vec<u8>, Option<Vec<u8>>>> {
 		read_child_proof_check::<H, _>(
 			convert_hash(request.header.state_root()),
@@ -419,7 +420,7 @@ impl<E, Block, H, S> FetchChecker<Block> for LightDataChecker<E, H, Block, S>
 	fn check_execution_proof(
 		&self,
 		request: &RemoteCallRequest<Block::Header>,
-		remote_proof: Vec<Vec<u8>>
+		remote_proof: StorageProof,
 	) -> ClientResult<Vec<u8>> {
 		check_execution_proof::<_, _, H>(&self.executor, request, remote_proof)
 	}
@@ -572,7 +573,7 @@ pub mod tests {
 		NativeExecutor::new(WasmExecutionMethod::Interpreted, None)
 	}
 
-	fn prepare_for_read_proof_check() -> (TestChecker, Header, Vec<Vec<u8>>, u32) {
+	fn prepare_for_read_proof_check() -> (TestChecker, Header, StorageProof, u32) {
 		// prepare remote client
 		let remote_client = test_client::new();
 		let remote_block_id = BlockId::Number(0);
@@ -606,7 +607,7 @@ pub mod tests {
 		(local_checker, remote_block_header, remote_read_proof, heap_pages)
 	}
 
-	fn prepare_for_read_child_proof_check() -> (TestChecker, Header, Vec<Vec<u8>>, Vec<u8>) {
+	fn prepare_for_read_child_proof_check() -> (TestChecker, Header, StorageProof, Vec<u8>) {
 		use test_client::DefaultTestClientBuilderExt;
 		use test_client::TestClientBuilderExt;
 		// prepare remote client
@@ -648,7 +649,7 @@ pub mod tests {
 		(local_checker, remote_block_header, remote_read_proof, child_value)
 	}
 
-	fn prepare_for_header_proof_check(insert_cht: bool) -> (TestChecker, Hash, Header, Vec<Vec<u8>>) {
+	fn prepare_for_header_proof_check(insert_cht: bool) -> (TestChecker, Hash, Header, StorageProof) {
 		// prepare remote client
 		let remote_client = test_client::new();
 		let mut local_headers_hashes = Vec::new();
@@ -899,13 +900,13 @@ pub mod tests {
 			max_block: remote_proof.max_block,
 			proof: remote_proof.proof.clone(),
 			roots: vec![(begin - 1, Default::default())].into_iter().collect(),
-			roots_proof: vec![],
+			roots_proof: StorageProof::empty(),
 		}).is_err());
 		assert!(local_checker.check_changes_proof(&request, ChangesProof {
 			max_block: remote_proof.max_block,
 			proof: remote_proof.proof.clone(),
 			roots: vec![(end + 1, Default::default())].into_iter().collect(),
-			roots_proof: vec![],
+			roots_proof: StorageProof::empty(),
 		}).is_err());
 	}
 
@@ -945,7 +946,10 @@ pub mod tests {
 			Arc::new(DummyBlockchain::new(local_storage)),
 			local_executor(),
 		);
-		assert!(local_checker.check_changes_tries_proof(4, &remote_proof.roots, vec![]).is_err());
+		let result = local_checker.check_changes_tries_proof(
+			4, &remote_proof.roots, StorageProof::empty()
+		);
+		assert!(result.is_err());
 	}
 
 	#[test]

substrate/core/client/src/runtime_api.rs

@@ -18,7 +18,7 @@
 
 #[doc(hidden)]
 #[cfg(feature = "std")]
-pub use state_machine::OverlayedChanges;
+pub use state_machine::{OverlayedChanges, StorageProof};
 #[doc(hidden)]
 #[cfg(feature = "std")]
 pub use primitives::NativeOrEncoded;
@@ -106,7 +106,7 @@ pub trait ApiExt<Block: BlockT> {
 
 	/// Extract the recorded proof.
 	/// This stops the proof recording.
-	fn extract_proof(&mut self) -> Option<Vec<Vec<u8>>>;
+	fn extract_proof(&mut self) -> Option<StorageProof>;
 }
 
 /// Before calling any runtime api function, the runtime need to be initialized