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

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

Closed
gui1117 opened this issue Feb 9, 2021 · 4 comments · Fixed by #9681
Labels
J0-enhancement An additional feature request. Z3-substantial Can be fixed by an experienced coder with a working knowledge of the codebase.

Comments

@gui1117
Copy link
Contributor

gui1117 commented Feb 9, 2021

Instead of having to declare:

construct_runtime!(
...
Example: example::{Module, Storage, Call},
);

we could be able to do:

construct_runtime!(
...
Example: example all_parts_except::{Call},
// which would result in using `Module, Storage` in this example
);

(syntax must be improved)

a stale PR exist #6494 but doesn't implement the except_part logic.

@gui1117 gui1117 added U3-nice_to_have Issue is worth doing eventually. Z3-substantial Can be fixed by an experienced coder with a working knowledge of the codebase. J0-enhancement An additional feature request. and removed U3-nice_to_have Issue is worth doing eventually. labels Feb 9, 2021
@KiChjang
Copy link
Contributor

KiChjang commented May 21, 2021

Are there any cases where a pallet part is included in a pallet, but the user doesn't want to declare it to be used? If not, then we could automatically declare the existence of a pallet part simply via the inclusion of its corresponding attribute, i.e. this pallet:

#[frame_support::pallet]
pub mod pallet {
    #[pallet::pallet]
    pub struct Pallet<T>(_);

    #[pallet::config]
    pub trait Config: frame_system::Config {}

    #[pallet::call]
    impl<T: Config> Pallet<T> {}
}

would automatically declare Pallet, Call and nothing else -- the #[pallet::*] attribute macros would automatically declare those parts. We can then remove the need for the user to explicitly declare pallet parts via the decl_construct_runtime_args! macro as implemented in #6494.

Then if a user tries to use a non-existent pallet part during runtime construction, we could then generate a more helpful error message saying something like "Pallet does not contain #[pallet::genesis_config], declare one in order to include Config for the pallet in the runtime".

@gui1117
Copy link
Contributor Author

gui1117 commented May 25, 2021

indeed I think decl_construct_runtime_args! of #6494 should be automatically generated by the pallet macro.

Then if a user tries to use a non-existent pallet part during runtime construction, we could then generate a more helpful error message saying something like "Pallet does not contain #[pallet::genesis_config], declare one in order to include Config for the pallet in the runtime".

I also agree and on that, but that would be not possible to support in old decl_* macros, so as long as it is not deprecated I'm not sure how it can be implemented.

But in general yes I think pallet macro declares pallet items, those items are to be aggregated in construct_runtime.
I think forcing construct_runtime to read exclusively items declared by pallet macro, allows better error message and more consistent code.

Also I don't think we need to support declaration of parts for decl_* macros. no need to enhance decl_* macros, but we shouldn't break them IMHO

@stale
Copy link

stale bot commented Jul 7, 2021

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.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 7, 2021
@gui1117
Copy link
Contributor Author

gui1117 commented Jul 8, 2021

still relevant

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. Z3-substantial Can be fixed by an experienced coder with a working knowledge of the codebase.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants