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

Adds ability to provide defaults for types provided by construct_runtime #14682

Merged
merged 46 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
bfc3ff7
Adds ability to use defaults for verbatim types
gupnik Jul 30, 2023
18dc0ac
Adds RuntimeOrigin and PalletInfo in DefaultConfig
gupnik Jul 30, 2023
7fb9749
Adds RuntimeEvent in DefaultConfig
gupnik Jul 30, 2023
43fff80
Adds RuntimeEvent in DefaultConfig
gupnik Jul 30, 2023
8e45209
Minor fix
gupnik Jul 30, 2023
9af59d6
Minor fix
gupnik Jul 30, 2023
44f2845
Everything in frame_system can now have a default
gupnik Jul 30, 2023
eed9dc8
Adds docs
gupnik Aug 4, 2023
d2a3546
Adds UI Test for no_bounds
gupnik Aug 4, 2023
821424a
Updates docs
gupnik Aug 4, 2023
5e54173
Adds UI tests for verbatim
gupnik Aug 4, 2023
cd3e530
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 4, 2023
641cc9b
Minor update
gupnik Aug 4, 2023
90af58e
Minor updates
gupnik Aug 4, 2023
c3f5134
Minor updates
gupnik Aug 4, 2023
7f80e65
Addresses review comments
gupnik Aug 9, 2023
38c0601
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 9, 2023
846f296
Fixes test
gupnik Aug 9, 2023
6dfb929
Update frame/support/procedural/src/derive_impl.rs
gupnik Aug 13, 2023
0ddf452
Minor fix
gupnik Aug 13, 2023
3e90665
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 14, 2023
de6e5b6
Minor
gupnik Aug 14, 2023
ce1ad29
Optionally keep verbatim to be replaced later
gupnik Aug 14, 2023
821758b
Fixes build
gupnik Aug 14, 2023
d33f51f
Uses runtime_type
gupnik Aug 15, 2023
96fc62c
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 17, 2023
e051985
Fixes comment
gupnik Aug 17, 2023
16028ff
Fixes comment
gupnik Aug 17, 2023
34f66f6
Fixes test
gupnik Aug 17, 2023
97e470c
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 17, 2023
92ddd1e
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 17, 2023
3524c14
Uses no_aggregated_types as an option in derive_impl
gupnik Aug 21, 2023
1218faf
Uses specific imports
gupnik Aug 21, 2023
01071a9
Fmt
gupnik Aug 21, 2023
a3db9f4
Updates doc
gupnik Aug 21, 2023
28f7cec
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 22, 2023
9d31723
Update frame/support/procedural/src/derive_impl.rs
gupnik Aug 23, 2023
3f1fcc6
Update frame/support/procedural/src/derive_impl.rs
gupnik Aug 23, 2023
58762cf
Addresses review comment
gupnik Aug 23, 2023
cb2a8e0
Addresses review comment
gupnik Aug 23, 2023
32f33ae
fmt
gupnik Aug 23, 2023
88f6253
Renames test files
gupnik Aug 23, 2023
bb30704
Adds docs using docify
gupnik Aug 24, 2023
8f3191f
Fixes test
gupnik Aug 24, 2023
718c1ce
Fixes UI tests
gupnik Aug 24, 2023
969bf0e
Merge branch 'master' of github.com:paritytech/substrate into gupnik/…
gupnik Aug 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,14 @@ pub mod pallet {

pub struct TestDefaultConfig;

#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig, false)]
gupnik marked this conversation as resolved.
Show resolved Hide resolved
impl frame_system::DefaultConfig for TestDefaultConfig {}

#[frame_support::register_default_impl(TestDefaultConfig)]
impl DefaultConfig for TestDefaultConfig {
#[runtime_type]
gupnik marked this conversation as resolved.
Show resolved Hide resolved
type RuntimeEvent = ();

type Balance = u64;

type ReserveIdentifier = ();
Expand All @@ -242,6 +245,7 @@ pub mod pallet {
#[pallet::config(with_default)]
pub trait Config<I: 'static = ()>: frame_system::Config {
/// The overarching event type.
#[pallet::no_default_bounds]
type RuntimeEvent: From<Event<Self, I>>
+ IsType<<Self as frame_system::Config>::RuntimeEvent>;

Expand Down
24 changes: 13 additions & 11 deletions frame/examples/default-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub mod pallet {
/// A type providing default configurations for this pallet in testing environment.
pub struct TestDefaultConfig;

#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig, false)]
impl frame_system::DefaultConfig for TestDefaultConfig {}

#[frame_support::register_default_impl(TestDefaultConfig)]
Expand All @@ -109,7 +109,7 @@ pub mod pallet {
/// example, we simple derive `frame_system::config_preludes::TestDefaultConfig` again.
pub struct OtherDefaultConfig;

#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig, false)]
impl frame_system::DefaultConfig for OtherDefaultConfig {}

#[frame_support::register_default_impl(OtherDefaultConfig)]
Expand Down Expand Up @@ -147,16 +147,7 @@ pub mod tests {
#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
impl frame_system::Config for Runtime {
// these items are defined by frame-system as `no_default`, so we must specify them here.
// Note that these are types that actually rely on the outer runtime, and can't sensibly
// have an _independent_ default.
type Block = Block;
type BlockHashCount = frame_support::traits::ConstU64<10>;
type BaseCallFilter = frame_support::traits::Everything;
type RuntimeOrigin = RuntimeOrigin;
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type PalletInfo = PalletInfo;
type OnSetCode = ();

// all of this is coming from `frame_system::config_preludes::TestDefaultConfig`.

Expand All @@ -177,6 +168,17 @@ pub mod tests {
// type BlockWeights = ();
// type BlockLength = ();
// type DbWeight = ();
// type BaseCallFilter = frame_support::traits::Everything;
// type BlockHashCount = frame_support::traits::ConstU64<10>;
// type OnSetCode = ();

// These are marked as `#[runtime_type]`. Hence, they are being injected as
// types generated by `construct_runtime`.

// type RuntimeOrigin = RuntimeOrigin;
// type RuntimeCall = RuntimeCall;
// type RuntimeEvent = RuntimeEvent;
// type PalletInfo = PalletInfo;

// you could still overwrite any of them if desired.
type SS58Prefix = frame_support::traits::ConstU16<456>;
Expand Down
73 changes: 68 additions & 5 deletions frame/support/procedural/src/derive_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,51 @@ use macro_magic::mm_core::ForeignPath;
use proc_macro2::TokenStream as TokenStream2;
use quote::{quote, ToTokens};
use std::collections::HashSet;
use syn::{parse2, parse_quote, spanned::Spanned, Ident, ImplItem, ItemImpl, Path, Result, Token};
use syn::{
parse2, parse_quote, spanned::Spanned, token, Ident, ImplItem, ItemImpl, LitBool, Path, Result,
Token,
};

#[derive(Parse)]
mod keyword {
syn::custom_keyword!(runtime_type);
}

#[derive(derive_syn_parse::Parse, PartialEq, Eq)]
pub enum PalletAttrType {
#[peek(keyword::runtime_type, name = "runtime_type")]
RuntimeType(keyword::runtime_type),
}

#[derive(derive_syn_parse::Parse)]
pub struct PalletAttr {
_pound: Token![#],
#[bracket]
_bracket: token::Bracket,
#[inside(_bracket)]
typ: PalletAttrType,
}

fn take_first_item_pallet_attr<Attr>(item: &mut syn::ImplItemType) -> syn::Result<Option<Attr>>
gupnik marked this conversation as resolved.
Show resolved Hide resolved
where
Attr: syn::parse::Parse,
{
let attrs = &mut item.attrs;
if let Some(pallet_attr) = attrs.get(0) {
Ok(Some(syn::parse2(pallet_attr.into_token_stream())?))
} else {
Ok(None)
}
gupnik marked this conversation as resolved.
Show resolved Hide resolved
}

#[derive(Parse, Debug)]
pub struct DeriveImplAttrArgs {
pub default_impl_path: Path,
_as: Option<Token![as]>,
#[parse_if(_as.is_some())]
pub disambiguation_path: Option<Path>,
_comma: Option<Token![,]>,
#[parse_if(_comma.is_some())]
pub inject_runtime_types: Option<LitBool>,
}

impl ForeignPath for DeriveImplAttrArgs {
Expand All @@ -43,6 +80,8 @@ impl ToTokens for DeriveImplAttrArgs {
tokens.extend(self.default_impl_path.to_token_stream());
tokens.extend(self._as.to_token_stream());
tokens.extend(self.disambiguation_path.to_token_stream());
tokens.extend(self._comma.to_token_stream());
tokens.extend(self.inject_runtime_types.to_token_stream());
}
}

Expand Down Expand Up @@ -78,6 +117,7 @@ fn combine_impls(
foreign_impl: ItemImpl,
default_impl_path: Path,
disambiguation_path: Path,
inject_runtime_types: bool,
) -> ItemImpl {
let (existing_local_keys, existing_unsupported_items): (HashSet<ImplItem>, HashSet<ImplItem>) =
local_impl
Expand All @@ -96,7 +136,20 @@ fn combine_impls(
// do not copy colliding items that have an ident
return None
}
if matches!(item, ImplItem::Type(_)) {
if let ImplItem::Type(typ) = item.clone() {
let mut typ = typ.clone();
if let Ok(Some(PalletAttr { typ: PalletAttrType::RuntimeType(_), .. })) =
take_first_item_pallet_attr::<PalletAttr>(&mut typ)
{
let item: ImplItem = if inject_runtime_types {
parse_quote! {
type #ident = #ident;
}
} else {
item
};
return Some(item)
}
// modify and insert uncolliding type items
let modified_item: ImplItem = parse_quote! {
type #ident = <#default_impl_path as #disambiguation_path>::#ident;
Expand Down Expand Up @@ -132,6 +185,7 @@ pub fn derive_impl(
foreign_tokens: TokenStream2,
local_tokens: TokenStream2,
disambiguation_path: Option<Path>,
inject_runtime_types: Option<LitBool>,
) -> Result<TokenStream2> {
let local_impl = parse2::<ItemImpl>(local_tokens)?;
let foreign_impl = parse2::<ItemImpl>(foreign_tokens)?;
Expand All @@ -150,9 +204,18 @@ pub fn derive_impl(
)),
};

let inject_runtime_types = match inject_runtime_types {
Some(LitBool { value, .. }) => value,
_ => true,
};
// generate the combined impl
let combined_impl =
combine_impls(local_impl, foreign_impl, default_impl_path, disambiguation_path);
let combined_impl = combine_impls(
local_impl,
foreign_impl,
default_impl_path,
disambiguation_path,
inject_runtime_types,
);

Ok(quote!(#combined_impl))
}
Expand Down
58 changes: 57 additions & 1 deletion frame/support/procedural/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use macro_magic::import_tokens_attr;
use proc_macro::TokenStream;
use quote::{quote, ToTokens};
use std::{cell::RefCell, str::FromStr};
use syn::{parse_macro_input, Error, ItemImpl, ItemMod};
use syn::{parse_macro_input, Error, ItemImpl, ItemMod, TraitItemType};

pub(crate) const INHERENT_INSTANCE_NAME: &str = "__InherentHiddenInstance";

Expand Down Expand Up @@ -596,6 +596,19 @@ pub fn storage_alias(attributes: TokenStream, input: TokenStream) -> TokenStream
///
/// Conversely, the `default_impl_path` argument is required and cannot be omitted.
///
/// Optionally, a boolean can be specified as follows:
///
/// ```ignore
/// #[derive_impl(default_impl_path as disambiguation_path, false)]
/// impl SomeTrait for SomeStruct {
/// ...
/// }
/// ```
///
/// The boolean if specified indicates whether the runtime types (as denoted by impl items
/// attached with [`#[runtime_type]`]) should be injected with the correct concrete types. By
/// default, all such types are injected.
///
/// You can also make use of `#[pallet::no_default]` on specific items in your default impl that you
/// want to ensure will not be copied over but that you nonetheless want to use locally in the
/// context of the foreign impl and the pallet (or context) in which it is defined.
Expand Down Expand Up @@ -759,6 +772,7 @@ pub fn derive_impl(attrs: TokenStream, input: TokenStream) -> TokenStream {
attrs.into(),
input.into(),
custom_attrs.disambiguation_path,
custom_attrs.inject_runtime_types,
)
.unwrap_or_else(|r| r.into_compile_error())
.into()
Expand All @@ -774,6 +788,19 @@ pub fn no_default(_: TokenStream, _: TokenStream) -> TokenStream {
pallet_macro_stub()
}

/// The optional attribute `#[pallet::no_default_bounds]` can be attached to trait items within a
/// `Config` trait impl that has [`#[pallet::config(with_default)]`](`macro@config`) attached.
///
/// Attaching this attribute to a trait item ensures that the generated trait `DefaultConfig`
/// will not have any bounds for this trait item.
///
/// As an example, if you have a trait item `type AccountId: SomeTrait;` in your `Config` trait,
/// the generated `DefaultConfig` will only have `type AccountId;` with no trait bound.
#[proc_macro_attribute]
pub fn no_default_bounds(_: TokenStream, _: TokenStream) -> TokenStream {
pallet_macro_stub()
}

/// Attach this attribute to an impl statement that you want to use with
/// [`#[derive_impl(..)]`](`macro@derive_impl`).
///
Expand Down Expand Up @@ -843,6 +870,35 @@ pub fn register_default_impl(attrs: TokenStream, tokens: TokenStream) -> TokenSt
}
}

/// The optional attribute `#[runtime_type]` can be attached to `RuntimeCall`, `RuntimeEvent`,
/// `RuntimeOrigin` or `PalletInfo` in an impl statement that has `#[register_default_impl]`
/// attached to indicate that this item is generated by `construct_runtime`.
///
/// Attaching this attribute to such an item ensures that the combined impl generated via
/// [`#[derive_impl(..)]`](`macro@derive_impl`) will use the correct type auto-generated by
/// [`construct_runtime!`].
///
/// As an example, if you have an impl item `#[runtime_type] type RuntimeEvent = ();` in
/// your impl statement, the combined impl will have `type RuntimeEvent = RuntimeEvent;` instead.
gupnik marked this conversation as resolved.
Show resolved Hide resolved
#[proc_macro_attribute]
pub fn runtime_type(_: TokenStream, tokens: TokenStream) -> TokenStream {
let item = tokens.clone();
let item = syn::parse_macro_input!(item as TraitItemType);
if item.ident != "RuntimeCall" &&
item.ident != "RuntimeEvent" &&
item.ident != "RuntimeOrigin" &&
item.ident != "PalletInfo"
{
return syn::Error::new_spanned(
item,
"`#[runtime_type]` can only be attached to `RuntimeCall`, `RuntimeEvent`, `RuntimeOrigin` or `PalletInfo`",
)
.to_compile_error()
.into();
}
tokens
}

/// Used internally to decorate pallet attribute macro stubs when they are erroneously used
/// outside of a pallet module
fn pallet_macro_stub() -> TokenStream {
Expand Down
18 changes: 17 additions & 1 deletion frame/support/procedural/src/pallet/expand/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,23 @@ Consequently, a runtime that wants to include this pallet must implement this tr
// impossible consequently.
match &config.default_sub_trait {
Some(default_sub_trait) if default_sub_trait.items.len() > 0 => {
let trait_items = &default_sub_trait.items;
let trait_items = &default_sub_trait
.items
.iter()
.map(|item| {
if item.1 {
if let syn::TraitItem::Type(item) = item.0.clone() {
let mut item = item.clone();
item.bounds.clear();
syn::TraitItem::Type(item)
} else {
item.0.clone()
}
} else {
item.0.clone()
}
})
.collect::<Vec<_>>();

let type_param_bounds = if default_sub_trait.has_system {
let system = &def.frame_system;
Expand Down
29 changes: 26 additions & 3 deletions frame/support/procedural/src/pallet/parse/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,16 @@ mod keyword {
syn::custom_keyword!(frame_system);
syn::custom_keyword!(disable_frame_system_supertrait_check);
syn::custom_keyword!(no_default);
syn::custom_keyword!(no_default_bounds);
syn::custom_keyword!(constant);
}

#[derive(Default)]
pub struct DefaultTrait {
pub items: Vec<syn::TraitItem>,
/// A bool for each sub-trait item indicates whether the item has
/// `#[pallet::no_default_bounds]` attached to it. If true, the item will not have any bounds
/// in the generated default sub-trait.
pub items: Vec<(syn::TraitItem, bool)>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use an enum instead of a boolean in here?

Copy link
Contributor Author

@gupnik gupnik Aug 21, 2023

Choose a reason for hiding this comment

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

I am not sure if it adds a lot of value here. What are the enum variants that you would suggest?

pub has_system: bool,
}

Expand Down Expand Up @@ -142,6 +146,8 @@ impl syn::parse::Parse for DisableFrameSystemSupertraitCheck {
pub enum PalletAttrType {
#[peek(keyword::no_default, name = "no_default")]
NoDefault(keyword::no_default),
#[peek(keyword::no_default_bounds, name = "no_default_bounds")]
NoBounds(keyword::no_default_bounds),
#[peek(keyword::constant, name = "constant")]
Constant(keyword::constant),
}
Expand Down Expand Up @@ -366,6 +372,7 @@ impl ConfigDef {

let mut already_no_default = false;
let mut already_constant = false;
let mut already_no_default_bounds = false;

while let Ok(Some(pallet_attr)) =
helper::take_first_item_pallet_attr::<PalletAttr>(trait_item)
Expand Down Expand Up @@ -403,15 +410,31 @@ impl ConfigDef {

already_no_default = true;
},
(PalletAttrType::NoBounds(_), _) => {
if !enable_default {
return Err(syn::Error::new(
pallet_attr._bracket.span.join(),
"`#[pallet:no_default_bounds]` can only be used if `#[pallet::config(with_default)]` \
has been specified"
))
}
if already_no_default_bounds {
return Err(syn::Error::new(
pallet_attr._bracket.span.join(),
"Duplicate #[pallet::no_default_bounds] attribute not allowed.",
))
}
already_no_default_bounds = true;
},
}
}

if !already_no_default && !is_event && enable_default {
if !already_no_default && enable_default {
default_sub_trait
.as_mut()
.expect("is 'Some(_)' if 'enable_default'; qed")
.items
.push(trait_item.clone());
.push((trait_item.clone(), already_no_default_bounds));
}
}

Expand Down
5 changes: 3 additions & 2 deletions frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2180,8 +2180,9 @@ pub mod pallet_macros {
call_index, compact, composite_enum, config, constant,
disable_frame_system_supertrait_check, error, event, extra_constants, generate_deposit,
generate_store, genesis_build, genesis_config, getter, hooks, import_section, inherent,
no_default, origin, pallet_section, storage, storage_prefix, storage_version, type_value,
unbounded, validate_unsigned, weight, whitelist_storage,
no_default, no_default_bounds, origin, pallet_section, runtime_type, storage,
storage_prefix, storage_version, type_value, unbounded, validate_unsigned, weight,
whitelist_storage,
};
}

Expand Down
Loading