Syscall: GetEpochStake

[buffalojoec]

Problem

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.

Summary of Changes

Introduce a new callback function to the SVM to retrieve the delegated active
stake for a vote account.

Add that callback to the program-runtime's InvokeContext.

Introduce a new syscall - SyscallGetEpochStake - that will use this callback
to retrieve the vote account's stake.

SIMD 0133

Feature Gate Tracking Issue: #884

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 23.52941% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 82.1%. Comparing base (65c3655) to head (66d1534).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master     #889     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%     
=========================================
  Files         880      881      +1     
  Lines      235644   235713     +69     
=========================================
+ Hits       193662   193675     +13     
- Misses      41982    42038     +56     

[buffalojoec]

@Lichtso @CriesofCarrots Hey guys, I wanted to flesh out some thoughts here and see what you thought. I figured I could mark this comment resolved if we decided to continue with the PR review.

I've introduced this syscall's implementation in the InvokeContext as a simple closure. Although this might appear a bit specific, I think it actually has merit. However, I went through some brainstorming about how else this might be implemented, and considered a few trade-offs.

First of all, @Lichtso has mentioned before that the InvokeContext is internal, and can be changed fairly trivially, so if we need to move away from a closure in the future we can do so without much trouble. With that being said, I felt like adding the lightest-weight query support to the InvokeContext made the most sense, rather than something like adding the entire EpochStakes structure to InvokeContext, or even a reference to it, ie. Arc<EpochStakes>. Although the reference is somewhat lightweight, it would require a refactoring of EpochStakes to avoid a cyclical dependency between runtime and program-runtime.

Although the closure and the concept of such a runtime callback might appear strange to add to InvokeContext, it's actually identical in nature to the way FeatureSet works. Take for example an SVM sidecar for simulating transactions. This service must provide an executing eBPF VM with the ability to run is_active: (Pubkey) -> bool on a given feature ID.

Firedancer also has their own implementation for such a feature set callback, and they've indicated that their implementation of SIMD 0133 (the implementation in this PR) would work similarly to what I've provided here.

First question: Is a simple closure, with the ability to trivially change this in the future, acceptable for this implementation, or would you rather I attempt to evict EpochStakes from the runtime to its own crate, for use by both runtime and program-runtime?

Additionally, I wanted to make sure I evaluated the potential for us to move this into a sysvar in the future, in case the need ever arose for it. It appears to me that we could use a feature gate within the syscall itself to shift to the sysvar cache whenever the feature to activate the sysvar is activated. Afterward, the original callback can be removed from InvokeContext and SVM. So we're not necessarily backing ourselves into a corner here.

Second question: Mostly just wanted a sanity check on the above paragraph.

[Lichtso]

It is conceptually feasible, but not nice. How about putting EpochStakes in EnvironmentConfig?

[buffalojoec]

It is conceptually feasible, but not nice. How about putting EpochStakes in EnvironmentConfig?

Yeah, this was what I was thinking, too if you guys didn't like the closure. I would lean into Arc<EpochStakes> over cloning the object. But that means we'd need to factor the struct out from the runtime in order to allow program-runtime to depend on it.

The weird bit here is that the rest of the object isn't really necessary right now. We just need that lookup. But a reference would be OK for that. The SVM callback becomes get_epoch_stakes -> Arc<EpochStakes>, though, which is a bit more difficult to serve outside the runtime.

[CriesofCarrots]

It is conceptually feasible, but not nice. How about putting EpochStakes in EnvironmentConfig?

I would prefer putting the epoch-stake data source in EnvironmentConfig as well.

I would lean into Arc over cloning the object.

We definitely cannot clone the object into the vm.
Really, the only thing needed to calculate GetEpochStake is the VoteAccounts struct (EpochStakes::stakes::vote_accounts). It is probably worth investigating if we can reference this (or its fields) directly, not least because VoteAccounts already lives outside the runtime.

Additionally, I wanted to make sure I evaluated the potential for us to move this into a sysvar in the future, in case the need ever arose for it.

I do see how the callback would make it slightly easier to transition to querying from a sysvar (which I assume would be something like Map<vote_pubkey, total_stake>, calculated on each epoch boundary) in the future. But if InvokeContext is as easy to update as you say, that doesn't seem worth the cost of using a less-desirable api until then to me.

[buffalojoec]

I would prefer putting the epoch-stake data source in EnvironmentConfig as well.

We definitely cannot clone the object into the vm. Really, the only thing needed to calculate GetEpochStake is the VoteAccounts struct (EpochStakes::stakes::vote_accounts). It is probably worth investigating if we can reference this (or its fields) directly, not least because VoteAccounts already lives outside the runtime.

Sounds good, I think I'll head in this direction, then.

Additionally, I wanted to make sure I evaluated the potential for us to move this into a sysvar in the future, in case the need ever arose for it.

I do see how the callback would make it slightly easier to transition to querying from a sysvar (which I assume would be something like Map<vote_pubkey, total_stake>, calculated on each epoch boundary) in the future. But if InvokeContext is as easy to update as you say, that doesn't seem worth the cost of using a less-desirable api until then to me.

Sounds good to me!

Thank you guys!

[buffalojoec]

Thanks to both of you for the insight! Closing in favor of #1152.