From 3556df32e12318551d52884d45edaf9f50f4d519 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 26 Aug 2020 19:59:35 +0200 Subject: [PATCH 01/31] implement index for pallet + some tests --- Cargo.lock | 1 + frame/metadata/src/lib.rs | 13 +- .../procedural/src/construct_runtime/mod.rs | 188 ++++++++-- .../procedural/src/construct_runtime/parse.rs | 26 +- frame/support/src/dispatch.rs | 3 +- frame/support/src/event.rs | 36 +- frame/support/src/metadata.rs | 16 +- frame/support/src/origin.rs | 25 +- frame/support/test/Cargo.toml | 1 + frame/support/test/tests/construct_runtime.rs | 348 +++++++++++++++++- frame/support/test/tests/system.rs | 5 +- 11 files changed, 573 insertions(+), 89 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65f5935a1e900..154cf55303a49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1689,6 +1689,7 @@ dependencies = [ name = "frame-support-test" version = "2.0.0-rc6" dependencies = [ + "frame-metadata", "frame-support", "parity-scale-codec", "pretty_assertions", diff --git a/frame/metadata/src/lib.rs b/frame/metadata/src/lib.rs index c0eeb76b6f976..61234cfb32fce 100644 --- a/frame/metadata/src/lib.rs +++ b/frame/metadata/src/lib.rs @@ -362,8 +362,10 @@ pub enum RuntimeMetadata { V9(RuntimeMetadataDeprecated), /// Version 10 for runtime metadata. No longer used. V10(RuntimeMetadataDeprecated), - /// Version 11 for runtime metadata. - V11(RuntimeMetadataV11), + /// Version 11 for runtime metadata. No longer used. + V11(RuntimeMetadataDeprecated), + /// Version 12 for runtime metadata. + V12(RuntimeMetadataV12), } /// Enum that should fail. @@ -387,7 +389,7 @@ impl Decode for RuntimeMetadataDeprecated { /// The metadata of a runtime. #[derive(Eq, Encode, PartialEq, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Decode, Serialize))] -pub struct RuntimeMetadataV11 { +pub struct RuntimeMetadataV12 { /// Metadata of all the modules. pub modules: DecodeDifferentArray, /// Metadata of the extrinsic. @@ -395,7 +397,7 @@ pub struct RuntimeMetadataV11 { } /// The latest version of the metadata. -pub type RuntimeMetadataLastVersion = RuntimeMetadataV11; +pub type RuntimeMetadataLastVersion = RuntimeMetadataV12; /// All metadata about an runtime module. #[derive(Clone, PartialEq, Eq, Encode, RuntimeDebug)] @@ -407,6 +409,7 @@ pub struct ModuleMetadata { pub event: ODFnA, pub constants: DFnA, pub errors: DFnA, + pub fixed_index: Option, } type ODFnA = Option>; @@ -420,6 +423,6 @@ impl Into for RuntimeMetadataPrefixed { impl Into for RuntimeMetadataLastVersion { fn into(self) -> RuntimeMetadataPrefixed { - RuntimeMetadataPrefixed(META_RESERVED, RuntimeMetadata::V11(self)) + RuntimeMetadataPrefixed(META_RESERVED, RuntimeMetadata::V12(self)) } } diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 57827b0673916..3dbfd7f7d7fd4 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -21,7 +21,7 @@ use frame_support_procedural_tools::syn_ext as ext; use frame_support_procedural_tools::{generate_crate_access, generate_hidden_includes}; use parse::{ModuleDeclaration, RuntimeDefinition, WhereSection}; use proc_macro::TokenStream; -use proc_macro2::TokenStream as TokenStream2; +use proc_macro2::{TokenStream as TokenStream2, Span}; use quote::quote; use syn::{Ident, Result, TypePath}; @@ -38,6 +38,7 @@ pub fn construct_runtime(input: TokenStream) -> TokenStream { fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result { let RuntimeDefinition { name, + deprecated_system_non_zero, where_section: WhereSection { block, node_block, @@ -51,40 +52,34 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result>(); + + let system_module = check_modules(deprecated_system_non_zero, &modules, modules_token.span)?; - // Assert we have system module declared - let system_module = match find_system_module(modules.iter()) { - Some(sm) => sm, - None => { - return Err(syn::Error::new( - modules_token.span, - "`System` module declaration is missing. \ - Please add this line: `System: frame_system::{Module, Call, Storage, Config, Event},`", - )) - } - }; let hidden_crate_name = "construct_runtime"; let scrate = generate_crate_access(&hidden_crate_name, "frame-support"); let scrate_decl = generate_hidden_includes(&hidden_crate_name, "frame-support"); - let all_but_system_modules = modules.iter().filter(|module| module.name != SYSTEM_MODULE_NAME); + let all_but_system_modules = modules.clone().into_iter() + .filter(|module| module.name != SYSTEM_MODULE_NAME) + .collect::>(); let outer_event = decl_outer_event( &name, - modules.iter(), + &modules, &scrate, )?; let outer_origin = decl_outer_origin( &name, - all_but_system_modules.clone(), + &all_but_system_modules, &system_module, &scrate, )?; let all_modules = decl_all_modules(&name, modules.iter()); - let module_to_index = decl_module_to_index(modules.iter(), modules.len(), &scrate); + let module_to_index = decl_module_to_index(&modules, &scrate); - let dispatch = decl_outer_dispatch(&name, modules.iter(), &scrate); + let dispatch = decl_outer_dispatch(&name, &modules, &scrate); let metadata = decl_runtime_metadata(&name, modules.iter(), &scrate, &unchecked_extrinsic); let outer_config = decl_outer_config(&name, modules.iter(), &scrate); let inherent = decl_outer_inherent( @@ -238,7 +233,12 @@ fn decl_runtime_metadata<'a>( .as_ref() .map(|name| quote!(<#name>)) .into_iter(); - quote!(#module::Module #(#instance)* as #name with #(#filtered_names)* ,) + + let index = module_declaration.index.map(|index| quote::quote!( { index #index } )); + + quote!( + #module::Module #(#instance)* as #name with #(#filtered_names)* #index, + ) }); quote!( #scrate::impl_runtime_metadata!{ @@ -250,16 +250,23 @@ fn decl_runtime_metadata<'a>( fn decl_outer_dispatch<'a>( runtime: &'a Ident, - module_declarations: impl Iterator, + module_declarations: &Vec, scrate: &'a TokenStream2, ) -> TokenStream2 { - let modules_tokens = module_declarations + let mut available_indices = get_available_indices(module_declarations); + let modules_tokens = module_declarations.iter() .filter(|module_declaration| module_declaration.exists_part("Call")) .map(|module_declaration| { let module = &module_declaration.module; let name = &module_declaration.name; - quote!(#module::#name) + let index = match module_declaration.index { + Some(index) => index, + None => available_indices.remove(0), + }; + let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); + quote!(#[codec(index = #index_string)] #module::#name) }); + quote!( #scrate::impl_outer_dispatch! { pub enum Call for #runtime where origin: Origin { @@ -271,12 +278,14 @@ fn decl_outer_dispatch<'a>( fn decl_outer_origin<'a>( runtime_name: &'a Ident, - module_declarations: impl Iterator, + modules_except_system: &Vec, system_name: &'a Ident, scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); - for module_declaration in module_declarations { + let mut available_indices = get_available_indices(modules_except_system); + available_indices.retain(|i| *i != 0); // index 0 is reserved for system in origin + for module_declaration in modules_except_system { match module_declaration.find_part("Origin") { Some(module_entry) => { let module = &module_declaration.module; @@ -290,7 +299,15 @@ fn decl_outer_origin<'a>( ); return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let tokens = quote!(#module #instance #generics ,); + let index = match module_declaration.index { + Some(index) => { + assert!(index != 0, "Internal error, some origin is at index 0"); + index + }, + None => available_indices.remove(0), + }; + let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); + let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics ,); modules_tokens.extend(tokens); } None => {} @@ -308,10 +325,11 @@ fn decl_outer_origin<'a>( fn decl_outer_event<'a>( runtime_name: &'a Ident, - module_declarations: impl Iterator, + module_declarations: &Vec, scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); + let mut available_indices = get_available_indices(module_declarations); for module_declaration in module_declarations { match module_declaration.find_part("Event") { Some(module_entry) => { @@ -326,7 +344,13 @@ fn decl_outer_event<'a>( ); return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let tokens = quote!(#module #instance #generics ,); + + let index = match module_declaration.index { + Some(index) => index, + None => available_indices.remove(0), + }; + let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); + let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); modules_tokens.extend(tokens); } None => {} @@ -377,12 +401,18 @@ fn decl_all_modules<'a>( } fn decl_module_to_index<'a>( - module_declarations: impl Iterator, - num_modules: usize, + module_declarations: &Vec, scrate: &TokenStream2, ) -> TokenStream2 { - let names = module_declarations.map(|d| &d.name); - let indices = 0..num_modules; + let names = module_declarations.iter().map(|d| &d.name); + let mut available_indices = get_available_indices(module_declarations); + let indices = module_declarations.iter() + .map(|module| { + match module.index { + Some(index) => index as usize, + None => available_indices.remove(0) as usize, + } + }); quote!( /// Provides an implementation of `ModuleToIndex` to map a module @@ -404,14 +434,6 @@ fn decl_module_to_index<'a>( ) } -fn find_system_module<'a>( - mut module_declarations: impl Iterator, -) -> Option<&'a Ident> { - module_declarations - .find(|decl| decl.name == SYSTEM_MODULE_NAME) - .map(|decl| &decl.module) -} - fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { quote!( #[cfg(test)] @@ -425,3 +447,93 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { } ) } + +fn get_available_indices(modules: &Vec) -> Vec { + let reserved_indices = modules.iter() + .filter_map(|module| module.index) + .collect::>(); + + (0..u8::max_value()) + .filter(|i| !reserved_indices.contains(i)) + .collect::>() +} + +/// Check: +/// * check system module is defined +/// * there is less than 256 modules +/// * no module use index 0 or except system +/// * system module index is 0 (either explicitly or implicitly). +/// * module indices don't conflict. +/// +/// returns system module ident +fn check_modules( + deprecated_system_non_zero: bool, + modules: &Vec, + modules_span: proc_macro2::Span, +) -> Result { + // Assert we have system module declared + let system_module = modules.iter() + .find(|decl| decl.name == SYSTEM_MODULE_NAME) + .ok_or_else(|| syn::Error::new( + modules_span, + "`System` module declaration is missing. \ + Please add this line: `System: frame_system::{Module, Call, Storage, Config, Event},`", + ))?; + + if modules.len() > u8::max_value() as usize { + let msg = "Too many modules defined, only 256 modules is currently allowed"; + return Err(syn::Error::new(modules_span, msg)); + } + + // No module use index 0 + if let Some(module) = modules.iter() + .find(|module| { + module.index.map_or(false, |i| i == 0) && module.name != SYSTEM_MODULE_NAME + }) + { + let msg = format!( + "Only system module is allowed to be defined at index `0`", + ); + return Err(syn::Error::new(module.name.span(), msg)); + } + + if !deprecated_system_non_zero { + if let Some(index) = system_module.index { + // System module is at index 0 explicitly + if index != 0 { + let msg = "System module must be defined at index `0`"; + return Err(syn::Error::new(system_module.module.span(), msg)); + } + } else { + // System module is at index 0 implicitly + let first_implicit_module = modules.iter().filter(|m| m.index.is_none()).next() + .expect("At least system module exists; qed"); + + if first_implicit_module.name != SYSTEM_MODULE_NAME { + let msg = format!( + "System module must be defined at index `0`, instead {} is found. (this check \ + is to avoid confusion for generation of origin_caller, (where system is \ + forced to be at index 0 anyway), to bypass this check use attribute \ + `#[deprecated_system_non_zero]` as \ + `construct_runtime!(#[deprecated_system_non_zero] ... )`).", + first_implicit_module.name, + ); + return Err(syn::Error::new(first_implicit_module.module.span(), msg)); + } + } + } + + // No module indices conflicts + let mut indices = vec![]; + for module in modules { + if let Some(index) = module.index { + if indices.contains(&index) { + return Err(syn::Error::new(module.module.span(), "module index is already used")); + } else { + indices.push(index) + } + } + } + + Ok(system_module.module.clone()) +} diff --git a/frame/support/procedural/src/construct_runtime/parse.rs b/frame/support/procedural/src/construct_runtime/parse.rs index c8481480baac5..a1fdcd3a140c7 100644 --- a/frame/support/procedural/src/construct_runtime/parse.rs +++ b/frame/support/procedural/src/construct_runtime/parse.rs @@ -36,6 +36,7 @@ mod keyword { syn::custom_keyword!(Origin); syn::custom_keyword!(Inherent); syn::custom_keyword!(ValidateUnsigned); + syn::custom_keyword!(deprecated_system_non_zero); } #[derive(Debug)] @@ -44,12 +45,24 @@ pub struct RuntimeDefinition { pub enum_token: Token![enum], pub name: Ident, pub where_section: WhereSection, + pub deprecated_system_non_zero: bool, pub modules: ext::Braces>, } impl Parse for RuntimeDefinition { fn parse(input: ParseStream) -> Result { + let mut deprecated_system_non_zero = false; + + if input.peek(Token![#]) { + input.parse::()?; + let attr_content; + syn::bracketed!(attr_content in input); + attr_content.parse::()?; + deprecated_system_non_zero = true; + } + Ok(Self { + deprecated_system_non_zero, visibility_token: input.parse()?, enum_token: input.parse()?, name: input.parse()?, @@ -149,9 +162,10 @@ impl Parse for WhereDefinition { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ModuleDeclaration { pub name: Ident, + pub index: Option, pub module: Ident, pub instance: Option, pub module_parts: Vec, @@ -175,11 +189,21 @@ impl Parse for ModuleDeclaration { let _: Token![::] = input.parse()?; let module_parts = parse_module_parts(input)?; + let index = if input.peek(Token![=]) { + input.parse::()?; + let index = input.parse::()?; + let index = index.base10_parse::()?; + Some(index) + } else { + None + }; + let parsed = Self { name, module, instance, module_parts, + index, }; Ok(parsed) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 442a99effadbc..67f228508efb1 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1899,7 +1899,7 @@ macro_rules! impl_outer_dispatch { $(#[$attr:meta])* pub enum $call_type:ident for $runtime:ident where origin: $origin:ty { $( - $module:ident::$camelcase:ident, + $( #[codec(index = $index:tt)] )? $module:ident::$camelcase:ident, )* } ) => { @@ -1912,6 +1912,7 @@ macro_rules! impl_outer_dispatch { )] pub enum $call_type { $( + $( #[codec(index = $index)] )? $camelcase ( $crate::dispatch::CallableCallFor<$camelcase, $runtime> ) ,)* } diff --git a/frame/support/src/event.rs b/frame/support/src/event.rs index 1184b379f4446..4fd7d5978c34c 100644 --- a/frame/support/src/event.rs +++ b/frame/support/src/event.rs @@ -346,7 +346,7 @@ macro_rules! impl_outer_event { $name; $runtime; Modules { $( $rest_events )* }; - ; + {}; ); }; // Generic + Instance @@ -355,17 +355,17 @@ macro_rules! impl_outer_event { $name:ident; $runtime:ident; Modules { - $module:ident $instance:ident, + $( #[codec(index = $index:tt)] )? $module:ident $instance:ident, $( $rest_event_generic_instance:tt )* }; - $( $module_name:ident::Event $( <$generic_param:ident> )? $( { $generic_instance:ident } )?, )*; + { $( $parsed:tt )* }; ) => { $crate::impl_outer_event!( $( #[$attr] )*; $name; $runtime; Modules { $( $rest_event_generic_instance )* }; - $( $module_name::Event $( <$generic_param> )? $( { $generic_instance } )?, )* $module::Event<$runtime>{ $instance },; + { $( $parsed )* $module::Event<$runtime>{ $instance } index { $( $index )? }, }; ); }; // Instance @@ -374,17 +374,17 @@ macro_rules! impl_outer_event { $name:ident; $runtime:ident; Modules { - $module:ident $instance:ident, + $( #[codec(index = $index:tt)] )? $module:ident $instance:ident, $( $rest_event_instance:tt )* }; - $( $module_name:ident::Event $( <$generic_param:ident> )? $( { $generic_instance:ident } )?, )*; + { $( $parsed:tt )* }; ) => { $crate::impl_outer_event!( $( #[$attr] )*; $name; $runtime; Modules { $( $rest_event_instance )* }; - $( $module_name::Event $( <$generic_param> )* $( { $generic_instance } )?, )* $module::Event { $instance },; + { $( $parsed )* $module::Event { $instance } index { $( $index )? }, }; ); }; // Generic @@ -393,17 +393,17 @@ macro_rules! impl_outer_event { $name:ident; $runtime:ident; Modules { - $module:ident, + $( #[codec(index = $index:tt)] )? $module:ident, $( $rest_event_generic:tt )* }; - $( $module_name:ident::Event $( <$generic_param:ident> )? $( { $generic_instance:ident } )?, )*; + { $( $parsed:tt )* }; ) => { $crate::impl_outer_event!( $( #[$attr] )*; $name; $runtime; Modules { $( $rest_event_generic )* }; - $( $module_name::Event $( <$generic_param> )? $( { $generic_instance } )?, )* $module::Event<$runtime>,; + { $( $parsed )* $module::Event<$runtime> index { $( $index )? }, }; ); }; // No Generic and no Instance @@ -412,17 +412,17 @@ macro_rules! impl_outer_event { $name:ident; $runtime:ident; Modules { - $module:ident, + $( #[codec(index = $index:tt)] )? $module:ident, $( $rest_event_no_generic_no_instance:tt )* }; - $( $module_name:ident::Event $( <$generic_param:ident> )? $( { $generic_instance:ident } )?, )*; + { $( $parsed:tt )* }; ) => { $crate::impl_outer_event!( $( #[$attr] )*; $name; $runtime; Modules { $( $rest_event_no_generic_no_instance )* }; - $( $module_name::Event $( <$generic_param> )? $( { $generic_instance } )?, )* $module::Event,; + { $( $parsed )* $module::Event index { $( $index )? }, }; ); }; @@ -432,7 +432,14 @@ macro_rules! impl_outer_event { $name:ident; $runtime:ident; Modules {}; - $( $module_name:ident::Event $( <$generic_param:ident> )? $( { $generic_instance:ident } )?, )*; + { + $( + $module_name:ident::Event + $( <$generic_param:ident> )? + $( { $generic_instance:ident } )? + index { $( $index:tt )? }, + )* + }; ) => { $crate::paste::item! { #[derive( @@ -445,6 +452,7 @@ macro_rules! impl_outer_event { #[allow(non_camel_case_types)] pub enum $name { $( + $( #[codec(index = $index)] )? [< $module_name $(_ $generic_instance )? >]( $module_name::Event < $( $generic_param )? $(, $module_name::$generic_instance )? > ), diff --git a/frame/support/src/metadata.rs b/frame/support/src/metadata.rs index aa7d71b52e0b6..9e507d2010fe8 100644 --- a/frame/support/src/metadata.rs +++ b/frame/support/src/metadata.rs @@ -53,7 +53,7 @@ pub use frame_metadata::{ /// for Runtime with modules where Extrinsic = UncheckedExtrinsic /// module0::Module as Module0 with, /// module1::Module as Module1 with, -/// module2::Module as Module2 with Storage, +/// module2::Module as Module2 with Storage { index 3 }, /// }; /// ``` /// @@ -91,13 +91,20 @@ macro_rules! __runtime_modules_to_metadata { ( $runtime: ident; $( $metadata:expr ),*; - $mod:ident::$module:ident $( < $instance:ident > )? as $name:ident $(with)+ $($kw:ident)*, + $mod:ident::$module:ident $( < $instance:ident > )? as $name:ident + $(with)+ $($kw:ident)* + $( { index $index:tt } )? + , $( $rest:tt )* ) => { $crate::__runtime_modules_to_metadata!( $runtime; $( $metadata, )* $crate::metadata::ModuleMetadata { name: $crate::metadata::DecodeDifferent::Encode(stringify!($name)), + fixed_index: { + #[allow(path_statements)] + { core::option::Option::::None $(; Some($index) )? } + }, storage: $crate::__runtime_modules_to_metadata_calls_storage!( $mod, $module $( <$instance> )?, $runtime, $(with $kw)* ), @@ -450,7 +457,7 @@ mod tests { impl_runtime_metadata!( for TestRuntime with modules where Extrinsic = TestExtrinsic system::Module as System with Event, - event_module::Module as Module with Event Call, + event_module::Module as Module with Event Call { index 2 }, event_module2::Module as Module2 with Event Storage Call, ); @@ -481,6 +488,7 @@ mod tests { modules: DecodeDifferent::Encode(&[ ModuleMetadata { name: DecodeDifferent::Encode("System"), + fixed_index: None, storage: None, calls: None, event: Some(DecodeDifferent::Encode( @@ -524,6 +532,7 @@ mod tests { }, ModuleMetadata { name: DecodeDifferent::Encode("Module"), + fixed_index: Some(2), storage: None, calls: Some( DecodeDifferent::Encode(FnEncode(|| &[ @@ -559,6 +568,7 @@ mod tests { }, ModuleMetadata { name: DecodeDifferent::Encode("Module2"), + fixed_index: None, storage: Some(DecodeDifferent::Encode( FnEncode(|| StorageMetadata { prefix: DecodeDifferent::Encode("TestStorage"), diff --git a/frame/support/src/origin.rs b/frame/support/src/origin.rs index df75f8dc65645..a6d5491fdb95c 100644 --- a/frame/support/src/origin.rs +++ b/frame/support/src/origin.rs @@ -65,7 +65,7 @@ macro_rules! impl_outer_origin { $runtime:ident; $system:ident; Modules { - $module:ident $instance:ident + $( #[codec(index = $index:tt)] )? $module:ident $instance:ident $(, $( $rest_module:tt )* )? }; $( $parsed:tt )* @@ -77,7 +77,7 @@ macro_rules! impl_outer_origin { $runtime; $system; Modules { $( $( $rest_module )* )? }; - $( $parsed )* $module <$runtime> { $instance }, + $( $parsed )* $module <$runtime> { $instance } index { $( $index )? }, ); }; @@ -89,7 +89,7 @@ macro_rules! impl_outer_origin { $runtime:ident; $system:ident; Modules { - $module:ident $instance:ident + $( #[codec(index = $index:tt )] )? $module:ident $instance:ident $(, $rest_module:tt )* }; $( $parsed:tt )* @@ -101,7 +101,7 @@ macro_rules! impl_outer_origin { $runtime; $system; Modules { $( $rest_module )* }; - $( $parsed )* $module { $instance }, + $( $parsed )* $module { $instance } index { $( $index )? }, ); }; @@ -113,7 +113,7 @@ macro_rules! impl_outer_origin { $runtime:ident; $system:ident; Modules { - $module:ident + $( #[codec(index = $index:tt )] )? $module:ident $(, $( $rest_module:tt )* )? }; $( $parsed:tt )* @@ -125,7 +125,7 @@ macro_rules! impl_outer_origin { $runtime; $system; Modules { $( $( $rest_module )* )? }; - $( $parsed )* $module <$runtime>, + $( $parsed )* $module <$runtime> index { $( $index )? }, ); }; @@ -137,7 +137,7 @@ macro_rules! impl_outer_origin { $runtime:ident; $system:ident; Modules { - $module:ident + $( #[codec(index = $index:tt )] )? $module:ident $(, $( $rest_module:tt )* )? }; $( $parsed:tt )* @@ -149,7 +149,7 @@ macro_rules! impl_outer_origin { $runtime; $system; Modules { $( $( $rest_module )* )? }; - $( $parsed )* $module, + $( $parsed )* $module index { $( $index )? }, ); }; @@ -161,7 +161,12 @@ macro_rules! impl_outer_origin { $runtime:ident; $system:ident; Modules { }; - $( $module:ident $( < $generic:ident > )? $( { $generic_instance:ident } )? ,)* + $( + $module:ident + $( < $generic:ident > )? + $( { $generic_instance:ident } )? + index { $( $index:tt )? }, + )* ) => { // WARNING: All instance must hold the filter `frame_system::Trait::BaseCallFilter`, except // when caller is system Root. One can use `OriginTrait::reset_filter` to do so. @@ -233,8 +238,10 @@ macro_rules! impl_outer_origin { $(#[$attr])* #[allow(non_camel_case_types)] pub enum $caller_name { + #[codec(index = "0")] system($system::Origin<$runtime>), $( + $( #[codec(index = $index )] )? [< $module $( _ $generic_instance )? >] ($module::Origin < $( $generic, )? $( $module::$generic_instance )? > ), )* diff --git a/frame/support/test/Cargo.toml b/frame/support/test/Cargo.toml index f2f70fb95279e..804dbca285f12 100644 --- a/frame/support/test/Cargo.toml +++ b/frame/support/test/Cargo.toml @@ -24,6 +24,7 @@ sp-std = { version = "2.0.0-rc6", default-features = false, path = "../../../pri trybuild = "1.0.17" pretty_assertions = "0.6.1" rustversion = "1.0.0" +frame-metadata = { version = "11.0.0-rc6", default-features = false, path = "../../metadata" } [features] default = ["std"] diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 10fc3319fb080..7cf3e8feec00b 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -49,6 +49,17 @@ mod module1 { } } + #[derive(Clone, PartialEq, Eq, Debug, codec::Encode, codec::Decode)] + pub struct Origin(pub core::marker::PhantomData::<(T, I)>); + + frame_support::decl_event! { + pub enum Event where + ::AccountId + { + A(AccountId), + } + } + frame_support::decl_error! { pub enum Error for Module, I: Instance> { Something @@ -80,6 +91,15 @@ mod module2 { } } + #[derive(Clone, PartialEq, Eq, Debug, codec::Encode, codec::Decode)] + pub struct Origin; + + frame_support::decl_event! { + pub enum Event { + A, + } + } + frame_support::decl_error! { pub enum Error for Module { Something @@ -91,8 +111,7 @@ mod module2 { } } -impl module1::Trait for Runtime {} -impl module1::Trait for Runtime {} +impl module1::Trait for Runtime {} impl module2::Trait for Runtime {} pub type Signature = sr25519::Signature; @@ -117,10 +136,17 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic { - System: system::{Module, Call, Event}, - Module1_1: module1::::{Module, Call, Storage}, - Module2: module2::{Module, Call, Storage}, - Module1_2: module1::::{Module, Call, Storage}, + System: system::{Module, Call, Event, Origin}, + Module1_1: module1::::{Module, Call, Storage, Event, Origin}, + Module2: module2::{Module, Call, Storage, Event, Origin}, + Module1_2: module1::::{Module, Call, Storage, Event, Origin}, + Module1_3: module1::::{Module, Storage} = 6, + Module1_4: module1::::{Module, Call} = 3, + Module1_5: module1::::{Module, Event} = 2, + Module1_6: module1::::{Module, Call, Storage, Event, Origin} = 1, + Module1_7: module1::::{Module, Call, Storage, Event, Origin}, + Module1_8: module1::::{Module, Call, Storage, Event, Origin} = 12, + Module1_9: module1::::{Module, Call, Storage, Event, Origin}, } ); @@ -129,27 +155,47 @@ pub type Block = generic::Block; pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; #[test] -fn check_module1_1_error_type() { +fn check_modules_error_type() { assert_eq!( Module1_1::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 1, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 4, error: 0, message: Some("Something") }), + ); + assert_eq!( + Module2::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 5, error: 0, message: Some("Something") }), ); -} - -#[test] -fn check_module1_2_error_type() { assert_eq!( Module1_2::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 7, error: 0, message: Some("Something") }), + ); + assert_eq!( + Module1_3::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 6, error: 0, message: Some("Something") }), + ); + assert_eq!( + Module1_4::fail(system::Origin::::Root.into()), Err(DispatchError::Module { index: 3, error: 0, message: Some("Something") }), ); -} - -#[test] -fn check_module2_error_type() { assert_eq!( - Module2::fail(system::Origin::::Root.into()), + Module1_5::fail(system::Origin::::Root.into()), Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), ); + assert_eq!( + Module1_6::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 1, error: 0, message: Some("Something") }), + ); + assert_eq!( + Module1_7::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 8, error: 0, message: Some("Something") }), + ); + assert_eq!( + Module1_8::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 12, error: 0, message: Some("Something") }), + ); + assert_eq!( + Module1_9::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 9, error: 0, message: Some("Something") }), + ); } #[test] @@ -157,3 +203,271 @@ fn integrity_test_works() { __construct_runtime_integrity_test::runtime_integrity_tests(); assert_eq!(INTEGRITY_TEST_EXEC.with(|i| *i.borrow()), 1); } + +#[test] +fn origin_codec() { + use codec::Encode; + assert_eq!(OriginCaller::system(system::RawOrigin::None).encode()[0], 0); + assert_eq!(OriginCaller::module1_Instance1(module1::Origin(Default::default())).encode()[0], 4); + assert_eq!(OriginCaller::module2(module2::Origin).encode()[0], 5); + assert_eq!(OriginCaller::module1_Instance2(module1::Origin(Default::default())).encode()[0], 7); + assert_eq!(OriginCaller::module1_Instance6(module1::Origin(Default::default())).encode()[0], 1); + assert_eq!(OriginCaller::module1_Instance7(module1::Origin(Default::default())).encode()[0], 8); + assert_eq!(OriginCaller::module1_Instance8(module1::Origin(Default::default())).encode()[0], 12); + assert_eq!(OriginCaller::module1_Instance9(module1::Origin(Default::default())).encode()[0], 9); +} + +#[test] +fn event_codec() { + use codec::Encode; + assert_eq!(Event::system(system::Event::::ExtrinsicSuccess).encode()[0], 0); + assert_eq!(Event::module1_Instance1(module1::Event::::A(Default::default())).encode()[0], 4); + assert_eq!(Event::module2(module2::Event::A).encode()[0], 5); + assert_eq!(Event::module1_Instance2(module1::Event::::A(Default::default())).encode()[0], 7); + assert_eq!(Event::module1_Instance5(module1::Event::::A(Default::default())).encode()[0], 2); + assert_eq!(Event::module1_Instance6(module1::Event::::A(Default::default())).encode()[0], 1); + assert_eq!(Event::module1_Instance7(module1::Event::::A(Default::default())).encode()[0], 8); + assert_eq!(Event::module1_Instance8(module1::Event::::A(Default::default())).encode()[0], 12); + assert_eq!(Event::module1_Instance9(module1::Event::::A(Default::default())).encode()[0], 9); +} + +#[test] +fn call_codec() { + use codec::Encode; + assert_eq!(Call::System(system::Call::noop()).encode()[0], 0); + assert_eq!(Call::Module1_1(module1::Call::fail()).encode()[0], 4); + assert_eq!(Call::Module2(module2::Call::fail()).encode()[0], 5); + assert_eq!(Call::Module1_2(module1::Call::fail()).encode()[0], 7); + assert_eq!(Call::Module1_4(module1::Call::fail()).encode()[0], 3); + assert_eq!(Call::Module1_6(module1::Call::fail()).encode()[0], 1); + assert_eq!(Call::Module1_7(module1::Call::fail()).encode()[0], 8); + assert_eq!(Call::Module1_8(module1::Call::fail()).encode()[0], 12); + assert_eq!(Call::Module1_9(module1::Call::fail()).encode()[0], 9); +} + +#[test] +fn test_metadata() { + use frame_metadata::*; + let expected_metadata: RuntimeMetadataLastVersion = RuntimeMetadataLastVersion { + modules: DecodeDifferent::Encode(&[ + ModuleMetadata { + name: DecodeDifferent::Encode("System"), + storage: None, + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("noop"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[ + EventMetadata { + name: DecodeDifferent::Encode("ExtrinsicSuccess"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }, + EventMetadata { + name: DecodeDifferent::Encode("ExtrinsicFailed"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }, + EventMetadata { + name: DecodeDifferent::Encode("Ignore"), + arguments: DecodeDifferent::Encode(&["BlockNumber"]), + documentation: DecodeDifferent::Encode(&[]), + }, + ]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: None, + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_1"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance1Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[ + FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }, + ]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: None, + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module2"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[ + FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }, + ]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[ + EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }, + ]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: None, + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_2"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance2Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: None, + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_3"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance3Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: None, + event: None, + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: Some(6), + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_4"), + storage: None, + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: None, + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: Some(3), + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_5"), + storage: None, + calls: None, + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: Some(2), + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_6"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance6Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: Some(1), + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_7"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance7Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: None, + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_8"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance8Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: Some(12), + }, + ModuleMetadata { + name: DecodeDifferent::Encode("Module1_9"), + storage: Some(DecodeDifferent::Encode(FnEncode(|| StorageMetadata { + prefix: DecodeDifferent::Encode("Instance9Module"), + entries: DecodeDifferent::Encode(&[]), + }))), + calls: Some(DecodeDifferent::Encode(FnEncode(|| &[FunctionMetadata { + name: DecodeDifferent::Encode("fail"), + arguments: DecodeDifferent::Encode(&[]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + event: Some(DecodeDifferent::Encode(FnEncode(|| &[EventMetadata { + name: DecodeDifferent::Encode("A"), + arguments: DecodeDifferent::Encode(&["AccountId"]), + documentation: DecodeDifferent::Encode(&[]), + }]))), + constants: DecodeDifferent::Encode(FnEncode(|| &[])), + errors: DecodeDifferent::Encode(FnEncode(|| &[])), + fixed_index: None, + }, + ]), + extrinsic: ExtrinsicMetadata { + version: 4, + signed_extensions: vec![DecodeDifferent::Encode("UnitSignedExtension")], + }, + }; + pretty_assertions::assert_eq!(Runtime::metadata().1, RuntimeMetadata::V12(expected_metadata)); +} diff --git a/frame/support/test/tests/system.rs b/frame/support/test/tests/system.rs index 8ca2e97789d54..dca1b35fc65b8 100644 --- a/frame/support/test/tests/system.rs +++ b/frame/support/test/tests/system.rs @@ -31,7 +31,10 @@ pub trait Trait: 'static + Eq + Clone { } frame_support::decl_module! { - pub struct Module for enum Call where origin: T::Origin, {} + pub struct Module for enum Call where origin: T::Origin, { + #[weight = 0] + fn noop(origin) {} + } } impl Module { From 766f510ef26f7a6fc472d5ccd04f755c137846e1 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 1 Sep 2020 14:22:15 +0200 Subject: [PATCH 02/31] add test and doc --- frame/metadata/src/lib.rs | 2 ++ .../procedural/src/construct_runtime/mod.rs | 18 +++++++++----- .../procedural/src/construct_runtime/parse.rs | 4 ++++ frame/support/src/event.rs | 15 +++++++++++- frame/support/src/origin.rs | 24 +++++++++++++++++++ .../system_at_index_non_zero_deprecated.rs | 14 +++++++++++ ...system_at_index_non_zero_deprecated.stderr | 5 ++++ 7 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs create mode 100644 frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr diff --git a/frame/metadata/src/lib.rs b/frame/metadata/src/lib.rs index 61234cfb32fce..3bb2147051f27 100644 --- a/frame/metadata/src/lib.rs +++ b/frame/metadata/src/lib.rs @@ -409,6 +409,8 @@ pub struct ModuleMetadata { pub event: ODFnA, pub constants: DFnA, pub errors: DFnA, + /// Define the fix index of the module, this index will be used for the encoding of module + /// event, call and origin variants if it has some. pub fixed_index: Option, } diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 3dbfd7f7d7fd4..f7b378c9f1f34 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -301,7 +301,11 @@ fn decl_outer_origin<'a>( } let index = match module_declaration.index { Some(index) => { - assert!(index != 0, "Internal error, some origin is at index 0"); + assert!( + index != 0, + "Internal error, some origin is at index 0, but it is reserved to \ + system" + ); index }, None => available_indices.remove(0), @@ -480,12 +484,13 @@ fn check_modules( Please add this line: `System: frame_system::{Module, Call, Storage, Config, Event},`", ))?; + // Assert no more than 256 modules if modules.len() > u8::max_value() as usize { let msg = "Too many modules defined, only 256 modules is currently allowed"; return Err(syn::Error::new(modules_span, msg)); } - // No module use index 0 + // No module use index 0 explicity (or it is system) if let Some(module) = modules.iter() .find(|module| { module.index.map_or(false, |i| i == 0) && module.name != SYSTEM_MODULE_NAME @@ -498,14 +503,15 @@ fn check_modules( } if !deprecated_system_non_zero { + // Assert system module index is 0 (either explicitly or implicitly). if let Some(index) = system_module.index { - // System module is at index 0 explicitly + // Assert system module is at index 0 explicitly if index != 0 { let msg = "System module must be defined at index `0`"; return Err(syn::Error::new(system_module.module.span(), msg)); } } else { - // System module is at index 0 implicitly + // Assert system module is at index 0 implicitly let first_implicit_module = modules.iter().filter(|m| m.index.is_none()).next() .expect("At least system module exists; qed"); @@ -514,8 +520,8 @@ fn check_modules( "System module must be defined at index `0`, instead {} is found. (this check \ is to avoid confusion for generation of origin_caller, (where system is \ forced to be at index 0 anyway), to bypass this check use attribute \ - `#[deprecated_system_non_zero]` as \ - `construct_runtime!(#[deprecated_system_non_zero] ... )`).", + `#[deprecated_system_non_zero]` as in \ + `construct_runtime! {{ #[deprecated_system_non_zero] ... }}`).", first_implicit_module.name, ); return Err(syn::Error::new(first_implicit_module.module.span(), msg)); diff --git a/frame/support/procedural/src/construct_runtime/parse.rs b/frame/support/procedural/src/construct_runtime/parse.rs index a1fdcd3a140c7..49d7f4d85b77d 100644 --- a/frame/support/procedural/src/construct_runtime/parse.rs +++ b/frame/support/procedural/src/construct_runtime/parse.rs @@ -45,6 +45,7 @@ pub struct RuntimeDefinition { pub enum_token: Token![enum], pub name: Ident, pub where_section: WhereSection, + /// Optional attribute `#[deprecated_system_non_zero]` pub deprecated_system_non_zero: bool, pub modules: ext::Braces>, } @@ -53,6 +54,8 @@ impl Parse for RuntimeDefinition { fn parse(input: ParseStream) -> Result { let mut deprecated_system_non_zero = false; + // check for optional attribute `#[deprecated_system_non_zero]` + // (no other attribute are allowed.) if input.peek(Token![#]) { input.parse::()?; let attr_content; @@ -165,6 +168,7 @@ impl Parse for WhereDefinition { #[derive(Debug, Clone)] pub struct ModuleDeclaration { pub name: Ident, + /// Optional fixed index (e.g. `MyPallet ... = 3,`) pub index: Option, pub module: Ident, pub instance: Option, diff --git a/frame/support/src/event.rs b/frame/support/src/event.rs index 4fd7d5978c34c..9680063b2e5bc 100644 --- a/frame/support/src/event.rs +++ b/frame/support/src/event.rs @@ -705,7 +705,7 @@ mod tests { pub enum TestEventSystemRenamed for TestRuntime2 { system_renamed, event_module, - event_module2, + #[codec(index = "5")] event_module2, event_module3, } } @@ -804,4 +804,17 @@ mod tests { fn outer_event_metadata() { assert_eq!(EXPECTED_METADATA, TestRuntime::outer_event_metadata()); } + + #[test] + fn test_codec() { + let runtime_1_event_module_2 = TestEvent::event_module2( + event_module2::Event::::TestEvent(3) + ); + assert_eq!(runtime_1_event_module_2.encode()[0], 2); + + let runtime_2_event_module_2 = TestEventSystemRenamed::event_module2( + event_module2::Event::::TestEvent(3) + ); + assert_eq!(runtime_2_event_module_2.encode()[0], 5); + } } diff --git a/frame/support/src/origin.rs b/frame/support/src/origin.rs index a6d5491fdb95c..c756283f506e1 100644 --- a/frame/support/src/origin.rs +++ b/frame/support/src/origin.rs @@ -244,6 +244,7 @@ macro_rules! impl_outer_origin { $( #[codec(index = $index )] )? [< $module $( _ $generic_instance )? >] ($module::Origin < $( $generic, )? $( $module::$generic_instance )? > ), + )* #[allow(dead_code)] Void($crate::Void) @@ -449,6 +450,13 @@ mod tests { pub enum OriginEmpty for TestRuntime where system = frame_system {} ); + impl_outer_origin!( + pub enum OriginIndices for TestRuntime { + origin_with_generic, + #[codec(index = "10")] origin_without_generic, + } + ); + #[test] fn test_default_filter() { assert_eq!(OriginWithSystem::root().filter_call(&0), true); @@ -479,4 +487,20 @@ mod tests { assert_eq!(origin.filter_call(&0), true); assert_eq!(origin.filter_call(&1), false); } + + #[test] + fn test_codec() { + use codec::Encode; + assert_eq!(OriginIndices::root().caller.encode()[0], 0); + let without_generic_variant = OriginIndicesCaller::origin_without_generic( + origin_without_generic::Origin + ); + assert_eq!(without_generic_variant.encode()[0], 10); + + assert_eq!(OriginWithoutSystem::root().caller.encode()[0], 0); + let without_generic_variant = OriginWithoutSystemCaller::origin_without_generic( + origin_without_generic::Origin + ); + assert_eq!(without_generic_variant.encode()[0], 1); + } } diff --git a/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs b/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs new file mode 100644 index 0000000000000..75d6e1f1531e0 --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs @@ -0,0 +1,14 @@ +use frame_support::construct_runtime; + +construct_runtime! { + pub enum Runtime where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + Balance: balances::{Config, Call, Origin}, + System: system::{Module}, + } +} + +fn main() {} diff --git a/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr b/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr new file mode 100644 index 0000000000000..24300e591f34b --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr @@ -0,0 +1,5 @@ +error: System module must be defined at index `0`, instead Balance is found. (this check is to avoid confusion for generation of origin_caller, (where system is forced to be at index 0 anyway), to bypass this check use attribute `#[deprecated_system_non_zero]` as in `construct_runtime! { #[deprecated_system_non_zero] ... }`). + --> $DIR/system_at_index_non_zero_deprecated.rs:9:12 + | +9 | Balance: balances::{Config, Call, Origin}, + | ^^^^^^^^ From d246bdfceec65385b7bb6ff8d0649754a9ea0ede Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 1 Sep 2020 14:56:52 +0200 Subject: [PATCH 03/31] remove deprecated and document behavior --- .../procedural/src/construct_runtime/mod.rs | 34 ++----------------- .../procedural/src/construct_runtime/parse.rs | 16 --------- frame/support/procedural/src/lib.rs | 16 ++++++--- .../system_at_index_non_zero_deprecated.rs | 14 -------- ...system_at_index_non_zero_deprecated.stderr | 5 --- 5 files changed, 15 insertions(+), 70 deletions(-) delete mode 100644 frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs delete mode 100644 frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index f7b378c9f1f34..bb0e967aa80a9 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -38,7 +38,6 @@ pub fn construct_runtime(input: TokenStream) -> TokenStream { fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result { let RuntimeDefinition { name, - deprecated_system_non_zero, where_section: WhereSection { block, node_block, @@ -54,7 +53,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result>(); - let system_module = check_modules(deprecated_system_non_zero, &modules, modules_token.span)?; + let system_module = check_modules(&modules, modules_token.span)?; let hidden_crate_name = "construct_runtime"; let scrate = generate_crate_access(&hidden_crate_name, "frame-support"); @@ -471,7 +470,6 @@ fn get_available_indices(modules: &Vec) -> Vec { /// /// returns system module ident fn check_modules( - deprecated_system_non_zero: bool, modules: &Vec, modules_span: proc_macro2::Span, ) -> Result { @@ -497,38 +495,12 @@ fn check_modules( }) { let msg = format!( - "Only system module is allowed to be defined at index `0`", + "Only system module is allowed to be defined at index `0`, this is because of origin \ + encoding", ); return Err(syn::Error::new(module.name.span(), msg)); } - if !deprecated_system_non_zero { - // Assert system module index is 0 (either explicitly or implicitly). - if let Some(index) = system_module.index { - // Assert system module is at index 0 explicitly - if index != 0 { - let msg = "System module must be defined at index `0`"; - return Err(syn::Error::new(system_module.module.span(), msg)); - } - } else { - // Assert system module is at index 0 implicitly - let first_implicit_module = modules.iter().filter(|m| m.index.is_none()).next() - .expect("At least system module exists; qed"); - - if first_implicit_module.name != SYSTEM_MODULE_NAME { - let msg = format!( - "System module must be defined at index `0`, instead {} is found. (this check \ - is to avoid confusion for generation of origin_caller, (where system is \ - forced to be at index 0 anyway), to bypass this check use attribute \ - `#[deprecated_system_non_zero]` as in \ - `construct_runtime! {{ #[deprecated_system_non_zero] ... }}`).", - first_implicit_module.name, - ); - return Err(syn::Error::new(first_implicit_module.module.span(), msg)); - } - } - } - // No module indices conflicts let mut indices = vec![]; for module in modules { diff --git a/frame/support/procedural/src/construct_runtime/parse.rs b/frame/support/procedural/src/construct_runtime/parse.rs index 49d7f4d85b77d..639451a65ca00 100644 --- a/frame/support/procedural/src/construct_runtime/parse.rs +++ b/frame/support/procedural/src/construct_runtime/parse.rs @@ -36,7 +36,6 @@ mod keyword { syn::custom_keyword!(Origin); syn::custom_keyword!(Inherent); syn::custom_keyword!(ValidateUnsigned); - syn::custom_keyword!(deprecated_system_non_zero); } #[derive(Debug)] @@ -45,27 +44,12 @@ pub struct RuntimeDefinition { pub enum_token: Token![enum], pub name: Ident, pub where_section: WhereSection, - /// Optional attribute `#[deprecated_system_non_zero]` - pub deprecated_system_non_zero: bool, pub modules: ext::Braces>, } impl Parse for RuntimeDefinition { fn parse(input: ParseStream) -> Result { - let mut deprecated_system_non_zero = false; - - // check for optional attribute `#[deprecated_system_non_zero]` - // (no other attribute are allowed.) - if input.peek(Token![#]) { - input.parse::()?; - let attr_content; - syn::bracketed!(attr_content in input); - attr_content.parse::()?; - deprecated_system_non_zero = true; - } - Ok(Self { - deprecated_system_non_zero, visibility_token: input.parse()?, enum_token: input.parse()?, name: input.parse()?, diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 054d90d7bbaeb..c80ad074b93f8 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -254,13 +254,13 @@ pub fn decl_storage(input: TokenStream) -> TokenStream { /// NodeBlock = runtime::Block, /// UncheckedExtrinsic = UncheckedExtrinsic /// { -/// System: system::{Module, Call, Event, Config}, -/// Test: test::{Module, Call}, -/// Test2: test_with_long_module::{Module}, +/// System: system::{Module, Call, Event, Config} = 0, +/// Test: test::{Module, Call} = 1, +/// Test2: test_with_long_module::{Module, Event}, /// /// // Module with instances /// Test3_Instance1: test3::::{Module, Call, Storage, Event, Config, Origin}, -/// Test3_DefaultInstance: test3::{Module, Call, Storage, Event, Config, Origin}, +/// Test3_DefaultInstance: test3::{Module, Call, Storage, Event, Config, Origin} = 4, /// } /// ) /// ``` @@ -281,6 +281,14 @@ pub fn decl_storage(input: TokenStream) -> TokenStream { /// - `Inherent` - If the module provides/can check inherents. /// - `ValidateUnsigned` - If the module validates unsigned extrinsics. /// +/// `= $n` is an optional part allowing to define at which index the module variants in +/// `OriginCaller`, `Call` and `Event` are encoded, and to define the ModuleToIndex value. +/// +/// Call and Event encodes its variant using the index provided or with the first available index. +/// OriginCaller encodes system as variant of index 0 and then using the index provided or with the +/// first available index. (E.g. In the above example `Call::Test3_Instance1` is encoded at index +/// 2, `Event::Test3_Instance1 is encoded at index 3). +/// /// # Note /// /// The population of the genesis storage depends on the order of modules. So, if one of your diff --git a/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs b/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs deleted file mode 100644 index 75d6e1f1531e0..0000000000000 --- a/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.rs +++ /dev/null @@ -1,14 +0,0 @@ -use frame_support::construct_runtime; - -construct_runtime! { - pub enum Runtime where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - Balance: balances::{Config, Call, Origin}, - System: system::{Module}, - } -} - -fn main() {} diff --git a/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr b/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr deleted file mode 100644 index 24300e591f34b..0000000000000 --- a/frame/support/test/tests/construct_runtime_ui/system_at_index_non_zero_deprecated.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: System module must be defined at index `0`, instead Balance is found. (this check is to avoid confusion for generation of origin_caller, (where system is forced to be at index 0 anyway), to bypass this check use attribute `#[deprecated_system_non_zero]` as in `construct_runtime! { #[deprecated_system_non_zero] ... }`). - --> $DIR/system_at_index_non_zero_deprecated.rs:9:12 - | -9 | Balance: balances::{Config, Call, Origin}, - | ^^^^^^^^ From 11a3af2044d5d4e3cb87e90aff092ce0dc82ed80 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 1 Sep 2020 15:03:34 +0200 Subject: [PATCH 04/31] update internal doc --- frame/support/procedural/src/construct_runtime/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index bb0e967aa80a9..7a78206a7f608 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -465,7 +465,6 @@ fn get_available_indices(modules: &Vec) -> Vec { /// * check system module is defined /// * there is less than 256 modules /// * no module use index 0 or except system -/// * system module index is 0 (either explicitly or implicitly). /// * module indices don't conflict. /// /// returns system module ident From ce2d1bd961c1ca2e5b55bc7944ec016a9ad31dde Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Mon, 7 Sep 2020 16:33:45 +0200 Subject: [PATCH 05/31] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/src/origin.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/src/origin.rs b/frame/support/src/origin.rs index c756283f506e1..1ef97796d1653 100644 --- a/frame/support/src/origin.rs +++ b/frame/support/src/origin.rs @@ -241,10 +241,9 @@ macro_rules! impl_outer_origin { #[codec(index = "0")] system($system::Origin<$runtime>), $( - $( #[codec(index = $index )] )? + $( #[codec(index = $index)] )? [< $module $( _ $generic_instance )? >] ($module::Origin < $( $generic, )? $( $module::$generic_instance )? > ), - )* #[allow(dead_code)] Void($crate::Void) From b56cbd704d7446bbe536e59719f5b2d602665525 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 7 Sep 2020 17:12:26 +0200 Subject: [PATCH 06/31] address review --- .../procedural/src/construct_runtime/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 7a78206a7f608..bcfa8cd5b9eea 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -24,6 +24,7 @@ use proc_macro::TokenStream; use proc_macro2::{TokenStream as TokenStream2, Span}; use quote::quote; use syn::{Ident, Result, TypePath}; +use std::collections::HashSet; /// The fixed name of the system module. const SYSTEM_MODULE_NAME: &str = "System"; @@ -493,21 +494,18 @@ fn check_modules( module.index.map_or(false, |i| i == 0) && module.name != SYSTEM_MODULE_NAME }) { - let msg = format!( - "Only system module is allowed to be defined at index `0`, this is because of origin \ - encoding", - ); + let msg = "Only system module is allowed to be defined at index `0`, this is to avoid \ + confusion for encoding of origin. Indeed, origin system variant is always encoded at \ + index 0"; return Err(syn::Error::new(module.name.span(), msg)); } // No module indices conflicts - let mut indices = vec![]; + let mut indices = HashSet::new(); for module in modules { if let Some(index) = module.index { - if indices.contains(&index) { + if !indices.insert(index) { return Err(syn::Error::new(module.module.span(), "module index is already used")); - } else { - indices.push(index) } } } From da702aee146395e9048c33111046d27e6e230aae Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 8 Sep 2020 12:43:40 +0200 Subject: [PATCH 07/31] use index for all module, break construct_runtime --- frame/metadata/src/lib.rs | 6 +- .../procedural/src/construct_runtime/mod.rs | 148 +++++++----------- frame/support/procedural/src/lib.rs | 12 +- frame/support/src/metadata.rs | 25 ++- frame/support/src/origin.rs | 21 ++- frame/support/test/tests/construct_runtime.rs | 76 ++++----- 6 files changed, 133 insertions(+), 155 deletions(-) diff --git a/frame/metadata/src/lib.rs b/frame/metadata/src/lib.rs index 3bb2147051f27..109f33f420191 100644 --- a/frame/metadata/src/lib.rs +++ b/frame/metadata/src/lib.rs @@ -409,9 +409,9 @@ pub struct ModuleMetadata { pub event: ODFnA, pub constants: DFnA, pub errors: DFnA, - /// Define the fix index of the module, this index will be used for the encoding of module - /// event, call and origin variants if it has some. - pub fixed_index: Option, + /// Define the index of the module, this index will be used for the encoding of module event, + /// call and origin variants. + pub index: u8, } type ODFnA = Option>; diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index bcfa8cd5b9eea..39e4f1692a796 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -24,7 +24,7 @@ use proc_macro::TokenStream; use proc_macro2::{TokenStream as TokenStream2, Span}; use quote::quote; use syn::{Ident, Result, TypePath}; -use std::collections::HashSet; +use std::collections::HashMap; /// The fixed name of the system module. const SYSTEM_MODULE_NAME: &str = "System"; @@ -52,9 +52,19 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result>(); - let system_module = check_modules(&modules, modules_token.span)?; + let mut modules = modules.into_iter().collect::>(); + + // Automatically assign implicit index: + assign_implicit_index(&mut modules)?; + + let system_module = modules.iter() + .find(|decl| decl.name == SYSTEM_MODULE_NAME) + .ok_or_else(|| syn::Error::new( + modules_token.span, + "`System` module declaration is missing. \ + Please add this line: `System: frame_system::{Module, Call, Storage, Config, Event},`", + ))?; let hidden_crate_name = "construct_runtime"; let scrate = generate_crate_access(&hidden_crate_name, "frame-support"); @@ -234,10 +244,10 @@ fn decl_runtime_metadata<'a>( .map(|name| quote!(<#name>)) .into_iter(); - let index = module_declaration.index.map(|index| quote::quote!( { index #index } )); + let index = module_declaration.index.expect("All index have been assigned"); quote!( - #module::Module #(#instance)* as #name with #(#filtered_names)* #index, + #module::Module #(#instance)* as #name { index #index } with #(#filtered_names)*, ) }); quote!( @@ -253,16 +263,12 @@ fn decl_outer_dispatch<'a>( module_declarations: &Vec, scrate: &'a TokenStream2, ) -> TokenStream2 { - let mut available_indices = get_available_indices(module_declarations); let modules_tokens = module_declarations.iter() .filter(|module_declaration| module_declaration.exists_part("Call")) .map(|module_declaration| { let module = &module_declaration.module; let name = &module_declaration.name; - let index = match module_declaration.index { - Some(index) => index, - None => available_indices.remove(0), - }; + let index = module_declaration.index.expect("All index have been assigned"); let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); quote!(#[codec(index = #index_string)] #module::#name) }); @@ -279,12 +285,10 @@ fn decl_outer_dispatch<'a>( fn decl_outer_origin<'a>( runtime_name: &'a Ident, modules_except_system: &Vec, - system_name: &'a Ident, + system_module: &'a ModuleDeclaration, scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); - let mut available_indices = get_available_indices(modules_except_system); - available_indices.retain(|i| *i != 0); // index 0 is reserved for system in origin for module_declaration in modules_except_system { match module_declaration.find_part("Origin") { Some(module_entry) => { @@ -299,28 +303,25 @@ fn decl_outer_origin<'a>( ); return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let index = match module_declaration.index { - Some(index) => { - assert!( - index != 0, - "Internal error, some origin is at index 0, but it is reserved to \ - system" - ); - index - }, - None => available_indices.remove(0), - }; + let index = module_declaration.index.expect("All index have been assigned"); let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); - let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics ,); + let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); modules_tokens.extend(tokens); } None => {} } } + let system_name = &system_module.module; + let system_index = system_module.index.expect("All index have been assigned"); + let system_index_string = syn::LitStr::new(&format!("{}", system_index), Span::call_site()); + Ok(quote!( #scrate::impl_outer_origin! { - pub enum Origin for #runtime_name where system = #system_name { + pub enum Origin for #runtime_name where + system = #system_name, + system_index = #system_index_string + { #modules_tokens } } @@ -333,7 +334,6 @@ fn decl_outer_event<'a>( scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); - let mut available_indices = get_available_indices(module_declarations); for module_declaration in module_declarations { match module_declaration.find_part("Event") { Some(module_entry) => { @@ -349,10 +349,7 @@ fn decl_outer_event<'a>( return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let index = match module_declaration.index { - Some(index) => index, - None => available_indices.remove(0), - }; + let index = module_declaration.index.expect("All index have been assigned"); let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); modules_tokens.extend(tokens); @@ -409,14 +406,8 @@ fn decl_module_to_index<'a>( scrate: &TokenStream2, ) -> TokenStream2 { let names = module_declarations.iter().map(|d| &d.name); - let mut available_indices = get_available_indices(module_declarations); let indices = module_declarations.iter() - .map(|module| { - match module.index { - Some(index) => index as usize, - None => available_indices.remove(0) as usize, - } - }); + .map(|module| module.index.expect("All index have been assigned") as usize); quote!( /// Provides an implementation of `ModuleToIndex` to map a module @@ -452,63 +443,36 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { ) } -fn get_available_indices(modules: &Vec) -> Vec { - let reserved_indices = modules.iter() - .filter_map(|module| module.index) - .collect::>(); - - (0..u8::max_value()) - .filter(|i| !reserved_indices.contains(i)) - .collect::>() -} - -/// Check: -/// * check system module is defined -/// * there is less than 256 modules -/// * no module use index 0 or except system -/// * module indices don't conflict. -/// -/// returns system module ident -fn check_modules( - modules: &Vec, - modules_span: proc_macro2::Span, -) -> Result { - // Assert we have system module declared - let system_module = modules.iter() - .find(|decl| decl.name == SYSTEM_MODULE_NAME) - .ok_or_else(|| syn::Error::new( - modules_span, - "`System` module declaration is missing. \ - Please add this line: `System: frame_system::{Module, Call, Storage, Config, Event},`", - ))?; - - // Assert no more than 256 modules - if modules.len() > u8::max_value() as usize { - let msg = "Too many modules defined, only 256 modules is currently allowed"; - return Err(syn::Error::new(modules_span, msg)); - } +/// Assign index to each modules using same rules as rust for fieldless enum. +/// I.e. implicit are assigned number incrementedly from last explicit or 0. +fn assign_implicit_index(modules: &mut Vec) -> syn::Result<()> { + let mut indices = HashMap::new(); + let mut last_index: u8 = 0; - // No module use index 0 explicity (or it is system) - if let Some(module) = modules.iter() - .find(|module| { - module.index.map_or(false, |i| i == 0) && module.name != SYSTEM_MODULE_NAME - }) - { - let msg = "Only system module is allowed to be defined at index `0`, this is to avoid \ - confusion for encoding of origin. Indeed, origin system variant is always encoded at \ - index 0"; - return Err(syn::Error::new(module.name.span(), msg)); - } - - // No module indices conflicts - let mut indices = HashSet::new(); for module in modules { - if let Some(index) = module.index { - if !indices.insert(index) { - return Err(syn::Error::new(module.module.span(), "module index is already used")); - } + match module.index { + Some(index) => { + last_index = index; + }, + None => { + last_index = match last_index.checked_add(1) { + Some(i) => i, + None => { + let msg = "module index doesn't fit into u8, index is 256"; + return Err(syn::Error::new(module.module.span(), msg)); + }, + }; + module.index = Some(last_index); + }, + } + + if let Some(used_span) = indices.insert(module.index.unwrap(), module.module.span()) { + let span = module.module.span().join(used_span) + .expect("Modules are defined in same file"); + let msg = format!("module index are conflicting, at index {}", module.index.unwrap()); + return Err(syn::Error::new(span, msg)); } } - Ok(system_module.module.clone()) + Ok(()) } diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index c80ad074b93f8..a963728964cfd 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -284,10 +284,14 @@ pub fn decl_storage(input: TokenStream) -> TokenStream { /// `= $n` is an optional part allowing to define at which index the module variants in /// `OriginCaller`, `Call` and `Event` are encoded, and to define the ModuleToIndex value. /// -/// Call and Event encodes its variant using the index provided or with the first available index. -/// OriginCaller encodes system as variant of index 0 and then using the index provided or with the -/// first available index. (E.g. In the above example `Call::Test3_Instance1` is encoded at index -/// 2, `Event::Test3_Instance1 is encoded at index 3). +/// if `= $n` is not given, then index is resolved same as fieldless enum in rust +/// (i.e. incrementedly from previous index): +/// ```nocompile +/// module1 .. = 2, +/// module2 .., // Here module2 is given index 3 +/// module3 .. = 0, +/// module4 .., // Here module4 is given index 1 +/// ``` /// /// # Note /// diff --git a/frame/support/src/metadata.rs b/frame/support/src/metadata.rs index 9e507d2010fe8..fa9553ce7cacf 100644 --- a/frame/support/src/metadata.rs +++ b/frame/support/src/metadata.rs @@ -51,9 +51,9 @@ pub use frame_metadata::{ /// struct Runtime; /// frame_support::impl_runtime_metadata! { /// for Runtime with modules where Extrinsic = UncheckedExtrinsic -/// module0::Module as Module0 with, -/// module1::Module as Module1 with, -/// module2::Module as Module2 with Storage { index 3 }, +/// module0::Module as Module0 { index 0 } with, +/// module1::Module as Module1 { index 1 } with, +/// module2::Module as Module2 { index 2 } with Storage, /// }; /// ``` /// @@ -92,8 +92,8 @@ macro_rules! __runtime_modules_to_metadata { $runtime: ident; $( $metadata:expr ),*; $mod:ident::$module:ident $( < $instance:ident > )? as $name:ident + { index $index:tt } $(with)+ $($kw:ident)* - $( { index $index:tt } )? , $( $rest:tt )* ) => { @@ -101,10 +101,7 @@ macro_rules! __runtime_modules_to_metadata { $runtime; $( $metadata, )* $crate::metadata::ModuleMetadata { name: $crate::metadata::DecodeDifferent::Encode(stringify!($name)), - fixed_index: { - #[allow(path_statements)] - { core::option::Option::::None $(; Some($index) )? } - }, + index: $index, storage: $crate::__runtime_modules_to_metadata_calls_storage!( $mod, $module $( <$instance> )?, $runtime, $(with $kw)* ), @@ -456,9 +453,9 @@ mod tests { impl_runtime_metadata!( for TestRuntime with modules where Extrinsic = TestExtrinsic - system::Module as System with Event, - event_module::Module as Module with Event Call { index 2 }, - event_module2::Module as Module2 with Event Storage Call, + system::Module as System { index 0 } with Event, + event_module::Module as Module { index 1 } with Event Call, + event_module2::Module as Module2 { index 2 } with Event Storage Call, ); struct ConstantBlockNumberByteGetter; @@ -488,7 +485,7 @@ mod tests { modules: DecodeDifferent::Encode(&[ ModuleMetadata { name: DecodeDifferent::Encode("System"), - fixed_index: None, + index: 0, storage: None, calls: None, event: Some(DecodeDifferent::Encode( @@ -532,7 +529,7 @@ mod tests { }, ModuleMetadata { name: DecodeDifferent::Encode("Module"), - fixed_index: Some(2), + index: 1, storage: None, calls: Some( DecodeDifferent::Encode(FnEncode(|| &[ @@ -568,7 +565,7 @@ mod tests { }, ModuleMetadata { name: DecodeDifferent::Encode("Module2"), - fixed_index: None, + index: 2, storage: Some(DecodeDifferent::Encode( FnEncode(|| StorageMetadata { prefix: DecodeDifferent::Encode("TestStorage"), diff --git a/frame/support/src/origin.rs b/frame/support/src/origin.rs index 1ef97796d1653..e4052337a0141 100644 --- a/frame/support/src/origin.rs +++ b/frame/support/src/origin.rs @@ -41,7 +41,10 @@ macro_rules! impl_outer_origin { ( $(#[$attr:meta])* - pub enum $name:ident for $runtime:ident where system = $system:ident { + pub enum $name:ident for $runtime:ident where + system = $system:ident + $(, system_index = $system_index:tt)? + { $( $rest_with_system:tt )* } ) => { @@ -52,6 +55,7 @@ macro_rules! impl_outer_origin { [< $name Caller >]; $runtime; $system; + system_index { $( $system_index )? }; Modules { $( $rest_with_system )* }; ); } @@ -64,6 +68,7 @@ macro_rules! impl_outer_origin { $caller_name:ident; $runtime:ident; $system:ident; + system_index { $( $system_index:tt )? }; Modules { $( #[codec(index = $index:tt)] )? $module:ident $instance:ident $(, $( $rest_module:tt )* )? @@ -76,6 +81,7 @@ macro_rules! impl_outer_origin { $caller_name; $runtime; $system; + system_index { $( $system_index )? }; Modules { $( $( $rest_module )* )? }; $( $parsed )* $module <$runtime> { $instance } index { $( $index )? }, ); @@ -88,6 +94,7 @@ macro_rules! impl_outer_origin { $caller_name:ident; $runtime:ident; $system:ident; + system_index { $( $system_index:tt )? }; Modules { $( #[codec(index = $index:tt )] )? $module:ident $instance:ident $(, $rest_module:tt )* @@ -100,6 +107,7 @@ macro_rules! impl_outer_origin { $caller_name; $runtime; $system; + system_index { $( $system_index )? }; Modules { $( $rest_module )* }; $( $parsed )* $module { $instance } index { $( $index )? }, ); @@ -112,6 +120,7 @@ macro_rules! impl_outer_origin { $caller_name:ident; $runtime:ident; $system:ident; + system_index { $( $system_index:tt )? }; Modules { $( #[codec(index = $index:tt )] )? $module:ident $(, $( $rest_module:tt )* )? @@ -124,6 +133,7 @@ macro_rules! impl_outer_origin { $caller_name; $runtime; $system; + system_index { $( $system_index )? }; Modules { $( $( $rest_module )* )? }; $( $parsed )* $module <$runtime> index { $( $index )? }, ); @@ -136,6 +146,7 @@ macro_rules! impl_outer_origin { $caller_name:ident; $runtime:ident; $system:ident; + system_index { $( $system_index:tt )? }; Modules { $( #[codec(index = $index:tt )] )? $module:ident $(, $( $rest_module:tt )* )? @@ -148,6 +159,7 @@ macro_rules! impl_outer_origin { $caller_name; $runtime; $system; + system_index { $( $system_index )? }; Modules { $( $( $rest_module )* )? }; $( $parsed )* $module index { $( $index )? }, ); @@ -160,6 +172,7 @@ macro_rules! impl_outer_origin { $caller_name:ident; $runtime:ident; $system:ident; + system_index { $( $system_index:tt )? }; Modules { }; $( $module:ident @@ -238,7 +251,7 @@ macro_rules! impl_outer_origin { $(#[$attr])* #[allow(non_camel_case_types)] pub enum $caller_name { - #[codec(index = "0")] + $( #[codec(index = $system_index)] )? system($system::Origin<$runtime>), $( $( #[codec(index = $index)] )? @@ -450,7 +463,7 @@ mod tests { ); impl_outer_origin!( - pub enum OriginIndices for TestRuntime { + pub enum OriginIndices for TestRuntime where system = frame_system, system_index = "11" { origin_with_generic, #[codec(index = "10")] origin_without_generic, } @@ -490,7 +503,7 @@ mod tests { #[test] fn test_codec() { use codec::Encode; - assert_eq!(OriginIndices::root().caller.encode()[0], 0); + assert_eq!(OriginIndices::root().caller.encode()[0], 11); let without_generic_variant = OriginIndicesCaller::origin_without_generic( origin_without_generic::Origin ); diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 7cf3e8feec00b..757877116aedf 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -136,13 +136,13 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic { - System: system::{Module, Call, Event, Origin}, + System: system::{Module, Call, Event, Origin} = 30, Module1_1: module1::::{Module, Call, Storage, Event, Origin}, Module2: module2::{Module, Call, Storage, Event, Origin}, Module1_2: module1::::{Module, Call, Storage, Event, Origin}, Module1_3: module1::::{Module, Storage} = 6, Module1_4: module1::::{Module, Call} = 3, - Module1_5: module1::::{Module, Event} = 2, + Module1_5: module1::::{Module, Event}, Module1_6: module1::::{Module, Call, Storage, Event, Origin} = 1, Module1_7: module1::::{Module, Call, Storage, Event, Origin}, Module1_8: module1::::{Module, Call, Storage, Event, Origin} = 12, @@ -158,15 +158,15 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic::Root.into()), - Err(DispatchError::Module { index: 4, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 31, error: 0, message: Some("Something") }), ); assert_eq!( Module2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 5, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 32, error: 0, message: Some("Something") }), ); assert_eq!( Module1_2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 7, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 33, error: 0, message: Some("Something") }), ); assert_eq!( Module1_3::fail(system::Origin::::Root.into()), @@ -178,7 +178,7 @@ fn check_modules_error_type() { ); assert_eq!( Module1_5::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 4, error: 0, message: Some("Something") }), ); assert_eq!( Module1_6::fail(system::Origin::::Root.into()), @@ -186,7 +186,7 @@ fn check_modules_error_type() { ); assert_eq!( Module1_7::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 8, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), ); assert_eq!( Module1_8::fail(system::Origin::::Root.into()), @@ -194,7 +194,7 @@ fn check_modules_error_type() { ); assert_eq!( Module1_9::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 9, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 13, error: 0, message: Some("Something") }), ); } @@ -207,42 +207,42 @@ fn integrity_test_works() { #[test] fn origin_codec() { use codec::Encode; - assert_eq!(OriginCaller::system(system::RawOrigin::None).encode()[0], 0); - assert_eq!(OriginCaller::module1_Instance1(module1::Origin(Default::default())).encode()[0], 4); - assert_eq!(OriginCaller::module2(module2::Origin).encode()[0], 5); - assert_eq!(OriginCaller::module1_Instance2(module1::Origin(Default::default())).encode()[0], 7); + assert_eq!(OriginCaller::system(system::RawOrigin::None).encode()[0], 30); + assert_eq!(OriginCaller::module1_Instance1(module1::Origin(Default::default())).encode()[0], 31); + assert_eq!(OriginCaller::module2(module2::Origin).encode()[0], 32); + assert_eq!(OriginCaller::module1_Instance2(module1::Origin(Default::default())).encode()[0], 33); assert_eq!(OriginCaller::module1_Instance6(module1::Origin(Default::default())).encode()[0], 1); - assert_eq!(OriginCaller::module1_Instance7(module1::Origin(Default::default())).encode()[0], 8); + assert_eq!(OriginCaller::module1_Instance7(module1::Origin(Default::default())).encode()[0], 2); assert_eq!(OriginCaller::module1_Instance8(module1::Origin(Default::default())).encode()[0], 12); - assert_eq!(OriginCaller::module1_Instance9(module1::Origin(Default::default())).encode()[0], 9); + assert_eq!(OriginCaller::module1_Instance9(module1::Origin(Default::default())).encode()[0], 13); } #[test] fn event_codec() { use codec::Encode; - assert_eq!(Event::system(system::Event::::ExtrinsicSuccess).encode()[0], 0); - assert_eq!(Event::module1_Instance1(module1::Event::::A(Default::default())).encode()[0], 4); - assert_eq!(Event::module2(module2::Event::A).encode()[0], 5); - assert_eq!(Event::module1_Instance2(module1::Event::::A(Default::default())).encode()[0], 7); - assert_eq!(Event::module1_Instance5(module1::Event::::A(Default::default())).encode()[0], 2); + assert_eq!(Event::system(system::Event::::ExtrinsicSuccess).encode()[0], 30); + assert_eq!(Event::module1_Instance1(module1::Event::::A(Default::default())).encode()[0], 31); + assert_eq!(Event::module2(module2::Event::A).encode()[0], 32); + assert_eq!(Event::module1_Instance2(module1::Event::::A(Default::default())).encode()[0], 33); + assert_eq!(Event::module1_Instance5(module1::Event::::A(Default::default())).encode()[0], 4); assert_eq!(Event::module1_Instance6(module1::Event::::A(Default::default())).encode()[0], 1); - assert_eq!(Event::module1_Instance7(module1::Event::::A(Default::default())).encode()[0], 8); + assert_eq!(Event::module1_Instance7(module1::Event::::A(Default::default())).encode()[0], 2); assert_eq!(Event::module1_Instance8(module1::Event::::A(Default::default())).encode()[0], 12); - assert_eq!(Event::module1_Instance9(module1::Event::::A(Default::default())).encode()[0], 9); + assert_eq!(Event::module1_Instance9(module1::Event::::A(Default::default())).encode()[0], 13); } #[test] fn call_codec() { use codec::Encode; - assert_eq!(Call::System(system::Call::noop()).encode()[0], 0); - assert_eq!(Call::Module1_1(module1::Call::fail()).encode()[0], 4); - assert_eq!(Call::Module2(module2::Call::fail()).encode()[0], 5); - assert_eq!(Call::Module1_2(module1::Call::fail()).encode()[0], 7); + assert_eq!(Call::System(system::Call::noop()).encode()[0], 30); + assert_eq!(Call::Module1_1(module1::Call::fail()).encode()[0], 31); + assert_eq!(Call::Module2(module2::Call::fail()).encode()[0], 32); + assert_eq!(Call::Module1_2(module1::Call::fail()).encode()[0], 33); assert_eq!(Call::Module1_4(module1::Call::fail()).encode()[0], 3); assert_eq!(Call::Module1_6(module1::Call::fail()).encode()[0], 1); - assert_eq!(Call::Module1_7(module1::Call::fail()).encode()[0], 8); + assert_eq!(Call::Module1_7(module1::Call::fail()).encode()[0], 2); assert_eq!(Call::Module1_8(module1::Call::fail()).encode()[0], 12); - assert_eq!(Call::Module1_9(module1::Call::fail()).encode()[0], 9); + assert_eq!(Call::Module1_9(module1::Call::fail()).encode()[0], 13); } #[test] @@ -277,7 +277,7 @@ fn test_metadata() { ]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: None, + index: 30, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_1"), @@ -299,7 +299,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: None, + index: 31, }, ModuleMetadata { name: DecodeDifferent::Encode("Module2"), @@ -323,7 +323,7 @@ fn test_metadata() { ]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: None, + index: 32, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_2"), @@ -343,7 +343,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: None, + index: 33, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_3"), @@ -355,7 +355,7 @@ fn test_metadata() { event: None, constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: Some(6), + index: 6, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_4"), @@ -368,7 +368,7 @@ fn test_metadata() { event: None, constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: Some(3), + index: 3, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_5"), @@ -381,7 +381,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: Some(2), + index: 4, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_6"), @@ -401,7 +401,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: Some(1), + index: 1, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_7"), @@ -421,7 +421,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: None, + index: 2, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_8"), @@ -441,7 +441,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: Some(12), + index: 12, }, ModuleMetadata { name: DecodeDifferent::Encode("Module1_9"), @@ -461,7 +461,7 @@ fn test_metadata() { }]))), constants: DecodeDifferent::Encode(FnEncode(|| &[])), errors: DecodeDifferent::Encode(FnEncode(|| &[])), - fixed_index: None, + index: 13, }, ]), extrinsic: ExtrinsicMetadata { From 0290c4776623f34ba1695ba9c769391b6b60e28d Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 8 Sep 2020 15:04:40 +0200 Subject: [PATCH 08/31] fix line length --- frame/support/test/tests/construct_runtime.rs | 68 ++++++++++++++----- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 757877116aedf..c0d2221a93370 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -207,28 +207,62 @@ fn integrity_test_works() { #[test] fn origin_codec() { use codec::Encode; - assert_eq!(OriginCaller::system(system::RawOrigin::None).encode()[0], 30); - assert_eq!(OriginCaller::module1_Instance1(module1::Origin(Default::default())).encode()[0], 31); - assert_eq!(OriginCaller::module2(module2::Origin).encode()[0], 32); - assert_eq!(OriginCaller::module1_Instance2(module1::Origin(Default::default())).encode()[0], 33); - assert_eq!(OriginCaller::module1_Instance6(module1::Origin(Default::default())).encode()[0], 1); - assert_eq!(OriginCaller::module1_Instance7(module1::Origin(Default::default())).encode()[0], 2); - assert_eq!(OriginCaller::module1_Instance8(module1::Origin(Default::default())).encode()[0], 12); - assert_eq!(OriginCaller::module1_Instance9(module1::Origin(Default::default())).encode()[0], 13); + + let origin = OriginCaller::system(system::RawOrigin::None); + assert_eq!(origin.encode()[0], 30); + + let origin = OriginCaller::module1_Instance1(module1::Origin(Default::default())); + assert_eq!(origin.encode()[0], 31); + + let origin = OriginCaller::module2(module2::Origin); + assert_eq!(origin.encode()[0], 32); + + let origin = OriginCaller::module1_Instance2(module1::Origin(Default::default())); + assert_eq!(origin.encode()[0], 33); + + let origin = OriginCaller::module1_Instance6(module1::Origin(Default::default())); + assert_eq!(origin.encode()[0], 1); + + let origin = OriginCaller::module1_Instance7(module1::Origin(Default::default())); + assert_eq!(origin.encode()[0], 2); + + let origin = OriginCaller::module1_Instance8(module1::Origin(Default::default())); + assert_eq!(origin.encode()[0], 12); + + let origin = OriginCaller::module1_Instance9(module1::Origin(Default::default())); + assert_eq!(origin.encode()[0], 13); } #[test] fn event_codec() { use codec::Encode; - assert_eq!(Event::system(system::Event::::ExtrinsicSuccess).encode()[0], 30); - assert_eq!(Event::module1_Instance1(module1::Event::::A(Default::default())).encode()[0], 31); - assert_eq!(Event::module2(module2::Event::A).encode()[0], 32); - assert_eq!(Event::module1_Instance2(module1::Event::::A(Default::default())).encode()[0], 33); - assert_eq!(Event::module1_Instance5(module1::Event::::A(Default::default())).encode()[0], 4); - assert_eq!(Event::module1_Instance6(module1::Event::::A(Default::default())).encode()[0], 1); - assert_eq!(Event::module1_Instance7(module1::Event::::A(Default::default())).encode()[0], 2); - assert_eq!(Event::module1_Instance8(module1::Event::::A(Default::default())).encode()[0], 12); - assert_eq!(Event::module1_Instance9(module1::Event::::A(Default::default())).encode()[0], 13); + + let event = system::Event::::ExtrinsicSuccess; + assert_eq!(Event::from(event).encode()[0], 30); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 31); + + let event = module2::Event::A; + assert_eq!(Event::from(event).encode()[0], 32); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 33); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 4); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 1); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 2); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 12); + + let event = module1::Event::::A(Default::default()); + assert_eq!(Event::from(event).encode()[0], 13); } #[test] From 6c8e80acaf1b90d92fba1b0a1a63e7e0df530426 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 9 Sep 2020 16:45:11 +0200 Subject: [PATCH 09/31] implement migration helper funciton in scheduler --- frame/scheduler/src/lib.rs | 136 ++++++++++++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 2 deletions(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 831ed64d438d7..1c7ca5613600b 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -438,6 +438,25 @@ impl Module { } } + /// Helper to migrate scheduler when the palelt origin type has changed. + pub fn migrate_origin + codec::Decode>() { + Agenda::::translate::< + Vec::Call, T::BlockNumber, OldOrigin, T::AccountId>>>, _ + >(|_, agenda| Some( + agenda + .into_iter() + .map(|schedule| schedule.map(|schedule| Scheduled { + maybe_id: schedule.maybe_id, + priority: schedule.priority, + call: schedule.call, + maybe_periodic: schedule.maybe_periodic, + origin: schedule.origin.into(), + _phantom: Default::default(), + })) + .collect::>() + )); + } + fn do_schedule( when: DispatchTime, maybe_periodic: Option>, @@ -620,6 +639,7 @@ mod tests { traits::{BlakeTwo256, IdentityLookup}, }; use frame_system::{EnsureOneOf, EnsureRoot, EnsureSignedBy}; + use substrate_test_utils::assert_eq_uvec; use crate as scheduler; mod logger { @@ -1147,8 +1167,6 @@ mod tests { #[test] fn migration_to_v2_works() { - use substrate_test_utils::assert_eq_uvec; - new_test_ext().execute_with(|| { for i in 0..3u64 { let k = i.twox_64_concat(); @@ -1250,4 +1268,118 @@ mod tests { assert_eq!(StorageVersion::get(), Releases::V2); }); } + + #[test] + fn test_migrate_origin() { + new_test_ext().execute_with(|| { + for i in 0..3u64 { + let k = i.twox_64_concat(); + let old: Vec>> = vec![ + Some(Scheduled { + maybe_id: None, + priority: i as u8 + 10, + call: Call::Logger(logger::Call::log(96, 100)), + origin: 3u32, + maybe_periodic: None, + _phantom: Default::default(), + }), + None, + Some(Scheduled { + maybe_id: Some(b"test".to_vec()), + priority: 123, + origin: 2u32, + call: Call::Logger(logger::Call::log(69, 1000)), + maybe_periodic: Some((456u64, 10)), + _phantom: Default::default(), + }), + ]; + frame_support::migration::put_storage_value( + b"Scheduler", + b"Agenda", + &k, + old, + ); + } + + impl Into for u32 { + fn into(self) -> OriginCaller { + match self { + 3u32 => system::RawOrigin::Root.into(), + 2u32 => system::RawOrigin::None.into(), + _ => unreachable!("test make no use of it"), + } + } + } + + Scheduler::migrate_origin::(); + + assert_eq_uvec!(Agenda::::iter().collect::>(), vec![ + ( + 0, + vec![ + Some(ScheduledV2::<_, _, OriginCaller, u64> { + maybe_id: None, + priority: 10, + call: Call::Logger(logger::Call::log(96, 100)), + maybe_periodic: None, + origin: system::RawOrigin::Root.into(), + _phantom: PhantomData::::default(), + }), + None, + Some(ScheduledV2 { + maybe_id: Some(b"test".to_vec()), + priority: 123, + call: Call::Logger(logger::Call::log(69, 1000)), + maybe_periodic: Some((456u64, 10)), + origin: system::RawOrigin::None.into(), + _phantom: PhantomData::::default(), + }), + ]), + ( + 1, + vec![ + Some(ScheduledV2 { + maybe_id: None, + priority: 11, + call: Call::Logger(logger::Call::log(96, 100)), + maybe_periodic: None, + origin: system::RawOrigin::Root.into(), + _phantom: PhantomData::::default(), + }), + None, + Some(ScheduledV2 { + maybe_id: Some(b"test".to_vec()), + priority: 123, + call: Call::Logger(logger::Call::log(69, 1000)), + maybe_periodic: Some((456u64, 10)), + origin: system::RawOrigin::None.into(), + _phantom: PhantomData::::default(), + }), + ] + ), + ( + 2, + vec![ + Some(ScheduledV2 { + maybe_id: None, + priority: 12, + call: Call::Logger(logger::Call::log(96, 100)), + maybe_periodic: None, + origin: system::RawOrigin::Root.into(), + _phantom: PhantomData::::default(), + }), + None, + Some(ScheduledV2 { + maybe_id: Some(b"test".to_vec()), + priority: 123, + call: Call::Logger(logger::Call::log(69, 1000)), + maybe_periodic: Some((456u64, 10)), + origin: system::RawOrigin::None.into(), + _phantom: PhantomData::::default(), + }), + ] + ) + ]); + }); + } } From 4e5b4f838b2790e3478b69d1ad65d8b1126a5830 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 9 Sep 2020 17:34:46 +0200 Subject: [PATCH 10/31] fix start at index 0 --- frame/support/procedural/src/construct_runtime/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 39e4f1692a796..bfc795aaf7009 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -447,22 +447,23 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { /// I.e. implicit are assigned number incrementedly from last explicit or 0. fn assign_implicit_index(modules: &mut Vec) -> syn::Result<()> { let mut indices = HashMap::new(); - let mut last_index: u8 = 0; + let mut last_index: Option = None; for module in modules { match module.index { Some(index) => { - last_index = index; + last_index = Some(index); }, None => { - last_index = match last_index.checked_add(1) { + let index = match last_index.map_or(Some(0), |i| i.checked_add(1)) { Some(i) => i, None => { let msg = "module index doesn't fit into u8, index is 256"; return Err(syn::Error::new(module.module.span(), msg)); }, }; - module.index = Some(last_index); + module.index = Some(index); + last_index = Some(index); }, } From 95a1eb754efb6638104707bd5bf4fcddf8fa9437 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Thu, 10 Sep 2020 11:41:46 +0200 Subject: [PATCH 11/31] Update frame/scheduler/src/lib.rs Co-authored-by: Shawn Tabrizi --- frame/scheduler/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 1c7ca5613600b..da08418bc2436 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -438,7 +438,7 @@ impl Module { } } - /// Helper to migrate scheduler when the palelt origin type has changed. + /// Helper to migrate scheduler when the pallet origin type has changed. pub fn migrate_origin + codec::Decode>() { Agenda::::translate::< Vec::Call, T::BlockNumber, OldOrigin, T::AccountId>>>, _ From a830e2f52fd1e50c1584b9a217e38d1df8a358f5 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Wed, 16 Sep 2020 18:20:09 +0200 Subject: [PATCH 12/31] Update frame/support/procedural/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/procedural/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index a963728964cfd..3bb2b831c1a7a 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -284,7 +284,7 @@ pub fn decl_storage(input: TokenStream) -> TokenStream { /// `= $n` is an optional part allowing to define at which index the module variants in /// `OriginCaller`, `Call` and `Event` are encoded, and to define the ModuleToIndex value. /// -/// if `= $n` is not given, then index is resolved same as fieldless enum in rust +/// if `= $n` is not given, then index is resolved same as fieldless enum in Rust /// (i.e. incrementedly from previous index): /// ```nocompile /// module1 .. = 2, From 9812da2684b8563b67d6beaf9fbb9755b758201b Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 17:42:38 +0200 Subject: [PATCH 13/31] bump frame-metadata crate --- Cargo.lock | 2 +- frame/metadata/Cargo.toml | 2 +- frame/support/Cargo.toml | 2 +- frame/support/test/Cargo.toml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b681637b9020f..9820c0d8a3244 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1620,7 +1620,7 @@ dependencies = [ [[package]] name = "frame-metadata" -version = "11.0.0-rc6" +version = "12.0.0-rc6" dependencies = [ "parity-scale-codec", "serde", diff --git a/frame/metadata/Cargo.toml b/frame/metadata/Cargo.toml index 7e2cb28f5e43d..cfda13c61d17f 100644 --- a/frame/metadata/Cargo.toml +++ b/frame/metadata/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "frame-metadata" -version = "11.0.0-rc6" +version = "12.0.0-rc6" authors = ["Parity Technologies "] edition = "2018" license = "Apache-2.0" diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 005638824b0ca..543ed7097c044 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"] log = "0.4" serde = { version = "1.0.101", optional = true, features = ["derive"] } codec = { package = "parity-scale-codec", version = "1.3.1", default-features = false, features = ["derive"] } -frame-metadata = { version = "11.0.0-rc6", default-features = false, path = "../metadata" } +frame-metadata = { version = "12.0.0-rc6", default-features = false, path = "../metadata" } sp-std = { version = "2.0.0-rc6", default-features = false, path = "../../primitives/std" } sp-io = { version = "2.0.0-rc6", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "2.0.0-rc6", default-features = false, path = "../../primitives/runtime" } diff --git a/frame/support/test/Cargo.toml b/frame/support/test/Cargo.toml index 804dbca285f12..2543ccdb5b201 100644 --- a/frame/support/test/Cargo.toml +++ b/frame/support/test/Cargo.toml @@ -24,7 +24,7 @@ sp-std = { version = "2.0.0-rc6", default-features = false, path = "../../../pri trybuild = "1.0.17" pretty_assertions = "0.6.1" rustversion = "1.0.0" -frame-metadata = { version = "11.0.0-rc6", default-features = false, path = "../../metadata" } +frame-metadata = { version = "12.0.0-rc6", default-features = false, path = "../../metadata" } [features] default = ["std"] From 5d86201cb3be194cc6da2547558f8c2bf3fa64bd Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 18:00:25 +0200 Subject: [PATCH 14/31] factorize --- frame/support/procedural/src/construct_runtime/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index bfc795aaf7009..aa1f267a7bf41 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -455,13 +455,11 @@ fn assign_implicit_index(modules: &mut Vec) -> syn::Result<() last_index = Some(index); }, None => { - let index = match last_index.map_or(Some(0), |i| i.checked_add(1)) { - Some(i) => i, - None => { + let index = last_index.map_or(Some(0), |i| i.checked_add(1)) + .ok_or_else(|| { let msg = "module index doesn't fit into u8, index is 256"; - return Err(syn::Error::new(module.module.span(), msg)); - }, - }; + syn::Error::new(module.module.span(), msg) + })?; module.index = Some(index); last_index = Some(index); }, From 078a543b705dc07936ec604037e45df3c7355d45 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 18:19:40 +0200 Subject: [PATCH 15/31] avoid some unwrap and remove nightly join --- .../procedural/src/construct_runtime/mod.rs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index aa1f267a7bf41..0fe7b75710af1 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -450,26 +450,26 @@ fn assign_implicit_index(modules: &mut Vec) -> syn::Result<() let mut last_index: Option = None; for module in modules { - match module.index { - Some(index) => { - last_index = Some(index); - }, - None => { - let index = last_index.map_or(Some(0), |i| i.checked_add(1)) - .ok_or_else(|| { - let msg = "module index doesn't fit into u8, index is 256"; - syn::Error::new(module.module.span(), msg) - })?; - module.index = Some(index); - last_index = Some(index); - }, - } - - if let Some(used_span) = indices.insert(module.index.unwrap(), module.module.span()) { - let span = module.module.span().join(used_span) - .expect("Modules are defined in same file"); - let msg = format!("module index are conflicting, at index {}", module.index.unwrap()); - return Err(syn::Error::new(span, msg)); + let final_index = match module.index { + Some(i) => i, + None => last_index.map_or(Some(0), |i| i.checked_add(1)) + .ok_or_else(|| { + let msg = "Module index doesn't fit into u8, index is 256"; + syn::Error::new(module.module.span(), msg) + })?, + }; + + module.index = Some(final_index); + last_index = Some(final_index); + + if let Some(used_module) = indices.insert(final_index, module.module.clone()) { + let msg = format!( + "Module index are conflicting, at index {}, modules {} and {}", + final_index, + module.module, + used_module, + ); + return Err(syn::Error::new(module.module.span(), msg)); } } From 4bba712287eb4434ae08b3acac2601774007c5ce Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Wed, 16 Sep 2020 18:26:24 +0200 Subject: [PATCH 16/31] Update frame/support/src/event.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/src/event.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frame/support/src/event.rs b/frame/support/src/event.rs index 9680063b2e5bc..e5e18c8566a58 100644 --- a/frame/support/src/event.rs +++ b/frame/support/src/event.rs @@ -816,5 +816,10 @@ mod tests { event_module2::Event::::TestEvent(3) ); assert_eq!(runtime_2_event_module_2.encode()[0], 5); + + let runtime_2_event_module_3 = TestEventSystemRenamed::event_module3( + event_module3::Event::::HiEvent + ); + assert_eq!(runtime_2_event_module_3.encode()[0], 6); } } From c6f073bfa0a41cdc3ecefc47a3b713d180dc3f4f Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 18:59:53 +0200 Subject: [PATCH 17/31] fix test --- frame/support/src/event.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/event.rs b/frame/support/src/event.rs index e5e18c8566a58..0f889f97f40a0 100644 --- a/frame/support/src/event.rs +++ b/frame/support/src/event.rs @@ -818,8 +818,8 @@ mod tests { assert_eq!(runtime_2_event_module_2.encode()[0], 5); let runtime_2_event_module_3 = TestEventSystemRenamed::event_module3( - event_module3::Event::::HiEvent + event_module3::Event::HiEvent ); - assert_eq!(runtime_2_event_module_3.encode()[0], 6); + assert_eq!(runtime_2_event_module_3.encode()[0], 3); } } From 43856fd0400236c02dcb78fb2dea0d1977df43df Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 19:12:35 +0200 Subject: [PATCH 18/31] add test and improve error message --- .../procedural/src/construct_runtime/mod.rs | 12 +- .../construct_runtime_ui/conflicting_index.rs | 14 + .../conflicting_index.stderr | 5 + .../conflicting_index_2.rs | 16 ++ .../conflicting_index_2.stderr | 5 + .../more_than_256_modules.rs | 269 ++++++++++++++++++ .../more_than_256_modules.stderr | 5 + 7 files changed, 320 insertions(+), 6 deletions(-) create mode 100644 frame/support/test/tests/construct_runtime_ui/conflicting_index.rs create mode 100644 frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr create mode 100644 frame/support/test/tests/construct_runtime_ui/conflicting_index_2.rs create mode 100644 frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr create mode 100644 frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs create mode 100644 frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 0fe7b75710af1..b2cc9b22fc818 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -455,21 +455,21 @@ fn assign_implicit_index(modules: &mut Vec) -> syn::Result<() None => last_index.map_or(Some(0), |i| i.checked_add(1)) .ok_or_else(|| { let msg = "Module index doesn't fit into u8, index is 256"; - syn::Error::new(module.module.span(), msg) + syn::Error::new(module.name.span(), msg) })?, }; module.index = Some(final_index); last_index = Some(final_index); - if let Some(used_module) = indices.insert(final_index, module.module.clone()) { + if let Some(used_module) = indices.insert(final_index, module.name.clone()) { let msg = format!( - "Module index are conflicting, at index {}, modules {} and {}", - final_index, - module.module, + "Module index are conflicting both modules {} and {} are at index {}", + module.name, used_module, + final_index, ); - return Err(syn::Error::new(module.module.span(), msg)); + return Err(syn::Error::new(module.name.span(), msg)); } } diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index.rs b/frame/support/test/tests/construct_runtime_ui/conflicting_index.rs new file mode 100644 index 0000000000000..a48b4bd097039 --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index.rs @@ -0,0 +1,14 @@ +use frame_support::construct_runtime; + +construct_runtime! { + pub enum Runtime where + UncheckedExtrinsic = UncheckedExtrinsic, + Block = Block, + NodeBlock = Block, + { + System: system::{}, + Pallet1: pallet1::{} = 0, + } +} + +fn main() {} diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr b/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr new file mode 100644 index 0000000000000..adb60a9bf1152 --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr @@ -0,0 +1,5 @@ +error: Module index are conflicting both modules Pallet1 and System are at index 0 + --> $DIR/conflicting_index.rs:10:3 + | +10 | Pallet1: pallet1::{} = 0, + | ^^^^^^^ diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.rs b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.rs new file mode 100644 index 0000000000000..c949cb41a23fa --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.rs @@ -0,0 +1,16 @@ +use frame_support::construct_runtime; + +construct_runtime! { + pub enum Runtime where + UncheckedExtrinsic = UncheckedExtrinsic, + Block = Block, + NodeBlock = Block, + { + System: system::{} = 5, + Pallet1: pallet1::{} = 3, + Pallet2: pallet2::{}, + Pallet3: pallet3::{}, + } +} + +fn main() {} diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr new file mode 100644 index 0000000000000..badcca970685b --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr @@ -0,0 +1,5 @@ +error: Module index are conflicting both modules Pallet3 and System are at index 5 + --> $DIR/conflicting_index_2.rs:12:3 + | +12 | Pallet3: pallet3::{}, + | ^^^^^^^ diff --git a/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs new file mode 100644 index 0000000000000..538623c78b603 --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs @@ -0,0 +1,269 @@ +use frame_support::construct_runtime; + +construct_runtime! { + pub enum Runtime where + UncheckedExtrinsic = UncheckedExtrinsic, + Block = Block, + NodeBlock = Block, + { + System: system::{}, + Pallet1: pallet1::{}, + Pallet2: pallet2::{}, + Pallet3: pallet3::{}, + Pallet4: pallet4::{}, + Pallet5: pallet5::{}, + Pallet6: pallet6::{}, + Pallet7: pallet7::{}, + Pallet8: pallet8::{}, + Pallet9: pallet9::{}, + Pallet10: pallet10::{}, + Pallet11: pallet11::{}, + Pallet12: pallet12::{}, + Pallet13: pallet13::{}, + Pallet14: pallet14::{}, + Pallet15: pallet15::{}, + Pallet16: pallet16::{}, + Pallet17: pallet17::{}, + Pallet18: pallet18::{}, + Pallet19: pallet19::{}, + Pallet20: pallet20::{}, + Pallet21: pallet21::{}, + Pallet22: pallet22::{}, + Pallet23: pallet23::{}, + Pallet24: pallet24::{}, + Pallet25: pallet25::{}, + Pallet26: pallet26::{}, + Pallet27: pallet27::{}, + Pallet28: pallet28::{}, + Pallet29: pallet29::{}, + Pallet30: pallet30::{}, + Pallet31: pallet31::{}, + Pallet32: pallet32::{}, + Pallet33: pallet33::{}, + Pallet34: pallet34::{}, + Pallet35: pallet35::{}, + Pallet36: pallet36::{}, + Pallet37: pallet37::{}, + Pallet38: pallet38::{}, + Pallet39: pallet39::{}, + Pallet40: pallet40::{}, + Pallet41: pallet41::{}, + Pallet42: pallet42::{}, + Pallet43: pallet43::{}, + Pallet44: pallet44::{}, + Pallet45: pallet45::{}, + Pallet46: pallet46::{}, + Pallet47: pallet47::{}, + Pallet48: pallet48::{}, + Pallet49: pallet49::{}, + Pallet50: pallet50::{}, + Pallet51: pallet51::{}, + Pallet52: pallet52::{}, + Pallet53: pallet53::{}, + Pallet54: pallet54::{}, + Pallet55: pallet55::{}, + Pallet56: pallet56::{}, + Pallet57: pallet57::{}, + Pallet58: pallet58::{}, + Pallet59: pallet59::{}, + Pallet60: pallet60::{}, + Pallet61: pallet61::{}, + Pallet62: pallet62::{}, + Pallet63: pallet63::{}, + Pallet64: pallet64::{}, + Pallet65: pallet65::{}, + Pallet66: pallet66::{}, + Pallet67: pallet67::{}, + Pallet68: pallet68::{}, + Pallet69: pallet69::{}, + Pallet70: pallet70::{}, + Pallet71: pallet71::{}, + Pallet72: pallet72::{}, + Pallet73: pallet73::{}, + Pallet74: pallet74::{}, + Pallet75: pallet75::{}, + Pallet76: pallet76::{}, + Pallet77: pallet77::{}, + Pallet78: pallet78::{}, + Pallet79: pallet79::{}, + Pallet80: pallet80::{}, + Pallet81: pallet81::{}, + Pallet82: pallet82::{}, + Pallet83: pallet83::{}, + Pallet84: pallet84::{}, + Pallet85: pallet85::{}, + Pallet86: pallet86::{}, + Pallet87: pallet87::{}, + Pallet88: pallet88::{}, + Pallet89: pallet89::{}, + Pallet90: pallet90::{}, + Pallet91: pallet91::{}, + Pallet92: pallet92::{}, + Pallet93: pallet93::{}, + Pallet94: pallet94::{}, + Pallet95: pallet95::{}, + Pallet96: pallet96::{}, + Pallet97: pallet97::{}, + Pallet98: pallet98::{}, + Pallet99: pallet99::{}, + Pallet100: pallet100::{}, + Pallet101: pallet101::{}, + Pallet102: pallet102::{}, + Pallet103: pallet103::{}, + Pallet104: pallet104::{}, + Pallet105: pallet105::{}, + Pallet106: pallet106::{}, + Pallet107: pallet107::{}, + Pallet108: pallet108::{}, + Pallet109: pallet109::{}, + Pallet110: pallet110::{}, + Pallet111: pallet111::{}, + Pallet112: pallet112::{}, + Pallet113: pallet113::{}, + Pallet114: pallet114::{}, + Pallet115: pallet115::{}, + Pallet116: pallet116::{}, + Pallet117: pallet117::{}, + Pallet118: pallet118::{}, + Pallet119: pallet119::{}, + Pallet120: pallet120::{}, + Pallet121: pallet121::{}, + Pallet122: pallet122::{}, + Pallet123: pallet123::{}, + Pallet124: pallet124::{}, + Pallet125: pallet125::{}, + Pallet126: pallet126::{}, + Pallet127: pallet127::{}, + Pallet128: pallet128::{}, + Pallet129: pallet129::{}, + Pallet130: pallet130::{}, + Pallet131: pallet131::{}, + Pallet132: pallet132::{}, + Pallet133: pallet133::{}, + Pallet134: pallet134::{}, + Pallet135: pallet135::{}, + Pallet136: pallet136::{}, + Pallet137: pallet137::{}, + Pallet138: pallet138::{}, + Pallet139: pallet139::{}, + Pallet140: pallet140::{}, + Pallet141: pallet141::{}, + Pallet142: pallet142::{}, + Pallet143: pallet143::{}, + Pallet144: pallet144::{}, + Pallet145: pallet145::{}, + Pallet146: pallet146::{}, + Pallet147: pallet147::{}, + Pallet148: pallet148::{}, + Pallet149: pallet149::{}, + Pallet150: pallet150::{}, + Pallet151: pallet151::{}, + Pallet152: pallet152::{}, + Pallet153: pallet153::{}, + Pallet154: pallet154::{}, + Pallet155: pallet155::{}, + Pallet156: pallet156::{}, + Pallet157: pallet157::{}, + Pallet158: pallet158::{}, + Pallet159: pallet159::{}, + Pallet160: pallet160::{}, + Pallet161: pallet161::{}, + Pallet162: pallet162::{}, + Pallet163: pallet163::{}, + Pallet164: pallet164::{}, + Pallet165: pallet165::{}, + Pallet166: pallet166::{}, + Pallet167: pallet167::{}, + Pallet168: pallet168::{}, + Pallet169: pallet169::{}, + Pallet170: pallet170::{}, + Pallet171: pallet171::{}, + Pallet172: pallet172::{}, + Pallet173: pallet173::{}, + Pallet174: pallet174::{}, + Pallet175: pallet175::{}, + Pallet176: pallet176::{}, + Pallet177: pallet177::{}, + Pallet178: pallet178::{}, + Pallet179: pallet179::{}, + Pallet180: pallet180::{}, + Pallet181: pallet181::{}, + Pallet182: pallet182::{}, + Pallet183: pallet183::{}, + Pallet184: pallet184::{}, + Pallet185: pallet185::{}, + Pallet186: pallet186::{}, + Pallet187: pallet187::{}, + Pallet188: pallet188::{}, + Pallet189: pallet189::{}, + Pallet190: pallet190::{}, + Pallet191: pallet191::{}, + Pallet192: pallet192::{}, + Pallet193: pallet193::{}, + Pallet194: pallet194::{}, + Pallet195: pallet195::{}, + Pallet196: pallet196::{}, + Pallet197: pallet197::{}, + Pallet198: pallet198::{}, + Pallet199: pallet199::{}, + Pallet200: pallet200::{}, + Pallet201: pallet201::{}, + Pallet202: pallet202::{}, + Pallet203: pallet203::{}, + Pallet204: pallet204::{}, + Pallet205: pallet205::{}, + Pallet206: pallet206::{}, + Pallet207: pallet207::{}, + Pallet208: pallet208::{}, + Pallet209: pallet209::{}, + Pallet210: pallet210::{}, + Pallet211: pallet211::{}, + Pallet212: pallet212::{}, + Pallet213: pallet213::{}, + Pallet214: pallet214::{}, + Pallet215: pallet215::{}, + Pallet216: pallet216::{}, + Pallet217: pallet217::{}, + Pallet218: pallet218::{}, + Pallet219: pallet219::{}, + Pallet220: pallet220::{}, + Pallet221: pallet221::{}, + Pallet222: pallet222::{}, + Pallet223: pallet223::{}, + Pallet224: pallet224::{}, + Pallet225: pallet225::{}, + Pallet226: pallet226::{}, + Pallet227: pallet227::{}, + Pallet228: pallet228::{}, + Pallet229: pallet229::{}, + Pallet230: pallet230::{}, + Pallet231: pallet231::{}, + Pallet232: pallet232::{}, + Pallet233: pallet233::{}, + Pallet234: pallet234::{}, + Pallet235: pallet235::{}, + Pallet236: pallet236::{}, + Pallet237: pallet237::{}, + Pallet238: pallet238::{}, + Pallet239: pallet239::{}, + Pallet240: pallet240::{}, + Pallet241: pallet241::{}, + Pallet242: pallet242::{}, + Pallet243: pallet243::{}, + Pallet244: pallet244::{}, + Pallet245: pallet245::{}, + Pallet246: pallet246::{}, + Pallet247: pallet247::{}, + Pallet248: pallet248::{}, + Pallet249: pallet249::{}, + Pallet250: pallet250::{}, + Pallet251: pallet251::{}, + Pallet252: pallet252::{}, + Pallet253: pallet253::{}, + Pallet254: pallet254::{}, + Pallet255: pallet255::{}, + Pallet256: pallet256::{}, + } +} + +fn main() {} diff --git a/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr new file mode 100644 index 0000000000000..8b442a2b9b060 --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr @@ -0,0 +1,5 @@ +error: Module index doesn't fit into u8, index is 256 + --> $DIR/more_than_256_modules.rs:265:3 + | +265 | Pallet256: pallet256::{}, + | ^^^^^^^^^ From 7da2738fcd2e9ee3bdb862032df2e38fc499ae59 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 19:45:44 +0200 Subject: [PATCH 19/31] factorize test --- .../more_than_256_modules.rs | 257 +----------------- .../more_than_256_modules.stderr | 8 +- 2 files changed, 5 insertions(+), 260 deletions(-) diff --git a/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs index 538623c78b603..4c8331ae442c8 100644 --- a/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs +++ b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.rs @@ -6,262 +6,7 @@ construct_runtime! { Block = Block, NodeBlock = Block, { - System: system::{}, - Pallet1: pallet1::{}, - Pallet2: pallet2::{}, - Pallet3: pallet3::{}, - Pallet4: pallet4::{}, - Pallet5: pallet5::{}, - Pallet6: pallet6::{}, - Pallet7: pallet7::{}, - Pallet8: pallet8::{}, - Pallet9: pallet9::{}, - Pallet10: pallet10::{}, - Pallet11: pallet11::{}, - Pallet12: pallet12::{}, - Pallet13: pallet13::{}, - Pallet14: pallet14::{}, - Pallet15: pallet15::{}, - Pallet16: pallet16::{}, - Pallet17: pallet17::{}, - Pallet18: pallet18::{}, - Pallet19: pallet19::{}, - Pallet20: pallet20::{}, - Pallet21: pallet21::{}, - Pallet22: pallet22::{}, - Pallet23: pallet23::{}, - Pallet24: pallet24::{}, - Pallet25: pallet25::{}, - Pallet26: pallet26::{}, - Pallet27: pallet27::{}, - Pallet28: pallet28::{}, - Pallet29: pallet29::{}, - Pallet30: pallet30::{}, - Pallet31: pallet31::{}, - Pallet32: pallet32::{}, - Pallet33: pallet33::{}, - Pallet34: pallet34::{}, - Pallet35: pallet35::{}, - Pallet36: pallet36::{}, - Pallet37: pallet37::{}, - Pallet38: pallet38::{}, - Pallet39: pallet39::{}, - Pallet40: pallet40::{}, - Pallet41: pallet41::{}, - Pallet42: pallet42::{}, - Pallet43: pallet43::{}, - Pallet44: pallet44::{}, - Pallet45: pallet45::{}, - Pallet46: pallet46::{}, - Pallet47: pallet47::{}, - Pallet48: pallet48::{}, - Pallet49: pallet49::{}, - Pallet50: pallet50::{}, - Pallet51: pallet51::{}, - Pallet52: pallet52::{}, - Pallet53: pallet53::{}, - Pallet54: pallet54::{}, - Pallet55: pallet55::{}, - Pallet56: pallet56::{}, - Pallet57: pallet57::{}, - Pallet58: pallet58::{}, - Pallet59: pallet59::{}, - Pallet60: pallet60::{}, - Pallet61: pallet61::{}, - Pallet62: pallet62::{}, - Pallet63: pallet63::{}, - Pallet64: pallet64::{}, - Pallet65: pallet65::{}, - Pallet66: pallet66::{}, - Pallet67: pallet67::{}, - Pallet68: pallet68::{}, - Pallet69: pallet69::{}, - Pallet70: pallet70::{}, - Pallet71: pallet71::{}, - Pallet72: pallet72::{}, - Pallet73: pallet73::{}, - Pallet74: pallet74::{}, - Pallet75: pallet75::{}, - Pallet76: pallet76::{}, - Pallet77: pallet77::{}, - Pallet78: pallet78::{}, - Pallet79: pallet79::{}, - Pallet80: pallet80::{}, - Pallet81: pallet81::{}, - Pallet82: pallet82::{}, - Pallet83: pallet83::{}, - Pallet84: pallet84::{}, - Pallet85: pallet85::{}, - Pallet86: pallet86::{}, - Pallet87: pallet87::{}, - Pallet88: pallet88::{}, - Pallet89: pallet89::{}, - Pallet90: pallet90::{}, - Pallet91: pallet91::{}, - Pallet92: pallet92::{}, - Pallet93: pallet93::{}, - Pallet94: pallet94::{}, - Pallet95: pallet95::{}, - Pallet96: pallet96::{}, - Pallet97: pallet97::{}, - Pallet98: pallet98::{}, - Pallet99: pallet99::{}, - Pallet100: pallet100::{}, - Pallet101: pallet101::{}, - Pallet102: pallet102::{}, - Pallet103: pallet103::{}, - Pallet104: pallet104::{}, - Pallet105: pallet105::{}, - Pallet106: pallet106::{}, - Pallet107: pallet107::{}, - Pallet108: pallet108::{}, - Pallet109: pallet109::{}, - Pallet110: pallet110::{}, - Pallet111: pallet111::{}, - Pallet112: pallet112::{}, - Pallet113: pallet113::{}, - Pallet114: pallet114::{}, - Pallet115: pallet115::{}, - Pallet116: pallet116::{}, - Pallet117: pallet117::{}, - Pallet118: pallet118::{}, - Pallet119: pallet119::{}, - Pallet120: pallet120::{}, - Pallet121: pallet121::{}, - Pallet122: pallet122::{}, - Pallet123: pallet123::{}, - Pallet124: pallet124::{}, - Pallet125: pallet125::{}, - Pallet126: pallet126::{}, - Pallet127: pallet127::{}, - Pallet128: pallet128::{}, - Pallet129: pallet129::{}, - Pallet130: pallet130::{}, - Pallet131: pallet131::{}, - Pallet132: pallet132::{}, - Pallet133: pallet133::{}, - Pallet134: pallet134::{}, - Pallet135: pallet135::{}, - Pallet136: pallet136::{}, - Pallet137: pallet137::{}, - Pallet138: pallet138::{}, - Pallet139: pallet139::{}, - Pallet140: pallet140::{}, - Pallet141: pallet141::{}, - Pallet142: pallet142::{}, - Pallet143: pallet143::{}, - Pallet144: pallet144::{}, - Pallet145: pallet145::{}, - Pallet146: pallet146::{}, - Pallet147: pallet147::{}, - Pallet148: pallet148::{}, - Pallet149: pallet149::{}, - Pallet150: pallet150::{}, - Pallet151: pallet151::{}, - Pallet152: pallet152::{}, - Pallet153: pallet153::{}, - Pallet154: pallet154::{}, - Pallet155: pallet155::{}, - Pallet156: pallet156::{}, - Pallet157: pallet157::{}, - Pallet158: pallet158::{}, - Pallet159: pallet159::{}, - Pallet160: pallet160::{}, - Pallet161: pallet161::{}, - Pallet162: pallet162::{}, - Pallet163: pallet163::{}, - Pallet164: pallet164::{}, - Pallet165: pallet165::{}, - Pallet166: pallet166::{}, - Pallet167: pallet167::{}, - Pallet168: pallet168::{}, - Pallet169: pallet169::{}, - Pallet170: pallet170::{}, - Pallet171: pallet171::{}, - Pallet172: pallet172::{}, - Pallet173: pallet173::{}, - Pallet174: pallet174::{}, - Pallet175: pallet175::{}, - Pallet176: pallet176::{}, - Pallet177: pallet177::{}, - Pallet178: pallet178::{}, - Pallet179: pallet179::{}, - Pallet180: pallet180::{}, - Pallet181: pallet181::{}, - Pallet182: pallet182::{}, - Pallet183: pallet183::{}, - Pallet184: pallet184::{}, - Pallet185: pallet185::{}, - Pallet186: pallet186::{}, - Pallet187: pallet187::{}, - Pallet188: pallet188::{}, - Pallet189: pallet189::{}, - Pallet190: pallet190::{}, - Pallet191: pallet191::{}, - Pallet192: pallet192::{}, - Pallet193: pallet193::{}, - Pallet194: pallet194::{}, - Pallet195: pallet195::{}, - Pallet196: pallet196::{}, - Pallet197: pallet197::{}, - Pallet198: pallet198::{}, - Pallet199: pallet199::{}, - Pallet200: pallet200::{}, - Pallet201: pallet201::{}, - Pallet202: pallet202::{}, - Pallet203: pallet203::{}, - Pallet204: pallet204::{}, - Pallet205: pallet205::{}, - Pallet206: pallet206::{}, - Pallet207: pallet207::{}, - Pallet208: pallet208::{}, - Pallet209: pallet209::{}, - Pallet210: pallet210::{}, - Pallet211: pallet211::{}, - Pallet212: pallet212::{}, - Pallet213: pallet213::{}, - Pallet214: pallet214::{}, - Pallet215: pallet215::{}, - Pallet216: pallet216::{}, - Pallet217: pallet217::{}, - Pallet218: pallet218::{}, - Pallet219: pallet219::{}, - Pallet220: pallet220::{}, - Pallet221: pallet221::{}, - Pallet222: pallet222::{}, - Pallet223: pallet223::{}, - Pallet224: pallet224::{}, - Pallet225: pallet225::{}, - Pallet226: pallet226::{}, - Pallet227: pallet227::{}, - Pallet228: pallet228::{}, - Pallet229: pallet229::{}, - Pallet230: pallet230::{}, - Pallet231: pallet231::{}, - Pallet232: pallet232::{}, - Pallet233: pallet233::{}, - Pallet234: pallet234::{}, - Pallet235: pallet235::{}, - Pallet236: pallet236::{}, - Pallet237: pallet237::{}, - Pallet238: pallet238::{}, - Pallet239: pallet239::{}, - Pallet240: pallet240::{}, - Pallet241: pallet241::{}, - Pallet242: pallet242::{}, - Pallet243: pallet243::{}, - Pallet244: pallet244::{}, - Pallet245: pallet245::{}, - Pallet246: pallet246::{}, - Pallet247: pallet247::{}, - Pallet248: pallet248::{}, - Pallet249: pallet249::{}, - Pallet250: pallet250::{}, - Pallet251: pallet251::{}, - Pallet252: pallet252::{}, - Pallet253: pallet253::{}, - Pallet254: pallet254::{}, - Pallet255: pallet255::{}, + System: system::{} = 255, Pallet256: pallet256::{}, } } diff --git a/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr index 8b442a2b9b060..c0ef5c8e60b9e 100644 --- a/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr +++ b/frame/support/test/tests/construct_runtime_ui/more_than_256_modules.stderr @@ -1,5 +1,5 @@ error: Module index doesn't fit into u8, index is 256 - --> $DIR/more_than_256_modules.rs:265:3 - | -265 | Pallet256: pallet256::{}, - | ^^^^^^^^^ + --> $DIR/more_than_256_modules.rs:10:3 + | +10 | Pallet256: pallet256::{}, + | ^^^^^^^^^ From b9fca7e2d5614f80e181359dc1d908788861dddc Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 22:09:39 +0200 Subject: [PATCH 20/31] keep iterator, and use slice instead of vec --- .../procedural/src/construct_runtime/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index b2cc9b22fc818..6a2e0caa843ae 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -76,20 +76,20 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result( fn decl_outer_dispatch<'a>( runtime: &'a Ident, - module_declarations: &Vec, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> TokenStream2 { - let modules_tokens = module_declarations.iter() + let modules_tokens = module_declarations .filter(|module_declaration| module_declaration.exists_part("Call")) .map(|module_declaration| { let module = &module_declaration.module; @@ -284,7 +284,7 @@ fn decl_outer_dispatch<'a>( fn decl_outer_origin<'a>( runtime_name: &'a Ident, - modules_except_system: &Vec, + modules_except_system: impl Iterator, system_module: &'a ModuleDeclaration, scrate: &'a TokenStream2, ) -> syn::Result { @@ -330,7 +330,7 @@ fn decl_outer_origin<'a>( fn decl_outer_event<'a>( runtime_name: &'a Ident, - module_declarations: &Vec, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); @@ -402,7 +402,7 @@ fn decl_all_modules<'a>( } fn decl_module_to_index<'a>( - module_declarations: &Vec, + module_declarations: &[ModuleDeclaration], scrate: &TokenStream2, ) -> TokenStream2 { let names = module_declarations.iter().map(|d| &d.name); @@ -445,7 +445,7 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { /// Assign index to each modules using same rules as rust for fieldless enum. /// I.e. implicit are assigned number incrementedly from last explicit or 0. -fn assign_implicit_index(modules: &mut Vec) -> syn::Result<()> { +fn assign_implicit_index(modules: &mut [ModuleDeclaration]) -> syn::Result<()> { let mut indices = HashMap::new(); let mut last_index: Option = None; From 61019e704f211ef400db9262d99fbf635dba7997 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 22:28:17 +0200 Subject: [PATCH 21/31] refactor to avoid to have expects --- .../procedural/src/construct_runtime/mod.rs | 140 +++++++++++------- .../procedural/src/construct_runtime/parse.rs | 15 -- 2 files changed, 86 insertions(+), 69 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 6a2e0caa843ae..4b853a53c2f22 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -19,7 +19,7 @@ mod parse; use frame_support_procedural_tools::syn_ext as ext; use frame_support_procedural_tools::{generate_crate_access, generate_hidden_includes}; -use parse::{ModuleDeclaration, RuntimeDefinition, WhereSection}; +use parse::{ModuleDeclaration, RuntimeDefinition, WhereSection, ModulePart}; use proc_macro::TokenStream; use proc_macro2::{TokenStream as TokenStream2, Span}; use quote::quote; @@ -29,6 +29,74 @@ use std::collections::HashMap; /// The fixed name of the system module. const SYSTEM_MODULE_NAME: &str = "System"; +/// The complete definition of a module with the resulting fixed index. +#[derive(Debug, Clone)] +pub struct Module { + pub name: Ident, + pub index: u8, + pub module: Ident, + pub instance: Option, + pub module_parts: Vec, +} + +impl Module { + /// Get resolved module parts + fn module_parts(&self) -> &[ModulePart] { + &self.module_parts + } + + /// Find matching parts + fn find_part(&self, name: &str) -> Option<&ModulePart> { + self.module_parts.iter().find(|part| part.name() == name) + } + + /// Return whether module contains part + fn exists_part(&self, name: &str) -> bool { + self.find_part(name).is_some() + } +} + +/// Convert from the parsed module to their final information. +/// Assign index to each modules using same rules as rust for fieldless enum. +/// I.e. implicit are assigned number incrementedly from last explicit or 0. +fn complete_modules(decl: impl Iterator) -> syn::Result> { + let mut indices = HashMap::new(); + let mut last_index: Option = None; + + decl + .map(|module| { + let final_index = match module.index { + Some(i) => i, + None => last_index.map_or(Some(0), |i| i.checked_add(1)) + .ok_or_else(|| { + let msg = "Module index doesn't fit into u8, index is 256"; + syn::Error::new(module.name.span(), msg) + })?, + }; + + last_index = Some(final_index); + + if let Some(used_module) = indices.insert(final_index, module.name.clone()) { + let msg = format!( + "Module index are conflicting both modules {} and {} are at index {}", + module.name, + used_module, + final_index, + ); + return Err(syn::Error::new(module.name.span(), msg)); + } + + Ok(Module { + name: module.name, + index: final_index, + module: module.module, + instance: module.instance, + module_parts: module.module_parts, + }) + }) + .collect() +} + pub fn construct_runtime(input: TokenStream) -> TokenStream { let definition = syn::parse_macro_input!(input as RuntimeDefinition); construct_runtime_parsed(definition) @@ -53,10 +121,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result>(); - - // Automatically assign implicit index: - assign_implicit_index(&mut modules)?; + let modules = complete_modules(modules.into_iter())?; let system_module = modules.iter() .find(|decl| decl.name == SYSTEM_MODULE_NAME) @@ -139,7 +204,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result( runtime: &'a Ident, - module_declarations: impl Iterator, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> TokenStream2 { let modules_tokens = module_declarations @@ -157,7 +222,7 @@ fn decl_validate_unsigned<'a>( fn decl_outer_inherent<'a>( block: &'a syn::TypePath, unchecked_extrinsic: &'a syn::TypePath, - module_declarations: impl Iterator, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> TokenStream2 { let modules_tokens = module_declarations.filter_map(|module_declaration| { @@ -181,7 +246,7 @@ fn decl_outer_inherent<'a>( fn decl_outer_config<'a>( runtime: &'a Ident, - module_declarations: impl Iterator, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> TokenStream2 { let modules_tokens = module_declarations @@ -219,7 +284,7 @@ fn decl_outer_config<'a>( fn decl_runtime_metadata<'a>( runtime: &'a Ident, - module_declarations: impl Iterator, + module_declarations: impl Iterator, scrate: &'a TokenStream2, extrinsic: &TypePath, ) -> TokenStream2 { @@ -244,7 +309,7 @@ fn decl_runtime_metadata<'a>( .map(|name| quote!(<#name>)) .into_iter(); - let index = module_declaration.index.expect("All index have been assigned"); + let index = module_declaration.index; quote!( #module::Module #(#instance)* as #name { index #index } with #(#filtered_names)*, @@ -260,7 +325,7 @@ fn decl_runtime_metadata<'a>( fn decl_outer_dispatch<'a>( runtime: &'a Ident, - module_declarations: impl Iterator, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> TokenStream2 { let modules_tokens = module_declarations @@ -268,7 +333,7 @@ fn decl_outer_dispatch<'a>( .map(|module_declaration| { let module = &module_declaration.module; let name = &module_declaration.name; - let index = module_declaration.index.expect("All index have been assigned"); + let index = module_declaration.index; let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); quote!(#[codec(index = #index_string)] #module::#name) }); @@ -284,8 +349,8 @@ fn decl_outer_dispatch<'a>( fn decl_outer_origin<'a>( runtime_name: &'a Ident, - modules_except_system: impl Iterator, - system_module: &'a ModuleDeclaration, + modules_except_system: impl Iterator, + system_module: &'a Module, scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); @@ -303,7 +368,7 @@ fn decl_outer_origin<'a>( ); return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let index = module_declaration.index.expect("All index have been assigned"); + let index = module_declaration.index; let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); modules_tokens.extend(tokens); @@ -313,7 +378,7 @@ fn decl_outer_origin<'a>( } let system_name = &system_module.module; - let system_index = system_module.index.expect("All index have been assigned"); + let system_index = system_module.index; let system_index_string = syn::LitStr::new(&format!("{}", system_index), Span::call_site()); Ok(quote!( @@ -330,7 +395,7 @@ fn decl_outer_origin<'a>( fn decl_outer_event<'a>( runtime_name: &'a Ident, - module_declarations: impl Iterator, + module_declarations: impl Iterator, scrate: &'a TokenStream2, ) -> syn::Result { let mut modules_tokens = TokenStream2::new(); @@ -349,7 +414,7 @@ fn decl_outer_event<'a>( return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let index = module_declaration.index.expect("All index have been assigned"); + let index = module_declaration.index; let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); modules_tokens.extend(tokens); @@ -369,7 +434,7 @@ fn decl_outer_event<'a>( fn decl_all_modules<'a>( runtime: &'a Ident, - module_declarations: impl Iterator, + module_declarations: impl Iterator, ) -> TokenStream2 { let mut types = TokenStream2::new(); let mut names = Vec::new(); @@ -402,12 +467,12 @@ fn decl_all_modules<'a>( } fn decl_module_to_index<'a>( - module_declarations: &[ModuleDeclaration], + module_declarations: &[Module], scrate: &TokenStream2, ) -> TokenStream2 { let names = module_declarations.iter().map(|d| &d.name); let indices = module_declarations.iter() - .map(|module| module.index.expect("All index have been assigned") as usize); + .map(|module| module.index as usize); quote!( /// Provides an implementation of `ModuleToIndex` to map a module @@ -442,36 +507,3 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { } ) } - -/// Assign index to each modules using same rules as rust for fieldless enum. -/// I.e. implicit are assigned number incrementedly from last explicit or 0. -fn assign_implicit_index(modules: &mut [ModuleDeclaration]) -> syn::Result<()> { - let mut indices = HashMap::new(); - let mut last_index: Option = None; - - for module in modules { - let final_index = match module.index { - Some(i) => i, - None => last_index.map_or(Some(0), |i| i.checked_add(1)) - .ok_or_else(|| { - let msg = "Module index doesn't fit into u8, index is 256"; - syn::Error::new(module.name.span(), msg) - })?, - }; - - module.index = Some(final_index); - last_index = Some(final_index); - - if let Some(used_module) = indices.insert(final_index, module.name.clone()) { - let msg = format!( - "Module index are conflicting both modules {} and {} are at index {}", - module.name, - used_module, - final_index, - ); - return Err(syn::Error::new(module.name.span(), msg)); - } - } - - Ok(()) -} diff --git a/frame/support/procedural/src/construct_runtime/parse.rs b/frame/support/procedural/src/construct_runtime/parse.rs index 639451a65ca00..4a45044d67f25 100644 --- a/frame/support/procedural/src/construct_runtime/parse.rs +++ b/frame/support/procedural/src/construct_runtime/parse.rs @@ -198,21 +198,6 @@ impl Parse for ModuleDeclaration { } } -impl ModuleDeclaration { - /// Get resolved module parts - pub fn module_parts(&self) -> &[ModulePart] { - &self.module_parts - } - - pub fn find_part(&self, name: &str) -> Option<&ModulePart> { - self.module_parts.iter().find(|part| part.name() == name) - } - - pub fn exists_part(&self, name: &str) -> bool { - self.find_part(name).is_some() - } -} - /// Parse [`ModulePart`]'s from a braces enclosed list that is split by commas, e.g. /// /// `{ Call, Event }` From ae0f85944a95213d7da210c4b217dab41a582110 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 16 Sep 2020 22:36:58 +0200 Subject: [PATCH 22/31] small refactor --- frame/support/procedural/src/construct_runtime/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 4b853a53c2f22..f5cc8ff675535 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -135,9 +135,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result>(); + let all_but_system_modules = modules.iter().filter(|module| module.name != SYSTEM_MODULE_NAME); let outer_event = decl_outer_event( &name, @@ -147,7 +145,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result Date: Thu, 17 Sep 2020 22:54:18 +0200 Subject: [PATCH 23/31] Test something --- .maintain/gitlab/check_polkadot_companion_build.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.maintain/gitlab/check_polkadot_companion_build.sh b/.maintain/gitlab/check_polkadot_companion_build.sh index 2ee1e824aed5d..03454c40338fc 100755 --- a/.maintain/gitlab/check_polkadot_companion_build.sh +++ b/.maintain/gitlab/check_polkadot_companion_build.sh @@ -88,6 +88,7 @@ fi cd .. $CARGO_HOME/bin/diener --substrate --branch $CI_COMMIT_REF_NAME --git https://gitlab.parity.io/parity/substrate.git --path polkadot cd polkadot +git diff # Test Polkadot pr or master branch with this Substrate commit. time cargo test --all --release --verbose From ab0c998a5033bd5a0ec7849699b4ea1812587b58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 17 Sep 2020 23:09:24 +0200 Subject: [PATCH 24/31] Make sure we update the `Cargo.lock` --- .maintain/gitlab/check_polkadot_companion_build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.maintain/gitlab/check_polkadot_companion_build.sh b/.maintain/gitlab/check_polkadot_companion_build.sh index 03454c40338fc..219af5001b053 100755 --- a/.maintain/gitlab/check_polkadot_companion_build.sh +++ b/.maintain/gitlab/check_polkadot_companion_build.sh @@ -88,7 +88,7 @@ fi cd .. $CARGO_HOME/bin/diener --substrate --branch $CI_COMMIT_REF_NAME --git https://gitlab.parity.io/parity/substrate.git --path polkadot cd polkadot -git diff # Test Polkadot pr or master branch with this Substrate commit. +cargo update -p sp-io time cargo test --all --release --verbose From fda8a1ad8226eb1acc3703161b1b686cff433163 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Fri, 18 Sep 2020 09:07:02 +0200 Subject: [PATCH 25/31] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- .../procedural/src/construct_runtime/mod.rs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index f5cc8ff675535..b6f3409b6ca4e 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -331,9 +331,8 @@ fn decl_outer_dispatch<'a>( .map(|module_declaration| { let module = &module_declaration.module; let name = &module_declaration.name; - let index = module_declaration.index; - let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); - quote!(#[codec(index = #index_string)] #module::#name) + let index = module_declaration.index.to_string(); + quote!(#[codec(index = #index)] #module::#name) }); quote!( @@ -366,9 +365,8 @@ fn decl_outer_origin<'a>( ); return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let index = module_declaration.index; - let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); - let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); + let index = module_declaration.index.to_string(); + let tokens = quote!(#[codec(index = #index)] #module #instance #generics,); modules_tokens.extend(tokens); } None => {} @@ -376,14 +374,13 @@ fn decl_outer_origin<'a>( } let system_name = &system_module.module; - let system_index = system_module.index; - let system_index_string = syn::LitStr::new(&format!("{}", system_index), Span::call_site()); + let system_index = system_module.index.to_string(); Ok(quote!( #scrate::impl_outer_origin! { pub enum Origin for #runtime_name where system = #system_name, - system_index = #system_index_string + system_index = #system_index { #modules_tokens } @@ -412,9 +409,8 @@ fn decl_outer_event<'a>( return Err(syn::Error::new(module_declaration.name.span(), msg)); } - let index = module_declaration.index; - let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site()); - let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,); + let index = module_declaration.index.to_string(); + let tokens = quote!(#[codec(index = #index)] #module #instance #generics,); modules_tokens.extend(tokens); } None => {} From 95ef421253368d06a16004ca744bdd0179a4d474 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 18 Sep 2020 09:19:23 +0200 Subject: [PATCH 26/31] return 2 error --- frame/support/procedural/src/construct_runtime/mod.rs | 8 +++++--- .../tests/construct_runtime_ui/conflicting_index.stderr | 8 +++++++- .../tests/construct_runtime_ui/conflicting_index_2.stderr | 8 +++++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index b6f3409b6ca4e..41d4142cb672d 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -21,7 +21,7 @@ use frame_support_procedural_tools::syn_ext as ext; use frame_support_procedural_tools::{generate_crate_access, generate_hidden_includes}; use parse::{ModuleDeclaration, RuntimeDefinition, WhereSection, ModulePart}; use proc_macro::TokenStream; -use proc_macro2::{TokenStream as TokenStream2, Span}; +use proc_macro2::{TokenStream as TokenStream2}; use quote::quote; use syn::{Ident, Result, TypePath}; use std::collections::HashMap; @@ -79,11 +79,13 @@ fn complete_modules(decl: impl Iterator) -> syn::Resul if let Some(used_module) = indices.insert(final_index, module.name.clone()) { let msg = format!( "Module index are conflicting both modules {} and {} are at index {}", - module.name, used_module, + module.name, final_index, ); - return Err(syn::Error::new(module.name.span(), msg)); + let mut err = syn::Error::new(used_module.span(), &msg); + err.combine(syn::Error::new(module.name.span(), msg)); + return Err(err); } Ok(Module { diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr b/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr index adb60a9bf1152..735efa7d22df3 100644 --- a/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr @@ -1,4 +1,10 @@ -error: Module index are conflicting both modules Pallet1 and System are at index 0 +error: Module index are conflicting both modules System and Pallet1 are at index 0 + --> $DIR/conflicting_index.rs:9:3 + | +9 | System: system::{}, + | ^^^^^^ + +error: Module index are conflicting both modules System and Pallet1 are at index 0 --> $DIR/conflicting_index.rs:10:3 | 10 | Pallet1: pallet1::{} = 0, diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr index badcca970685b..3ff02746bd272 100644 --- a/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr @@ -1,4 +1,10 @@ -error: Module index are conflicting both modules Pallet3 and System are at index 5 +error: Module index are conflicting both modules System and Pallet3 are at index 5 + --> $DIR/conflicting_index_2.rs:9:3 + | +9 | System: system::{} = 5, + | ^^^^^^ + +error: Module index are conflicting both modules System and Pallet3 are at index 5 --> $DIR/conflicting_index_2.rs:12:3 | 12 | Pallet3: pallet3::{}, From 68b472edd04f4621e76d4be2e986e19d8fd025fc Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Mon, 21 Sep 2020 11:37:19 +0200 Subject: [PATCH 27/31] Apply suggestions from code review Co-authored-by: Alexander Popiak --- frame/support/procedural/src/construct_runtime/mod.rs | 2 +- .../test/tests/construct_runtime_ui/conflicting_index.stderr | 4 ++-- .../tests/construct_runtime_ui/conflicting_index_2.stderr | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 41d4142cb672d..81cd50088c9a1 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -78,7 +78,7 @@ fn complete_modules(decl: impl Iterator) -> syn::Resul if let Some(used_module) = indices.insert(final_index, module.name.clone()) { let msg = format!( - "Module index are conflicting both modules {} and {} are at index {}", + "Module indices are conflicting: Both modules {} and {} are at index {}", used_module, module.name, final_index, diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr b/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr index 735efa7d22df3..65368666c88fe 100644 --- a/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index.stderr @@ -1,10 +1,10 @@ -error: Module index are conflicting both modules System and Pallet1 are at index 0 +error: Module indices are conflicting: Both modules System and Pallet1 are at index 0 --> $DIR/conflicting_index.rs:9:3 | 9 | System: system::{}, | ^^^^^^ -error: Module index are conflicting both modules System and Pallet1 are at index 0 +error: Module indices are conflicting: Both modules System and Pallet1 are at index 0 --> $DIR/conflicting_index.rs:10:3 | 10 | Pallet1: pallet1::{} = 0, diff --git a/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr index 3ff02746bd272..b792ff5d2a541 100644 --- a/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr +++ b/frame/support/test/tests/construct_runtime_ui/conflicting_index_2.stderr @@ -1,10 +1,10 @@ -error: Module index are conflicting both modules System and Pallet3 are at index 5 +error: Module indices are conflicting: Both modules System and Pallet3 are at index 5 --> $DIR/conflicting_index_2.rs:9:3 | 9 | System: system::{} = 5, | ^^^^^^ -error: Module index are conflicting both modules System and Pallet3 are at index 5 +error: Module indices are conflicting: Both modules System and Pallet3 are at index 5 --> $DIR/conflicting_index_2.rs:12:3 | 12 | Pallet3: pallet3::{}, From 6feb4605c6f784b64591e229de7a6fec6dbffb4b Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Mon, 21 Sep 2020 14:14:30 +0200 Subject: [PATCH 28/31] Update frame/scheduler/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/scheduler/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index f74d6901c3a64..a0133d93056ec 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -446,12 +446,9 @@ impl Module { agenda .into_iter() .map(|schedule| schedule.map(|schedule| Scheduled { - maybe_id: schedule.maybe_id, - priority: schedule.priority, - call: schedule.call, - maybe_periodic: schedule.maybe_periodic, origin: schedule.origin.into(), _phantom: Default::default(), + ..scheduled })) .collect::>() )); From f2de8f2db34e8ac72bc9c34437c60dca3fa4ac22 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 21 Sep 2020 18:11:53 +0200 Subject: [PATCH 29/31] fix typo --- frame/scheduler/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index a0133d93056ec..95e6516a8e0d5 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -448,7 +448,7 @@ impl Module { .map(|schedule| schedule.map(|schedule| Scheduled { origin: schedule.origin.into(), _phantom: Default::default(), - ..scheduled + ..schedule })) .collect::>() )); From 3f38e1a0f1796ed7f81a9abcdfb364a15edb1f12 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 21 Sep 2020 18:14:19 +0200 Subject: [PATCH 30/31] Revert "fix typo" This reverts commit f2de8f2db34e8ac72bc9c34437c60dca3fa4ac22. --- frame/scheduler/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 95e6516a8e0d5..a0133d93056ec 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -448,7 +448,7 @@ impl Module { .map(|schedule| schedule.map(|schedule| Scheduled { origin: schedule.origin.into(), _phantom: Default::default(), - ..schedule + ..scheduled })) .collect::>() )); From 492a83f51f558e47a553add243e796ed60aab6b0 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 21 Sep 2020 18:14:32 +0200 Subject: [PATCH 31/31] Revert "Update frame/scheduler/src/lib.rs" This reverts commit 6feb4605c6f784b64591e229de7a6fec6dbffb4b. --- frame/scheduler/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index a0133d93056ec..f74d6901c3a64 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -446,9 +446,12 @@ impl Module { agenda .into_iter() .map(|schedule| schedule.map(|schedule| Scheduled { + maybe_id: schedule.maybe_id, + priority: schedule.priority, + call: schedule.call, + maybe_periodic: schedule.maybe_periodic, origin: schedule.origin.into(), _phantom: Default::default(), - ..scheduled })) .collect::>() ));