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

refactor: pin runtime pallet ids #363

Merged
merged 3 commits into from
Feb 7, 2022
Merged

refactor: pin runtime pallet ids #363

merged 3 commits into from
Feb 7, 2022

Conversation

enthusiastmartin
Copy link
Contributor

@enthusiastmartin enthusiastmartin commented Feb 7, 2022

PR makes sure that each pallet in runtime is assigned persistent id.

This makes sure that module ids are the same when a pallet is either removed or added.

I decided to group ids as follows:

  • starting at 0 - substrate related pallets
  • starting at 50 - parachain and xcm related stuff
  • stating at 100 - basilisk pallets and warehouse pallets
  • starting at 150 - orml
  • starting at 200 - testing stuff - pallets which are not present in basilisk, only in testing runtime

The reasons being:

  • allows quicker identify area of an error
  • allows adding new pallets to be together with related stuff
  • order is also important - therefore having some spaces prevents unwanted shifts in future.

Note that the order of pallets is important.

Eg.

  • Session must be after collator selection and prior to aura.
  • Asset registry must be before orml-tokens otherwise it can cause issue with existential deposit.

fixes #346

@github-actions
Copy link

github-actions bot commented Feb 7, 2022

Crate versions that have been updated:

  • basilisk-runtime: v35.0.0 -> v36.0.0
  • testing-basilisk-runtime: v35.0.0 -> v36.0.0

Runtime versions don't match.

Runtime version has been increased.

@Roznovjak
Copy link
Collaborator

What about moving testing stuff to 250 or 300 and leaving some space for potential new groups? It's just an idea.

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #363 (db2a030) into master (c2728a6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #363   +/-   ##
=======================================
  Coverage   82.02%   82.02%           
=======================================
  Files          20       20           
  Lines        2075     2075           
=======================================
  Hits         1702     1702           
  Misses        373      373           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2728a6...db2a030. Read the comment docs.

@enthusiastmartin
Copy link
Contributor Author

What about moving testing stuff to 250 or 300 and leaving some space for potential new groups? It's just an idea.

it is u8. max is 255.

@enthusiastmartin enthusiastmartin linked an issue Feb 7, 2022 that may be closed by this pull request
@mrq1911 mrq1911 changed the title feat: pin runtime pallet ids refactor: pin runtime pallet ids Feb 7, 2022
@mrq1911 mrq1911 merged commit b8fc426 into master Feb 7, 2022
@jak-pan jak-pan deleted the feat/runtime-ids branch February 22, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persistent id's in Runtime enum
3 participants