-
Notifications
You must be signed in to change notification settings - Fork 189
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
Added checkvalue function and testcase for AddVote api #1109
Conversation
governance/handler.go
Outdated
if key == "istanbul.committeesize" && val == uint64(0) { | ||
return false | ||
} | ||
if key == "reward.minimumstake" { | ||
if v, ok := new(big.Int).SetString(val.(string), 10); ok { | ||
if v.Cmp(common.Big0) <= 0 { | ||
return false | ||
} | ||
} | ||
} |
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.
Why do you check only for these 2 governance items?
Are other items good to go with no value-check in this function?
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.
Other items can be discussed and added if there are problems.
947a75f
Codecov Report
@@ Coverage Diff @@
## dev #1109 +/- ##
==========================================
+ Coverage 61.30% 61.34% +0.04%
==========================================
Files 478 478
Lines 62618 62620 +2
==========================================
+ Hits 38386 38413 +27
+ Misses 21512 21489 -23
+ Partials 2720 2718 -2
Continue to review full report at Codecov.
|
Proposed changes
This PR implemented function to check the value of AddVote api and added test cases for AddVote function.
In the previous governance voting, voting was possible with a value that should not be applied.
Currently, if an api call is made for an invalid value as shown below, the call is blocked.
AS-IS
TO-BE
Closes #1107
Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test
)Related issues
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...