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

Rename "accum" in distribution #2916

Closed
cwgoes opened this issue Nov 27, 2018 · 9 comments · Fixed by #3099
Closed

Rename "accum" in distribution #2916

cwgoes opened this issue Nov 27, 2018 · 9 comments · Fixed by #3099
Assignees
Labels
C:x/distribution distribution module related T: UX

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Nov 27, 2018

Validators already have an "accum" in the Tendermint proposer election algorithm, also giving them an unrelated "accum" in the SDK-side fee distribution logic invites confusion. I think we should rename the latter.

cc @rigelrozanski @alexanderbez

@cwgoes cwgoes added C:x/distribution distribution module related prelaunch T: UX labels Nov 27, 2018
@alexanderbez
Copy link
Contributor

Per @ebuchman we should rename both ideally 😄

@cwgoes
Copy link
Contributor Author

cwgoes commented Nov 27, 2018

Ref tendermint/tendermint#2915

@rigelrozanski rigelrozanski changed the title Rename "accum" Rename "accum" in distribution Nov 27, 2018
@ValarDragon
Copy link
Contributor

How about renaming SDK accum to SharesOfFeeRewards or some similar variant? (I think of it as shares in my head at least)

@rigelrozanski
Copy link
Contributor

rigelrozanski commented Nov 28, 2018

This is definitely some kind of shares mechanism, but it also an accum mechanism (as these units accumulate) - I think that we should maintain "accum" in the title - but maybe prefix it: distrAccum

I'd also be okay with collections -> maybe this is the best, unique from staking shares, and tendermint accum, distribution collections :)

@alexanderbez
Copy link
Contributor

accumRewardShares?

@ValarDragon
Copy link
Contributor

ValarDragon commented Nov 29, 2018

I'm not the biggest fan of the name accum, but its probably not worth the bikeshed, I'm happy with rewardShares, sharesOfFeeReward, accumRewardShares, and distrAccum

(rewardShares / feeRewardShares are my favorite)

I'm kind of confused by collection, since collections are what you get after withdrawing, not the actual share itself. (I do like collectionShares though). also tendermint's accum has been renamed to proposer priority.

@rigelrozanski
Copy link
Contributor

oh I like priority... I'm good with collectionShares

@rigelrozanski
Copy link
Contributor

@cwgoes is this issue relevant in F1?

@cwgoes
Copy link
Contributor Author

cwgoes commented Jan 3, 2019

No; validators don't have an "accum" by any name.

@cwgoes cwgoes mentioned this issue Jan 13, 2019
5 tasks
@cwgoes cwgoes self-assigned this Jan 16, 2019
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 T: UX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants