-
Notifications
You must be signed in to change notification settings - Fork 119
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
Consistent params validation #394
Merged
Merged
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
shaspitz
requested review from
jtremback,
mpoke,
sainoe and
danwt
as code owners
October 13, 2022 18:55
mpoke
approved these changes
Oct 14, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. See my comments below.
mpoke
reviewed
Oct 17, 2022
sainoe
approved these changes
Oct 18, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just two tiny adjustments in x/ccv/consumer/types/params_test.go
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
danwt
pushed a commit
that referenced
this pull request
Oct 23, 2022
commit 3f9100a Author: Shawn Marshall-Spitzbart <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Thu Oct 20 10:47:27 2022 -0700 Replace hardcoded constants with params (#393) * changes * edits * Update params_test.go * small * Update genesis_test.go * remove "num" from historical entries param * comment * use params p1 * use params p2 * use params p3 * p4 * change default transfer timeout period * Update proposal_test.go * default historical entries * is negative * add test case * forgot one * comment commit 00ea78b Author: Shawn Marshall-Spitzbart <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Thu Oct 20 08:15:57 2022 -0700 Importable e2e tests (#401) commit 9dda4b6 Author: Marius Poke <marius.poke@posteo.de> Date: Tue Oct 18 16:36:08 2022 +0200 Fixing and updating QA plan (#375) * fixing and updating QA plan * add link to 4.07 e2e test * add link to issue * fix typo * updating links * Update docs/quality_assurance.md Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> * remove ambigous concern * Patches qa plan with diff testing changes (#399) * Start changes (to be reverted) * Revert "Start changes (to be reverted)" This reverts commit 330ee3a. * Update QA plan Co-authored-by: Daniel <danwtisdall@gmail.com> * markdown linter * formatting Co-authored-by: Shawn Marshall-Spitzbart <44221603+smarshall-spitzbart@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: Daniel T <30197399+danwt@users.noreply.github.com> Co-authored-by: Daniel <danwtisdall@gmail.com> commit 27e55b4 Author: Shawn Marshall-Spitzbart <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue Oct 18 04:48:26 2022 -0700 Consistent params validation (#394) * changes * Update shared_params.go * Create params_test.go * corrections * remove println * Update x/ccv/consumer/types/params_test.go Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> * Update x/ccv/consumer/types/params_test.go Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> * fixing TestValidateParams Co-authored-by: Marius Poke <marius.poke@posteo.de> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Consumer genesis params were previously not validated on
ValidateGenesis
, now they are. This is consistent with how validation is done for provider genesis. These changes are needed for #393Note genesis param validation may be redundant with the checks that happen internally for
SetParams
inInitGenesis
. An alternative to the current changes in this PR is removing genesis param validation altogether from consumer and provider. No matter what, we should be consistent between consumer and provider