From 30ce8bbc8c5b66e14379bc570eb22f3078de5168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 24 Aug 2021 11:54:53 +0200 Subject: [PATCH] Fetch runtime code from storage cache when using proofing backend (#9611) Before we fetched the runtime code from the `TrieBackend` and this lead to not using the storage cache. Thus, we recalculated the storage hash for the runtime code on every call into the runtime and this killed the performance on parachains block authoring. The solution is to fetch the runtime code from the storage cache, to make sure we use the cached storage cache. --- substrate/client/api/src/cht.rs | 2 +- substrate/client/db/src/bench.rs | 6 ----- substrate/client/db/src/lib.rs | 2 +- substrate/client/db/src/storage_cache.rs | 6 ++--- substrate/client/light/src/backend.rs | 4 +-- .../service/src/client/call_executor.rs | 26 ++++++++----------- .../primitives/state-machine/src/backend.rs | 2 +- .../state-machine/src/in_memory_backend.rs | 2 +- substrate/primitives/state-machine/src/lib.rs | 16 ++++++------ .../state-machine/src/proving_backend.rs | 4 +-- .../state-machine/src/trie_backend.rs | 2 +- 11 files changed, 31 insertions(+), 41 deletions(-) diff --git a/substrate/client/api/src/cht.rs b/substrate/client/api/src/cht.rs index 83bc84c6ec..ee7854b5d8 100644 --- a/substrate/client/api/src/cht.rs +++ b/substrate/client/api/src/cht.rs @@ -117,7 +117,7 @@ where .into_iter() .map(|(k, v)| (k, Some(v))) .collect::>(); - let mut storage = InMemoryBackend::::default().update(vec![(None, transaction)]); + let storage = InMemoryBackend::::default().update(vec![(None, transaction)]); let trie_storage = storage .as_trie_backend() .expect("InMemoryState::as_trie_backend always returns Some; qed"); diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index a4b8f6696e..1b7826f973 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -454,12 +454,6 @@ impl StateBackend> for BenchmarkingState { .map_or(Default::default(), |s| s.child_keys(child_info, prefix)) } - fn as_trie_backend( - &mut self, - ) -> Option<&sp_state_machine::TrieBackend>> { - None - } - fn commit( &self, storage_root: as Hasher>::Out, diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 9f1dd4c0ec..c7d6029c53 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -274,7 +274,7 @@ impl StateBackend> for RefTrackingState { } fn as_trie_backend( - &mut self, + &self, ) -> Option<&sp_state_machine::TrieBackend>> { self.state.as_trie_backend() } diff --git a/substrate/client/db/src/storage_cache.rs b/substrate/client/db/src/storage_cache.rs index 3193d34796..a895324a2e 100644 --- a/substrate/client/db/src/storage_cache.rs +++ b/substrate/client/db/src/storage_cache.rs @@ -703,7 +703,7 @@ impl>, B: BlockT> StateBackend> for Cachin self.state.child_keys(child_info, prefix) } - fn as_trie_backend(&mut self) -> Option<&TrieBackend>> { + fn as_trie_backend(&self) -> Option<&TrieBackend>> { self.state.as_trie_backend() } @@ -901,9 +901,9 @@ impl>, B: BlockT> StateBackend> self.caching_state().child_keys(child_info, prefix) } - fn as_trie_backend(&mut self) -> Option<&TrieBackend>> { + fn as_trie_backend(&self) -> Option<&TrieBackend>> { self.caching_state - .as_mut() + .as_ref() .expect("`caching_state` is valid for the lifetime of the object; qed") .as_trie_backend() } diff --git a/substrate/client/light/src/backend.rs b/substrate/client/light/src/backend.rs index 87d7dba3dd..3091dce625 100644 --- a/substrate/client/light/src/backend.rs +++ b/substrate/client/light/src/backend.rs @@ -569,9 +569,9 @@ where sp_state_machine::UsageInfo::empty() } - fn as_trie_backend(&mut self) -> Option<&TrieBackend> { + fn as_trie_backend(&self) -> Option<&TrieBackend> { match self { - GenesisOrUnavailableState::Genesis(ref mut state) => state.as_trie_backend(), + GenesisOrUnavailableState::Genesis(ref state) => state.as_trie_backend(), GenesisOrUnavailableState::Unavailable => None, } } diff --git a/substrate/client/service/src/client/call_executor.rs b/substrate/client/service/src/client/call_executor.rs index 41cc1526fa..9b8774ce6d 100644 --- a/substrate/client/service/src/client/call_executor.rs +++ b/substrate/client/service/src/client/call_executor.rs @@ -212,7 +212,7 @@ where backend::changes_tries_state_at_block(at, self.backend.changes_trie_storage())?; let mut storage_transaction_cache = storage_transaction_cache.map(|c| c.borrow_mut()); - let mut state = self.backend.state_at(*at)?; + let state = self.backend.state_at(*at)?; let changes = &mut *changes.borrow_mut(); @@ -220,6 +220,15 @@ where sp_blockchain::Error::UnknownBlock(format!("Could not find block hash for {:?}", at)) })?; + // It is important to extract the runtime code here before we create the proof + // recorder to not record it. We also need to fetch the runtime code from `state` to + // make sure we use the caching layers. + let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state); + + let runtime_code = + state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; + let runtime_code = self.check_override(runtime_code, at)?; + match recorder { Some(recorder) => { let trie_state = state.as_trie_backend().ok_or_else(|| { @@ -227,14 +236,6 @@ where as Box })?; - let state_runtime_code = - sp_state_machine::backend::BackendRuntimeCode::new(trie_state); - // It is important to extract the runtime code here before we create the proof - // recorder. - let runtime_code = - state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, at)?; - let backend = sp_state_machine::ProvingBackend::new_with_recorder( trie_state, recorder.clone(), @@ -259,11 +260,6 @@ where ) }, None => { - let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state); - let runtime_code = - state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, at)?; - let mut state_machine = StateMachine::new( &state, changes_trie_state, @@ -309,7 +305,7 @@ where method: &str, call_data: &[u8], ) -> sp_blockchain::Result<(Vec, StorageProof)> { - let mut state = self.backend.state_at(*at)?; + let state = self.backend.state_at(*at)?; let trie_backend = state.as_trie_backend().ok_or_else(|| { Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof) diff --git a/substrate/primitives/state-machine/src/backend.rs b/substrate/primitives/state-machine/src/backend.rs index de4ff33b51..1b1a732f8d 100644 --- a/substrate/primitives/state-machine/src/backend.rs +++ b/substrate/primitives/state-machine/src/backend.rs @@ -173,7 +173,7 @@ pub trait Backend: sp_std::fmt::Debug { } /// Try convert into trie backend. - fn as_trie_backend(&mut self) -> Option<&TrieBackend> { + fn as_trie_backend(&self) -> Option<&TrieBackend> { None } diff --git a/substrate/primitives/state-machine/src/in_memory_backend.rs b/substrate/primitives/state-machine/src/in_memory_backend.rs index 4daf1004a8..3e75ff5126 100644 --- a/substrate/primitives/state-machine/src/in_memory_backend.rs +++ b/substrate/primitives/state-machine/src/in_memory_backend.rs @@ -175,7 +175,7 @@ mod tests { let storage = new_in_mem::(); let child_info = ChildInfo::new_default(b"1"); let child_info = &child_info; - let mut storage = storage + let storage = storage .update(vec![(Some(child_info.clone()), vec![(b"2".to_vec(), Some(b"3".to_vec()))])]); let trie_backend = storage.as_trie_backend().unwrap(); assert_eq!(trie_backend.child_storage(child_info, b"2").unwrap(), Some(b"3".to_vec())); diff --git a/substrate/primitives/state-machine/src/lib.rs b/substrate/primitives/state-machine/src/lib.rs index 3c4acdccb1..07d7e54530 100644 --- a/substrate/primitives/state-machine/src/lib.rs +++ b/substrate/primitives/state-machine/src/lib.rs @@ -719,7 +719,7 @@ mod execution { } /// Generate storage read proof. - pub fn prove_read(mut backend: B, keys: I) -> Result> + pub fn prove_read(backend: B, keys: I) -> Result> where B: Backend, H: Hasher, @@ -735,7 +735,7 @@ mod execution { /// Generate range storage read proof. pub fn prove_range_read_with_size( - mut backend: B, + backend: B, child_info: Option<&ChildInfo>, prefix: Option<&[u8]>, size_limit: usize, @@ -794,7 +794,7 @@ mod execution { /// Generate child storage read proof. pub fn prove_child_read( - mut backend: B, + backend: B, child_info: &ChildInfo, keys: I, ) -> Result> @@ -1197,7 +1197,7 @@ mod tests { b"abc".to_vec() => b"2".to_vec(), b"bbb".to_vec() => b"3".to_vec() ]; - let mut state = InMemoryBackend::::from(initial); + let state = InMemoryBackend::::from(initial); let backend = state.as_trie_backend().unwrap(); let mut overlay = OverlayedChanges::default(); @@ -1350,7 +1350,7 @@ mod tests { fn set_child_storage_works() { let child_info = ChildInfo::new_default(b"sub1"); let child_info = &child_info; - let mut state = new_in_mem::(); + let state = new_in_mem::(); let backend = state.as_trie_backend().unwrap(); let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); @@ -1372,7 +1372,7 @@ mod tests { fn append_storage_works() { let reference_data = vec![b"data1".to_vec(), b"2".to_vec(), b"D3".to_vec(), b"d4".to_vec()]; let key = b"key".to_vec(); - let mut state = new_in_mem::(); + let state = new_in_mem::(); let backend = state.as_trie_backend().unwrap(); let mut overlay = OverlayedChanges::default(); let mut cache = StorageTransactionCache::default(); @@ -1427,7 +1427,7 @@ mod tests { let key = b"events".to_vec(); let mut cache = StorageTransactionCache::default(); - let mut state = new_in_mem::(); + let state = new_in_mem::(); let backend = state.as_trie_backend().unwrap(); let mut overlay = OverlayedChanges::default(); @@ -1696,7 +1696,7 @@ mod tests { b"aaa".to_vec() => b"0".to_vec(), b"bbb".to_vec() => b"".to_vec() ]; - let mut state = InMemoryBackend::::from(initial); + let state = InMemoryBackend::::from(initial); let backend = state.as_trie_backend().unwrap(); let mut overlay = OverlayedChanges::default(); diff --git a/substrate/primitives/state-machine/src/proving_backend.rs b/substrate/primitives/state-machine/src/proving_backend.rs index 3a242313a6..690266dab1 100644 --- a/substrate/primitives/state-machine/src/proving_backend.rs +++ b/substrate/primitives/state-machine/src/proving_backend.rs @@ -433,7 +433,7 @@ mod tests { fn proof_recorded_and_checked() { let contents = (0..64).map(|i| (vec![i], Some(vec![i]))).collect::>(); let in_memory = InMemoryBackend::::default(); - let mut in_memory = in_memory.update(vec![(None, contents)]); + let in_memory = in_memory.update(vec![(None, contents)]); let in_memory_root = in_memory.storage_root(::std::iter::empty()).0; (0..64).for_each(|i| assert_eq!(in_memory.storage(&[i]).unwrap().unwrap(), vec![i])); @@ -464,7 +464,7 @@ mod tests { (Some(child_info_2.clone()), (10..15).map(|i| (vec![i], Some(vec![i]))).collect()), ]; let in_memory = InMemoryBackend::::default(); - let mut in_memory = in_memory.update(contents); + let in_memory = in_memory.update(contents); let child_storage_keys = vec![child_info_1.to_owned(), child_info_2.to_owned()]; let in_memory_root = in_memory .full_storage_root( diff --git a/substrate/primitives/state-machine/src/trie_backend.rs b/substrate/primitives/state-machine/src/trie_backend.rs index 9500765332..4cdf1d3b75 100644 --- a/substrate/primitives/state-machine/src/trie_backend.rs +++ b/substrate/primitives/state-machine/src/trie_backend.rs @@ -253,7 +253,7 @@ where (root, is_default, write_overlay) } - fn as_trie_backend(&mut self) -> Option<&TrieBackend> { + fn as_trie_backend(&self) -> Option<&TrieBackend> { Some(self) }