Skip to content
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

Move community pool into its own module #10811

Closed
4 tasks
ValarDragon opened this issue Dec 19, 2021 · 13 comments · Fixed by #17657
Closed
4 tasks

Move community pool into its own module #10811

ValarDragon opened this issue Dec 19, 2021 · 13 comments · Fixed by #17657
Assignees
Labels
C:x/distribution distribution module related

Comments

@ValarDragon
Copy link
Contributor

Summary

Currently the community pool is wrtitten in the Distribution module. However, a module wanting to send coins to the community pool is a pretty common feature. This causes lots of awkward dependencies for modules on the distribution module, which really should just be handling token distribution between modules.

Proposal

I suggest that we split out the community pool into its own, quite small module. This module can even be our reference for how simple a module that just handles a module account is.

I also suggest we make type alias' / re-exports of methods in Distribution of the community pool modules methods, so as to not break existing code.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ValarDragon ValarDragon added discussion C:x/distribution distribution module related labels Dec 19, 2021
@tac0turtle
Copy link
Member

Huge fan of this!! I always found it weird it was combined.

@alexanderbez
Copy link
Contributor

What would actually be in this module? I guess just the community spend proposal handler? There isn't any state machine logic unique to it. The main dep is the module name of distribution. Not opposed to the idea, but i'm just curious

@ValarDragon
Copy link
Contributor Author

ValarDragon commented Dec 20, 2021

community spend proposal handler, fund community pool message & keeper methods.

Really not much!

@alexanderbez
Copy link
Contributor

Which keeper methods?

@ValarDragon
Copy link
Contributor Author

ValarDragon commented Dec 20, 2021

only seem seems to be Fund Community Pool

@alexanderbez
Copy link
Contributor

Ok, why not. ACK 👍

@ValarDragon
Copy link
Contributor Author

ValarDragon commented Jan 1, 2022

Another reason to do this: Currently there is no module account just for the community pool. There is a module account for distribution, which contains both unclaimed commissions, and the community pool assets.

It'll give a better user-flow if folks can monitor the community pool on block explorers just like every other address. (e.g. in Osmosis, the distribution address has 46M osmo https://www.mintscan.io/osmosis/account/osmo1jv65s3grqf6v6jl3dp4t6c9t9rk99cd80yhvld, but the community pool only has 40M. Theres 6M in unclaimed commissions)

Thanks to @czarcas7ic for pointing this out

@alexanderbez
Copy link
Contributor

Agreed.

@aaronc
Copy link
Member

aaronc commented Jan 2, 2022

We should probably sync discussion this with the ongoing gov refactor. /cc @cmwaters

@cmwaters
Copy link
Contributor

cmwaters commented Jan 3, 2022

Another idea that was brought up in the governance working group was to have the community pool just be the gov module account that way a community pool spend is just the gov account sending tokens to another account or module

@aaronc
Copy link
Member

aaronc commented Jan 3, 2022

I do question whether the community pool actually needs much more than a gov controlled address as @cmwaters is suggesting. Is there any need to protect it with invariants for instance? I'm not sure there's a good reason people shouldn't be able to send arbitrary coins to it.

@ValarDragon
Copy link
Contributor Author

People should imo be able to send arbitrary coins to it. I believe the current FundCommunityPool message only exists because the community pool is managed as a subset of the distribution module account atm

@tac0turtle
Copy link
Member

this ties into the work that @testinginprod is doing on accounts v2

@github-project-automation github-project-automation bot moved this to 📝 Todo in Cosmos-SDK Sep 1, 2023
@tac0turtle tac0turtle moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK Sep 4, 2023
@github-project-automation github-project-automation bot moved this from 💪 In Progress to 👏 Done in Cosmos-SDK Sep 27, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Done in Cosmos SDK Maintenance Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/distribution distribution module related
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants