Vision: Improve Developer Experience with better Config Traits

[shawntabrizi]

There are common types defined within the config of different pallets, such as:

type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;

type Call: IsType<<Self as frame_system::Config>::Call>
    + Dispatchable<Origin = Self::Origin, PostInfo = PostDispatchInfo>
    + GetDispatchInfo
    + FullCodec
    + From<frame_system::Call<Self>>;

type Origin: From<RawOrigin<Self::AccountId, I>>;

etc...

All of these types have some complicated traits which are expected from the construct_runtime! macro, and often the user struggles to define all those types when doing runtime development.

We could simply wrap all of these traits in a simple to understand trait exposed in frame_support like:

trait FrameCall: IsType<<Self as frame_system::Config>::Call>
    + Dispatchable<Origin = Self::Origin, PostInfo = PostDispatchInfo>
    + GetDispatchInfo
    + FullCodec
    + From<frame_system::Call<Self>>
{}

Then simply define:

type Call: frame_support::FrameCall

or something like that.

[dharjeezy]

Hello @shawntabrizi is this particular issue available for pick up? I am willing to pick it up

[bkchr]

All of these types have some complicated traits which are expected from the construct_runtime! macro

That is not correct. You add the traits bounds that you are require for a certain type in your pallet. The runtime is only required to provide types that implement the given traits.

[shawntabrizi]

@bkchr I think you are saying what I am saying?

For example, we know construct_runtime! implements a bunch of traits, like From/Into for inner and outer events, inner and outer origin, inner and outer Call type.

We also know that something like the Call type must satisfy a bunch of traits which are expected in other parts of the codebase, like Dispatchable or GetDispatchInfo. The whole thing I am saying is we can take all these assumptions about common types in the FRAME runtime and place them under a single simple trait.

Or what am I misunderstanding?

[bkchr]

That is right what you are saying. I just wanted to highlight that some pallets may only require that a Call implements Encode. Then you also just need to bound for Encode.

[kianenigma]

Related to paritytech/substrate#13454

[kianenigma]

I would say this is done with derive_impl's delivery.

[shawntabrizi]

@kianenigma can you link context / solution?

I saw https://github.com/paritytech/substrate/pull/13454, but does not look like the same thing is being solved.

this is still a common issue I found in the academy. Would love traits for storage items, callable parameters, basic unsigned integers, etc...

[kianenigma]

True, I misjudged this based on the title.

[kianenigma]

If and when done, the author should update https://paritytech.github.io/polkadot-sdk/master/polkadot_sdk_docs/reference_docs/frame_runtime_types/index.html as it goes into a lot of depth about how these Runtime* types and their trait bounds work.

Admittedly. if knowing the above article in depth, you would not feel confused by the trait bounds, but I get that having a simple setup is necessary, especially for something like type RuntimeEvent which is used very very frequently.

[kianenigma]

A solution should look like this :

// in frame/src/lib.rs

mod trait_bounds {
    pub use frame_support::traits::{Parameter, Members};
    // TODO: and fix for instancing
    pub trait TRuntimeEvent<PalletEvent, T> = From<PalletEvent<T>> + IsType<<T as frame_system::Config>::RuntimeEvent>
    pub trait TRuntimeCall<PalletCall, T> = ..;
    // This one is a bit more tricky, as `PalletOrigin` might not exist.
    pub trait TRuntimeOrigin<PalletOrigin, T> = ..;

[bkchr]

Isn't this going to be solved by this: https://github.com/paritytech/polkadot-sdk/issues/3743?

Then we can get rid off all these extra RuntimeCall, RuntimeEvent etc in every pallet and just keep it in frame_system. We should even be able to let the macro generate this for us. Thus, users will not need to write these extra bounds at all.

[shawntabrizi]

@bkchr agree that will def solve a lot of the stuff inside the config.

But I think there is much more overall that can be improved.

From my original issue, I want to change things like:

trait FrameCall: IsType<<Self as frame_system::Config>::Call>
    + Dispatchable<Origin = Self::Origin, PostInfo = PostDispatchInfo>
    + GetDispatchInfo
    + FullCodec
    + From<frame_system::Call<Self>>
{}

To be a single trait which already has all the requirements we need, like:

type Call: frame_support::FrameCall

Again, coming from someone who does not know so deeply about all these different traits, we can def improve on things.

Another example, in the same idea, is to create more easy to use derive macros.

For example, instead of:

#[derive(Encode, Decode, MaxEncodedLen, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct SomeStructForStorage<T: Config> {}

We should be able to do:

#[derive(SubstrateStorage(T))]
pub struct SomeStructForStorage<T: Config> {}

I think this captures what I originally intended with the issue, and as long as we use valid rust syntax and rules (not a ton of macro magic), I think this will be a better experience for everyone.

[kianenigma]

@shawntabrizi the storage thing you wished for will be solved by https://github.com/paritytech/polkadot-sdk/pull/4079