construct_runtime auto pallet parts: allow pallets to expand `{ Event, Call... }` automatically. #6494

gui1117 · 2020-06-24T13:53:53Z

This PR is to be followed by its generalized usage in substrate https://github.com/paritytech/substrate/compare/gui-automatic-construct-runtime...gui-automatic-construct-runtime-usage?expand=1

Description

Introduce new decl_construct_runtime_args so:

pallet declare construct_runtime args with: decl_construct_runtime_args!( Module, Config<T>, Storage.. )

and then construct_runtime can be called as such:

construct_runtime!(
  Treasury: frame_treasury,
)

this will expand to

construct_runtime!(
  Treasury: frame_treasury::{Module, Call, Storage, Config, Event<T>},
)

if there is multiple pallet in the same crate, a unique id must be provided, this unique id is only internally used:
decl_construct_runtime_args!(#[unique_id = _2] Module, Config<T>, Storage.. )

see https://github.com/paritytech/substrate/pull/6494/files#diff-a8500018e076e76e369b45df6010dd2d

Goal

simplify construct_runtime so: less error prone construct_runtime and more usable in tests instead of impl_outer_* which are not consistent and very hacky.

General notes:

multiple pallet per crate is still doable
no breaking change, runtime can still define the part themself
This makes construct_runtime smaller, I think it should replace impl_outer_* in tests. Thus test environment are more consistent. (some helpful type has been added to system for this purpose)
if you want to see how looks substrate once tests and runtime are moved to new syntax see https://github.com/paritytech/substrate/compare/gui-automatic-construct-runtime...gui-automatic-construct-runtime-usage?expand=1
while implementing decl_construct_runtime_args for pallets I saw that some Config were missed.
(maybe I could do more sanity check: if Storage is provided but not Config then decl_construct_runtime_args generate a test that Config is not generated by decl_storage (i.e. decl_storage will provide a function __hidden_has_generated_config). but this is for another PR and fixed by Move support macros to attribute procedural macros #5678).
Move support macros to attribute procedural macros #5678 will implement this macro automatically.

How does it works internally

for the given example

construct_runtime!(
  Treasury: frame_treasury,
  Treasury2: frame_treasury2,
)

construct_runtime do a preprocessing and expand for each auto pallet parts a overarching macro call:

frame_treasury::construct_runtime_args! {
	{ Treasury: frame_treasury } // mathing pattern
	frame_treasury2::construct_runtime_args! {
		{ Treasury2: frame_treasury2 } // matchine pattern
		construct_runtime! {
			Treasury: frame_treasury,
			Treasury2: frame_treasury2,
		}
	}
}

Each auto_construct_runtime replace the match of their matching pattern in the following token stream.
Resulting in:

frame_treasury2::construct_runtime_args! {
	{ Treasury2: frame_treasury2 } // matchine pattern
	construct_runtime! {
		Treasury: frame_treasury::{Module, Call, Storage, Config, Event<T>},
		Treasury2: frame_treasury2,
	}
}

until the expanded construct_runtime call:

construct_runtime! {
  Treasury: frame_treasury::{Module, Call, Storage, Config, Event<T>},
  Treasury2: frame_treasury2::{Module, Call, Storage, Config, Event<T>},
}

bkchr

Nice! 👍

bkchr · 2020-06-24T15:47:29Z

frame/treasury/src/lib.rs

@@ -111,6 +111,16 @@ type BalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trai
 type PositiveImbalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::PositiveImbalance;
 type NegativeImbalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::NegativeImbalance;

+#[macro_export]
+macro_rules! auto_construct_runtime {


Nice idea, as you already said, it would be nice to replace this with

construct_runtime_args!( Module, Call, Storage, Config, Event<T> )

construct_runtime_args can be replaced with a better name.

bkchr · 2020-06-24T15:48:09Z

And we should not require auto keyword, it should be the default.

gui1117 · 2020-06-25T10:19:45Z

Yes so final syntax should be something like:

construct_runtime!(.... { System: frame_system, Treasury: frame_treasury, OldSyntax: frame_old::{Module...} })

and for pallet side:

decl_construct_runtime_args!( Module, Call, Storage, Config,  Event<T> )

Though one thing is that construct_runtime_args doesn't really support Inherent(Timestamp) use by babe for instance. Though I think we can just not make use of this syntax anymore, and instead make babe make the check in on_finalize directly.

bkchr · 2020-06-25T12:29:54Z

Though one thing is that construct_runtime_args doesn't really support Inherent(Timestamp) use by babe for instance. Though I think we can just not make use of this syntax anymore, and instead make babe make the check in on_finalize directly.

I have something prepared for this already :D I need to revive the branch and open a Pr ;)

gui1117 · 2020-06-26T12:14:52Z

I added better syntax and also now that construct_runtime is smaller I think we can use it in test instead of using impl_outer_*.
Test will consist of writing just a very small runtime: see pallet-assets in this PR

gui1117 · 2020-06-30T13:12:06Z

I updated the PR to only contains the changes in frame-support frame-system and primitive, the generalized usage of it is moved to another branch https://github.com/paritytech/substrate/compare/gui-automatic-construct-runtime...gui-automatic-construct-runtime-usage?expand=1 where I implemented for all pallets, and most tests except non-obvious ones (i.e. system, balance, contract, and some other (just grep impl_outer_ to see remainings usage))

…ruct-runtime

primitives/runtime/src/lib.rs

gui1117 · 2020-07-17T15:37:41Z

All limitations have been solved:

multiple pallet per crate is allowed
no need for local_macro attribute in construct_runtime when using pallet define inside same crate

kianenigma · 2020-08-05T07:54:47Z

frame/support/procedural/src/construct_runtime/mod.rs

@@ -163,6 +229,12 @@ fn decl_outer_inherent<'a>(
 		})
 	});
 	quote!(
+		// Prevent UncheckedExtrinsic to print unused warning.
+		const _: () = {


I see this pattern in the expansions of derive macros of codec as well, what is the purpose? is the keyword __hidden_use_unchecked_extrinsic used anywhere now?

no, keyword __hidden_use_unchecked_extrinsic is not used anywhere, warpping in const is a pattern used by many derive macro, for instance serde.
I think the main advantage is that it makes type __hidden_use_unchecked_extrinsic unavailable in doc and rust error will not recommand it.

But if the type alias is not even accessible and used anywhere, I don't see how it is used.

For example I've this and it makes sense:

const _: () { impl Something for SomethingElse { ... } }

but something like:

const _: () = { type __unused_alias = Foo; }

I don't get.

Unless is __hidden_use_unchecked_extrinsic used in the other branch? Asking because I couldn't find any usage here.

this is just to // Prevent UncheckedExtrinsic to print unused warning.
so I have const _: () = { #[allow(unused)] type __unused_alias = Foo; }
UncheckedExtrinsic is unused if there is no inherent.

frame/support/procedural/src/construct_runtime_args.rs

frame/support/procedural/src/expand_after.rs

frame/support/procedural/src/lib.rs

kianenigma · 2020-08-05T08:05:00Z

frame/system/src/lib.rs

@@ -1375,3 +1377,14 @@ impl<T: Trait> Lookup for ChainContext<T> {
 		<T::Lookup as StaticLookup>::lookup(s)
 	}
 }
+
+/// An implementation of `sp_runtime::traits::Block` to be used in test.
+pub type MockBlock<T> = generic::Block<


This does not seem used to me, else why not feature gate it to test or at least std?

in this following PR https://github.com/paritytech/substrate/compare/gui-automatic-construct-runtime...gui-automatic-construct-runtime-usage?expand=1
I used it a lot for pallet mocks.

I guess we could feature get but I don't see strong reason not to have in no_std as well. some people could make use of it for no_std mock.

frame/support/procedural/src/construct_runtime_args.rs

gui1117 · 2020-08-08T07:00:36Z

One main issue of this PR is that pallet can remove their call or their origin and it changes the outer call and outer origin types.
maybe we should put it on ice until we have indices for pallet in construct_runtime.

#6855

gnunicorn · 2020-09-09T07:04:34Z

closing because of inactivity.

gui1117 · 2020-09-09T08:52:46Z

NOTE: I still think this makes definition of construct_runtime really easier. Either we should do like this or we could make new attribute macro implement an empty Event, Inherent, ValidateUnsigned, ... and make pallet in construct_runtime having by default all attribute (which is simpler).

bkchr · 2020-09-09T09:18:19Z

NOTE: I still think this makes definition of construct_runtime really easier. Either we should do like this or we could make new attribute macro implement an empty Event, Inherent, ValidateUnsigned, ... and make pallet in construct_runtime having by default all attribute (which is simpler).

Yeah, I'm also in for this.

One question, did your version supported negation? So let's say I have a pallet where I expose a Call, but I as a runtime developer don't want to expose this Call, could I write something like !Call?

gui1117 · 2020-09-09T09:20:00Z

I didn't supported negation but indeed after macro attribute I can work on this

kianenigma · 2020-09-09T14:12:29Z

Hope to see the PR be reincarnated soon.

github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jun 24, 2020

gui1117 force-pushed the gui-automatic-construct-runtime branch from 2616950 to 6dddfb8 Compare June 24, 2020 13:54

bkchr reviewed Jun 24, 2020

View reviewed changes

macro modification

8e71a22

gui1117 force-pushed the gui-automatic-construct-runtime branch from 4ddf971 to 8e71a22 Compare June 30, 2020 13:06

gui1117 marked this pull request as ready for review June 30, 2020 13:17

gui1117 added A3-in_progress Pull request is in progress. No review needed at this stage. A0-please_review Pull request needs code review. and removed A0-please_review Pull request needs code review. A3-in_progress Pull request is in progress. No review needed at this stage. labels Jun 30, 2020

gui1117 added 3 commits June 30, 2020 15:26

doc

ebcebfd

add tests

8700d74

removed unused

164722c

gui1117 requested a review from kianenigma as a code owner July 1, 2020 09:31

Merge remote-tracking branch 'origin/master' into gui-automatic-const…

a9c30ac

…ruct-runtime

paritytech-ci added the D2-breaksapi label Jul 16, 2020

ensure module are expanded through rust types

41439ec

gui1117 commented Jul 16, 2020

View reviewed changes

primitives/runtime/src/lib.rs Outdated Show resolved Hide resolved

gui1117 added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Jul 17, 2020

remove need for local_macro and multiple pallet per crate

c714a72

gui1117 added 3 commits July 17, 2020 16:21

fix ui test

0bee948

allow multiple call to impl_outer_config

2a62399

fix wasm build

6445d9a

gui1117 added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jul 17, 2020

kianenigma reviewed Aug 5, 2020

View reviewed changes

frame/support/procedural/src/construct_runtime_args.rs Show resolved Hide resolved

kianenigma reviewed Aug 5, 2020

View reviewed changes

frame/support/procedural/src/expand_after.rs Show resolved Hide resolved

kianenigma reviewed Aug 5, 2020

View reviewed changes

frame/support/procedural/src/lib.rs Outdated Show resolved Hide resolved

kianenigma reviewed Aug 5, 2020

View reviewed changes

frame/support/procedural/src/construct_runtime_args.rs Show resolved Hide resolved

add license and fix doc

b26ef34

github-actions bot added the A7-needspolkadotpr label Aug 5, 2020

gui1117 mentioned this pull request Aug 12, 2020

Add pallet attribute macro to declare pallets #6877

Merged

3 tasks

gnunicorn closed this Sep 9, 2020

gui1117 mentioned this pull request Feb 9, 2021

Allow pallet to declare its default parts and make construct_runtime automatically use them by default #8084

Closed

gui1117 mentioned this pull request Aug 27, 2021

Remove the need to specify <T> for parts in construct_runtime! (like Event<T>) #8743

Closed

gui1117 deleted the gui-automatic-construct-runtime branch September 23, 2021 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

construct_runtime auto pallet parts: allow pallets to expand `{ Event, Call... }` automatically. #6494

construct_runtime auto pallet parts: allow pallets to expand `{ Event, Call... }` automatically. #6494

gui1117 commented Jun 24, 2020 •

edited

Loading

bkchr left a comment

bkchr Jun 24, 2020

bkchr commented Jun 24, 2020

gui1117 commented Jun 25, 2020

bkchr commented Jun 25, 2020

gui1117 commented Jun 26, 2020

gui1117 commented Jun 30, 2020

gui1117 commented Jul 17, 2020 •

edited

Loading

kianenigma Aug 5, 2020

gui1117 Aug 5, 2020 •

edited

Loading

kianenigma Aug 5, 2020 •

edited

Loading

gui1117 Aug 5, 2020

kianenigma Aug 5, 2020

gui1117 Aug 5, 2020

gui1117 commented Aug 8, 2020 •

edited

Loading

gnunicorn commented Sep 9, 2020

gui1117 commented Sep 9, 2020

bkchr commented Sep 9, 2020

gui1117 commented Sep 9, 2020

kianenigma commented Sep 9, 2020

construct_runtime auto pallet parts: allow pallets to expand { Event, Call... } automatically. #6494

construct_runtime auto pallet parts: allow pallets to expand { Event, Call... } automatically. #6494

Conversation

gui1117 commented Jun 24, 2020 • edited Loading

This PR is to be followed by its generalized usage in substrate https://github.com/paritytech/substrate/compare/gui-automatic-construct-runtime...gui-automatic-construct-runtime-usage?expand=1

Description

Goal

General notes:

How does it works internally

bkchr left a comment

Choose a reason for hiding this comment

bkchr Jun 24, 2020

Choose a reason for hiding this comment

bkchr commented Jun 24, 2020

gui1117 commented Jun 25, 2020

bkchr commented Jun 25, 2020

gui1117 commented Jun 26, 2020

gui1117 commented Jun 30, 2020

gui1117 commented Jul 17, 2020 • edited Loading

kianenigma Aug 5, 2020

Choose a reason for hiding this comment

gui1117 Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

kianenigma Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

gui1117 Aug 5, 2020

Choose a reason for hiding this comment

kianenigma Aug 5, 2020

Choose a reason for hiding this comment

gui1117 Aug 5, 2020

Choose a reason for hiding this comment

gui1117 commented Aug 8, 2020 • edited Loading

gnunicorn commented Sep 9, 2020

gui1117 commented Sep 9, 2020

bkchr commented Sep 9, 2020

gui1117 commented Sep 9, 2020

kianenigma commented Sep 9, 2020

construct_runtime auto pallet parts: allow pallets to expand `{ Event, Call... }` automatically. #6494

construct_runtime auto pallet parts: allow pallets to expand `{ Event, Call... }` automatically. #6494

gui1117 commented Jun 24, 2020 •

edited

Loading

gui1117 commented Jul 17, 2020 •

edited

Loading

gui1117 Aug 5, 2020 •

edited

Loading

kianenigma Aug 5, 2020 •

edited

Loading

gui1117 commented Aug 8, 2020 •

edited

Loading