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

[gui1117]

Instead of fetching the type from the macro we can fetch the type from the Pallet struct placeholder.

We introduce a trait trait PalletEvent { type Event },
we implement it for the pallet placeholder.
and then we use <Pallet<Runtime> as PalletEvent>::Event when building the aggregated runtime event.

Thus there is no longer need for specify the generics of the Event type.

Same for Origin and Config.

This main advantage of it is in to not have these difficult compilation error when the generics are wrongly specified.

[stale]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

[gui1117]

still relevant

[KiChjang]

What happens if there is data in an Event enum that requires access to Config? Currently with generics, we can access it via T::AccountId for example, but without T, we would not be able to specify any of the types in Config.

[gui1117]

no this isn't what I meant, event type can still be generic, but the configuration of the part doesn't require to give this information.

This can be achieve by using trait implemented on the pallet struct:

trait PalletEvent {
type Event;
}
impl<T: Config> PalletEvent for Pallet<T> {
type Event = TheEventDefinedInThePallet<T>;
}

(maybe the trait PalletEvent needs generics too like PalletEvent<T, I=()>, but I think it is no needed)

and construct_runtime will always access the Event from the pallet struct throught the trait.

[KiChjang]

Solving this issue is actually a prerequisite to #8084, because an "import all declared pallet parts" syntax by definition does not include a generic for the pallet parts that require it.

[gui1117]

actually when pallet declare their parts they can also declare their part with the correct generic. But I agree it can make code simpler.

[KiChjang]

Hmm ok, so the issue is when we try to implement PalletEvent on the Pallet itself, we'd still need to specify generics on it. We can't just do:

impl<T: Config> PalletEvent for Pallet<T> {
    type Event = path::to::pallet::Event<T>;
}

... because the Event might not take a T parameter at all.

[gui1117]

but the event type is declared inside the pallet macro, the pallet macro knows about the generic of Event type. So it can implement PalletEvent correctly.

[KiChjang]

Not when we're calling construct_runtime. By then, all information that we have in the pallet macro is lost, unless we have a special way of storing these information and have the construct_runtime macro access them during expansion. This is actually the same issue that I run into when I try to solve #8084.

[KiChjang]

Huh, so the inability to store local state and have it reused between macro invocations is actually a known issue since 2017: rust-lang/rust#44034

[KiChjang]

Oh ok, I guess what I can do instead is declare the trait in the pallet, and implement the trait there. Then construct_runtime would just reuse PalletEvent::Event.

[gui1117]

Oh ok, I guess what I can do instead is declare the trait in the pallet, and implement the trait there. Then construct_runtime would just reuse PalletEvent::Event.

yes this is the plan

[gui1117]

This is actually the same issue that I run into when I try to solve #8084.

To solve #8084 we need to use macros: pallet expose a macro which give the parts to construct_runtime: #6494

[gui1117]

This PR #9681 doe the automatic declaration of pallet parts.
So user only declare some part to exclude, without giving any information about the generics.

Maybe this issue can be considered fixed then

[gui1117]

personally I would close this issue, with the new syntax nobody should declare the part manually, instead they only whitelist or blacklist parts.
Maybe the syntax is not ideal, but this is not what this issue is about.

Feel free to re-open if you disagree

[KiChjang]

My current PR can only support non-generic Event and Config -- I don't think there is a way to support non-generic Origin for FRAMEv1 macros, so until we get every FRAME macro to use v2, then I don't think it's worth pursuing yet.