-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Support for sending funds to the community pool #5249
Support for sending funds to the community pool #5249
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5249 +/- ##
==========================================
+ Coverage 54.55% 54.64% +0.08%
==========================================
Files 311 311
Lines 18653 18529 -124
==========================================
- Hits 10176 10125 -51
+ Misses 7700 7631 -69
+ Partials 777 773 -4
|
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.
Thanks @RiccardoM, I would like to see the following:
- A changelog entry
- A test case testing you can send to the community pool address and only that address
Thanks for the contribution!
…pool-funding � Conflicts: � simapp/app.go
I've implemented the tests. The only thing missing is the changelog. Under which section and tag should I put it? |
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.
ACK -- @RiccardoM you can simply add the following under Features
and state-machine-breaking
in the changelog:
* (modules) [\#5249](https://github.com/cosmos/cosmos-sdk/pull/5249) Funds are now allowed to be directly sent to the community pool (via the distribution module account).
Done! :D |
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.
Apologies @RiccardoM, but after testing this, I've realized that this solution is not correct as it turns out that the community fund pool is unique and not an actual module account.
We'll need to adjust this PR slightly. We'll need a custom message type that sends funds from an account to the distribution module account AND also adds the funds to the pool itself:
k.supplyKeeper.SendCoinsFromAccountToModule(ctx, sender, types.ModuleName, amount)
feePool.CommunityPool = feePool.CommunityPool.Add(amount)
k.SetFeePool(ctx, feePool)
@RiccardoM do you need assistance on this PR? We can complete it together if you wish. |
@alexanderbez Sorry, been very busy with other stuff. It should be mostly done now |
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.
Looks great @RiccardoM -- just a few bits of minor feedback.
Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
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.
ACK -- I think the message type name can change, but we can do that in another PR.
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.
Minor nits mostly. Please add the msg validation on the cli as well
I'm addressing all comments in a subsequent PR @fedekunze + adding support for simulation. |
Description
Closes #5206
To do
Checklist
docs/
)Unreleased
section inCHANGELOG.md
Files changed
in the github PR explorerFor Admin Use: