From 6859d652b2789587a1015567a6c603fe4361bd64 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Thu, 30 May 2024 13:14:47 -0400 Subject: [PATCH] consensus: make shallow threshold checks log only (#1506) * consensus: make shallow threshold checks log only * pr feedback: comment, make check more readable --- core/src/consensus.rs | 50 ++++++++++++++++++------------ core/src/consensus/progress_map.rs | 2 +- core/src/replay_stage.rs | 17 +++++++--- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index c18eb931a20f29..6ad9b58da9e9ea 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -242,7 +242,7 @@ pub(crate) enum BlockhashStatus { #[derive(Clone, Serialize, Deserialize, Debug, PartialEq)] pub struct Tower { pub node_pubkey: Pubkey, - threshold_depth: usize, + pub(crate) threshold_depth: usize, threshold_size: f64, pub(crate) vote_state: VoteState, last_vote: VoteTransaction, @@ -1198,18 +1198,24 @@ impl Tower { slot: Slot, voted_stakes: &VotedStakes, total_stake: Stake, - ) -> ThresholdDecision { + ) -> Vec { + let mut threshold_decisions = vec![]; // Generate the vote state assuming this vote is included. let mut vote_state = self.vote_state.clone(); process_slot_vote_unchecked(&mut vote_state, slot); // Assemble all the vote thresholds and depths to check. let vote_thresholds_and_depths = vec![ + // The following two checks are log only and are currently being used for experimentation + // purposes. We wish to impose a shallow threshold check to prevent the frequent 8 deep + // lockouts seen multiple times a day. We check both the 4th and 5th deep here to collect + // metrics to determine the right depth and threshold percentage to set in the future. (VOTE_THRESHOLD_DEPTH_SHALLOW, SWITCH_FORK_THRESHOLD), + (VOTE_THRESHOLD_DEPTH_SHALLOW + 1, SWITCH_FORK_THRESHOLD), (self.threshold_depth, self.threshold_size), ]; - // Check one by one. If any threshold fails, return failure. + // Check one by one and add any failures to be returned for (threshold_depth, threshold_size) in vote_thresholds_and_depths { if let ThresholdDecision::FailedThreshold(vote_depth, stake) = Self::check_vote_stake_threshold( @@ -1222,10 +1228,10 @@ impl Tower { total_stake, ) { - return ThresholdDecision::FailedThreshold(vote_depth, stake); + threshold_decisions.push(ThresholdDecision::FailedThreshold(vote_depth, stake)); } } - ThresholdDecision::PassedThreshold + threshold_decisions } /// Update lockouts for all the ancestors @@ -2435,7 +2441,7 @@ pub mod test { fn test_check_vote_threshold_without_votes() { let tower = Tower::new_for_tests(1, 0.67); let stakes = vec![(0, 1)].into_iter().collect(); - assert!(tower.check_vote_stake_thresholds(0, &stakes, 2).passed()); + assert!(tower.check_vote_stake_thresholds(0, &stakes, 2).is_empty()); } #[test] @@ -2449,7 +2455,7 @@ pub mod test { } assert!(!tower .check_vote_stake_thresholds(MAX_LOCKOUT_HISTORY as u64 + 1, &stakes, 2) - .passed()); + .is_empty()); } #[test] @@ -2585,28 +2591,32 @@ pub mod test { let mut tower = Tower::new_for_tests(1, 0.67); let stakes = vec![(0, 1)].into_iter().collect(); tower.record_vote(0, Hash::default()); - assert!(!tower.check_vote_stake_thresholds(1, &stakes, 2).passed()); + assert!(!tower.check_vote_stake_thresholds(1, &stakes, 2).is_empty()); } #[test] fn test_check_vote_threshold_above_threshold() { let mut tower = Tower::new_for_tests(1, 0.67); let stakes = vec![(0, 2)].into_iter().collect(); tower.record_vote(0, Hash::default()); - assert!(tower.check_vote_stake_thresholds(1, &stakes, 2).passed()); + assert!(tower.check_vote_stake_thresholds(1, &stakes, 2).is_empty()); } #[test] fn test_check_vote_thresholds_above_thresholds() { let mut tower = Tower::new_for_tests(VOTE_THRESHOLD_DEPTH, 0.67); - let stakes = vec![(0, 3), (VOTE_THRESHOLD_DEPTH_SHALLOW as u64, 2)] - .into_iter() - .collect(); + let stakes = vec![ + (0, 3), + (VOTE_THRESHOLD_DEPTH_SHALLOW as u64, 2), + ((VOTE_THRESHOLD_DEPTH_SHALLOW as u64) - 1, 2), + ] + .into_iter() + .collect(); for slot in 0..VOTE_THRESHOLD_DEPTH { tower.record_vote(slot as Slot, Hash::default()); } assert!(tower .check_vote_stake_thresholds(VOTE_THRESHOLD_DEPTH.try_into().unwrap(), &stakes, 4) - .passed()); + .is_empty()); } #[test] @@ -2620,7 +2630,7 @@ pub mod test { } assert!(!tower .check_vote_stake_thresholds(VOTE_THRESHOLD_DEPTH.try_into().unwrap(), &stakes, 10) - .passed()); + .is_empty()); } #[test] @@ -2634,7 +2644,7 @@ pub mod test { } assert!(!tower .check_vote_stake_thresholds(VOTE_THRESHOLD_DEPTH.try_into().unwrap(), &stakes, 10) - .passed()); + .is_empty()); } #[test] @@ -2644,7 +2654,7 @@ pub mod test { tower.record_vote(0, Hash::default()); tower.record_vote(1, Hash::default()); tower.record_vote(2, Hash::default()); - assert!(tower.check_vote_stake_thresholds(6, &stakes, 2).passed()); + assert!(tower.check_vote_stake_thresholds(6, &stakes, 2).is_empty()); } #[test] @@ -2652,7 +2662,7 @@ pub mod test { let mut tower = Tower::new_for_tests(1, 0.67); let stakes = HashMap::new(); tower.record_vote(0, Hash::default()); - assert!(!tower.check_vote_stake_thresholds(1, &stakes, 2).passed()); + assert!(!tower.check_vote_stake_thresholds(1, &stakes, 2).is_empty()); } #[test] @@ -2663,7 +2673,7 @@ pub mod test { tower.record_vote(0, Hash::default()); tower.record_vote(1, Hash::default()); tower.record_vote(2, Hash::default()); - assert!(tower.check_vote_stake_thresholds(6, &stakes, 2).passed()); + assert!(tower.check_vote_stake_thresholds(6, &stakes, 2).is_empty()); } #[test] @@ -2728,7 +2738,7 @@ pub mod test { ); assert!(tower .check_vote_stake_thresholds(vote_to_evaluate, &voted_stakes, total_stake) - .passed()); + .is_empty()); // CASE 2: Now we want to evaluate a vote for slot VOTE_THRESHOLD_DEPTH + 1. This slot // will expire the vote in one of the vote accounts, so we should have insufficient @@ -2748,7 +2758,7 @@ pub mod test { ); assert!(!tower .check_vote_stake_thresholds(vote_to_evaluate, &voted_stakes, total_stake) - .passed()); + .is_empty()); } fn vote_and_check_recent(num_votes: usize) { diff --git a/core/src/consensus/progress_map.rs b/core/src/consensus/progress_map.rs index 82951123aab66f..0925182e721a5e 100644 --- a/core/src/consensus/progress_map.rs +++ b/core/src/consensus/progress_map.rs @@ -183,7 +183,7 @@ pub struct ForkStats { pub has_voted: bool, pub is_recent: bool, pub is_empty: bool, - pub vote_threshold: ThresholdDecision, + pub vote_threshold: Vec, pub is_locked_out: bool, pub voted_stakes: VotedStakes, pub is_supermajority_confirmed: bool, diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 465428ad9aac08..86b9efb0e3cf38 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -3855,7 +3855,7 @@ impl ReplayStage { // checks let ( is_locked_out, - vote_threshold, + vote_thresholds, propagated_stake, is_leader_slot, fork_weight, @@ -3868,7 +3868,7 @@ impl ReplayStage { .unwrap(); ( fork_stats.is_locked_out, - fork_stats.vote_threshold, + &fork_stats.vote_threshold, propagated_stats.propagated_validators_stake, propagated_stats.is_leader_slot, fork_stats.fork_weight(), @@ -3885,13 +3885,22 @@ impl ReplayStage { if is_locked_out { failure_reasons.push(HeaviestForkFailures::LockedOut(candidate_vote_bank.slot())); } - if let ThresholdDecision::FailedThreshold(vote_depth, fork_stake) = vote_threshold { + let mut threshold_passed = true; + for threshold_failure in vote_thresholds { + let &ThresholdDecision::FailedThreshold(vote_depth, fork_stake) = threshold_failure + else { + continue; + }; failure_reasons.push(HeaviestForkFailures::FailedThreshold( candidate_vote_bank.slot(), vote_depth, fork_stake, total_threshold_stake, )); + // Ignore shallow checks for voting purposes + if (vote_depth as usize) >= tower.threshold_depth { + threshold_passed = false; + } } if !propagation_confirmed { failure_reasons.push(HeaviestForkFailures::NoPropagatedConfirmation( @@ -3902,7 +3911,7 @@ impl ReplayStage { } if !is_locked_out - && vote_threshold.passed() + && threshold_passed && propagation_confirmed && switch_fork_decision.can_vote() {