fix: WithdrawRewards event emit value when no rewards #9599
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
*returns 0baseDenom in the case of 0 rewards so that a default value is present in event *add test for the case of 0 rewards via 100% commission
| @@ -99,6 +99,14 @@ func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddres | |||
| return nil, err | |||
| } | |||
|
|
|||
| if rewards.IsZero() { | |||
There was a problem hiding this comment.
A zero coin technically isn't valid. I don't have strong feeling here though in terms of emitting the event. I'd have to defer to others.
There was a problem hiding this comment.
IsZero doubles as an IsEmpty check so 0 rewards is the same as no rewards here 🤷🏻♂️
but yeah just to clarify, without the added check, the value simply gets axed from the event
There was a problem hiding this comment.
Totally. I meant in the state-machine we do not allow zero coins. However, since this is for UX/client purposes only, I suppose it's OK.
Codecov Report
@@ Coverage Diff @@
## master #9599 +/- ##
=======================================
Coverage 60.57% 60.57%
=======================================
Files 588 588
Lines 37452 37458 +6
=======================================
+ Hits 22686 22692 +6
Misses 12776 12776
Partials 1990 1990
|
technicallyty
changed the title
amaury1093
approved these changes
Jul 5, 2021
alexanderbez
approved these changes
Jul 5, 2021
amaury1093
added
the
A:automerge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Description
Rewards values of 0 were omitted from the
WithdrawRewardsevent. To provide consistency in event responses, this is caught in the keeper and emitted as a value of0[baseDenom].Closes: #8581
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change