From 8b127b122d34cd303a9aa74f668c7281695234d3 Mon Sep 17 00:00:00 2001 From: gupnik <17176722+gupnik@users.noreply.github.com> Date: Fri, 13 Oct 2023 07:48:51 +0200 Subject: [PATCH] Adds instance support for composite enums (#1857) Fixes https://github.com/paritytech/polkadot-sdk/issues/1839 Currently, `composite_enum`s do not support pallet instances. This PR allows the following: ```rust #[pallet::composite_enum] pub enum HoldReason { SomeHoldReason } ``` ### Todo - [x] UI Test --- substrate/frame/balances/src/lib.rs | 4 +- .../expand/composite_helper.rs | 70 ++++++++++++++++ .../construct_runtime/expand/freeze_reason.rs | 40 +++++----- .../construct_runtime/expand/hold_reason.rs | 40 +++++----- .../src/construct_runtime/expand/lock_id.rs | 40 +++++----- .../src/construct_runtime/expand/mod.rs | 1 + .../construct_runtime/expand/slash_reason.rs | 40 +++++----- .../procedural/src/pallet/parse/composite.rs | 24 +++++- substrate/frame/support/src/lib.rs | 17 +++- .../test/tests/construct_runtime_ui.rs | 1 + .../pass/composite_enum_instance.rs | 80 +++++++++++++++++++ 11 files changed, 264 insertions(+), 93 deletions(-) create mode 100644 substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs create mode 100644 substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index a2cacc45369a2..c408bf3f35f3b 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/substrate/frame/balances/src/lib.rs @@ -256,7 +256,7 @@ pub mod pallet { /// The overarching hold reason. #[pallet::no_default_bounds] - type RuntimeHoldReason: Parameter + Member + MaxEncodedLen + Ord + Copy; + type RuntimeHoldReason: Parameter + Member + MaxEncodedLen + Copy; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; @@ -300,7 +300,7 @@ pub mod pallet { type ReserveIdentifier: Parameter + Member + MaxEncodedLen + Ord + Copy; /// The ID type for freezes. - type FreezeIdentifier: Parameter + Member + MaxEncodedLen + Ord + Copy; + type FreezeIdentifier: Parameter + Member + MaxEncodedLen + Copy; /// The maximum number of locks that should exist on an account. /// Not strictly enforced, but used for weight estimation. diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs new file mode 100644 index 0000000000000..3c81d2360cb7b --- /dev/null +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs @@ -0,0 +1,70 @@ +// This file is part of Substrate. + +// Copyright (C) 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 + +use crate::construct_runtime::parse::PalletPath; +use proc_macro2::{Ident, TokenStream}; +use quote::quote; + +pub(crate) fn expand_conversion_fn( + composite_name: &str, + path: &PalletPath, + instance: Option<&Ident>, + variant_name: &Ident, +) -> TokenStream { + let composite_name = quote::format_ident!("{}", composite_name); + let runtime_composite_name = quote::format_ident!("Runtime{}", composite_name); + + if let Some(inst) = instance { + quote! { + impl From<#path::#composite_name<#path::#inst>> for #runtime_composite_name { + fn from(hr: #path::#composite_name<#path::#inst>) -> Self { + #runtime_composite_name::#variant_name(hr) + } + } + } + } else { + quote! { + impl From<#path::#composite_name> for #runtime_composite_name { + fn from(hr: #path::#composite_name) -> Self { + #runtime_composite_name::#variant_name(hr) + } + } + } + } +} + +pub(crate) fn expand_variant( + composite_name: &str, + index: u8, + path: &PalletPath, + instance: Option<&Ident>, + variant_name: &Ident, +) -> TokenStream { + let composite_name = quote::format_ident!("{}", composite_name); + + if let Some(inst) = instance { + quote! { + #[codec(index = #index)] + #variant_name(#path::#composite_name<#path::#inst>), + } + } else { + quote! { + #[codec(index = #index)] + #variant_name(#path::#composite_name), + } + } +} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs index b142f8e84c92f..18790850d6b3c 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "FreezeReason", + path, + instance, + variant_name, + )); - freeze_reason_variants.push(expand_variant(index, path, variant_name)); + freeze_reason_variants.push(composite_helper::expand_variant( + "FreezeReason", + index, + path, + instance, + variant_name, + )); } } quote! { /// A reason for placing a freeze on funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::FreezeReason> for RuntimeFreezeReason { - fn from(hr: #path::FreezeReason) -> Self { - RuntimeFreezeReason::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::FreezeReason), - } -} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs index ed7183c4a150d..1bb7462133cdd 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) - let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "HoldReason", + path, + instance, + variant_name, + )); - hold_reason_variants.push(expand_variant(index, path, variant_name)); + hold_reason_variants.push(composite_helper::expand_variant( + "HoldReason", + index, + path, + instance, + variant_name, + )); } } quote! { /// A reason for placing a hold on funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) - #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::HoldReason> for RuntimeHoldReason { - fn from(hr: #path::HoldReason) -> Self { - RuntimeHoldReason::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::HoldReason), - } -} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs index ba35147a051fb..e67c0da00ea15 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_lock_id(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_lock_id(pallet_decls: &[Pallet], scrate: &TokenStream) -> To let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "LockId", + path, + instance, + variant_name, + )); - lock_id_variants.push(expand_variant(index, path, variant_name)); + lock_id_variants.push(composite_helper::expand_variant( + "LockId", + index, + path, + instance, + variant_name, + )); } } quote! { /// An identifier for each lock placed on funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_lock_id(pallet_decls: &[Pallet], scrate: &TokenStream) -> To #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::LockId> for RuntimeLockId { - fn from(hr: #path::LockId) -> Self { - RuntimeLockId::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::LockId), - } -} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs index 830338f9265ff..a0fc6b8130b3c 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs @@ -16,6 +16,7 @@ // limitations under the License mod call; +pub mod composite_helper; mod config; mod freeze_reason; mod hold_reason; diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs index 2a3283230ad84..892b842b17487 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_slash_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_slash_reason(pallet_decls: &[Pallet], scrate: &TokenStream) let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "SlashReason", + path, + instance, + variant_name, + )); - slash_reason_variants.push(expand_variant(index, path, variant_name)); + slash_reason_variants.push(composite_helper::expand_variant( + "SlashReason", + index, + path, + instance, + variant_name, + )); } } quote! { /// A reason for slashing funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_slash_reason(pallet_decls: &[Pallet], scrate: &TokenStream) #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::SlashReason> for RuntimeSlashReason { - fn from(hr: #path::SlashReason) -> Self { - RuntimeSlashReason::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::SlashReason), - } -} diff --git a/substrate/frame/support/procedural/src/pallet/parse/composite.rs b/substrate/frame/support/procedural/src/pallet/parse/composite.rs index cb554a116175c..ddcc2d07f93b3 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/composite.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/composite.rs @@ -15,6 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::helper; use quote::ToTokens; use syn::spanned::Spanned; @@ -108,6 +109,13 @@ impl CompositeDef { return Err(syn::Error::new(item.span(), msg)) } + let has_instance = if item.generics.params.first().is_some() { + helper::check_config_def_gen(&item.generics, item.ident.span())?; + true + } else { + false + }; + let has_derive_attr = item.attrs.iter().any(|attr| { if let syn::Meta::List(syn::MetaList { path, .. }) = &attr.meta { path.get_ident().map(|ident| ident == "derive").unwrap_or(false) @@ -119,7 +127,7 @@ impl CompositeDef { if !has_derive_attr { let derive_attr: syn::Attribute = syn::parse_quote! { #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -128,6 +136,20 @@ impl CompositeDef { item.attrs.push(derive_attr); } + if has_instance { + item.attrs.push(syn::parse_quote! { + #[scale_info(skip_type_params(I))] + }); + + item.variants.push(syn::parse_quote! { + #[doc(hidden)] + #[codec(skip)] + __Ignore( + #scrate::__private::sp_std::marker::PhantomData, + ) + }); + } + let composite_keyword = syn::parse2::(item.ident.to_token_stream())?; diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 8c4b3de49b5d5..700d777a14818 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -1646,8 +1646,7 @@ pub mod pallet_prelude { /// the enum: /// /// ```ignore -/// Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, MaxEncodedLen, TypeInfo, -/// RuntimeDebug +/// Copy, Clone, Eq, PartialEq, Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebug /// ``` /// /// The inverse is also true: if there are any #[derive] attributes present for the enum, then @@ -1815,6 +1814,15 @@ pub mod pallet_prelude { /// #[pallet::origin] /// pub struct Origin(PhantomData); /// +/// // Declare a hold reason (this is optional). +/// // +/// // Creates a hold reason for this pallet that is aggregated by `construct_runtime`. +/// // A similar enum can be defined for `FreezeReason`, `LockId` or `SlashReason`. +/// #[pallet::composite_enum] +/// pub enum HoldReason { +/// SomeHoldReason +/// } +/// /// // Declare validate_unsigned implementation (this is optional). /// #[pallet::validate_unsigned] /// impl ValidateUnsigned for Pallet { @@ -1949,6 +1957,11 @@ pub mod pallet_prelude { /// #[pallet::origin] /// pub struct Origin(PhantomData<(T, I)>); /// +/// #[pallet::composite_enum] +/// pub enum HoldReason { +/// SomeHoldReason +/// } +/// /// #[pallet::validate_unsigned] /// impl, I: 'static> ValidateUnsigned for Pallet { /// type Call = Call; diff --git a/substrate/frame/support/test/tests/construct_runtime_ui.rs b/substrate/frame/support/test/tests/construct_runtime_ui.rs index c3197c99a72bf..0cf857e2d73c0 100644 --- a/substrate/frame/support/test/tests/construct_runtime_ui.rs +++ b/substrate/frame/support/test/tests/construct_runtime_ui.rs @@ -32,4 +32,5 @@ fn ui() { let t = trybuild::TestCases::new(); t.compile_fail("tests/construct_runtime_ui/*.rs"); + t.pass("tests/construct_runtime_ui/pass/*.rs"); } diff --git a/substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs b/substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs new file mode 100644 index 0000000000000..ad63708747694 --- /dev/null +++ b/substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs @@ -0,0 +1,80 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +#![cfg_attr(not(feature = "std"), no_std)] + +use frame_support::derive_impl; + +pub use pallet::*; + +#[frame_support::pallet(dev_mode)] +pub mod pallet { + use frame_support::pallet_prelude::*; + + // The struct on which we build all of our Pallet logic. + #[pallet::pallet] + pub struct Pallet(PhantomData<(T, I)>); + + // Your Pallet's configuration trait, representing custom external types and interfaces. + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::composite_enum] + pub enum HoldReason { + SomeHoldReason + } + + #[pallet::composite_enum] + pub enum FreezeReason { + SomeFreezeReason + } + + #[pallet::composite_enum] + pub enum SlashReason { + SomeSlashReason + } + + #[pallet::composite_enum] + pub enum LockId { + SomeLockId + } +} + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] +impl frame_system::Config for Runtime { + type Block = Block; +} + +pub type Header = sp_runtime::generic::Header; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; +pub type Block = sp_runtime::generic::Block; + +frame_support::construct_runtime!( + pub struct Runtime + { + // Exclude part `Storage` in order not to check its metadata in tests. + System: frame_system, + Pallet1: pallet, + Pallet2: pallet::, + } +); + +impl pallet::Config for Runtime {} + +impl pallet::Config for Runtime {} + +fn main() {}