From 816e132cd79b4ef5b8213d003a29d992902eff33 Mon Sep 17 00:00:00 2001 From: cheme <emericchevalier.pro@gmail.com> Date: Sun, 1 Sep 2019 02:19:54 +0200 Subject: [PATCH] Add missing child trie rpc on_custom_message handler (#3522) * missing on_custom for child read. * fix and complete, add test cases. * shorten line. * replace vecs of key value tuple with maps. --- substrate/core/client/src/genesis.rs | 11 ++-- substrate/core/client/src/light/fetcher.rs | 63 ++++++++++++++++++- substrate/core/network/src/chain.rs | 13 ++++ substrate/core/network/src/protocol.rs | 33 +++++++++- substrate/core/rpc/src/state/tests.rs | 10 +++ substrate/core/test-runtime/client/src/lib.rs | 37 ++++++++++- substrate/core/test-runtime/src/genesismap.rs | 14 +++-- 7 files changed, 167 insertions(+), 14 deletions(-) diff --git a/substrate/core/client/src/genesis.rs b/substrate/core/client/src/genesis.rs index 4791a34d174..88529114e27 100644 --- a/substrate/core/client/src/genesis.rs +++ b/substrate/core/client/src/genesis.rs @@ -48,7 +48,7 @@ mod tests { runtime::{Hash, Transfer, Block, BlockNumber, Header, Digest}, AccountKeyring, Sr25519Keyring, }; - use primitives::Blake2Hasher; + use primitives::{Blake2Hasher, map}; use hex::*; native_executor_instance!( @@ -152,7 +152,8 @@ mod tests { vec![AccountKeyring::One.into(), AccountKeyring::Two.into()], 1000, None, - vec![], + map![], + map![], ).genesis_map(); let genesis_hash = insert_genesis_block(&mut storage); @@ -181,7 +182,8 @@ mod tests { vec![AccountKeyring::One.into(), AccountKeyring::Two.into()], 1000, None, - vec![], + map![], + map![], ).genesis_map(); let genesis_hash = insert_genesis_block(&mut storage); @@ -210,7 +212,8 @@ mod tests { vec![AccountKeyring::One.into(), AccountKeyring::Two.into()], 68, None, - vec![], + map![], + map![], ).genesis_map(); let genesis_hash = insert_genesis_block(&mut storage); diff --git a/substrate/core/client/src/light/fetcher.rs b/substrate/core/client/src/light/fetcher.rs index 8502a19ba64..d09a1893611 100644 --- a/substrate/core/client/src/light/fetcher.rs +++ b/substrate/core/client/src/light/fetcher.rs @@ -570,7 +570,8 @@ 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 heap_pages = remote_client.storage(&remote_block_id, &StorageKey(well_known_keys::HEAP_PAGES.to_vec())) @@ -592,6 +593,46 @@ 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>) { + use test_client::DefaultTestClientBuilderExt; + use test_client::TestClientBuilderExt; + // prepare remote client + let remote_client = test_client::TestClientBuilder::new() + .add_extra_child_storage(b":child_storage:default:child1".to_vec(), b"key1".to_vec(), b"value1".to_vec()) + .build(); + 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(); + + // 'fetch' child read proof from remote node + let child_value = remote_client.child_storage( + &remote_block_id, + &StorageKey(b":child_storage:default:child1".to_vec()), + &StorageKey(b"key1".to_vec()), + ).unwrap().unwrap().0; + assert_eq!(b"value1"[..], child_value[..]); + let remote_read_proof = remote_client.read_child_proof( + &remote_block_id, + b":child_storage:default:child1", + b"key1", + ).unwrap(); + + // check locally + let local_storage = InMemoryBlockchain::<Block>::new(); + local_storage.insert( + remote_block_hash, + remote_block_header.clone(), + None, + None, + crate::backend::NewBlockState::Final, + ).unwrap(); + let local_executor = NativeExecutor::<test_client::LocalExecutor>::new(None); + let local_checker = LightDataChecker::new(Arc::new(DummyBlockchain::new(DummyStorage::new())), local_executor); + (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>>) { // prepare remote client let remote_client = test_client::new(); @@ -638,6 +679,26 @@ pub mod tests { }, remote_read_proof).unwrap().unwrap()[0], heap_pages as u8); } + #[test] + fn storage_child_read_proof_is_generated_and_checked() { + let ( + local_checker, + remote_block_header, + remote_read_proof, + result, + ) = prepare_for_read_child_proof_check(); + assert_eq!((&local_checker as &dyn FetchChecker<Block>).check_read_child_proof( + &RemoteReadChildRequest::<Header> { + block: remote_block_header.hash(), + header: remote_block_header, + storage_key: b":child_storage:default:child1".to_vec(), + key: b"key1".to_vec(), + retry_count: None, + }, + remote_read_proof + ).unwrap().unwrap(), result); + } + #[test] fn header_proof_is_generated_and_checked() { let (local_checker, local_cht_root, remote_block_header, remote_header_proof) = prepare_for_header_proof_check(true); diff --git a/substrate/core/network/src/chain.rs b/substrate/core/network/src/chain.rs index a73d7e53e21..99068698bef 100644 --- a/substrate/core/network/src/chain.rs +++ b/substrate/core/network/src/chain.rs @@ -51,6 +51,9 @@ pub trait Client<Block: BlockT>: Send + Sync { /// Get storage read execution proof. fn read_proof(&self, block: &Block::Hash, key: &[u8]) -> Result<Vec<Vec<u8>>, Error>; + /// Get child storage read execution proof. + fn read_child_proof(&self, block: &Block::Hash, storage_key: &[u8], key: &[u8]) -> Result<Vec<Vec<u8>>, Error>; + /// Get method execution proof. fn execution_proof(&self, block: &Block::Hash, method: &str, data: &[u8]) -> Result<(Vec<u8>, Vec<Vec<u8>>), Error>; @@ -113,6 +116,16 @@ impl<B, E, Block, RA> Client<Block> for SubstrateClient<B, E, Block, RA> where (self as &SubstrateClient<B, E, Block, RA>).read_proof(&BlockId::Hash(block.clone()), key) } + fn read_child_proof( + &self, + block: &Block::Hash, + storage_key: &[u8], + key: &[u8] + ) -> Result<Vec<Vec<u8>>, Error> { + (self as &SubstrateClient<B, E, Block, RA>) + .read_child_proof(&BlockId::Hash(block.clone()), storage_key, key) + } + fn execution_proof(&self, block: &Block::Hash, method: &str, data: &[u8]) -> Result<(Vec<u8>, Vec<Vec<u8>>), Error> { (self as &SubstrateClient<B, E, Block, RA>).execution_proof(&BlockId::Hash(block.clone()), method, data) } diff --git a/substrate/core/network/src/protocol.rs b/substrate/core/network/src/protocol.rs index b561322b5bb..cbe6bc356c4 100644 --- a/substrate/core/network/src/protocol.rs +++ b/substrate/core/network/src/protocol.rs @@ -547,7 +547,8 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Protocol<B, S, H> { self.on_finality_proof_request(who, request), GenericMessage::FinalityProofResponse(response) => return self.on_finality_proof_response(who, response), - GenericMessage::RemoteReadChildRequest(_) => {} + GenericMessage::RemoteReadChildRequest(request) => + self.on_remote_read_child_request(who, request), GenericMessage::Consensus(msg) => { if self.context_data.peers.get(&who).map_or(false, |peer| peer.info.protocol_version > 2) { self.consensus_gossip.on_incoming( @@ -1293,6 +1294,36 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Protocol<B, S, H> { ); } + fn on_remote_read_child_request( + &mut self, + who: PeerId, + request: message::RemoteReadChildRequest<B::Hash>, + ) { + trace!(target: "sync", "Remote read child request {} from {} ({} {} at {})", + request.id, who, request.storage_key.to_hex::<String>(), request.key.to_hex::<String>(), request.block); + let proof = match self.context_data.chain.read_child_proof(&request.block, &request.storage_key, &request.key) { + Ok(proof) => proof, + Err(error) => { + trace!(target: "sync", "Remote read child request {} from {} ({} {} at {}) failed with: {}", + request.id, + who, + request.storage_key.to_hex::<String>(), + request.key.to_hex::<String>(), + request.block, + error + ); + Default::default() + } + }; + self.send_message( + who, + GenericMessage::RemoteReadResponse(message::RemoteReadResponse { + id: request.id, + proof, + }), + ); + } + fn on_remote_read_response( &mut self, who: PeerId, diff --git a/substrate/core/rpc/src/state/tests.rs b/substrate/core/rpc/src/state/tests.rs index 656abf56a96..0b581d166f0 100644 --- a/substrate/core/rpc/src/state/tests.rs +++ b/substrate/core/rpc/src/state/tests.rs @@ -30,14 +30,18 @@ use test_client::{ fn should_return_storage() { const KEY: &[u8] = b":mock"; const VALUE: &[u8] = b"hello world"; + const STORAGE_KEY: &[u8] = b":child_storage:default:child"; + const CHILD_VALUE: &[u8] = b"hello world !"; let core = tokio::runtime::Runtime::new().unwrap(); let client = TestClientBuilder::new() .add_extra_storage(KEY.to_vec(), VALUE.to_vec()) + .add_extra_child_storage(STORAGE_KEY.to_vec(), KEY.to_vec(), CHILD_VALUE.to_vec()) .build(); let genesis_hash = client.genesis_hash(); let client = State::new(Arc::new(client), Subscriptions::new(Arc::new(core.executor()))); let key = StorageKey(KEY.to_vec()); + let storage_key = StorageKey(STORAGE_KEY.to_vec()); assert_eq!( client.storage(key.clone(), Some(genesis_hash).into()) @@ -52,6 +56,12 @@ fn should_return_storage() { client.storage_size(key.clone(), None).unwrap().unwrap() as usize, VALUE.len(), ); + assert_eq!( + client.child_storage(storage_key, key, Some(genesis_hash).into()) + .map(|x| x.map(|x| x.0.len())).unwrap().unwrap() as usize, + CHILD_VALUE.len(), + ); + } #[test] diff --git a/substrate/core/test-runtime/client/src/lib.rs b/substrate/core/test-runtime/client/src/lib.rs index 767c862083f..aeed1e7ad44 100644 --- a/substrate/core/test-runtime/client/src/lib.rs +++ b/substrate/core/test-runtime/client/src/lib.rs @@ -23,6 +23,7 @@ pub mod trait_tests; mod block_builder_ext; use std::sync::Arc; +use std::collections::HashMap; pub use block_builder_ext::BlockBuilderExt; pub use generic_test_client::*; pub use runtime; @@ -97,7 +98,8 @@ pub type LightExecutor = client::light::call_executor::RemoteOrLocalCallExecutor pub struct GenesisParameters { support_changes_trie: bool, heap_pages_override: Option<u64>, - extra_storage: Vec<(Vec<u8>, Vec<u8>)>, + extra_storage: HashMap<Vec<u8>, Vec<u8>>, + child_extra_storage: HashMap<Vec<u8>, HashMap<Vec<u8>, Vec<u8>>>, } impl GenesisParameters { @@ -117,6 +119,7 @@ impl GenesisParameters { 1000, self.heap_pages_override, self.extra_storage.clone(), + self.child_extra_storage.clone(), ) } } @@ -184,6 +187,18 @@ pub trait TestClientBuilderExt<B>: Sized { /// # Panics /// /// Panics if the key is empty. + fn add_extra_child_storage<SK: Into<Vec<u8>>, K: Into<Vec<u8>>, V: Into<Vec<u8>>>( + self, + storage_key: SK, + key: K, + value: V, + ) -> Self; + + /// Add an extra child value into the genesis storage. + /// + /// # Panics + /// + /// Panics if the key is empty. fn add_extra_storage<K: Into<Vec<u8>>, V: Into<Vec<u8>>>(self, key: K, value: V) -> Self; /// Build the test client. @@ -214,10 +229,28 @@ impl<B> TestClientBuilderExt<B> for TestClientBuilder< fn add_extra_storage<K: Into<Vec<u8>>, V: Into<Vec<u8>>>(mut self, key: K, value: V) -> Self { let key = key.into(); assert!(!key.is_empty()); - self.genesis_init_mut().extra_storage.push((key, value.into())); + self.genesis_init_mut().extra_storage.insert(key, value.into()); + self + } + + fn add_extra_child_storage<SK: Into<Vec<u8>>, K: Into<Vec<u8>>, V: Into<Vec<u8>>>( + mut self, + storage_key: SK, + key: K, + value: V, + ) -> Self { + let storage_key = storage_key.into(); + let key = key.into(); + assert!(!storage_key.is_empty()); + assert!(!key.is_empty()); + self.genesis_init_mut().child_extra_storage + .entry(storage_key) + .or_insert_with(Default::default) + .insert(key, value.into()); self } + fn build_with_longest_chain(self) -> (Client<B>, client::LongestChain<B, runtime::Block>) { self.build_with_native_executor(None) } diff --git a/substrate/core/test-runtime/src/genesismap.rs b/substrate/core/test-runtime/src/genesismap.rs index c2dd49156d7..34d7eecae0c 100644 --- a/substrate/core/test-runtime/src/genesismap.rs +++ b/substrate/core/test-runtime/src/genesismap.rs @@ -30,7 +30,8 @@ pub struct GenesisConfig { balances: Vec<(AccountId, u64)>, heap_pages_override: Option<u64>, /// Additional storage key pairs that will be added to the genesis map. - extra_storage: Vec<(Vec<u8>, Vec<u8>)>, + extra_storage: HashMap<Vec<u8>, Vec<u8>>, + child_extra_storage: HashMap<Vec<u8>, HashMap<Vec<u8>, Vec<u8>>>, } impl GenesisConfig { @@ -40,7 +41,8 @@ impl GenesisConfig { endowed_accounts: Vec<AccountId>, balance: u64, heap_pages_override: Option<u64>, - extra_storage: Vec<(Vec<u8>, Vec<u8>)>, + extra_storage: HashMap<Vec<u8>, Vec<u8>>, + child_extra_storage: HashMap<Vec<u8>, HashMap<Vec<u8>, Vec<u8>>>, ) -> Self { GenesisConfig { changes_trie_config: match support_changes_trie { @@ -51,6 +53,7 @@ impl GenesisConfig { balances: endowed_accounts.into_iter().map(|a| (a, balance)).collect(), heap_pages_override, extra_storage, + child_extra_storage, } } @@ -75,10 +78,9 @@ impl GenesisConfig { } map.insert(twox_128(&b"sys:auth"[..])[..].to_vec(), self.authorities.encode()); // Finally, add the extra storage entries. - for (key, value) in self.extra_storage.iter().cloned() { - map.insert(key, value); - } - (map, Default::default()) + map.extend(self.extra_storage.clone().into_iter()); + + (map, self.child_extra_storage.clone()) } } -- GitLab