From 1ebcbe1c346adecc02bd2e408b3561f03ea476b8 Mon Sep 17 00:00:00 2001
From: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Date: Mon, 27 Sep 2021 10:25:24 +0200
Subject: [PATCH] pallet macro: allow to declare individual unbounded storage
for those who cannot go into PoV (#9670)
* allow unbounded individual storage
* better doc
* fix UI tests
* update doc
* Update frame/support/procedural/src/pallet/parse/storage.rs
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
---
.../src/pallet/expand/pallet_struct.rs | 49 +++++++++------
.../procedural/src/pallet/parse/storage.rs | 62 +++++++++++++------
substrate/frame/support/src/lib.rs | 11 +++-
substrate/frame/support/test/tests/pallet.rs | 22 +++++++
.../storage_invalid_attribute.stderr | 2 +-
.../pallet_ui/storage_multiple_getters.stderr | 2 +-
.../pallet_ui/storage_multiple_renames.stderr | 2 +-
7 files changed, 106 insertions(+), 44 deletions(-)
diff --git a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs
index a217742fec5..0ad8a25b8e9 100644
--- a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs
+++ b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs
@@ -98,28 +98,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::<Vec<_>>();
let storage_cfg_attrs =
&def.storages.iter().map(|storage| &storage.cfg_attrs).collect::<Vec<_>>();
+ // 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::<Vec<_>>();
+
+ 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::<Vec<_>>();
+
let storage_info = quote::quote_spanned!(storage_info_span =>
impl<#type_impl_gen> #frame_support::traits::StorageInfoTrait
for #pallet_ident<#type_use_gen>
@@ -136,8 +147,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/substrate/frame/support/procedural/src/pallet/parse/storage.rs b/substrate/frame/support/procedural/src/pallet/parse/storage.rs
index 8075daacb6f..cd29baf93d8 100644
--- a/substrate/frame/support/procedural/src/pallet/parse/storage.rs
+++ b/substrate/frame/support/procedural/src/pallet/parse/storage.rs
@@ -27,6 +27,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);
}
@@ -34,15 +35,17 @@ 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),
+ 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,
}
}
}
@@ -76,12 +79,45 @@ impl syn::parse::Parse for PalletStorageAttr {
})?;
Ok(Self::StorageName(renamed_prefix, attr_span))
+ } else if lookahead.peek(keyword::unbounded) {
+ content.parse::<keyword::unbounded>()?;
+
+ Ok(Self::Unbounded(attr_span))
} else {
Err(lookahead.error())
}
}
}
+struct PalletStorageAttrInfo {
+ getter: Option<syn::Ident>,
+ rename_as: Option<syn::LitStr>,
+ unbounded: bool,
+}
+
+impl PalletStorageAttrInfo {
+ fn from_attrs(attrs: Vec<PalletStorageAttr>) -> syn::Result<Self> {
+ let mut getter = None;
+ let mut rename_as = 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 => unbounded = true,
+ attr =>
+ return Err(syn::Error::new(
+ attr.attr_span(),
+ "Invalid attribute: Duplicate attribute",
+ )),
+ }
+ }
+
+ Ok(PalletStorageAttrInfo { getter, rename_as, unbounded })
+ }
+}
+
/// The value and key types used by storages. Needed to expand metadata.
pub enum Metadata {
Value { value: syn::Type },
@@ -131,6 +167,8 @@ pub struct StorageDef {
/// generics of the storage.
/// If generics are not named, this is none.
pub named_generics: Option<StorageGenerics>,
+ /// If the value stored in this storage is unbounded.
+ pub unbounded: bool,
}
/// The parsed generic from the
@@ -629,25 +667,8 @@ impl StorageDef {
};
let attrs: Vec<PalletStorageAttr> = helper::take_item_pallet_attrs(&mut item.attrs)?;
- let (mut getters, mut names) = attrs
- .into_iter()
- .partition::<Vec<_>, _>(|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);
@@ -704,6 +725,7 @@ impl StorageDef {
where_clause,
cfg_attrs,
named_generics,
+ unbounded,
})
}
}
diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs
index 45969870736..f56af036eb2 100644
--- a/substrate/frame/support/src/lib.rs
+++ b/substrate/frame/support/src/lib.rs
@@ -1411,15 +1411,17 @@ pub mod pallet_prelude {
/// `<Pallet as Store>::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<T>(_);
/// ```
///
/// 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,11 @@ pub mod pallet_prelude {
/// pub(super) type MyStorage<T> = StorageMap<_, Blake2_128Concat, u32, u32>;
/// ```
///
+/// The optional attribute `#[pallet::unbounded]` allows to declare the storage as unbounded.
+/// 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.
///
/// E.g:
diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs
index 7eb8431ce14..25fc2d46d25 100644
--- a/substrate/frame/support/test/tests/pallet.rs
+++ b/substrate/frame/support/test/tests/pallet.rs
@@ -329,6 +329,10 @@ pub mod pallet {
pub type SomeCountedStorageMap<T> =
CountedStorageMap<Hasher = Twox64Concat, Key = u8, Value = u32>;
+ #[pallet::storage]
+ #[pallet::unbounded]
+ pub type Unbounded<T> = StorageValue<Value = Vec<u8>>;
+
#[pallet::genesis_config]
#[derive(Default)]
pub struct GenesisConfig {
@@ -917,6 +921,10 @@ fn storage_expand() {
assert_eq!(unhashed::get::<u32>(&k), Some(2u32));
let k = [twox_128(b"Example"), twox_128(b"CounterForRenamedCountedMap")].concat();
assert_eq!(unhashed::get::<u32>(&k), Some(1u32));
+
+ pallet::Unbounded::<Runtime>::put(vec![1, 2]);
+ let k = [twox_128(b"Example"), twox_128(b"Unbounded")].concat();
+ assert_eq!(unhashed::get::<Vec<u8>>(&k), Some(vec![1, 2]));
})
}
@@ -1170,6 +1178,13 @@ fn metadata() {
default: vec![0, 0, 0, 0],
docs: vec!["Counter for the related counted storage map"],
},
+ StorageEntryMetadata {
+ name: "Unbounded",
+ modifier: StorageEntryModifier::Optional,
+ ty: StorageEntryType::Plain(meta_type::<Vec<u8>>()),
+ default: vec![0],
+ docs: vec![],
+ },
],
}),
calls: Some(meta_type::<pallet::Call<Runtime>>().into()),
@@ -1411,6 +1426,13 @@ fn test_storage_info() {
max_values: Some(1),
max_size: Some(4),
},
+ 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,
+ },
],
);
diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr
index bf93d99cf56..6313bd691f9 100644
--- a/substrate/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr
+++ b/substrate/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/substrate/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr
index 188eed3cb0d..40f57f16e0d 100644
--- a/substrate/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr
+++ b/substrate/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/substrate/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr
index 9288d131d95..52cb7e85adf 100644
--- a/substrate/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr
+++ b/substrate/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"]
--
GitLab