PalletId should be hardcoded into Pallet, not exposed as Config

[shawntabrizi]

PalletId is intended to be a unique global identifier for a Pallet, and should not be configurable in the Pallet Config.

Wherever PalletId is exposed in the runtime config, move it into the Pallet as a const and make sure that const is exposed via the metadata.

Make sure there are no breaking changes for Polkadot / Kusama

cc @gavofyork

[bkchr]

PalletId is intended to be a unique global identifier for a Pallet, and should not be configurable in the Pallet Config.

The same as we had with the storage prefix? Why?

We switched to using the name of the pallet in the runtime to prevent clashing of the identifiers. How should we prevent here that someone uses the same identifier?

[gavofyork]

(Pallet ID, instance ID) pairs are intended to be unique within a runtime. As long as this holds, then there's no reason to clog up runtimes with another trait config item. If there's some reason this is not achievable, then the decision could be revisited.

[thiolliere]

in pallet macro we don't bound the Instance trait since we don't use it to get the pallet prefix to use for storages.
(Instead we only bound 'static because we need Pallet<T, I> to be static in order to call PalletInfo::name::<Pallet<T, I>>())

so there is no id associated to the instance.

But we can use PalletInfo::index to get a unique index for the pallet.
<T as frame_system::Config::PalletInfo>::index::<Pallet<T, I>>().expect("pallet is part of the runtime")

Though I agree would be better to have fn index directly implemented on Pallet by the macro which would call that inside (or anything so we can quickly get access to the index).

[gavofyork]

Either way, I would prefer that a unique ID per pallet instance be provided by Frame rather than be passed in through a config trait type.

[thiolliere]

paritytech/substrate#8630 give easy accessor to pallet index and pallet name.

Such index and name are unique for the pallet and can be used through PalletInfoAccess trait which is implemented for the Pallet struct.

Pallet::<T>::index();

The name and the index are part of the metadata.

Thus I think this issue is solved

[shawntabrizi]

@thiolliere also needs to update existing pallets which do expose module id to close this fully

[stale]

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.

[ggwpez]

Also needs proper testing to ensure that we dont break existing deposits in pallet accounts.

[xlc]

I don’t know how it is possible to implement this without potentially breaking existing chains? And we should not require chains to perform migration for sake of nothing