Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Impose new restrictions on paras init and cleanup #4360

Merged
merged 4 commits into from
Nov 26, 2021
Merged

Conversation

pepyakin
Copy link
Contributor

First PR of the #3211 series.
Related #4322

For upcoming PVF pre-checking feature we will need to impose a couple of
new restrictions for:

  • schedule_para_initialize.
  • schedule_para_cleanup.

Specifically, for the former we do not want to allow registration of
wasm blob that is empty, i.e. 0 bytes. While that currently already
does not make a lot of sense, it allows us to simplify the PVF
pre-checking logic: if this PR is deployed before the following changes
for PVF prechecking then we can be sure that no paras onboarding have to
have to go through the PVF pre-checking. In case, we deploy it
altogether this property will allow us to distingush paras that came in
before PVF pre-checking.

For schedule_para_cleanup we do not want to allow offboarding of paras
that are undergoing the upgrade process. While this is not a harsh
restriction this change allows us to avoid making the PVF prechecking
more complicated than it has to be.

@pepyakin pepyakin added A0-please_review Pull request needs code review. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Nov 24, 2021
Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine to me, not sure I can judge the broader implications (because I'm not deeply familiar with the paras_registrar)

runtime/parachains/src/paras.rs Show resolved Hide resolved
runtime/common/src/paras_registrar.rs Show resolved Hide resolved
@pepyakin pepyakin force-pushed the pep-new-constraints branch 2 times, most recently from 580a028 to b947c90 Compare November 24, 2021 13:55
runtime/parachains/src/paras.rs Outdated Show resolved Hide resolved
@pepyakin pepyakin changed the base branch from master to pep-annoying-pruning-log November 25, 2021 13:08
@pepyakin
Copy link
Contributor Author

pepyakin commented Nov 25, 2021

Current dependencies on/for this PR:

This comment was auto-generated by Graphite and will continue to be automatically updated while this PR remains open.

For upcoming PVF pre-checking feature we will need to impose a couple of
new restrictions for:

- `schedule_para_initialize`.
- `schedule_para_cleanup`.

Specifically, for the former we do not want to allow registration of
wasm blob that is empty, i.e. 0 bytes. While that currently already
does not make a lot of sense, it allows us to simplify the PVF
pre-checking logic: if this PR is deployed before the following changes
for PVF prechecking then we can be sure that no paras onboarding have to
have to go through the PVF pre-checking. In case, we deploy it
altogether this property will allow us to distingush paras that came in
before PVF pre-checking.

For `schedule_para_cleanup` we do not want to allow offboarding of paras
that are undergoing the upgrade process. While this is not a harsh
restriction this change allows us to avoid making the PVF prechecking
more complicated than it has to be.
Now they link to their original declarations in the pallet for more
details.
@pepyakin pepyakin changed the base branch from pep-annoying-pruning-log to master November 25, 2021 13:21
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants