Skip to content

Commit

Permalink
consensus: make shallow threshold checks log only (solana-labs#1506)
Browse files Browse the repository at this point in the history 
* consensus: make shallow threshold checks log only

* pr feedback: comment, make check more readable
  • Loading branch information
@AshwinSekar
AshwinSekar authored May 30, 2024
1 parent 7476d56 commit 6859d65
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 25 deletions.
50 changes: 30 additions & 20 deletions core/src/consensus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1198,18 +1198,24 @@ impl Tower {
slot: Slot,
voted_stakes: &VotedStakes,
total_stake: Stake,
) -> ThresholdDecision {
) -> Vec<ThresholdDecision> {
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(
Expand All @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -2644,15 +2654,15 @@ 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]
fn test_check_vote_threshold_above_threshold_no_stake() {
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]
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/consensus/progress_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ThresholdDecision>,
pub is_locked_out: bool,
pub voted_stakes: VotedStakes,
pub is_supermajority_confirmed: bool,
Expand Down
17 changes: 13 additions & 4 deletions core/src/replay_stage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3855,7 +3855,7 @@ impl ReplayStage {
// checks
let (
is_locked_out,
vote_threshold,
vote_thresholds,
propagated_stake,
is_leader_slot,
fork_weight,
Expand All @@ -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(),
Expand All @@ -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(
Expand All @@ -3902,7 +3911,7 @@ impl ReplayStage {
}

if !is_locked_out
&& vote_threshold.passed()
&& threshold_passed
&& propagation_confirmed
&& switch_fork_decision.can_vote()
{
Expand Down

0 comments on commit 6859d65

Please sign in to comment.