Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

pallet macro: allow to declare individual unbounded storage for those who cannot go into PoV #9670

Merged
9 commits merged into from
Sep 27, 2021
49 changes: 30 additions & 19 deletions frame/support/procedural/src/pallet/expand/pallet_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<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>
Expand All @@ -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);
}
)*
Expand Down
61 changes: 41 additions & 20 deletions frame/support/procedural/src/pallet/parse/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -36,12 +37,13 @@ mod keyword {
pub enum PalletStorageAttr {
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
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,
}
}
}
Expand Down Expand Up @@ -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::<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 = 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() =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a change in api: before we errored if there was more than 1 getter or name. Now we simply skip anything after the first one.

for example we had

		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))
		}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the UI tests though it seems like it still complains about a duplicate attribute, is this just now an error we can rely on the compiler for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the duplicate attribute is not from the compiler it is from the last matching arm below, in case rename_as is already some, then the last matching are is branched and the error is raised

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() })
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// The value and key types used by storages. Needed to expand metadata.
pub enum Metadata {
Value { value: syn::Type },
Expand Down Expand Up @@ -129,6 +164,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
Expand Down Expand Up @@ -583,25 +620,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);

Expand Down Expand Up @@ -658,6 +678,7 @@ impl StorageDef {
where_clause,
cfg_attrs,
named_generics,
unbounded,
})
}
}
11 changes: 9 additions & 2 deletions frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1398,15 +1398,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.
emostov marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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:
Expand Down Expand Up @@ -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:
Expand Down
22 changes: 22 additions & 0 deletions frame/support/test/tests/pallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ pub mod pallet {
pub type ConditionalNMap<T> =
StorageNMap<_, (storage::Key<Blake2_128Concat, u8>, storage::Key<Twox64Concat, u16>), u32>;

#[pallet::storage]
#[pallet::unbounded]
pub type Unbounded<T> = StorageValue<Value = Vec<u8>>;

#[pallet::genesis_config]
#[derive(Default)]
pub struct GenesisConfig {
Expand Down Expand Up @@ -889,6 +893,10 @@ fn storage_expand() {
pallet::ConditionalDoubleMap::<Runtime>::insert(1, 2, 3);
pallet::ConditionalNMap::<Runtime>::insert((1, 2), 3);
}

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]));
})
}

Expand Down Expand Up @@ -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<u8>".to_string())),
default: DecodeDifferent::Decoded(vec![0]),
documentation: DecodeDifferent::Decoded(vec![]),
},
]),
})),
calls: Some(DecodeDifferent::Decoded(vec![
Expand Down Expand Up @@ -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,
}
],
);

Expand Down
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
Original file line number Diff line number Diff line change
@@ -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"]
Expand Down