-
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
[R4R] Slight cleanup of distribution specification #2765
Conversation
docs/spec/distribution/overview.md
Outdated
exclusive. If there are Atom commissions and auto-bonding Atoms, the portion | ||
of Atoms the reward distribution calculation would become very large as the Atom | ||
of Atoms in the reward distribution calculation would become very large as the Atom |
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.
Also, I find the following to be a mouthful and somewhat hard to grasp:
If there are Atom commissions and auto-bonding Atoms, the portion
of Atoms in the reward distribution calculation would become very large as the Atom
portion for each delegator would change each block making a withdrawal of rewards
for a delegator require a calculation for every single block since the last
withdrawal.
Maybe it's just me? 😅
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.
No, it could be better phrased, maybe: If there are both commissions and auto-bonding on the staking token, the amount of staking tokens each validator and delegator has would change each block, so we would have to iterate through all of them
Codecov Report
@@ Coverage Diff @@
## develop #2765 +/- ##
========================================
Coverage 56.68% 56.68%
========================================
Files 156 156
Lines 9788 9788
========================================
Hits 5548 5548
Misses 3862 3862
Partials 378 378 |
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 for the updates, a few more suggestions.
docs/spec/distribution/overview.md
Outdated
exclusive. If there are Atom commissions and auto-bonding Atoms, the portion | ||
of Atoms the reward distribution calculation would become very large as the Atom | ||
of Atoms in the reward distribution calculation would become very large as the Atom |
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.
No, it could be better phrased, maybe: If there are both commissions and auto-bonding on the staking token, the amount of staking tokens each validator and delegator has would change each block, so we would have to iterate through all of them
Updated @cwgoes -- thanks for the feedback. |
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 from me. @rigelrozanski should review this too.
reviewing this morning |
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.
just blocking merge until I review
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.
check em' changes oot
Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>
Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>
Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>
@rigelrozanski Updated! |
Was re-reading through the spec and found some sentences that were worded...weirdly, so this PR simply includes some minor Markdown fixes and a few bits of rephrasing.
Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (
docs/
)Added entries in
PENDING.md
with issue #rereviewed
Files changed
in the github PR explorerFor Admin Use: