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

A more relaxed way to define DefaultConfig for pallets #5246

Closed
wants to merge 3 commits into from

Conversation

kianenigma
Copy link
Contributor

The underlying problem is this:

In a given pallet, we want to provide defaults for (usually) all types in TestDefaultConfig, but for SolochainDefaultConfig and such, we might want to provide defaults in only a subset of them.

#4689 and #4965 and #4056 have all faces this issue one way or another.

What I have done here is a simple and near zero effort way to fix this. The compromise is that we use a bit more type-safety, but as long as we have at least one test for usage of each register_default_impl, we are good.

The trick is to mark a type that is ambiguous as #[no_default_bound]. Then, we can provide a default that actually works in TestDefaultConfig, and one that does not in production.

The error message will look something like this:

  error[E0277]: the trait bound `you_cannot_use_derive_impl_on_the_associated_type_raising_this_error_please_override_it: polkadot_sdk_frame::prelude::Get<RuntimeDbWeight>` is not satisfied
     --> /Users/kianenigma/Desktop/Parity/polkadot-sdk-2/templates/minimal/runtime/src/lib.rs:133:1
      |
  133 | #[derive_impl(frame_system::config_preludes::SolochainDefaultConfig)]
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `polkadot_sdk_frame::prelude::Get<RuntimeDbWeight>` is not implemented for `you_cannot_use_derive_impl_on_the_associated_type_raising_this_error_please_override_it`
      |
      = help: the following other types implement trait `polkadot_sdk_frame::prelude::Get<T>`:
                <Version as polkadot_sdk_frame::prelude::Get<_I>>
                <NextFeeMultiplierOnEmpty as polkadot_sdk_frame::prelude::Get<FixedU128>>
                <GetDefault as polkadot_sdk_frame::prelude::Get<T>>
                <ConstBool<T> as polkadot_sdk_frame::prelude::Get<bool>>
                <ConstBool<T> as polkadot_sdk_frame::prelude::Get<core::option::Option<bool>>>
                <ConstU8<T> as polkadot_sdk_frame::prelude::Get<u8>>
                <ConstU8<T> as polkadot_sdk_frame::prelude::Get<core::option::Option<u8>>>
                <ConstU16<T> as polkadot_sdk_frame::prelude::Get<u16>>
              and 29 others
  note: required by a bound in `polkadot_sdk_frame::prelude::frame_system::Config::DbWeight`
     --> /Users/kianenigma/Desktop/Parity/polkadot-sdk-2/substrate/frame/system/src/lib.rs:562:18
      |
  562 |         type DbWeight: Get<RuntimeDbWeight>;
      |                        ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Config::DbWeight`
      = note: this error originates in the attribute macro `derive_impl` which comes from the expansion of the macro `frame::deps::frame_support::macro_magic::forward_tokens_verbatim` (in Nightly builds, run with -Z macro-backtrace for more info)

@kianenigma kianenigma requested a review from a team as a code owner August 5, 2024 14:33
@kianenigma kianenigma added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Aug 5, 2024
@kianenigma kianenigma added R0-silent Changes should not be mentioned in any release notes and removed R0-silent Changes should not be mentioned in any release notes labels Aug 5, 2024
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6930409

@kianenigma kianenigma closed this Nov 29, 2024
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.

2 participants