From 13f9256c45c298861a28abfb35fe0179957c30bd Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 1 Sep 2021 15:10:15 +0200 Subject: [PATCH 1/5] allow unbounded individual storage --- .../src/pallet/expand/pallet_struct.rs | 49 +++++++++------ .../procedural/src/pallet/parse/storage.rs | 61 +++++++++++++------ frame/support/src/lib.rs | 10 ++- frame/support/test/tests/pallet.rs | 22 +++++++ 4 files changed, 101 insertions(+), 41 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 40fc39b161f15..c3eef7769646a 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -103,28 +103,39 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { ) }; - // Depending on the flag `generate_storage_info` we use partial or full storage info from - // storage. - let (storage_info_span, storage_info_trait, storage_info_method) = - if let Some(span) = def.pallet_struct.generate_storage_info { - ( - span, - quote::quote_spanned!(span => StorageInfoTrait), - quote::quote_spanned!(span => storage_info), - ) - } else { - let span = def.pallet_struct.attr_span; - ( - span, - quote::quote_spanned!(span => PartialStorageInfoTrait), - quote::quote_spanned!(span => partial_storage_info), - ) - }; + let storage_info_span = + def.pallet_struct.generate_storage_info.unwrap_or(def.pallet_struct.attr_span); let storage_names = &def.storages.iter().map(|storage| &storage.ident).collect::>(); let storage_cfg_attrs = &def.storages.iter().map(|storage| &storage.cfg_attrs).collect::>(); + // Depending on the flag `generate_storage_info` and the storage attribute `unbounded`, we use + // partial or full storage info from storage. + let storage_info_traits = &def + .storages + .iter() + .map(|storage| { + if storage.unbounded || def.pallet_struct.generate_storage_info.is_none() { + quote::quote_spanned!(storage_info_span => PartialStorageInfoTrait) + } else { + quote::quote_spanned!(storage_info_span => StorageInfoTrait) + } + }) + .collect::>(); + + let storage_info_methods = &def + .storages + .iter() + .map(|storage| { + if storage.unbounded || def.pallet_struct.generate_storage_info.is_none() { + quote::quote_spanned!(storage_info_span => partial_storage_info) + } else { + quote::quote_spanned!(storage_info_span => storage_info) + } + }) + .collect::>(); + let storage_info = quote::quote_spanned!(storage_info_span => impl<#type_impl_gen> #frame_support::traits::StorageInfoTrait for #pallet_ident<#type_use_gen> @@ -141,8 +152,8 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { { let mut storage_info = < #storage_names<#type_use_gen> - as #frame_support::traits::#storage_info_trait - >::#storage_info_method(); + as #frame_support::traits::#storage_info_traits + >::#storage_info_methods(); res.append(&mut storage_info); } )* diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 5df7bc132dff4..d323fa3c416be 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -26,6 +26,7 @@ mod keyword { syn::custom_keyword!(pallet); syn::custom_keyword!(getter); syn::custom_keyword!(storage_prefix); + syn::custom_keyword!(unbounded); syn::custom_keyword!(OptionQuery); syn::custom_keyword!(ValueQuery); } @@ -36,12 +37,13 @@ mod keyword { pub enum PalletStorageAttr { Getter(syn::Ident, proc_macro2::Span), StorageName(syn::LitStr, proc_macro2::Span), + Unbounded(proc_macro2::Span), } impl PalletStorageAttr { fn attr_span(&self) -> proc_macro2::Span { match self { - Self::Getter(_, span) | Self::StorageName(_, span) => *span, + Self::Getter(_, span) | Self::StorageName(_, span) | Self::Unbounded(span) => *span, } } } @@ -75,12 +77,45 @@ impl syn::parse::Parse for PalletStorageAttr { })?; Ok(Self::StorageName(renamed_prefix, attr_span)) + } else if lookahead.peek(keyword::unbounded) { + content.parse::()?; + + Ok(Self::Unbounded(attr_span)) } else { Err(lookahead.error()) } } } +struct PalletStorageAttrInfo { + getter: Option, + rename_as: Option, + unbounded: bool, +} + +impl PalletStorageAttrInfo { + fn from_attrs(attrs: Vec) -> syn::Result { + let mut getter = None; + let mut rename_as = None; + let mut unbounded = None; + for attr in attrs { + match attr { + PalletStorageAttr::Getter(ident, ..) if getter.is_none() => getter = Some(ident), + PalletStorageAttr::StorageName(name, ..) if rename_as.is_none() => + rename_as = Some(name), + PalletStorageAttr::Unbounded(..) if unbounded.is_none() => unbounded = Some(()), + attr => + return Err(syn::Error::new( + attr.attr_span(), + "Invalid attribute: Duplicate attribute", + )), + } + } + + Ok(PalletStorageAttrInfo { getter, rename_as, unbounded: unbounded.is_some() }) + } +} + /// The value and key types used by storages. Needed to expand metadata. pub enum Metadata { Value { value: syn::Type }, @@ -129,6 +164,8 @@ pub struct StorageDef { /// generics of the storage. /// If generics are not named, this is none. pub named_generics: Option, + /// If the value stored in this storage is unbounded. + pub unbounded: bool, } /// The parsed generic from the @@ -583,25 +620,8 @@ impl StorageDef { }; let attrs: Vec = helper::take_item_pallet_attrs(&mut item.attrs)?; - let (mut getters, mut names) = attrs - .into_iter() - .partition::, _>(|attr| matches!(attr, PalletStorageAttr::Getter(..))); - if getters.len() > 1 { - let msg = "Invalid pallet::storage, multiple argument pallet::getter found"; - return Err(syn::Error::new(getters[1].attr_span(), msg)) - } - if names.len() > 1 { - let msg = "Invalid pallet::storage, multiple argument pallet::storage_prefix found"; - return Err(syn::Error::new(names[1].attr_span(), msg)) - } - let getter = getters.pop().map(|attr| match attr { - PalletStorageAttr::Getter(ident, _) => ident, - _ => unreachable!(), - }); - let rename_as = names.pop().map(|attr| match attr { - PalletStorageAttr::StorageName(lit, _) => lit, - _ => unreachable!(), - }); + let PalletStorageAttrInfo { getter, rename_as, unbounded } = + PalletStorageAttrInfo::from_attrs(attrs)?; let cfg_attrs = helper::get_item_cfg_attrs(&item.attrs); @@ -658,6 +678,7 @@ impl StorageDef { where_clause, cfg_attrs, named_generics, + unbounded, }) } } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 105b2328f2325..c06d8abc74a24 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1398,15 +1398,17 @@ pub mod pallet_prelude { /// `::Foo`. /// /// To generate the full storage info (used for PoV calculation) use the attribute -/// `#[pallet::set_storage_max_encoded_len]`, e.g.: +/// `#[pallet::generate_storage_info]`, e.g.: /// ```ignore /// #[pallet::pallet] -/// #[pallet::set_storage_max_encoded_len] +/// #[pallet::generate_storage_info] /// pub struct Pallet(_); /// ``` /// /// This require all storage to implement the trait [`traits::StorageInfoTrait`], thus all keys /// and value types must bound [`pallet_prelude::MaxEncodedLen`]. +/// Some individual storage can opt-out from this constraint by using `#[pallet::unbounded]`, +/// see `#[pallet::storage]` documentation. /// /// As the macro implements [`traits::GetStorageVersion`], the current storage version needs to /// be communicated to the macro. This can be done by using the `storage_version` attribute: @@ -1721,6 +1723,10 @@ pub mod pallet_prelude { /// pub(super) type MyStorage = StorageMap<_, Blake2_128Concat, u32, u32>; /// ``` /// +/// The optional attribute `#[pallet::unbounded]` allows to declare the storage as unbounded. +/// When implementating the storage info, the size of the storage will be declared as +/// unbounded. This can be useful for storage which can never go into PoV (Proof of Validity). +/// /// The optional attributes `#[cfg(..)]` allow conditional compilation for the storage. /// /// E.g: diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 80bae000b6c77..70d6bdfe584ba 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -322,6 +322,10 @@ pub mod pallet { pub type ConditionalNMap = StorageNMap<_, (storage::Key, storage::Key), u32>; + #[pallet::storage] + #[pallet::unbounded] + pub type Unbounded = StorageValue>; + #[pallet::genesis_config] #[derive(Default)] pub struct GenesisConfig { @@ -889,6 +893,10 @@ fn storage_expand() { pallet::ConditionalDoubleMap::::insert(1, 2, 3); pallet::ConditionalNMap::::insert((1, 2), 3); } + + pallet::Unbounded::::put(vec![1, 2]); + let k = [twox_128(b"Example"), twox_128(b"Unbounded")].concat(); + assert_eq!(unhashed::get::>(&k), Some(vec![1, 2])); }) } @@ -1126,6 +1134,13 @@ fn metadata() { default: DecodeDifferent::Decoded(vec![0]), documentation: DecodeDifferent::Decoded(vec![]), }, + StorageEntryMetadata { + name: DecodeDifferent::Decoded("Unbounded".to_string()), + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Plain(DecodeDifferent::Decoded("Vec".to_string())), + default: DecodeDifferent::Decoded(vec![0]), + documentation: DecodeDifferent::Decoded(vec![]), + }, ]), })), calls: Some(DecodeDifferent::Decoded(vec![ @@ -1377,6 +1392,13 @@ fn test_storage_info() { max_size: Some(7 + 16 + 8), } }, + StorageInfo { + pallet_name: b"Example".to_vec(), + storage_name: b"Unbounded".to_vec(), + prefix: prefix(b"Example", b"Unbounded").to_vec(), + max_values: Some(1), + max_size: None, + } ], ); From 6cf1f535d9c8b0f92f367acc94692228a216ae3e Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 1 Sep 2021 15:13:18 +0200 Subject: [PATCH 2/5] better doc --- frame/support/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index c06d8abc74a24..ee39db98ece7e 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1724,8 +1724,9 @@ pub mod pallet_prelude { /// ``` /// /// The optional attribute `#[pallet::unbounded]` allows to declare the storage as unbounded. -/// When implementating the storage info, the size of the storage will be declared as -/// unbounded. This can be useful for storage which can never go into PoV (Proof of Validity). +/// When implementating the storage info (when #[pallet::generate_storage_info]` is specified +/// on the pallet struct placeholder), the size of the storage will be declared as unbounded. +/// This can be useful for storage which can never go into PoV (Proof of Validity). /// /// The optional attributes `#[cfg(..)]` allow conditional compilation for the storage. /// From 203ed15935c794ca7d6f7a9cfbe401ee66fc1fa3 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 7 Sep 2021 14:07:22 +0200 Subject: [PATCH 3/5] fix UI tests --- .../test/tests/pallet_ui/storage_invalid_attribute.stderr | 2 +- .../test/tests/pallet_ui/storage_multiple_getters.stderr | 2 +- .../test/tests/pallet_ui/storage_multiple_renames.stderr | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr index bf93d99cf56bd..6313bd691f943 100644 --- a/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr +++ b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr @@ -1,4 +1,4 @@ -error: expected `getter` or `storage_prefix` +error: expected one of: `getter`, `storage_prefix`, `unbounded` --> $DIR/storage_invalid_attribute.rs:16:12 | 16 | #[pallet::generate_store(pub trait Store)] diff --git a/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr b/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr index 188eed3cb0d17..40f57f16e0df5 100644 --- a/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr +++ b/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr @@ -1,4 +1,4 @@ -error: Invalid pallet::storage, multiple argument pallet::getter found +error: Invalid attribute: Duplicate attribute --> $DIR/storage_multiple_getters.rs:20:3 | 20 | #[pallet::getter(fn foo_error)] diff --git a/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr b/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr index 9288d131d95af..52cb7e85adf21 100644 --- a/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr +++ b/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr @@ -1,4 +1,4 @@ -error: Invalid pallet::storage, multiple argument pallet::storage_prefix found +error: Invalid attribute: Duplicate attribute --> $DIR/storage_multiple_renames.rs:20:3 | 20 | #[pallet::storage_prefix = "Baz"] From 66b43f54cb745f24577c5d317871f1eeacf959f8 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 14 Sep 2021 12:35:37 +0200 Subject: [PATCH 4/5] update doc --- frame/support/procedural/src/pallet/parse/storage.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index d323fa3c416be..3f6c2b68086f7 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -34,6 +34,7 @@ mod keyword { /// Parse for one of the following: /// * `#[pallet::getter(fn dummy)]` /// * `#[pallet::storage_prefix = "CustomName"]` +/// * `#[pallet::unbounded]` pub enum PalletStorageAttr { Getter(syn::Ident, proc_macro2::Span), StorageName(syn::LitStr, proc_macro2::Span), From 89d472a522e7a1f50bdff8f3d2f14cefd83601b0 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Tue, 14 Sep 2021 12:40:18 +0200 Subject: [PATCH 5/5] Update frame/support/procedural/src/pallet/parse/storage.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/support/procedural/src/pallet/parse/storage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 3f6c2b68086f7..00f73f9f1d429 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -98,13 +98,13 @@ impl PalletStorageAttrInfo { fn from_attrs(attrs: Vec) -> syn::Result { let mut getter = None; let mut rename_as = None; - let mut unbounded = None; + let mut unbounded = false; for attr in attrs { match attr { PalletStorageAttr::Getter(ident, ..) if getter.is_none() => getter = Some(ident), PalletStorageAttr::StorageName(name, ..) if rename_as.is_none() => rename_as = Some(name), - PalletStorageAttr::Unbounded(..) if unbounded.is_none() => unbounded = Some(()), + PalletStorageAttr::Unbounded(..) if !unbounded => unbounded = true, attr => return Err(syn::Error::new( attr.attr_span(), @@ -113,7 +113,7 @@ impl PalletStorageAttrInfo { } } - Ok(PalletStorageAttrInfo { getter, rename_as, unbounded: unbounded.is_some() }) + Ok(PalletStorageAttrInfo { getter, rename_as, unbounded }) } }