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

Add ReserveTax and ReserveTaxPool #583

Closed
zramsay opened this issue Mar 5, 2018 · 7 comments
Closed

Add ReserveTax and ReserveTaxPool #583

zramsay opened this issue Mar 5, 2018 · 7 comments
Milestone

Comments

@zramsay
Copy link
Contributor

zramsay commented Mar 5, 2018

ref: cosmos/gaia#61

closed (?) from spec in feature/provisions branch in gaia repo

@cwgoes
Copy link
Contributor

cwgoes commented Apr 6, 2018

@cwgoes
Copy link
Contributor

cwgoes commented Apr 9, 2018

Should ReserveTax and ReservePool be kept in the staking state?

Where can ReserveTax fee deduction be implemented - the x/auth ante handler, where fees are deducted? Ideally it would be specific to a Cosmos Hub module, as presumably most zones won't want this kind of reserve pool

@rigelrozanski
Copy link
Contributor

From slack Convo:

rige
What you’re saying is actually incorrect - the fee collection should be in the AnteHandler, but the fee-reward distribution logic is a part of the staking module

cwgoes
OK, wasn't sure, at the moment deductFees just erases coins from existence - so those should go into a common FeePool account then, then get redistributed later in the staking module? (edited)


In the fee pool design for staking there is already a fee pool account - maybe we should pass in some limited keeper to the antehandler which informs where to move the fees too... Or maybe just an account address would be enough. If it was an interface on the keeper being passed in, we could allow for the staking keeper to execute arbitrary logic as to how the fees taking is handled. For instance, I don't the the fee pool should be an actual "account" it only needs to be a Coins which is accessible only from protocol.

@cwgoes
Copy link
Contributor

cwgoes commented Apr 10, 2018

Can we separate the fee pool and the staking implementation of fee distribution if possible? Charging fees on transactions - and sending those fees somewhere, so the coins aren't just destroyed - is something most (all?) applications using the Cosmos SDK will want to do, whereas the ReserveTax / ReservePool are Cosmos Hub-specific features.

How about the AccountMapper in x/auth, which handles fee logic in its AnteHandler, also keeps track of the current fees - without a special account, just as a Coins value incremented when fees are deducted in deductFees, and exposes functions which other modules can use to modify the fee pool? x/stake, for example, could then allocate fees at the end of every block (or whatever interval is desired, possibly some efficiency concerns) and reset the AccountMapper's fee pool to zero.

@rigelrozanski
Copy link
Contributor

Can we separate the fee pool and the staking implementation of fee distribution if possible

of course, but I don't see the problem with requiring passing in an function to the feeHandler in auth which dictates what happens to the fees. - yeah maybe it can already happen through AccountMapper somehow - need to dig in - also need to expose this interface in baseapp I'd imagine, we want it's requirement to be fulfilled at a high level

@cwgoes
Copy link
Contributor

cwgoes commented Apr 16, 2018

Fee calculation / tracking / allocation doc reference - https://github.com/cosmos/cosmos-sdk/blob/master/docs/spec/staking/old/spec2.md#fee-calculations

@sunnya97 Did you want to work on fees (you mentioned)? I think this has to be done as part of the full fee implementation, it isn't a standalone task.

@jackzampolin
Copy link
Member

Going to close this issue as addressed by #2236

MSalopek pushed a commit to MSalopek/cosmos-sdk that referenced this issue Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants