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

Relax genesis config to not require Default impl #1221

Merged
merged 3 commits into from
Aug 29, 2023

Conversation

nazar-pc
Copy link
Contributor

There wasn't any meaningful "default" for one of the pallets I'm working with, so I looked at why it is required and I didn't find a good reason for that.

@paritytech-ci paritytech-ci requested review from a team August 28, 2023 14:53
@bkchr bkchr requested a review from michalkucharczyk August 28, 2023 22:06
@bkchr bkchr added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Aug 28, 2023
@paritytech-ci paritytech-ci requested a review from a team August 28, 2023 22:07
@ggwpez ggwpez enabled auto-merge (squash) August 29, 2023 11:06
@ggwpez ggwpez merged commit 430edd7 into paritytech:master Aug 29, 2023
@nazar-pc nazar-pc deleted the relax-genesis-config-default branch August 29, 2023 17:44
@ggwpez ggwpez mentioned this pull request Aug 29, 2023
bkchr pushed a commit that referenced this pull request Aug 30, 2023
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Daanvdplas pushed a commit that referenced this pull request Sep 11, 2023
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@nazar-pc
Copy link
Contributor Author

nazar-pc commented Sep 14, 2023

Hm, looks like construct_runtime still requires Default implementation on pallet::GenesisConfig for some reason 🤔

@michalkucharczyk
Copy link
Contributor

Hm, looks like construct_runtime still requires Default implementation on pallet::GenesisConfig for some reason 🤔

Can you share more details? (with this env enabled, maybe it will show some more details)?

@nazar-pc
Copy link
Contributor Author

No, I'm just running cargo clippy --all-targets without FRAME_EXPAND and getting this:

error[E0277]: the trait bound `pallet::GenesisConfig<mock::Test>: std::default::Default` is not satisfied
   --> crates/pallet-subspace/src/mock.rs:70:1
    |
70  | // frame_support::construct_runtime!(
71  | ||     pub struct Test {
72  | ||         System: frame_system,
73  | ||         Balances: pallet_balances,
...   ||
77  | ||     }
78  | || );
    | ||_- in this macro invocation
...   |
    |
    = note: this error originates in the derive macro `Default` which comes from the expansion of the macro `frame_support::construct_runtime` (in Nightly builds, run with -Z macro-backtrace for more info)

@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented Sep 14, 2023

It originates here:

#[derive(#scrate::__private::serde::Serialize, #scrate::__private::serde::Deserialize, Default)]

Default for sure will be required if the runtime is supposed to support GenesisBuilder API.

@nazar-pc
Copy link
Contributor Author

I don't see why, we create RuntimeGenesisConfig struct explicitly and not using Default, why is Default a hard requirement there? Looks like it should be removed.

@michalkucharczyk
Copy link
Contributor

It has been there for a while. IMO it can be relaxed.

@nazar-pc
Copy link
Contributor Author

nazar-pc commented Sep 14, 2023

I'll look into it later then, thanks!

bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
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.

7 participants