-
Notifications
You must be signed in to change notification settings - Fork 119
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
feat: add message to withdraw emission rewards #1811
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1811 +/- ##
===========================================
+ Coverage 38.03% 38.70% +0.66%
===========================================
Files 205 208 +3
Lines 12524 12623 +99
===========================================
+ Hits 4764 4886 +122
+ Misses 7372 7349 -23
Partials 388 388
|
…into withdraw-emissions
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'm not sure to understand.
Why creating a withdraw object in the message and processing it in the endblocker, instead of directly processing it in the message?
What does the WithdrawEmission
object brings on top of WithdrawableEmission
?
defer func() { | ||
for _, withdrawEmission := range allWithdrawEmissions { | ||
keeper.DeleteWithdrawEmissions(ctx, withdrawEmission.Address) | ||
} | ||
}() |
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 using a defer here? It seems we never return an error in the function body
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 used it just for code readability.
This is to prevent more than one emission withdraw in the same block. Although I can't really think of a condition which,would allow observers to withdraw more rewards than they own , Using the endblock and processing all withdraws in the block at once makes the logic a bit safer |
I don't get it. We have a Why not directly having
Why is it a problem if we verify that the withdrawn amount is below withdrawable amount?
How does using a intermediate end blocker add more protection for this? |
Closing pr for now to make changes mentioned above cc @lumtis |
Description
Closes: #1653
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.
Checklist: