From 9d98d157597c094f7ebfa0bdc2583be57f3c214c Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 12 Jun 2021 14:39:57 +0100 Subject: [PATCH 01/25] initial impl --- frame/support/src/storage/counted_map.rs | 307 +++++++++++++++++++++++ frame/support/src/storage/mod.rs | 1 + 2 files changed, 308 insertions(+) create mode 100644 frame/support/src/storage/counted_map.rs diff --git a/frame/support/src/storage/counted_map.rs b/frame/support/src/storage/counted_map.rs new file mode 100644 index 0000000000000..4ad4dbb94b05d --- /dev/null +++ b/frame/support/src/storage/counted_map.rs @@ -0,0 +1,307 @@ +use codec::{FullCodec, Decode, EncodeLike, Encode}; +use crate::{ + storage::{ + StorageMap, StorageValue, + StorageAppend, StorageTryAppend, StorageDecodeLength, StoragePrefixedMap, + generator::{StorageMap as StorageMapT, StorageValue as StorageValueT}, + }, +}; +use sp_std::prelude::*; +use sp_runtime::traits::Saturating; + +/// A wrapper around a `StorageMap` and a `StorageValue` to keep track of how many items are in +/// a map, without needing to iterate all the values. +/// +/// This storage item has additional storage read and write overhead when manipulating values +/// compared to a regular storage map. +/// +/// For functions where we only add or remove a value, a single storage read is needed to check if +/// that value already exists. For mutate functions, two storage reads are used to check if the +/// value existed before and after the mutation. +/// +/// Whenever the counter needs to be updated, an additional read and write occurs to update that +/// counter. +pub struct CountedStorageMap( + core::marker::PhantomData<(Key, Value, Map, Counter)> +); + +impl + StorageMapT + for CountedStorageMap +where + Key: FullCodec, + Value: FullCodec, + Map: StorageMapT, + Counter: StorageValueT +{ + type Query = Map::Query; + type Hasher = Map::Hasher; + fn module_prefix() -> &'static [u8] { + Map::module_prefix() + } + fn storage_prefix() -> &'static [u8] { + Map::storage_prefix() + } + fn from_optional_value_to_query(v: Option) -> Self::Query { + Map::from_optional_value_to_query(v) + } + fn from_query_to_optional_value(v: Self::Query) -> Option { + Map::from_query_to_optional_value(v) + } +} + +impl + StoragePrefixedMap for + CountedStorageMap +where + Value: FullCodec, + Map: crate::StoragePrefixedMap, + Counter: StorageValueT +{ + fn module_prefix() -> &'static [u8] { + Map::module_prefix() + } + fn storage_prefix() -> &'static [u8] { + Map::storage_prefix() + } +} + +impl + CountedStorageMap +where + Key: FullCodec, + Value: FullCodec, + Map: StorageMapT + crate::StoragePrefixedMap, + Counter: StorageValueT, +{ + // Internal helper function to track the counter as a value is mutated. + fn mutate_counter< + KeyArg: EncodeLike + Clone, + M: FnOnce(KeyArg, F) -> R, + F: FnOnce(I) -> R, + I, + R, + >(m: M, key: KeyArg, f: F) -> R { + let val_existed = Map::contains_key(key.clone()); + let res = m(key.clone(), f); + let val_exists = Map::contains_key(key); + + if val_existed && !val_exists { + // Value was deleted + Counter::mutate(|value| value.saturating_dec()); + } else if !val_existed && val_exists { + // Value was added + Counter::mutate(|value| value.saturating_inc()); + } + + res + } + + /// Get the storage key used to fetch a value corresponding to a specific key. + pub fn hashed_key_for>(key: KeyArg) -> Vec { + Map::hashed_key_for(key) + } + + /// Does the value (explicitly) exist in storage? + pub fn contains_key>(key: KeyArg) -> bool { + Map::contains_key(key) + } + + /// Load the value associated with the given key from the map. + pub fn get>(key: KeyArg) -> Map::Query { + Map::get(key) + } + + /// Try to get the value for the given key from the map. + /// + /// Returns `Ok` if it exists, `Err` if not. + pub fn try_get>(key: KeyArg) -> Result { + Map::try_get(key) + } + + /// Swap the values of two keys. + pub fn swap, KeyArg2: EncodeLike>(key1: KeyArg1, key2: KeyArg2) { + Map::swap(key1, key2) + } + + /// Store a value to be associated with the given key from the map. + pub fn insert + Clone, ValArg: EncodeLike>(key: KeyArg, val: ValArg) { + if !Map::contains_key(key.clone()) { + Counter::mutate(|value| value.saturating_inc()); + } + Map::insert(key, val) + } + + /// Remove the value under a key. + pub fn remove + Clone>(key: KeyArg) { + if Map::contains_key(key.clone()) { + Counter::mutate(|value| value.saturating_dec()); + } + Map::remove(key) + } + + /// Mutate the value under a key. + pub fn mutate + Clone, R, F: FnOnce(&mut Map::Query) -> R>( + key: KeyArg, + f: F + ) -> R { + Self::mutate_counter( + Map::mutate, + key, + f, + ) + } + + /// Mutate the item, only if an `Ok` value is returned. + pub fn try_mutate(key: KeyArg, f: F) -> Result + where + KeyArg: EncodeLike + Clone, + F: FnOnce(&mut Map::Query) -> Result, + { + Self::mutate_counter( + Map::try_mutate, + key, + f, + ) + } + + /// Mutate the value under a key. Deletes the item if mutated to a `None`. + pub fn mutate_exists + Clone, R, F: FnOnce(&mut Option) -> R>( + key: KeyArg, + f: F + ) -> R { + Self::mutate_counter( + Map::mutate_exists, + key, + f, + ) + } + + /// Mutate the item, only if an `Ok` value is returned. Deletes the item if mutated to a `None`. + pub fn try_mutate_exists(key: KeyArg, f: F) -> Result + where + KeyArg: EncodeLike + Clone, + F: FnOnce(&mut Option) -> Result, + { + Self::mutate_counter( + Map::try_mutate_exists, + key, + f, + ) + } + + /// Take the value under a key. + pub fn take + Clone>(key: KeyArg) -> Map::Query { + if Map::contains_key(key.clone()) { + Counter::mutate(|value| value.saturating_dec()); + } + Map::take(key) + } + + /// Append the given items to the value in the storage. + /// + /// `Value` is required to implement `codec::EncodeAppend`. + /// + /// # Warning + /// + /// If the storage item is not encoded properly, the storage will be overwritten and set to + /// `[item]`. Any default value set for the storage item will be ignored on overwrite. + pub fn append(key: EncodeLikeKey, item: EncodeLikeItem) + where + EncodeLikeKey: EncodeLike + Clone, + Item: Encode, + EncodeLikeItem: EncodeLike, + Value: StorageAppend + { + if !Map::contains_key(key.clone()) { + Counter::mutate(|value| value.saturating_inc()); + } + Map::append(key, item) + } + + /// Read the length of the storage value without decoding the entire value under the given + /// `key`. + /// + /// `Value` is required to implement [`StorageDecodeLength`]. + /// + /// If the value does not exists or it fails to decode the length, `None` is returned. Otherwise + /// `Some(len)` is returned. + /// + /// # Warning + /// + /// `None` does not mean that `get()` does not return a value. The default value is completly + /// ignored by this function. + pub fn decode_len>(key: KeyArg) -> Option + where Value: StorageDecodeLength, + { + Map::decode_len(key) + } + + /// Migrate an item with the given `key` from a defunct `OldHasher` to the current hasher. + /// + /// If the key doesn't exist, then it's a no-op. If it does, then it returns its value. + pub fn migrate_key>( + key: KeyArg + ) -> Option { + Map::migrate_key::(key) + } + + /// Remove all value of the storage. + pub fn remove_all() { + Counter::set(0u32); + Map::remove_all() + } + + /// Iter over all value of the storage. + /// + /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. + pub fn iter_values() -> crate::storage::PrefixIterator { + Map::iter_values() + } + + /// 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. + /// + /// NOTE: If a value fail to decode because storage is corrupted then it is skipped. + /// + /// # 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. + pub fn translate_values Option>(f: F) { + Map::translate_values(f) + } + + /// Try and append the given item to the value in the storage. + /// + /// Is only available if `Value` of the storage implements [`StorageTryAppend`]. + pub fn try_append( + key: KArg, + item: EncodeLikeItem, + ) -> Result<(), ()> + where + KArg: EncodeLike + Clone, + Item: Encode, + EncodeLikeItem: EncodeLike, + Value: StorageTryAppend, + { + < + Self as crate::storage::TryAppendMap + >::try_append(key, item) + } + + /// Initialize the counter with the actual number of items in the map. + /// + /// This function iterates through all the items in the map and sets the counter. This operation + /// can be very heavy, so use with caution. + pub fn initialize_counter() -> u32 { + let count = Self::iter_values().count() as u32; + Counter::set(count); + count + } +} diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 34d217f5c31b6..e572536c2b02d 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -37,6 +37,7 @@ pub mod bounded_btree_map; pub mod bounded_btree_set; pub mod bounded_vec; pub mod weak_bounded_vec; +pub mod counted_map; pub mod child; #[doc(hidden)] pub mod generator; From 989d02bab7f4d8433443a70837896dead9d2290e Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 12 Jun 2021 15:47:21 +0100 Subject: [PATCH 02/25] expose in pallet_prelude --- frame/support/src/lib.rs | 2 ++ frame/support/src/storage/counted_map.rs | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 1d4d7e461834c..79ca4cbdaa7fb 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -77,6 +77,7 @@ pub use self::storage::{ StorageValue, StorageMap, StorageDoubleMap, StorageNMap, StoragePrefixedMap, IterableStorageMap, IterableStorageDoubleMap, IterableStorageNMap, migration, bounded_vec::{BoundedVec, BoundedSlice}, weak_bounded_vec::WeakBoundedVec, + counted_map::CountedStorageMap, }; pub use self::dispatch::{Parameter, Callable}; pub use sp_runtime::{self, ConsensusEngineId, print, traits::Printable}; @@ -1248,6 +1249,7 @@ pub mod pallet_prelude { OptionQuery, }, storage::bounded_vec::BoundedVec, + CountedStorageMap, }; pub use codec::{Encode, Decode}; pub use crate::inherent::{InherentData, InherentIdentifier, ProvideInherent}; diff --git a/frame/support/src/storage/counted_map.rs b/frame/support/src/storage/counted_map.rs index 4ad4dbb94b05d..d5bf72dfa7146 100644 --- a/frame/support/src/storage/counted_map.rs +++ b/frame/support/src/storage/counted_map.rs @@ -299,9 +299,16 @@ where /// /// This function iterates through all the items in the map and sets the counter. This operation /// can be very heavy, so use with caution. + /// + /// Returns the number of items in the map which is used to set the counter. pub fn initialize_counter() -> u32 { let count = Self::iter_values().count() as u32; Counter::set(count); count } + + /// Return the count. + pub fn count() -> u32 { + Counter::get() + } } From a6321b1b9493c5c0e0e1bfa69b134b43cafb9d4a Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 12 Jun 2021 15:59:17 +0100 Subject: [PATCH 03/25] temp test --- frame/example/src/lib.rs | 8 +++++++ frame/example/src/tests.rs | 44 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index f5014b75640ba..225022beacbf4 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -598,6 +598,14 @@ pub mod pallet { #[pallet::getter(fn foo)] pub(super) type Foo = StorageValue<_, T::Balance, ValueQuery>; + #[pallet::storage] + pub type MyMap = StorageMap<_, Blake2_128Concat, u8, u16>; + + #[pallet::storage] + pub type MyCounter = StorageValue<_, u32, ValueQuery>; + + pub type CountedMap = CountedStorageMap, MyCounter>; + // The genesis config type. #[pallet::genesis_config] pub struct GenesisConfig { diff --git a/frame/example/src/tests.rs b/frame/example/src/tests.rs index a290ea0f6576f..79a3321d35ea8 100644 --- a/frame/example/src/tests.rs +++ b/frame/example/src/tests.rs @@ -175,6 +175,50 @@ fn signed_ext_watch_dummy_works() { }) } +#[test] +fn counted_map_works() { + new_test_ext().execute_with(|| { + assert_eq!(CountedMap::::count(), 0); + CountedMap::::insert(3, 3); + assert_eq!(CountedMap::::count(), 1); + CountedMap::::insert(3, 5); + assert_eq!(CountedMap::::count(), 1); + CountedMap::::insert(5, 5); + assert_eq!(CountedMap::::count(), 2); + CountedMap::::remove(5); + assert_eq!(CountedMap::::count(), 1); + // Does nothing + CountedMap::::take(4); + assert_eq!(CountedMap::::count(), 1); + CountedMap::::take(3); + assert_eq!(CountedMap::::count(), 0); + + CountedMap::::mutate(3, |old| { + if old.is_none() { + *old = Some(69) + } else { + *old = Some(420) + } + }); + assert_eq!(CountedMap::::get(3), Some(69)); + assert_eq!(CountedMap::::count(), 1); + + CountedMap::::mutate(3, |old| { + if old.is_none() { + *old = Some(69) + } else { + *old = Some(420) + } + }); + assert_eq!(CountedMap::::get(3), Some(420)); + assert_eq!(CountedMap::::count(), 1); + + CountedMap::::mutate(3, |old| *old = None); + assert_eq!(CountedMap::::get(3), None); + assert_eq!(CountedMap::::count(), 0); + }) +} + #[test] fn weights_work() { // must have a defined weight. From 9ce58da452bb6b82f72b23fa8921626694c2cef5 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Mon, 14 Jun 2021 16:37:57 +0200 Subject: [PATCH 04/25] Apply suggestions from code review Co-authored-by: Peter Goodspeed-Niklaus Co-authored-by: Xiliang Chen --- frame/support/src/storage/counted_map.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/support/src/storage/counted_map.rs b/frame/support/src/storage/counted_map.rs index d5bf72dfa7146..d233f1107634c 100644 --- a/frame/support/src/storage/counted_map.rs +++ b/frame/support/src/storage/counted_map.rs @@ -83,7 +83,7 @@ where R, >(m: M, key: KeyArg, f: F) -> R { let val_existed = Map::contains_key(key.clone()); - let res = m(key.clone(), f); + let res = m(&key, f); let val_exists = Map::contains_key(key); if val_existed && !val_exists { @@ -126,7 +126,7 @@ where /// Store a value to be associated with the given key from the map. pub fn insert + Clone, ValArg: EncodeLike>(key: KeyArg, val: ValArg) { - if !Map::contains_key(key.clone()) { + if !Map::contains_key(&key) { Counter::mutate(|value| value.saturating_inc()); } Map::insert(key, val) @@ -134,7 +134,7 @@ where /// Remove the value under a key. pub fn remove + Clone>(key: KeyArg) { - if Map::contains_key(key.clone()) { + if Map::contains_key(&key) { Counter::mutate(|value| value.saturating_dec()); } Map::remove(key) @@ -213,7 +213,7 @@ where EncodeLikeItem: EncodeLike, Value: StorageAppend { - if !Map::contains_key(key.clone()) { + if !Map::contains_key(&key) { Counter::mutate(|value| value.saturating_inc()); } Map::append(key, item) From 899eea4a592f57b2962e162d4772e76f94ea06db Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 14 Jun 2021 19:53:41 +0200 Subject: [PATCH 05/25] implement with macro help. --- frame/example/src/lib.rs | 7 +- .../procedural/src/pallet/expand/storage.rs | 126 +++++++++- .../procedural/src/pallet/parse/storage.rs | 45 ++++ frame/support/src/lib.rs | 4 +- frame/support/src/storage/mod.rs | 1 - .../src/storage/{ => types}/counted_map.rs | 225 +++++++++++------- frame/support/src/storage/types/mod.rs | 2 + 7 files changed, 311 insertions(+), 99 deletions(-) rename frame/support/src/storage/{ => types}/counted_map.rs (52%) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index 225022beacbf4..db609ed7b9110 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -599,12 +599,7 @@ pub mod pallet { pub(super) type Foo = StorageValue<_, T::Balance, ValueQuery>; #[pallet::storage] - pub type MyMap = StorageMap<_, Blake2_128Concat, u8, u16>; - - #[pallet::storage] - pub type MyCounter = StorageValue<_, u32, ValueQuery>; - - pub type CountedMap = CountedStorageMap, MyCounter>; + pub type CountedMap = CountedStorageMap<_, Blake2_128Concat, u8, u16>; // The genesis config type. #[pallet::genesis_config] diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index c956425379c53..d0a6e814a36a3 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -19,12 +19,21 @@ use crate::pallet::Def; use crate::pallet::parse::storage::{Metadata, QueryKind, StorageGenerics}; use frame_support_procedural_tools::clean_type_string; -/// Generate the prefix_ident related the the storage. +/// Generate the prefix_ident related to the storage. /// prefix_ident is used for the prefix struct to be given to storage as first generic param. fn prefix_ident(storage_ident: &syn::Ident) -> syn::Ident { syn::Ident::new(&format!("_GeneratedPrefixForStorage{}", storage_ident), storage_ident.span()) } +/// Generate the counter_prefix_ident related to the storage. +/// counter_prefix_ident is used for the prefix struct to be given to counted storage map. +fn counter_prefix_ident(storage_ident: &syn::Ident) -> syn::Ident { + syn::Ident::new( + &format!("_GeneratedCounterPrefixForStorage{}", storage_ident), + storage_ident.span(), + ) +} + /// * if generics are unnamed: replace the first generic `_` by the generated prefix structure /// * if generics are named: reorder the generic, remove their name, and add the missing ones. /// * Add `#[allow(type_alias_bounds)]` @@ -86,6 +95,19 @@ pub fn process_generics(def: &mut Def) { let max_values = max_values.unwrap_or_else(|| default_max_values.clone()); args.args.push(syn::GenericArgument::Type(max_values)); } + StorageGenerics::CountedMap { + hasher, key, value, query_kind, on_empty, max_values + } => { + args.args.push(syn::GenericArgument::Type(hasher)); + args.args.push(syn::GenericArgument::Type(key)); + args.args.push(syn::GenericArgument::Type(value)); + let query_kind = query_kind.unwrap_or_else(|| default_query_kind.clone()); + args.args.push(syn::GenericArgument::Type(query_kind)); + let on_empty = on_empty.unwrap_or_else(|| default_on_empty.clone()); + args.args.push(syn::GenericArgument::Type(on_empty)); + let max_values = max_values.unwrap_or_else(|| default_max_values.clone()); + args.args.push(syn::GenericArgument::Type(max_values)); + } StorageGenerics::DoubleMap { hasher1, key1, hasher2, key2, value, query_kind, on_empty, max_values, } => { @@ -149,6 +171,9 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { Metadata::Map { .. } => quote::quote_spanned!(storage.attr_span => #frame_support::storage::types::StorageMapMetadata ), + Metadata::CountedMap { .. } => quote::quote_spanned!(storage.attr_span => + #frame_support::storage::types::CountedStorageMapMetadata + ), Metadata::DoubleMap { .. } => quote::quote_spanned!(storage.attr_span => #frame_support::storage::types::StorageDoubleMapMetadata ), @@ -178,6 +203,18 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { } ) }, + Metadata::CountedMap { key, value } => { + let value = clean_type_string("e::quote!(#value).to_string()); + let key = clean_type_string("e::quote!(#key).to_string()); + quote::quote_spanned!(storage.attr_span => + #frame_support::metadata::StorageEntryType::Map { + hasher: <#full_ident as #metadata_trait>::HASHER, + key: #frame_support::metadata::DecodeDifferent::Encode(#key), + value: #frame_support::metadata::DecodeDifferent::Encode(#value), + unused: false, + } + ) + }, Metadata::DoubleMap { key1, key2, value } => { let value = clean_type_string("e::quote!(#value).to_string()); let key1 = clean_type_string("e::quote!(#key1).to_string()); @@ -212,7 +249,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { } }; - quote::quote_spanned!(storage.attr_span => + let mut metadata = quote::quote_spanned!(storage.attr_span => #(#cfg_attrs)* #frame_support::metadata::StorageEntryMetadata { name: #frame_support::metadata::DecodeDifferent::Encode( <#full_ident as #metadata_trait>::NAME @@ -226,7 +263,32 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { #( #docs, )* ]), } - ) + ); + + // Additional metadata for some storages: + if let Metadata::CountedMap { .. } = storage.metadata { + metadata.extend(quote::quote_spanned!(storage.attr_span => + , #(#cfg_attrs)* #frame_support::metadata::StorageEntryMetadata { + name: #frame_support::metadata::DecodeDifferent::Encode( + <#full_ident as #metadata_trait>::COUNTER_NAME + ), + modifier: <#full_ident as #metadata_trait>::COUNTER_MODIFIER, + ty: #frame_support::metadata::StorageEntryType::Plain( + #frame_support::metadata::DecodeDifferent::Encode( + <#full_ident as #metadata_trait>::COUNTER_TY + ) + ), + default: #frame_support::metadata::DecodeDifferent::Encode( + <#full_ident as #metadata_trait>::COUNTER_DEFAULT + ), + documentation: #frame_support::metadata::DecodeDifferent::Encode(&[ + <#full_ident as #metadata_trait>::COUNTER_DOC + ]), + } + )); + } + + metadata }); let getters = def.storages.iter() @@ -287,6 +349,27 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { } ) }, + Metadata::CountedMap { key, value } => { + let query = match storage.query_kind.as_ref().expect("Checked by def") { + QueryKind::OptionQuery => quote::quote_spanned!(storage.attr_span => + Option<#value> + ), + QueryKind::ValueQuery => quote::quote!(#value), + }; + quote::quote_spanned!(storage.attr_span => + #(#cfg_attrs)* + impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause { + #( #docs )* + pub fn #getter(k: KArg) -> #query where + KArg: #frame_support::codec::EncodeLike<#key>, + { + // NOTE: we can't use any trait here because CountedStorageMap + // doesn't implement any. + #full_ident::get(k) + } + } + ) + }, Metadata::DoubleMap { key1, key2, value } => { let query = match storage.query_kind.as_ref().expect("Checked by def") { QueryKind::OptionQuery => quote::quote_spanned!(storage.attr_span => @@ -351,7 +434,44 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { let cfg_attrs = &storage_def.cfg_attrs; + let maybe_counter = if let Metadata::CountedMap { .. } = storage_def.metadata { + let counter_prefix_struct_ident = counter_prefix_ident(&storage_def.ident); + let counter_prefix_struct_const = format!("CounterFor{}", prefix_struct_const); + + quote::quote_spanned!(storage_def.attr_span => + #(#cfg_attrs)* + #prefix_struct_vis struct #counter_prefix_struct_ident<#type_use_gen>( + core::marker::PhantomData<(#type_use_gen,)> + ); + #(#cfg_attrs)* + impl<#type_impl_gen> #frame_support::traits::StorageInstance + for #counter_prefix_struct_ident<#type_use_gen> + #config_where_clause + { + fn pallet_prefix() -> &'static str { + < + ::PalletInfo + as #frame_support::traits::PalletInfo + >::name::>() + .expect("Every active pallet has a name in the runtime; qed") + } + const STORAGE_PREFIX: &'static str = #counter_prefix_struct_const; + } + #(#cfg_attrs)* + impl<#type_impl_gen> #frame_support::storage::types::CountedStorageMapInstance + for #prefix_struct_ident<#type_use_gen> + #config_where_clause + { + type CounterPrefix = #counter_prefix_struct_ident<#type_use_gen>; + } + ) + } else { + proc_macro2::TokenStream::default() + }; + quote::quote_spanned!(storage_def.attr_span => + #maybe_counter + #(#cfg_attrs)* #prefix_struct_vis struct #prefix_struct_ident<#type_use_gen>( core::marker::PhantomData<(#type_use_gen,)> diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 6b842ab7fa401..a6c3ca138b76f 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -54,6 +54,7 @@ impl syn::parse::Parse for PalletStorageAttr { pub enum Metadata { Value { value: syn::Type }, Map { value: syn::Type, key: syn::Type }, + CountedMap { value: syn::Type, key: syn::Type }, DoubleMap { value: syn::Type, key1: syn::Type, @@ -127,6 +128,14 @@ pub enum StorageGenerics { on_empty: Option, max_values: Option, }, + CountedMap { + hasher: syn::Type, + key: syn::Type, + value: syn::Type, + query_kind: Option, + on_empty: Option, + max_values: Option, + }, Value { value: syn::Type, query_kind: Option, @@ -147,6 +156,7 @@ impl StorageGenerics { let res = match self.clone() { Self::DoubleMap { value, key1, key2, .. } => Metadata::DoubleMap { value, key1, key2 }, Self::Map { value, key, .. } => Metadata::Map { value, key }, + Self::CountedMap { value, key, .. } => Metadata::CountedMap { value, key }, Self::Value { value, .. } => Metadata::Value { value }, Self::NMap { keygen, value, .. } => Metadata::NMap { keys: collect_keys(&keygen)?, @@ -163,6 +173,7 @@ impl StorageGenerics { match &self { Self::DoubleMap { query_kind, .. } | Self::Map { query_kind, .. } + | Self::CountedMap { query_kind, .. } | Self::Value { query_kind, .. } | Self::NMap { query_kind, .. } => query_kind.clone(), @@ -173,6 +184,7 @@ impl StorageGenerics { enum StorageKind { Value, Map, + CountedMap, DoubleMap, NMap, } @@ -300,6 +312,30 @@ fn process_named_generics( max_values: parsed.remove("MaxValues").map(|binding| binding.ty), } } + StorageKind::CountedMap => { + check_generics( + &parsed, + &["Hasher", "Key", "Value"], + &["QueryKind", "OnEmpty", "MaxValues"], + "CountedStorageMap", + args_span, + )?; + + StorageGenerics::CountedMap { + hasher: parsed.remove("Hasher") + .map(|binding| binding.ty) + .expect("checked above as mandatory generic"), + key: parsed.remove("Key") + .map(|binding| binding.ty) + .expect("checked above as mandatory generic"), + value: parsed.remove("Value") + .map(|binding| binding.ty) + .expect("checked above as mandatory generic"), + query_kind: parsed.remove("QueryKind").map(|binding| binding.ty), + on_empty: parsed.remove("OnEmpty").map(|binding| binding.ty), + max_values: parsed.remove("MaxValues").map(|binding| binding.ty), + } + } StorageKind::DoubleMap => { check_generics( &parsed, @@ -403,6 +439,14 @@ fn process_unnamed_generics( }, retrieve_arg(4).ok(), ), + StorageKind::CountedMap => ( + None, + Metadata::CountedMap { + key: retrieve_arg(2)?, + value: retrieve_arg(3)?, + }, + retrieve_arg(4).ok(), + ), StorageKind::DoubleMap => ( None, Metadata::DoubleMap { @@ -437,6 +481,7 @@ fn process_generics( let storage_kind = match &*segment.ident.to_string() { "StorageValue" => StorageKind::Value, "StorageMap" => StorageKind::Map, + "CountedStorageMap" => StorageKind::CountedMap, "StorageDoubleMap" => StorageKind::DoubleMap, "StorageNMap" => StorageKind::NMap, found => { diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 79ca4cbdaa7fb..a36012c6e4670 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -77,7 +77,6 @@ pub use self::storage::{ StorageValue, StorageMap, StorageDoubleMap, StorageNMap, StoragePrefixedMap, IterableStorageMap, IterableStorageDoubleMap, IterableStorageNMap, migration, bounded_vec::{BoundedVec, BoundedSlice}, weak_bounded_vec::WeakBoundedVec, - counted_map::CountedStorageMap, }; pub use self::dispatch::{Parameter, Callable}; pub use sp_runtime::{self, ConsensusEngineId, print, traits::Printable}; @@ -1246,10 +1245,9 @@ pub mod pallet_prelude { weights::{DispatchClass, Pays, Weight}, storage::types::{ Key as NMapKey, StorageDoubleMap, StorageMap, StorageNMap, StorageValue, ValueQuery, - OptionQuery, + OptionQuery, CountedStorageMap, }, storage::bounded_vec::BoundedVec, - CountedStorageMap, }; pub use codec::{Encode, Decode}; pub use crate::inherent::{InherentData, InherentIdentifier, ProvideInherent}; diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index e572536c2b02d..34d217f5c31b6 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -37,7 +37,6 @@ pub mod bounded_btree_map; pub mod bounded_btree_set; pub mod bounded_vec; pub mod weak_bounded_vec; -pub mod counted_map; pub mod child; #[doc(hidden)] pub mod generator; diff --git a/frame/support/src/storage/counted_map.rs b/frame/support/src/storage/types/counted_map.rs similarity index 52% rename from frame/support/src/storage/counted_map.rs rename to frame/support/src/storage/types/counted_map.rs index d233f1107634c..0baf14d2f90ef 100644 --- a/frame/support/src/storage/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -1,13 +1,20 @@ use codec::{FullCodec, Decode, EncodeLike, Encode}; use crate::{ + traits::{StorageInstance, GetDefault, Get, StorageInfo}, storage::{ - StorageMap, StorageValue, - StorageAppend, StorageTryAppend, StorageDecodeLength, StoragePrefixedMap, - generator::{StorageMap as StorageMapT, StorageValue as StorageValueT}, + StorageAppend, StorageTryAppend, StorageDecodeLength, + types::{ + OptionQuery, ValueQuery, StorageMap, StorageValue, QueryKindTrait, + StorageMapMetadata, StorageValueMetadata + }, }, }; use sp_std::prelude::*; use sp_runtime::traits::Saturating; +use frame_metadata::{ + DefaultByteGetter, StorageEntryModifier, +}; +use max_encoded_len::MaxEncodedLen; /// A wrapper around a `StorageMap` and a `StorageValue` to keep track of how many items are in /// a map, without needing to iterate all the values. @@ -21,58 +28,46 @@ use sp_runtime::traits::Saturating; /// /// Whenever the counter needs to be updated, an additional read and write occurs to update that /// counter. -pub struct CountedStorageMap( - core::marker::PhantomData<(Key, Value, Map, Counter)> +/// +/// The storage prefix for the storage value is the given prefix concatenated with `"Counter"`. +pub struct CountedStorageMap< + Prefix, Hasher, Key, Value, QueryKind=OptionQuery, OnEmpty=GetDefault, MaxValues=GetDefault, +>( + core::marker::PhantomData<(Prefix, Hasher, Key, Value, QueryKind, OnEmpty, MaxValues)> ); -impl - StorageMapT - for CountedStorageMap -where - Key: FullCodec, - Value: FullCodec, - Map: StorageMapT, - Counter: StorageValueT -{ - type Query = Map::Query; - type Hasher = Map::Hasher; - fn module_prefix() -> &'static [u8] { - Map::module_prefix() - } - fn storage_prefix() -> &'static [u8] { - Map::storage_prefix() - } - fn from_optional_value_to_query(v: Option) -> Self::Query { - Map::from_optional_value_to_query(v) - } - fn from_query_to_optional_value(v: Self::Query) -> Option { - Map::from_query_to_optional_value(v) - } +/// The requirement for an instance of [`CountedStorageMap`]. +pub trait CountedStorageMapInstance: StorageInstance { + /// The prefix to use for the counter storage value. + type CounterPrefix: StorageInstance; } -impl - StoragePrefixedMap for - CountedStorageMap -where - Value: FullCodec, - Map: crate::StoragePrefixedMap, - Counter: StorageValueT +// Internal helper trait to access map from counted storage map. +trait Helper { + type Map; + type Counter; +} + +impl + Helper + for CountedStorageMap { - fn module_prefix() -> &'static [u8] { - Map::module_prefix() - } - fn storage_prefix() -> &'static [u8] { - Map::storage_prefix() - } + type Map = StorageMap; + type Counter = StorageValue; } -impl - CountedStorageMap +// Do not use storage value, simply implement some get/set/inc/dec + +impl + CountedStorageMap where + Prefix: CountedStorageMapInstance, + Hasher: crate::hash::StorageHasher, Key: FullCodec, Value: FullCodec, - Map: StorageMapT + crate::StoragePrefixedMap, - Counter: StorageValueT, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, { // Internal helper function to track the counter as a value is mutated. fn mutate_counter< @@ -82,16 +77,16 @@ where I, R, >(m: M, key: KeyArg, f: F) -> R { - let val_existed = Map::contains_key(key.clone()); - let res = m(&key, f); - let val_exists = Map::contains_key(key); + let val_existed = ::Map::contains_key(key.clone()); + let res = m(key.clone(), f); + let val_exists = ::Map::contains_key(key); if val_existed && !val_exists { // Value was deleted - Counter::mutate(|value| value.saturating_dec()); + ::Counter::mutate(|value| value.saturating_dec()); } else if !val_existed && val_exists { // Value was added - Counter::mutate(|value| value.saturating_inc()); + ::Counter::mutate(|value| value.saturating_inc()); } res @@ -99,54 +94,54 @@ where /// Get the storage key used to fetch a value corresponding to a specific key. pub fn hashed_key_for>(key: KeyArg) -> Vec { - Map::hashed_key_for(key) + ::Map::hashed_key_for(key) } /// Does the value (explicitly) exist in storage? pub fn contains_key>(key: KeyArg) -> bool { - Map::contains_key(key) + ::Map::contains_key(key) } /// Load the value associated with the given key from the map. - pub fn get>(key: KeyArg) -> Map::Query { - Map::get(key) + pub fn get>(key: KeyArg) -> QueryKind::Query { + ::Map::get(key) } /// Try to get the value for the given key from the map. /// /// Returns `Ok` if it exists, `Err` if not. pub fn try_get>(key: KeyArg) -> Result { - Map::try_get(key) + ::Map::try_get(key) } /// Swap the values of two keys. pub fn swap, KeyArg2: EncodeLike>(key1: KeyArg1, key2: KeyArg2) { - Map::swap(key1, key2) + ::Map::swap(key1, key2) } /// Store a value to be associated with the given key from the map. pub fn insert + Clone, ValArg: EncodeLike>(key: KeyArg, val: ValArg) { - if !Map::contains_key(&key) { - Counter::mutate(|value| value.saturating_inc()); + if !::Map::contains_key(key.clone()) { + ::Counter::mutate(|value| value.saturating_inc()); } - Map::insert(key, val) + ::Map::insert(key, val) } /// Remove the value under a key. pub fn remove + Clone>(key: KeyArg) { - if Map::contains_key(&key) { - Counter::mutate(|value| value.saturating_dec()); + if ::Map::contains_key(key.clone()) { + ::Counter::mutate(|value| value.saturating_dec()); } - Map::remove(key) + ::Map::remove(key) } /// Mutate the value under a key. - pub fn mutate + Clone, R, F: FnOnce(&mut Map::Query) -> R>( + pub fn mutate + Clone, R, F: FnOnce(&mut QueryKind::Query) -> R>( key: KeyArg, f: F ) -> R { Self::mutate_counter( - Map::mutate, + ::Map::mutate, key, f, ) @@ -156,10 +151,10 @@ where pub fn try_mutate(key: KeyArg, f: F) -> Result where KeyArg: EncodeLike + Clone, - F: FnOnce(&mut Map::Query) -> Result, + F: FnOnce(&mut QueryKind::Query) -> Result, { Self::mutate_counter( - Map::try_mutate, + ::Map::try_mutate, key, f, ) @@ -171,7 +166,7 @@ where f: F ) -> R { Self::mutate_counter( - Map::mutate_exists, + ::Map::mutate_exists, key, f, ) @@ -184,18 +179,18 @@ where F: FnOnce(&mut Option) -> Result, { Self::mutate_counter( - Map::try_mutate_exists, + ::Map::try_mutate_exists, key, f, ) } /// Take the value under a key. - pub fn take + Clone>(key: KeyArg) -> Map::Query { - if Map::contains_key(key.clone()) { - Counter::mutate(|value| value.saturating_dec()); + pub fn take + Clone>(key: KeyArg) -> QueryKind::Query { + if ::Map::contains_key(key.clone()) { + ::Counter::mutate(|value| value.saturating_dec()); } - Map::take(key) + ::Map::take(key) } /// Append the given items to the value in the storage. @@ -213,10 +208,10 @@ where EncodeLikeItem: EncodeLike, Value: StorageAppend { - if !Map::contains_key(&key) { - Counter::mutate(|value| value.saturating_inc()); + if !::Map::contains_key(key.clone()) { + ::Counter::mutate(|value| value.saturating_inc()); } - Map::append(key, item) + ::Map::append(key, item) } /// Read the length of the storage value without decoding the entire value under the given @@ -234,7 +229,7 @@ where pub fn decode_len>(key: KeyArg) -> Option where Value: StorageDecodeLength, { - Map::decode_len(key) + ::Map::decode_len(key) } /// Migrate an item with the given `key` from a defunct `OldHasher` to the current hasher. @@ -243,20 +238,20 @@ where pub fn migrate_key>( key: KeyArg ) -> Option { - Map::migrate_key::(key) + ::Map::migrate_key::(key) } /// Remove all value of the storage. pub fn remove_all() { - Counter::set(0u32); - Map::remove_all() + ::Counter::set(0u32); + ::Map::remove_all() } /// Iter over all value of the storage. /// /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. pub fn iter_values() -> crate::storage::PrefixIterator { - Map::iter_values() + ::Map::iter_values() } /// Translate the values of all elements by a function `f`, in the map in no particular order. @@ -274,7 +269,7 @@ where /// /// This would typically be called inside the module implementation of on_runtime_upgrade. pub fn translate_values Option>(f: F) { - Map::translate_values(f) + ::Map::translate_values(f) } /// Try and append the given item to the value in the storage. @@ -290,9 +285,16 @@ where EncodeLikeItem: EncodeLike, Value: StorageTryAppend, { - < - Self as crate::storage::TryAppendMap - >::try_append(key, item) + let bound = Value::bound(); + let current = ::Map::decode_len(key.clone()).unwrap_or_default(); + if current < bound { + ::Counter::mutate(|value| value.saturating_inc()); + let key = ::Map::hashed_key_for(key); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } } /// Initialize the counter with the actual number of items in the map. @@ -303,12 +305,63 @@ where /// Returns the number of items in the map which is used to set the counter. pub fn initialize_counter() -> u32 { let count = Self::iter_values().count() as u32; - Counter::set(count); + ::Counter::set(count); count } /// Return the count. pub fn count() -> u32 { - Counter::get() + ::Counter::get() + } +} + +/// Part of storage metadata for a counted storage map. +pub trait CountedStorageMapMetadata { + const MODIFIER: StorageEntryModifier; + const NAME: &'static str; + const DEFAULT: DefaultByteGetter; + const HASHER: frame_metadata::StorageHasher; + const COUNTER_NAME: &'static str; + const COUNTER_MODIFIER: StorageEntryModifier; + const COUNTER_TY: &'static str; + const COUNTER_DEFAULT: DefaultByteGetter; + const COUNTER_DOC: &'static str; +} + +impl CountedStorageMapMetadata + for CountedStorageMap where + Prefix: CountedStorageMapInstance, + Hasher: crate::hash::StorageHasher, + Key: FullCodec, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + const MODIFIER: StorageEntryModifier = ::Map::MODIFIER; + const HASHER: frame_metadata::StorageHasher = ::Map::HASHER; + const NAME: &'static str = ::Map::NAME; + const DEFAULT: DefaultByteGetter = ::Map::DEFAULT; + const COUNTER_NAME: &'static str = ::Counter::NAME; + const COUNTER_MODIFIER: StorageEntryModifier = ::Counter::MODIFIER; + const COUNTER_TY: &'static str = "u32"; + const COUNTER_DEFAULT: DefaultByteGetter = ::Counter::DEFAULT; + const COUNTER_DOC: &'static str = &"Counter for the related counted storage map"; +} + +impl + crate::traits::StorageInfoTrait for + CountedStorageMap +where + Prefix: CountedStorageMapInstance, + Hasher: crate::hash::StorageHasher, + Key: FullCodec + MaxEncodedLen, + Value: FullCodec + MaxEncodedLen, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn storage_info() -> Vec { + [::Map::storage_info(), ::Counter::storage_info()].concat() } } diff --git a/frame/support/src/storage/types/mod.rs b/frame/support/src/storage/types/mod.rs index f61065671315f..b259e50fbec34 100644 --- a/frame/support/src/storage/types/mod.rs +++ b/frame/support/src/storage/types/mod.rs @@ -26,6 +26,7 @@ mod key; mod map; mod nmap; mod value; +mod counted_map; pub use double_map::{StorageDoubleMap, StorageDoubleMapMetadata}; pub use key::{ @@ -35,6 +36,7 @@ pub use key::{ pub use map::{StorageMap, StorageMapMetadata}; pub use nmap::{StorageNMap, StorageNMapMetadata}; pub use value::{StorageValue, StorageValueMetadata}; +pub use counted_map::{CountedStorageMap, CountedStorageMapMetadata, CountedStorageMapInstance}; /// Trait implementing how the storage optional value is converted into the queried type. /// From 43e635f5ab0f6349d305e256edf0d3ff07013dd9 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 15 Jun 2021 14:34:54 +0200 Subject: [PATCH 06/25] test for macro generation --- .../procedural/src/pallet/expand/storage.rs | 2 +- .../support/src/storage/types/counted_map.rs | 2 + frame/support/test/tests/pallet.rs | 47 +++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index f7c31749434c0..b3c89f0808d4f 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -427,7 +427,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { { // NOTE: we can't use any trait here because CountedStorageMap // doesn't implement any. - #full_ident::get(k) + <#full_ident>::get(k) } } ) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 0baf14d2f90ef..06ab430da61a6 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -365,3 +365,5 @@ where [::Map::storage_info(), ::Counter::storage_info()].concat() } } + +// TODO TODO: test that diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 412622b3b194d..5e91dec9f37d5 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -277,6 +277,15 @@ pub mod pallet { u32, >; + #[pallet::storage] + #[pallet::storage_prefix = "RenamedCountedMap"] + #[pallet::getter(fn counted_storage_map)] + pub type SomeCountedStorageMap = CountedStorageMap< + Hasher = Twox64Concat, + Key = u8, + Value = u32, + >; + #[pallet::genesis_config] #[derive(Default)] pub struct GenesisConfig { @@ -631,6 +640,13 @@ fn storage_expand() { pallet::ConditionalDoubleMap::::insert(1, 2, 3); pallet::ConditionalNMap::::insert((1, 2), 3); } + + pallet::SomeCountedStorageMap::::insert(1, 2); + let mut k = [twox_128(b"Example"), twox_128(b"RenamedCountedMap")].concat(); + k.extend(1u8.using_encoded(twox_64_concat)); + assert_eq!(unhashed::get::(&k), Some(2u32)); + let k = [twox_128(b"Example"), twox_128(b"CounterForRenamedCountedMap")].concat(); + assert_eq!(unhashed::get::(&k), Some(1u32)); }) } @@ -838,6 +854,27 @@ fn metadata() { default: DecodeDifferent::Decoded(vec![0]), documentation: DecodeDifferent::Decoded(vec![]), }, + StorageEntryMetadata { + name: DecodeDifferent::Decoded("RenamedCountedMap".to_string()), + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + key: DecodeDifferent::Decoded("u8".to_string()), + value: DecodeDifferent::Decoded("u32".to_string()), + hasher: StorageHasher::Twox64Concat, + unused: false, + }, + default: DecodeDifferent::Decoded(vec![0]), + documentation: DecodeDifferent::Decoded(vec![]), + }, + StorageEntryMetadata { + name: DecodeDifferent::Decoded("CounterForRenamedCountedMap".to_string()), + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Plain(DecodeDifferent::Decoded("u32".to_string())), + default: DecodeDifferent::Decoded(vec![0, 0, 0, 0]), + documentation: DecodeDifferent::Decoded(vec![ + "Counter for the related counted storage map".to_string(), + ]), + }, ]), })), calls: Some(DecodeDifferent::Decoded(vec![ @@ -1075,6 +1112,16 @@ fn test_storage_info() { max_size: Some(7 + 16 + 8), } }, + StorageInfo { + prefix: prefix(b"Example", b"RenamedCountedMap"), + max_values: None, + max_size: Some(1 + 4 + 8), + }, + StorageInfo { + prefix: prefix(b"Example", b"CounterForRenamedCountedMap"), + max_values: Some(1), + max_size: Some(4), + }, ], ); } From 98858912f7536dccd67ed4bc4de20229077801ad Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 15 Jun 2021 18:16:42 +0200 Subject: [PATCH 07/25] add iterable functions, some test and fixes --- frame/example/src/tests.rs | 35 - .../src/storage/generator/double_map.rs | 3 + frame/support/src/storage/generator/map.rs | 1 + frame/support/src/storage/generator/nmap.rs | 3 + frame/support/src/storage/migration.rs | 7 +- frame/support/src/storage/mod.rs | 25 +- .../support/src/storage/types/counted_map.rs | 744 ++++++++++++++++-- 7 files changed, 704 insertions(+), 114 deletions(-) diff --git a/frame/example/src/tests.rs b/frame/example/src/tests.rs index 79a3321d35ea8..5d39c53d7b149 100644 --- a/frame/example/src/tests.rs +++ b/frame/example/src/tests.rs @@ -181,41 +181,6 @@ fn counted_map_works() { assert_eq!(CountedMap::::count(), 0); CountedMap::::insert(3, 3); assert_eq!(CountedMap::::count(), 1); - CountedMap::::insert(3, 5); - assert_eq!(CountedMap::::count(), 1); - CountedMap::::insert(5, 5); - assert_eq!(CountedMap::::count(), 2); - CountedMap::::remove(5); - assert_eq!(CountedMap::::count(), 1); - // Does nothing - CountedMap::::take(4); - assert_eq!(CountedMap::::count(), 1); - CountedMap::::take(3); - assert_eq!(CountedMap::::count(), 0); - - CountedMap::::mutate(3, |old| { - if old.is_none() { - *old = Some(69) - } else { - *old = Some(420) - } - }); - assert_eq!(CountedMap::::get(3), Some(69)); - assert_eq!(CountedMap::::count(), 1); - - CountedMap::::mutate(3, |old| { - if old.is_none() { - *old = Some(69) - } else { - *old = Some(420) - } - }); - assert_eq!(CountedMap::::get(3), Some(420)); - assert_eq!(CountedMap::::count(), 1); - - CountedMap::::mutate(3, |old| *old = None); - assert_eq!(CountedMap::::get(3), None); - assert_eq!(CountedMap::::count(), 0); }) } diff --git a/frame/support/src/storage/generator/double_map.rs b/frame/support/src/storage/generator/double_map.rs index c02ebe48290eb..ca679a6523bba 100644 --- a/frame/support/src/storage/generator/double_map.rs +++ b/frame/support/src/storage/generator/double_map.rs @@ -225,6 +225,7 @@ impl storage::StorageDoubleMap for G where previous_key: prefix, drain: false, closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), + phantom: Default::default(), } } @@ -352,6 +353,7 @@ impl< let mut key_material = G::Hasher2::reverse(raw_key_without_prefix); Ok((K2::decode(&mut key_material)?, V::decode(&mut raw_value)?)) }, + phantom: Default::default(), } } @@ -374,6 +376,7 @@ impl< let k2 = K2::decode(&mut k2_material)?; Ok((k1, k2, V::decode(&mut raw_value)?)) }, + phantom: Default::default(), } } diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index 9abc7883937dd..3cfc634c40bb2 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -152,6 +152,7 @@ impl< let mut key_material = G::Hasher::reverse(raw_key_without_prefix); Ok((K::decode(&mut key_material)?, V::decode(&mut raw_value)?)) }, + phantom: Default::default(), } } diff --git a/frame/support/src/storage/generator/nmap.rs b/frame/support/src/storage/generator/nmap.rs index d1f00adda5e55..90dbcc9b1f2b7 100755 --- a/frame/support/src/storage/generator/nmap.rs +++ b/frame/support/src/storage/generator/nmap.rs @@ -213,6 +213,7 @@ where previous_key: prefix, drain: false, closure: |_raw_key, mut raw_value| V::decode(&mut raw_value), + phantom: Default::default(), } } @@ -325,6 +326,7 @@ impl> let partial_key = K::decode_partial_key(raw_key_without_prefix)?; Ok((partial_key, V::decode(&mut raw_value)?)) }, + phantom: Default::default(), } } @@ -347,6 +349,7 @@ impl> let (final_key, _) = K::decode_final_key(raw_key_without_prefix)?; Ok((final_key, V::decode(&mut raw_value)?)) }, + phantom: Default::default(), } } diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs index b4a1a9225dd1f..00b9e53347bee 100644 --- a/frame/support/src/storage/migration.rs +++ b/frame/support/src/storage/migration.rs @@ -175,7 +175,7 @@ pub fn storage_iter_with_suffix( Ok((raw_key_without_prefix.to_vec(), value)) }; - PrefixIterator { prefix, previous_key, drain: false, closure } + PrefixIterator { prefix, previous_key, drain: false, closure, phantom: Default::default() } } /// Construct iterator to iterate over map items in `module` for the map called `item`. @@ -203,7 +203,7 @@ pub fn storage_key_iter_with_suffix { prefix: from_prefix.to_vec(), previous_key: from_prefix.to_vec(), drain: true, closure: |key, value| Ok((key.to_vec(), value.to_vec())), + phantom: Default::default(), }; for (key, value) in iter { diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 34d217f5c31b6..4a553ea09f507 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -666,10 +666,12 @@ pub trait StorageNMap { KArg: EncodeLikeTuple + TupleToEncodedIter; } -/// Iterate over a prefix and decode raw_key and raw_value into `T`. +/// Iterate or drain over a prefix and decode raw_key and raw_value into `T`. /// /// If any decoding fails it skips it and continues to the next key. -pub struct PrefixIterator { +/// +/// If draining, then the hook `OnRemoval::on_removal` is called after each removal. +pub struct PrefixIterator { prefix: Vec, previous_key: Vec, /// If true then value are removed while iterating @@ -677,9 +679,20 @@ pub struct PrefixIterator { /// 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, + phantom: core::marker::PhantomData, +} + +/// Trait for specialising on removal logic of [`PrefixIterator`]. +pub trait PrefixIteratorOnRemoval { + fn on_removal(); +} + +/// No-op implement. +impl PrefixIteratorOnRemoval for () { + fn on_removal() {} } -impl PrefixIterator { +impl PrefixIterator { /// Mutate this iterator into a draining iterator; items iterated are removed from storage. pub fn drain(mut self) -> Self { self.drain = true; @@ -687,7 +700,7 @@ impl PrefixIterator { } } -impl Iterator for PrefixIterator { +impl Iterator for PrefixIterator { type Item = T; fn next(&mut self) -> Option { @@ -708,7 +721,8 @@ impl Iterator for PrefixIterator { } }; if self.drain { - unhashed::kill(&self.previous_key) + unhashed::kill(&self.previous_key); + OnRemoval::on_removal(); } let raw_key_without_prefix = &self.previous_key[self.prefix.len()..]; let item = match (self.closure)(raw_key_without_prefix, &raw_value[..]) { @@ -894,6 +908,7 @@ pub trait StoragePrefixedMap { previous_key: prefix.to_vec(), drain: false, closure: |_raw_key, mut raw_value| Value::decode(&mut raw_value), + phantom: Default::default(), } } diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 06ab430da61a6..058f65000c509 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -1,8 +1,9 @@ -use codec::{FullCodec, Decode, EncodeLike, Encode}; +use codec::{FullCodec, Decode, EncodeLike, Encode, Ref}; use crate::{ + Never, traits::{StorageInstance, GetDefault, Get, StorageInfo}, storage::{ - StorageAppend, StorageTryAppend, StorageDecodeLength, + StorageAppend, StorageTryAppend, StorageDecodeLength, generator::StorageMap as _, types::{ OptionQuery, ValueQuery, StorageMap, StorageValue, QueryKindTrait, StorageMapMetadata, StorageValueMetadata @@ -45,7 +46,6 @@ pub trait CountedStorageMapInstance: StorageInstance { // Internal helper trait to access map from counted storage map. trait Helper { type Map; - type Counter; } impl @@ -53,10 +53,22 @@ impl for CountedStorageMap { type Map = StorageMap; - type Counter = StorageValue; } -// Do not use storage value, simply implement some get/set/inc/dec +type CounterFor

= StorageValue<

::CounterPrefix, u32, ValueQuery>; + +/// On removal logic for updating counter while draining upon some prefix with +/// [`crate::storage::PrefixIterator`]. +pub struct OnRemovalCounterUpdate(core::marker::PhantomData); + +impl + crate::storage::PrefixIteratorOnRemoval + for OnRemovalCounterUpdate +{ + fn on_removal() { + CounterFor::::mutate(|value| value.saturating_dec()); + } +} impl CountedStorageMap @@ -69,29 +81,6 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - // Internal helper function to track the counter as a value is mutated. - fn mutate_counter< - KeyArg: EncodeLike + Clone, - M: FnOnce(KeyArg, F) -> R, - F: FnOnce(I) -> R, - I, - R, - >(m: M, key: KeyArg, f: F) -> R { - let val_existed = ::Map::contains_key(key.clone()); - let res = m(key.clone(), f); - let val_exists = ::Map::contains_key(key); - - if val_existed && !val_exists { - // Value was deleted - ::Counter::mutate(|value| value.saturating_dec()); - } else if !val_existed && val_exists { - // Value was added - ::Counter::mutate(|value| value.saturating_inc()); - } - - res - } - /// Get the storage key used to fetch a value corresponding to a specific key. pub fn hashed_key_for>(key: KeyArg) -> Vec { ::Map::hashed_key_for(key) @@ -121,16 +110,16 @@ where /// Store a value to be associated with the given key from the map. pub fn insert + Clone, ValArg: EncodeLike>(key: KeyArg, val: ValArg) { - if !::Map::contains_key(key.clone()) { - ::Counter::mutate(|value| value.saturating_inc()); + if !::Map::contains_key(Ref::from(&key)) { + CounterFor::::mutate(|value| value.saturating_inc()); } ::Map::insert(key, val) } /// Remove the value under a key. pub fn remove + Clone>(key: KeyArg) { - if ::Map::contains_key(key.clone()) { - ::Counter::mutate(|value| value.saturating_dec()); + if ::Map::contains_key(Ref::from(&key)) { + CounterFor::::mutate(|value| value.saturating_dec()); } ::Map::remove(key) } @@ -140,11 +129,8 @@ where key: KeyArg, f: F ) -> R { - Self::mutate_counter( - ::Map::mutate, - key, - f, - ) + Self::try_mutate(key, |v| Ok::(f(v))) + .expect("`Never` can not be constructed; qed") } /// Mutate the item, only if an `Ok` value is returned. @@ -153,11 +139,14 @@ where KeyArg: EncodeLike + Clone, F: FnOnce(&mut QueryKind::Query) -> Result, { - Self::mutate_counter( - ::Map::try_mutate, - key, - f, - ) + Self::try_mutate_exists(key, |option_value_ref| { + let option_value = core::mem::replace(option_value_ref, None); + let mut query = ::Map::from_optional_value_to_query(option_value); + let res = f(&mut query); + let option_value = ::Map::from_query_to_optional_value(query); + let _ = core::mem::replace(option_value_ref, option_value); + res + }) } /// Mutate the value under a key. Deletes the item if mutated to a `None`. @@ -165,11 +154,8 @@ where key: KeyArg, f: F ) -> R { - Self::mutate_counter( - ::Map::mutate_exists, - key, - f, - ) + Self::try_mutate_exists(key, |v| Ok::(f(v))) + .expect("`Never` can not be constructed; qed") } /// Mutate the item, only if an `Ok` value is returned. Deletes the item if mutated to a `None`. @@ -178,19 +164,34 @@ where KeyArg: EncodeLike + Clone, F: FnOnce(&mut Option) -> Result, { - Self::mutate_counter( - ::Map::try_mutate_exists, - key, - f, - ) + ::Map::try_mutate_exists(key, |option_value| { + let existed = option_value.is_some(); + let res = f(option_value); + let exist = option_value.is_some(); + + if res.is_ok() { + if existed && !exist { + // Value was deleted + CounterFor::::mutate(|value| value.saturating_dec()); + } else if !existed && exist { + // Value was added + CounterFor::::mutate(|value| value.saturating_inc()); + } + } + res + }) } /// Take the value under a key. pub fn take + Clone>(key: KeyArg) -> QueryKind::Query { - if ::Map::contains_key(key.clone()) { - ::Counter::mutate(|value| value.saturating_dec()); + let removed_value = ::Map::mutate_exists( + key, + |value| core::mem::replace(value, None) + ); + if removed_value.is_some() { + CounterFor::::mutate(|value| value.saturating_dec()); } - ::Map::take(key) + ::Map::from_optional_value_to_query(removed_value) } /// Append the given items to the value in the storage. @@ -208,8 +209,8 @@ where EncodeLikeItem: EncodeLike, Value: StorageAppend { - if !::Map::contains_key(key.clone()) { - ::Counter::mutate(|value| value.saturating_inc()); + if !::Map::contains_key(Ref::from(&key)) { + CounterFor::::mutate(|value| value.saturating_inc()); } ::Map::append(key, item) } @@ -243,15 +244,22 @@ where /// Remove all value of the storage. pub fn remove_all() { - ::Counter::set(0u32); + CounterFor::::set(0u32); ::Map::remove_all() } /// Iter over all value of the storage. /// /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. - pub fn iter_values() -> crate::storage::PrefixIterator { - ::Map::iter_values() + pub fn iter_values() -> crate::storage::PrefixIterator> { + let map_iterator = ::Map::iter_values(); + crate::storage::PrefixIterator { + prefix: map_iterator.prefix, + previous_key: map_iterator.previous_key, + drain: map_iterator.drain, + closure: map_iterator.closure, + phantom: Default::default(), + } } /// Translate the values of all elements by a function `f`, in the map in no particular order. @@ -268,8 +276,14 @@ where /// # Usage /// /// This would typically be called inside the module implementation of on_runtime_upgrade. - pub fn translate_values Option>(f: F) { - ::Map::translate_values(f) + pub fn translate_values Option>(mut f: F) { + ::Map::translate_values(|old_value| { + let res = f(old_value); + if res.is_none() { + CounterFor::::mutate(|value| value.saturating_dec()); + } + res + }) } /// Try and append the given item to the value in the storage. @@ -286,9 +300,9 @@ where Value: StorageTryAppend, { let bound = Value::bound(); - let current = ::Map::decode_len(key.clone()).unwrap_or_default(); + let current = ::Map::decode_len(Ref::from(&key)).unwrap_or_default(); if current < bound { - ::Counter::mutate(|value| value.saturating_inc()); + CounterFor::::mutate(|value| value.saturating_inc()); let key = ::Map::hashed_key_for(key); sp_io::storage::append(&key, item.encode()); Ok(()) @@ -305,13 +319,68 @@ where /// Returns the number of items in the map which is used to set the counter. pub fn initialize_counter() -> u32 { let count = Self::iter_values().count() as u32; - ::Counter::set(count); + CounterFor::::set(count); count } /// Return the count. pub fn count() -> u32 { - ::Counter::get() + CounterFor::::get() + } +} + +impl + CountedStorageMap +where + Prefix: CountedStorageMapInstance, + Hasher: crate::hash::StorageHasher + crate::ReversibleStorageHasher, + Key: FullCodec, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + /// Enumerate all elements in the map in no particular order. + /// + /// If you alter the map while doing this, you'll get undefined results. + pub fn iter() -> crate::storage::PrefixIterator<(Key, Value), OnRemovalCounterUpdate> { + let map_iterator = ::Map::iter(); + crate::storage::PrefixIterator { + prefix: map_iterator.prefix, + previous_key: map_iterator.previous_key, + drain: map_iterator.drain, + closure: map_iterator.closure, + phantom: Default::default(), + } + } + + /// 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. + pub fn drain() -> crate::storage::PrefixIterator<(Key, Value), OnRemovalCounterUpdate> { + let map_iterator = ::Map::drain(); + crate::storage::PrefixIterator { + prefix: map_iterator.prefix, + previous_key: map_iterator.previous_key, + drain: map_iterator.drain, + closure: map_iterator.closure, + phantom: Default::default(), + } + } + + /// 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. + /// + /// NOTE: If a value fail to decode because storage is corrupted then it is skipped. + pub fn translate Option>(mut f: F) { + ::Map::translate(|key, old_value| { + let res = f(key, old_value); + if res.is_none() { + CounterFor::::mutate(|value| value.saturating_dec()); + } + res + }) } } @@ -342,10 +411,10 @@ impl CountedStorageMa const HASHER: frame_metadata::StorageHasher = ::Map::HASHER; const NAME: &'static str = ::Map::NAME; const DEFAULT: DefaultByteGetter = ::Map::DEFAULT; - const COUNTER_NAME: &'static str = ::Counter::NAME; - const COUNTER_MODIFIER: StorageEntryModifier = ::Counter::MODIFIER; + const COUNTER_NAME: &'static str = CounterFor::::NAME; + const COUNTER_MODIFIER: StorageEntryModifier = CounterFor::::MODIFIER; const COUNTER_TY: &'static str = "u32"; - const COUNTER_DEFAULT: DefaultByteGetter = ::Counter::DEFAULT; + const COUNTER_DEFAULT: DefaultByteGetter = CounterFor::::DEFAULT; const COUNTER_DOC: &'static str = &"Counter for the related counted storage map"; } @@ -362,8 +431,541 @@ where MaxValues: Get>, { fn storage_info() -> Vec { - [::Map::storage_info(), ::Counter::storage_info()].concat() + [::Map::storage_info(), CounterFor::::storage_info()].concat() } } -// TODO TODO: test that +#[cfg(test)] +mod test { + use super::*; + use sp_io::{TestExternalities, hashing::twox_128}; + use crate::hash::*; + use crate::storage::types::ValueQuery; + use crate::storage::bounded_vec::BoundedVec; + use crate::traits::ConstU32; + + struct Prefix; + impl StorageInstance for Prefix { + fn pallet_prefix() -> &'static str { "test" } + const STORAGE_PREFIX: &'static str = "foo"; + } + + struct CounterPrefix; + impl StorageInstance for CounterPrefix { + fn pallet_prefix() -> &'static str { "test" } + const STORAGE_PREFIX: &'static str = "counter_for_foo"; + } + impl CountedStorageMapInstance for Prefix { + type CounterPrefix = CounterPrefix; + } + + struct ADefault; + impl crate::traits::Get for ADefault { + fn get() -> u32 { + 97 + } + } + + #[test] + fn test_value_query() { + type A = CountedStorageMap; + + TestExternalities::default().execute_with(|| { + let mut k: Vec = vec![]; + k.extend(&twox_128(b"test")); + k.extend(&twox_128(b"foo")); + k.extend(&3u16.twox_64_concat()); + assert_eq!(A::hashed_key_for(3).to_vec(), k); + + assert_eq!(A::contains_key(3), false); + assert_eq!(A::get(3), ADefault::get()); + assert_eq!(A::try_get(3), Err(())); + assert_eq!(A::count(), 0); + + // Insert non-existing. + A::insert(3, 10); + + assert_eq!(A::contains_key(3), true); + assert_eq!(A::get(3), 10); + assert_eq!(A::try_get(3), Ok(10)); + assert_eq!(A::count(), 1); + + // Swap non-existing with existing. + A::swap(4, 3); + + assert_eq!(A::contains_key(3), false); + assert_eq!(A::get(3), ADefault::get()); + assert_eq!(A::try_get(3), Err(())); + assert_eq!(A::contains_key(4), true); + assert_eq!(A::get(4), 10); + assert_eq!(A::try_get(4), Ok(10)); + assert_eq!(A::count(), 1); + + // Swap existing with non-existing. + A::swap(4, 3); + + assert_eq!(A::try_get(3), Ok(10)); + assert_eq!(A::contains_key(4), false); + assert_eq!(A::get(4), ADefault::get()); + assert_eq!(A::try_get(4), Err(())); + assert_eq!(A::count(), 1); + + A::insert(4, 11); + + assert_eq!(A::try_get(3), Ok(10)); + assert_eq!(A::try_get(4), Ok(11)); + assert_eq!(A::count(), 2); + + // Swap 2 existing. + A::swap(3, 4); + + assert_eq!(A::try_get(3), Ok(11)); + assert_eq!(A::try_get(4), Ok(10)); + assert_eq!(A::count(), 2); + + // Insert an existing key, shouldn't increment counted values. + A::insert(3, 11); + + assert_eq!(A::count(), 2); + + // Remove non-existing. + A::remove(2); + + assert_eq!(A::contains_key(2), false); + assert_eq!(A::count(), 2); + + // Remove existing. + A::remove(3); + + assert_eq!(A::try_get(3), Err(())); + assert_eq!(A::count(), 1); + + // Mutate non-existing to existing. + A::mutate(3, |query| { + assert_eq!(*query, ADefault::get()); + *query = 40; + }); + + assert_eq!(A::try_get(3), Ok(40)); + assert_eq!(A::count(), 2); + + // Mutate existing to existing. + A::mutate(3, |query| { + assert_eq!(*query, 40); + *query = 40; + }); + + assert_eq!(A::try_get(3), Ok(40)); + assert_eq!(A::count(), 2); + + // Try fail mutate non-existing to existing. + A::try_mutate(2, |query| { + assert_eq!(*query, ADefault::get()); + *query = 4; + Result::<(), ()>::Err(()) + }).err().unwrap(); + + assert_eq!(A::try_get(2), Err(())); + assert_eq!(A::count(), 2); + + // Try succeed mutate non-existing to existing. + A::try_mutate(2, |query| { + assert_eq!(*query, ADefault::get()); + *query = 41; + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(A::try_get(2), Ok(41)); + assert_eq!(A::count(), 3); + + // Try succeed mutate existing to existing. + A::try_mutate(2, |query| { + assert_eq!(*query, 41); + *query = 41; + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(A::try_get(2), Ok(41)); + assert_eq!(A::count(), 3); + + // Try fail mutate non-existing to existing. + A::try_mutate_exists(1, |query| { + assert_eq!(*query, None); + *query = Some(4); + Result::<(), ()>::Err(()) + }).err().unwrap(); + + assert_eq!(A::try_get(1), Err(())); + assert_eq!(A::count(), 3); + + // Try succeed mutate non-existing to existing. + A::try_mutate_exists(1, |query| { + assert_eq!(*query, None); + *query = Some(43); + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(A::try_get(1), Ok(43)); + assert_eq!(A::count(), 4); + + // Try succeed mutate existing to existing. + A::try_mutate_exists(1, |query| { + assert_eq!(*query, Some(43)); + *query = Some(43); + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(A::try_get(1), Ok(43)); + assert_eq!(A::count(), 4); + + // Try succeed mutate existing to non-existing. + A::try_mutate_exists(1, |query| { + assert_eq!(*query, Some(43)); + *query = None; Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(A::try_get(1), Err(())); + assert_eq!(A::count(), 3); + + // Take exsisting. + assert_eq!(A::take(4), 10); + + assert_eq!(A::try_get(4), Err(())); + assert_eq!(A::count(), 2); + + // Take non-exsisting. + assert_eq!(A::take(4), ADefault::get()); + + assert_eq!(A::try_get(4), Err(())); + assert_eq!(A::count(), 2); + + // Remove all. + A::remove_all(); + + assert_eq!(A::count(), 0); + assert_eq!(A::initialize_counter(), 0); + + A::insert(1, 1); + A::insert(2, 2); + + // Iter values. + assert_eq!(A::iter_values().collect::>(), vec![2, 1]); + + // Iter drain values. + assert_eq!(A::iter_values().drain().collect::>(), vec![2, 1]); + assert_eq!(A::count(), 0); + + A::insert(1, 1); + A::insert(2, 2); + + // Test initialize_counter. + assert_eq!(A::initialize_counter(), 2); + }) + } + + #[test] + fn test_option_query() { + type B = CountedStorageMap; + + TestExternalities::default().execute_with(|| { + let mut k: Vec = vec![]; + k.extend(&twox_128(b"test")); + k.extend(&twox_128(b"foo")); + k.extend(&3u16.twox_64_concat()); + assert_eq!(B::hashed_key_for(3).to_vec(), k); + + assert_eq!(B::contains_key(3), false); + assert_eq!(B::get(3), None); + assert_eq!(B::try_get(3), Err(())); + assert_eq!(B::count(), 0); + + // Insert non-existing. + B::insert(3, 10); + + assert_eq!(B::contains_key(3), true); + assert_eq!(B::get(3), Some(10)); + assert_eq!(B::try_get(3), Ok(10)); + assert_eq!(B::count(), 1); + + // Swap non-existing with existing. + B::swap(4, 3); + + assert_eq!(B::contains_key(3), false); + assert_eq!(B::get(3), None); + assert_eq!(B::try_get(3), Err(())); + assert_eq!(B::contains_key(4), true); + assert_eq!(B::get(4), Some(10)); + assert_eq!(B::try_get(4), Ok(10)); + assert_eq!(B::count(), 1); + + // Swap existing with non-existing. + B::swap(4, 3); + + assert_eq!(B::try_get(3), Ok(10)); + assert_eq!(B::contains_key(4), false); + assert_eq!(B::get(4), None); + assert_eq!(B::try_get(4), Err(())); + assert_eq!(B::count(), 1); + + B::insert(4, 11); + + assert_eq!(B::try_get(3), Ok(10)); + assert_eq!(B::try_get(4), Ok(11)); + assert_eq!(B::count(), 2); + + // Swap 2 existing. + B::swap(3, 4); + + assert_eq!(B::try_get(3), Ok(11)); + assert_eq!(B::try_get(4), Ok(10)); + assert_eq!(B::count(), 2); + + // Insert an existing key, shouldn't increment counted values. + B::insert(3, 11); + + assert_eq!(B::count(), 2); + + // Remove non-existing. + B::remove(2); + + assert_eq!(B::contains_key(2), false); + assert_eq!(B::count(), 2); + + // Remove existing. + B::remove(3); + + assert_eq!(B::try_get(3), Err(())); + assert_eq!(B::count(), 1); + + // Mutate non-existing to existing. + B::mutate(3, |query| { + assert_eq!(*query, None); + *query = Some(40) + }); + + assert_eq!(B::try_get(3), Ok(40)); + assert_eq!(B::count(), 2); + + // Mutate existing to existing. + B::mutate(3, |query| { + assert_eq!(*query, Some(40)); + *query = Some(40) + }); + + assert_eq!(B::try_get(3), Ok(40)); + assert_eq!(B::count(), 2); + + // Mutate existing to non-existing. + B::mutate(3, |query| { + assert_eq!(*query, Some(40)); + *query = None + }); + + assert_eq!(B::try_get(3), Err(())); + assert_eq!(B::count(), 1); + + B::insert(3, 40); + + // Try fail mutate non-existing to existing. + B::try_mutate(2, |query| { + assert_eq!(*query, None); + *query = Some(4); + Result::<(), ()>::Err(()) + }).err().unwrap(); + + assert_eq!(B::try_get(2), Err(())); + assert_eq!(B::count(), 2); + + // Try succeed mutate non-existing to existing. + B::try_mutate(2, |query| { + assert_eq!(*query, None); + *query = Some(41); + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(B::try_get(2), Ok(41)); + assert_eq!(B::count(), 3); + + // Try succeed mutate existing to existing. + B::try_mutate(2, |query| { + assert_eq!(*query, Some(41)); + *query = Some(41); + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(B::try_get(2), Ok(41)); + assert_eq!(B::count(), 3); + + // Try succeed mutate existing to non-existing. + B::try_mutate(2, |query| { + assert_eq!(*query, Some(41)); + *query = None; + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(B::try_get(2), Err(())); + assert_eq!(B::count(), 2); + + B::insert(2, 41); + + // Try fail mutate non-existing to existing. + B::try_mutate_exists(1, |query| { + assert_eq!(*query, None); + *query = Some(4); + Result::<(), ()>::Err(()) + }).err().unwrap(); + + assert_eq!(B::try_get(1), Err(())); + assert_eq!(B::count(), 3); + + // Try succeed mutate non-existing to existing. + B::try_mutate_exists(1, |query| { + assert_eq!(*query, None); + *query = Some(43); + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(B::try_get(1), Ok(43)); + assert_eq!(B::count(), 4); + + // Try succeed mutate existing to existing. + B::try_mutate_exists(1, |query| { + assert_eq!(*query, Some(43)); + *query = Some(43); + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(B::try_get(1), Ok(43)); + assert_eq!(B::count(), 4); + + // Try succeed mutate existing to non-existing. + B::try_mutate_exists(1, |query| { + assert_eq!(*query, Some(43)); + *query = None; + Result::<(), ()>::Ok(()) + }).unwrap(); + + assert_eq!(B::try_get(1), Err(())); + assert_eq!(B::count(), 3); + + // Take exsisting. + assert_eq!(B::take(4), Some(10)); + + assert_eq!(B::try_get(4), Err(())); + assert_eq!(B::count(), 2); + + // Take non-exsisting. + assert_eq!(B::take(4), None); + + assert_eq!(B::try_get(4), Err(())); + assert_eq!(B::count(), 2); + + // Remove all. + B::remove_all(); + + assert_eq!(B::count(), 0); + assert_eq!(B::initialize_counter(), 0); + + B::insert(1, 1); + B::insert(2, 2); + + // Iter values. + assert_eq!(B::iter_values().collect::>(), vec![2, 1]); + + // Iter drain values. + assert_eq!(B::iter_values().drain().collect::>(), vec![2, 1]); + assert_eq!(B::count(), 0); + + B::insert(1, 1); + B::insert(2, 2); + + // Test initialize_counter. + assert_eq!(B::initialize_counter(), 2); + }) + } + + #[test] + fn append_decode_len_works() { + type B = CountedStorageMap>; + + TestExternalities::default().execute_with(|| { + assert_eq!(B::decode_len(0), None); + B::append(0, 3); + assert_eq!(B::decode_len(0), Some(1)); + B::append(0, 3); + assert_eq!(B::decode_len(0), Some(2)); + B::append(0, 3); + assert_eq!(B::decode_len(0), Some(3)); + }) + } + + #[test] + fn try_append_decode_len_works() { + type B = CountedStorageMap>>; + + TestExternalities::default().execute_with(|| { + assert_eq!(B::decode_len(0), None); + B::try_append(0, 3).unwrap(); + assert_eq!(B::decode_len(0), Some(1)); + B::try_append(0, 3).unwrap(); + assert_eq!(B::decode_len(0), Some(2)); + B::try_append(0, 3).unwrap(); + assert_eq!(B::decode_len(0), Some(3)); + B::try_append(0, 3).err().unwrap(); + assert_eq!(B::decode_len(0), Some(3)); + }) + } + + #[test] + fn migrate_keys_works() { + type A = CountedStorageMap; + type B = CountedStorageMap; + TestExternalities::default().execute_with(|| { + A::insert(1, 1); + assert_eq!(B::migrate_key::(1), Some(1)); + assert_eq!(B::get(1), Some(1)); + }) + } + + #[test] + fn translate_values() { + type A = CountedStorageMap; + TestExternalities::default().execute_with(|| { + A::insert(1, 1); + A::insert(2, 2); + A::translate_values::(|old_value| if old_value == 1 { + None + } else { + Some(1) + }); + assert_eq!(A::count(), 1); + assert_eq!(A::get(2), Some(1)); + }) + } + + #[test] + fn test_iter_drain_translate() { + type A = CountedStorageMap; + TestExternalities::default().execute_with(|| { + A::insert(1, 1); + A::insert(2, 2); + + assert_eq!(A::iter().collect::>(), vec![(2, 2), (1, 1)]); + + assert_eq!(A::count(), 2); + + A::translate::(|key, value| if key == 1 { + None + } else { + Some(key as u32 * value) + }); + + assert_eq!(A::count(), 1); + + assert_eq!(A::drain().collect::>(), vec![(2, 4)]); + + assert_eq!(A::count(), 0); + }) + } + +} From 4f31878fe495c90a7e2ba2c0e17946722059044d Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Jun 2021 11:44:44 +0200 Subject: [PATCH 08/25] fix merge --- frame/support/src/storage/types/counted_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 058f65000c509..56acc1ffcf679 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -245,7 +245,7 @@ where /// Remove all value of the storage. pub fn remove_all() { CounterFor::::set(0u32); - ::Map::remove_all() + ::Map::remove_all(None); } /// Iter over all value of the storage. From c588233c6b2dc6033dc9caca043dc7fd57726b27 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Jun 2021 12:06:18 +0200 Subject: [PATCH 09/25] doc --- frame/support/src/lib.rs | 23 ++++++++++++++++--- .../support/src/storage/types/counted_map.rs | 6 ++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 81a13222bd678..01f4f1a8bc741 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1626,23 +1626,37 @@ pub mod pallet_prelude { /// * [`pallet_prelude::StorageValue`] expect `Value` and optionally `QueryKind` and `OnEmpty`, /// * [`pallet_prelude::StorageMap`] expect `Hasher`, `Key`, `Value` and optionally `QueryKind` and /// `OnEmpty`, +/// * [`pallet_prelude::CountedStorageMap`] expect `Hasher`, `Key`, `Value` and optionally +/// `QueryKind` and `OnEmpty`, /// * [`pallet_prelude::StorageDoubleMap`] expect `Hasher1`, `Key1`, `Hasher2`, `Key2`, `Value` and /// optionally `QueryKind` and `OnEmpty`. /// /// For unnamed generic argument: Their first generic must be `_` as it is replaced by the macro /// and other generic must declared as a normal declaration of type generic in rust. /// -/// The Prefix generic written by the macro is generated using `PalletInfo::name::>()` -/// and the name of the storage type. +/// The `Prefix` generic written by the macro. It implements `StorageInstance` using +/// `PalletInfo::name::>()` for the pallet prefix and the name of the storage type or +/// the rename as in `#[pallet::storage_prefix = "Bar"]` for storage prefix. /// E.g. if runtime names the pallet "MyExample" then the storage `type Foo = ...` use the -/// prefix: `Twox128(b"MyExample") ++ Twox128(b"Foo")`. +/// pallet prefix: "MyExample" and the storage prefix "Foo". +/// +/// NOTE: Storages use those prefixes so that values are stored after the key +/// `Twox128(b"MyExample") ++ Twox128(b"Foo")`. +/// +/// For the `CountedStorageMap` variant, the Prefix also implements `CountedStorageMapInstance`. +/// It associate a `CounterPrefix`, which is implemented same as above, but the storage prefix is +/// prepend with `"CounterFor"`. /// /// The optional attribute `#[pallet::getter(fn $my_getter_fn_name)]` allow to define a /// getter function on `Pallet`. /// +/// The optional attribute `#[pallet::storage_prefix = "SomeName"]` allow to define the storage +/// prefix to use, see how `Prefix` generic is implemented above. +/// /// E.g: /// ```ignore /// #[pallet::storage] +/// #[pallet::storage_prefix = "foo"] /// #[pallet::getter(fn my_storage)] /// pub(super) type MyStorage = StorageMap; /// ``` @@ -1679,6 +1693,8 @@ pub mod pallet_prelude { /// `_GeneratedPrefixForStorage$NameOfStorage`, and implements /// [`StorageInstance`](traits::StorageInstance) on it using the pallet and storage name. It then /// uses it as the first generic of the aliased type. +/// For `CountedStorageMap`, `CountedStorageMapInstance` is implemented, and another similar struct +/// is generated. /// /// For named generic, the macro will reorder the generics, and remove the names. /// @@ -1966,6 +1982,7 @@ pub mod pallet_prelude { /// /// // Another storage declaration /// #[pallet::storage] +/// #[pallet::storage_prefix = "SomeName"] /// #[pallet::getter(fn my_storage)] /// pub(super) type MyStorage = /// StorageMap; diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 56acc1ffcf679..217b26ee9df06 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -17,8 +17,8 @@ use frame_metadata::{ }; use max_encoded_len::MaxEncodedLen; -/// A wrapper around a `StorageMap` and a `StorageValue` to keep track of how many items are in -/// a map, without needing to iterate all the values. +/// A wrapper around a `StorageMap` and a `StorageValue` to keep track of how many items +/// are in a map, without needing to iterate all the values. /// /// This storage item has additional storage read and write overhead when manipulating values /// compared to a regular storage map. @@ -29,8 +29,6 @@ use max_encoded_len::MaxEncodedLen; /// /// Whenever the counter needs to be updated, an additional read and write occurs to update that /// counter. -/// -/// The storage prefix for the storage value is the given prefix concatenated with `"Counter"`. pub struct CountedStorageMap< Prefix, Hasher, Key, Value, QueryKind=OptionQuery, OnEmpty=GetDefault, MaxValues=GetDefault, >( From 9c362a6bda2bea3201bb109fc182700f6e3722ff Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Mon, 5 Jul 2021 16:02:47 +0200 Subject: [PATCH 10/25] Update frame/support/src/storage/types/counted_map.rs Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> --- frame/support/src/storage/types/counted_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 217b26ee9df06..87ba997bd092a 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -248,7 +248,7 @@ where /// Iter over all value of the storage. /// - /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. + /// NOTE: If a value failed to decode because storage is corrupted then it is skipped. pub fn iter_values() -> crate::storage::PrefixIterator> { let map_iterator = ::Map::iter_values(); crate::storage::PrefixIterator { From cdb3fa3fc35ccbafd1a8232ddcd0ae516fd12491 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 6 Aug 2021 15:31:25 +0200 Subject: [PATCH 11/25] fix merge --- frame/support/src/storage/mod.rs | 2 +- frame/support/src/storage/types/counted_map.rs | 3 +-- frame/support/test/tests/pallet.rs | 8 ++++++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 09a0df3d5a71c..61a4939e2d1fb 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -825,7 +825,7 @@ impl PrefixIterator { previous_key: Vec, decode_fn: fn(&[u8], &[u8]) -> Result, ) -> Self { - PrefixIterator { prefix, previous_key, drain: false, closure: decode_fn } + PrefixIterator { prefix, previous_key, drain: false, closure: decode_fn, phantom: Default::default() } } /// Get the last key that has been iterated upon and return it. diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 87ba997bd092a..abe53bbab0a24 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -1,4 +1,4 @@ -use codec::{FullCodec, Decode, EncodeLike, Encode, Ref}; +use codec::{FullCodec, Decode, EncodeLike, Encode, Ref, MaxEncodedLen}; use crate::{ Never, traits::{StorageInstance, GetDefault, Get, StorageInfo}, @@ -15,7 +15,6 @@ use sp_runtime::traits::Saturating; use frame_metadata::{ DefaultByteGetter, StorageEntryModifier, }; -use max_encoded_len::MaxEncodedLen; /// A wrapper around a `StorageMap` and a `StorageValue` to keep track of how many items /// are in a map, without needing to iterate all the values. diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 8ead608e7de02..7a0744b2f8cd2 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -1430,12 +1430,16 @@ fn test_storage_info() { } }, StorageInfo { - prefix: prefix(b"Example", b"RenamedCountedMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"RenamedCountedMap".to_vec(), + prefix: prefix(b"Example", b"RenamedCountedMap").to_vec(), max_values: None, max_size: Some(1 + 4 + 8), }, StorageInfo { - prefix: prefix(b"Example", b"CounterForRenamedCountedMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"CounterForRenamedCountedMap".to_vec(), + prefix: prefix(b"Example", b"CounterForRenamedCountedMap").to_vec(), max_values: Some(1), max_size: Some(4), }, From f7417cf3ab9f0f69820bd9eb33070293a14200f8 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 6 Aug 2021 15:32:27 +0200 Subject: [PATCH 12/25] fmt --- .../procedural/src/pallet/expand/storage.rs | 15 +- .../procedural/src/pallet/parse/storage.rs | 14 +- frame/support/src/lib.rs | 4 +- frame/support/src/storage/mod.rs | 10 +- .../support/src/storage/types/counted_map.rs | 158 ++++++++++-------- frame/support/src/storage/types/mod.rs | 4 +- frame/support/test/tests/pallet.rs | 7 +- 7 files changed, 120 insertions(+), 92 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 77a9ce9edf1f5..4cd8c50d88aa5 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -61,7 +61,7 @@ fn check_prefix_duplicates( if let Some(other_dup_err) = set.insert(prefix.clone(), dup_err.clone()) { let mut err = dup_err; err.combine(other_dup_err); - return Err(err); + return Err(err) } if let Metadata::CountedMap { .. } = storage_def.metadata { @@ -78,7 +78,7 @@ fn check_prefix_duplicates( if let Some(other_dup_err) = set.insert(counter_prefix.clone(), counter_dup_err.clone()) { let mut err = counter_dup_err; err.combine(other_dup_err); - return Err(err); + return Err(err) } } @@ -146,7 +146,12 @@ pub fn process_generics(def: &mut Def) -> syn::Result<()> { args.args.push(syn::GenericArgument::Type(max_values)); }, StorageGenerics::CountedMap { - hasher, key, value, query_kind, on_empty, max_values + hasher, + key, + value, + query_kind, + on_empty, + max_values, } => { args.args.push(syn::GenericArgument::Type(hasher)); args.args.push(syn::GenericArgument::Type(key)); @@ -212,7 +217,9 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { // Check for duplicate prefixes let mut prefix_set = HashMap::new(); - let mut errors = def.storages.iter() + let mut errors = def + .storages + .iter() .filter_map(|storage_def| check_prefix_duplicates(storage_def, &mut prefix_set).err()); if let Some(mut final_error) = errors.next() { errors.for_each(|error| final_error.combine(error)); diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 85d8391ffffe7..8cf9b2ae381ae 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -344,13 +344,16 @@ fn process_named_generics( )?; StorageGenerics::CountedMap { - hasher: parsed.remove("Hasher") + hasher: parsed + .remove("Hasher") .map(|binding| binding.ty) .expect("checked above as mandatory generic"), - key: parsed.remove("Key") + key: parsed + .remove("Key") .map(|binding| binding.ty) .expect("checked above as mandatory generic"), - value: parsed.remove("Value") + value: parsed + .remove("Value") .map(|binding| binding.ty) .expect("checked above as mandatory generic"), query_kind: parsed.remove("QueryKind").map(|binding| binding.ty), @@ -461,10 +464,7 @@ fn process_unnamed_generics( ), StorageKind::CountedMap => ( None, - Metadata::CountedMap { - key: retrieve_arg(2)?, - value: retrieve_arg(3)?, - }, + Metadata::CountedMap { key: retrieve_arg(2)?, value: retrieve_arg(3)? }, retrieve_arg(4).ok(), ), StorageKind::DoubleMap => ( diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 9b1fe3b883bf6..826800e0b8778 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1282,8 +1282,8 @@ pub mod pallet_prelude { storage::{ bounded_vec::BoundedVec, types::{ - Key as NMapKey, OptionQuery, StorageDoubleMap, StorageMap, StorageNMap, - StorageValue, ValueQuery, CountedStorageMap, + CountedStorageMap, Key as NMapKey, OptionQuery, StorageDoubleMap, StorageMap, + StorageNMap, StorageValue, ValueQuery, }, }, traits::{ diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 61a4939e2d1fb..d5ecc60c1fe64 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -790,7 +790,7 @@ pub trait StorageNMap { /// If any decoding fails it skips it and continues to the next key. /// /// If draining, then the hook `OnRemoval::on_removal` is called after each removal. -pub struct PrefixIterator { +pub struct PrefixIterator { prefix: Vec, previous_key: Vec, /// If true then value are removed while iterating @@ -825,7 +825,13 @@ impl PrefixIterator { previous_key: Vec, decode_fn: fn(&[u8], &[u8]) -> Result, ) -> Self { - PrefixIterator { prefix, previous_key, drain: false, closure: decode_fn, phantom: Default::default() } + PrefixIterator { + prefix, + previous_key, + drain: false, + closure: decode_fn, + phantom: Default::default(), + } } /// Get the last key that has been iterated upon and return it. diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index abe53bbab0a24..a58e3b3c6ed39 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -1,20 +1,19 @@ -use codec::{FullCodec, Decode, EncodeLike, Encode, Ref, MaxEncodedLen}; use crate::{ - Never, - traits::{StorageInstance, GetDefault, Get, StorageInfo}, storage::{ - StorageAppend, StorageTryAppend, StorageDecodeLength, generator::StorageMap as _, + generator::StorageMap as _, types::{ - OptionQuery, ValueQuery, StorageMap, StorageValue, QueryKindTrait, - StorageMapMetadata, StorageValueMetadata + OptionQuery, QueryKindTrait, StorageMap, StorageMapMetadata, StorageValue, + StorageValueMetadata, ValueQuery, }, + StorageAppend, StorageDecodeLength, StorageTryAppend, }, + traits::{Get, GetDefault, StorageInfo, StorageInstance}, + Never, }; -use sp_std::prelude::*; +use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen, Ref}; +use frame_metadata::{DefaultByteGetter, StorageEntryModifier}; use sp_runtime::traits::Saturating; -use frame_metadata::{ - DefaultByteGetter, StorageEntryModifier, -}; +use sp_std::prelude::*; /// A wrapper around a `StorageMap` and a `StorageValue` to keep track of how many items /// are in a map, without needing to iterate all the values. @@ -29,10 +28,14 @@ use frame_metadata::{ /// Whenever the counter needs to be updated, an additional read and write occurs to update that /// counter. pub struct CountedStorageMap< - Prefix, Hasher, Key, Value, QueryKind=OptionQuery, OnEmpty=GetDefault, MaxValues=GetDefault, ->( - core::marker::PhantomData<(Prefix, Hasher, Key, Value, QueryKind, OnEmpty, MaxValues)> -); + Prefix, + Hasher, + Key, + Value, + QueryKind = OptionQuery, + OnEmpty = GetDefault, + MaxValues = GetDefault, +>(core::marker::PhantomData<(Prefix, Hasher, Key, Value, QueryKind, OnEmpty, MaxValues)>); /// The requirement for an instance of [`CountedStorageMap`]. pub trait CountedStorageMapInstance: StorageInstance { @@ -45,8 +48,7 @@ trait Helper { type Map; } -impl - Helper +impl Helper for CountedStorageMap { type Map = StorageMap; @@ -58,8 +60,7 @@ type CounterFor

= StorageValue<

::CounterPrefi /// [`crate::storage::PrefixIterator`]. pub struct OnRemovalCounterUpdate(core::marker::PhantomData); -impl - crate::storage::PrefixIteratorOnRemoval +impl crate::storage::PrefixIteratorOnRemoval for OnRemovalCounterUpdate { fn on_removal() { @@ -106,7 +107,10 @@ where } /// Store a value to be associated with the given key from the map. - pub fn insert + Clone, ValArg: EncodeLike>(key: KeyArg, val: ValArg) { + pub fn insert + Clone, ValArg: EncodeLike>( + key: KeyArg, + val: ValArg, + ) { if !::Map::contains_key(Ref::from(&key)) { CounterFor::::mutate(|value| value.saturating_inc()); } @@ -124,7 +128,7 @@ where /// Mutate the value under a key. pub fn mutate + Clone, R, F: FnOnce(&mut QueryKind::Query) -> R>( key: KeyArg, - f: F + f: F, ) -> R { Self::try_mutate(key, |v| Ok::(f(v))) .expect("`Never` can not be constructed; qed") @@ -149,7 +153,7 @@ where /// Mutate the value under a key. Deletes the item if mutated to a `None`. pub fn mutate_exists + Clone, R, F: FnOnce(&mut Option) -> R>( key: KeyArg, - f: F + f: F, ) -> R { Self::try_mutate_exists(key, |v| Ok::(f(v))) .expect("`Never` can not be constructed; qed") @@ -181,10 +185,8 @@ where /// Take the value under a key. pub fn take + Clone>(key: KeyArg) -> QueryKind::Query { - let removed_value = ::Map::mutate_exists( - key, - |value| core::mem::replace(value, None) - ); + let removed_value = + ::Map::mutate_exists(key, |value| core::mem::replace(value, None)); if removed_value.is_some() { CounterFor::::mutate(|value| value.saturating_dec()); } @@ -204,7 +206,7 @@ where EncodeLikeKey: EncodeLike + Clone, Item: Encode, EncodeLikeItem: EncodeLike, - Value: StorageAppend + Value: StorageAppend, { if !::Map::contains_key(Ref::from(&key)) { CounterFor::::mutate(|value| value.saturating_inc()); @@ -225,7 +227,8 @@ where /// `None` does not mean that `get()` does not return a value. The default value is completly /// ignored by this function. pub fn decode_len>(key: KeyArg) -> Option - where Value: StorageDecodeLength, + where + Value: StorageDecodeLength, { ::Map::decode_len(key) } @@ -234,7 +237,7 @@ where /// /// If the key doesn't exist, then it's a no-op. If it does, then it returns its value. pub fn migrate_key>( - key: KeyArg + key: KeyArg, ) -> Option { ::Map::migrate_key::(key) } @@ -286,10 +289,7 @@ where /// Try and append the given item to the value in the storage. /// /// Is only available if `Value` of the storage implements [`StorageTryAppend`]. - pub fn try_append( - key: KArg, - item: EncodeLikeItem, - ) -> Result<(), ()> + pub fn try_append(key: KArg, item: EncodeLikeItem) -> Result<(), ()> where KArg: EncodeLike + Clone, Item: Encode, @@ -395,7 +395,8 @@ pub trait CountedStorageMapMetadata { } impl CountedStorageMapMetadata - for CountedStorageMap where + for CountedStorageMap +where Prefix: CountedStorageMapInstance, Hasher: crate::hash::StorageHasher, Key: FullCodec, @@ -415,9 +416,8 @@ impl CountedStorageMa const COUNTER_DOC: &'static str = &"Counter for the related counted storage map"; } -impl - crate::traits::StorageInfoTrait for - CountedStorageMap +impl crate::traits::StorageInfoTrait + for CountedStorageMap where Prefix: CountedStorageMapInstance, Hasher: crate::hash::StorageHasher, @@ -435,21 +435,26 @@ where #[cfg(test)] mod test { use super::*; - use sp_io::{TestExternalities, hashing::twox_128}; - use crate::hash::*; - use crate::storage::types::ValueQuery; - use crate::storage::bounded_vec::BoundedVec; - use crate::traits::ConstU32; + use crate::{ + hash::*, + storage::{bounded_vec::BoundedVec, types::ValueQuery}, + traits::ConstU32, + }; + use sp_io::{hashing::twox_128, TestExternalities}; struct Prefix; impl StorageInstance for Prefix { - fn pallet_prefix() -> &'static str { "test" } + fn pallet_prefix() -> &'static str { + "test" + } const STORAGE_PREFIX: &'static str = "foo"; } struct CounterPrefix; impl StorageInstance for CounterPrefix { - fn pallet_prefix() -> &'static str { "test" } + fn pallet_prefix() -> &'static str { + "test" + } const STORAGE_PREFIX: &'static str = "counter_for_foo"; } impl CountedStorageMapInstance for Prefix { @@ -560,7 +565,9 @@ mod test { assert_eq!(*query, ADefault::get()); *query = 4; Result::<(), ()>::Err(()) - }).err().unwrap(); + }) + .err() + .unwrap(); assert_eq!(A::try_get(2), Err(())); assert_eq!(A::count(), 2); @@ -570,7 +577,8 @@ mod test { assert_eq!(*query, ADefault::get()); *query = 41; Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(A::try_get(2), Ok(41)); assert_eq!(A::count(), 3); @@ -580,7 +588,8 @@ mod test { assert_eq!(*query, 41); *query = 41; Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(A::try_get(2), Ok(41)); assert_eq!(A::count(), 3); @@ -590,7 +599,9 @@ mod test { assert_eq!(*query, None); *query = Some(4); Result::<(), ()>::Err(()) - }).err().unwrap(); + }) + .err() + .unwrap(); assert_eq!(A::try_get(1), Err(())); assert_eq!(A::count(), 3); @@ -600,7 +611,8 @@ mod test { assert_eq!(*query, None); *query = Some(43); Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(A::try_get(1), Ok(43)); assert_eq!(A::count(), 4); @@ -610,7 +622,8 @@ mod test { assert_eq!(*query, Some(43)); *query = Some(43); Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(A::try_get(1), Ok(43)); assert_eq!(A::count(), 4); @@ -618,8 +631,10 @@ mod test { // Try succeed mutate existing to non-existing. A::try_mutate_exists(1, |query| { assert_eq!(*query, Some(43)); - *query = None; Result::<(), ()>::Ok(()) - }).unwrap(); + *query = None; + Result::<(), ()>::Ok(()) + }) + .unwrap(); assert_eq!(A::try_get(1), Err(())); assert_eq!(A::count(), 3); @@ -768,7 +783,9 @@ mod test { assert_eq!(*query, None); *query = Some(4); Result::<(), ()>::Err(()) - }).err().unwrap(); + }) + .err() + .unwrap(); assert_eq!(B::try_get(2), Err(())); assert_eq!(B::count(), 2); @@ -778,7 +795,8 @@ mod test { assert_eq!(*query, None); *query = Some(41); Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(B::try_get(2), Ok(41)); assert_eq!(B::count(), 3); @@ -788,7 +806,8 @@ mod test { assert_eq!(*query, Some(41)); *query = Some(41); Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(B::try_get(2), Ok(41)); assert_eq!(B::count(), 3); @@ -798,7 +817,8 @@ mod test { assert_eq!(*query, Some(41)); *query = None; Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(B::try_get(2), Err(())); assert_eq!(B::count(), 2); @@ -810,7 +830,9 @@ mod test { assert_eq!(*query, None); *query = Some(4); Result::<(), ()>::Err(()) - }).err().unwrap(); + }) + .err() + .unwrap(); assert_eq!(B::try_get(1), Err(())); assert_eq!(B::count(), 3); @@ -820,7 +842,8 @@ mod test { assert_eq!(*query, None); *query = Some(43); Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(B::try_get(1), Ok(43)); assert_eq!(B::count(), 4); @@ -830,7 +853,8 @@ mod test { assert_eq!(*query, Some(43)); *query = Some(43); Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(B::try_get(1), Ok(43)); assert_eq!(B::count(), 4); @@ -840,7 +864,8 @@ mod test { assert_eq!(*query, Some(43)); *query = None; Result::<(), ()>::Ok(()) - }).unwrap(); + }) + .unwrap(); assert_eq!(B::try_get(1), Err(())); assert_eq!(B::count(), 3); @@ -930,11 +955,7 @@ mod test { TestExternalities::default().execute_with(|| { A::insert(1, 1); A::insert(2, 2); - A::translate_values::(|old_value| if old_value == 1 { - None - } else { - Some(1) - }); + A::translate_values::(|old_value| if old_value == 1 { None } else { Some(1) }); assert_eq!(A::count(), 1); assert_eq!(A::get(2), Some(1)); }) @@ -951,11 +972,9 @@ mod test { assert_eq!(A::count(), 2); - A::translate::(|key, value| if key == 1 { - None - } else { - Some(key as u32 * value) - }); + A::translate::( + |key, value| if key == 1 { None } else { Some(key as u32 * value) }, + ); assert_eq!(A::count(), 1); @@ -964,5 +983,4 @@ mod test { assert_eq!(A::count(), 0); }) } - } diff --git a/frame/support/src/storage/types/mod.rs b/frame/support/src/storage/types/mod.rs index 065d91faf673b..c265f379d8553 100644 --- a/frame/support/src/storage/types/mod.rs +++ b/frame/support/src/storage/types/mod.rs @@ -21,13 +21,14 @@ use codec::FullCodec; use frame_metadata::{DefaultByte, StorageEntryModifier}; +mod counted_map; mod double_map; mod key; mod map; mod nmap; mod value; -mod counted_map; +pub use counted_map::{CountedStorageMap, CountedStorageMapInstance, CountedStorageMapMetadata}; pub use double_map::{StorageDoubleMap, StorageDoubleMapMetadata}; pub use key::{ EncodeLikeTuple, HasKeyPrefix, HasReversibleKeyPrefix, Key, KeyGenerator, @@ -36,7 +37,6 @@ pub use key::{ pub use map::{StorageMap, StorageMapMetadata}; pub use nmap::{StorageNMap, StorageNMapMetadata}; pub use value::{StorageValue, StorageValueMetadata}; -pub use counted_map::{CountedStorageMap, CountedStorageMapMetadata, CountedStorageMapInstance}; /// Trait implementing how the storage optional value is converted into the queried type. /// diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 7a0744b2f8cd2..5d2d8de135cad 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -325,11 +325,8 @@ pub mod pallet { #[pallet::storage] #[pallet::storage_prefix = "RenamedCountedMap"] #[pallet::getter(fn counted_storage_map)] - pub type SomeCountedStorageMap = CountedStorageMap< - Hasher = Twox64Concat, - Key = u8, - Value = u32, - >; + pub type SomeCountedStorageMap = + CountedStorageMap; #[pallet::genesis_config] #[derive(Default)] From 61208a47581ce13d6e16cec16f294079d5b11ed4 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 6 Aug 2021 15:34:03 +0200 Subject: [PATCH 13/25] fix spelling --- frame/support/src/storage/mod.rs | 2 +- frame/support/src/storage/types/double_map.rs | 2 +- frame/support/src/storage/types/map.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index d5ecc60c1fe64..7a78873f64dc6 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -1139,7 +1139,7 @@ pub trait StoragePrefixedMap { /// Iter over all value of the storage. /// - /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. + /// NOTE: If a value failed to decode because storage is corrupted then it is skipped. fn iter_values() -> PrefixIterator { let prefix = Self::final_prefix(); PrefixIterator { diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 2db8a845c568c..7b0bc670aca4b 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -342,7 +342,7 @@ where /// Iter over all value of the storage. /// - /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. + /// NOTE: If a value failed to decode because storage is corrupted then it is skipped. pub fn iter_values() -> crate::storage::PrefixIterator { >::iter_values() } diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index 6b3cfe64eaec0..7951a81bd69a3 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -241,7 +241,7 @@ where /// Iter over all value of the storage. /// - /// NOTE: If a value failed to decode becaues storage is corrupted then it is skipped. + /// NOTE: If a value failed to decode because storage is corrupted then it is skipped. pub fn iter_values() -> crate::storage::PrefixIterator { >::iter_values() } From 30da101b600574e5c1beeb74e6ac07b33e5fbd76 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 6 Aug 2021 15:52:32 +0200 Subject: [PATCH 14/25] improve on removal --- frame/support/src/storage/mod.rs | 11 ++++++----- frame/support/src/storage/types/counted_map.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 7a78873f64dc6..a938f31b6a7e4 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -803,12 +803,13 @@ pub struct PrefixIterator { /// Trait for specialising on removal logic of [`PrefixIterator`]. pub trait PrefixIteratorOnRemoval { - fn on_removal(); + /// This function is called whenever a key/value is removed. + fn on_removal(key: &[u8], value: &[u8]); } -/// No-op implement. +/// No-op implementation. impl PrefixIteratorOnRemoval for () { - fn on_removal() {} + fn on_removal(_key: &[u8], _value: &[u8]) {} } impl PrefixIterator { @@ -878,7 +879,7 @@ impl Iterator for PrefixIterator::new( iter.prefix().to_vec(), stored_key, |mut raw_key_without_prefix, mut raw_value| { diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index a58e3b3c6ed39..ddaf05add4d17 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -63,7 +63,7 @@ pub struct OnRemovalCounterUpdate(core::marker::PhantomData); impl crate::storage::PrefixIteratorOnRemoval for OnRemovalCounterUpdate { - fn on_removal() { + fn on_removal(_key: &[u8], _value: &[u8]) { CounterFor::::mutate(|value| value.saturating_dec()); } } From c546c2e1f712b13d6f0c24f04987c2b969fe5354 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 6 Aug 2021 22:28:26 +0200 Subject: [PATCH 15/25] fix partial storage info --- .../support/src/storage/types/counted_map.rs | 24 ++++++++++++- frame/support/test/tests/pallet.rs | 35 +++++++++++++++---- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index ddaf05add4d17..d48d9e56e3282 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -7,7 +7,7 @@ use crate::{ }, StorageAppend, StorageDecodeLength, StorageTryAppend, }, - traits::{Get, GetDefault, StorageInfo, StorageInstance}, + traits::{Get, GetDefault, StorageInfo, StorageInstance, StorageInfoTrait}, Never, }; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen, Ref}; @@ -432,6 +432,28 @@ where } } +/// It doesn't require to implement `MaxEncodedLen` and give no information for `max_size`. +impl + crate::traits::PartialStorageInfoTrait + for CountedStorageMap +where + Prefix: CountedStorageMapInstance, + Hasher: crate::hash::StorageHasher, + Key: FullCodec, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn partial_storage_info() -> Vec { + [ + ::Map::partial_storage_info(), + CounterFor::::storage_info(), + ].concat() + } +} + + #[cfg(test)] mod test { use super::*; diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 5d2d8de135cad..e5b4ae79ddc32 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -419,6 +419,7 @@ pub mod pallet { } // Test that a pallet with non generic event and generic genesis_config is correctly handled +// and that a pallet without the attribute generate_storage_info is correctly handled. #[frame_support::pallet] pub mod pallet2 { use super::{SomeAssociation1, SomeType1}; @@ -449,6 +450,10 @@ pub mod pallet2 { #[pallet::storage] pub type SomeValue = StorageValue<_, Vec>; + #[pallet::storage] + pub type SomeCountedStorageMap = + CountedStorageMap; + #[pallet::event] pub enum Event { /// Something @@ -1445,12 +1450,28 @@ fn test_storage_info() { assert_eq!( Example2::storage_info(), - vec![StorageInfo { - pallet_name: b"Example2".to_vec(), - storage_name: b"SomeValue".to_vec(), - prefix: prefix(b"Example2", b"SomeValue").to_vec(), - max_values: Some(1), - max_size: None, - },], + vec![ + StorageInfo { + pallet_name: b"Example2".to_vec(), + storage_name: b"SomeValue".to_vec(), + prefix: prefix(b"Example2", b"SomeValue").to_vec(), + max_values: Some(1), + max_size: None, + }, + StorageInfo { + pallet_name: b"Example2".to_vec(), + storage_name: b"SomeCountedStorageMap".to_vec(), + prefix: prefix(b"Example2", b"SomeCountedStorageMap").to_vec(), + max_values: None, + max_size: None, + }, + StorageInfo { + pallet_name: b"Example2".to_vec(), + storage_name: b"CounterForSomeCountedStorageMap".to_vec(), + prefix: prefix(b"Example2", b"CounterForSomeCountedStorageMap").to_vec(), + max_values: Some(1), + max_size: Some(4), + }, + ], ); } From 1e11c1693b0425245e70b575921d7062f3fd531c Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 6 Aug 2021 22:36:55 +0200 Subject: [PATCH 16/25] fmt --- frame/support/src/storage/types/counted_map.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index d48d9e56e3282..2fc33a14dadf1 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -7,7 +7,7 @@ use crate::{ }, StorageAppend, StorageDecodeLength, StorageTryAppend, }, - traits::{Get, GetDefault, StorageInfo, StorageInstance, StorageInfoTrait}, + traits::{Get, GetDefault, StorageInfo, StorageInfoTrait, StorageInstance}, Never, }; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen, Ref}; @@ -446,14 +446,11 @@ where MaxValues: Get>, { fn partial_storage_info() -> Vec { - [ - ::Map::partial_storage_info(), - CounterFor::::storage_info(), - ].concat() + [::Map::partial_storage_info(), CounterFor::::storage_info()] + .concat() } } - #[cfg(test)] mod test { use super::*; From 497bc695a41fdef27a7253993018015abe56783f Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sat, 7 Aug 2021 12:33:31 +0200 Subject: [PATCH 17/25] add license --- .../support/src/storage/types/counted_map.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 2fc33a14dadf1..40625adcb371c 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -1,3 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Storage counted map type. + use crate::{ storage::{ generator::StorageMap as _, From cc249fbd1b8053da7205beed2559e7e94b51bbac Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sat, 7 Aug 2021 12:36:02 +0200 Subject: [PATCH 18/25] suggested renames --- .../procedural/src/pallet/expand/storage.rs | 8 ++- .../support/src/storage/types/counted_map.rs | 70 +++++++++---------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 4cd8c50d88aa5..0e8a52d43b103 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -50,7 +50,7 @@ fn counter_prefix(prefix: &str) -> String { fn check_prefix_duplicates( storage_def: &StorageDef, // A hashmap of all already used prefix and their associated error if duplication - set: &mut HashMap, + used_prefix: &mut HashMap, ) -> syn::Result<()> { let prefix = storage_def.prefix(); let dup_err = syn::Error::new( @@ -58,7 +58,7 @@ fn check_prefix_duplicates( format!("Duplicate storage prefixes found for `{}`", prefix), ); - if let Some(other_dup_err) = set.insert(prefix.clone(), dup_err.clone()) { + if let Some(other_dup_err) = used_prefix.insert(prefix.clone(), dup_err.clone()) { let mut err = dup_err; err.combine(other_dup_err); return Err(err) @@ -75,7 +75,9 @@ fn check_prefix_duplicates( ), ); - if let Some(other_dup_err) = set.insert(counter_prefix.clone(), counter_dup_err.clone()) { + if let Some(other_dup_err) = + used_prefix.insert(counter_prefix.clone(), counter_dup_err.clone()) + { let mut err = counter_dup_err; err.combine(other_dup_err); return Err(err) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 40625adcb371c..341931c87b930 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -62,12 +62,12 @@ pub trait CountedStorageMapInstance: StorageInstance { type CounterPrefix: StorageInstance; } -// Internal helper trait to access map from counted storage map. -trait Helper { +// Private helper trait to access map from counted storage map. +trait MapWrapper { type Map; } -impl Helper +impl MapWrapper for CountedStorageMap { type Map = StorageMap; @@ -100,29 +100,29 @@ where { /// Get the storage key used to fetch a value corresponding to a specific key. pub fn hashed_key_for>(key: KeyArg) -> Vec { - ::Map::hashed_key_for(key) + ::Map::hashed_key_for(key) } /// Does the value (explicitly) exist in storage? pub fn contains_key>(key: KeyArg) -> bool { - ::Map::contains_key(key) + ::Map::contains_key(key) } /// Load the value associated with the given key from the map. pub fn get>(key: KeyArg) -> QueryKind::Query { - ::Map::get(key) + ::Map::get(key) } /// Try to get the value for the given key from the map. /// /// Returns `Ok` if it exists, `Err` if not. pub fn try_get>(key: KeyArg) -> Result { - ::Map::try_get(key) + ::Map::try_get(key) } /// Swap the values of two keys. pub fn swap, KeyArg2: EncodeLike>(key1: KeyArg1, key2: KeyArg2) { - ::Map::swap(key1, key2) + ::Map::swap(key1, key2) } /// Store a value to be associated with the given key from the map. @@ -130,18 +130,18 @@ where key: KeyArg, val: ValArg, ) { - if !::Map::contains_key(Ref::from(&key)) { + if !::Map::contains_key(Ref::from(&key)) { CounterFor::::mutate(|value| value.saturating_inc()); } - ::Map::insert(key, val) + ::Map::insert(key, val) } /// Remove the value under a key. pub fn remove + Clone>(key: KeyArg) { - if ::Map::contains_key(Ref::from(&key)) { + if ::Map::contains_key(Ref::from(&key)) { CounterFor::::mutate(|value| value.saturating_dec()); } - ::Map::remove(key) + ::Map::remove(key) } /// Mutate the value under a key. @@ -161,9 +161,9 @@ where { Self::try_mutate_exists(key, |option_value_ref| { let option_value = core::mem::replace(option_value_ref, None); - let mut query = ::Map::from_optional_value_to_query(option_value); + let mut query = ::Map::from_optional_value_to_query(option_value); let res = f(&mut query); - let option_value = ::Map::from_query_to_optional_value(query); + let option_value = ::Map::from_query_to_optional_value(query); let _ = core::mem::replace(option_value_ref, option_value); res }) @@ -184,7 +184,7 @@ where KeyArg: EncodeLike + Clone, F: FnOnce(&mut Option) -> Result, { - ::Map::try_mutate_exists(key, |option_value| { + ::Map::try_mutate_exists(key, |option_value| { let existed = option_value.is_some(); let res = f(option_value); let exist = option_value.is_some(); @@ -205,11 +205,11 @@ where /// Take the value under a key. pub fn take + Clone>(key: KeyArg) -> QueryKind::Query { let removed_value = - ::Map::mutate_exists(key, |value| core::mem::replace(value, None)); + ::Map::mutate_exists(key, |value| core::mem::replace(value, None)); if removed_value.is_some() { CounterFor::::mutate(|value| value.saturating_dec()); } - ::Map::from_optional_value_to_query(removed_value) + ::Map::from_optional_value_to_query(removed_value) } /// Append the given items to the value in the storage. @@ -227,10 +227,10 @@ where EncodeLikeItem: EncodeLike, Value: StorageAppend, { - if !::Map::contains_key(Ref::from(&key)) { + if !::Map::contains_key(Ref::from(&key)) { CounterFor::::mutate(|value| value.saturating_inc()); } - ::Map::append(key, item) + ::Map::append(key, item) } /// Read the length of the storage value without decoding the entire value under the given @@ -249,7 +249,7 @@ where where Value: StorageDecodeLength, { - ::Map::decode_len(key) + ::Map::decode_len(key) } /// Migrate an item with the given `key` from a defunct `OldHasher` to the current hasher. @@ -258,20 +258,20 @@ where pub fn migrate_key>( key: KeyArg, ) -> Option { - ::Map::migrate_key::(key) + ::Map::migrate_key::(key) } /// Remove all value of the storage. pub fn remove_all() { CounterFor::::set(0u32); - ::Map::remove_all(None); + ::Map::remove_all(None); } /// Iter over all value of the storage. /// /// NOTE: If a value failed to decode because storage is corrupted then it is skipped. pub fn iter_values() -> crate::storage::PrefixIterator> { - let map_iterator = ::Map::iter_values(); + let map_iterator = ::Map::iter_values(); crate::storage::PrefixIterator { prefix: map_iterator.prefix, previous_key: map_iterator.previous_key, @@ -296,7 +296,7 @@ where /// /// This would typically be called inside the module implementation of on_runtime_upgrade. pub fn translate_values Option>(mut f: F) { - ::Map::translate_values(|old_value| { + ::Map::translate_values(|old_value| { let res = f(old_value); if res.is_none() { CounterFor::::mutate(|value| value.saturating_dec()); @@ -316,10 +316,10 @@ where Value: StorageTryAppend, { let bound = Value::bound(); - let current = ::Map::decode_len(Ref::from(&key)).unwrap_or_default(); + let current = ::Map::decode_len(Ref::from(&key)).unwrap_or_default(); if current < bound { CounterFor::::mutate(|value| value.saturating_inc()); - let key = ::Map::hashed_key_for(key); + let key = ::Map::hashed_key_for(key); sp_io::storage::append(&key, item.encode()); Ok(()) } else { @@ -360,7 +360,7 @@ where /// /// If you alter the map while doing this, you'll get undefined results. pub fn iter() -> crate::storage::PrefixIterator<(Key, Value), OnRemovalCounterUpdate> { - let map_iterator = ::Map::iter(); + let map_iterator = ::Map::iter(); crate::storage::PrefixIterator { prefix: map_iterator.prefix, previous_key: map_iterator.previous_key, @@ -374,7 +374,7 @@ where /// /// If you add elements to the map while doing this, you'll get undefined results. pub fn drain() -> crate::storage::PrefixIterator<(Key, Value), OnRemovalCounterUpdate> { - let map_iterator = ::Map::drain(); + let map_iterator = ::Map::drain(); crate::storage::PrefixIterator { prefix: map_iterator.prefix, previous_key: map_iterator.previous_key, @@ -390,7 +390,7 @@ where /// /// NOTE: If a value fail to decode because storage is corrupted then it is skipped. pub fn translate Option>(mut f: F) { - ::Map::translate(|key, old_value| { + ::Map::translate(|key, old_value| { let res = f(key, old_value); if res.is_none() { CounterFor::::mutate(|value| value.saturating_dec()); @@ -424,10 +424,10 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - const MODIFIER: StorageEntryModifier = ::Map::MODIFIER; - const HASHER: frame_metadata::StorageHasher = ::Map::HASHER; - const NAME: &'static str = ::Map::NAME; - const DEFAULT: DefaultByteGetter = ::Map::DEFAULT; + const MODIFIER: StorageEntryModifier = ::Map::MODIFIER; + const HASHER: frame_metadata::StorageHasher = ::Map::HASHER; + const NAME: &'static str = ::Map::NAME; + const DEFAULT: DefaultByteGetter = ::Map::DEFAULT; const COUNTER_NAME: &'static str = CounterFor::::NAME; const COUNTER_MODIFIER: StorageEntryModifier = CounterFor::::MODIFIER; const COUNTER_TY: &'static str = "u32"; @@ -447,7 +447,7 @@ where MaxValues: Get>, { fn storage_info() -> Vec { - [::Map::storage_info(), CounterFor::::storage_info()].concat() + [::Map::storage_info(), CounterFor::::storage_info()].concat() } } @@ -465,7 +465,7 @@ where MaxValues: Get>, { fn partial_storage_info() -> Vec { - [::Map::partial_storage_info(), CounterFor::::storage_info()] + [::Map::partial_storage_info(), CounterFor::::storage_info()] .concat() } } From 4d6de597993c2381619a93435304b57e90c1e814 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sat, 7 Aug 2021 12:36:50 +0200 Subject: [PATCH 19/25] fix typo --- frame/support/procedural/src/pallet/expand/storage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 0e8a52d43b103..7a839c2c0c5a2 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -50,7 +50,7 @@ fn counter_prefix(prefix: &str) -> String { fn check_prefix_duplicates( storage_def: &StorageDef, // A hashmap of all already used prefix and their associated error if duplication - used_prefix: &mut HashMap, + used_prefixes: &mut HashMap, ) -> syn::Result<()> { let prefix = storage_def.prefix(); let dup_err = syn::Error::new( @@ -58,7 +58,7 @@ fn check_prefix_duplicates( format!("Duplicate storage prefixes found for `{}`", prefix), ); - if let Some(other_dup_err) = used_prefix.insert(prefix.clone(), dup_err.clone()) { + if let Some(other_dup_err) = used_prefixes.insert(prefix.clone(), dup_err.clone()) { let mut err = dup_err; err.combine(other_dup_err); return Err(err) @@ -76,7 +76,7 @@ fn check_prefix_duplicates( ); if let Some(other_dup_err) = - used_prefix.insert(counter_prefix.clone(), counter_dup_err.clone()) + used_prefixes.insert(counter_prefix.clone(), counter_dup_err.clone()) { let mut err = counter_dup_err; err.combine(other_dup_err); From a4c7f3fbb7df446d01d7744a78f832f452f7b43d Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 15 Sep 2021 15:55:11 +0200 Subject: [PATCH 20/25] fix test --- frame/support/test/tests/pallet.rs | 61 ++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index e2977f0bd3c8c..e159e3afbe0fe 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -1198,6 +1198,26 @@ fn metadata() { default: vec![0], docs: vec![], }, + StorageEntryMetadata { + name: "RenamedCountedMap", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Twox64Concat, + ], + key: meta_type::(), + value: meta_type::(), + }, + default: vec![0], + docs: vec![], + }, + StorageEntryMetadata { + name: "CounterForRenamedCountedMap", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Plain(meta_type::()), + default: vec![0, 0, 0, 0], + docs: vec!["Counter for the related counted storage map"], + }, ], }), calls: Some(meta_type::>().into()), @@ -1249,27 +1269,6 @@ fn metadata() { default: vec![0], docs: vec![], }, - // StorageEntryMetadata { - // name: DecodeDifferent::Decoded("RenamedCountedMap".to_string()), - // modifier: StorageEntryModifier::Optional, - // ty: StorageEntryType::Map { - // key: DecodeDifferent::Decoded("u8".to_string()), - // value: DecodeDifferent::Decoded("u32".to_string()), - // hasher: StorageHasher::Twox64Concat, - // unused: false, - // }, - // default: DecodeDifferent::Decoded(vec![0]), - // documentation: DecodeDifferent::Decoded(vec![]), - // }, - // StorageEntryMetadata { - // name: DecodeDifferent::Decoded("CounterForRenamedCountedMap".to_string()), - // modifier: StorageEntryModifier::Default, - // ty: StorageEntryType::Plain(DecodeDifferent::Decoded("u32".to_string())), - // default: DecodeDifferent::Decoded(vec![0, 0, 0, 0]), - // documentation: DecodeDifferent::Decoded(vec![ - // "Counter for the related counted storage map".to_string(), - // ]), - // }, StorageEntryMetadata { name: "Value", modifier: StorageEntryModifier::Optional, @@ -1409,6 +1408,26 @@ fn metadata() { default: vec![0], docs: vec![], }, + StorageEntryMetadata { + name: "RenamedCountedMap", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Twox64Concat, + ], + key: meta_type::(), + value: meta_type::(), + }, + default: vec![0], + docs: vec![], + }, + StorageEntryMetadata { + name: "CounterForRenamedCountedMap", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Plain(meta_type::()), + default: vec![0, 0, 0, 0], + docs: vec!["Counter for the related counted storage map"], + }, ], }), calls: Some(meta_type::>().into()), From 4841dbb2ee122e4301aab6ff34ac4b20368b40f0 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 15 Sep 2021 15:55:46 +0200 Subject: [PATCH 21/25] fmt --- frame/support/src/lib.rs | 10 +- .../support/src/storage/types/counted_map.rs | 12 +- frame/support/src/storage/types/double_map.rs | 53 +++--- frame/support/src/storage/types/map.rs | 47 +++--- frame/support/src/storage/types/mod.rs | 2 +- frame/support/src/storage/types/nmap.rs | 153 ++++++++++-------- frame/support/src/storage/types/value.rs | 33 ++-- frame/support/test/tests/pallet.rs | 8 +- 8 files changed, 173 insertions(+), 145 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 02818af487b17..a4a467452391d 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1689,9 +1689,9 @@ pub mod pallet_prelude { /// NOTE: Storages use those prefixes so that values are stored after the key /// `Twox128(b"MyExample") ++ Twox128(b"Foo")`. /// -/// For the `CountedStorageMap` variant, the Prefix also implements `CountedStorageMapInstance`. -/// It associate a `CounterPrefix`, which is implemented same as above, but the storage prefix is -/// prepend with `"CounterFor"`. +/// For the `CountedStorageMap` variant, the Prefix also implements +/// `CountedStorageMapInstance`. It associate a `CounterPrefix`, which is implemented same as +/// above, but the storage prefix is prepend with `"CounterFor"`. /// /// The optional attribute `#[pallet::storage_prefix = "$custom_name"]` allows to define a /// specific name to use for the prefix. @@ -1751,8 +1751,8 @@ pub mod pallet_prelude { /// `_GeneratedPrefixForStorage$NameOfStorage`, and implements /// [`StorageInstance`](traits::StorageInstance) on it using the pallet and storage name. It /// then uses it as the first generic of the aliased type. -/// For `CountedStorageMap`, `CountedStorageMapInstance` is implemented, and another similar struct -/// is generated. +/// For `CountedStorageMap`, `CountedStorageMapInstance` is implemented, and another similar +/// struct is generated. /// /// For named generic, the macro will reorder the generics, and remove the names. /// diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index e9d720105d828..385e9f8f3046b 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -18,14 +18,17 @@ //! Storage counted map type. use crate::{ + metadata::StorageEntryMetadata, storage::{ generator::StorageMap as _, - types::{OptionQuery, QueryKindTrait, StorageMap, StorageValue, ValueQuery, StorageEntryMetadataBuilder}, + types::{ + OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder, StorageMap, StorageValue, + ValueQuery, + }, StorageAppend, StorageDecodeLength, StorageTryAppend, }, traits::{Get, GetDefault, StorageInfo, StorageInfoTrait, StorageInstance}, Never, - metadata::StorageEntryMetadata, }; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen, Ref}; use sp_runtime::traits::Saturating; @@ -410,7 +413,10 @@ where { fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { ::Map::build_metadata(docs, entries); - CounterFor::::build_metadata(vec![&"Counter for the related counted storage map"], entries); + CounterFor::::build_metadata( + vec![&"Counter for the related counted storage map"], + entries, + ); } } diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index f9dc439245f5f..b9af4a621b92a 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -19,7 +19,7 @@ //! StoragePrefixedDoubleMap traits and their methods directly. use crate::{ - metadata::{StorageEntryType, StorageEntryMetadata}, + metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, @@ -770,30 +770,37 @@ mod test { let mut entries = vec![]; A::build_metadata(vec![], &mut entries); AValueQueryWithAnOnEmpty::build_metadata(vec![], &mut entries); - assert_eq!(entries, + assert_eq!( + entries, vec![ - StorageEntryMetadata { - name: "foo", - modifier: StorageEntryModifier::Optional, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat, StorageHasher::Twox64Concat], - key: scale_info::meta_type::<(u16, u8)>(), - value: scale_info::meta_type::(), + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Blake2_128Concat, + StorageHasher::Twox64Concat + ], + key: scale_info::meta_type::<(u16, u8)>(), + value: scale_info::meta_type::(), + }, + default: Option::::None.encode(), + docs: vec![], }, - default: Option::::None.encode(), - docs: vec![], - }, - StorageEntryMetadata { - name: "foo", - modifier: StorageEntryModifier::Default, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat, StorageHasher::Twox64Concat], - key: scale_info::meta_type::<(u16, u8)>(), - value: scale_info::meta_type::(), - }, - default: 97u32.encode(), - docs: vec![], - } + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Blake2_128Concat, + StorageHasher::Twox64Concat + ], + key: scale_info::meta_type::<(u16, u8)>(), + value: scale_info::meta_type::(), + }, + default: 97u32.encode(), + docs: vec![], + } ] ); diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index b5dac1bce9aa7..45340f9015eaa 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -19,7 +19,7 @@ //! methods directly. use crate::{ - metadata::{StorageEntryType, StorageEntryMetadata}, + metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, @@ -576,30 +576,31 @@ mod test { let mut entries = vec![]; A::build_metadata(vec![], &mut entries); AValueQueryWithAnOnEmpty::build_metadata(vec![], &mut entries); - assert_eq!(entries, + assert_eq!( + entries, vec![ - StorageEntryMetadata { - name: "foo", - modifier: StorageEntryModifier::Optional, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat], - key: scale_info::meta_type::(), - value: scale_info::meta_type::(), + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![StorageHasher::Blake2_128Concat], + key: scale_info::meta_type::(), + value: scale_info::meta_type::(), + }, + default: Option::::None.encode(), + docs: vec![], }, - default: Option::::None.encode(), - docs: vec![], - }, - StorageEntryMetadata { - name: "foo", - modifier: StorageEntryModifier::Default, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat], - key: scale_info::meta_type::(), - value: scale_info::meta_type::(), - }, - default: 97u32.encode(), - docs: vec![], - } + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Map { + hashers: vec![StorageHasher::Blake2_128Concat], + key: scale_info::meta_type::(), + value: scale_info::meta_type::(), + }, + default: 97u32.encode(), + docs: vec![], + } ] ); diff --git a/frame/support/src/storage/types/mod.rs b/frame/support/src/storage/types/mod.rs index 2b4301400fb64..bcab996f68323 100644 --- a/frame/support/src/storage/types/mod.rs +++ b/frame/support/src/storage/types/mod.rs @@ -18,7 +18,7 @@ //! Storage types to build abstraction on storage, they implements storage traits such as //! StorageMap and others. -use crate::metadata::{StorageEntryModifier, StorageEntryMetadata}; +use crate::metadata::{StorageEntryMetadata, StorageEntryModifier}; use codec::FullCodec; use sp_std::prelude::*; diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index 4a70cfdcd9475..96d6f383ae117 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -19,7 +19,7 @@ //! StoragePrefixedDoubleMap traits and their methods directly. use crate::{ - metadata::{StorageEntryType, StorageEntryMetadata}, + metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{ EncodeLikeTuple, HasKeyPrefix, HasReversibleKeyPrefix, OptionQuery, QueryKindTrait, @@ -517,7 +517,7 @@ where mod test { use super::*; use crate::{ - hash::{*, StorageHasher as _}, + hash::{StorageHasher as _, *}, metadata::{StorageEntryModifier, StorageHasher}, storage::types::{Key, ValueQuery}, }; @@ -688,30 +688,31 @@ mod test { let mut entries = vec![]; A::build_metadata(vec![], &mut entries); AValueQueryWithAnOnEmpty::build_metadata(vec![], &mut entries); - assert_eq!(entries, + assert_eq!( + entries, vec![ - StorageEntryMetadata { - name: "Foo", - modifier: StorageEntryModifier::Optional, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat], - key: scale_info::meta_type::(), - value: scale_info::meta_type::(), - }, - default: Option::::None.encode(), - docs: vec![], - }, - StorageEntryMetadata { - name: "Foo", - modifier: StorageEntryModifier::Default, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat], - key: scale_info::meta_type::(), - value: scale_info::meta_type::(), + StorageEntryMetadata { + name: "Foo", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![StorageHasher::Blake2_128Concat], + key: scale_info::meta_type::(), + value: scale_info::meta_type::(), + }, + default: Option::::None.encode(), + docs: vec![], }, - default: 98u32.encode(), - docs: vec![], - } + StorageEntryMetadata { + name: "Foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Map { + hashers: vec![StorageHasher::Blake2_128Concat], + key: scale_info::meta_type::(), + value: scale_info::meta_type::(), + }, + default: 98u32.encode(), + docs: vec![], + } ] ); @@ -880,30 +881,37 @@ mod test { let mut entries = vec![]; A::build_metadata(vec![], &mut entries); AValueQueryWithAnOnEmpty::build_metadata(vec![], &mut entries); - assert_eq!(entries, + assert_eq!( + entries, vec![ - StorageEntryMetadata { - name: "Foo", - modifier: StorageEntryModifier::Optional, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat, StorageHasher::Twox64Concat], - key: scale_info::meta_type::<(u16, u8)>(), - value: scale_info::meta_type::(), + StorageEntryMetadata { + name: "Foo", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Blake2_128Concat, + StorageHasher::Twox64Concat + ], + key: scale_info::meta_type::<(u16, u8)>(), + value: scale_info::meta_type::(), + }, + default: Option::::None.encode(), + docs: vec![], }, - default: Option::::None.encode(), - docs: vec![], - }, - StorageEntryMetadata { - name: "Foo", - modifier: StorageEntryModifier::Default, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat, StorageHasher::Twox64Concat], - key: scale_info::meta_type::<(u16, u8)>(), - value: scale_info::meta_type::(), - }, - default: 98u32.encode(), - docs: vec![], - } + StorageEntryMetadata { + name: "Foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Blake2_128Concat, + StorageHasher::Twox64Concat + ], + key: scale_info::meta_type::<(u16, u8)>(), + value: scale_info::meta_type::(), + }, + default: 98u32.encode(), + docs: vec![], + } ] ); @@ -1094,30 +1102,39 @@ mod test { let mut entries = vec![]; A::build_metadata(vec![], &mut entries); AValueQueryWithAnOnEmpty::build_metadata(vec![], &mut entries); - assert_eq!(entries, + assert_eq!( + entries, vec![ - StorageEntryMetadata { - name: "Foo", - modifier: StorageEntryModifier::Optional, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat, StorageHasher::Blake2_128Concat, StorageHasher::Twox64Concat], - key: scale_info::meta_type::<(u16, u16, u16)>(), - value: scale_info::meta_type::(), - }, - default: Option::::None.encode(), - docs: vec![], - }, - StorageEntryMetadata { - name: "Foo", - modifier: StorageEntryModifier::Default, - ty: StorageEntryType::Map { - hashers: vec![StorageHasher::Blake2_128Concat, StorageHasher::Blake2_128Concat, StorageHasher::Twox64Concat], - key: scale_info::meta_type::<(u16, u16, u16)>(), - value: scale_info::meta_type::(), + StorageEntryMetadata { + name: "Foo", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Blake2_128Concat, + StorageHasher::Blake2_128Concat, + StorageHasher::Twox64Concat + ], + key: scale_info::meta_type::<(u16, u16, u16)>(), + value: scale_info::meta_type::(), + }, + default: Option::::None.encode(), + docs: vec![], }, - default: 98u32.encode(), - docs: vec![], - } + StorageEntryMetadata { + name: "Foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Map { + hashers: vec![ + StorageHasher::Blake2_128Concat, + StorageHasher::Blake2_128Concat, + StorageHasher::Twox64Concat + ], + key: scale_info::meta_type::<(u16, u16, u16)>(), + value: scale_info::meta_type::(), + }, + default: 98u32.encode(), + docs: vec![], + } ] ); diff --git a/frame/support/src/storage/types/value.rs b/frame/support/src/storage/types/value.rs index 2ee33672a3519..c5e7173bd0af7 100644 --- a/frame/support/src/storage/types/value.rs +++ b/frame/support/src/storage/types/value.rs @@ -18,7 +18,7 @@ //! Storage value type. Implements StorageValue trait and its method directly. use crate::{ - metadata::{StorageEntryType, StorageEntryMetadata}, + metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ generator::StorageValue as StorageValueT, types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, @@ -346,22 +346,23 @@ mod test { let mut entries = vec![]; A::build_metadata(vec![], &mut entries); AValueQueryWithAnOnEmpty::build_metadata(vec![], &mut entries); - assert_eq!(entries, + assert_eq!( + entries, vec![ - StorageEntryMetadata { - name: "foo", - modifier: StorageEntryModifier::Optional, - ty: StorageEntryType::Plain(scale_info::meta_type::()), - default: Option::::None.encode(), - docs: vec![], - }, - StorageEntryMetadata { - name: "foo", - modifier: StorageEntryModifier::Default, - ty: StorageEntryType::Plain(scale_info::meta_type::()), - default: 97u32.encode(), - docs: vec![], - } + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Plain(scale_info::meta_type::()), + default: Option::::None.encode(), + docs: vec![], + }, + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Plain(scale_info::meta_type::()), + default: 97u32.encode(), + docs: vec![], + } ] ); diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index e159e3afbe0fe..6a9a18ea48d4b 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -1202,9 +1202,7 @@ fn metadata() { name: "RenamedCountedMap", modifier: StorageEntryModifier::Optional, ty: StorageEntryType::Map { - hashers: vec![ - StorageHasher::Twox64Concat, - ], + hashers: vec![StorageHasher::Twox64Concat], key: meta_type::(), value: meta_type::(), }, @@ -1412,9 +1410,7 @@ fn metadata() { name: "RenamedCountedMap", modifier: StorageEntryModifier::Optional, ty: StorageEntryType::Map { - hashers: vec![ - StorageHasher::Twox64Concat, - ], + hashers: vec![StorageHasher::Twox64Concat], key: meta_type::(), value: meta_type::(), }, From b3d2111bc696c7363111e6bac60a3d4fcb24ac5f Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 16 Sep 2021 11:52:35 +0200 Subject: [PATCH 22/25] fix ui tests --- ...torage_ensure_span_are_ok_on_wrong_gen.stderr | 16 ++++++++-------- ...nsure_span_are_ok_on_wrong_gen_unnamed.stderr | 16 ++++++++-------- .../storage_info_unsatisfied_nmap.stderr | 4 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr index e78eb7ff9537b..239de4dba949b 100644 --- a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr +++ b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr @@ -5,8 +5,8 @@ error[E0277]: the trait bound `Bar: TypeInfo` is not satisfied | ^^^^^^^ the trait `TypeInfo` is not implemented for `Bar` | = note: required because of the requirements on the impl of `StaticTypeInfo` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen.rs:20:12 @@ -16,8 +16,8 @@ error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | = note: required because of the requirements on the impl of `Decode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen.rs:20:12 @@ -27,8 +27,8 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied | = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen.rs:20:12 @@ -39,8 +39,8 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied = note: required because of the requirements on the impl of `Encode` for `Bar` = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen.rs:9:12 diff --git a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr index d9a7ddbf3443e..a5bf32a0ef2d2 100644 --- a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr +++ b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr @@ -5,8 +5,8 @@ error[E0277]: the trait bound `Bar: TypeInfo` is not satisfied | ^^^^^^^ the trait `TypeInfo` is not implemented for `Bar` | = note: required because of the requirements on the impl of `StaticTypeInfo` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:20:12 @@ -16,8 +16,8 @@ error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | = note: required because of the requirements on the impl of `Decode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:20:12 @@ -27,8 +27,8 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied | = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:20:12 @@ -39,8 +39,8 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied = note: required because of the requirements on the impl of `Encode` for `Bar` = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` - = note: required because of the requirements on the impl of `frame_support::storage::StorageEntryMetadata` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` - = note: required by `NAME` + = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` + = note: required by `build_metadata` error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied --> $DIR/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:9:12 diff --git a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr index 545520124bfee..6c92423c6a7fe 100644 --- a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr +++ b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr @@ -4,6 +4,6 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied 10 | #[pallet::generate_storage_info] | ^^^^^^^^^^^^^^^^^^^^^ the trait `MaxEncodedLen` is not implemented for `Bar` | - = note: required because of the requirements on the impl of `KeyGeneratorMaxEncodedLen` for `NMapKey` - = note: required because of the requirements on the impl of `StorageInfoTrait` for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, NMapKey, u32>` + = note: required because of the requirements on the impl of `KeyGeneratorMaxEncodedLen` for `Key` + = note: required because of the requirements on the impl of `StorageInfoTrait` for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, Key, u32>` = note: required by `storage_info` From 4b62aab7ebbac495581f23025f4ab5e46903361b Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 16 Sep 2021 12:41:51 +0200 Subject: [PATCH 23/25] clearer doc --- frame/support/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index a4a467452391d..f9758573b5c53 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1684,14 +1684,14 @@ pub mod pallet_prelude { /// The Prefix generic written by the macro is generated using /// `PalletInfo::name::>()` and the name of the storage type. /// E.g. if runtime names the pallet "MyExample" then the storage `type Foo = ...` use the -/// pallet prefix: "MyExample" and the storage prefix "Foo". -/// -/// NOTE: Storages use those prefixes so that values are stored after the key -/// `Twox128(b"MyExample") ++ Twox128(b"Foo")`. +/// prefix: `Twox128(b"MyExample") ++ Twox128(b"Foo")`. /// /// For the `CountedStorageMap` variant, the Prefix also implements /// `CountedStorageMapInstance`. It associate a `CounterPrefix`, which is implemented same as /// above, but the storage prefix is prepend with `"CounterFor"`. +/// E.g. if runtime names the pallet "MyExample" then the storage +/// `type Foo = CountedStorageaMap<...>` will store its counter at the prefix: +/// `Twox128(b"MyExample") ++ Twox128(b"CounterForFoo")`. /// /// The optional attribute `#[pallet::storage_prefix = "$custom_name"]` allows to define a /// specific name to use for the prefix. From 448937dfc7de5bf173eed0597f074f93db2c289b Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 16 Sep 2021 12:42:48 +0200 Subject: [PATCH 24/25] better doc --- frame/support/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index f9758573b5c53..459698707366d 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1693,13 +1693,9 @@ pub mod pallet_prelude { /// `type Foo = CountedStorageaMap<...>` will store its counter at the prefix: /// `Twox128(b"MyExample") ++ Twox128(b"CounterForFoo")`. /// -/// The optional attribute `#[pallet::storage_prefix = "$custom_name"]` allows to define a -/// specific name to use for the prefix. -/// /// E.g: /// ```ignore /// #[pallet::storage] -/// #[pallet::storage_prefix = "OtherName"] /// pub(super) type MyStorage = StorageMap; /// ``` /// In this case the final prefix used by the map is From cc13432edf9d8900cce3aba2290e8a2fb89a1b45 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 16 Sep 2021 12:54:09 +0200 Subject: [PATCH 25/25] add metadata test --- .../support/src/storage/types/counted_map.rs | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 385e9f8f3046b..0860a4ed541c6 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -460,6 +460,7 @@ mod test { use super::*; use crate::{ hash::*, + metadata::{StorageEntryModifier, StorageEntryType, StorageHasher}, storage::{bounded_vec::BoundedVec, types::ValueQuery}, traits::ConstU32, }; @@ -1006,5 +1007,34 @@ mod test { assert_eq!(A::count(), 0); }) } - // TODO TODO: add test for metadata similar map + + #[test] + fn test_metadata() { + type A = CountedStorageMap; + let mut entries = vec![]; + A::build_metadata(vec![], &mut entries); + assert_eq!( + entries, + vec![ + StorageEntryMetadata { + name: "foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Map { + hashers: vec![StorageHasher::Twox64Concat], + key: scale_info::meta_type::(), + value: scale_info::meta_type::(), + }, + default: 97u32.encode(), + docs: vec![], + }, + StorageEntryMetadata { + name: "counter_for_foo", + modifier: StorageEntryModifier::Default, + ty: StorageEntryType::Plain(scale_info::meta_type::()), + default: vec![0, 0, 0, 0], + docs: vec!["Counter for the related counted storage map"], + }, + ] + ); + } }