Raise stake minimum delegation to 1 SOL #24603

brooksprumo · 2022-04-22T19:52:43Z

Problem

The minimum stake delegation is too low. For more information, see the main issue: #22559

Summary of Changes

Add feature gate to raise minimum stake delegation to 1.0 SOL
Actually set minimum delegation to 1 SOL
Use stake_raise_minimum_delegation feature in the stake program impl
Refactor tests to run for both old and new behavior

Feature Gate Issue: #24357

brooksprumo · 2022-05-03T21:05:37Z

I still need to figure out local-cluster tests timing out, but I think this PR is in a good enough state to begin the review process. Thanks in advance, reviewers!

cli/tests/stake.rs

brooksprumo · 2022-05-03T21:23:40Z

programs/stake/src/stake_instruction.rs

+    mod old_behavior {
+        use super::*;
+
+        fn new_feature_set() -> FeatureSet {
+            let mut feature_set = FeatureSet::all_enabled();
+            feature_set.deactivate(&feature_set::raise_minimum_stake_delegation_to_1_sol::id());
+            feature_set
+        }
+
+        #[test]
+        fn test_stake_process_instruction() {
+            do_test_stake_process_instruction(new_feature_set());
+        }


You'll see that I now run basically all the tests twice—once for the old minimum delegation, and once for the new minimum delegation. I wanted to be extra sure I didn't break any existing behavior and also had the correct new behavior as well, so this was the best way I found to do that. It would've been nice to change the test's process_instruction() to just always do both, but since it returns accounts now, that wasn't feasible[^1].

[^1] I could've, by returning a tuple of accounts for old and new behavior, but then tests with multiple calls to process_instruction() wouldn't really be fixed.

programs/stake/src/stake_state.rs

runtime/src/bank.rs

brooksprumo · 2022-05-03T21:26:17Z

programs/stake/src/lib.rs

+pub fn get_minimum_delegation(feature_set: &FeatureSet) -> u64 {
+    if feature_set.is_active(&feature_set::raise_minimum_stake_delegation_to_1_sol::id()) {
+        const MINIMUM_DELEGATION_SOL: u64 = 1;
+        MINIMUM_DELEGATION_SOL * LAMPORTS_PER_SOL
+    } else {
+        #[allow(deprecated)]
+        solana_sdk::stake::MINIMUM_STAKE_DELEGATION
+    }


And here's basically the whole change.

brooksprumo · 2022-05-04T16:02:19Z

programs/stake/src/stake_state.rs

+    if (feature_set.is_active(&stake_allow_zero_undelegated_amount::id())
+        || feature_set.is_active(&feature_set::stake_raise_minimum_delegation_to_1_sol::id()))
        && stake_amount < crate::get_minimum_delegation(feature_set)
    {
        return Err(StakeError::InsufficientStake.into());


I could've not added || feature_set.is_active(&feature_set::stake_raise_minimum_delegation_to_1_sol::id() to this if, but then I would require that the stake_allow_zero_undelegated_amount feature got activated first.

Granted, it likely will get activated first, but I didn't want to require it here.

Sounds good to me

brooksprumo · 2022-05-04T16:08:18Z

programs/stake/src/stake_instruction.rs

-        let minimum_delegation = crate::get_minimum_delegation(&FeatureSet::all_enabled());
-        let stake_lamports = rent_exempt_reserve + minimum_delegation;


With the stake_allow_zero_undelegated_amount feature, the minimum balance needed to initialize a stake account is just the rent exempt amount. With a higher minimum delegation, the test below that says // not enough balance for rent incorrectly passes since it contained extra lamports to still meet the minimum balance requirements.

brooksprumo · 2022-05-06T10:30:45Z

CI has finally passed! This PR is now ready to merge, in addition to being ready to review.

programs/stake/src/stake_instruction.rs

jstarry · 2022-05-06T16:10:07Z

programs/stake/src/stake_state.rs

+    if (feature_set.is_active(&stake_allow_zero_undelegated_amount::id())
+        || feature_set.is_active(&feature_set::stake_raise_minimum_delegation_to_1_sol::id()))
        && stake_amount < crate::get_minimum_delegation(feature_set)
    {
        return Err(StakeError::InsufficientStake.into());


Sounds good to me

programs/stake/src/stake_state.rs

sdk/src/feature_set.rs

Pull request has been modified.

codecov · 2022-05-07T03:38:47Z

Codecov Report

Merging #24603 (f4fcd7a) into master (69a0ff9) will increase coverage by 0.0%.
The diff coverage is 87.0%.

❗ Current head f4fcd7a differs from pull request most recent head f0264b8. Consider uploading reports for the commit f0264b8 to get more accurate results

@@           Coverage Diff            @@
##           master   #24603    +/-   ##
========================================
  Coverage    82.0%    82.1%            
========================================
  Files         598      598            
  Lines      165882   166376   +494     
========================================
+ Hits       136125   136635   +510     
+ Misses      29757    29741    -16

brooksprumo added the feature-gate Pull Request adds or modifies a runtime feature gate label Apr 22, 2022

brooksprumo mentioned this pull request Apr 22, 2022

Raise stake minimum delegation #22559

Open

18 tasks

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch 4 times, most recently from 51cc796 to d38b4f8 Compare April 25, 2022 19:34

This was referenced Apr 25, 2022

Make rpc test_account_subscribe aware of stake minimum delegation #24659

Merged

Make test_next_leader_slot_next_epoch() aware of stake minimum delegation #24660

Merged

mvines mentioned this pull request Apr 25, 2022

Remove StakeState::get_rent_exempt_reserve() #24665

Merged

This was referenced Apr 25, 2022

Feature Gate: Allow undelegated stakes to be below the minimum delegation #24669

Closed

stake: Allow initialized stakes to be below the min delegation #24670

Merged

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from d38b4f8 to 6bbdb7e Compare April 27, 2022 17:25

brooksprumo mentioned this pull request Apr 27, 2022

Make test_bank_cloned_stake_delegations() aware of stake minimum delegation #24753

Merged

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from 6bbdb7e to b55196c Compare April 27, 2022 21:24

brooksprumo mentioned this pull request Apr 28, 2022

Make stake integration tests aware of stake minimum delegation #24809

Merged

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from b55196c to 05c2981 Compare April 28, 2022 23:23

brooksprumo mentioned this pull request Apr 29, 2022

cli stake tests: Increase stake amounts to handle raising minimum delegation #24835

Merged

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from 05c2981 to 9002f63 Compare April 29, 2022 15:27

joncinque mentioned this pull request Apr 29, 2022

stake-pool: Update program to work with minimum delegation solana-labs/solana-program-library#3127

Merged

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from 9002f63 to 320dbb7 Compare May 3, 2022 17:59

brooksprumo marked this pull request as ready for review May 3, 2022 21:04

brooksprumo requested review from joncinque, jstarry, CriesofCarrots and ryoqun May 3, 2022 21:14

brooksprumo commented May 3, 2022

View reviewed changes

cli/tests/stake.rs Outdated Show resolved Hide resolved

brooksprumo commented May 3, 2022

View reviewed changes

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from 320dbb7 to 1f8d1aa Compare May 4, 2022 15:58

brooksprumo commented May 4, 2022

View reviewed changes

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from 1f8d1aa to a4e5394 Compare May 4, 2022 16:05

brooksprumo commented May 4, 2022

View reviewed changes

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from a4e5394 to 3fa55d8 Compare May 4, 2022 16:13

joncinque mentioned this pull request May 4, 2022

stake: Fix redelegation test for new behavior #24969

Closed

brooksprumo mentioned this pull request May 5, 2022

Update local-cluster tests to work with higher minimum stake delegation #25023

Merged

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from 3fa55d8 to 2b52884 Compare May 6, 2022 03:01

jstarry previously approved these changes May 6, 2022

View reviewed changes

CriesofCarrots previously approved these changes May 6, 2022

View reviewed changes

sdk/src/feature_set.rs Show resolved Hide resolved

brooksprumo added 7 commits May 7, 2022 14:55

Add feature gate to raise minimum stake delegation to 1.0 sol

f25299c

Actually set minimum delegation to 1 SOL

5a44c10

Use raise_minimum_stake_delegation feature in the stake program impl

759d670

Refactor tests to run for both old and new behavior

14527eb

pr: comment

1dd5cd0

pr: remove erroneous withdraw

8cdf75f

pr: a comment on feature id

f0264b8

brooksprumo force-pushed the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch from f4fcd7a to f0264b8 Compare May 7, 2022 18:56

brooksprumo merged commit e6c02f3 into solana-labs:master May 7, 2022

brooksprumo deleted the minimum-stake/feature-gate/raise-minimum-delegation-to-1-sol branch May 9, 2022 17:23

brooksprumo changed the title ~~Raise minimum stake delegation to 1 SOL~~ Raise stake minimum delegation to 1 SOL Jul 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Raise stake minimum delegation to 1 SOL #24603

Raise stake minimum delegation to 1 SOL #24603

brooksprumo commented Apr 22, 2022 •

edited

Loading

brooksprumo commented May 3, 2022

brooksprumo May 3, 2022

brooksprumo May 3, 2022

brooksprumo May 4, 2022

jstarry May 6, 2022

brooksprumo May 4, 2022

brooksprumo commented May 6, 2022

jstarry May 6, 2022

codecov bot commented May 7, 2022 •

edited

Loading

		let minimum_delegation = crate::get_minimum_delegation(&FeatureSet::all_enabled());
		let stake_lamports = rent_exempt_reserve + minimum_delegation;

Raise stake minimum delegation to 1 SOL #24603

Raise stake minimum delegation to 1 SOL #24603

Conversation

brooksprumo commented Apr 22, 2022 • edited Loading

Problem

Summary of Changes

brooksprumo commented May 3, 2022

brooksprumo May 3, 2022

Choose a reason for hiding this comment

brooksprumo May 3, 2022

Choose a reason for hiding this comment

brooksprumo May 4, 2022

Choose a reason for hiding this comment

jstarry May 6, 2022

Choose a reason for hiding this comment

brooksprumo May 4, 2022

Choose a reason for hiding this comment

brooksprumo commented May 6, 2022

jstarry May 6, 2022

Choose a reason for hiding this comment

codecov bot commented May 7, 2022 • edited Loading

Codecov Report

brooksprumo commented Apr 22, 2022 •

edited

Loading

codecov bot commented May 7, 2022 •

edited

Loading