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

Improve keeper field validation #766

Merged
merged 4 commits into from
Mar 14, 2023
Merged

Conversation

shaspitz
Copy link
Contributor

@shaspitz shaspitz commented Mar 13, 2023

Description

Builds off #740, with improved validation that checks for keeper fields being nil valued.

Details

Note there's a weird intricacy when calling reflect.ValueOf(x).IsZero(), where ValueOf() returns a zero valued reflect.Value if x is nil, which panics IsZero() in a way that doesn't print an easily debuggable panic message. This PR doesn't change panicking functionality, but it does ensure that a useful panic message is printed when a keeper field is nil. mustValidateFields only exists for ease of debugging anyways.

Type of change

  • Refactor: Changes existing code style, naming, structure, etc.

Regression tests

Existing UTs act as regression tests

New behavior tests

n/a

Versioning Implications

If the above box is checked, which version should be bumped?

  • MAJOR: Consensus breaking changes to both the provider and consumers(s), including updates/breaking changes to IBC communication between provider and consumer(s)
  • MINOR: Consensus breaking changes which affect either only the provider or only the consumer(s)
  • PATCH: Non consensus breaking changes

Targeting

Please select one of the following:

  • This PR is only relevant to main
  • This PR is relevant to main, and should also be back-ported to ____ (ex: v1.0.0 and v1.1.0)
  • This PR is only relevant to ____ (ex: v1.0.0, v1.1.0, and v1.2.0)

@shaspitz shaspitz changed the title improve validation msg Improve keeper field validation Mar 13, 2023
@shaspitz shaspitz marked this pull request as ready for review March 13, 2023 19:08
@shaspitz shaspitz merged commit 2ab8d1b into main Mar 14, 2023
@shaspitz shaspitz deleted the improved-keeper-field-validation branch March 14, 2023 17:03
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.

3 participants