Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Add MINIMUM_STAKE_DELEGATION constant#23259

Merged
brooksprumo merged 5 commits intosolana-labs:masterfrom
brooksprumo:minimum-stake/add-constant-and-tests-no-behavior-changes
Feb 22, 2022
Merged

Add MINIMUM_STAKE_DELEGATION constant#23259
brooksprumo merged 5 commits intosolana-labs:masterfrom
brooksprumo:minimum-stake/add-constant-and-tests-no-behavior-changes

Conversation

@brooksprumo
Copy link
Copy Markdown
Contributor

@brooksprumo brooksprumo commented Feb 21, 2022

Problem

There are not tests for enforcing the minimum stake delegation.

Summary of Changes

  • Add MINIMUM_STAKE_DELEGATION constant
  • Add tests for enforcing current behavior w.r.t. the minimum delegation
  • Use MINIMUM_STAKE_DELEGATION constant in existing tests

NOTE: No behavior was changed in this PR.

Context

In an effort to simplify #22663, I've pulled out the changes from that PR that pertain to just the tests and adding the constant. This should make it simpler to review (1) this PR, and then (2) once I rebase the other PR on this one, it'll be quite obvious what has and has not changed.

@brooksprumo brooksprumo marked this pull request as ready for review February 21, 2022 18:10
Comment thread programs/stake/src/stake_state.rs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 21, 2022

Codecov Report

Merging #23259 (351fff7) into master (c696944) will increase coverage by 0.0%.
The diff coverage is 94.4%.

@@           Coverage Diff            @@
##           master   #23259    +/-   ##
========================================
  Coverage    81.3%    81.3%            
========================================
  Files         570      571     +1     
  Lines      155438   155665   +227     
========================================
+ Hits       126452   126665   +213     
- Misses      28986    29000    +14     

CriesofCarrots
CriesofCarrots previously approved these changes Feb 22, 2022
Copy link
Copy Markdown
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

wfm! It will be nice to have this clarity for #22663
r+ one nit

Comment thread programs/stake/src/stake_state.rs Outdated
Comment thread programs/stake/src/stake_state.rs Outdated
Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
@mergify mergify Bot dismissed CriesofCarrots’s stale review February 22, 2022 02:24

Pull request has been modified.

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
@brooksprumo brooksprumo merged commit ac70070 into solana-labs:master Feb 22, 2022
@brooksprumo brooksprumo deleted the minimum-stake/add-constant-and-tests-no-behavior-changes branch February 22, 2022 15:21
Comment on lines +6726 to +6728
/// The stake program currently allows delegations below the minimum stake delegation (see also
/// `test_delegate_minimum_stake_delegation()`). This is not the ultimate desired behavior,
/// but this test ensures the existing behavior is not changed inadvertently.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good doc. :)

jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Mar 4, 2022
Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants