Proposal: Flatten `AllPallets` and similar types #11813

kianenigma · 2022-07-11T13:04:43Z

Currently, all of the types generated via this macro are nested. For example, you have:

type AllPallets = (System, (Balances, Babe));

As far as I know, the main advantage of this is that we can use recursive implementations in some places. For example, PalletsInfoAccess introduced in https://github.com/paritytech/substrate/pull/10053/files is only implemented for (T1, T2), and there's no need to generate the code for longer tuples.

In fact, if we use only nested tuples, you can probably get away with impl_trait_for_tuple(2) for most items that are implemented for all pallets, such as OnInitialize etc.

The problem is: certain traits like OnInitialize are perfectly comprehensible both for T, and (T1, T2, T3), and (T1, (T2, T3)). But, certain traits like PalletInfoAccess are only comprehensible on individual items, i.e. T, but not on tuples.

And then you have an unfortunate scenario where you want to assert where T: PalletInfoAccess, and you know that each T does indeed implement PalletInfoAccess, but if demonstrated as a nested tuple, this does not work.

You can see a concrete example of this in what I am trying to do here: https://github.com/paritytech/substrate/pull/10174/files#diff-1c449417be9803cb427eed15d51557931141d47d0047561407f1651d319f7103R183

Lastly, in general, arguing about a flat array of tuples rather then nested is generally easier.

measure how slower it is to bump impl_trait_for_tuples to 100 instead of 30.

frame/support/src/traits/filter.rs

frame/support/src/traits/hooks.rs

frame/support/src/traits/metadata.rs

kianenigma · 2022-08-08T09:01:54Z

bot rebase

…lets-type

paritytech-processbot · 2022-08-08T09:02:02Z

Rebased

kianenigma · 2022-08-08T09:03:19Z

/cmd queue -c fmt $

command-bot · 2022-08-08T09:03:22Z

@kianenigma Could not find start of command (" $ ")

kianenigma · 2022-08-08T09:03:31Z

/cmd queue -c fmt

command-bot · 2022-08-08T09:03:33Z

@kianenigma Could not find start of command (" $ ")

kianenigma · 2022-08-08T09:04:01Z

/cmd queue -c fmt $ 1

command-bot · 2022-08-08T09:04:05Z

@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1726286 was started for your command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 11-58e217d0-cfd8-4c51-94b8-464e4f4538b4 to cancel this command or /cmd cancel to cancel all commands in this pull request.

command-bot · 2022-08-08T09:05:00Z

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1 has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1726286 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1726286/artifacts/download.

…tten-all-pallets-type

frame/support/src/traits/metadata.rs

frame/support/src/migrations.rs

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

bkchr · 2022-08-11T10:32:11Z

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

 		/// All pallets included in the runtime as a nested tuple of types.
 		/// Excludes the System pallet.
-		pub type AllPalletsWithoutSystem = ( #all_pallets_without_system );
+		pub type AllPalletsWithoutSystem = ( #(#names_without_system),* );


Suggested change

pub type AllPalletsWithoutSystem = ( #(#names_without_system),* );

pub type AllPalletsWithoutSystem = ( #(#all_pallets_without_system),* );

bkchr · 2022-08-11T10:32:37Z

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

-		pub type AllPalletsWithoutSystemReversed = ( #all_pallets_without_system_reversed );
+		pub type AllPalletsWithoutSystemReversed =( #(#names_without_system_reverse),* );

 		/// All pallets included in the runtime as a nested tuple of types in reversed order.
-		pub type AllPalletsWithSystemReversed = ( #all_pallets_with_system_reversed );
+		pub type AllPalletsWithSystemReversed = ( #(#names_reversed),* );

 		/// All pallets included in the runtime as a nested tuple of types in reversed order.
 		/// With the system pallet first.
-		pub type AllPalletsReversedWithSystemFirst = (
-			#system_pallet,
-			AllPalletsWithoutSystemReversed
-		);
+		pub type AllPalletsReversedWithSystemFirst = ( #(#names_reversed_with_system_first),* );


I think we can probably now get rid of them?

I would start by marking them as deprecated? or are you sure they are not needed anywhere?

Yeah good idea.

Co-authored-by: Bastian Köcher <[email protected]>

kianenigma · 2022-08-12T18:39:53Z

bot rebase

…lets-type

paritytech-processbot · 2022-08-12T18:40:02Z

Rebased

paritytech-cicd-pr · 2022-08-12T18:54:28Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1740275

frame/support/Cargo.toml

…substrate into kiz-flatten-all-pallets-type

kianenigma · 2022-08-14T17:03:05Z

frame/support/test/tests/pallet_ui/hooks_invalid_item.stderr

@@ -1,13 +1,13 @@
 error[E0107]: missing generics for trait `Hooks`
-   --> $DIR/hooks_invalid_item.rs:12:18
+   --> tests/pallet_ui/hooks_invalid_item.rs:12:18


I am a little bit unsure about why I had to change this, but I assume it is harmless.

kianenigma · 2022-08-14T19:05:54Z

bot merge

* flratten AllPallets types * feature flag it * fix * fix * fmt * remove todo * Update frame/support/src/traits/metadata.rs Co-authored-by: Bastian Köcher <[email protected]> * Update frame/support/src/migrations.rs Co-authored-by: Bastian Köcher <[email protected]> * fix * mark as deprecated * add docs * fix ui test? * fmt Co-authored-by: parity-processbot <> Co-authored-by: Bastian Köcher <[email protected]>

Polkadot-Forum · 2023-05-11T12:14:01Z

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-41-v0-9-42/2828/1

flratten AllPallets types

5d07608

github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jul 11, 2022

kianenigma added B3-apinoteworthy labels Jul 11, 2022

kianenigma marked this pull request as ready for review July 18, 2022 10:14

kianenigma 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 18, 2022

kianenigma requested a review from bkchr July 18, 2022 10:14

kianenigma mentioned this pull request Jul 22, 2022

Runtime State Test + Integration with try-runtime #10174

Merged

7 tasks

bkchr reviewed Jul 25, 2022

View reviewed changes

frame/support/src/traits/filter.rs Outdated Show resolved Hide resolved

feature flag it

f46e648

shawntabrizi reviewed Jul 30, 2022

View reviewed changes

frame/support/src/traits/hooks.rs Outdated Show resolved Hide resolved

KiChjang reviewed Jul 30, 2022

View reviewed changes

frame/support/src/traits/metadata.rs Outdated Show resolved Hide resolved

kianenigma added 3 commits August 2, 2022 20:00

fix

1bb98a2

fix

9bc95fd

fmt

dc211b9

ggwpez approved these changes Aug 3, 2022

View reviewed changes

Merge remote-tracking branch 'origin/master' into kiz-flatten-all-pal…

9bef35c

…lets-type

kianenigma added C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Aug 8, 2022

kianenigma requested a review from bkchr August 11, 2022 06:42

kianenigma requested a review from KiChjang August 11, 2022 06:42

kianenigma added 2 commits August 11, 2022 11:12

Merge branch 'master' of github.com:paritytech/substrate into kiz-fla…

e1f8c25

…tten-all-pallets-type

remove todo

81cbf48

bkchr approved these changes Aug 11, 2022

View reviewed changes

kianenigma and others added 5 commits August 12, 2022 10:59

master

3aea087

Update frame/support/src/traits/metadata.rs

d4a8139

Co-authored-by: Bastian Köcher <[email protected]>

Update frame/support/src/migrations.rs

cf4790a

Co-authored-by: Bastian Köcher <[email protected]>

fix

ed03bfc

mark as deprecated

fdd602e

Merge remote-tracking branch 'origin/master' into kiz-flatten-all-pal…

f94a859

…lets-type

KiChjang reviewed Aug 13, 2022

View reviewed changes

frame/support/Cargo.toml Show resolved Hide resolved

kianenigma added 3 commits August 14, 2022 21:29

add docs

5898880

fix ui test?

f9b788b

Merge branch 'kiz-flatten-all-pallets-type' of github.com:paritytech/…

7737659

…substrate into kiz-flatten-all-pallets-type

kianenigma commented Aug 14, 2022

View reviewed changes

fmt

a2ab6ec

paritytech-processbot bot merged commit 669ed36 into master Aug 14, 2022

paritytech-processbot bot deleted the kiz-flatten-all-pallets-type branch August 14, 2022 19:06

NingLin-P mentioned this pull request Sep 5, 2022

support upgrade hooks to directly pass data #12185

Merged

librelois mentioned this pull request Sep 15, 2022

Update substrate/polkadot/cumulus from v0.9.26 to v0.9.29 moonbeam-foundation/moonbeam#1814

Closed

2 tasks

wischli mentioned this pull request Sep 21, 2022

feat: upgrade to Polkadot v0.9.29 KILTprotocol/kilt-node#416

Merged

6 tasks

girazoki mentioned this pull request Sep 30, 2022

v0.9.29 Dependency Update moonbeam-foundation/moonbeam#1825

Merged

13 tasks

	pub type AllPalletsWithoutSystem = ( #(#names_without_system),* );
	pub type AllPalletsWithoutSystem = ( #(#all_pallets_without_system),* );

Proposal: Flatten AllPallets and similar types #11813

Proposal: Flatten AllPallets and similar types #11813

Uh oh!

Conversation

kianenigma commented Jul 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kianenigma commented Aug 8, 2022

Uh oh!

paritytech-processbot bot commented Aug 8, 2022

Uh oh!

kianenigma commented Aug 8, 2022

Uh oh!

command-bot bot commented Aug 8, 2022

Uh oh!

kianenigma commented Aug 8, 2022

Uh oh!

command-bot bot commented Aug 8, 2022

Uh oh!

kianenigma commented Aug 8, 2022

Uh oh!

command-bot bot commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

command-bot bot commented Aug 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkchr Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

bkchr Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

kianenigma Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

bkchr Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

kianenigma commented Aug 12, 2022

Uh oh!

paritytech-processbot bot commented Aug 12, 2022

Uh oh!

paritytech-cicd-pr commented Aug 12, 2022

Uh oh!

Uh oh!

kianenigma Aug 14, 2022

Choose a reason for hiding this comment

Uh oh!

kianenigma commented Aug 14, 2022

Uh oh!

Polkadot-Forum commented May 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Proposal: Flatten `AllPallets` and similar types #11813

Proposal: Flatten `AllPallets` and similar types #11813

kianenigma commented Jul 11, 2022 •

edited

Loading

command-bot bot commented Aug 8, 2022 •

edited

Loading