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 staking rewards design proposal for delegation #3148

Merged

Conversation

aeyakovenko
Copy link
Member

@aeyakovenko aeyakovenko commented Mar 6, 2019

Problem

Current staking architecture requires a validator vote per delegated stake. It has two main problems

  • The number of votes is high, since the number of delegated stakes is expected to be much higher than the number of validators.
  • The validators are actively acknowledging delegation.

Summary of Changes

Cover the current architecture and its problems and propose an alternative that allows for passive delegation and a single vote for many delegated stakes.

Fixes #

tag: @garious (I'll add you as the last reviewer)

book/src/stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
@garious garious changed the title Stake delegation and rewads Add staking rewards design proposal Mar 6, 2019
@aeyakovenko aeyakovenko changed the title Add staking rewards design proposal Add staking rewards design proposal for passive delegation Mar 6, 2019
@aeyakovenko aeyakovenko changed the title Add staking rewards design proposal for passive delegation Add staking rewards design proposal for delegation Mar 6, 2019
@garious
Copy link
Contributor

garious commented Mar 8, 2019

This needs a rebase/rewrite that acknowledges the current design that we merged earlier this week.

@aeyakovenko aeyakovenko force-pushed the stake_delegation_and_rewads branch from 91b1859 to f1ed938 Compare March 9, 2019 15:41
@aeyakovenko
Copy link
Member Author

@garious, rebased

@garious
Copy link
Contributor

garious commented Mar 9, 2019

@aeyakovenko, this proposal needs more work. It starts off with a first sentence that doesn't make any sense. The second sentence has a spelling error. The terminology is confusing because it doesn't acknowledge the terminology in the existing architecture (which there's already an implementation of). It'd be especially helpful to acknowledge the existing definition of "delegate" and note that there's currently a RewardsState whereas you introduce a very different state under the subtlety different name RewardState.

I think you could take a very different approach here, which is simply to move delegate_id and authorized_voter_id from VoteState to a new StakeState. The instance of the VoteState would be called the "voting account" and the instance of the StakeState would be called the "staking account". The interesting part of the design will be what follows.

@aeyakovenko
Copy link
Member Author

@garious why would the authorized_voter be part of the StakeState? authorized_voter is a function of the validator/delegate, not the stake owner.

Copy link
Contributor

@garious garious left a comment

Choose a reason for hiding this comment

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

Getting closer. To me, the big innovation here is the claimed_credits field that's set to VoteState::credits when the staker chooses a voter.

book/src/passive-stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/passive-stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/passive-stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/passive-stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/passive-stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
book/src/passive-stake-delegation-and-rewards.md Outdated Show resolved Hide resolved
### RewardsInstruction::Initialize

* `account[0]` - Out Param - The RewardsState instance.
`RewardsState::claimed_credits` is initialized to `VoteState::credits`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the clever bit, but why do it in . Initialize and not as a separate instruction? Why can't the staker change its VoteSigner?

Copy link
Member Author

Choose a reason for hiding this comment

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

That can be implemented. It doesn’t add any expressive power to the program over creating a new instance for every new stake.

We might need that instruction if there is a warmup and cool down period for creating stakes.

@garious
Copy link
Contributor

garious commented Mar 11, 2019

Can you add rust,ignore anywhere you have triple-backticks? That should get CI passing.

@garious
Copy link
Contributor

garious commented Mar 11, 2019

Looks like our designs pretty much converged. Anything else you want to grab from https://github.com/solana-labs/solana/pull/3212/files so I can close it?

@aeyakovenko aeyakovenko merged commit 9eb7e63 into solana-labs:master Mar 11, 2019
steviez added a commit to steviez/solana that referenced this pull request Oct 13, 2024
…olana-labs#3148)

The arguments to specify full and incremental snapshot archives paths
used to be a global argument; these were moved to only be instantiated
on commands that needed them in solana-labs#1773.

But, when the arguments were moved from app-level to subcommand-level,
the code that matches the arguments was not updated to look at
subcommand-matches instead of app-matches.
Szymongib pushed a commit to ChorusOne/solana that referenced this pull request Oct 28, 2024
…tory (backport of solana-labs#3148) (solana-labs#3153)

ledger-tool: Fix create-snapshot default value for output_directory (solana-labs#3148)

The arguments to specify full and incremental snapshot archives paths
used to be a global argument; these were moved to only be instantiated
on commands that needed them in solana-labs#1773.

But, when the arguments were moved from app-level to subcommand-level,
the code that matches the arguments was not updated to look at
subcommand-matches instead of app-matches.

(cherry picked from commit 1d9947c)

Co-authored-by: steviez <steven@anza.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants