From 76efb19491fad5fd7dbd6bab913aa6249c5b40ef Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 20 Mar 2020 15:51:55 +0100 Subject: [PATCH 1/7] implement DoubleMap --- frame/support/procedural/src/lib.rs | 2 - frame/support/procedural/src/storage/mod.rs | 1 - .../procedural/src/storage/storage_struct.rs | 24 - frame/support/src/hash.rs | 113 ++++- frame/support/src/lib.rs | 27 +- .../src/storage/generator/double_map.rs | 196 ++++++--- frame/support/src/storage/generator/map.rs | 241 +++++----- frame/support/src/storage/generator/mod.rs | 8 +- frame/support/src/storage/mod.rs | 414 +++++++++--------- frame/support/test/tests/final_keys.rs | 14 +- 10 files changed, 587 insertions(+), 453 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index a9662f530a598..78c933f9ec782 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -67,7 +67,6 @@ use proc_macro::TokenStream; /// * Map: `Foo: map hasher($hash) type => type`: Implements the /// [`StorageMap`](../frame_support/storage/trait.StorageMap.html) trait using the /// [`StorageMap generator`](../frame_support/storage/generator/trait.StorageMap.html). -/// And [`StoragePrefixedMap`](../frame_support/storage/trait.StoragePrefixedMap.html). /// /// `$hash` representing a choice of hashing algorithms available in the /// [`Hashable`](../frame_support/trait.Hashable.html) trait. You will generally want to use one @@ -106,7 +105,6 @@ use proc_macro::TokenStream; /// * Double map: `Foo: double_map hasher($hash1) u32, hasher($hash2) u32 => u32`: Implements the /// [`StorageDoubleMap`](../frame_support/storage/trait.StorageDoubleMap.html) trait using the /// [`StorageDoubleMap generator`](../frame_support/storage/generator/trait.StorageDoubleMap.html). -/// And [`StoragePrefixedMap`](../frame_support/storage/trait.StoragePrefixedMap.html). /// /// `$hash1` and `$hash2` representing choices of hashing algorithms available in the /// [`Hashable`](../frame_support/trait.Hashable.html) trait. They must be chosen with care, see diff --git a/frame/support/procedural/src/storage/mod.rs b/frame/support/procedural/src/storage/mod.rs index e8599c52a9071..c94b05de2f20c 100644 --- a/frame/support/procedural/src/storage/mod.rs +++ b/frame/support/procedural/src/storage/mod.rs @@ -414,7 +414,6 @@ pub fn decl_storage_impl(input: proc_macro::TokenStream) -> proc_macro::TokenStr StorageValue as _, StorageMap as _, StorageDoubleMap as _, - StoragePrefixedMap as _, }; #scrate_decl diff --git a/frame/support/procedural/src/storage/storage_struct.rs b/frame/support/procedural/src/storage/storage_struct.rs index cbd477354e820..0c7008c8e06b6 100644 --- a/frame/support/procedural/src/storage/storage_struct.rs +++ b/frame/support/procedural/src/storage/storage_struct.rs @@ -122,18 +122,6 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre StorageLineTypeDef::Map(map) => { let hasher = map.hasher.to_storage_hasher_struct(); quote!( - impl<#impl_trait> #scrate::storage::StoragePrefixedMap<#value_type> - for #storage_struct #optional_storage_where_clause - { - fn module_prefix() -> &'static [u8] { - #instance_or_inherent::PREFIX.as_bytes() - } - - fn storage_prefix() -> &'static [u8] { - #storage_name_str.as_bytes() - } - } - impl<#impl_trait> #scrate::#storage_generator_trait for #storage_struct #optional_storage_where_clause { @@ -162,18 +150,6 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre let hasher1 = map.hasher1.to_storage_hasher_struct(); let hasher2 = map.hasher2.to_storage_hasher_struct(); quote!( - impl<#impl_trait> #scrate::storage::StoragePrefixedMap<#value_type> - for #storage_struct #optional_storage_where_clause - { - fn module_prefix() -> &'static [u8] { - #instance_or_inherent::PREFIX.as_bytes() - } - - fn storage_prefix() -> &'static [u8] { - #storage_name_str.as_bytes() - } - } - impl<#impl_trait> #scrate::#storage_generator_trait for #storage_struct #optional_storage_where_clause { diff --git a/frame/support/src/hash.rs b/frame/support/src/hash.rs index f8d6409060f19..10b1fa0bd8ac0 100644 --- a/frame/support/src/hash.rs +++ b/frame/support/src/hash.rs @@ -16,7 +16,7 @@ //! Hash utilities. -use codec::Codec; +use codec::{Codec, Decode}; use sp_std::prelude::Vec; use sp_io::hashing::{blake2_128, blake2_256, twox_64, twox_128, twox_256}; @@ -64,6 +64,20 @@ pub trait ReversibleStorageHasher: StorageHasher { fn reverse(x: &[u8]) -> &[u8]; } +/// Trait to retrieve some info from hash of type `Key` encoded. +pub trait StorageHasherInfo { + /// Some info contained in the hash of type `Key` encoded. + type Info; + + /// Decode the hash and then decode the info from the decoded hash. + /// + /// # WARNING + /// + /// Even if info is (), input must be modified to have read the entire encoded hash. + fn decode_hash_and_then_info(input: &mut I) + -> Result; +} + /// Store the key directly. pub struct Identity; impl StorageHasher for Identity { @@ -77,6 +91,14 @@ impl ReversibleStorageHasher for Identity { x } } +impl StorageHasherInfo for Identity { + type Info = Key; + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + Key::decode(input) + } +} /// Hash storage keys with `concat(twox64(key), key)` pub struct Twox64Concat; @@ -95,6 +117,15 @@ impl ReversibleStorageHasher for Twox64Concat { &x[8..] } } +impl StorageHasherInfo for Twox64Concat { + type Info = Key; + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + input.read(&mut [0u8; 8])?; + Key::decode(input) + } +} /// Hash storage keys with `concat(blake2_128(key), key)` pub struct Blake2_128Concat; @@ -113,6 +144,15 @@ impl ReversibleStorageHasher for Blake2_128Concat { &x[16..] } } +impl StorageHasherInfo for Blake2_128Concat { + type Info = Key; + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + input.read(&mut [0u8; 16])?; + Key::decode(input) + } +} /// Hash storage keys with blake2 128 pub struct Blake2_128; @@ -122,6 +162,15 @@ impl StorageHasher for Blake2_128 { blake2_128(x) } } +impl StorageHasherInfo for Blake2_128 { + type Info = (); + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + input.read(&mut [0u8; 16])?; + Ok(()) + } +} /// Hash storage keys with blake2 256 pub struct Blake2_256; @@ -131,6 +180,15 @@ impl StorageHasher for Blake2_256 { blake2_256(x) } } +impl StorageHasherInfo for Blake2_256 { + type Info = (); + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + input.read(&mut [0u8; 32])?; + Ok(()) + } +} /// Hash storage keys with twox 128 pub struct Twox128; @@ -140,6 +198,15 @@ impl StorageHasher for Twox128 { twox_128(x) } } +impl StorageHasherInfo for Twox128 { + type Info = (); + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + input.read(&mut [0u8; 16])?; + Ok(()) + } +} /// Hash storage keys with twox 256 pub struct Twox256; @@ -149,10 +216,20 @@ impl StorageHasher for Twox256 { twox_256(x) } } +impl StorageHasherInfo for Twox256 { + type Info = (); + fn decode_hash_and_then_info(input: &mut I) + -> Result + { + input.read(&mut [0u8; 32])?; + Ok(()) + } +} #[cfg(test)] mod tests { use super::*; + use codec::Encode; #[test] fn test_twox_64_concat() { @@ -165,4 +242,38 @@ mod tests { let r = Blake2_128Concat::hash(b"foo"); assert_eq!(r.split_at(16), (&blake2_128(b"foo")[..], &b"foo"[..])) } + + #[test] + fn test_storage_hasher_info() { + type KeyType = [u8; 15]; + let key: KeyType = [3u8; 15]; + + let mut r = &Identity::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(key)); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + + let mut r = &Twox64Concat::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(key)); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + + let mut r = &Twox128::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + + let mut r = &Twox256::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + + let mut r = &Blake2_128Concat::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(key)); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + + let mut r = &Blake2_128::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + + let mut r = &Blake2_256::hash(&(&key).encode()[..])[..]; + assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); + assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. + } } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 8fe32cbda9999..5960bfb9b5ac2 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -70,10 +70,7 @@ pub use self::hash::{ Twox256, Twox128, Blake2_256, Blake2_128, Identity, Twox64Concat, Blake2_128Concat, Hashable, StorageHasher }; -pub use self::storage::{ - StorageValue, StorageMap, StorageDoubleMap, StoragePrefixedMap, IterableStorageMap, - IterableStorageDoubleMap, -}; +pub use self::storage::{StorageValue, StorageMap, StorageDoubleMap}; pub use self::dispatch::{Parameter, Callable, IsSubType}; pub use sp_runtime::{self, ConsensusEngineId, print, traits::Printable}; @@ -313,7 +310,7 @@ mod tests { OptionLinkedMap::insert(2, 2); OptionLinkedMap::insert(3, 3); - let collect = || OptionLinkedMap::iter().collect::>().sorted(); + let collect = || OptionLinkedMap::iter_key_value().collect::>().sorted(); assert_eq!(collect(), vec![(0, 0), (1, 1), (2, 2), (3, 3)]); // Two existing @@ -393,43 +390,43 @@ mod tests { #[test] fn map_iteration_should_work() { new_test_ext().execute_with(|| { - assert_eq!(Map::iter().collect::>().sorted(), vec![(15, 42)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(15, 42)]); // insert / remove let key = 17u32; Map::insert(key, 4u64); - assert_eq!(Map::iter().collect::>().sorted(), vec![(15, 42), (key, 4)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(15, 42), (key, 4)]); assert_eq!(Map::take(&15), 42u64); assert_eq!(Map::take(&key), 4u64); - assert_eq!(Map::iter().collect::>().sorted(), vec![]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![]); // Add couple of more elements Map::insert(key, 42u64); - assert_eq!(Map::iter().collect::>().sorted(), vec![(key, 42)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key, 42)]); Map::insert(key + 1, 43u64); - assert_eq!(Map::iter().collect::>().sorted(), vec![(key, 42), (key + 1, 43)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key, 42), (key + 1, 43)]); // mutate let key = key + 2; Map::mutate(&key, |val| { *val = 15; }); - assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 15)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 15)]); Map::mutate(&key, |val| { *val = 17; }); - assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 17)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 17)]); // remove first Map::remove(&key); - assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43)]); // remove last from the list Map::remove(&(key - 2)); - assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 1, 43)]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 1, 43)]); // remove the last element Map::remove(&(key - 1)); - assert_eq!(Map::iter().collect::>().sorted(), vec![]); + assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![]); }); } diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 9d05ff0b2d6d9..00ab605af29be 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -16,9 +16,9 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; -use codec::{Ref, FullCodec, FullEncode, Decode, Encode, EncodeLike, EncodeAppend}; +use codec::{Ref, FullCodec, Decode, Encode, EncodeLike, EncodeAppend}; use crate::{storage::{self, unhashed}, traits::Len}; -use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; +use crate::hash::{StorageHasher, Twox128, StorageHasherInfo}; /// Generator for `StorageDoubleMap` used by `decl_storage`. /// @@ -40,15 +40,15 @@ use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// If the key2s are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as /// `blake2_256` must be used for Hasher2. Otherwise, other items in storage with the same first /// key can be compromised. -pub trait StorageDoubleMap { +pub trait StorageDoubleMap { /// The type that get/take returns. type Query; /// Hasher for the first key. - type Hasher1: StorageHasher; + type Hasher1: StorageHasher + StorageHasherInfo; /// Hasher for the second key. - type Hasher2: StorageHasher; + type Hasher2: StorageHasher + StorageHasherInfo; /// Module prefix. Used for generating final key. fn module_prefix() -> &'static [u8]; @@ -124,12 +124,14 @@ pub trait StorageDoubleMap { } impl storage::StorageDoubleMap for G where - K1: FullEncode, - K2: FullEncode, + K1: FullCodec, + K2: FullCodec, V: FullCodec, G: StorageDoubleMap, { type Query = G::Query; + type Hasher1 = G::Hasher1; + type Hasher2 = G::Hasher2; fn hashed_key_for(k1: KArg1, k2: KArg2) -> Vec where KArg1: EncodeLike, @@ -208,14 +210,19 @@ impl storage::StorageDoubleMap for G where unhashed::kill_prefix(Self::storage_double_map_final_key1(k1).as_ref()) } - fn iter_prefix(k1: KArg1) -> storage::PrefixIterator where + fn remove_all() { + sp_io::storage::clear_prefix(&Self::prefix_hash()) + } + + fn iter_prefix_value(k1: KArg1) -> storage::PrefixIterator where KArg1: ?Sized + EncodeLike { let prefix = Self::storage_double_map_final_key1(k1); - storage::PrefixIterator:: { + storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, - phantom_data: Default::default(), + drain: false, + closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), } } @@ -332,80 +339,90 @@ impl storage::StorageDoubleMap for G where value }) } -} -/// Utility to iterate through items in a storage map. -pub struct MapIterator { - prefix: Vec, - previous_key: Vec, - drain: bool, - _phantom: ::sp_std::marker::PhantomData<(K, V, Hasher)>, -} + fn iter_prefix_key2_value(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where + Self::Hasher2: StorageHasherInfo, + { + let prefix = G::storage_double_map_final_key1(k1); + storage::PrefixIterator { + prefix: prefix.clone(), + previous_key: prefix, + drain: false, + closure: |raw_key, raw_value| { + from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) + .map(|(_k1, k2, v)| (k2, v)) + } + } + } -impl< - K: Decode + Sized, - V: Decode + Sized, - Hasher: ReversibleStorageHasher -> Iterator for MapIterator { - type Item = (K, V); + fn drain_prefix_key2_value(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where + Self::Hasher2: StorageHasherInfo, + { + let prefix = G::storage_double_map_final_key1(k1); + storage::PrefixIterator { + prefix: prefix.clone(), + previous_key: prefix, + drain: true, + closure: |raw_key, raw_value| { + from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) + .map(|(_k1, k2, v)| (k2, v)) + } + } + } - fn next(&mut self) -> Option<(K, V)> { - loop { - let maybe_next = sp_io::storage::next_key(&self.previous_key) - .filter(|n| n.starts_with(&self.prefix)); - break match maybe_next { - Some(next) => { - self.previous_key = next; - match unhashed::get::(&self.previous_key) { - Some(value) => { - if self.drain { - unhashed::kill(&self.previous_key) - } - let mut key_material = Hasher::reverse(&self.previous_key[self.prefix.len()..]); - match K::decode(&mut key_material) { - Ok(key) => Some((key, value)), - Err(_) => continue, - } - } - None => continue, - } - } - None => None, + fn iter_key1_value() -> storage::PrefixIterator<(K1, V)> where + Self::Hasher1: StorageHasherInfo, + { + let prefix = G::prefix_hash(); + storage::PrefixIterator { + prefix: prefix.clone(), + previous_key: prefix, + drain: false, + closure: |raw_key, raw_value| { + from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) + .map(|(k1, _k2, v)| (k1, v)) } } } -} -impl< - K1: FullCodec, - K2: FullCodec, - V: FullCodec, - G: StorageDoubleMap, -> storage::IterableStorageDoubleMap for G where - G::Hasher1: ReversibleStorageHasher, - G::Hasher2: ReversibleStorageHasher -{ - type Iterator = MapIterator; + fn iter_key2_value() -> storage::PrefixIterator<(K2, V)> where + Self::Hasher2: StorageHasherInfo, + { + let prefix = G::prefix_hash(); + storage::PrefixIterator { + prefix: prefix.clone(), + previous_key: prefix, + drain: false, + closure: |raw_key, raw_value| { + from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) + .map(|(_k1, k2, v)| (k2, v)) + } + } + } - /// Enumerate all elements in the map. - fn iter(k1: impl EncodeLike) -> Self::Iterator { - let prefix = G::storage_double_map_final_key1(k1); - Self::Iterator { + fn iter_key1_key2_value() -> storage::PrefixIterator<(K1, K2, V)> where + Self::Hasher1: StorageHasherInfo, + Self::Hasher2: StorageHasherInfo, + { + let prefix = G::prefix_hash(); + storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - _phantom: Default::default(), + closure: |raw_key, raw_value| { + from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) + .map(|(k1, k2, v)| (k1, k2, v)) + } } } - /// Enumerate all elements in the map. - fn drain(k1: impl EncodeLike) -> Self::Iterator { - let prefix = G::storage_double_map_final_key1(k1); - Self::Iterator { + fn iter_value() -> storage::PrefixIterator { + let prefix = G::prefix_hash(); + storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, - drain: true, - _phantom: Default::default(), + drain: false, + closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), } } @@ -422,7 +439,13 @@ impl< Some(new) => unhashed::put::(&previous_key, &new), None => unhashed::kill(&previous_key), }, - None => continue, + None => { + crate::debug::error!( + "next_key returned a key which failed to decode at {:?}", + previous_key + ); + continue + } } } None => return, @@ -431,6 +454,37 @@ impl< } } +fn from_raw_to_key_info_value( + raw_key: &[u8], + mut raw_value: &[u8] +) -> + Result< + ( + >::Info, + >::Info, + V, + ), + codec::Error, + > +where + K1: FullCodec, + K2: FullCodec, + V: FullCodec, + G: StorageDoubleMap, +{ + let prefix_hash = G::prefix_hash(); + if raw_key.len() < prefix_hash.len() { + return Err("Input length is too small".into()) + } + + let mut decode_key_input = &raw_key[prefix_hash.len()..]; + let key1 = G::Hasher1::decode_hash_and_then_info(&mut decode_key_input)?; + let key2 = G::Hasher2::decode_hash_and_then_info(&mut decode_key_input)?; + let value = V::decode(&mut raw_value)?; + + Ok((key1, key2, value)) +} + #[cfg(test)] mod test { use sp_io::TestExternalities; @@ -456,8 +510,8 @@ mod test { MyStorage::insert(2, 5, 9); MyStorage::insert(2, 6, 10); - assert_eq!(MyStorage::iter_prefix(1).collect::>(), vec![7, 8]); - assert_eq!(MyStorage::iter_prefix(2).collect::>(), vec![10, 9]); + assert_eq!(MyStorage::iter_prefix_value(1).collect::>(), vec![7, 8]); + assert_eq!(MyStorage::iter_prefix_value(2).collect::>(), vec![10, 9]); }); } } diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index c29a9a223aacf..768e6614545b2 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -17,9 +17,9 @@ #[cfg(not(feature = "std"))] use sp_std::prelude::*; use sp_std::borrow::Borrow; -use codec::{FullCodec, FullEncode, Decode, Encode, EncodeLike, Ref, EncodeAppend}; +use codec::{FullCodec, Decode, Encode, EncodeLike, Ref, EncodeAppend}; use crate::{storage::{self, unhashed}, traits::Len}; -use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; +use crate::hash::{StorageHasher, Twox128, StorageHasherInfo}; /// Generator for `StorageMap` used by `decl_storage`. /// @@ -32,12 +32,12 @@ use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// /// If the keys are not trusted (e.g. can be set by a user), a cryptographic `hasher` such as /// `blake2_256` must be used. Otherwise, other values in storage can be compromised. -pub trait StorageMap { +pub trait StorageMap { /// The type that get/take returns. type Query; /// Hasher. Used for generating final key. - type Hasher: StorageHasher; + type Hasher: StorageHasher + StorageHasherInfo; /// Module prefix. Used for generating final key. fn module_prefix() -> &'static [u8]; @@ -87,109 +87,9 @@ pub trait StorageMap { } } -/// Utility to iterate through items in a storage map. -pub struct StorageMapIterator { - prefix: Vec, - previous_key: Vec, - drain: bool, - _phantom: ::sp_std::marker::PhantomData<(K, V, Hasher)>, -} - -impl< - K: Decode + Sized, - V: Decode + Sized, - Hasher: ReversibleStorageHasher -> Iterator for StorageMapIterator { - type Item = (K, V); - - fn next(&mut self) -> Option<(K, V)> { - loop { - let maybe_next = sp_io::storage::next_key(&self.previous_key) - .filter(|n| n.starts_with(&self.prefix)); - break match maybe_next { - Some(next) => { - self.previous_key = next; - match unhashed::get::(&self.previous_key) { - Some(value) => { - if self.drain { - unhashed::kill(&self.previous_key) - } - let mut key_material = Hasher::reverse(&self.previous_key[self.prefix.len()..]); - match K::decode(&mut key_material) { - Ok(key) => Some((key, value)), - Err(_) => continue, - } - } - None => continue, - } - } - None => None, - } - } - } -} - -impl< - K: FullCodec, - V: FullCodec, - G: StorageMap, -> storage::IterableStorageMap for G where - G::Hasher: ReversibleStorageHasher -{ - type Iterator = StorageMapIterator; - - /// Enumerate all elements in the map. - fn iter() -> Self::Iterator { - let prefix = G::prefix_hash(); - Self::Iterator { - prefix: prefix.clone(), - previous_key: prefix, - drain: false, - _phantom: Default::default(), - } - } - - /// Enumerate all elements in the map. - fn drain() -> Self::Iterator { - let prefix = G::prefix_hash(); - Self::Iterator { - prefix: prefix.clone(), - previous_key: prefix, - drain: true, - _phantom: Default::default(), - } - } - - fn translate Option>(f: F) { - let prefix = G::prefix_hash(); - let mut previous_key = prefix.clone(); - loop { - match sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix)) { - Some(next) => { - previous_key = next; - let maybe_value = unhashed::get::(&previous_key); - match maybe_value { - Some(value) => { - let mut key_material = G::Hasher::reverse(&previous_key[prefix.len()..]); - match K::decode(&mut key_material) { - Ok(key) => match f(key, value) { - Some(new) => unhashed::put::(&previous_key, &new), - None => unhashed::kill(&previous_key), - }, - Err(_) => continue, - } - } - None => continue, - } - } - None => return, - } - } - } -} - -impl> storage::StorageMap for G { +impl> storage::StorageMap for G { type Query = G::Query; + type Hasher = G::Hasher; fn hashed_key_for>(key: KeyArg) -> Vec { Self::storage_map_final_key(key) @@ -367,4 +267,133 @@ impl> storage::StorageMap value }) } + + fn iter_key_value() -> storage::PrefixIterator<(K, V)> where + Self::Hasher: StorageHasherInfo + { + let prefix = G::prefix_hash(); + storage::PrefixIterator { + prefix: prefix.clone(), + previous_key: prefix, + drain: false, + closure: |raw_key, raw_value| from_raw_to_key_info_value::<_, _, G>(raw_key, raw_value), + } + } + + fn drain_key_value() -> storage::PrefixIterator<(K, V)> where + Self::Hasher: StorageHasherInfo + { + let prefix = G::prefix_hash(); + storage::PrefixIterator { + prefix: prefix.clone(), + previous_key: prefix, + drain: true, + closure: |raw_key, raw_value| from_raw_to_key_info_value::<_, _, G>(raw_key, raw_value), + } + } + + fn iter_value() -> storage::PrefixIterator { + let prefix = Self::prefix_hash(); + storage::PrefixIterator { + prefix: prefix.to_vec(), + previous_key: prefix.to_vec(), + drain: false, + closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), + } + } + + fn translate_key_value Option>(f: F) where + Self::Hasher: StorageHasherInfo + { + let prefix = G::prefix_hash(); + let mut previous_key = prefix.clone(); + loop { + match sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix)) { + Some(next) => { + previous_key = next; + let maybe_value = unhashed::get::(&previous_key); + let value = match maybe_value { + Some(value) => value, + None => { + crate::debug::error!( + "next_key returned a key with no value at {:?}", previous_key + ); + continue + }, + }; + + // Slice in bound because already check by `starts_with` + let mut hashed_key = &previous_key[prefix.len()..]; + + let key = match G::Hasher::decode_hash_and_then_info(&mut hashed_key) { + Ok(key) => key, + Err(e) => { + crate::debug::error!( + "old key failed to decode at {:?}: {:?}", + previous_key, e + ); + continue + }, + }; + + match f(key, value) { + Some(new) => unhashed::put::(&previous_key, &new), + None => unhashed::kill(&previous_key), + } + } + None => return, + } + } + } + + fn translate_value(translate_val: TranslateV) -> Result<(), u32> + where OldV: Decode, TranslateV: Fn(OldV) -> V + { + let prefix = Self::prefix_hash(); + let mut previous_key = prefix.to_vec(); + let mut errors = 0; + while let Some(next_key) = sp_io::storage::next_key(&previous_key) + .filter(|n| n.starts_with(&prefix[..])) + { + if let Some(value) = unhashed::get(&next_key) { + unhashed::put(&next_key[..], &translate_val(value)); + } else { + // We failed to read the value. Remove the key and increment errors. + unhashed::kill(&next_key[..]); + errors += 1; + } + + previous_key = next_key; + } + + if errors == 0 { + Ok(()) + } else { + Err(errors) + } + } + + fn remove_all() { + sp_io::storage::clear_prefix(&Self::prefix_hash()) + } +} + +fn from_raw_to_key_info_value( + raw_key: &[u8], + mut raw_value: &[u8] +) -> Result<(>::Info, V), codec::Error> where + K: FullCodec, + V: FullCodec, + G: StorageMap, +{ + let prefix_hash = G::prefix_hash(); + if raw_key.len() < prefix_hash.len() { + return Err("Input length is too small".into()) + } + + let mut decode_key_input = &raw_key[prefix_hash.len()..]; + let key = G::Hasher::decode_hash_and_then_info(&mut decode_key_input)?; + let value = V::decode(&mut raw_value)?; + + Ok((key, value)) } diff --git a/frame/support/src/storage/generator/mod.rs b/frame/support/src/storage/generator/mod.rs index 687d8a3c9361b..6f970e3b76535 100644 --- a/frame/support/src/storage/generator/mod.rs +++ b/frame/support/src/storage/generator/mod.rs @@ -36,7 +36,7 @@ pub use value::StorageValue; mod tests { use sp_io::TestExternalities; use codec::Encode; - use crate::storage::{unhashed, generator::StorageValue, IterableStorageMap}; + use crate::storage::{unhashed, generator::StorageValue}; struct Runtime {} pub trait Trait { @@ -89,15 +89,15 @@ mod tests { } assert_eq!( - NumberMap::iter().collect::>(), + NumberMap::iter_key_value().collect::>(), (0..100).map(|x| (x as u32, x as u64)).collect::>(), ); // do translation. - NumberMap::translate(|k: u32, v: u64| if k % 2 == 0 { Some((k as u64) << 32 | v) } else { None }); + NumberMap::translate_key_value(|k: u32, v: u64| if k % 2 == 0 { Some((k as u64) << 32 | v) } else { None }); assert_eq!( - NumberMap::iter().collect::>(), + NumberMap::iter_key_value().collect::>(), (0..50u32).map(|x| x * 2).map(|x| (x, (x as u64) << 32 | x as u64)).collect::>(), ); }) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index efec36b540abc..f7ae6d50c55e2 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -16,9 +16,9 @@ //! Stuff to do with the runtime's storage. -use sp_std::{prelude::*, marker::PhantomData}; +use sp_std::prelude::*; use codec::{FullCodec, FullEncode, Encode, EncodeAppend, EncodeLike, Decode}; -use crate::{traits::Len, hash::{Twox128, StorageHasher}}; +use crate::{traits::Len, hash::{StorageHasher, StorageHasherInfo}}; pub mod unhashed; pub mod hashed; @@ -131,6 +131,9 @@ pub trait StorageMap { /// The type that get/take return. type Query; + /// Hasher used to hash the key. + type Hasher; + /// Get the storage key used to fetch a value corresponding to a specific key. fn hashed_key_for>(key: KeyArg) -> Vec; @@ -149,6 +152,9 @@ pub trait StorageMap { /// Remove the value under a key. fn remove>(key: KeyArg); + /// Remove all values. + fn remove_all(); + /// Mutate the value under a key. fn mutate, R, F: FnOnce(&mut Self::Query) -> R>(key: KeyArg, f: F) -> R; @@ -214,48 +220,46 @@ pub trait StorageMap { fn migrate_key_from_blake>(key: KeyArg) -> Option { Self::migrate_key::(key) } -} - -/// A strongly-typed map in storage whose keys and values can be iterated over. -pub trait IterableStorageMap: StorageMap { - /// The type that iterates over all `(key, value)`. - type Iterator: Iterator; /// Enumerate all elements in the map in no particular order. If you alter the map while doing /// this, you'll get undefined results. - fn iter() -> Self::Iterator; + fn iter_key_value() -> PrefixIterator<(K, V)> where + Self::Hasher: StorageHasherInfo; /// Remove all elements from the map and iterate through them in no particular order. If you /// add elements to the map while doing this, you'll get undefined results. - fn drain() -> Self::Iterator; - - /// Translate the values of all elements by a function `f`, in the map in no particular order. - /// By returning `None` from `f` for an element, you'll remove it from the map. - fn translate Option>(f: F); -} - -/// A strongly-typed double map in storage whose secondary keys and values can be iterated over. -pub trait IterableStorageDoubleMap< - K1: FullCodec, - K2: FullCodec, - V: FullCodec ->: StorageDoubleMap { - /// The type that iterates over all `(key, value)`. - type Iterator: Iterator; - - /// Enumerate all elements in the map with first key `k1` in no particular order. If you add or - /// remove values whose first key is `k1` to the map while doing this, you'll get undefined - /// results. - fn iter(k1: impl EncodeLike) -> Self::Iterator; + fn drain_key_value() -> PrefixIterator<(K, V)> where + Self::Hasher: StorageHasherInfo; - /// Remove all elements from the map with first key `k1` and iterate through them in no - /// particular order. If you add elements with first key `k1` to the map while doing this, - /// you'll get undefined results. - fn drain(k1: impl EncodeLike) -> Self::Iterator; + /// Enumerate all elements in the map in no particular order. If you alter the map while doing + /// this, you'll get undefined results. + fn iter_value() -> PrefixIterator; /// Translate the values of all elements by a function `f`, in the map in no particular order. /// By returning `None` from `f` for an element, you'll remove it from the map. - fn translate Option>(f: F); + fn translate_key_value Option>(f: F) where + Self::Hasher: StorageHasherInfo; + + /// Translate the values from some previous `OldValue` to the current type. + /// + /// `TV` translates values. + /// + /// Returns `Err` if the map could not be interpreted as the old type, and Ok if it could. + /// The `Err` contains the number of value that couldn't be interpreted, those value are + /// removed from the map. + /// + /// # Warning + /// + /// This function must be used with care, before being updated the storage still contains the + /// old type, thus other calls (such as `get`) will fail at decoding it. + /// + /// # Usage + /// + /// This would typically be called inside the module implementation of on_runtime_upgrade, while + /// ensuring **no usage of this storage are made before the call to `on_runtime_upgrade`**. (More + /// precisely prior initialized modules doesn't make use of this storage). + fn translate_value(translate_val: TranslateV) -> Result<(), u32> + where OldV: Decode, TranslateV: Fn(OldV) -> V; } /// An implementation of a map with a two keys. @@ -269,6 +273,12 @@ pub trait StorageDoubleMap { /// The type that get/take returns. type Query; + /// Hasher used to hash first key. + type Hasher1; + + /// Hasher used to hash second key. + type Hasher2; + fn hashed_key_for(k1: KArg1, k2: KArg2) -> Vec where KArg1: EncodeLike, @@ -310,8 +320,7 @@ pub trait StorageDoubleMap { fn remove_prefix(k1: KArg1) where KArg1: ?Sized + EncodeLike; - fn iter_prefix(k1: KArg1) -> PrefixIterator - where KArg1: ?Sized + EncodeLike; + fn remove_all(); fn mutate(k1: KArg1, k2: KArg2, f: F) -> R where @@ -370,201 +379,174 @@ pub trait StorageDoubleMap { KeyArg1: EncodeLike, KeyArg2: EncodeLike, >(key1: KeyArg1, key2: KeyArg2) -> Option; -} -/// Iterator for prefixed map. -pub struct PrefixIterator { - prefix: Vec, - previous_key: Vec, - phantom_data: PhantomData, -} + /// Translate the values of all elements by a function `f`, in the map in no particular order. + /// By returning `None` from `f` for an element, you'll remove it from the map. + fn translate Option>(f: F); -impl Iterator for PrefixIterator { - type Item = Value; + /// Enumerate all elements in the map with first key `k1` in no particular order. If you add or + /// remove values whose first key is `k1` to the map while doing this, you'll get undefined + /// results. + fn iter_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where + Self::Hasher2: StorageHasherInfo; - fn next(&mut self) -> Option { - match sp_io::storage::next_key(&self.previous_key) - .filter(|n| n.starts_with(&self.prefix[..])) - { - Some(next_key) => { - let value = unhashed::get(&next_key); - - if value.is_none() { - runtime_print!( - "ERROR: returned next_key has no value:\nkey is {:?}\nnext_key is {:?}", - &self.previous_key, &next_key, - ); - } + /// Remove all elements from the map with first key `k1` and iterate through them in no + /// particular order. If you add elements with first key `k1` to the map while doing this, + /// you'll get undefined results. + fn drain_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where + Self::Hasher2: StorageHasherInfo; - self.previous_key = next_key; + /// Iter over all value associated to the first key. + fn iter_prefix_value(k1: KArg1) -> PrefixIterator where + KArg1: ?Sized + EncodeLike; - value - }, - _ => None, - } - } -} + /// Iter over all value and decode the first key. + fn iter_key1_value() -> PrefixIterator<(K1, V)> where + Self::Hasher1: StorageHasherInfo; -/// Trait for maps that store all its value after a unique prefix. -/// -/// By default the final prefix is: -/// ```nocompile -/// Twox128(module_prefix) ++ Twox128(storage_prefix) -/// ``` -pub trait StoragePrefixedMap { - - /// Module prefix. Used for generating final key. - fn module_prefix() -> &'static [u8]; - - /// Storage prefix. Used for generating final key. - fn storage_prefix() -> &'static [u8]; - - /// Final full prefix that prefixes all keys. - fn final_prefix() -> [u8; 32] { - let mut final_key = [0u8; 32]; - final_key[0..16].copy_from_slice(&Twox128::hash(Self::module_prefix())); - final_key[16..32].copy_from_slice(&Twox128::hash(Self::storage_prefix())); - final_key - } + /// Iter over all value and decode the second key. + fn iter_key2_value() -> PrefixIterator<(K2, V)> where + Self::Hasher2: StorageHasherInfo; - /// Remove all value of the storage. - fn remove_all() { - sp_io::storage::clear_prefix(&Self::final_prefix()) - } + /// Iter over all value and decode the first key and the second key. + fn iter_key1_key2_value() -> PrefixIterator<(K1, K2, V)> where + Self::Hasher1: StorageHasherInfo, + Self::Hasher2: StorageHasherInfo; - /// Iter over all value of the storage. - fn iter_values() -> PrefixIterator { - let prefix = Self::final_prefix(); - PrefixIterator { - prefix: prefix.to_vec(), - previous_key: prefix.to_vec(), - phantom_data: Default::default(), - } - } + /// Iter over all value. + fn iter_value() -> PrefixIterator; +} - /// Translate the values from some previous `OldValue` to the current type. - /// - /// `TV` translates values. - /// - /// Returns `Err` if the map could not be interpreted as the old type, and Ok if it could. - /// The `Err` contains the number of value that couldn't be interpreted, those value are - /// removed from the map. - /// - /// # Warning - /// - /// This function must be used with care, before being updated the storage still contains the - /// old type, thus other calls (such as `get`) will fail at decoding it. - /// - /// # Usage - /// - /// This would typically be called inside the module implementation of on_runtime_upgrade, while - /// ensuring **no usage of this storage are made before the call to `on_runtime_upgrade`**. (More - /// precisely prior initialized modules doesn't make use of this storage). - fn translate_values(translate_val: TV) -> Result<(), u32> - where OldValue: Decode, TV: Fn(OldValue) -> Value - { - let prefix = Self::final_prefix(); - let mut previous_key = prefix.to_vec(); - let mut errors = 0; - while let Some(next_key) = sp_io::storage::next_key(&previous_key) - .filter(|n| n.starts_with(&prefix[..])) - { - if let Some(value) = unhashed::get(&next_key) { - unhashed::put(&next_key[..], &translate_val(value)); - } else { - // We failed to read the value. Remove the key and increment errors. - unhashed::kill(&next_key[..]); - errors += 1; - } +/// Iterate over a prefix and decode raw_key and raw_value into `T`. +pub struct PrefixIterator { + prefix: Vec, + previous_key: Vec, + /// If true then value are removed while iterating + drain: bool, + /// Function that take `(raw_key, raw_value)` and decode `T` + closure: fn(&[u8], &[u8]) -> Result, +} - previous_key = next_key; - } +impl Iterator for PrefixIterator { + type Item = T; - if errors == 0 { - Ok(()) - } else { - Err(errors) + fn next(&mut self) -> Option { + loop { + let maybe_next = sp_io::storage::next_key(&self.previous_key) + .filter(|n| n.starts_with(&self.prefix)); + break match maybe_next { + Some(next) => { + self.previous_key = next; + match unhashed::get_raw(&self.previous_key) { + Some(raw_value) => { + if self.drain { + unhashed::kill(&self.previous_key) + } + match (self.closure)(&self.previous_key[..], &raw_value[..]) { + Ok(t) => Some(t), + Err(e) => { + crate::debug::error!( + "(key, value) failed to decode at {:?}: {:?}", + self.previous_key, e + ); + continue + } + } + } + None => { + crate::debug::error!( + "next_key returned a key with no value at {:?}", + self.previous_key + ); + continue + } + } + } + None => None, + } } } } + #[cfg(test)] mod test { - use sp_core::hashing::twox_128; - use sp_io::TestExternalities; - use crate::storage::{unhashed, StoragePrefixedMap}; - - #[test] - fn prefixed_map_works() { - TestExternalities::default().execute_with(|| { - struct MyStorage; - impl StoragePrefixedMap for MyStorage { - fn module_prefix() -> &'static [u8] { - b"MyModule" - } - - fn storage_prefix() -> &'static [u8] { - b"MyStorage" - } - } - - let key_before = { - let mut k = MyStorage::final_prefix(); - let last = k.iter_mut().last().unwrap(); - *last = last.checked_sub(1).unwrap(); - k - }; - let key_after = { - let mut k = MyStorage::final_prefix(); - let last = k.iter_mut().last().unwrap(); - *last = last.checked_add(1).unwrap(); - k - }; - - unhashed::put(&key_before[..], &32u64); - unhashed::put(&key_after[..], &33u64); - - let k = [twox_128(b"MyModule"), twox_128(b"MyStorage")].concat(); - assert_eq!(MyStorage::final_prefix().to_vec(), k); - - // test iteration - assert_eq!(MyStorage::iter_values().collect::>(), vec![]); - - unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u64); - unhashed::put(&[&k[..], &vec![1, 1][..]].concat(), &2u64); - unhashed::put(&[&k[..], &vec![8][..]].concat(), &3u64); - unhashed::put(&[&k[..], &vec![10][..]].concat(), &4u64); - - assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3, 4]); - - // test removal - MyStorage::remove_all(); - assert_eq!(MyStorage::iter_values().collect::>(), vec![]); - - // test migration - unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u32); - unhashed::put(&[&k[..], &vec![8][..]].concat(), &2u32); - - assert_eq!(MyStorage::iter_values().collect::>(), vec![]); - MyStorage::translate_values(|v: u32| v as u64).unwrap(); - assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2]); - MyStorage::remove_all(); - - // test migration 2 - unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u128); - unhashed::put(&[&k[..], &vec![1, 1][..]].concat(), &2u64); - unhashed::put(&[&k[..], &vec![8][..]].concat(), &3u128); - unhashed::put(&[&k[..], &vec![10][..]].concat(), &4u32); - - // (contains some value that successfully decoded to u64) - assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); - assert_eq!(MyStorage::translate_values(|v: u128| v as u64), Err(2)); - assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 3]); - MyStorage::remove_all(); - - // test that other values are not modified. - assert_eq!(unhashed::get(&key_before[..]), Some(32u64)); - assert_eq!(unhashed::get(&key_after[..]), Some(33u64)); - }); - } + // TODO TODO: reuse!!!! + // use sp_core::hashing::twox_128; + // use sp_io::TestExternalities; + // use crate::storage::{unhashed, StoragePrefixedMap}; + + // #[test] + // fn prefixed_map_works() { + // TestExternalities::default().execute_with(|| { + // struct MyStorage; + // impl StoragePrefixedMap for MyStorage { + // fn module_prefix() -> &'static [u8] { + // b"MyModule" + // } + + // fn storage_prefix() -> &'static [u8] { + // b"MyStorage" + // } + // } + + // let key_before = { + // let mut k = MyStorage::final_prefix(); + // let last = k.iter_mut().last().unwrap(); + // *last = last.checked_sub(1).unwrap(); + // k + // }; + // let key_after = { + // let mut k = MyStorage::final_prefix(); + // let last = k.iter_mut().last().unwrap(); + // *last = last.checked_add(1).unwrap(); + // k + // }; + + // unhashed::put(&key_before[..], &32u64); + // unhashed::put(&key_after[..], &33u64); + + // let k = [twox_128(b"MyModule"), twox_128(b"MyStorage")].concat(); + // assert_eq!(MyStorage::final_prefix().to_vec(), k); + + // // test iteration + // assert_eq!(MyStorage::iter_values().collect::>(), vec![]); + + // unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u64); + // unhashed::put(&[&k[..], &vec![1, 1][..]].concat(), &2u64); + // unhashed::put(&[&k[..], &vec![8][..]].concat(), &3u64); + // unhashed::put(&[&k[..], &vec![10][..]].concat(), &4u64); + + // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3, 4]); + + // // test removal + // MyStorage::remove_all(); + // assert_eq!(MyStorage::iter_values().collect::>(), vec![]); + + // // test migration + // unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u32); + // unhashed::put(&[&k[..], &vec![8][..]].concat(), &2u32); + + // assert_eq!(MyStorage::iter_values().collect::>(), vec![]); + // MyStorage::translate_values(|v: u32| v as u64).unwrap(); + // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2]); + // MyStorage::remove_all(); + + // // test migration 2 + // unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u128); + // unhashed::put(&[&k[..], &vec![1, 1][..]].concat(), &2u64); + // unhashed::put(&[&k[..], &vec![8][..]].concat(), &3u128); + // unhashed::put(&[&k[..], &vec![10][..]].concat(), &4u32); + + // // (contains some value that successfully decoded to u64) + // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); + // assert_eq!(MyStorage::translate_values(|v: u128| v as u64), Err(2)); + // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 3]); + // MyStorage::remove_all(); + + // // test that other values are not modified. + // assert_eq!(unhashed::get(&key_before[..]), Some(32u64)); + // assert_eq!(unhashed::get(&key_after[..]), Some(33u64)); + // }); + // } } diff --git a/frame/support/test/tests/final_keys.rs b/frame/support/test/tests/final_keys.rs index ae23c5a64c200..82573ddd49bdb 100644 --- a/frame/support/test/tests/final_keys.rs +++ b/frame/support/test/tests/final_keys.rs @@ -16,7 +16,7 @@ use frame_support::storage::unhashed; use codec::Encode; -use frame_support::{StorageDoubleMap, StorageMap, StorageValue, StoragePrefixedMap}; +use frame_support::{StorageDoubleMap, StorageMap, StorageValue}; use sp_io::{TestExternalities, hashing::{twox_64, twox_128, blake2_128}}; mod no_instance { @@ -102,27 +102,23 @@ fn final_keys_no_instance() { let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"Map")].concat(); k.extend(1u32.using_encoded(blake2_128_concat)); assert_eq!(unhashed::get::(&k), Some(2u32)); - assert_eq!(&k[..32], &::final_prefix()); no_instance::Map2::insert(1, 2); let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"Map2")].concat(); k.extend(1u32.using_encoded(twox_64_concat)); assert_eq!(unhashed::get::(&k), Some(2u32)); - assert_eq!(&k[..32], &::final_prefix()); no_instance::DoubleMap::insert(&1, &2, &3); let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"DoubleMap")].concat(); k.extend(1u32.using_encoded(blake2_128_concat)); k.extend(2u32.using_encoded(blake2_128_concat)); assert_eq!(unhashed::get::(&k), Some(3u32)); - assert_eq!(&k[..32], &::final_prefix()); no_instance::DoubleMap2::insert(&1, &2, &3); let mut k = [twox_128(b"FinalKeysNone"), twox_128(b"DoubleMap2")].concat(); k.extend(1u32.using_encoded(twox_64_concat)); k.extend(2u32.using_encoded(twox_64_concat)); assert_eq!(unhashed::get::(&k), Some(3u32)); - assert_eq!(&k[..32], &::final_prefix()); }); } @@ -137,27 +133,23 @@ fn final_keys_default_instance() { let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"Map")].concat(); k.extend(1u32.using_encoded(blake2_128_concat)); assert_eq!(unhashed::get::(&k), Some(2u32)); - assert_eq!(&k[..32], &>::final_prefix()); >::insert(1, 2); let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"Map2")].concat(); k.extend(1u32.using_encoded(twox_64_concat)); assert_eq!(unhashed::get::(&k), Some(2u32)); - assert_eq!(&k[..32], &>::final_prefix()); >::insert(&1, &2, &3); let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"DoubleMap")].concat(); k.extend(1u32.using_encoded(blake2_128_concat)); k.extend(2u32.using_encoded(blake2_128_concat)); assert_eq!(unhashed::get::(&k), Some(3u32)); - assert_eq!(&k[..32], &>::final_prefix()); >::insert(&1, &2, &3); let mut k = [twox_128(b"FinalKeysSome"), twox_128(b"DoubleMap2")].concat(); k.extend(1u32.using_encoded(twox_64_concat)); k.extend(2u32.using_encoded(twox_64_concat)); assert_eq!(unhashed::get::(&k), Some(3u32)); - assert_eq!(&k[..32], &>::final_prefix()); }); } @@ -172,26 +164,22 @@ fn final_keys_instance_2() { let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"Map")].concat(); k.extend(1u32.using_encoded(blake2_128_concat)); assert_eq!(unhashed::get::(&k), Some(2u32)); - assert_eq!(&k[..32], &>::final_prefix()); >::insert(1, 2); let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"Map2")].concat(); k.extend(1u32.using_encoded(twox_64_concat)); assert_eq!(unhashed::get::(&k), Some(2u32)); - assert_eq!(&k[..32], &>::final_prefix()); >::insert(&1, &2, &3); let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"DoubleMap")].concat(); k.extend(1u32.using_encoded(blake2_128_concat)); k.extend(2u32.using_encoded(blake2_128_concat)); assert_eq!(unhashed::get::(&k), Some(3u32)); - assert_eq!(&k[..32], &>::final_prefix()); >::insert(&1, &2, &3); let mut k = [twox_128(b"Instance2FinalKeysSome"), twox_128(b"DoubleMap2")].concat(); k.extend(1u32.using_encoded(twox_64_concat)); k.extend(2u32.using_encoded(twox_64_concat)); assert_eq!(unhashed::get::(&k), Some(3u32)); - assert_eq!(&k[..32], &>::final_prefix()); }); } From 9d8fa5b84664fa52dbe57fbfe47590f792fcb368 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 23 Mar 2020 11:29:47 +0100 Subject: [PATCH 2/7] add deprecated to avoid modifiying other yet --- frame/democracy/src/lib.rs | 2 +- frame/elections-phragmen/src/lib.rs | 2 +- frame/staking/src/lib.rs | 3 ++- frame/staking/src/mock.rs | 4 ++-- frame/support/src/storage/mod.rs | 15 +++++++++++++++ utils/frame/rpc/support/src/lib.rs | 6 +++--- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/frame/democracy/src/lib.rs b/frame/democracy/src/lib.rs index cbc766499fe6e..128d5653b3e7b 100644 --- a/frame/democracy/src/lib.rs +++ b/frame/democracy/src/lib.rs @@ -159,7 +159,7 @@ use sp_runtime::{ }; use codec::{Ref, Encode, Decode, Input, Output}; use frame_support::{ - decl_module, decl_storage, decl_event, decl_error, ensure, Parameter, IterableStorageMap, + decl_module, decl_storage, decl_event, decl_error, ensure, Parameter, weights::SimpleDispatchInfo, traits::{ Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get, diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index ac85b3047d0ba..330b1c2ccc87d 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -88,7 +88,7 @@ use sp_runtime::{ }; use frame_support::{ decl_storage, decl_event, ensure, decl_module, decl_error, weights::SimpleDispatchInfo, - storage::{StorageMap, IterableStorageMap}, traits::{ + storage::StorageMap, traits::{ Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons, ChangeMembers, OnUnbalanced, WithdrawReason, Contains, BalanceStatus } diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 312e70963e30a..1c574a11cbc9f 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -263,7 +263,8 @@ use sp_std::{prelude::*, result, collections::btree_map::BTreeMap}; use codec::{HasCompact, Encode, Decode}; use frame_support::{ decl_module, decl_event, decl_storage, ensure, decl_error, weights::SimpleDispatchInfo, - dispatch::DispatchResult, storage::IterableStorageMap, traits::{ + dispatch::DispatchResult, + traits::{ Currency, LockIdentifier, LockableCurrency, WithdrawReasons, OnUnbalanced, Imbalance, Get, Time } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 381ddf6fa9891..795a7edc1582f 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -25,8 +25,8 @@ use sp_staking::{SessionIndex, offence::{OffenceDetails, OnOffenceHandler}}; use sp_core::{H256, crypto::key_types}; use sp_io; use frame_support::{ - assert_ok, impl_outer_origin, parameter_types, StorageValue, StorageMap, - StorageDoubleMap, IterableStorageMap, traits::{Currency, Get, FindAuthor}, weights::Weight, + assert_ok, impl_outer_origin, parameter_types, StorageValue, StorageMap, StorageDoubleMap, + traits::{Currency, Get, FindAuthor}, weights::Weight, }; use crate::{ EraIndex, GenesisConfig, Module, Trait, StakerStatus, ValidatorPrefs, RewardDestination, diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index f7ae6d50c55e2..569d57fc8d024 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -231,6 +231,15 @@ pub trait StorageMap { fn drain_key_value() -> PrefixIterator<(K, V)> where Self::Hasher: StorageHasherInfo; + /// Enumerate all elements in the map in no particular order. If you alter the map while doing + /// this, you'll get undefined results. + #[deprecated(note = "please use `iter_value` instead")] + fn iter() -> PrefixIterator<(K, V)> where + Self::Hasher: StorageHasherInfo + { + Self::iter_key_value() + } + /// Enumerate all elements in the map in no particular order. If you alter the map while doing /// this, you'll get undefined results. fn iter_value() -> PrefixIterator; @@ -396,6 +405,12 @@ pub trait StorageDoubleMap { fn drain_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where Self::Hasher2: StorageHasherInfo; + /// Iter over all value associated to the first key. + #[deprecated(note = "please use `iter_prefix_value` instead")] + fn iter_prefix(k1: KArg1) -> PrefixIterator where KArg1: ?Sized + EncodeLike { + Self::iter_prefix_value(k1) + } + /// Iter over all value associated to the first key. fn iter_prefix_value(k1: KArg1) -> PrefixIterator where KArg1: ?Sized + EncodeLike; diff --git a/utils/frame/rpc/support/src/lib.rs b/utils/frame/rpc/support/src/lib.rs index 118f5709a6b70..911caedd13aac 100644 --- a/utils/frame/rpc/support/src/lib.rs +++ b/utils/frame/rpc/support/src/lib.rs @@ -22,7 +22,7 @@ use core::marker::PhantomData; use futures::compat::Future01CompatExt; use jsonrpc_client_transports::RpcError; -use codec::{DecodeAll, FullCodec, FullEncode}; +use codec::{DecodeAll, FullCodec}; use serde::{de::DeserializeOwned, Serialize}; use frame_support::storage::generator::{ StorageDoubleMap, StorageMap, StorageValue @@ -104,7 +104,7 @@ impl StorageQuery { } /// Create a storage query for a value in a StorageMap. - pub fn map, K: FullEncode>(key: K) -> Self { + pub fn map, K: FullCodec>(key: K) -> Self { Self { key: StorageKey(St::storage_map_final_key(key)), _spook: PhantomData, @@ -112,7 +112,7 @@ impl StorageQuery { } /// Create a storage query for a value in a StorageDoubleMap. - pub fn double_map, K1: FullEncode, K2: FullEncode>( + pub fn double_map, K1: FullCodec, K2: FullCodec>( key1: K1, key2: K2, ) -> Self { From 009da75059409db5b988d5fbc4e6f773c55a2c92 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 23 Mar 2020 12:23:14 +0100 Subject: [PATCH 3/7] test --- frame/support/src/storage/mod.rs | 83 ---------- frame/support/test/tests/decl_storage.rs | 193 +++++++++++++++++++++++ 2 files changed, 193 insertions(+), 83 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 569d57fc8d024..c42444a508a3d 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -482,86 +482,3 @@ impl Iterator for PrefixIterator { } } } - - -#[cfg(test)] -mod test { - // TODO TODO: reuse!!!! - // use sp_core::hashing::twox_128; - // use sp_io::TestExternalities; - // use crate::storage::{unhashed, StoragePrefixedMap}; - - // #[test] - // fn prefixed_map_works() { - // TestExternalities::default().execute_with(|| { - // struct MyStorage; - // impl StoragePrefixedMap for MyStorage { - // fn module_prefix() -> &'static [u8] { - // b"MyModule" - // } - - // fn storage_prefix() -> &'static [u8] { - // b"MyStorage" - // } - // } - - // let key_before = { - // let mut k = MyStorage::final_prefix(); - // let last = k.iter_mut().last().unwrap(); - // *last = last.checked_sub(1).unwrap(); - // k - // }; - // let key_after = { - // let mut k = MyStorage::final_prefix(); - // let last = k.iter_mut().last().unwrap(); - // *last = last.checked_add(1).unwrap(); - // k - // }; - - // unhashed::put(&key_before[..], &32u64); - // unhashed::put(&key_after[..], &33u64); - - // let k = [twox_128(b"MyModule"), twox_128(b"MyStorage")].concat(); - // assert_eq!(MyStorage::final_prefix().to_vec(), k); - - // // test iteration - // assert_eq!(MyStorage::iter_values().collect::>(), vec![]); - - // unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u64); - // unhashed::put(&[&k[..], &vec![1, 1][..]].concat(), &2u64); - // unhashed::put(&[&k[..], &vec![8][..]].concat(), &3u64); - // unhashed::put(&[&k[..], &vec![10][..]].concat(), &4u64); - - // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3, 4]); - - // // test removal - // MyStorage::remove_all(); - // assert_eq!(MyStorage::iter_values().collect::>(), vec![]); - - // // test migration - // unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u32); - // unhashed::put(&[&k[..], &vec![8][..]].concat(), &2u32); - - // assert_eq!(MyStorage::iter_values().collect::>(), vec![]); - // MyStorage::translate_values(|v: u32| v as u64).unwrap(); - // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2]); - // MyStorage::remove_all(); - - // // test migration 2 - // unhashed::put(&[&k[..], &vec![1][..]].concat(), &1u128); - // unhashed::put(&[&k[..], &vec![1, 1][..]].concat(), &2u64); - // unhashed::put(&[&k[..], &vec![8][..]].concat(), &3u128); - // unhashed::put(&[&k[..], &vec![10][..]].concat(), &4u32); - - // // (contains some value that successfully decoded to u64) - // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 2, 3]); - // assert_eq!(MyStorage::translate_values(|v: u128| v as u64), Err(2)); - // assert_eq!(MyStorage::iter_values().collect::>(), vec![1, 3]); - // MyStorage::remove_all(); - - // // test that other values are not modified. - // assert_eq!(unhashed::get(&key_before[..]), Some(32u64)); - // assert_eq!(unhashed::get(&key_after[..]), Some(33u64)); - // }); - // } -} diff --git a/frame/support/test/tests/decl_storage.rs b/frame/support/test/tests/decl_storage.rs index ea9b09f9d7bc2..97b89580e6659 100644 --- a/frame/support/test/tests/decl_storage.rs +++ b/frame/support/test/tests/decl_storage.rs @@ -620,3 +620,196 @@ mod test_append_and_len { }); } } + +/// Test iterators for StorageMap and StorageDoubleMap with key information +#[cfg(test)] +#[allow(dead_code)] +mod test_iterators { + use codec::{Encode, Decode}; + use frame_support::storage::{generator::{StorageMap, StorageDoubleMap}, unhashed}; + + pub trait Trait { + type Origin; + type BlockNumber; + } + + frame_support::decl_module! { + pub struct Module for enum Call where origin: T::Origin {} + } + + #[derive(PartialEq, Eq, Clone, Encode, Decode)] + struct NoDef(u32); + + frame_support::decl_storage! { + trait Store for Module as Test { + MapInfo: map hasher(blake2_128_concat) u16 => u64; + + DoubleMapInfoInfo: double_map hasher(blake2_128_concat) u16, hasher(blake2_128_concat) u32 => u64; + DoubleMapOpaqueInfo: double_map hasher(opaque_blake2_128) u16, hasher(blake2_128_concat) u32 => u64; + } + } + + struct Test {} + + impl Trait for Test { + type Origin = u32; + type BlockNumber = u32; + } + + fn key_before_prefix(mut prefix: Vec) -> Vec { + let last = prefix.iter_mut().last().unwrap(); + *last = last.checked_sub(1).unwrap_or_else(|| unimplemented!()); + prefix + } + + fn key_after_prefix(mut prefix: Vec) -> Vec { + let last = prefix.iter_mut().last().unwrap(); + *last = last.checked_add(1).unwrap_or_else(|| unimplemented!()); + prefix + } + + fn key_in_prefix(mut prefix: Vec) -> Vec { + prefix.push(0); + prefix + } + + #[test] + fn map_info_iteration() { + sp_io::TestExternalities::default().execute_with(|| { + let prefix = MapInfo::prefix_hash(); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + MapInfo::insert(i as u16, i as u64); + } + + assert_eq!( + MapInfo::iter_key_value().collect::>(), + vec![(3, 3), (0, 0), (2, 2), (1, 1)], + ); + + assert_eq!( + MapInfo::iter_value().collect::>(), + vec![3, 0, 2, 1], + ); + }) + } + + #[test] + fn double_map_info_info_iteration() { + sp_io::TestExternalities::default().execute_with(|| { + // All map iterator + let prefix = DoubleMapInfoInfo::prefix_hash(); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + DoubleMapInfoInfo::insert(i as u16, i as u32, i as u64); + } + + assert_eq!( + DoubleMapInfoInfo::iter_key1_value().collect::>(), + vec![(3, 3), (0, 0), (2, 2), (1, 1)], + ); + + assert_eq!( + DoubleMapInfoInfo::iter_key2_value().collect::>(), + vec![(3, 3), (0, 0), (2, 2), (1, 1)], + ); + + assert_eq!( + DoubleMapInfoInfo::iter_key1_key2_value().collect::>(), + vec![(3, 3, 3), (0, 0, 0), (2, 2, 2), (1, 1, 1)], + ); + + assert_eq!( + DoubleMapInfoInfo::iter_value().collect::>(), + vec![3, 0, 2, 1], + ); + + DoubleMapInfoInfo::remove_all(); + + // Prefix iterator + let k1 = 3 << 8; + let prefix = DoubleMapInfoInfo::storage_double_map_final_key1(k1); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + DoubleMapInfoInfo::insert(k1, i as u32, i as u64); + } + + assert_eq!( + DoubleMapInfoInfo::iter_prefix_key2_value(k1).collect::>(), + vec![(0, 0), (2, 2), (1, 1), (3, 3)], + ); + + assert_eq!( + DoubleMapInfoInfo::iter_prefix_value(k1).collect::>(), + vec![0, 2, 1, 3], + ); + }) + } + + #[test] + fn double_map_opaque_info_iteration() { + sp_io::TestExternalities::default().execute_with(|| { + // All map iterator + let prefix = DoubleMapOpaqueInfo::prefix_hash(); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + DoubleMapOpaqueInfo::insert(i as u16, i as u32, i as u64); + } + + assert_eq!( + DoubleMapOpaqueInfo::iter_key2_value().collect::>(), + vec![(3, 3), (0, 0), (2, 2), (1, 1)], + ); + + assert_eq!( + DoubleMapOpaqueInfo::iter_value().collect::>(), + vec![3, 0, 2, 1], + ); + + DoubleMapOpaqueInfo::remove_all(); + + // Prefix iterator + let k1 = 3 << 8; + let prefix = DoubleMapOpaqueInfo::storage_double_map_final_key1(k1); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + DoubleMapOpaqueInfo::insert(k1, i as u32, i as u64); + } + + assert_eq!( + DoubleMapOpaqueInfo::iter_prefix_key2_value(k1).collect::>(), + vec![(0, 0), (2, 2), (1, 1), (3, 3)], + ); + + assert_eq!( + DoubleMapOpaqueInfo::iter_prefix_value(k1).collect::>(), + vec![0, 2, 1, 3], + ); + }) + } +} From 39f027ed27c1c3d78bb50695a35230020a6213e3 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 23 Mar 2020 15:56:03 +0100 Subject: [PATCH 4/7] remove StorageHasherInfo traits --- frame/support/src/hash.rs | 130 ++++-------------- .../src/storage/generator/double_map.rs | 90 ++++++------ frame/support/src/storage/generator/map.rs | 33 ++--- frame/support/src/storage/mod.rs | 33 +++-- frame/support/test/tests/decl_storage.rs | 53 ++++++- 5 files changed, 146 insertions(+), 193 deletions(-) diff --git a/frame/support/src/hash.rs b/frame/support/src/hash.rs index 10b1fa0bd8ac0..fd9e793fe6861 100644 --- a/frame/support/src/hash.rs +++ b/frame/support/src/hash.rs @@ -16,7 +16,7 @@ //! Hash utilities. -use codec::{Codec, Decode}; +use codec::Codec; use sp_std::prelude::Vec; use sp_io::hashing::{blake2_128, blake2_256, twox_64, twox_128, twox_256}; @@ -64,18 +64,9 @@ pub trait ReversibleStorageHasher: StorageHasher { fn reverse(x: &[u8]) -> &[u8]; } -/// Trait to retrieve some info from hash of type `Key` encoded. -pub trait StorageHasherInfo { - /// Some info contained in the hash of type `Key` encoded. - type Info; - - /// Decode the hash and then decode the info from the decoded hash. - /// - /// # WARNING - /// - /// Even if info is (), input must be modified to have read the entire encoded hash. - fn decode_hash_and_then_info(input: &mut I) - -> Result; +/// Hasher with a fixed length. +pub trait FixedLengthHasher: StorageHasher { + const LENGTH: u32; } /// Store the key directly. @@ -91,14 +82,6 @@ impl ReversibleStorageHasher for Identity { x } } -impl StorageHasherInfo for Identity { - type Info = Key; - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - Key::decode(input) - } -} /// Hash storage keys with `concat(twox64(key), key)` pub struct Twox64Concat; @@ -114,18 +97,13 @@ impl StorageHasher for Twox64Concat { } impl ReversibleStorageHasher for Twox64Concat { fn reverse(x: &[u8]) -> &[u8] { + if x.len() < 8 { + crate::debug::error!("Invalid reverse: hash length too short"); + return &[] + } &x[8..] } } -impl StorageHasherInfo for Twox64Concat { - type Info = Key; - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - input.read(&mut [0u8; 8])?; - Key::decode(input) - } -} /// Hash storage keys with `concat(blake2_128(key), key)` pub struct Blake2_128Concat; @@ -141,18 +119,13 @@ impl StorageHasher for Blake2_128Concat { } impl ReversibleStorageHasher for Blake2_128Concat { fn reverse(x: &[u8]) -> &[u8] { + if x.len() < 16 { + crate::debug::error!("Invalid reverse: hash length too short"); + return &[] + } &x[16..] } } -impl StorageHasherInfo for Blake2_128Concat { - type Info = Key; - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - input.read(&mut [0u8; 16])?; - Key::decode(input) - } -} /// Hash storage keys with blake2 128 pub struct Blake2_128; @@ -162,14 +135,9 @@ impl StorageHasher for Blake2_128 { blake2_128(x) } } -impl StorageHasherInfo for Blake2_128 { - type Info = (); - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - input.read(&mut [0u8; 16])?; - Ok(()) - } + +impl FixedLengthHasher for Blake2_128 { + const LENGTH: u32 = 16; } /// Hash storage keys with blake2 256 @@ -180,14 +148,9 @@ impl StorageHasher for Blake2_256 { blake2_256(x) } } -impl StorageHasherInfo for Blake2_256 { - type Info = (); - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - input.read(&mut [0u8; 32])?; - Ok(()) - } + +impl FixedLengthHasher for Blake2_256 { + const LENGTH: u32 = 32; } /// Hash storage keys with twox 128 @@ -198,14 +161,9 @@ impl StorageHasher for Twox128 { twox_128(x) } } -impl StorageHasherInfo for Twox128 { - type Info = (); - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - input.read(&mut [0u8; 16])?; - Ok(()) - } + +impl FixedLengthHasher for Twox128 { + const LENGTH: u32 = 16; } /// Hash storage keys with twox 256 @@ -216,20 +174,14 @@ impl StorageHasher for Twox256 { twox_256(x) } } -impl StorageHasherInfo for Twox256 { - type Info = (); - fn decode_hash_and_then_info(input: &mut I) - -> Result - { - input.read(&mut [0u8; 32])?; - Ok(()) - } + +impl FixedLengthHasher for Twox256 { + const LENGTH: u32 = 32; } #[cfg(test)] mod tests { use super::*; - use codec::Encode; #[test] fn test_twox_64_concat() { @@ -242,38 +194,4 @@ mod tests { let r = Blake2_128Concat::hash(b"foo"); assert_eq!(r.split_at(16), (&blake2_128(b"foo")[..], &b"foo"[..])) } - - #[test] - fn test_storage_hasher_info() { - type KeyType = [u8; 15]; - let key: KeyType = [3u8; 15]; - - let mut r = &Identity::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(key)); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - - let mut r = &Twox64Concat::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(key)); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - - let mut r = &Twox128::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - - let mut r = &Twox256::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - - let mut r = &Blake2_128Concat::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(key)); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - - let mut r = &Blake2_128::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - - let mut r = &Blake2_256::hash(&(&key).encode()[..])[..]; - assert_eq!(>::decode_hash_and_then_info(&mut r), Ok(())); - assert_eq!(r.len(), 0); // Assert input has indeed decoded the hash. - } } diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 00ab605af29be..735ff53929bb7 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -18,7 +18,7 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{Ref, FullCodec, Decode, Encode, EncodeLike, EncodeAppend}; use crate::{storage::{self, unhashed}, traits::Len}; -use crate::hash::{StorageHasher, Twox128, StorageHasherInfo}; +use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher, FixedLengthHasher}; /// Generator for `StorageDoubleMap` used by `decl_storage`. /// @@ -45,10 +45,10 @@ pub trait StorageDoubleMap { type Query; /// Hasher for the first key. - type Hasher1: StorageHasher + StorageHasherInfo; + type Hasher1: StorageHasher; /// Hasher for the second key. - type Hasher2: StorageHasher + StorageHasherInfo; + type Hasher2: StorageHasher; /// Module prefix. Used for generating final key. fn module_prefix() -> &'static [u8]; @@ -341,77 +341,87 @@ impl storage::StorageDoubleMap for G where } fn iter_prefix_key2_value(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where - Self::Hasher2: StorageHasherInfo, + Self::Hasher2: ReversibleStorageHasher { let prefix = G::storage_double_map_final_key1(k1); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - closure: |raw_key, raw_value| { - from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) - .map(|(_k1, k2, v)| (k2, v)) - } + closure: from_hashed_key2_to_key2_value::<_, _, _, G>, } } fn drain_prefix_key2_value(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where - Self::Hasher2: StorageHasherInfo, + Self::Hasher2: ReversibleStorageHasher { let prefix = G::storage_double_map_final_key1(k1); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: true, - closure: |raw_key, raw_value| { - from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) - .map(|(_k1, k2, v)| (k2, v)) - } + closure: from_hashed_key2_to_key2_value::<_, _, _, G>, } } fn iter_key1_value() -> storage::PrefixIterator<(K1, V)> where - Self::Hasher1: StorageHasherInfo, + Self::Hasher1: ReversibleStorageHasher, { let prefix = G::prefix_hash(); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - closure: |raw_key, raw_value| { - from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) - .map(|(k1, _k2, v)| (k1, v)) + closure: |raw_key_without_prefix, mut raw_value| { + let mut key1_material = G::Hasher1::reverse(raw_key_without_prefix); + let key1 = K1::decode(&mut key1_material)?; + let value = V::decode(&mut raw_value)?; + + Ok((key1, value)) } } } fn iter_key2_value() -> storage::PrefixIterator<(K2, V)> where - Self::Hasher2: StorageHasherInfo, + Self::Hasher2: ReversibleStorageHasher, + Self::Hasher1: FixedLengthHasher, { let prefix = G::prefix_hash(); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - closure: |raw_key, raw_value| { - from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) - .map(|(_k1, k2, v)| (k2, v)) + closure: |raw_key_without_prefix, mut raw_value| { + let hasher1_len = G::Hasher1::LENGTH as usize; + if raw_key_without_prefix.len() < hasher1_len { + return Err("Invalid storage: invalid key length".into()) + } + let mut key2_material = G::Hasher2::reverse(&mut &raw_key_without_prefix[hasher1_len..]); + let key2 = K2::decode(&mut key2_material)?; + let value = V::decode(&mut raw_value)?; + + Ok((key2, value)) } } } fn iter_key1_key2_value() -> storage::PrefixIterator<(K1, K2, V)> where - Self::Hasher1: StorageHasherInfo, - Self::Hasher2: StorageHasherInfo, + Self::Hasher1: ReversibleStorageHasher, + Self::Hasher2: ReversibleStorageHasher, { let prefix = G::prefix_hash(); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - closure: |raw_key, raw_value| { - from_raw_to_key_info_value::<_, _, _, G>(raw_key, raw_value) - .map(|(k1, k2, v)| (k1, k2, v)) + closure: |mut raw_key_without_prefix, mut raw_value| { + let mut key1_key2_material = G::Hasher1::reverse(&mut raw_key_without_prefix); + let key1 = K1::decode(&mut key1_key2_material)?; + let mut key2_material = G::Hasher2::reverse(&mut key1_key2_material); + let key2 = K2::decode(&mut key2_material)?; + let value = V::decode(&mut raw_value)?; + + Ok((key1, key2, value)) } } } @@ -454,35 +464,21 @@ impl storage::StorageDoubleMap for G where } } -fn from_raw_to_key_info_value( - raw_key: &[u8], +fn from_hashed_key2_to_key2_value( + hashed_key2: &[u8], mut raw_value: &[u8] -) -> - Result< - ( - >::Info, - >::Info, - V, - ), - codec::Error, - > -where +) -> Result<(K2, V), codec::Error> where K1: FullCodec, K2: FullCodec, V: FullCodec, G: StorageDoubleMap, + G::Hasher2: ReversibleStorageHasher, { - let prefix_hash = G::prefix_hash(); - if raw_key.len() < prefix_hash.len() { - return Err("Input length is too small".into()) - } - - let mut decode_key_input = &raw_key[prefix_hash.len()..]; - let key1 = G::Hasher1::decode_hash_and_then_info(&mut decode_key_input)?; - let key2 = G::Hasher2::decode_hash_and_then_info(&mut decode_key_input)?; + let mut key2_material = G::Hasher2::reverse(hashed_key2); + let key2 = K2::decode(&mut key2_material)?; let value = V::decode(&mut raw_value)?; - Ok((key1, key2, value)) + Ok((key2, value)) } #[cfg(test)] diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index 768e6614545b2..be3f1367d07a7 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -19,7 +19,7 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{FullCodec, Decode, Encode, EncodeLike, Ref, EncodeAppend}; use crate::{storage::{self, unhashed}, traits::Len}; -use crate::hash::{StorageHasher, Twox128, StorageHasherInfo}; +use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// Generator for `StorageMap` used by `decl_storage`. /// @@ -37,7 +37,7 @@ pub trait StorageMap { type Query; /// Hasher. Used for generating final key. - type Hasher: StorageHasher + StorageHasherInfo; + type Hasher: StorageHasher; /// Module prefix. Used for generating final key. fn module_prefix() -> &'static [u8]; @@ -269,26 +269,26 @@ impl> storage::StorageMap } fn iter_key_value() -> storage::PrefixIterator<(K, V)> where - Self::Hasher: StorageHasherInfo + Self::Hasher: ReversibleStorageHasher { let prefix = G::prefix_hash(); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - closure: |raw_key, raw_value| from_raw_to_key_info_value::<_, _, G>(raw_key, raw_value), + closure: from_hashed_key_to_key_value::<_, _, G>, } } fn drain_key_value() -> storage::PrefixIterator<(K, V)> where - Self::Hasher: StorageHasherInfo + Self::Hasher: ReversibleStorageHasher { let prefix = G::prefix_hash(); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: true, - closure: |raw_key, raw_value| from_raw_to_key_info_value::<_, _, G>(raw_key, raw_value), + closure: from_hashed_key_to_key_value::<_, _, G>, } } @@ -303,7 +303,7 @@ impl> storage::StorageMap } fn translate_key_value Option>(f: F) where - Self::Hasher: StorageHasherInfo + Self::Hasher: ReversibleStorageHasher { let prefix = G::prefix_hash(); let mut previous_key = prefix.clone(); @@ -325,7 +325,8 @@ impl> storage::StorageMap // Slice in bound because already check by `starts_with` let mut hashed_key = &previous_key[prefix.len()..]; - let key = match G::Hasher::decode_hash_and_then_info(&mut hashed_key) { + let mut key_material = G::Hasher::reverse(&mut hashed_key); + let key = match K::decode(&mut key_material) { Ok(key) => key, Err(e) => { crate::debug::error!( @@ -378,21 +379,17 @@ impl> storage::StorageMap } } -fn from_raw_to_key_info_value( - raw_key: &[u8], +fn from_hashed_key_to_key_value( + hashed_key: &[u8], mut raw_value: &[u8] -) -> Result<(>::Info, V), codec::Error> where +) -> Result<(K, V), codec::Error> where K: FullCodec, V: FullCodec, G: StorageMap, + G::Hasher: ReversibleStorageHasher, { - let prefix_hash = G::prefix_hash(); - if raw_key.len() < prefix_hash.len() { - return Err("Input length is too small".into()) - } - - let mut decode_key_input = &raw_key[prefix_hash.len()..]; - let key = G::Hasher::decode_hash_and_then_info(&mut decode_key_input)?; + let mut key_material = G::Hasher::reverse(hashed_key); + let key = K::decode(&mut key_material)?; let value = V::decode(&mut raw_value)?; Ok((key, value)) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index c42444a508a3d..141e23a78358e 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -18,7 +18,7 @@ use sp_std::prelude::*; use codec::{FullCodec, FullEncode, Encode, EncodeAppend, EncodeLike, Decode}; -use crate::{traits::Len, hash::{StorageHasher, StorageHasherInfo}}; +use crate::{traits::Len, hash::{StorageHasher, ReversibleStorageHasher, FixedLengthHasher}}; pub mod unhashed; pub mod hashed; @@ -223,20 +223,16 @@ pub trait StorageMap { /// Enumerate all elements in the map in no particular order. If you alter the map while doing /// this, you'll get undefined results. - fn iter_key_value() -> PrefixIterator<(K, V)> where - Self::Hasher: StorageHasherInfo; + fn iter_key_value() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher; /// Remove all elements from the map and iterate through them in no particular order. If you /// add elements to the map while doing this, you'll get undefined results. - fn drain_key_value() -> PrefixIterator<(K, V)> where - Self::Hasher: StorageHasherInfo; + fn drain_key_value() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher; /// Enumerate all elements in the map in no particular order. If you alter the map while doing /// this, you'll get undefined results. #[deprecated(note = "please use `iter_value` instead")] - fn iter() -> PrefixIterator<(K, V)> where - Self::Hasher: StorageHasherInfo - { + fn iter() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher { Self::iter_key_value() } @@ -247,7 +243,7 @@ pub trait StorageMap { /// Translate the values of all elements by a function `f`, in the map in no particular order. /// By returning `None` from `f` for an element, you'll remove it from the map. fn translate_key_value Option>(f: F) where - Self::Hasher: StorageHasherInfo; + Self::Hasher: ReversibleStorageHasher; /// Translate the values from some previous `OldValue` to the current type. /// @@ -397,13 +393,13 @@ pub trait StorageDoubleMap { /// remove values whose first key is `k1` to the map while doing this, you'll get undefined /// results. fn iter_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where - Self::Hasher2: StorageHasherInfo; + Self::Hasher2: ReversibleStorageHasher; /// Remove all elements from the map with first key `k1` and iterate through them in no /// particular order. If you add elements with first key `k1` to the map while doing this, /// you'll get undefined results. fn drain_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where - Self::Hasher2: StorageHasherInfo; + Self::Hasher2: ReversibleStorageHasher; /// Iter over all value associated to the first key. #[deprecated(note = "please use `iter_prefix_value` instead")] @@ -417,16 +413,17 @@ pub trait StorageDoubleMap { /// Iter over all value and decode the first key. fn iter_key1_value() -> PrefixIterator<(K1, V)> where - Self::Hasher1: StorageHasherInfo; + Self::Hasher1: ReversibleStorageHasher; /// Iter over all value and decode the second key. fn iter_key2_value() -> PrefixIterator<(K2, V)> where - Self::Hasher2: StorageHasherInfo; + Self::Hasher1: FixedLengthHasher, + Self::Hasher2: ReversibleStorageHasher; /// Iter over all value and decode the first key and the second key. fn iter_key1_key2_value() -> PrefixIterator<(K1, K2, V)> where - Self::Hasher1: StorageHasherInfo, - Self::Hasher2: StorageHasherInfo; + Self::Hasher1: ReversibleStorageHasher, + Self::Hasher2: ReversibleStorageHasher; /// Iter over all value. fn iter_value() -> PrefixIterator; @@ -438,7 +435,8 @@ pub struct PrefixIterator { previous_key: Vec, /// If true then value are removed while iterating drain: bool, - /// Function that take `(raw_key, raw_value)` and decode `T` + /// Function that take `(raw_key_without_prefix, raw_value)` and decode `T`. + /// `raw_key_without_prefix` is the raw storage key without the prefix iterated on. closure: fn(&[u8], &[u8]) -> Result, } @@ -457,7 +455,8 @@ impl Iterator for PrefixIterator { if self.drain { unhashed::kill(&self.previous_key) } - match (self.closure)(&self.previous_key[..], &raw_value[..]) { + let raw_key_without_prefix = &self.previous_key[self.prefix.len()..]; + match (self.closure)(raw_key_without_prefix, &raw_value[..]) { Ok(t) => Some(t), Err(e) => { crate::debug::error!( diff --git a/frame/support/test/tests/decl_storage.rs b/frame/support/test/tests/decl_storage.rs index 97b89580e6659..126c0808c4b03 100644 --- a/frame/support/test/tests/decl_storage.rs +++ b/frame/support/test/tests/decl_storage.rs @@ -646,6 +646,7 @@ mod test_iterators { DoubleMapInfoInfo: double_map hasher(blake2_128_concat) u16, hasher(blake2_128_concat) u32 => u64; DoubleMapOpaqueInfo: double_map hasher(opaque_blake2_128) u16, hasher(blake2_128_concat) u32 => u64; + DoubleMapInfoOpaque: double_map hasher(blake2_128_concat) u16, hasher(opaque_blake2_128) u32 => u64; } } @@ -719,11 +720,6 @@ mod test_iterators { vec![(3, 3), (0, 0), (2, 2), (1, 1)], ); - assert_eq!( - DoubleMapInfoInfo::iter_key2_value().collect::>(), - vec![(3, 3), (0, 0), (2, 2), (1, 1)], - ); - assert_eq!( DoubleMapInfoInfo::iter_key1_key2_value().collect::>(), vec![(3, 3, 3), (0, 0, 0), (2, 2, 2), (1, 1, 1)], @@ -812,4 +808,51 @@ mod test_iterators { ); }) } + + #[test] + fn double_map_info_opaque_iteration() { + sp_io::TestExternalities::default().execute_with(|| { + // All map iterator + let prefix = DoubleMapInfoOpaque::prefix_hash(); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + DoubleMapInfoOpaque::insert(i as u16, i as u32, i as u64); + } + + assert_eq!( + DoubleMapInfoOpaque::iter_key1_value().collect::>(), + vec![(3, 3), (0, 0), (2, 2), (1, 1)], + ); + + assert_eq!( + DoubleMapInfoOpaque::iter_value().collect::>(), + vec![3, 0, 2, 1], + ); + + DoubleMapInfoOpaque::remove_all(); + + // Prefix iterator + let k1 = 3 << 8; + let prefix = DoubleMapInfoOpaque::storage_double_map_final_key1(k1); + + unhashed::put(&key_before_prefix(prefix.clone()), &1u64); + unhashed::put(&key_after_prefix(prefix.clone()), &1u64); + // Put an invalid storage value inside the map. + unhashed::put(&key_in_prefix(prefix), &1u8); + + for i in 0..4 { + DoubleMapInfoOpaque::insert(k1, i as u32, i as u64); + } + + assert_eq!( + DoubleMapInfoOpaque::iter_prefix_value(k1).collect::>(), + vec![0, 2, 1, 3], + ); + }) + } } From fdcd185733151294cef51afabf66dde6cd32d60d Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 25 Mar 2020 14:14:30 +0100 Subject: [PATCH 5/7] remove useless function --- frame/support/src/hash.rs | 21 ------- frame/support/src/lib.rs | 22 +++---- .../src/storage/generator/double_map.rs | 63 ++++--------------- frame/support/src/storage/generator/map.rs | 10 +-- frame/support/src/storage/generator/mod.rs | 6 +- frame/support/src/storage/mod.rs | 44 ++++--------- 6 files changed, 41 insertions(+), 125 deletions(-) diff --git a/frame/support/src/hash.rs b/frame/support/src/hash.rs index fd9e793fe6861..693e929a309e3 100644 --- a/frame/support/src/hash.rs +++ b/frame/support/src/hash.rs @@ -64,11 +64,6 @@ pub trait ReversibleStorageHasher: StorageHasher { fn reverse(x: &[u8]) -> &[u8]; } -/// Hasher with a fixed length. -pub trait FixedLengthHasher: StorageHasher { - const LENGTH: u32; -} - /// Store the key directly. pub struct Identity; impl StorageHasher for Identity { @@ -136,10 +131,6 @@ impl StorageHasher for Blake2_128 { } } -impl FixedLengthHasher for Blake2_128 { - const LENGTH: u32 = 16; -} - /// Hash storage keys with blake2 256 pub struct Blake2_256; impl StorageHasher for Blake2_256 { @@ -149,10 +140,6 @@ impl StorageHasher for Blake2_256 { } } -impl FixedLengthHasher for Blake2_256 { - const LENGTH: u32 = 32; -} - /// Hash storage keys with twox 128 pub struct Twox128; impl StorageHasher for Twox128 { @@ -162,10 +149,6 @@ impl StorageHasher for Twox128 { } } -impl FixedLengthHasher for Twox128 { - const LENGTH: u32 = 16; -} - /// Hash storage keys with twox 256 pub struct Twox256; impl StorageHasher for Twox256 { @@ -175,10 +158,6 @@ impl StorageHasher for Twox256 { } } -impl FixedLengthHasher for Twox256 { - const LENGTH: u32 = 32; -} - #[cfg(test)] mod tests { use super::*; diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 9addd2cbfee00..de64cf70e03bd 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -310,7 +310,7 @@ mod tests { OptionLinkedMap::insert(2, 2); OptionLinkedMap::insert(3, 3); - let collect = || OptionLinkedMap::iter_key_value().collect::>().sorted(); + let collect = || OptionLinkedMap::iter().collect::>().sorted(); assert_eq!(collect(), vec![(0, 0), (1, 1), (2, 2), (3, 3)]); // Two existing @@ -390,43 +390,43 @@ mod tests { #[test] fn map_iteration_should_work() { new_test_ext().execute_with(|| { - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(15, 42)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(15, 42)]); // insert / remove let key = 17u32; Map::insert(key, 4u64); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(15, 42), (key, 4)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(15, 42), (key, 4)]); assert_eq!(Map::take(&15), 42u64); assert_eq!(Map::take(&key), 4u64); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![]); + assert_eq!(Map::iter().collect::>().sorted(), vec![]); // Add couple of more elements Map::insert(key, 42u64); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key, 42)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(key, 42)]); Map::insert(key + 1, 43u64); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key, 42), (key + 1, 43)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(key, 42), (key + 1, 43)]); // mutate let key = key + 2; Map::mutate(&key, |val| { *val = 15; }); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 15)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 15)]); Map::mutate(&key, |val| { *val = 17; }); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 17)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43), (key, 17)]); // remove first Map::remove(&key); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 2, 42), (key - 1, 43)]); // remove last from the list Map::remove(&(key - 2)); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![(key - 1, 43)]); + assert_eq!(Map::iter().collect::>().sorted(), vec![(key - 1, 43)]); // remove the last element Map::remove(&(key - 1)); - assert_eq!(Map::iter_key_value().collect::>().sorted(), vec![]); + assert_eq!(Map::iter().collect::>().sorted(), vec![]); }); } diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 735ff53929bb7..030afef7bb336 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -18,7 +18,7 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{Ref, FullCodec, Decode, Encode, EncodeLike, EncodeAppend}; use crate::{storage::{self, unhashed}, traits::Len}; -use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher, FixedLengthHasher}; +use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// Generator for `StorageDoubleMap` used by `decl_storage`. /// @@ -214,18 +214,6 @@ impl storage::StorageDoubleMap for G where sp_io::storage::clear_prefix(&Self::prefix_hash()) } - fn iter_prefix_value(k1: KArg1) -> storage::PrefixIterator where - KArg1: ?Sized + EncodeLike - { - let prefix = Self::storage_double_map_final_key1(k1); - storage::PrefixIterator { - prefix: prefix.clone(), - previous_key: prefix, - drain: false, - closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), - } - } - fn mutate(k1: KArg1, k2: KArg2, f: F) -> R where KArg1: EncodeLike, KArg2: EncodeLike, @@ -340,7 +328,7 @@ impl storage::StorageDoubleMap for G where }) } - fn iter_prefix_key2_value(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where + fn iter_prefix(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where Self::Hasher2: ReversibleStorageHasher { let prefix = G::storage_double_map_final_key1(k1); @@ -352,7 +340,7 @@ impl storage::StorageDoubleMap for G where } } - fn drain_prefix_key2_value(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where + fn drain_prefix(k1: impl EncodeLike) -> storage::PrefixIterator<(K2, V)> where Self::Hasher2: ReversibleStorageHasher { let prefix = G::storage_double_map_final_key1(k1); @@ -364,48 +352,19 @@ impl storage::StorageDoubleMap for G where } } - fn iter_key1_value() -> storage::PrefixIterator<(K1, V)> where - Self::Hasher1: ReversibleStorageHasher, - { - let prefix = G::prefix_hash(); - storage::PrefixIterator { - prefix: prefix.clone(), - previous_key: prefix, - drain: false, - closure: |raw_key_without_prefix, mut raw_value| { - let mut key1_material = G::Hasher1::reverse(raw_key_without_prefix); - let key1 = K1::decode(&mut key1_material)?; - let value = V::decode(&mut raw_value)?; - - Ok((key1, value)) - } - } - } - - fn iter_key2_value() -> storage::PrefixIterator<(K2, V)> where - Self::Hasher2: ReversibleStorageHasher, - Self::Hasher1: FixedLengthHasher, + fn iter_prefix_values(k1: KArg1) -> storage::PrefixIterator where + KArg1: ?Sized + EncodeLike { - let prefix = G::prefix_hash(); + let prefix = Self::storage_double_map_final_key1(k1); storage::PrefixIterator { prefix: prefix.clone(), previous_key: prefix, drain: false, - closure: |raw_key_without_prefix, mut raw_value| { - let hasher1_len = G::Hasher1::LENGTH as usize; - if raw_key_without_prefix.len() < hasher1_len { - return Err("Invalid storage: invalid key length".into()) - } - let mut key2_material = G::Hasher2::reverse(&mut &raw_key_without_prefix[hasher1_len..]); - let key2 = K2::decode(&mut key2_material)?; - let value = V::decode(&mut raw_value)?; - - Ok((key2, value)) - } + closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), } } - fn iter_key1_key2_value() -> storage::PrefixIterator<(K1, K2, V)> where + fn iter() -> storage::PrefixIterator<(K1, K2, V)> where Self::Hasher1: ReversibleStorageHasher, Self::Hasher2: ReversibleStorageHasher, { @@ -426,7 +385,7 @@ impl storage::StorageDoubleMap for G where } } - fn iter_value() -> storage::PrefixIterator { + fn iter_values() -> storage::PrefixIterator { let prefix = G::prefix_hash(); storage::PrefixIterator { prefix: prefix.clone(), @@ -506,8 +465,8 @@ mod test { MyStorage::insert(2, 5, 9); MyStorage::insert(2, 6, 10); - assert_eq!(MyStorage::iter_prefix_value(1).collect::>(), vec![7, 8]); - assert_eq!(MyStorage::iter_prefix_value(2).collect::>(), vec![10, 9]); + assert_eq!(MyStorage::iter_prefix_values(1).collect::>(), vec![7, 8]); + assert_eq!(MyStorage::iter_prefix_values(2).collect::>(), vec![10, 9]); }); } } diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index be3f1367d07a7..49b5703e3957e 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -268,7 +268,7 @@ impl> storage::StorageMap }) } - fn iter_key_value() -> storage::PrefixIterator<(K, V)> where + fn iter() -> storage::PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher { let prefix = G::prefix_hash(); @@ -280,7 +280,7 @@ impl> storage::StorageMap } } - fn drain_key_value() -> storage::PrefixIterator<(K, V)> where + fn drain() -> storage::PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher { let prefix = G::prefix_hash(); @@ -292,7 +292,7 @@ impl> storage::StorageMap } } - fn iter_value() -> storage::PrefixIterator { + fn iter_values() -> storage::PrefixIterator { let prefix = Self::prefix_hash(); storage::PrefixIterator { prefix: prefix.to_vec(), @@ -302,7 +302,7 @@ impl> storage::StorageMap } } - fn translate_key_value Option>(f: F) where + fn translate Option>(f: F) where Self::Hasher: ReversibleStorageHasher { let prefix = G::prefix_hash(); @@ -347,7 +347,7 @@ impl> storage::StorageMap } } - fn translate_value(translate_val: TranslateV) -> Result<(), u32> + fn translate_values(translate_val: TranslateV) -> Result<(), u32> where OldV: Decode, TranslateV: Fn(OldV) -> V { let prefix = Self::prefix_hash(); diff --git a/frame/support/src/storage/generator/mod.rs b/frame/support/src/storage/generator/mod.rs index 6f970e3b76535..df919300ab740 100644 --- a/frame/support/src/storage/generator/mod.rs +++ b/frame/support/src/storage/generator/mod.rs @@ -89,15 +89,15 @@ mod tests { } assert_eq!( - NumberMap::iter_key_value().collect::>(), + NumberMap::iter().collect::>(), (0..100).map(|x| (x as u32, x as u64)).collect::>(), ); // do translation. - NumberMap::translate_key_value(|k: u32, v: u64| if k % 2 == 0 { Some((k as u64) << 32 | v) } else { None }); + NumberMap::translate(|k: u32, v: u64| if k % 2 == 0 { Some((k as u64) << 32 | v) } else { None }); assert_eq!( - NumberMap::iter_key_value().collect::>(), + NumberMap::iter().collect::>(), (0..50u32).map(|x| x * 2).map(|x| (x, (x as u64) << 32 | x as u64)).collect::>(), ); }) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 141e23a78358e..23e66e3e94a09 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -18,7 +18,7 @@ use sp_std::prelude::*; use codec::{FullCodec, FullEncode, Encode, EncodeAppend, EncodeLike, Decode}; -use crate::{traits::Len, hash::{StorageHasher, ReversibleStorageHasher, FixedLengthHasher}}; +use crate::{traits::Len, hash::{StorageHasher, ReversibleStorageHasher}}; pub mod unhashed; pub mod hashed; @@ -223,26 +223,19 @@ pub trait StorageMap { /// Enumerate all elements in the map in no particular order. If you alter the map while doing /// this, you'll get undefined results. - fn iter_key_value() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher; + fn iter() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher; /// Remove all elements from the map and iterate through them in no particular order. If you /// add elements to the map while doing this, you'll get undefined results. - fn drain_key_value() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher; + fn drain() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher; /// Enumerate all elements in the map in no particular order. If you alter the map while doing /// this, you'll get undefined results. - #[deprecated(note = "please use `iter_value` instead")] - fn iter() -> PrefixIterator<(K, V)> where Self::Hasher: ReversibleStorageHasher { - Self::iter_key_value() - } - - /// Enumerate all elements in the map in no particular order. If you alter the map while doing - /// this, you'll get undefined results. - fn iter_value() -> PrefixIterator; + fn iter_values() -> PrefixIterator; /// Translate the values of all elements by a function `f`, in the map in no particular order. /// By returning `None` from `f` for an element, you'll remove it from the map. - fn translate_key_value Option>(f: F) where + fn translate Option>(f: F) where Self::Hasher: ReversibleStorageHasher; /// Translate the values from some previous `OldValue` to the current type. @@ -263,7 +256,7 @@ pub trait StorageMap { /// This would typically be called inside the module implementation of on_runtime_upgrade, while /// ensuring **no usage of this storage are made before the call to `on_runtime_upgrade`**. (More /// precisely prior initialized modules doesn't make use of this storage). - fn translate_value(translate_val: TranslateV) -> Result<(), u32> + fn translate_values(translate_val: TranslateV) -> Result<(), u32> where OldV: Decode, TranslateV: Fn(OldV) -> V; } @@ -392,41 +385,26 @@ pub trait StorageDoubleMap { /// Enumerate all elements in the map with first key `k1` in no particular order. If you add or /// remove values whose first key is `k1` to the map while doing this, you'll get undefined /// results. - fn iter_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where + fn iter_prefix(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where Self::Hasher2: ReversibleStorageHasher; /// Remove all elements from the map with first key `k1` and iterate through them in no /// particular order. If you add elements with first key `k1` to the map while doing this, /// you'll get undefined results. - fn drain_prefix_key2_value(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where + fn drain_prefix(k1: impl EncodeLike) -> PrefixIterator<(K2, V)> where Self::Hasher2: ReversibleStorageHasher; /// Iter over all value associated to the first key. - #[deprecated(note = "please use `iter_prefix_value` instead")] - fn iter_prefix(k1: KArg1) -> PrefixIterator where KArg1: ?Sized + EncodeLike { - Self::iter_prefix_value(k1) - } - - /// Iter over all value associated to the first key. - fn iter_prefix_value(k1: KArg1) -> PrefixIterator where + fn iter_prefix_values(k1: KArg1) -> PrefixIterator where KArg1: ?Sized + EncodeLike; - /// Iter over all value and decode the first key. - fn iter_key1_value() -> PrefixIterator<(K1, V)> where - Self::Hasher1: ReversibleStorageHasher; - - /// Iter over all value and decode the second key. - fn iter_key2_value() -> PrefixIterator<(K2, V)> where - Self::Hasher1: FixedLengthHasher, - Self::Hasher2: ReversibleStorageHasher; - /// Iter over all value and decode the first key and the second key. - fn iter_key1_key2_value() -> PrefixIterator<(K1, K2, V)> where + fn iter() -> PrefixIterator<(K1, K2, V)> where Self::Hasher1: ReversibleStorageHasher, Self::Hasher2: ReversibleStorageHasher; /// Iter over all value. - fn iter_value() -> PrefixIterator; + fn iter_values() -> PrefixIterator; } /// Iterate over a prefix and decode raw_key and raw_value into `T`. From 3841e34c3af55e580efcd518ab0d46ba81fd16c6 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 25 Mar 2020 14:17:32 +0100 Subject: [PATCH 6/7] update test --- frame/support/test/tests/decl_storage.rs | 138 +++-------------------- 1 file changed, 16 insertions(+), 122 deletions(-) diff --git a/frame/support/test/tests/decl_storage.rs b/frame/support/test/tests/decl_storage.rs index 126c0808c4b03..4746921fbed8c 100644 --- a/frame/support/test/tests/decl_storage.rs +++ b/frame/support/test/tests/decl_storage.rs @@ -642,11 +642,9 @@ mod test_iterators { frame_support::decl_storage! { trait Store for Module as Test { - MapInfo: map hasher(blake2_128_concat) u16 => u64; + MapRev: map hasher(blake2_128_concat) u16 => u64; - DoubleMapInfoInfo: double_map hasher(blake2_128_concat) u16, hasher(blake2_128_concat) u32 => u64; - DoubleMapOpaqueInfo: double_map hasher(opaque_blake2_128) u16, hasher(blake2_128_concat) u32 => u64; - DoubleMapInfoOpaque: double_map hasher(blake2_128_concat) u16, hasher(opaque_blake2_128) u32 => u64; + DoubleMapRevRev: double_map hasher(blake2_128_concat) u16, hasher(blake2_128_concat) u32 => u64; } } @@ -677,7 +675,7 @@ mod test_iterators { #[test] fn map_info_iteration() { sp_io::TestExternalities::default().execute_with(|| { - let prefix = MapInfo::prefix_hash(); + let prefix = MapRev::prefix_hash(); unhashed::put(&key_before_prefix(prefix.clone()), &1u64); unhashed::put(&key_after_prefix(prefix.clone()), &1u64); @@ -685,26 +683,26 @@ mod test_iterators { unhashed::put(&key_in_prefix(prefix), &1u8); for i in 0..4 { - MapInfo::insert(i as u16, i as u64); + MapRev::insert(i as u16, i as u64); } assert_eq!( - MapInfo::iter_key_value().collect::>(), + MapRev::iter().collect::>(), vec![(3, 3), (0, 0), (2, 2), (1, 1)], ); assert_eq!( - MapInfo::iter_value().collect::>(), + MapRev::iter_values().collect::>(), vec![3, 0, 2, 1], ); }) } #[test] - fn double_map_info_info_iteration() { + fn double_map_reversible_reversible_iteration() { sp_io::TestExternalities::default().execute_with(|| { // All map iterator - let prefix = DoubleMapInfoInfo::prefix_hash(); + let prefix = DoubleMapRevRev::prefix_hash(); unhashed::put(&key_before_prefix(prefix.clone()), &1u64); unhashed::put(&key_after_prefix(prefix.clone()), &1u64); @@ -712,81 +710,24 @@ mod test_iterators { unhashed::put(&key_in_prefix(prefix), &1u8); for i in 0..4 { - DoubleMapInfoInfo::insert(i as u16, i as u32, i as u64); + DoubleMapRevRev::insert(i as u16, i as u32, i as u64); } assert_eq!( - DoubleMapInfoInfo::iter_key1_value().collect::>(), - vec![(3, 3), (0, 0), (2, 2), (1, 1)], - ); - - assert_eq!( - DoubleMapInfoInfo::iter_key1_key2_value().collect::>(), + DoubleMapRevRev::iter().collect::>(), vec![(3, 3, 3), (0, 0, 0), (2, 2, 2), (1, 1, 1)], ); assert_eq!( - DoubleMapInfoInfo::iter_value().collect::>(), - vec![3, 0, 2, 1], - ); - - DoubleMapInfoInfo::remove_all(); - - // Prefix iterator - let k1 = 3 << 8; - let prefix = DoubleMapInfoInfo::storage_double_map_final_key1(k1); - - unhashed::put(&key_before_prefix(prefix.clone()), &1u64); - unhashed::put(&key_after_prefix(prefix.clone()), &1u64); - // Put an invalid storage value inside the map. - unhashed::put(&key_in_prefix(prefix), &1u8); - - for i in 0..4 { - DoubleMapInfoInfo::insert(k1, i as u32, i as u64); - } - - assert_eq!( - DoubleMapInfoInfo::iter_prefix_key2_value(k1).collect::>(), - vec![(0, 0), (2, 2), (1, 1), (3, 3)], - ); - - assert_eq!( - DoubleMapInfoInfo::iter_prefix_value(k1).collect::>(), - vec![0, 2, 1, 3], - ); - }) - } - - #[test] - fn double_map_opaque_info_iteration() { - sp_io::TestExternalities::default().execute_with(|| { - // All map iterator - let prefix = DoubleMapOpaqueInfo::prefix_hash(); - - unhashed::put(&key_before_prefix(prefix.clone()), &1u64); - unhashed::put(&key_after_prefix(prefix.clone()), &1u64); - // Put an invalid storage value inside the map. - unhashed::put(&key_in_prefix(prefix), &1u8); - - for i in 0..4 { - DoubleMapOpaqueInfo::insert(i as u16, i as u32, i as u64); - } - - assert_eq!( - DoubleMapOpaqueInfo::iter_key2_value().collect::>(), - vec![(3, 3), (0, 0), (2, 2), (1, 1)], - ); - - assert_eq!( - DoubleMapOpaqueInfo::iter_value().collect::>(), + DoubleMapRevRev::iter_values().collect::>(), vec![3, 0, 2, 1], ); - DoubleMapOpaqueInfo::remove_all(); + DoubleMapRevRev::remove_all(); // Prefix iterator let k1 = 3 << 8; - let prefix = DoubleMapOpaqueInfo::storage_double_map_final_key1(k1); + let prefix = DoubleMapRevRev::storage_double_map_final_key1(k1); unhashed::put(&key_before_prefix(prefix.clone()), &1u64); unhashed::put(&key_after_prefix(prefix.clone()), &1u64); @@ -794,63 +735,16 @@ mod test_iterators { unhashed::put(&key_in_prefix(prefix), &1u8); for i in 0..4 { - DoubleMapOpaqueInfo::insert(k1, i as u32, i as u64); + DoubleMapRevRev::insert(k1, i as u32, i as u64); } assert_eq!( - DoubleMapOpaqueInfo::iter_prefix_key2_value(k1).collect::>(), + DoubleMapRevRev::iter_prefix(k1).collect::>(), vec![(0, 0), (2, 2), (1, 1), (3, 3)], ); assert_eq!( - DoubleMapOpaqueInfo::iter_prefix_value(k1).collect::>(), - vec![0, 2, 1, 3], - ); - }) - } - - #[test] - fn double_map_info_opaque_iteration() { - sp_io::TestExternalities::default().execute_with(|| { - // All map iterator - let prefix = DoubleMapInfoOpaque::prefix_hash(); - - unhashed::put(&key_before_prefix(prefix.clone()), &1u64); - unhashed::put(&key_after_prefix(prefix.clone()), &1u64); - // Put an invalid storage value inside the map. - unhashed::put(&key_in_prefix(prefix), &1u8); - - for i in 0..4 { - DoubleMapInfoOpaque::insert(i as u16, i as u32, i as u64); - } - - assert_eq!( - DoubleMapInfoOpaque::iter_key1_value().collect::>(), - vec![(3, 3), (0, 0), (2, 2), (1, 1)], - ); - - assert_eq!( - DoubleMapInfoOpaque::iter_value().collect::>(), - vec![3, 0, 2, 1], - ); - - DoubleMapInfoOpaque::remove_all(); - - // Prefix iterator - let k1 = 3 << 8; - let prefix = DoubleMapInfoOpaque::storage_double_map_final_key1(k1); - - unhashed::put(&key_before_prefix(prefix.clone()), &1u64); - unhashed::put(&key_after_prefix(prefix.clone()), &1u64); - // Put an invalid storage value inside the map. - unhashed::put(&key_in_prefix(prefix), &1u8); - - for i in 0..4 { - DoubleMapInfoOpaque::insert(k1, i as u32, i as u64); - } - - assert_eq!( - DoubleMapInfoOpaque::iter_prefix_value(k1).collect::>(), + DoubleMapRevRev::iter_prefix_values(k1).collect::>(), vec![0, 2, 1, 3], ); }) From 831df57c5bf54f9164ada00c1bf58dde5b41a05e Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 25 Mar 2020 14:40:34 +0100 Subject: [PATCH 7/7] rename doublemap translate to translate_values --- frame/support/src/storage/generator/double_map.rs | 2 +- frame/support/src/storage/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index 030afef7bb336..e35b7783f6765 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -395,7 +395,7 @@ impl storage::StorageDoubleMap for G where } } - fn translate Option>(f: F) { + fn translate_values Option>(f: F) { let prefix = G::prefix_hash(); let mut previous_key = prefix.clone(); loop { diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 23e66e3e94a09..205a335a761af 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -380,7 +380,7 @@ pub trait StorageDoubleMap { /// Translate the values of all elements by a function `f`, in the map in no particular order. /// By returning `None` from `f` for an element, you'll remove it from the map. - fn translate Option>(f: F); + fn translate_values Option>(f: F); /// Enumerate all elements in the map with first key `k1` in no particular order. If you add or /// remove values whose first key is `k1` to the map while doing this, you'll get undefined