From 82a832bc3a7f833aac77727946340e8475420088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sat, 23 May 2020 11:28:34 +0200 Subject: [PATCH] Don't clone values when calculating storage root (#6108) * Don't clone values when calculating storage root Instead of cloning all the keys and values of the overlay when calculating the storage root, we pass all the values by reference. This should probably bring some performance improvements when calculating the storage root. * no cow version (#6113) Co-authored-by: cheme --- substrate/client/api/src/in_mem.rs | 17 +++-- substrate/client/db/src/bench.rs | 23 ++++--- substrate/client/db/src/lib.rs | 52 +++++++-------- substrate/client/db/src/storage_cache.rs | 34 ++++------ .../service/src/client/light/backend.rs | 31 ++++----- .../primitives/state-machine/src/backend.rs | 65 +++++++++---------- .../primitives/state-machine/src/basic.rs | 2 +- substrate/primitives/state-machine/src/ext.rs | 2 +- .../state-machine/src/overlayed_changes.rs | 24 ++++--- .../state-machine/src/proving_backend.rs | 32 ++++----- .../state-machine/src/trie_backend.rs | 36 +++++----- substrate/primitives/trie/src/lib.rs | 34 +++++----- 12 files changed, 165 insertions(+), 187 deletions(-) diff --git a/substrate/client/api/src/in_mem.rs b/substrate/client/api/src/in_mem.rs index 0eb0681a82..45c41fbcb7 100644 --- a/substrate/client/api/src/in_mem.rs +++ b/substrate/client/api/src/in_mem.rs @@ -21,9 +21,8 @@ use std::collections::HashMap; use std::sync::Arc; use parking_lot::RwLock; -use sp_core::storage::well_known_keys; -use sp_core::offchain::storage::{ - InMemOffchainStorage as OffchainStorage +use sp_core::{ + storage::well_known_keys, offchain::storage::InMemOffchainStorage as OffchainStorage, }; use sp_runtime::generic::BlockId; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, Zero, NumberFor, HashFor}; @@ -519,13 +518,17 @@ impl backend::BlockImportOperation for BlockImportOperatio fn reset_storage(&mut self, storage: Storage) -> sp_blockchain::Result { check_genesis_storage(&storage)?; - let child_delta = storage.children_default.into_iter() + let child_delta = storage.children_default.iter() .map(|(_storage_key, child_content)| - (child_content.child_info, child_content.data.into_iter().map(|(k, v)| (k, Some(v))))); + ( + &child_content.child_info, + child_content.data.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))) + ) + ); let (root, transaction) = self.old_state.full_storage_root( - storage.top.into_iter().map(|(k, v)| (k, Some(v))), - child_delta + storage.top.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))), + child_delta, ); self.new_state = Some(transaction); diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index 807e8c68e1..99ce1edae0 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -79,12 +79,12 @@ impl BenchmarkingState { }; state.reopen()?; - let child_delta = genesis.children_default.into_iter().map(|(_storage_key, child_content)| ( - child_content.child_info, - child_content.data.into_iter().map(|(k, v)| (k, Some(v))), + let child_delta = genesis.children_default.iter().map(|(_storage_key, child_content)| ( + &child_content.child_info, + child_content.data.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))), )); let (root, transaction): (B::Hash, _) = state.state.borrow_mut().as_mut().unwrap().full_storage_root( - genesis.top.into_iter().map(|(k, v)| (k, Some(v))), + genesis.top.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))), child_delta, ); state.genesis = transaction.clone().drain(); @@ -193,19 +193,18 @@ impl StateBackend> for BenchmarkingState { } } - fn storage_root(&self, delta: I) -> (B::Hash, Self::Transaction) where - I: IntoIterator, Option>)> - { + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (B::Hash, Self::Transaction) where B::Hash: Ord { self.state.borrow().as_ref().map_or(Default::default(), |s| s.storage_root(delta)) } - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (B::Hash, bool, Self::Transaction) where - I: IntoIterator, Option>)>, - { + delta: impl Iterator)>, + ) -> (B::Hash, bool, Self::Transaction) where B::Hash: Ord { self.state.borrow().as_ref().map_or(Default::default(), |s| s.child_storage_root(child_info, delta)) } diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 037409dfc4..9fb8f3c8c0 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -212,21 +212,18 @@ impl StateBackend> for RefTrackingState { self.state.for_child_keys_with_prefix(child_info, prefix, f) } - fn storage_root(&self, delta: I) -> (B::Hash, Self::Transaction) - where - I: IntoIterator, Option>)> - { + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (B::Hash, Self::Transaction) where B::Hash: Ord { self.state.storage_root(delta) } - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (B::Hash, bool, Self::Transaction) - where - I: IntoIterator, Option>)>, - { + delta: impl Iterator)>, + ) -> (B::Hash, bool, Self::Transaction) where B::Hash: Ord { self.state.child_storage_root(child_info, delta) } @@ -605,26 +602,25 @@ impl sc_client_api::backend::BlockImportOperation for Bloc &mut self, storage: Storage, ) -> ClientResult { - - if storage.top.iter().any(|(k, _)| well_known_keys::is_child_storage_key(k)) { + if storage.top.keys().any(|k| well_known_keys::is_child_storage_key(&k)) { return Err(sp_blockchain::Error::GenesisInvalid.into()); } - let child_delta = storage.children_default.into_iter().map(|(_storage_key, child_content)|( - child_content.child_info, - child_content.data.into_iter().map(|(k, v)| (k, Some(v))), + let child_delta = storage.children_default.iter().map(|(_storage_key, child_content)|( + &child_content.child_info, + child_content.data.iter().map(|(k, v)| (&k[..], Some(&v[..]))), )); let mut changes_trie_config: Option = None; let (root, transaction) = self.old_state.full_storage_root( - storage.top.into_iter().map(|(k, v)| { - if k == well_known_keys::CHANGES_TRIE_CONFIG { + storage.top.iter().map(|(k, v)| { + if &k[..] == well_known_keys::CHANGES_TRIE_CONFIG { changes_trie_config = Some( Decode::decode(&mut &v[..]) .expect("changes trie configuration is encoded properly at genesis") ); } - (k, Some(v)) + (&k[..], Some(&v[..])) }), child_delta ); @@ -1810,13 +1806,12 @@ pub(crate) mod tests { header.state_root = op.old_state.storage_root(storage .iter() - .cloned() - .map(|(x, y)| (x, Some(y))) + .map(|(x, y)| (&x[..], Some(&y[..]))) ).0.into(); let hash = header.hash(); op.reset_storage(Storage { - top: storage.iter().cloned().collect(), + top: storage.into_iter().collect(), children_default: Default::default(), }).unwrap(); op.set_block_data( @@ -1853,7 +1848,10 @@ pub(crate) mod tests { (vec![5, 5, 5], Some(vec![4, 5, 6])), ]; - let (root, overlay) = op.old_state.storage_root(storage.iter().cloned()); + let (root, overlay) = op.old_state.storage_root( + storage.iter() + .map(|(k, v)| (&k[..], v.as_ref().map(|v| &v[..]))) + ); op.update_db_storage(overlay).unwrap(); header.state_root = root.into(); @@ -1892,17 +1890,11 @@ pub(crate) mod tests { extrinsics_root: Default::default(), }; - let storage: Vec<(_, _)> = vec![]; - - header.state_root = op.old_state.storage_root(storage - .iter() - .cloned() - .map(|(x, y)| (x, Some(y))) - ).0.into(); + header.state_root = op.old_state.storage_root(std::iter::empty()).0.into(); let hash = header.hash(); op.reset_storage(Storage { - top: storage.iter().cloned().collect(), + top: Default::default(), children_default: Default::default(), }).unwrap(); diff --git a/substrate/client/db/src/storage_cache.rs b/substrate/client/db/src/storage_cache.rs index 66ac74afa4..434b301ed6 100644 --- a/substrate/client/db/src/storage_cache.rs +++ b/substrate/client/db/src/storage_cache.rs @@ -621,21 +621,18 @@ impl>, B: BlockT> StateBackend> for Cachin self.state.for_child_keys_with_prefix(child_info, prefix, f) } - fn storage_root(&self, delta: I) -> (B::Hash, Self::Transaction) - where - I: IntoIterator, Option>)>, - { + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (B::Hash, Self::Transaction) where B::Hash: Ord { self.state.storage_root(delta) } - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (B::Hash, bool, Self::Transaction) - where - I: IntoIterator, Option>)>, - { + delta: impl Iterator)>, + ) -> (B::Hash, bool, Self::Transaction) where B::Hash: Ord { self.state.child_storage_root(child_info, delta) } @@ -806,21 +803,18 @@ impl>, B: BlockT> StateBackend> for Syncin self.caching_state().for_child_keys_with_prefix(child_info, prefix, f) } - fn storage_root(&self, delta: I) -> (B::Hash, Self::Transaction) - where - I: IntoIterator, Option>)>, - { + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (B::Hash, Self::Transaction) where B::Hash: Ord { self.caching_state().storage_root(delta) } - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (B::Hash, bool, Self::Transaction) - where - I: IntoIterator, Option>)>, - { + delta: impl Iterator)>, + ) -> (B::Hash, bool, Self::Transaction) where B::Hash: Ord { self.caching_state().child_storage_root(child_info, delta) } diff --git a/substrate/client/service/src/client/light/backend.rs b/substrate/client/service/src/client/light/backend.rs index d336127131..2cf994d3f5 100644 --- a/substrate/client/service/src/client/light/backend.rs +++ b/substrate/client/service/src/client/light/backend.rs @@ -318,12 +318,12 @@ impl BlockImportOperation for ImportOperation storage.insert(None, input.top); // create a list of children keys to re-compute roots for - let child_delta = input.children_default.iter() - .map(|(_storage_key, storage_child)| (storage_child.child_info.clone(), None)) - .collect::>(); + let child_delta = input.children_default + .iter() + .map(|(_storage_key, storage_child)| (&storage_child.child_info, std::iter::empty())); // make sure to persist the child storage - for (_child_key, storage_child) in input.children_default { + for (_child_key, storage_child) in input.children_default.clone() { storage.insert(Some(storage_child.child_info), storage_child.data); } @@ -350,7 +350,11 @@ impl BlockImportOperation for ImportOperation Ok(()) } - fn mark_finalized(&mut self, block: BlockId, _justification: Option) -> ClientResult<()> { + fn mark_finalized( + &mut self, + block: BlockId, + _justification: Option, + ) -> ClientResult<()> { self.finalized_blocks.push(block); Ok(()) } @@ -459,10 +463,10 @@ impl StateBackend for GenesisOrUnavailableState } } - fn storage_root(&self, delta: I) -> (H::Out, Self::Transaction) - where - I: IntoIterator, Option>)> - { + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (H::Out, Self::Transaction) where H::Out: Ord { match *self { GenesisOrUnavailableState::Genesis(ref state) => state.storage_root(delta), @@ -470,14 +474,11 @@ impl StateBackend for GenesisOrUnavailableState } } - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (H::Out, bool, Self::Transaction) - where - I: IntoIterator, Option>)> - { + delta: impl Iterator)>, + ) -> (H::Out, bool, Self::Transaction) where H::Out: Ord { match *self { GenesisOrUnavailableState::Genesis(ref state) => { let (root, is_equal, _) = state.child_storage_root(child_info, delta); diff --git a/substrate/primitives/state-machine/src/backend.rs b/substrate/primitives/state-machine/src/backend.rs index f689357eb9..20a3ab7500 100644 --- a/substrate/primitives/state-machine/src/backend.rs +++ b/substrate/primitives/state-machine/src/backend.rs @@ -20,7 +20,6 @@ use hash_db::Hasher; use codec::{Decode, Encode}; use sp_core::{traits::RuntimeCode, storage::{ChildInfo, well_known_keys}}; - use crate::{ trie_backend::TrieBackend, trie_backend_essence::TrieBackendStorage, @@ -119,22 +118,19 @@ pub trait Backend: std::fmt::Debug { /// Calculate the storage root, with given delta over what is already stored in /// the backend, and produce a "transaction" that can be used to commit. /// Does not include child storage updates. - fn storage_root(&self, delta: I) -> (H::Out, Self::Transaction) - where - I: IntoIterator)>, - H::Out: Ord; + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (H::Out, Self::Transaction) where H::Out: Ord; /// Calculate the child storage root, with given delta over what is already stored in /// the backend, and produce a "transaction" that can be used to commit. The second argument /// is true if child storage root equals default storage root. - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (H::Out, bool, Self::Transaction) - where - I: IntoIterator)>, - H::Out: Ord; + delta: impl Iterator)>, + ) -> (H::Out, bool, Self::Transaction) where H::Out: Ord; /// Get all key/value pairs into a Vec. fn pairs(&self) -> Vec<(StorageKey, StorageValue)>; @@ -165,17 +161,14 @@ pub trait Backend: std::fmt::Debug { /// Calculate the storage root, with given delta over what is already stored /// in the backend, and produce a "transaction" that can be used to commit. /// Does include child storage updates. - fn full_storage_root( + fn full_storage_root<'a>( &self, - delta: I1, - child_deltas: I2) - -> (H::Out, Self::Transaction) - where - I1: IntoIterator)>, - I2i: IntoIterator)>, - I2: IntoIterator, - H::Out: Ord + Encode, - { + delta: impl Iterator)>, + child_deltas: impl Iterator)>, + )>, + ) -> (H::Out, Self::Transaction) where H::Out: Ord + Encode { let mut txs: Self::Transaction = Default::default(); let mut child_roots: Vec<_> = Default::default(); // child first @@ -190,8 +183,13 @@ pub trait Backend: std::fmt::Debug { child_roots.push((prefixed_storage_key.into_inner(), Some(child_root.encode()))); } } - let (root, parent_txs) = self.storage_root( - delta.into_iter().chain(child_roots.into_iter()) + let (root, parent_txs) = self.storage_root(delta + .map(|(k, v)| (&k[..], v.as_ref().map(|v| &v[..]))) + .chain( + child_roots + .iter() + .map(|(k, v)| (&k[..], v.as_ref().map(|v| &v[..]))) + ) ); txs.consolidate(parent_txs); (root, txs) @@ -214,7 +212,7 @@ pub trait Backend: std::fmt::Debug { } /// Commit given transaction to storage. - fn commit(&self, _storage_root: H::Out, _transaction: Self::Transaction) -> Result<(), Self::Error> { + fn commit(&self, _: H::Out, _: Self::Transaction) -> Result<(), Self::Error> { unimplemented!() } } @@ -269,23 +267,18 @@ impl<'a, T: Backend, H: Hasher> Backend for &'a T { (*self).for_child_keys_with_prefix(child_info, prefix, f) } - fn storage_root(&self, delta: I) -> (H::Out, Self::Transaction) - where - I: IntoIterator)>, - H::Out: Ord, - { + fn storage_root<'b>( + &self, + delta: impl Iterator)>, + ) -> (H::Out, Self::Transaction) where H::Out: Ord { (*self).storage_root(delta) } - fn child_storage_root( + fn child_storage_root<'b>( &self, child_info: &ChildInfo, - delta: I, - ) -> (H::Out, bool, Self::Transaction) - where - I: IntoIterator)>, - H::Out: Ord, - { + delta: impl Iterator)>, + ) -> (H::Out, bool, Self::Transaction) where H::Out: Ord { (*self).child_storage_root(child_info, delta) } diff --git a/substrate/primitives/state-machine/src/basic.rs b/substrate/primitives/state-machine/src/basic.rs index 7aa75cec70..917e41f33d 100644 --- a/substrate/primitives/state-machine/src/basic.rs +++ b/substrate/primitives/state-machine/src/basic.rs @@ -295,7 +295,7 @@ impl Externalities for BasicExternalities { child_info: &ChildInfo, ) -> Vec { if let Some(child) = self.inner.children_default.get(child_info.storage_key()) { - let delta = child.data.clone().into_iter().map(|(k, v)| (k, Some(v))); + let delta = child.data.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))); crate::in_memory_backend::new_in_mem::() .child_storage_root(&child.child_info, delta).0 } else { diff --git a/substrate/primitives/state-machine/src/ext.rs b/substrate/primitives/state-machine/src/ext.rs index 25c20644f7..7e805250e7 100644 --- a/substrate/primitives/state-machine/src/ext.rs +++ b/substrate/primitives/state-machine/src/ext.rs @@ -481,7 +481,7 @@ where if let Some(child_info) = self.overlay.default_child_info(storage_key) { let (root, is_empty, _) = { let delta = self.overlay.changes(Some(child_info)) - .map(|(k, v)| (k.clone(), v.value().cloned())); + .map(|(k, v)| (k.as_ref(), v.value().map(AsRef::as_ref))); self.backend.child_storage_root(child_info, delta) }; diff --git a/substrate/primitives/state-machine/src/overlayed_changes.rs b/substrate/primitives/state-machine/src/overlayed_changes.rs index 2da063c96e..b0259c2b85 100644 --- a/substrate/primitives/state-machine/src/overlayed_changes.rs +++ b/substrate/primitives/state-machine/src/overlayed_changes.rs @@ -26,13 +26,10 @@ use crate::{ stats::StateMachineStats, }; -#[cfg(test)] -use std::iter::FromIterator; -use std::collections::{HashMap, BTreeMap, BTreeSet}; +use std::{mem, ops, collections::{HashMap, BTreeMap, BTreeSet}}; use codec::{Decode, Encode}; use sp_core::storage::{well_known_keys::EXTRINSIC_INDEX, ChildInfo, ChildType}; use sp_core::offchain::storage::OffchainOverlayedChanges; -use std::{mem, ops}; use hash_db::Hasher; @@ -178,7 +175,7 @@ impl Default for StorageChanges } #[cfg(test)] -impl FromIterator<(StorageKey, OverlayedValue)> for OverlayedChangeSet { +impl std::iter::FromIterator<(StorageKey, OverlayedValue)> for OverlayedChangeSet { fn from_iter>(iter: T) -> Self { Self { top: iter.into_iter().collect(), @@ -646,22 +643,29 @@ impl OverlayedChanges { .chain(self.committed.children_default.keys()); let child_delta_iter = child_storage_keys.map(|storage_key| ( - self.default_child_info(storage_key).cloned() + self.default_child_info(storage_key) .expect("child info initialized in either committed or prospective"), self.committed.children_default.get(storage_key) .into_iter() - .flat_map(|(map, _)| map.iter().map(|(k, v)| (k.clone(), v.value.clone()))) + .flat_map(|(map, _)| + map.iter().map(|(k, v)| (&k[..], v.value().map(|v| &v[..]))) + ) .chain( self.prospective.children_default.get(storage_key) .into_iter() - .flat_map(|(map, _)| map.iter().map(|(k, v)| (k.clone(), v.value.clone()))) + .flat_map(|(map, _)| + map.iter().map(|(k, v)| (&k[..], v.value().map(|v| &v[..]))) + ) ), ) ); // compute and memoize - let delta = self.committed.top.iter().map(|(k, v)| (k.clone(), v.value.clone())) - .chain(self.prospective.top.iter().map(|(k, v)| (k.clone(), v.value.clone()))); + let delta = self.committed + .top + .iter() + .map(|(k, v)| (&k[..], v.value().map(|v| &v[..]))) + .chain(self.prospective.top.iter().map(|(k, v)| (&k[..], v.value().map(|v| &v[..])))); let (root, transaction) = backend.full_storage_root(delta, child_delta_iter); diff --git a/substrate/primitives/state-machine/src/proving_backend.rs b/substrate/primitives/state-machine/src/proving_backend.rs index 1d38d578a2..1f25005bc3 100644 --- a/substrate/primitives/state-machine/src/proving_backend.rs +++ b/substrate/primitives/state-machine/src/proving_backend.rs @@ -17,7 +17,7 @@ //! Proving state machine backend. -use std::sync::Arc; +use std::{sync::Arc, collections::HashMap}; use parking_lot::RwLock; use codec::{Decode, Codec}; use log::debug; @@ -26,13 +26,10 @@ use sp_trie::{ MemoryDB, empty_child_trie_root, read_trie_value_with, read_child_trie_value_with, record_all_keys, StorageProof, }; -pub use sp_trie::Recorder; -pub use sp_trie::trie_types::{Layout, TrieError}; +pub use sp_trie::{Recorder, trie_types::{Layout, TrieError}}; use crate::trie_backend::TrieBackend; use crate::trie_backend_essence::{Ephemeral, TrieBackendEssence, TrieBackendStorage}; -use crate::{Error, ExecutionError, Backend}; -use std::collections::HashMap; -use crate::DBValue; +use crate::{Error, ExecutionError, Backend, DBValue}; use sp_core::storage::ChildInfo; /// Patricia trie-based backend specialized in get value proofs. @@ -260,21 +257,18 @@ impl<'a, S, H> Backend for ProvingBackend<'a, S, H> self.0.child_keys(child_info, prefix) } - fn storage_root(&self, delta: I) -> (H::Out, Self::Transaction) - where I: IntoIterator, Option>)> - { + fn storage_root<'b>( + &self, + delta: impl Iterator)>, + ) -> (H::Out, Self::Transaction) where H::Out: Ord { self.0.storage_root(delta) } - fn child_storage_root( + fn child_storage_root<'b>( &self, child_info: &ChildInfo, - delta: I, - ) -> (H::Out, bool, Self::Transaction) - where - I: IntoIterator, Option>)>, - H::Out: Ord - { + delta: impl Iterator)>, + ) -> (H::Out, bool, Self::Transaction) where H::Out: Ord { self.0.child_storage_root(child_info, delta) } @@ -393,9 +387,9 @@ mod tests { let in_memory = InMemoryBackend::::default(); let mut 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::<_, Vec<_>, _>( - ::std::iter::empty(), - child_storage_keys.into_iter().map(|k|(k.to_owned(), Vec::new())) + let in_memory_root = in_memory.full_storage_root( + std::iter::empty(), + child_storage_keys.iter().map(|k|(k, std::iter::empty())) ).0; (0..64).for_each(|i| assert_eq!( in_memory.storage(&[i]).unwrap().unwrap(), diff --git a/substrate/primitives/state-machine/src/trie_backend.rs b/substrate/primitives/state-machine/src/trie_backend.rs index 3016647191..2d4ab782cb 100644 --- a/substrate/primitives/state-machine/src/trie_backend.rs +++ b/substrate/primitives/state-machine/src/trie_backend.rs @@ -167,9 +167,10 @@ impl, H: Hasher> Backend for TrieBackend where collect_all().map_err(|e| debug!(target: "trie", "Error extracting trie keys: {}", e)).unwrap_or_default() } - fn storage_root(&self, delta: I) -> (H::Out, S::Overlay) - where I: IntoIterator)> - { + fn storage_root<'a>( + &self, + delta: impl Iterator)>, + ) -> (H::Out, Self::Transaction) where H::Out: Ord { let mut write_overlay = S::Overlay::default(); let mut root = *self.essence.root(); @@ -179,8 +180,7 @@ impl, H: Hasher> Backend for TrieBackend where &mut write_overlay, ); - let delta: Vec<_> = delta.into_iter().collect(); - match delta_trie_root::, _, _, _, _>(&mut eph, root, delta) { + match delta_trie_root::, _, _, _, _, _>(&mut eph, root, delta) { Ok(ret) => root = ret, Err(e) => warn!(target: "trie", "Failed to write to trie: {}", e), } @@ -189,15 +189,11 @@ impl, H: Hasher> Backend for TrieBackend where (root, write_overlay) } - fn child_storage_root( + fn child_storage_root<'a>( &self, child_info: &ChildInfo, - delta: I, - ) -> (H::Out, bool, Self::Transaction) - where - I: IntoIterator)>, - H::Out: Ord, - { + delta: impl Iterator)>, + ) -> (H::Out, bool, Self::Transaction) where H::Out: Ord { let default_root = match child_info.child_type() { ChildType::ParentKeyId => empty_child_trie_root::>() }; @@ -219,11 +215,11 @@ impl, H: Hasher> Backend for TrieBackend where &mut write_overlay, ); - match child_delta_trie_root::, _, _, _, _, _>( + match child_delta_trie_root::, _, _, _, _, _, _>( child_info.keyspace(), &mut eph, root, - delta + delta, ) { Ok(ret) => root = ret, Err(e) => warn!(target: "trie", "Failed to write to trie: {}", e), @@ -252,7 +248,7 @@ impl, H: Hasher> Backend for TrieBackend where #[cfg(test)] pub mod tests { - use std::collections::HashSet; + use std::{collections::HashSet, iter}; use sp_core::H256; use codec::Encode; use sp_trie::{TrieMut, PrefixedMemoryDB, trie_types::TrieDBMut, KeySpacedDBMut}; @@ -328,19 +324,21 @@ pub mod tests { #[test] fn storage_root_is_non_default() { - assert!(test_trie().storage_root(::std::iter::empty()).0 != H256::repeat_byte(0)); + assert!(test_trie().storage_root(iter::empty()).0 != H256::repeat_byte(0)); } #[test] fn storage_root_transaction_is_empty() { - assert!(test_trie().storage_root(::std::iter::empty()).1.drain().is_empty()); + assert!(test_trie().storage_root(iter::empty()).1.drain().is_empty()); } #[test] fn storage_root_transaction_is_non_empty() { - let (new_root, mut tx) = test_trie().storage_root(vec![(b"new-key".to_vec(), Some(b"new-value".to_vec()))]); + let (new_root, mut tx) = test_trie().storage_root( + iter::once((&b"new-key"[..], Some(&b"new-value"[..]))), + ); assert!(!tx.drain().is_empty()); - assert!(new_root != test_trie().storage_root(::std::iter::empty()).0); + assert!(new_root != test_trie().storage_root(iter::empty()).0); } #[test] diff --git a/substrate/primitives/trie/src/lib.rs b/substrate/primitives/trie/src/lib.rs index f328b71750..db471fd713 100644 --- a/substrate/primitives/trie/src/lib.rs +++ b/substrate/primitives/trie/src/lib.rs @@ -24,9 +24,7 @@ mod node_codec; mod storage_proof; mod trie_stream; -use sp_std::boxed::Box; -use sp_std::marker::PhantomData; -use sp_std::vec::Vec; +use sp_std::{boxed::Box, marker::PhantomData, vec::Vec, borrow::Borrow}; use hash_db::{Hasher, Prefix}; use trie_db::proof::{generate_proof, verify_proof}; pub use trie_db::proof::VerifyError; @@ -162,23 +160,24 @@ pub fn verify_trie_proof<'a, L: TrieConfiguration, I, K, V>( } /// Determine a trie root given a hash DB and delta values. -pub fn delta_trie_root( +pub fn delta_trie_root( db: &mut DB, mut root: TrieHash, delta: I ) -> Result, Box>> where - I: IntoIterator)>, - A: AsRef<[u8]> + Ord, - B: AsRef<[u8]>, + I: IntoIterator, + A: Borrow<[u8]>, + B: Borrow>, + V: Borrow<[u8]>, DB: hash_db::HashDB, { { let mut trie = TrieDBMut::::from_existing(&mut *db, &mut root)?; for (key, change) in delta { - match change { - Some(val) => trie.insert(key.as_ref(), val.as_ref())?, - None => trie.remove(key.as_ref())?, + match change.borrow() { + Some(val) => trie.insert(key.borrow(), val.borrow())?, + None => trie.remove(key.borrow())?, }; } } @@ -230,16 +229,17 @@ pub fn child_trie_root( /// Determine a child trie root given a hash DB and delta values. H is the default hasher, /// but a generic implementation may ignore this type parameter and use other hashers. -pub fn child_delta_trie_root( +pub fn child_delta_trie_root( keyspace: &[u8], db: &mut DB, root_data: RD, delta: I, ) -> Result<::Out, Box>> where - I: IntoIterator)>, - A: AsRef<[u8]> + Ord, - B: AsRef<[u8]>, + I: IntoIterator, + A: Borrow<[u8]>, + B: Borrow>, + V: Borrow<[u8]>, RD: AsRef<[u8]>, DB: hash_db::HashDB { @@ -252,9 +252,9 @@ pub fn child_delta_trie_root( let mut trie = TrieDBMut::::from_existing(&mut db, &mut root)?; for (key, change) in delta { - match change { - Some(val) => trie.insert(key.as_ref(), val.as_ref())?, - None => trie.remove(key.as_ref())?, + match change.borrow() { + Some(val) => trie.insert(key.borrow(), val.borrow())?, + None => trie.remove(key.borrow())?, }; } }