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: Use stake program minimum delegation #3547

Merged
merged 6 commits into from
Aug 30, 2022

Conversation

joncinque
Copy link
Contributor

Problem

Currently, the stake pool program hardcodes a minimum delegation amount of 0.001 SOL in the program, but the stake program will soon enforce a higher minimum delegation amount.

Solution

Use the maximum of the previous value (0.001 SOL) and the value reported by the stake program's get_minimum_delegation instruction.

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Did a first pass; here's a few comments. I'll do another pass tomorrow as well.

stake-pool/program/src/instruction.rs Outdated Show resolved Hide resolved
stake-pool/program/src/processor.rs Outdated Show resolved Hide resolved
stake-pool/program/tests/helpers/mod.rs Outdated Show resolved Hide resolved
stake-pool/program/tests/vsa_remove.rs Show resolved Hide resolved
@brooksprumo brooksprumo self-requested a review August 29, 2022 21:10
@joncinque
Copy link
Contributor Author

It's all ready for you tomorrow, thanks for the review!

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Looks good! Some questions due to my ignorance, then I imagine it'll be good to go.

Are there any concerns about needing new/more tests? Nothing jumped out at me w.r.t. the minimum delegation, since the pool's minimum is still effectively 1 SOL. Same for MINIMUM_RESERVE_LAMPORTS, since that was 1 lamport before as well.

stake-pool/program/src/lib.rs Show resolved Hide resolved
stake-pool/program/src/lib.rs Show resolved Hide resolved
stake-pool/program/src/lib.rs Outdated Show resolved Hide resolved
stake-pool/program/src/processor.rs Outdated Show resolved Hide resolved
stake-pool/program/src/processor.rs Outdated Show resolved Hide resolved
@joncinque
Copy link
Contributor Author

joncinque commented Aug 30, 2022

Are there any concerns about needing new/more tests?

For this one, not yet, since it'll work as expected. The next PR is adding a check during increase to make sure that the resulting stake will be at least 1 SOL, to avoid bricking any pools when either proposal goes through.

@joncinque
Copy link
Contributor Author

Ok, this should be good for another pass

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@joncinque joncinque merged commit f0dd422 into solana-labs:master Aug 30, 2022
@joncinque joncinque deleted the sp-mind branch August 30, 2022 19:45
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