A0-000: remove pallet randomness #1261
Conversation
| @@ -759,7 +765,6 @@ construct_runtime!( | |||
| UncheckedExtrinsic = UncheckedExtrinsic | |||
| { | |||
| System: frame_system, | |||
| RandomnessCollectiveFlip: pallet_insecure_randomness_collective_flip, | |||
There was a problem hiding this comment.
this pallet shift will probably make some current tools fail (unless they use new codec approach for dynamic encoding like subxt does); I would consider keeping the original indexing - this can be done by explicitly defining pallet indices within this construct_runtime macro (maybe it's sufficient to mark only Scheduler, I'm not sure)
I will summon @DamianStraszak here for support or roast
There was a problem hiding this comment.
I don't know how the explicit indexing works, and if one can have gaps (I would assume so), but since it's quite standard for other chains to use it, I think it would be good to also do it. It requires research ofc... The question is can we add indexing + remove the pallet right away in one update? I think it should be possible, but we should be consistent with the current indexing, which is probably some default one?
There was a problem hiding this comment.
yes yes, we can do it right away, with gaps etc
There was a problem hiding this comment.
Right, then it sounds like a good idea :)
There was a problem hiding this comment.
I'm OK merging this, but I see this as a high-risk change, since I'm not that familiar with runtime internals and I'm not sure how should one go about removing pallets. Also we will be adding a new pallet in the next release which looks kind of risky.
My recommendation would be one of:
- Merge, but prepare a plan for testing this in jira. So that we are sure that after running such an update no pallet breaks or so.
- Merge only indexing of the pallets, and schedule removing it for later. In a release when we are not adding any pallets.
| Identity: pallet_identity, | ||
| CommitteeManagement: pallet_committee_management, | ||
| System: frame_system = 0, | ||
| Scheduler: pallet_scheduler = 2, |
There was a problem hiding this comment.
I'd leave a doc comment about the reason of the gap and a note, that we shouldn't put here any new pallet (in order to avoid any possible confusion with interpreting old/new blocks)
Description
Lets slim down runtime a little bit :) This will need to have migration included before the next release. The migration is quite simple with the newly implement RemovePallet utility from substrate.
Type of change