-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
x/distribution: fix module's parameters validation #8918
Conversation
9115634
to
af872fe
Compare
Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=af872fe5f64c7948d234bb1d69a69188b81ed08f |
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.
utACK
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.
🎉
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=6fa12a9c2bff3e7f1fa2795f18f1c8dbdf351ba5 |
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=46d19e00a00c4164bfba53cc89192943 |
Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=b01cd9a2ae0d79c2c9e58228550c8315b5e1f41a |
Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=366e964a6dbc447cba8b9cfb917d6b60 |
@zmanian @shahankhatch this looks state-breaking, i.e. parameters that were valid before will no longer be so once this patch is merged in. It must be said that even though this patch might potentially break applications state, any application that uses such invalid parameters would be anyway misusing the distribution module. Here is the conundrum: should we backport it and include it in v0.42.2 or should we be gentle and mindful about those clients who misuse the Cosmos SDK? |
Codecov Report
@@ Coverage Diff @@
## master #8918 +/- ##
==========================================
+ Coverage 59.17% 59.24% +0.06%
==========================================
Files 571 571
Lines 31807 31807
==========================================
+ Hits 18822 18844 +22
+ Misses 10782 10760 -22
Partials 2203 2203
|
Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=2e20265a9a9a4dfeaf80d42b52612818 |
Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=1fddce73b361e1b9442811e36a401e8501a87468 |
closes: #8914
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes