Removes production configs for derive_impl and updates system config

[gupnik]

This PR adds the following changes:

1. As discussed in the forum post, this PR removes the usage of derive_impl from production runtimes. Consequently, it removes SolochainDefaultConfig, ParachainDefaultConfig and RelaychainDefaultConfig. TestDefaultConfig would continue to be used in test runtimes and templates.
2. As @seadanda suggested here, this PR updates the default for Lookup to sp_runtime::traits::IdentityLookup<Self::AccountId> to avoid the need to override in most cases.

[kianenigma]

what is the downside if we keep ParachainDefaultConfig/SolochainDefaultConfig but it keeps only the uncontroversial stuff, like PalletInfo, Runtime*?

Seems a bit backwards to me, although I have not followed the conversation where this was concluded.

[kianenigma]

What I see here is also that there were indeed mistakes in the previous version of this type:

1. type AccountId should be Multi* for maximum compatibility
2. type SystemWeightInfo/WeightInfo = (); is not a good production advice.
3. same as type SS58Prefix and Version, you don't want a production runtime to have these set to ()

[kianenigma]

How about this:

• TestDefaultConfig which fills as many as types as it can, and it fakes them
• DefaultConfig only fills the uncontroversial stuff

I think there is still a cateogry of people that want to build a real runtime, perhaps for a Local network, so they want realistic, but non-production types. TestDefaultConfig will not be okay for this, namely because of its AccountId.

[gupnik]

what is the downside if we keep ParachainDefaultConfig/SolochainDefaultConfig but it keeps only the uncontroversial stuff, like PalletInfo, Runtime*?

The major concern here was the impact on downstream impls as the defaults could be added/modified without their knowledge. The current infra for derive_impl needs us to provide defaults for all params that are not marked with #[pallet::no_default] and its not possible to choose these for each config. So, ParachainDefaultConfig would still need to override all such params, which might eventually find there way in the downstream projects.

It would be possible if we could annotate #[pallet::no_default] with a config type. This would allow us to keep most params open to defaults for TestConfigs but restricted for others.

I think there is still a cateogry of people that want to build a real runtime, perhaps for a Local network, so they want realistic, but non-production types. TestDefaultConfig will not be okay for this.

As long as it is for a local network, why would TestDefaultConfig not be okay? I believe it clearly signifies the fact that it should be changed before going to production?

[seadanda]

Could a good compromise be to just have a TestDefaultConfig in the pallet itself, and then have a config shared by our testnet system parachains in testnets-common or parachains-common (something that I started to do in #4056 anyway) which people could then use and inherit from as a sensible baseline. Likewise for the testnet relays. The fellowship repo would have its own DefaultConfigs defined for production networks.
For extra visibility of changes we could even version the config so people would be aware of changes to existing values, but when we add a new config item we could just use a new sensible value within the same version.
WDYT? This changes the contract from "here's a stable sensible baseline for you to adapt which we'll try to keep up to date" to "this is the config we use for the actual system chains"

[kianenigma]

The major concern here was the impact on downstream impls as the defaults could be added/modified without their knowledge. The current infra for derive_impl needs us to provide defaults for all params that are not marked with #[pallet::no_default] and its not possible to choose these for each config.

I see. This is a real issue, but I am inclined to suggest that there should be a trick to just fill these things that have to be overwritten in with a struct UnImplemented, which would do nothing other than raising a compiler error, prompting users to overwrite it.

As long as it is for a local network, why would TestDefaultConfig not be okay?

TestDefaultConfig is optimized for in-code testing, and sometimes this diverges from real-life-testing. For example the AccountId type is one instance of this.

[gupnik]

then have a config shared by our testnet system parachains in testnets-common or parachains-common (something that I started to do in #4056 anyway) which people could then use and inherit from as a sensible baseline.

Yes, this makes sense to me. I think @kianenigma also previously mentioned keeping the configs separate that could then be reused by other folks as needed.

[gupnik]

I see. This is a real issue, but I am inclined to suggest that there should be a trick to just fill these things that have to be overwritten in with a struct UnImplemented, which would do nothing other than raising a compiler error, prompting users to overwrite it.

Let me think about this a bit more.

TestDefaultConfig is optimized for in-code testing, and sometimes this diverges from real-life-testing. For example the AccountId type is one instance of this.

True, but I was imagining this to be a minimal change. For example, I had to override AccountId and Lookup to make it work with the minimal template.

[muharem]

@seadanda I like your idea, should be useful for system chains

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6392135

[muharem]

@seadanda I like your idea, should be useful for system chains

[muharem]

something wrong with formatting, there are few places like this