Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Consider removing StakeInstruction::RedeemVoteCredits #7835

Closed
mvines opened this issue Jan 15, 2020 · 8 comments · Fixed by #7916
Closed

Consider removing StakeInstruction::RedeemVoteCredits #7835

mvines opened this issue Jan 15, 2020 · 8 comments · Fixed by #7916
Assignees

Comments

@mvines
Copy link
Contributor

mvines commented Jan 15, 2020

RedeemVoteCredits has a number of problems:

  1. Somebody must issue this instruction every MAX_EPOCH_CREDITS_HISTORY epochs for each stake/vote account or rewards evaporate.
  2. Reward point values are computed based on the latest epoch, which can produce confusing and hard to understand results as this example from TdS DR6 shows:
    After the first 24 hours of idling, redeeming votes
    using a ~1 SOL stake produced ~6221 SOL in voter rewards. Redeeming
    that same stake 12 hours later produced ~54 SOL in voter rewards. The reason
    for this huge reward drop is because the total stake increased from 435 SOL to 25695
    SOL in those 12 hours, so the credit point value was much smaller.
  3. The stake/vote accounts/programs as well as runtime contains code/data bloat that exists only to enable RedeemVoteCredits in its current form.

The first two points highlight the unintentional game that will be played around RedeemVoteCredits: if you see observe stake decreasing in the cluster than you're better off delaying RedeemVoteCredits if possible to capitalize on the increasing credit point value. But don't delay too long or you'll lost it all. Worse, RedeemVoteCredits can be run by anybody so others can inflict less desirable credit point values on you.

The alternative is to dispense of RedeemVoteCredits entirely, and have banks automatically redeem vote credits at the end of each epoch for all parties.

@garious
Copy link
Contributor

garious commented Jan 16, 2020

Any gaming implies a bug in the implementation. If I had to guess, credits probably need to be converted to lamports as soon as they are earned.

@mvines
Copy link
Contributor Author

mvines commented Jan 16, 2020

Anybody remember why RedeemVoteCredits was added in the first place instead of auto redemption by the runtime?

@garious
Copy link
Contributor

garious commented Jan 16, 2020

Yep, I wrote it. The goal was simply to minimize the size of the bank (aka, the kernel).

@garious
Copy link
Contributor

garious commented Jan 16, 2020

No objection to moving that functionality to the runtime.

@mvines
Copy link
Contributor Author

mvines commented Jan 16, 2020

Cool yeah these days the bank knows a ton about vote and stake so bank/vote/stake are already tightly coupled

@rob-solana
Copy link
Contributor

removing the instruction pays big dividends in vote and stake state size, too

@garious
Copy link
Contributor

garious commented Jan 17, 2020

@rob-solana, can you take a stab at this?

@garious garious added this to the Tofino v0.23.0 milestone Jan 17, 2020
@rob-solana
Copy link
Contributor

yeap, will do

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants