Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds example for custom default configs #4965

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Jul 8, 2024

Closes #4827

This PR provides an example to create custom default configs for any pallet that could be created externally and shared across runtimes. Such a paradigm can be used in #4056.

It requires the following:

  1. A trait that carries those configuration parameters for a pallet that should be shared across runtimes
  2. An implementation of that trait registered via register_default_impl
  3. Usage of derive_impl at the actual impl site pointing to this default implementation

@gupnik gupnik added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Jul 8, 2024
@gupnik gupnik requested a review from a team as a code owner July 8, 2024 05:24
@gupnik gupnik requested review from kianenigma, muharem and seadanda July 8, 2024 05:26
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the docs, I have no idea how to use this.

substrate/frame/support/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/lib.rs Outdated Show resolved Hide resolved
gupnik and others added 3 commits July 9, 2024 05:13
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <git@kchr.de>
@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 9, 2024 03:14
@gupnik
Copy link
Contributor Author

gupnik commented Jul 9, 2024

Reading the docs, I have no idea how to use this.

Will update and provide more details.

substrate/frame/support/src/lib.rs Outdated Show resolved Hide resolved
@franciscoaguirre
Copy link
Contributor

Should we add this to the polkadot-sdk-docs as well? We should probably have a section on macros, even if it is just a list of macros with links to their rust docs

@gupnik
Copy link
Contributor Author

gupnik commented Jul 10, 2024

Reading the docs, I have no idea how to use this.

Updated.

Screenshot 2024-07-10 at 12 25 12 PM

assert_type_eq_all!(<DummyRuntime as Config>::AccountId, u16);
assert_type_eq_all!(<DummyRuntime as Config>::RuntimeOrigin, ());
assert_type_eq_all!(<DummyRuntime as Config>::RuntimeCall, ());
}
Copy link
Contributor

@kianenigma kianenigma Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

  1. One example that demonstrate how this can be used in a pallet, instead of #[pallet::config(with_default)] to create "partial configs" and skip providing defaults for things that we are not sure about, for example to solve the issue of @seadanda.
  2. (This is the one I am more interested in) One example of how this can be used in a standalone trait. For example, to build a custom trait like:
trait OpinionatedSystem: frame_system::Config { // more types and fns.}
// register default for this. 

#[frame::pallet] mod pallet {
    #[pallet::config]
    pub trait Config: OpinionatedSystem { .. }
}

mod runtime {
    // Use the default provided
    impl pallet::Config for Runtime { .. }
}

And please focus on making both as self-explanatory and e2e as possible?

@kianenigma
Copy link
Contributor

Should we add this to the polkadot-sdk-docs as well? We should probably have a section on macros, even if it is just a list of macros with links to their rust docs

  1. https://paritytech.github.io/polkadot-sdk/master/frame_support/pallet_macros/index.html
  2. https://paritytech.github.io/polkadot-sdk/master/polkadot_sdk_frame/pallet_macros/index.html

Although the latter is not working properly, the re-export is not expanded.

@kianenigma
Copy link
Contributor

Reading #4056 (comment) again, the success scenario here is to make a new trait DefaultConfig for system that has a subset of the other types, that allows #4056 to use it.

The manual approach here works, but it does lose all the type safety, FYI. In that, the trait DefaultConfig needs to manually have the same trait bounds.

@kianenigma
Copy link
Contributor

And if we are to lose type-safety, we can alternatively do #5246.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom default configs using derive_impl
4 participants