-
Notifications
You must be signed in to change notification settings - Fork 94
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
SIMD-0133: Syscall: Get Epoch Stake #133
SIMD-0133: Syscall: Get Epoch Stake #133
Conversation
e60913f
to
f920ec1
Compare
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.
Looks great. This is a highly requested feature. I suggested two changes
Changes addressed! |
f920ec1
to
305babc
Compare
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.
Mostly suggestions for conciseness and clarity.
d6372ab
to
83fc06b
Compare
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.
Please include the one change. Otherwise, approved.
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.
Needs CU documentation
CUs added. |
83ae29d
to
c082f29
Compare
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.
Small edits
Co-authored-by: lheeger-jump <[email protected]>
Co-authored-by: lheeger-jump <[email protected]>
Looks like we have approvals from both clients. Leaving open until April 10 for any additional comments/revisions before merging. |
We will merge this SIMD if there are no more comments by April 12th. |
Really late to the party, but I just wanted to point out one small niggle here: For governance we need the fractional stake a validator has of total active stake, i.e. we need the total stake, too. If I'm reading this right it would require separate syscalls for every vote account to tally up the total active stake. Would it be possible to add an option to simply query total active stake to this syscall or to include such a separate syscall in this SIMD? |
@michaelh-laine That's a great point. I suggest we send an amendment SIMD that specifies that |
The concept of being able to obtain the total epoch stake is salient, but I think this proposed solution sounds like overloading the syscall. Personally I'd rather see two syscalls for this, one for a vote account, one for total stake. That being said, we still probably want to rename this one, and name the new one |
I actually like @ripatel-fd 's idea; using a null pointer to return total epoch stake, vs filtering with a provided vote address, makes semantic sense to me. Side note: if we amend this SIMD, let's please clarify the |
@CriesofCarrots @buffalojoec Which one of us should write the amend SIMD? I'm happy to do a first pass and then hand it off. Or before we start writing, should we get on a call and agree how the "get total" API should look like first? |
|
Thanks, @buffalojoec, that is looking great to me . Let's get it PRed and reviewed. I think since SIMD-0133 is still a draft, we can just amend it in place, rather than creating a superseding SIMD. ...at least, that's my vote. |
Thanks for picking up this suggestion so quickly everyone! |
Quick question regarding this: from my understanding, we have both the current and next epoch stakes available as the schedule for epoch N+1 is set once we cross epoch N, why no providing two sysvars, one for the current and one for the next epoch? That would allow trivial implementation of light clients by aggregating the next epoch vote accounts and their stake on a program and proving that the current vote accounts signed the block containing this program (after having aggregated)? As far as I know, there is currently no way to compute the next validator set on-chain and have it signed by the current validator set, hence light clients are pretty difficult to build because you'd have to prove all stake accounts for epoch N to know the validator set for N+1? Imagine this simple accumulator program: assert(state.expected_epoch = clock.epoch);
validator_stake = get_next_epoch_stake(vote_account);
assert(!pda_exists(validator_pda(vote_account, epoch)));
init_pda(validator_pda(vote_account, epoch));
assert(validator_stake <= state.highest_state);
state.highest_stake = validator_stake;
state.chain = hash(state.chain, (vote_account, validator_stake));
state.total_stakes += validator_stake;
if(state.total_stakes = get_next_epoch_stake(null)));
state.expected_epoch++;
state.highest_stake = u64::MAX;
state.total_stakes = 0;
state.valset = state.chain;
state.chain = seed; You would be able to prove this off-chain with a merkle inclusion proof of the block where the final validator has been aggregated in |
Interesting idea! I think since this SIMD has been implemented and feature-gated, the best direction for your suggested improvements would be to discuss this in channels like the Solana Tech Discord, or propose a new SIMD that expands on the syscall introduced by this SIMD, where your suggested changes are detailed. |
A syscall to retrieve a vote account's delegated stake for the current epoch.
Currently, on-chain programs have no knowledge of the current epoch's stake and
how much active stake is delegated to a certain vote account.
If this data was available for querying by on-chain programs, it would unblock
many use cases, such as validator governance and secondary consensus mechanisms,
that were previously not possible on Solana.
Additionally, this would enable the Feature Gate program defined in
SIMD 0089 to tally vote account stake in
support for a pending feature gate.