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

feat: rewards overhaul #1876

Merged
merged 27 commits into from
Aug 24, 2023
Merged

feat: rewards overhaul #1876

merged 27 commits into from
Aug 24, 2023

Conversation

cdummett
Copy link
Collaborator

@cdummett cdummett commented Aug 3, 2023

PR updates the rewards specification with updates defined in the requirements document.

Comment on lines 9 to 10
- `rewards.vesting.baseRate`: the proportion of rewards in a vesting account which are vested each epoch
- `rewards.vesting.minimumTransfer`: the minimum amount (expressed in quantum) which can be vested each epoch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we specify the default values for these in the specs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure @barnabee to confirm these are sensible but for now have set

  • rewards.vesting.baseRate=0.1 (means ~50% of rewards are unlocked after 7 epochs and ~95% after 31 epochs)
  • rewards.vesting.minimumTransfer=100 (same as the minimum transfer network parameter).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimum transfer means that someone with 1000 of rewards will be fully unlocked after 10 epochs regardless of the baserate. That seems too high.

Additionally I think the base rate ought to suggest unlocking over months rather than days for anyone with non-trivial rewards.

I think the minimum transfer amount should be something like 5 or 10, and the base rate probably in the range 1–2.5%.

However, I think this one we need to discuss with @MM0819 and Steven so lets hold off on confirming anything until that is done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdummett - can we add some sensible values to both this and the spreadsheet and these will be used. A core isue to be raised to update once the real values have ben decided

protocol/0085-RVST-rewards_vesting.md Outdated Show resolved Hide resolved
@cdummett cdummett changed the title WIP: rewards overhaul feat: rewards overhaul Aug 14, 2023
@cdummett cdummett marked this pull request as ready for review August 14, 2023 17:54
Copy link
Contributor

@davidsiska-vega davidsiska-vega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks basically fine, but I've left a couple of tiny comments.

It needs review from Lewis and Steven as they're the ones who have the best picture of how this functionality will be used.

@cdummett cdummett merged commit 4389506 into cosmicelevator Aug 24, 2023
4 checks passed
@cdummett cdummett deleted the feat/reward_updates branch August 24, 2023 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants