From 6a9f72910141df9a27cc3985d2f615bd33f94938 Mon Sep 17 00:00:00 2001 From: HaoranYi Date: Thu, 18 Jan 2024 10:35:53 -0500 Subject: [PATCH] ForkStats: compute and wire up fork_weight (#34623) * wire up fork stats fork_weight * convert weight to percentage for logging * add bank_stake * remove old fork choice measure and stats - vote stake * lockout * update tests * fix bank_weight and rename it to fork_weight * fix u64 multiple overflow in fork_weight calculation * format fork_weight as percentage in logging --------- Co-authored-by: HaoranYi --- core/src/consensus.rs | 50 ++++++++++++++++-------------- core/src/consensus/progress_map.rs | 10 ++++-- core/src/replay_stage.rs | 24 ++++++++------ 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 917d373b5..54312baf3 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -156,8 +156,7 @@ pub type PubkeyVotes = Vec<(Pubkey, Slot)>; pub(crate) struct ComputedBankState { pub voted_stakes: VotedStakes, pub total_stake: Stake, - #[allow(dead_code)] - bank_weight: u128, + pub fork_stake: Stake, // Tree of intervals of lockouts of the form [slot, slot + slot.lockout], // keyed by end of the range pub lockout_intervals: LockoutIntervals, @@ -319,7 +318,7 @@ impl Tower { let mut vote_slots = HashSet::new(); let mut voted_stakes = HashMap::new(); let mut total_stake = 0; - let mut bank_weight = 0; + // Tree of intervals of lockouts of the form [slot, slot + slot.lockout], // keyed by end of the range let mut lockout_intervals = LockoutIntervals::new(); @@ -390,7 +389,6 @@ impl Tower { process_slot_vote_unchecked(&mut vote_state, bank_slot); for vote in &vote_state.votes { - bank_weight += vote.lockout.lockout() as u128 * voted_stake as u128; vote_slots.insert(vote.slot()); } @@ -399,13 +397,11 @@ impl Tower { let vote = Lockout::new_with_confirmation_count(root, MAX_LOCKOUT_HISTORY as u32); trace!("ROOT: {}", vote.slot()); - bank_weight += vote.lockout() as u128 * voted_stake as u128; vote_slots.insert(vote.slot()); } } if let Some(root) = vote_state.root_slot { let vote = Lockout::new_with_confirmation_count(root, MAX_LOCKOUT_HISTORY as u32); - bank_weight += vote.lockout() as u128 * voted_stake as u128; vote_slots.insert(vote.slot()); } @@ -437,10 +433,26 @@ impl Tower { // TODO: populate_ancestor_voted_stakes only adds zeros. Comment why // that is necessary (if so). Self::populate_ancestor_voted_stakes(&mut voted_stakes, vote_slots, ancestors); + + // As commented above, since the votes at current bank_slot are + // simulated votes, the voted_stake for `bank_slot` is not populated. + // Therefore, we use the voted_stake for the parent of bank_slot as the + // `fork_stake` instead. + let fork_stake = ancestors + .get(&bank_slot) + .and_then(|ancestors| { + ancestors + .iter() + .max() + .and_then(|parent| voted_stakes.get(parent)) + .copied() + }) + .unwrap_or(0); + ComputedBankState { voted_stakes, total_stake, - bank_weight, + fork_stake, lockout_intervals, my_latest_landed_vote, } @@ -2271,7 +2283,6 @@ pub mod test { let ComputedBankState { voted_stakes, total_stake, - bank_weight, .. } = Tower::collect_vote_lockouts( &Pubkey::default(), @@ -2286,10 +2297,6 @@ pub mod test { let mut new_votes = latest_validator_votes_for_frozen_banks.take_votes_dirty_set(0); new_votes.sort(); assert_eq!(new_votes, account_latest_votes); - - // Each account has 1 vote in it. After simulating a vote in collect_vote_lockouts, - // the account will have 2 votes, with lockout 2 + 4 = 6. So expected weight for - assert_eq!(bank_weight, 12) } #[test] @@ -2314,21 +2321,15 @@ pub mod test { ancestors.insert(i as u64, (0..i as u64).collect()); } let root = Lockout::new_with_confirmation_count(0, MAX_LOCKOUT_HISTORY as u32); - let root_weight = root.lockout() as u128; - let vote_account_expected_weight = tower - .vote_state - .votes - .iter() - .map(|v| v.lockout.lockout() as u128) - .sum::() - + root_weight; - let expected_bank_weight = 2 * vote_account_expected_weight; + let expected_bank_stake = 2; + let expected_total_stake = 2; assert_eq!(tower.vote_state.root_slot, Some(0)); let mut latest_validator_votes_for_frozen_banks = LatestValidatorVotesForFrozenBanks::default(); let ComputedBankState { voted_stakes, - bank_weight, + fork_stake, + total_stake, .. } = Tower::collect_vote_lockouts( &Pubkey::default(), @@ -2342,8 +2343,9 @@ pub mod test { assert_eq!(voted_stakes[&(i as u64)], 2); } - // should be the sum of all the weights for root - assert_eq!(bank_weight, expected_bank_weight); + // should be the sum of all voted stake for on the fork + assert_eq!(fork_stake, expected_bank_stake); + assert_eq!(total_stake, expected_total_stake); let mut new_votes = latest_validator_votes_for_frozen_banks.take_votes_dirty_set(root.slot()); new_votes.sort(); diff --git a/core/src/consensus/progress_map.rs b/core/src/consensus/progress_map.rs index a34509ef8..40efa58ff 100644 --- a/core/src/consensus/progress_map.rs +++ b/core/src/consensus/progress_map.rs @@ -293,8 +293,7 @@ impl ForkProgress { #[derive(Debug, Clone, Default)] pub struct ForkStats { - pub weight: u128, - pub fork_weight: u128, + pub fork_stake: Stake, pub total_stake: Stake, pub block_height: u64, pub has_voted: bool, @@ -310,6 +309,13 @@ pub struct ForkStats { pub my_latest_landed_vote: Option, } +impl ForkStats { + /// Return fork_weight, i.e. bank_stake over total_stake. + pub fn fork_weight(&self) -> f64 { + self.fork_stake as f64 / self.total_stake as f64 + } +} + #[derive(Clone, Default)] pub struct PropagatedStats { pub propagated_validators: HashSet, diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index d03dfd082..0ba4cdc50 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -3295,6 +3295,7 @@ impl ReplayStage { let ComputedBankState { voted_stakes, total_stake, + fork_stake, lockout_intervals, my_latest_landed_vote, .. @@ -3302,6 +3303,7 @@ impl ReplayStage { let stats = progress .get_fork_stats_mut(bank_slot) .expect("All frozen banks must exist in the Progress map"); + stats.fork_stake = fork_stake; stats.total_stake = total_stake; stats.voted_stakes = voted_stakes; stats.lockout_intervals = lockout_intervals; @@ -3312,15 +3314,15 @@ impl ReplayStage { datapoint_info!( "bank_weight", ("slot", bank_slot, i64), - // u128 too large for influx, convert to hex - ("weight", format!("{:X}", stats.weight), String), + ("fork_stake", stats.fork_stake, i64), + ("fork_weight", stats.fork_weight(), f64), ); + info!( - "{} slot_weight: {} {} {} {}", + "{} slot_weight: {} {:.1}% {}", my_vote_pubkey, bank_slot, - stats.weight, - stats.fork_weight, + 100.0 * stats.fork_weight(), // percentage fork_stake in total_stake bank.parent().map(|b| b.slot()).unwrap_or(0) ); } @@ -3677,7 +3679,7 @@ impl ReplayStage { fork_stats.vote_threshold, propagated_stats.propagated_validators_stake, propagated_stats.is_leader_slot, - fork_stats.weight, + fork_stats.fork_weight(), fork_stats.total_stake, propagated_stats.total_epoch_stake, ) @@ -3712,7 +3714,11 @@ impl ReplayStage { && propagation_confirmed && switch_fork_decision.can_vote() { - info!("voting: {} {}", candidate_vote_bank.slot(), fork_weight); + info!( + "voting: {} {:.1}%", + candidate_vote_bank.slot(), + 100.0 * fork_weight + ); SelectVoteAndResetForkResult { vote_bank: Some((candidate_vote_bank.clone(), switch_fork_decision)), reset_bank: Some(candidate_vote_bank.clone()), @@ -5427,12 +5433,12 @@ pub(crate) mod tests { .progress .get_fork_stats(pair[0].slot()) .unwrap() - .fork_weight; + .fork_weight(); let second = vote_simulator .progress .get_fork_stats(pair[1].slot()) .unwrap() - .fork_weight; + .fork_weight(); assert!(second >= first); } for bank in frozen_banks {