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

refactor: leader schedule generation #4662

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

jstarry
Copy link

@jstarry jstarry commented Jan 28, 2025

Problem

Need to do a little refactoring before modifying the leader schedule in a follow up change (#4597)

Summary of Changes

Refactored leader schedule generation code so that parts can be shared with the new vote account keyed leader schedule algorithm

Fixes #

@jstarry jstarry force-pushed the refactor/leader-schedule branch 2 times, most recently from 6681859 to d2e5ead Compare February 5, 2025 12:44
@jstarry jstarry changed the title refactor: leader schedule refactor: leader schedule generation Feb 5, 2025
@jstarry jstarry force-pushed the refactor/leader-schedule branch 3 times, most recently from 778b92c to 568c040 Compare February 5, 2025 13:21
@jstarry jstarry force-pushed the refactor/leader-schedule branch from 568c040 to d19fad1 Compare February 5, 2025 13:26
Copy link

@AshwinSekar AshwinSekar left a comment

Choose a reason for hiding this comment

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

lgtm just some minor nits

// deterministic result.
// Note: Use unstable sort, because we dedup right after to remove the equal elements.
stakes.sort_unstable_by(|(l_pubkey, l_stake), (r_pubkey, r_stake)| {
if r_stake == l_stake {

Choose a reason for hiding this comment

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

nit: you could compare the tuples directly:

(r_stake, r_pubkey).cmp(&(l_stake, l_pubkey))

Copy link
Author

Choose a reason for hiding this comment

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

This PR just moves this sort_stakes function from another file, I'm fine with leaving it as is


// Note: passing in zero stakers will cause a panic.
fn stake_weighted_slot_leaders(
mut keyed_stakes: Vec<(&Pubkey, u64)>,

Choose a reason for hiding this comment

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

nit: you could avoid the intermediate collection here by taking in an iter (flipped) Iterator<Item = (u64, &Pubkey)> and using the iter variants in sort_stakes:

stakes.sorted_unstable().dedup().reverse()

Copy link
Author

Choose a reason for hiding this comment

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

IterTools often allocates internally in its helper methods FYI:

fn sorted_unstable(..) {
        let mut v = Vec::from_iter(self);
        v.sort_unstable();
        v.into_iter()
}

@jstarry jstarry merged commit 2d4d7c1 into anza-xyz:master Feb 6, 2025
66 checks passed
@jstarry jstarry deleted the refactor/leader-schedule branch February 6, 2025 02:21
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