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

rpc: getStakeActivation now correctly returns the inactivate lamports for deactivat{ed,ing} stake account in all cases #26595

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

mvines
Copy link
Member

@mvines mvines commented Jul 12, 2022

For a newly created stake account, getStakeActivation returns StakeActivationState::Inactive with an inactive amount equal to the number of lamports held by the stake account, minus the rent exempt reserve.

However for a fully deactivated stake account, getStakeActivation returns StakeActivationState::Inactive with an inactive amount equal to the number of lamports in the previously active delegation. This is wrong because the user may have deposited or withdrawn lamports from the account since it was deactivated, and is inconsistent with the result returned for newly created stake accounts.

Likewise StakeActivationState::Deactivating could return an incorrect amount of inactive stake due to user deposits or withdrawals

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Drive-by!

rpc/src/rpc.rs Outdated
@@ -1783,7 +1783,9 @@ impl JsonRpcRequestProcessor {
StakeActivationState::Activating => activating,
StakeActivationState::Active => 0,
StakeActivationState::Deactivating => delegation.stake.saturating_sub(effective),
Copy link
Contributor

@joncinque joncinque Jul 12, 2022

Choose a reason for hiding this comment

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

When looking at this earlier, I thought this might need to be

            StakeActivationState::Deactivating => deactivating,

But this inactive_stake really means Deactivated stake, and is only non-zero in the case of multi-epoch deactivations.

If a redelegated stake ever goes through a multi-epoch deactivation, this would also show too much stake overall on the system. What do you think about this being stake_account.lamports().saturating_sub(effective + rent_exempt_reserve)?

Copy link
Member Author

Choose a reason for hiding this comment

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

ty, done!

…deactivat{ed,ing} stake account in all cases
@mvines mvines changed the title rpc: getStakeActivation now correctly returns the inactivate lamports for deactivated stake account in all cases rpc: getStakeActivation now correctly returns the inactivate lamports for deactivat{ed,ing} stake account in all cases Jul 12, 2022
@mvines mvines merged commit 06ae906 into solana-labs:master Jul 13, 2022
@mvines mvines deleted the sa branch July 13, 2022 01:18
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.

2 participants