-
Notifications
You must be signed in to change notification settings - Fork 767
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
omni-node: add metadata checks for runtime/parachain compatibility #6450
Merged
iulianbarbu
merged 71 commits into
paritytech:master
from
iulianbarbu:ib-omni-node-ps-check
Dec 12, 2024
Merged
omni-node: add metadata checks for runtime/parachain compatibility #6450
iulianbarbu
merged 71 commits into
paritytech:master
from
iulianbarbu:ib-omni-node-ps-check
Dec 12, 2024
+510
−195
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
iulianbarbu
changed the title
omni-node: check if parachain system pallet exists
omni-node: add metadata checks for runtime/parachain compatibility
Nov 12, 2024
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
lexnv
reviewed
Nov 12, 2024
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
lexnv
reviewed
Nov 18, 2024
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 12, 2024
# Description Upgrades parity-publish with fixes for `Error: no dep` and panic triggered when running `plan` and there is a new unpublished member crate introduced in the workspace. ## Integration N/A ## Review Notes Context: #6450 (comment) Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Merged
via the queue into
paritytech:master
with commit Dec 12, 2024
7cc5cdd
198 of 199 checks passed
github-actions bot
pushed a commit
that referenced
this pull request
Dec 13, 2024
# Description This PR changes a few things: * `--dev` flag will not conflict with `--chain` anymore, but if `--chain` is not given will set `--chain=dev`. * `--dev-block-time` is optional and it defaults to 3000ms if not set after setting `--dev`. * to start OmniNode with manual seal it is enough to pass just `--dev`. * `--dev-block-time` can still be used to start a node with manual seal, but it will not set it up as `--dev` does (it will not set a bunch of flags which are enabled by default when `--dev` is set: e.g. `--tmp`, `--alice` and `--force-authoring`. Closes: #6537 ## Integration Relevant for node/runtime developers that use OmniNode lib, including `polkadot-omni-node` binary, although the recommended way for runtime development is to use `chopsticks`. ## Review Notes * Decided to focus only on OmniNode & templates docs in relation to it, and leave the `parachain-template-node` as is (meaning `--dev` isn't usable and testing a runtime with the `parachain-template-node` still needs a relay chain here). I am doing this because I think we want either way to phase out `parachain-template-node` and adding manual seal support for it is wasted effort. We might add support though if the demand is for `parachain-template-node`. * Decided to not infer the block time based on AURA config yet because there is still the option of setting a specific block time by using `--dev-block-time`. Also, would want first to align & merge on runtime metadata checks we added in Omni Node here: #6450 before starting to infer AURA config slot duration via the same way. - [x] update the docs to mention `--dev` now. - [x] mention about chopsticks in the context of runtime development --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> (cherry picked from commit 48c28d4)
iulianbarbu
added
the
A4-needs-backport
Pull request must be backported to all maintained releases.
label
Dec 13, 2024
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-6450-to-stable2407
git worktree add --checkout .worktree/backport-6450-to-stable2407 backport-6450-to-stable2407
cd .worktree/backport-6450-to-stable2407
git reset --hard HEAD^
git cherry-pick -x 7cc5cdd0d98ae3466dc33b339197c169cf241fc0
git push --force-with-lease |
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-6450-to-stable2409
git worktree add --checkout .worktree/backport-6450-to-stable2409 backport-6450-to-stable2409
cd .worktree/backport-6450-to-stable2409
git reset --hard HEAD^
git cherry-pick -x 7cc5cdd0d98ae3466dc33b339197c169cf241fc0
git push --force-with-lease |
This was referenced Dec 13, 2024
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-6450-to-stable2412
git worktree add --checkout .worktree/backport-6450-to-stable2412 backport-6450-to-stable2412
cd .worktree/backport-6450-to-stable2412
git reset --hard HEAD^
git cherry-pick -x 7cc5cdd0d98ae3466dc33b339197c169cf241fc0
git push --force-with-lease |
iulianbarbu
had a problem deploying
to
subsystem-benchmarks
December 13, 2024 10:31 — with
GitHub Actions
Failure
iulianbarbu
added a commit
that referenced
this pull request
Dec 13, 2024
…6450) Get runtime's metadata, parse it and verify pallets list for a pallet named `ParachainSystem` (for now), and block number to be the same for both node and runtime. Ideally we'll add other pallets checks too, at least a small set of pallets we think right away as mandatory for parachain compatibility. Closes: #5565 Runtime devs must be made aware that to be fully compatible with Omni Node, certain naming conventions should be respected when defining pallets (e.g we verify parachain-system pallet existence by searching for a pallet with `name` `ParachainSystem` in runtime's metadata). Not finding such a pallet will not influence the functionality yet, but by doing these checks we could provide useful feedback for runtimes that are clearly not implementing what's required for full parachain compatibility with Omni Node. - [x] parachain system check - [x] check frame_system's metadata to ensure the block number in there is the same as the one in the node side - [x] add tests for the previous checking logic - [x] update omni node polkadot-sdk docs to make these conventions visible. - [ ] add more pallets checks? --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 16, 2024
# Description Redundant dep that made its way in #6450 . 😅. It can be brought up when using `cargo udeps`. Added a github action that runs `cargo udeps` on the repo too. ## Integration N/A ## Review Notes N/A --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: Bastian Köcher <git@kchr.de>
dudo50
pushed a commit
to paraspell-research/polkadot-sdk
that referenced
this pull request
Jan 4, 2025
…ytech#6646) # Description This PR changes a few things: * `--dev` flag will not conflict with `--chain` anymore, but if `--chain` is not given will set `--chain=dev`. * `--dev-block-time` is optional and it defaults to 3000ms if not set after setting `--dev`. * to start OmniNode with manual seal it is enough to pass just `--dev`. * `--dev-block-time` can still be used to start a node with manual seal, but it will not set it up as `--dev` does (it will not set a bunch of flags which are enabled by default when `--dev` is set: e.g. `--tmp`, `--alice` and `--force-authoring`. Closes: paritytech#6537 ## Integration Relevant for node/runtime developers that use OmniNode lib, including `polkadot-omni-node` binary, although the recommended way for runtime development is to use `chopsticks`. ## Review Notes * Decided to focus only on OmniNode & templates docs in relation to it, and leave the `parachain-template-node` as is (meaning `--dev` isn't usable and testing a runtime with the `parachain-template-node` still needs a relay chain here). I am doing this because I think we want either way to phase out `parachain-template-node` and adding manual seal support for it is wasted effort. We might add support though if the demand is for `parachain-template-node`. * Decided to not infer the block time based on AURA config yet because there is still the option of setting a specific block time by using `--dev-block-time`. Also, would want first to align & merge on runtime metadata checks we added in Omni Node here: paritytech#6450 before starting to infer AURA config slot duration via the same way. - [x] update the docs to mention `--dev` now. - [x] mention about chopsticks in the context of runtime development --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
dudo50
pushed a commit
to paraspell-research/polkadot-sdk
that referenced
this pull request
Jan 4, 2025
# Description Upgrades parity-publish with fixes for `Error: no dep` and panic triggered when running `plan` and there is a new unpublished member crate introduced in the workspace. ## Integration N/A ## Review Notes Context: paritytech#6450 (comment) Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
dudo50
pushed a commit
to paraspell-research/polkadot-sdk
that referenced
this pull request
Jan 4, 2025
…aritytech#6450) # Description Get runtime's metadata, parse it and verify pallets list for a pallet named `ParachainSystem` (for now), and block number to be the same for both node and runtime. Ideally we'll add other pallets checks too, at least a small set of pallets we think right away as mandatory for parachain compatibility. Closes: paritytech#5565 ## Integration Runtime devs must be made aware that to be fully compatible with Omni Node, certain naming conventions should be respected when defining pallets (e.g we verify parachain-system pallet existence by searching for a pallet with `name` `ParachainSystem` in runtime's metadata). Not finding such a pallet will not influence the functionality yet, but by doing these checks we could provide useful feedback for runtimes that are clearly not implementing what's required for full parachain compatibility with Omni Node. ## Review Notes - [x] parachain system check - [x] check frame_system's metadata to ensure the block number in there is the same as the one in the node side - [x] add tests for the previous checking logic - [x] update omni node polkadot-sdk docs to make these conventions visible. - [ ] add more pallets checks? --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
dudo50
pushed a commit
to paraspell-research/polkadot-sdk
that referenced
this pull request
Jan 4, 2025
# Description Redundant dep that made its way in paritytech#6450 . 😅. It can be brought up when using `cargo udeps`. Added a github action that runs `cargo udeps` on the repo too. ## Integration N/A ## Review Notes N/A --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: Bastian Köcher <git@kchr.de>
dudo50
pushed a commit
to paraspell-research/polkadot-sdk
that referenced
this pull request
Jan 4, 2025
paritytech#6450 introduced metadata checks. Supported are metadata v14 and higher. However, of course old chain-specs have a genesis code blob that might be on older version. This needs to be tolerated. We should just skip the checks in that case. Fixes paritytech#6921 --------- Co-authored-by: command-bot <>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A4-needs-backport
Pull request must be backported to all maintained releases.
T9-cumulus
This PR/Issue is related to cumulus.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Get runtime's metadata, parse it and verify pallets list for a pallet named
ParachainSystem
(for now), and block number to be the same for both node and runtime. Ideally we'll add other pallets checks too, at least a small set of pallets we think right away as mandatory for parachain compatibility.Closes: #5565
Integration
Runtime devs must be made aware that to be fully compatible with Omni Node, certain naming conventions should be respected when defining pallets (e.g we verify parachain-system pallet existence by searching for a pallet with
name
ParachainSystem
in runtime's metadata). Not finding such a pallet will not influence the functionality yet, but by doing these checks we could provide useful feedback for runtimes that are clearly not implementing what's required for full parachain compatibility with Omni Node.Review Notes