R4R: Checking Vesting Coins with AmountOf #3486
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
zmanian
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
…oins for a more expensive but correct call to AmountOf
zmanian
force-pushed
the
zaki/VestingAmountof
branch
from
5d586d0 to
c256b10
Compare
Codecov Report
@@ Coverage Diff @@
## develop #3486 +/- ##
===========================================
+ Coverage 52.95% 52.97% +0.01%
===========================================
Files 137 137
Lines 10746 10719 -27
===========================================
- Hits 5691 5678 -13
+ Misses 4705 4698 -7
+ Partials 350 343 -7 |
alexanderbez
approved these changes
Feb 2, 2019
There was a problem hiding this comment.
LGTM -- we should definitely add a test cases for multi-denom accounts. I can add those to this PR or as another. Thoughts @zmanian ?
|
Looks good to me - pending a changelog line and multi-denom coins set test case |
alessio
approved these changes
Feb 2, 2019
cwgoes
previously requested changes
Feb 2, 2019
|
I'll take this over @zmanian 👍 |
alexanderbez
changed the title
|
Is this ready to merge? |
Can we add a testcase or two? |
|
Updated. Should be good to merge @jackzampolin |
4 tasks
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Removes the performance optimization when querying to see if vesting coins are in the correct state and just use AmountOf.
This bug was discovered on Game of Stakes 5.
Fixes #3483 and #3482
Would be benefit from multicoin unit tests.
For Admin Use: