cli: stake-account/stakes: Use GetStakeActivation#13497
cli: stake-account/stakes: Use GetStakeActivation#13497ryoqun wants to merge 2 commits intosolana-labs:masterfrom
Conversation
| let current_epoch = clock.epoch; | ||
| let (active_stake, activating_stake, deactivating_stake) = stake | ||
| .delegation | ||
| .stake_activating_and_deactivating(current_epoch, Some(stake_history)); |
| StakeActivationState::Active => (stake_activation.active, 0, 0), | ||
| StakeActivationState::Inactive => (0, 0, 0), | ||
| StakeActivationState::Activating => { | ||
| (stake_activation.active, stake_activation.inactive, 0) | ||
| } | ||
| StakeActivationState::Deactivating => { | ||
| (stake_activation.active, 0, stake_activation.inactive) | ||
| } |
There was a problem hiding this comment.
shameless inverse conversion from RpcStakeActivation back to the original stake tuple ((u64, u64, u64)). If I had more time, I would fix CliStakeState itself... ;)
| self.get_epoch_info_with_commitment(CommitmentConfig::default()) | ||
| } | ||
|
|
||
| pub fn get_stake_activation(&self, pubkey: &Pubkey) -> ClientResult<RpcStakeActivation> { |
There was a problem hiding this comment.
I think adding new fns is ok in terms of compatibility even with a patch release.
|
@CriesofCarrots Could you quick review this? Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #13497 +/- ##
=========================================
- Coverage 82.1% 82.0% -0.1%
=========================================
Files 378 378
Lines 90505 90525 +20
=========================================
- Hits 74317 74314 -3
- Misses 16188 16211 +23 |
| let mut stake_accounts: Vec<CliKeyedStakeState> = vec![]; | ||
| for (stake_pubkey, stake_account) in all_stake_accounts { | ||
| if let Ok(stake_state) = stake_account.state() { | ||
| let stake_activation = rpc_client.get_stake_activation(&stake_pubkey)?; |
There was a problem hiding this comment.
Unfortunately, this doesn't quite work. If a stake account is undelegated, getStakeActivation returns a JSON-rpc error: {"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid param: stake account has not been delegated"},"id":1}
...which errors out here and breaks the loop (or in the case of solana stake-account, prevents the account info being shown).
A simple (naive) fix might be to write this as:
| let stake_activation = rpc_client.get_stake_activation(&stake_pubkey)?; | |
| let stake_activation = rpc_client.get_stake_activation(&stake_pubkey).unwrap_or( | |
| RpcStakeActivation { | |
| state: StakeActivationState::Inactive, | |
| active: 0, | |
| inactive: 0, | |
| } | |
| ); |
although that would disguise other types of errors.
|
This change makes |
|
ref: similar change was incorporated into #13501 |
2 participants
Problem
CLI calculates stakes locally, which is very hard to properly feature-gate it. And it's not useful for fixing the stake calculation code.
Summary of Changes
Use aptly-provided
getStakeActivationrpc endpoint.based-on: #10902