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

feat: consider all proposals for chunk validators #11252

Merged
merged 22 commits into from
May 15, 2024
3 changes: 0 additions & 3 deletions chain/chain/src/test_utils/kv_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,9 +530,6 @@ impl EpochManagerAdapter for MockEpochManager {
validator_to_index,
bp_settlement,
cp_settlement,
vec![],
vec![],
HashMap::new(),
BTreeMap::new(),
HashMap::new(),
HashMap::new(),
Expand Down
10 changes: 0 additions & 10 deletions chain/epoch-manager/src/proposals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ pub fn proposals_to_epoch_info(
validator_kickout,
validator_reward,
minted_amount,
current_version,
next_version,
);
} else {
Expand Down Expand Up @@ -237,12 +236,6 @@ mod old_validator_selection {
chunk_producers_settlement.push(shard_settlement);
}

let fishermen_to_index = fishermen
.iter()
.enumerate()
.map(|(index, s)| (s.account_id().clone(), index as ValidatorId))
.collect::<HashMap<_, _>>();

let validator_to_index = final_proposals
.iter()
.enumerate()
Expand All @@ -258,9 +251,6 @@ mod old_validator_selection {
validator_to_index,
block_producers_settlement,
chunk_producers_settlement,
vec![],
fishermen,
fishermen_to_index,
stake_change,
validator_reward,
validator_kickout,
Expand Down
27 changes: 11 additions & 16 deletions chain/epoch-manager/src/shard_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,20 @@ use near_primitives::utils::min_heap::{MinHeap, PeekMut};
/// producer will be assigned to a single shard. If there are fewer producers,
/// some of them will be assigned to multiple shards.
///
/// Panics if chunk_producers vector is not sorted in descending order by
/// producer’s stake.
/// Panics if `chunk_producers.len() < min_validators_per_shard` or chunk_producers
/// vector is not sorted in descending order by producer’s stake.
pub fn assign_shards<T: HasStake + Eq + Clone>(
chunk_producers: Vec<T>,
num_shards: NumShards,
min_validators_per_shard: usize,
) -> Result<Vec<Vec<T>>, NotEnoughValidators> {
) -> Result<Vec<Vec<T>>, ()> {
// If there's not enough chunk producers to fill up a single shard there’s
// nothing we can do. Return with an error.
let num_chunk_producers = chunk_producers.len();
if num_chunk_producers < min_validators_per_shard {
return Err(());
}

for (idx, pair) in chunk_producers.windows(2).enumerate() {
assert!(
pair[0].get_stake() >= pair[1].get_stake(),
Expand All @@ -29,13 +36,6 @@ pub fn assign_shards<T: HasStake + Eq + Clone>(
);
}

// If there’s not enough chunk producers to fill up a single shard there’s
// nothing we can do. Return with an error.
let num_chunk_producers = chunk_producers.len();
if num_chunk_producers < min_validators_per_shard {
return Err(NotEnoughValidators);
}

let mut result: Vec<Vec<T>> = (0..num_shards).map(|_| Vec::new()).collect();

// Initially, sort by number of validators first so we fill shards up.
Expand Down Expand Up @@ -135,11 +135,6 @@ fn assign_with_possible_repeats<T: HasStake + Eq, I: Iterator<Item = (usize, T)>
}
}

/// Marker struct to communicate the error where you try to assign validators to shards
/// and there are not enough to even meet the minimum per shard.
#[derive(Debug)]
pub struct NotEnoughValidators;
tayfunelmas marked this conversation as resolved.
Show resolved Hide resolved

pub trait HasStake {
fn get_stake(&self) -> Balance;
}
Expand Down Expand Up @@ -226,7 +221,7 @@ mod tests {
stakes: &[Balance],
num_shards: NumShards,
min_validators_per_shard: usize,
) -> Result<Vec<(usize, Balance)>, super::NotEnoughValidators> {
) -> Result<Vec<(usize, Balance)>, ()> {
let chunk_producers = stakes.iter().copied().enumerate().collect();
let assignments =
super::assign_shards(chunk_producers, num_shards, min_validators_per_shard)?;
Expand Down
9 changes: 2 additions & 7 deletions chain/epoch-manager/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ pub fn epoch_info_with_num_seats(
mut accounts: Vec<(AccountId, Balance)>,
block_producers_settlement: Vec<ValidatorId>,
chunk_producers_settlement: Vec<Vec<ValidatorId>>,
hidden_validators_settlement: Vec<ValidatorWeight>,
fishermen: Vec<(AccountId, Balance)>,
_hidden_validators_settlement: Vec<ValidatorWeight>,
_fishermen: Vec<(AccountId, Balance)>,
stake_change: BTreeMap<AccountId, Balance>,
validator_kickout: Vec<(AccountId, ValidatorKickoutReason)>,
validator_reward: HashMap<AccountId, Balance>,
Expand All @@ -91,8 +91,6 @@ pub fn epoch_info_with_num_seats(
acc.insert(x.0.clone(), i as u64);
acc
});
let fishermen_to_index =
fishermen.iter().enumerate().map(|(i, (s, _))| (s.clone(), i as ValidatorId)).collect();
let account_to_validators = |accounts: Vec<(AccountId, Balance)>| -> Vec<ValidatorStake> {
accounts
.into_iter()
Expand Down Expand Up @@ -121,9 +119,6 @@ pub fn epoch_info_with_num_seats(
validator_to_index,
block_producers_settlement,
chunk_producers_settlement,
hidden_validators_settlement,
account_to_validators(fishermen),
fishermen_to_index,
stake_change,
validator_reward,
validator_kickout.into_iter().collect(),
Expand Down