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

stake-pool: Prefund split account during redelegate #5285

Merged
merged 6 commits into from
Sep 20, 2023

Conversation

joncinque
Copy link
Contributor

Problem

During redelegate, the first split account should have the rent-exempt reserve populated. This makes it clearer how many lamports are actually moving to the new validator.

Solution

Include the stake pool reserve in redelegate, and withdraw the required rent during redelegate.

Comment on lines 1950 to 1952
if required_lamports_for_rent_exemption >= reserve_stake_info.lamports() {
return Err(StakePoolError::ReserveDepleted.into());
}
Copy link
Member

Choose a reason for hiding this comment

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

this case is probably not worth caring about but note it fails if the reserve is empty but the transient account is fully funded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean... the logic is really touchy around here. Note that the reserve shouldn't ever be empty, since it needs to have at least the rent-exemption to exist, so this line shouldn't ever trip unnecessarily.

I put this in because some later tests showed that I was accidentally drawing everything out of the reserve and destroying it, so the intent is to make sure that we don't delete the reserve ever.

How about this then?

if required_lamports_for_rent_exemption > 0 {
    if required_lamports_for_rent_exemption >= reserve_stake_info.lamports() {
        return Err(StakePoolError::ReserveDepleted.into());
    }
    Self::stake_withdraw(.....)?; 
}

Copy link
Member

Choose a reason for hiding this comment

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

yes thats what i had in mind!

@2501babe
Copy link
Member

iirc modifying the instruction like this is fine because it hasnt been deployed yet

@joncinque
Copy link
Contributor Author

iirc modifying the instruction like this is fine because it hasnt been deployed yet

That's exactly right, thanks for double-checking!

@joncinque joncinque merged commit 1b3a980 into solana-labs:master Sep 20, 2023
9 checks passed
@joncinque joncinque deleted the spresplit branch September 20, 2023 19:31
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.

None yet

2 participants