Skip to content

ledger: Require leader schedules to be repeat-aligned #10047

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

Merged
vadorovsky merged 1 commit into from
Jan 16, 2026
Merged

ledger: Require leader schedules to be repeat-aligned #10047

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
5 changes: 4 additions & 1 deletion ledger/src/leader_schedule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ fn stake_weighted_slot_leaders(
len: u64,
repeat: u64,
) -> Vec<Pubkey> {
debug_assert!(
len.is_multiple_of(repeat),
"expected `len` {len} to be divisible by `repeat` {repeat}"
);
sort_stakes(&mut keyed_stakes);
let (keys, stakes): (Vec<_>, Vec<_>) = keyed_stakes.into_iter().unzip();
let weighted_index = WeightedU64Index::new(stakes).unwrap();
Expand Down Expand Up @@ -192,7 +196,6 @@ mod tests {
#[test_case(457470, &[10, 20, 30], 12, 1, &[2, 1, 1, 1, 1, 1, 1, 1, 1, 2, 0, 2])]
#[test_case(3466545, &[10, 20, 30], 12, 1, &[2, 2, 0, 0, 2, 1, 1, 1, 0, 0, 2, 2])]
#[test_case(3466545, &[10, 20, 30], 13, 1, &[2, 2, 0, 0, 2, 1, 1, 1, 0, 0, 2, 2, 1])]
#[test_case(3466545, &[10, 20, 30], 13, 2, &[2, 2, 2, 2, 0, 0, 0, 0, 2, 2, 1, 1, 1])]
Copy link
Copy Markdown
Member Author

@vadorovsky vadorovsky Jan 15, 2026

Choose a reason for hiding this comment

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

This test case makes no sense - the last 1 is repeated 3 times and the whole thing is not divisible by 2. Given that there are many other cases which are correct, I'm just removing it.

Copy link
Copy Markdown

@kskalski kskalski Jan 16, 2026

Choose a reason for hiding this comment

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

The test case was supposed to test exactly the scenario you are forbidding - using len that is not a multiple of repeat. I agree though, that is impractical "feature", so should be fine to remove.

#[test_case(3466545, &[10, 20, 30], 14, 1, &[2, 2, 0, 0, 2, 1, 1, 1, 0, 0, 2, 2, 1, 2])]
#[test_case(3466545, &[10, 20, 30], 14, 2, &[2, 2, 2, 2, 0, 0, 0, 0, 2, 2, 1, 1, 1, 1])]
fn test_stake_leader_schedule_exact_order(
Expand Down
2 changes: 1 addition & 1 deletion ledger/src/leader_schedule/identity_keyed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ mod tests {
.collect();

let epoch = rand::random::<Epoch>();
let len = num_keys * 10;
let repeat = 8;
let len = num_keys * repeat;
let leader_schedule = LeaderSchedule::new(&stakes, epoch, len, repeat);
assert_eq!(leader_schedule.num_slots() as u64, len);
let mut leader_node = Pubkey::default();
Expand Down
2 changes: 1 addition & 1 deletion ledger/src/leader_schedule/vote_keyed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ mod tests {
.collect();

let epoch = rand::random::<Epoch>();
let len = num_keys * 10;
let repeat = 8;
let len = num_keys * repeat;
let leader_schedule = LeaderSchedule::new(&vote_accounts_map, epoch, len, repeat);
assert_eq!(leader_schedule.num_slots() as u64, len);
let mut leader_node = Pubkey::default();
Expand Down
4 changes: 2 additions & 2 deletions rpc/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4608,7 +4608,7 @@ pub mod tests {

const TEST_MINT_LAMPORTS: u64 = 1_000_000_000;
const TEST_SIGNATURE_FEE: u64 = 5_000;
const TEST_SLOTS_PER_EPOCH: u64 = DELINQUENT_VALIDATOR_SLOT_DISTANCE + 1;
const TEST_SLOTS_PER_EPOCH: u64 = 256;
Copy link
Copy Markdown
Member Author

@vadorovsky vadorovsky Jan 15, 2026

Choose a reason for hiding this comment

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

Happy to change it to some other value if anyone has strong feelings. What I know for sure is that using the validator's default (432_000) would make the tests running multiple minutes each.


pub(crate) fn new_test_cluster_info() -> ClusterInfo {
let keypair = Arc::new(Keypair::new());
Expand Down Expand Up @@ -5483,7 +5483,7 @@ pub mod tests {
parse_success_result(rpc.handle_request_sync(request));
let expected = Some(HashMap::from_iter(std::iter::once((
rpc.leader_pubkey().to_string(),
Vec::from_iter(0..=128),
Vec::from_iter(0..TEST_SLOTS_PER_EPOCH as usize),
))));
assert_eq!(result, expected);
}
Expand Down
Loading