Skip to content

Commit

Permalink
fix(epoch_manager): Some fixes and comments
Browse files Browse the repository at this point in the history 
Some fixes, refactoring and comments.

Test plan
---------
Run existing tests
Add more asserts
  • Loading branch information
@mikhailOK
mikhailOK committed Mar 5, 2020
1 parent 299940d commit df3800d
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 98 deletions.
29 changes: 28 additions & 1 deletion chain/epoch_manager/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ impl EpochManager {
validator_reward,
0,
)?;
// Dummy block info.
// Artificial block we add to simplify implementation: dummy block is the
// parent of genesis block that points to itself.
// If we view it as block in epoch -1 and height -1, it naturally extends the
// EpochId formula using T-2 for T=1, and height field is unused.
let block_info = BlockInfo::default();
let mut store_update = epoch_manager.store.store_update();
epoch_manager.save_epoch_info(&mut store_update, &genesis_epoch_id, epoch_info)?;
Expand All @@ -86,6 +91,21 @@ impl EpochManager {
Ok(epoch_manager)
}

/// # Parameters
/// epoch_info
/// block_validator_tracker
/// chunk_validator_tracker
///
/// slashed: set of slashed validators
/// prev_validator_kickout: previously kicked out
///
/// # Returns
/// (set of validators to kickout, set of validators to reward with stats)
///
/// - Slashed validators are ignored (they are handled separately)
/// - A validator is kicked out if he produced too few blocks or chunks
/// - If all validators are either previously kicked out or to be kicked out, we choose one not to
/// kick out
fn compute_kickout_info(
&self,
epoch_info: &EpochInfo,
Expand Down Expand Up @@ -203,7 +223,7 @@ impl EpochManager {
&slashed_validators,
&prev_validator_kickout,
);
validator_kickout = validator_kickout.union(&kickout).cloned().collect();
validator_kickout.extend(kickout);
debug!(
"All proposals: {:?}, Kickouts: {:?}, Block Tracker: {:?}, Shard Tracker: {:?}",
all_proposals, validator_kickout, block_validator_tracker, chunk_validator_tracker
Expand Down Expand Up @@ -938,6 +958,10 @@ impl EpochManager {
}
}

/// Get BlockInfo for a block
/// # Errors
/// EpochError::Other if storage returned an error
/// EpochError::MissingBlock if block is not in storage
pub fn get_block_info(&mut self, hash: &CryptoHash) -> Result<&BlockInfo, EpochError> {
if self.blocks_info.cache_get(hash).is_none() {
let block_info = self
Expand Down Expand Up @@ -1404,8 +1428,11 @@ mod tests {
epoch_manager.get_slashed_validators(&h[2]).unwrap().clone().into_iter().collect();
let slashed2: Vec<_> =
epoch_manager.get_slashed_validators(&h[3]).unwrap().clone().into_iter().collect();
let slashed3: Vec<_> =
epoch_manager.get_slashed_validators(&h[5]).unwrap().clone().into_iter().collect();
assert_eq!(slashed1, vec![("test1".to_string(), SlashState::Other)]);
assert_eq!(slashed2, vec![("test1".to_string(), SlashState::AlreadySlashed)]);
assert_eq!(slashed3, vec![("test1".to_string(), SlashState::AlreadySlashed)]);
}

/// Test that double sign interacts with other challenges in the correct way.
Expand Down
118 changes: 49 additions & 69 deletions chain/epoch_manager/src/proposals.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::btree_map::Entry;
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
use std::iter;

Expand Down Expand Up @@ -42,36 +41,31 @@ pub fn proposals_to_epoch_info(
) -> Result<EpochInfo, EpochError> {
// Combine proposals with rollovers.
let mut ordered_proposals = BTreeMap::new();
// Account -> new_stake
let mut stake_change = BTreeMap::new();
let mut fishermen = vec![];
let mut fishermen_to_index = HashMap::new();
debug_assert!(
proposals.iter().map(|stake| &stake.account_id).collect::<HashSet<_>>().len()
== proposals.len(),
"Proposals should not have duplicates"
);

for p in proposals {
if validator_kickout.contains(&p.account_id) {
stake_change.insert(p.account_id, (0, p.stake));
stake_change.insert(p.account_id, 0);
} else {
stake_change.insert(p.account_id.clone(), p.stake);
ordered_proposals.insert(p.account_id.clone(), p);
}
}
for r in epoch_info.validators.iter() {
match ordered_proposals.entry(r.account_id.clone()) {
Entry::Occupied(mut e) => {
let p = e.get_mut();
let return_stake = if r.stake > p.stake { r.stake - p.stake } else { 0 };
let reward = *validator_reward.get(&r.account_id).unwrap_or(&0);
p.stake += reward;
stake_change.insert(r.account_id.clone(), (p.stake, return_stake));
}
Entry::Vacant(e) => {
if !validator_kickout.contains(&r.account_id) {
let mut proposal = r.clone();
proposal.stake += *validator_reward.get(&r.account_id).unwrap_or(&0);
e.insert(proposal);
} else {
stake_change.insert(r.account_id.clone(), (0, r.stake));
}
}
if validator_kickout.contains(&r.account_id) {
stake_change.insert(r.account_id.clone(), 0);
continue;
}
let p = ordered_proposals.entry(r.account_id.clone()).or_insert_with(|| r.clone());
p.stake += *validator_reward.get(&p.account_id).unwrap_or(&0);
stake_change.insert(p.account_id.clone(), p.stake);
}

for r in epoch_info.fishermen.iter() {
Expand All @@ -80,9 +74,8 @@ pub fn proposals_to_epoch_info(
{
// safe to do this here because fishermen from previous epoch is guaranteed to have no
// duplicates.
fishermen_to_index.insert(r.account_id.clone(), fishermen.len() as ValidatorId);
fishermen.push(r.clone());
stake_change.insert(r.account_id.clone(), (r.stake, 0));
stake_change.insert(r.account_id.clone(), r.stake);
}
}

Expand All @@ -97,25 +90,12 @@ pub fn proposals_to_epoch_info(

for (account_id, p) in ordered_proposals {
if p.stake >= threshold {
if !stake_change.contains_key(&account_id) {
stake_change.insert(account_id, (p.stake, 0));
}
final_proposals.push(p);
} else if p.stake >= epoch_config.fishermen_threshold {
// Do not return stake back since they will become fishermen
stake_change.entry(account_id.clone()).or_insert((p.stake, 0));
fishermen_to_index.insert(account_id, fishermen.len() as ValidatorId);
fishermen.push(p);
} else {
stake_change
.entry(account_id.clone())
.and_modify(|(new_stake, return_stake)| {
if *new_stake != 0 {
*return_stake += *new_stake;
*new_stake = 0;
}
})
.or_insert((0, p.stake));
*stake_change.get_mut(&account_id).unwrap() = 0;
if epoch_info.validator_to_index.contains_key(&account_id)
|| epoch_info.fishermen_to_index.contains_key(&account_id)
{
Expand All @@ -130,12 +110,8 @@ pub fn proposals_to_epoch_info(
.enumerate()
.flat_map(|(i, p)| iter::repeat(i as u64).take((p.stake / threshold) as usize))
.collect::<Vec<_>>();
if dup_proposals.len() < num_total_seats as usize {
return Err(EpochError::SelectedSeatsMismatch(
dup_proposals.len() as NumSeats,
num_total_seats,
));
}

assert!(dup_proposals.len() >= num_total_seats as usize, "bug in find_threshold");
{
use protocol_defining_rand::seq::SliceRandom;
use protocol_defining_rand::{rngs::StdRng, SeedableRng};
Expand All @@ -148,29 +124,24 @@ pub fn proposals_to_epoch_info(
let mut block_producers_settlement =
dup_proposals[..epoch_config.num_block_producer_seats as usize].to_vec();
// remove proposals that are not selected
let mut indices_to_remove = (0..final_proposals.len()).collect::<BTreeSet<_>>();
for index in block_producers_settlement.iter() {
indices_to_remove.remove(&(*index as usize));
}
let (final_proposals, proposals_to_remove, validator_to_index) =
final_proposals.into_iter().enumerate().fold(
(vec![], vec![], HashMap::new()),
|(mut proposals, mut to_remove, mut indices), (i, p)| {
if indices_to_remove.contains(&i) {
to_remove.push(p);
} else {
indices.insert(p.account_id.clone(), proposals.len() as ValidatorId);
proposals.push(p);
}
(proposals, to_remove, indices)
},
);
let indices_to_keep = block_producers_settlement.iter().copied().collect::<BTreeSet<_>>();
let (final_proposals, proposals_to_remove) = final_proposals.into_iter().enumerate().fold(
(vec![], vec![]),
|(mut proposals, mut to_remove), (i, p)| {
if indices_to_keep.contains(&(i as u64)) {
proposals.push(p);
} else {
to_remove.push(p);
}
(proposals, to_remove)
},
);
for p in proposals_to_remove {
debug_assert!(p.stake >= threshold);
if p.stake >= epoch_config.fishermen_threshold {
fishermen_to_index.insert(p.account_id.clone(), fishermen.len() as ValidatorId);
fishermen.push(p);
} else {
stake_change.insert(p.account_id.clone(), (0, p.stake));
stake_change.insert(p.account_id.clone(), 0);
if epoch_info.validator_to_index.contains_key(&p.account_id)
|| epoch_info.fishermen_to_index.contains_key(&p.account_id)
{
Expand All @@ -181,26 +152,35 @@ pub fn proposals_to_epoch_info(

// reset indices
for index in block_producers_settlement.iter_mut() {
*index -= indices_to_remove.range(..(*index as usize)).count() as u64;
*index = indices_to_keep.range(..*index).count() as u64;
}

// Collect proposals into block producer assignments.
let mut chunk_producers_settlement: Vec<Vec<ValidatorId>> = vec![];
let mut last_index: u64 = 0;
for num_seats_in_shard in epoch_config.num_block_producer_seats_per_shard.iter() {
let mut shard_settlement: Vec<ValidatorId> = vec![];
for i in 0..*num_seats_in_shard {
let proposal_index = block_producers_settlement
[((i + last_index) % epoch_config.num_block_producer_seats) as usize];
for _ in 0..*num_seats_in_shard {
let proposal_index = block_producers_settlement[last_index as usize];
shard_settlement.push(proposal_index);
last_index = (last_index + 1) % epoch_config.num_block_producer_seats;
}
chunk_producers_settlement.push(shard_settlement);
last_index = (last_index + num_seats_in_shard) % epoch_config.num_block_producer_seats;
}

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

let final_stake_change = stake_change.into_iter().map(|(k, (v, _))| (k, v)).collect();
let validator_to_index = final_proposals
.iter()
.enumerate()
.map(|(index, s)| (s.account_id.clone(), index as ValidatorId))
.collect::<HashMap<_, _>>();

// TODO(1050): implement fishermen allocation.

Ok(EpochInfo {
validators: final_proposals,
Expand All @@ -209,7 +189,7 @@ pub fn proposals_to_epoch_info(
block_producers_settlement,
chunk_producers_settlement,
hidden_validators_settlement: vec![],
stake_change: final_stake_change,
stake_change,
validator_reward,
inflation,
validator_kickout,
Expand Down
18 changes: 0 additions & 18 deletions chain/epoch_manager/src/test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,6 @@ pub fn stake(account_id: &str, amount: Balance) -> ValidatorStake {
ValidatorStake::new(account_id.to_string(), public_key, amount)
}

pub fn reward_calculator(
max_inflation_rate: u8,
num_blocks_per_year: u64,
epoch_length: BlockHeightDelta,
validator_reward_percentage: u8,
protocol_reward_percentage: u8,
protocol_treasury_account: AccountId,
) -> RewardCalculator {
RewardCalculator {
max_inflation_rate,
num_blocks_per_year,
epoch_length,
validator_reward_percentage,
protocol_reward_percentage,
protocol_treasury_account,
}
}

/// No-op reward calculator. Will produce no reward
pub fn default_reward_calculator() -> RewardCalculator {
RewardCalculator {
Expand Down
14 changes: 4 additions & 10 deletions chain/epoch_manager/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub struct BlockInfo {
pub epoch_id: EpochId,
pub proposals: Vec<ValidatorStake>,
pub chunk_mask: Vec<bool>,
/// Validators slashed since the start of epoch or in previous epoch
pub slashed: HashMap<AccountId, SlashState>,
/// Total rent paid in this block.
pub rent_paid: Balance,
Expand Down Expand Up @@ -196,8 +197,6 @@ pub enum EpochError {
ThresholdError(Balance, u64),
/// Requesting validators for an epoch that wasn't computed yet.
EpochOutOfBounds,
/// Number of selected seats doesn't match requested.
SelectedSeatsMismatch(NumSeats, NumSeats),
/// Missing block hash in the storage (means there is some structural issue).
MissingBlock(CryptoHash),
/// Other error.
Expand All @@ -215,11 +214,6 @@ impl fmt::Debug for EpochError {
stakes_sum, num_seats
),
EpochError::EpochOutOfBounds => write!(f, "Epoch out of bounds"),
EpochError::SelectedSeatsMismatch(selected, required) => write!(
f,
"Number of selected seats {} < total number of seats {}",
selected, required
),
EpochError::MissingBlock(hash) => write!(f, "Missing block {}", hash),
EpochError::Other(err) => write!(f, "Other: {}", err),
}
Expand All @@ -233,9 +227,6 @@ impl fmt::Display for EpochError {
write!(f, "ThresholdError({}, {})", stake, num_seats)
}
EpochError::EpochOutOfBounds => write!(f, "EpochOutOfBounds"),
EpochError::SelectedSeatsMismatch(selected, required) => {
write!(f, "SelectedSeatsMismatch({}, {})", selected, required)
}
EpochError::MissingBlock(hash) => write!(f, "MissingBlock({})", hash),
EpochError::Other(err) => write!(f, "Other({})", err),
}
Expand All @@ -261,8 +252,11 @@ impl From<EpochError> for near_chain::Error {

pub struct EpochSummary {
pub prev_epoch_last_block_hash: CryptoHash,
// Proposals from the epoch, only the latest one per account
pub all_proposals: Vec<ValidatorStake>,
// Kickout set, includes slashed
pub validator_kickout: HashSet<AccountId>,
// Only for validators who met the threshold and didn't get slashed
pub validator_block_chunk_stats: HashMap<AccountId, BlockChunkValidatorStats>,
pub total_storage_rent: Balance,
pub total_validator_reward: Balance,
Expand Down
2 changes: 2 additions & 0 deletions core/primitives/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ impl StateRootNode {
}

/// Epoch identifier -- wrapped hash, to make it easier to distinguish.
/// EpochId of epoch T is the hash of last block in T-2
/// EpochId of first two epochs is 0
#[derive(
Hash,
Eq,
Expand Down

0 comments on commit df3800d

Please sign in to comment.