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

Partitioned Epoch Rewards Distribution #15

Merged
merged 22 commits into from
May 4, 2023

Conversation

HaoranYi
Copy link
Contributor

No description provided.

@HaoranYi HaoranYi changed the title add epoch reward proposal Add epoch reward proposal V2 Dec 20, 2022
@HaoranYi HaoranYi marked this pull request as ready for review January 23, 2023 17:22
@HaoranYi HaoranYi marked this pull request as draft January 23, 2023 18:10
@HaoranYi HaoranYi marked this pull request as ready for review January 23, 2023 18:10
@mvines
Copy link
Contributor

mvines commented Jan 25, 2023

We should get review from Brooks, Jon, and other Labs folks that have been actively involved in the discussion here

@HaoranYi
Copy link
Contributor Author

HaoranYi commented Jan 25, 2023

We should get review from Brooks, Jon, and other Labs folks that have been actively involved in the discussion here

yeah. I have asked jwash, brooks, behzad, jon on slack DM to join the repo and start our discussion here.

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

I'll let others chime in with their questions -- you've brought up some great challenges!

proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

First-pass over the rewards calculation

proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

First-pass over the distribution proposal

proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
@HaoranYi HaoranYi added the core Standard SIMD with type Core label Feb 9, 2023
@HaoranYi
Copy link
Contributor Author

@t-nelson @behzadnouri @mvines Can you help to review this proposal and let me know what you think? Thanks!

@t-nelson
Copy link
Contributor

yep! i'll add to the queue. everytime i've tried to review previously, you were actively updating it 😅

proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
Copy link

@behzadnouri behzadnouri left a comment

Choose a reason for hiding this comment

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

Overall comments:

  • I think the proposal should be self-contained and allow the reviewers to discuss any piece. Directing to the old proposal when it had major design issues is not helping here.
  • The split between computation vs distribution is just adding more ambiguity; specially in the parts related to merkle trees. The process can be better described if it is end-to-end and one coherent piece.
  • Some major issues (e.g. cluster going down) are not yet addressed or only posed as questions, whereas I believe these should be resolved in the proposal before we can move to the implementation.
  • The document is pretty light in detail and incomplete in some critical parts of the design. e.g. the whole merkle tree application.

proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-calculation.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
proposals/reward-proposal-v2-reward-distribution.md Outdated Show resolved Hide resolved
@t-nelson
Copy link
Contributor

please ping me here when you've resolved michael and behzad's concerns. it looks like they gave you plenty to chew on 🙂

@mvines
Copy link
Contributor

mvines commented Mar 3, 2023

Hey @HaoranYi, can you please let us know once the proposal reflects what we chatted about earlier this week?

@HaoranYi
Copy link
Contributor Author

HaoranYi commented Mar 3, 2023

Yes, I will. Working on it today.

@HaoranYi HaoranYi deleted the reward branch June 21, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Standard SIMD with type Core standard SIMD in the Standard category
Projects
Status: SIMDs
Development

Successfully merging this pull request may close these issues.