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<I: 'static = ()> {
		SomeHoldReason
	}
```

### Todo

- [x]  UI Test

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.

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

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

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

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

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;

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

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<I>,
+				)
+			});
+		}
+
 		let composite_keyword =
 			syn::parse2::<keyword::CompositeKeyword>(item.ident.to_token_stream())?;
 

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<T>(PhantomData<T>);
 ///
+///     // 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<T: Config> ValidateUnsigned for Pallet<T> {
@@ -1949,6 +1957,11 @@ pub mod pallet_prelude {
 /// 	#[pallet::origin]
 /// 	pub struct Origin<T, I = ()>(PhantomData<(T, I)>);
 ///
+///     #[pallet::composite_enum]
+/// 	pub enum HoldReason<I: 'static = ()> {
+/// 		SomeHoldReason
+/// 	}
+///
 /// 	#[pallet::validate_unsigned]
 /// 	impl<T: Config<I>, I: 'static> ValidateUnsigned for Pallet<T, I> {
 /// 		type Call = Call<T, I>;

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

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<T, I = ()>(PhantomData<(T, I)>);
+
+	// Your Pallet's configuration trait, representing custom external types and interfaces.
+	#[pallet::config]
+	pub trait Config<I: 'static = ()>: frame_system::Config {}
+	
+	#[pallet::composite_enum]
+	pub enum HoldReason<I: 'static = ()> {
+		SomeHoldReason
+	}
+
+	#[pallet::composite_enum]
+	pub enum FreezeReason<I: 'static = ()> {
+		SomeFreezeReason
+	}
+
+	#[pallet::composite_enum]
+	pub enum SlashReason<I: 'static = ()> {
+		SomeSlashReason
+	}
+
+	#[pallet::composite_enum]
+	pub enum LockId<I: 'static = ()> {
+		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<u64, sp_runtime::traits::BlakeTwo256>;
+pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic<u64, RuntimeCall, (), ()>;
+pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
+
+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::<Instance2>,
+	}
+);
+
+impl pallet::Config for Runtime {}
+
+impl pallet::Config<pallet::Instance2> for Runtime {}
+
+fn main() {}