Remove generation of instance trait by decl_storage. (paritytech#6812)

* remove generation of instance trait, no breaking change

* doc

* doc

* Update frame/support/src/traits.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update frame/support/procedural/src/storage/instance_trait.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

frame/support/procedural/src/storage/genesis_config/mod.rs

@@ -20,7 +20,7 @@
 
 use proc_macro2::{TokenStream, Span};
 use quote::quote;
-use super::{DeclStorageDefExt, instance_trait::DEFAULT_INSTANTIABLE_TRAIT_NAME};
+use super::DeclStorageDefExt;
 use genesis_config_def::GenesisConfigDef;
 use builder_def::BuilderDef;
 
@@ -104,10 +104,9 @@ fn impl_build_storage(
 		let name = syn::Ident::new(DEFAULT_INSTANCE_NAME, Span::call_site());
 		quote!( #name )
 	});
-	let inherent_instance_bound = def.optional_instance_bound.clone().unwrap_or_else(|| {
-		let bound = syn::Ident::new(DEFAULT_INSTANTIABLE_TRAIT_NAME, Span::call_site());
-		quote!( #inherent_instance: #bound )
-	});
+	let inherent_instance_bound = quote!(
+		#inherent_instance: #scrate::traits::Instance
+	);
 
 	let build_storage_impl = quote!(
 		<#runtime_generic: #runtime_trait, #inherent_instance_bound>

frame/support/procedural/src/storage/instance_trait.rs

@@ -24,7 +24,6 @@ use super::DeclStorageDefExt;
 
 const NUMBER_OF_INSTANCE: usize = 16;
 pub(crate) const INHERENT_INSTANCE_NAME: &str = "__InherentHiddenInstance";
-pub(crate) const DEFAULT_INSTANTIABLE_TRAIT_NAME: &str = "__GeneratedInstantiable";
 
 // Used to generate an instance implementation.
 struct InstanceDef {
@@ -36,7 +35,7 @@ struct InstanceDef {
 pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStream {
 	let mut impls = TokenStream::new();
 
-	impls.extend(create_instance_trait(def));
+	impls.extend(reexport_instance_trait(scrate, def));
 
 	// Implementation of instances.
 	if let Some(module_instance) = &def.module_instance {
@@ -70,42 +69,40 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre
 		.and_then(|i| i.instance_default.as_ref())
 	{
 		impls.extend(quote! {
+			/// Hidden instance generated to be internally used when module is used without
+			/// instance.
 			#[doc(hidden)]
 			pub type #inherent_instance = #default_instance;
 		});
 	} else {
 		let instance_def = InstanceDef {
 			prefix: String::new(),
 			instance_struct: inherent_instance,
-			doc: quote!(#[doc(hidden)]),
+			doc: quote!(
+				/// Hidden instance generated to be internally used when module is used without
+				/// instance.
+				#[doc(hidden)]
+			),
 		};
 		impls.extend(create_and_impl_instance_struct(scrate, &instance_def, def));
 	}
 
 	impls
 }
 
-fn create_instance_trait(
+fn reexport_instance_trait(
+	scrate: &TokenStream,
 	def: &DeclStorageDefExt,
 ) -> TokenStream {
-	let instance_trait = def.module_instance.as_ref().map(|i| i.instance_trait.clone())
-		.unwrap_or_else(|| syn::Ident::new(DEFAULT_INSTANTIABLE_TRAIT_NAME, Span::call_site()));
-
-	let optional_hide = if def.module_instance.is_some() {
-		quote!()
+	if let Some(i) = def.module_instance.as_ref() {
+		let instance_trait = &i.instance_trait;
+		quote!(
+			/// Local import of frame_support::traits::Instance
+			// This import is not strictly needed but made in order not to have breaking change.
+			use #scrate::traits::Instance as #instance_trait;
+		)
 	} else {
-		quote!(#[doc(hidden)])
-	};
-
-	quote! {
-		/// Tag a type as an instance of a module.
-		///
-		/// Defines storage prefixes, they must be unique.
-		#optional_hide
-		pub trait #instance_trait: 'static {
-			/// The prefix used by any storage entry of an instance.
-			const PREFIX: &'static str;
-		}
+		quote!()
 	}
 }
 
@@ -114,8 +111,7 @@ fn create_and_impl_instance_struct(
 	instance_def: &InstanceDef,
 	def: &DeclStorageDefExt,
 ) -> TokenStream {
-	let instance_trait = def.module_instance.as_ref().map(|i| i.instance_trait.clone())
-		.unwrap_or_else(|| syn::Ident::new(DEFAULT_INSTANTIABLE_TRAIT_NAME, Span::call_site()));
+	let instance_trait = quote!( #scrate::traits::Instance );
 
 	let instance_struct = &instance_def.instance_struct;
 	let prefix = format!("{}{}", instance_def.prefix, def.crate_name.to_string());

frame/support/procedural/src/storage/parse.rs

@@ -324,7 +324,16 @@ fn get_module_instance(
 	instantiable: Option<syn::Ident>,
 	default_instance: Option<syn::Ident>,
 ) -> syn::Result<Option<super::ModuleInstanceDef>> {
-	let right_syntax = "Should be $Instance: $Instantiable = $DefaultInstance";
+	let right_syntax = "Should be $I: $Instance = $DefaultInstance";
+
+	if instantiable.as_ref().map_or(false, |i| i != "Instance") {
+		let msg = format!(
+			"Instance trait must be named `Instance`, other names are no longer supported, because \
+			it is now defined at frame_support::traits::Instance. Expect `Instance` found `{}`",
+			instantiable.as_ref().unwrap(),
+		);
+		return Err(syn::Error::new(instantiable.span(), msg));
+	}
 
 	match (instance, instantiable, default_instance) {
 		(Some(instance), Some(instantiable), default_instance) => {

frame/support/procedural/src/storage/storage_struct.rs

@@ -106,7 +106,7 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre
 						type Query = #query_type;
 
 						fn module_prefix() -> &'static [u8] {
-							#instance_or_inherent::PREFIX.as_bytes()
+							<#instance_or_inherent as #scrate::traits::Instance>::PREFIX.as_bytes()
 						}
 
 						fn storage_prefix() -> &'static [u8] {
@@ -130,7 +130,7 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre
 						for #storage_struct #optional_storage_where_clause
 					{
 						fn module_prefix() -> &'static [u8] {
-							#instance_or_inherent::PREFIX.as_bytes()
+							<#instance_or_inherent as #scrate::traits::Instance>::PREFIX.as_bytes()
 						}
 
 						fn storage_prefix() -> &'static [u8] {
@@ -145,7 +145,7 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre
 						type Hasher = #scrate::#hasher;
 
 						fn module_prefix() -> &'static [u8] {
-							#instance_or_inherent::PREFIX.as_bytes()
+							<#instance_or_inherent as #scrate::traits::Instance>::PREFIX.as_bytes()
 						}
 
 						fn storage_prefix() -> &'static [u8] {
@@ -170,7 +170,7 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre
 						for #storage_struct #optional_storage_where_clause
 					{
 						fn module_prefix() -> &'static [u8] {
-							#instance_or_inherent::PREFIX.as_bytes()
+							<#instance_or_inherent as #scrate::traits::Instance>::PREFIX.as_bytes()
 						}
 
 						fn storage_prefix() -> &'static [u8] {
@@ -188,7 +188,7 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre
 						type Hasher2 = #scrate::#hasher2;
 
 						fn module_prefix() -> &'static [u8] {
-							#instance_or_inherent::PREFIX.as_bytes()
+							<#instance_or_inherent as #scrate::traits::Instance>::PREFIX.as_bytes()
 						}
 
 						fn storage_prefix() -> &'static [u8] {

frame/support/src/traits.rs

@@ -1653,6 +1653,17 @@ impl<T> IsType<T> for T {
 	fn into_mut(&mut self) -> &mut T { self }
 }
 
+/// An instance of a pallet in the storage.
+///
+/// It is required that these instances are unique, to support multiple instances per pallet in the same runtime!
+///
+/// E.g. for module MyModule default instance will have prefix "MyModule" and other instances
+/// "InstanceNMyModule".
+pub trait Instance: 'static {
+    /// Unique module prefix. E.g. "InstanceNMyModule" or "MyModule"
+    const PREFIX: &'static str ;
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

frame/support/test/tests/final_keys.rs

@@ -53,12 +53,12 @@ mod instance {
 	pub trait Trait<I = DefaultInstance>: super::no_instance::Trait {}
 
 	frame_support::decl_module! {
-		pub struct Module<T: Trait<I>, I: Instantiable = DefaultInstance>
+		pub struct Module<T: Trait<I>, I: Instance = DefaultInstance>
 			for enum Call where origin: T::Origin {}
 	}
 
 	frame_support::decl_storage!{
-		trait Store for Module<T: Trait<I>, I: Instantiable = DefaultInstance>
+		trait Store for Module<T: Trait<I>, I: Instance = DefaultInstance>
 			as FinalKeysSome
 		{
 			pub Value config(value): u32;

frame/support/test/tests/instance.rs

@@ -36,7 +36,6 @@ pub trait Currency {}
 
 // Test for:
 // * No default instance
-// * Custom InstantiableTrait
 // * Origin, Inherent, Event
 mod module1 {
 	use super::*;
@@ -49,7 +48,7 @@ mod module1 {
 	}
 
 	frame_support::decl_module! {
-		pub struct Module<T: Trait<I>, I: InstantiableThing> for enum Call where
+		pub struct Module<T: Trait<I>, I: Instance> for enum Call where
 			origin: <T as system::Trait>::Origin,
 			system = system,
 			T::BlockNumber: From<u32>
@@ -67,7 +66,7 @@ mod module1 {
 	}
 
 	frame_support::decl_storage! {
-		trait Store for Module<T: Trait<I>, I: InstantiableThing> as Module1 where
+		trait Store for Module<T: Trait<I>, I: Instance> as Module1 where
 			T::BlockNumber: From<u32> + std::fmt::Display
 		{
 			pub Value config(value): T::GenericType;
@@ -97,7 +96,7 @@ mod module1 {
 
 	pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"12345678";
 
-	impl<T: Trait<I>, I: InstantiableThing> ProvideInherent for Module<T, I> where
+	impl<T: Trait<I>, I: Instance> ProvideInherent for Module<T, I> where
 		T::BlockNumber: From<u32>
 	{
 		type Call = Call<T, I>;