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

configuration: Unified consistency checks #4581

Merged
merged 1 commit into from
Dec 27, 2021

Conversation

pepyakin
Copy link
Contributor

This commit refactors the consistency checks. Instead of each individual
setter performs its checks locally, we delegate those checks to the
already existing function check_consistency. This removes duplication
and simplifies the logic.

A motivating example of this one is the next PR in the stack that will
introduce a check for a field, which validity depends on the validity of
other two fields. Without this refactoring we will have to place a check
not only to the field in question, but also to the other two fields so
that if they are changed they do not violate consistency criteria. It's
easy to imagine how this can go unwieldy with the number of checks.

@pepyakin
Copy link
Contributor Author

pepyakin commented Dec 22, 2021

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 22, 2021
@pepyakin pepyakin added B0-silent Changes should not be mentioned in any release notes B7-runtimenoteworthy D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. C1-low PR touches the given topic and has a low impact on builders. and removed B0-silent Changes should not be mentioned in any release notes labels Dec 22, 2021
@pepyakin pepyakin force-pushed the pep-configuration-init-refactor branch from 848aa04 to 8937a58 Compare December 22, 2021 15:21
@pepyakin pepyakin force-pushed the pep-unified-consistency-checks branch from 2a56742 to 425bf75 Compare December 22, 2021 15:21
@pepyakin pepyakin added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Dec 22, 2021
Base automatically changed from pep-configuration-init-refactor to master December 22, 2021 16:07
runtime/parachains/src/configuration.rs Show resolved Hide resolved
runtime/parachains/src/configuration.rs Show resolved Hide resolved
This commit refactors the consistency checks. Instead of each individual
setter performs its checks locally, we delegate those checks to the
already existing function `check_consistency`. This removes duplication
and simplifies the logic.

A motivating example of this one is the next PR in the stack that will
introduce a check for a field, which validity depends on the validity of
other two fields. Without this refactoring we will have to place a check
not only to the field in question, but also to the other two fields so
that if they are changed they do not violate consistency criteria. It's
easy to imagine how this can go unwieldy with the number of checks.

This also adds a test that verifies that the default chain spec host
configuration is consistent.
@pepyakin pepyakin force-pushed the pep-unified-consistency-checks branch from 425bf75 to f0223e0 Compare December 24, 2021 14:43
@pepyakin pepyakin added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Dec 27, 2021
@ordian
Copy link
Member

ordian commented Dec 27, 2021

bot merge

@paritytech-processbot paritytech-processbot bot merged commit d8bd492 into master Dec 27, 2021
@paritytech-processbot paritytech-processbot bot deleted the pep-unified-consistency-checks branch December 27, 2021 14:09
pepyakin added a commit that referenced this pull request Dec 27, 2021
This commit incorporates the changes made to the runtime in the
following PRs:

- #4408
- #4457
- #4540
- #4542
- #4581

Note that this PR does not include the description of the PVF
pre-checker subsystem. This should be addressed within
#4611
pepyakin added a commit that referenced this pull request Dec 28, 2021
This commit incorporates the changes made to the runtime in the
following PRs:

- #4408
- #4457
- #4540
- #4542
- #4581

Note that this PR does not include the description of the PVF
pre-checker subsystem. This should be addressed within
#4611

Co-authored-by: sandreim <54316454+sandreim@users.noreply.github.com>
pepyakin added a commit that referenced this pull request Dec 28, 2021
This commit incorporates the changes made to the runtime in the
following PRs:

- #4408
- #4457
- #4540
- #4542
- #4581

Note that this PR does not include the description of the PVF
pre-checker subsystem. This should be addressed within
#4611

Co-authored-by: sandreim <54316454+sandreim@users.noreply.github.com>
pepyakin added a commit that referenced this pull request Dec 29, 2021
This commit incorporates the changes made to the runtime in the
following PRs:

- #4408
- #4457
- #4540
- #4542
- #4581

Note that this PR does not include the description of the PVF
pre-checker subsystem. This should be addressed within
#4611

Co-authored-by: sandreim <54316454+sandreim@users.noreply.github.com>
drahnr pushed a commit that referenced this pull request Jan 4, 2022
This commit refactors the consistency checks. Instead of each individual
setter performs its checks locally, we delegate those checks to the
already existing function `check_consistency`. This removes duplication
and simplifies the logic.

A motivating example of this one is the next PR in the stack that will
introduce a check for a field, which validity depends on the validity of
other two fields. Without this refactoring we will have to place a check
not only to the field in question, but also to the other two fields so
that if they are changed they do not violate consistency criteria. It's
easy to imagine how this can go unwieldy with the number of checks.

This also adds a test that verifies that the default chain spec host
configuration is consistent.
drahnr pushed a commit that referenced this pull request Jan 4, 2022
This commit incorporates the changes made to the runtime in the
following PRs:

- #4408
- #4457
- #4540
- #4542
- #4581

Note that this PR does not include the description of the PVF
pre-checker subsystem. This should be addressed within
#4611

Co-authored-by: sandreim <54316454+sandreim@users.noreply.github.com>
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
This commit refactors the consistency checks. Instead of each individual
setter performs its checks locally, we delegate those checks to the
already existing function `check_consistency`. This removes duplication
and simplifies the logic.

A motivating example of this one is the next PR in the stack that will
introduce a check for a field, which validity depends on the validity of
other two fields. Without this refactoring we will have to place a check
not only to the field in question, but also to the other two fields so
that if they are changed they do not violate consistency criteria. It's
easy to imagine how this can go unwieldy with the number of checks.

This also adds a test that verifies that the default chain spec host
configuration is consistent.
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
This commit incorporates the changes made to the runtime in the
following PRs:

- paritytech#4408
- paritytech#4457
- paritytech#4540
- paritytech#4542
- paritytech#4581

Note that this PR does not include the description of the PVF
pre-checker subsystem. This should be addressed within
paritytech#4611

Co-authored-by: sandreim <54316454+sandreim@users.noreply.github.com>
@louismerlin louismerlin added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jul 6, 2022
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. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants