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

Allow pallet in construct_runtime to have fixed index #6969

Merged
37 commits merged into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3556df3
implement index for pallet + some tests
gui1117 Aug 26, 2020
766f510
add test and doc
gui1117 Sep 1, 2020
d246bdf
remove deprecated and document behavior
gui1117 Sep 1, 2020
11a3af2
update internal doc
gui1117 Sep 1, 2020
ce2d1bd
Apply suggestions from code review
gui1117 Sep 7, 2020
b56cbd7
address review
gui1117 Sep 7, 2020
da702ae
use index for all module, break construct_runtime
gui1117 Sep 8, 2020
0290c47
fix line length
gui1117 Sep 8, 2020
82e36e4
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 9, 2020
6c8e80a
implement migration helper funciton in scheduler
gui1117 Sep 9, 2020
4e5b4f8
fix start at index 0
gui1117 Sep 9, 2020
95a1eb7
Update frame/scheduler/src/lib.rs
gui1117 Sep 10, 2020
9518e5b
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 10, 2020
a830e2f
Update frame/support/procedural/src/lib.rs
gui1117 Sep 16, 2020
9812da2
bump frame-metadata crate
gui1117 Sep 16, 2020
5d86201
factorize
gui1117 Sep 16, 2020
078a543
avoid some unwrap and remove nightly join
gui1117 Sep 16, 2020
4bba712
Update frame/support/src/event.rs
gui1117 Sep 16, 2020
c6f073b
fix test
gui1117 Sep 16, 2020
43856fd
add test and improve error message
gui1117 Sep 16, 2020
7da2738
factorize test
gui1117 Sep 16, 2020
b9fca7e
keep iterator, and use slice instead of vec
gui1117 Sep 16, 2020
61019e7
refactor to avoid to have expects
gui1117 Sep 16, 2020
a9d454a
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 16, 2020
ae0f859
small refactor
gui1117 Sep 16, 2020
85a65f2
Test something
bkchr Sep 17, 2020
ab0c998
Make sure we update the `Cargo.lock`
bkchr Sep 17, 2020
fda8a1a
Apply suggestions from code review
gui1117 Sep 18, 2020
95ef421
return 2 error
gui1117 Sep 18, 2020
68b472e
Apply suggestions from code review
gui1117 Sep 21, 2020
21510d4
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 21, 2020
6feb460
Update frame/scheduler/src/lib.rs
gui1117 Sep 21, 2020
f2de8f2
fix typo
gui1117 Sep 21, 2020
3f38e1a
Revert "fix typo"
gui1117 Sep 21, 2020
492a83f
Revert "Update frame/scheduler/src/lib.rs"
gui1117 Sep 21, 2020
d04f8cd
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 22, 2020
986155b
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 22, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 10 additions & 5 deletions frame/metadata/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
}

/// Enum that should fail.
Expand All @@ -387,15 +389,15 @@ 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<ModuleMetadata>,
/// Metadata of the extrinsic.
pub extrinsic: ExtrinsicMetadata,
}

/// 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)]
Expand All @@ -407,6 +409,9 @@ pub struct ModuleMetadata {
pub event: ODFnA<EventMetadata>,
pub constants: DFnA<ModuleConstantMetadata>,
pub errors: DFnA<ErrorMetadata>,
/// 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<u8>,
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
}

type ODFnA<T> = Option<DFnA<T>>;
Expand All @@ -420,6 +425,6 @@ impl Into<sp_core::OpaqueMetadata> for RuntimeMetadataPrefixed {

impl Into<RuntimeMetadataPrefixed> for RuntimeMetadataLastVersion {
fn into(self) -> RuntimeMetadataPrefixed {
RuntimeMetadataPrefixed(META_RESERVED, RuntimeMetadata::V11(self))
RuntimeMetadataPrefixed(META_RESERVED, RuntimeMetadata::V12(self))
}
}
165 changes: 127 additions & 38 deletions frame/support/procedural/src/construct_runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -51,40 +51,34 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result<TokenStream
},
..
} = definition;
let modules = modules.into_iter().collect::<Vec<_>>();

let system_module = check_modules(&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<T>},`",
))
}
};
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::<Vec<_>>();

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(
Expand Down Expand Up @@ -238,7 +232,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!{
Expand All @@ -250,16 +249,23 @@ fn decl_runtime_metadata<'a>(

fn decl_outer_dispatch<'a>(
runtime: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: &Vec<ModuleDeclaration>,
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 {
Expand All @@ -271,12 +277,14 @@ fn decl_outer_dispatch<'a>(

fn decl_outer_origin<'a>(
runtime_name: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
modules_except_system: &Vec<ModuleDeclaration>,
system_name: &'a Ident,
scrate: &'a TokenStream2,
) -> syn::Result<TokenStream2> {
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;
Expand All @@ -290,7 +298,19 @@ 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, but it is reserved to \
system"
);
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 => {}
Expand All @@ -308,10 +328,11 @@ fn decl_outer_origin<'a>(

fn decl_outer_event<'a>(
runtime_name: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: &Vec<ModuleDeclaration>,
scrate: &'a TokenStream2,
) -> syn::Result<TokenStream2> {
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) => {
Expand All @@ -326,7 +347,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 => {}
Expand Down Expand Up @@ -377,12 +404,18 @@ fn decl_all_modules<'a>(
}

fn decl_module_to_index<'a>(
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
num_modules: usize,
module_declarations: &Vec<ModuleDeclaration>,
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -404,14 +437,6 @@ fn decl_module_to_index<'a>(
)
}

fn find_system_module<'a>(
mut module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
) -> 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)]
Expand All @@ -425,3 +450,67 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 {
}
)
}

fn get_available_indices(modules: &Vec<ModuleDeclaration>) -> Vec<u8> {
let reserved_indices = modules.iter()
.filter_map(|module| module.index)
.collect::<Vec<_>>();

(0..u8::max_value())
.filter(|i| !reserved_indices.contains(i))
.collect::<Vec<_>>()
}

/// 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<ModuleDeclaration>,
modules_span: proc_macro2::Span,
) -> Result<syn::Ident> {
// 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<T>},`",
))?;

// 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 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 = format!(
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
"Only system module is allowed to be defined at index `0`, this is because of origin \
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
encoding",
);
return Err(syn::Error::new(module.name.span(), msg));
}

// No module indices conflicts
let mut indices = vec![];
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
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())
}
14 changes: 13 additions & 1 deletion frame/support/procedural/src/construct_runtime/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,11 @@ impl Parse for WhereDefinition {
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct ModuleDeclaration {
pub name: Ident,
/// Optional fixed index (e.g. `MyPallet ... = 3,`)
pub index: Option<u8>,
pub module: Ident,
pub instance: Option<Ident>,
pub module_parts: Vec<ModulePart>,
Expand All @@ -175,11 +177,21 @@ impl Parse for ModuleDeclaration {
let _: Token![::] = input.parse()?;
let module_parts = parse_module_parts(input)?;

let index = if input.peek(Token![=]) {
input.parse::<Token![=]>()?;
let index = input.parse::<syn::LitInt>()?;
let index = index.base10_parse::<u8>()?;
Some(index)
} else {
None
};

let parsed = Self {
name,
module,
instance,
module_parts,
index,
};

Ok(parsed)
Expand Down
16 changes: 12 additions & 4 deletions frame/support/procedural/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,13 @@ pub fn decl_storage(input: TokenStream) -> TokenStream {
/// NodeBlock = runtime::Block,
/// UncheckedExtrinsic = UncheckedExtrinsic
/// {
/// System: system::{Module, Call, Event<T>, Config<T>},
/// Test: test::{Module, Call},
/// Test2: test_with_long_module::{Module},
/// System: system::{Module, Call, Event<T>, Config<T>} = 0,
/// Test: test::{Module, Call} = 1,
/// Test2: test_with_long_module::{Module, Event<T>},
///
/// // Module with instances
/// Test3_Instance1: test3::<Instance1>::{Module, Call, Storage, Event<T, I>, Config<T, I>, Origin<T, I>},
/// Test3_DefaultInstance: test3::{Module, Call, Storage, Event<T>, Config<T>, Origin<T>},
/// Test3_DefaultInstance: test3::{Module, Call, Storage, Event<T>, Config<T>, Origin<T>} = 4,
/// }
/// )
/// ```
Expand All @@ -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
Expand Down
Loading