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.

Sign up for GitHub

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

Make test_next_leader_slot_next_epoch() aware of stake minimum delegation #24660

Merged
merged 1 commit into from
Apr 26, 2022
Merged

Make test_next_leader_slot_next_epoch() aware of stake minimum delegation #24660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ledger/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ solana-program-runtime = { path = "../program-runtime", version = "=1.11.0" }
solana-rayon-threadlimit = { path = "../rayon-threadlimit", version = "=1.11.0" }
solana-runtime = { path = "../runtime", version = "=1.11.0" }
solana-sdk = { path = "../sdk", version = "=1.11.0" }
solana-stake-program = { path = "../programs/stake", version = "=1.11.0" }
solana-storage-bigtable = { path = "../storage-bigtable", version = "=1.11.0" }
solana-storage-proto = { path = "../storage-proto", version = "=1.11.0" }
solana-transaction-status = { path = "../transaction-status", version = "=1.11.0" }
Expand Down
3 changes: 2 additions & 1 deletion ledger/src/leader_schedule_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,8 @@ mod tests {
&mint_keypair,
&vote_account,
&validator_identity,
bootstrap_validator_stake_lamports(),
bootstrap_validator_stake_lamports()
+ solana_stake_program::get_minimum_delegation(&bank.feature_set),
);
let node_pubkey = validator_identity.pubkey();

Expand Down
1 change: 1 addition & 0 deletions programs/bpf/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion programs/stake/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub fn add_genesis_accounts(genesis_config: &mut GenesisConfig) -> u64 {
/// NOTE: This is also used to calculate the minimum balance of a stake account, which is the
/// rent exempt reserve _plus_ the minimum stake delegation.
#[inline(always)]
pub(crate) fn get_minimum_delegation(_feature_set: &FeatureSet) -> u64 {
pub fn get_minimum_delegation(_feature_set: &FeatureSet) -> u64 {
Copy link
Member

Choose a reason for hiding this comment

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

how about moving this into solana-program, so we don’t need a new dep in ledger/
When it’s moved into solana-program, we could probably remove the FeatureSet as well (since that’s only available in solana-sdk) and instead just always use 1 SOL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mvines Sorry 😢 I already merged this specific part via #24659. I like this suggestion though as it sounds like it'll simplify a number of testing/impl details. So, going forward I could:

  1. Revert Make rpc test_account_subscribe aware of stake minimum delegation #24659, then move get_minimum_delegation(), then re-do/fix these tests
  2. Merge this PR (which will end up not having this specific change since it has already been merged), then move get_minimum_delegation()
  3. Move get_minimum_delegation(), then fixup this PR, then redo Make rpc test_account_subscribe aware of stake minimum delegation #24659

Do you have a preference here? I feel like (2) is going to be the fastest and most straight forward. Maybe there's another option too?

Copy link
Member

Choose a reason for hiding this comment

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

(2), or whatever is easiest wfm!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracking this in #24690

// If/when the minimum delegation amount is changed, the `feature_set` parameter will be used
// to chose the correct value. And since the MINIMUM_STAKE_DELEGATION constant cannot be
// removed, use it here as to not duplicate magic constants.
Expand Down