-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
R4R: Add slashing genesis validation #3158
Conversation
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.
@jackzampolin you need to update the simulated random genesis parameters for this I presume (see the x/auth param store PR as an example).
Codecov Report
@@ Coverage Diff @@
## develop #3158 +/- ##
===========================================
- Coverage 55.05% 54.93% -0.13%
===========================================
Files 133 133
Lines 9438 9459 +21
===========================================
Hits 5196 5196
- Misses 3922 3943 +21
Partials 320 320 |
@alexanderbez You are right! Only saw these errors when I pushed to CI. Don't think I was running the right sims locally. I've fixed the issue. |
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.
One minor change I think, but otherwise LGTM. Also warrants a review from @cwgoes and/or @rigelrozanski on the actual validation of params (are they stated in spec anywhere?).
cmd/gaia/app/sim_test.go
Outdated
@@ -167,6 +167,10 @@ func appStateFn(r *rand.Rand, accs []simulation.Account) json.RawMessage { | |||
return appState | |||
} | |||
|
|||
func between(r *rand.Rand, min, max int) int { |
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.
I'd call this randIntRange
.
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.
or at least randIntBetween
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.
Changes LGTM, parameter ranges seem reasonable, agreed on renaming between
.
cmd/gaia/app/sim_test.go
Outdated
@@ -167,6 +167,10 @@ func appStateFn(r *rand.Rand, accs []simulation.Account) json.RawMessage { | |||
return appState | |||
} | |||
|
|||
func between(r *rand.Rand, min, max int) int { |
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.
or at least randIntBetween
PENDING.md
with issue #Files changed
in the github PR explorerFor Admin Use: