From fcd8a706129ad1466158f8183d0c903c3631670d Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 25 Sep 2023 19:34:05 -0700 Subject: [PATCH] v1.14: Update is_locked_out cache when adopting on chain vote state (backport of #33341) (#33402) * Update is_locked_out cache when adopting on chain vote state (#33341) * Update is_locked_out cache when adopting on chain vote state * extend to all cached tower checks * upgrade error to panic (cherry picked from commit 85cc6ace05fb9845f692a692d2e826414c2c0a46) # Conflicts: # core/src/consensus.rs * Fix conflicts --------- Co-authored-by: Ashwin Sekar --- core/src/consensus.rs | 2 +- core/src/replay_stage.rs | 157 +++++++++++++++++++++++++++++++-------- 2 files changed, 127 insertions(+), 32 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 2fdaab936a6560..6c29a9df15f108 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -538,7 +538,7 @@ impl Tower { let vote = Vote::new(vec![vote_slot], vote_hash); let result = self.vote_state.process_vote_unchecked(vote); if result.is_err() { - error!( + panic!( "Error while recording vote {} {} in local tower {:?}", vote_slot, vote_hash, result ); diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index f7ec6cbc1866f9..6913cc327a16f9 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -2823,7 +2823,7 @@ impl ReplayStage { pub fn compute_bank_stats( my_vote_pubkey: &Pubkey, ancestors: &HashMap>, - frozen_banks: &mut Vec>, + frozen_banks: &mut [Arc], tower: &mut Tower, progress: &mut ProgressMap, vote_tracker: &VoteTracker, @@ -2834,7 +2834,7 @@ impl ReplayStage { ) -> Vec { frozen_banks.sort_by_key(|bank| bank.slot()); let mut new_stats = vec![]; - for bank in frozen_banks { + for bank in frozen_banks.iter() { let bank_slot = bank.slot(); // Only time progress map should be missing a bank slot // is if this node was the leader for this slot as those banks @@ -2937,6 +2937,11 @@ impl ReplayStage { .get_hash(last_voted_slot) .expect("Must exist for us to have frozen descendant"), ); + // Since we are updating our tower we need to update associated caches for previously computed + // slots as well. + for slot in frozen_banks.iter().map(|b| b.slot()) { + Self::cache_tower_stats(progress, tower, slot, ancestors); + } } } } @@ -2997,24 +3002,33 @@ impl ReplayStage { cluster_slots, ); - let stats = progress - .get_fork_stats_mut(bank_slot) - .expect("All frozen banks must exist in the Progress map"); - - stats.vote_threshold = - tower.check_vote_stake_threshold(bank_slot, &stats.voted_stakes, stats.total_stake); - stats.is_locked_out = tower.is_locked_out( - bank_slot, - ancestors - .get(&bank_slot) - .expect("Ancestors map should contain slot for is_locked_out() check"), - ); - stats.has_voted = tower.has_voted(bank_slot); - stats.is_recent = tower.is_recent(bank_slot); + Self::cache_tower_stats(progress, tower, bank_slot, ancestors); } new_stats } + fn cache_tower_stats( + progress: &mut ProgressMap, + tower: &Tower, + slot: Slot, + ancestors: &HashMap>, + ) { + let stats = progress + .get_fork_stats_mut(slot) + .expect("All frozen banks must exist in the Progress map"); + + stats.vote_threshold = + tower.check_vote_stake_threshold(slot, &stats.voted_stakes, stats.total_stake); + stats.is_locked_out = tower.is_locked_out( + slot, + ancestors + .get(&slot) + .expect("Ancestors map should contain slot for is_locked_out() check"), + ); + stats.has_voted = tower.has_voted(slot); + stats.is_recent = tower.is_recent(slot); + } + fn update_propagation_status( progress: &mut ProgressMap, slot: Slot, @@ -5825,7 +5839,7 @@ pub(crate) mod tests { // All forks have same weight so heaviest bank to vote/reset on should be the tip of // the fork with the lower slot - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -5844,7 +5858,7 @@ pub(crate) mod tests { // 4 should be the heaviest slot, but should not be votable // because of lockout. 5 is the heaviest slot on the same fork as the last vote. - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -5887,7 +5901,7 @@ pub(crate) mod tests { // 4 should be the heaviest slot, but should not be votable // because of lockout. 5 is no longer valid due to it being a duplicate. - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -5923,7 +5937,7 @@ pub(crate) mod tests { // the right version of the block, so `duplicate_slots_to_repair` // should be empty assert!(duplicate_slots_to_repair.is_empty()); - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -5975,7 +5989,7 @@ pub(crate) mod tests { // All forks have same weight so heaviest bank to vote/reset on should be the tip of // the fork with the lower slot - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -6022,7 +6036,7 @@ pub(crate) mod tests { SlotStateUpdate::Duplicate(duplicate_state), ); - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -6057,7 +6071,7 @@ pub(crate) mod tests { SlotStateUpdate::Duplicate(duplicate_state), ); - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -6097,7 +6111,7 @@ pub(crate) mod tests { // the right version of the block, so `duplicate_slots_to_repair` // should be empty assert!(duplicate_slots_to_repair.is_empty()); - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, _) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -7123,7 +7137,7 @@ pub(crate) mod tests { heaviest_subtree_fork_choice: &mut HeaviestSubtreeForkChoice, latest_validator_votes_for_frozen_banks: &mut LatestValidatorVotesForFrozenBanks, my_vote_pubkey: Option, - ) -> (Option, Option) { + ) -> (Option, Option, Vec) { let mut frozen_banks: Vec<_> = bank_forks .read() .unwrap() @@ -7150,7 +7164,7 @@ pub(crate) mod tests { let SelectVoteAndResetForkResult { vote_bank, reset_bank, - .. + heaviest_fork_failures, } = ReplayStage::select_vote_and_reset_forks( &heaviest_bank, heaviest_bank_on_same_fork.as_ref(), @@ -7164,6 +7178,7 @@ pub(crate) mod tests { ( vote_bank.map(|(b, _)| b.slot()), reset_bank.map(|b| b.slot()), + heaviest_fork_failures, ) } @@ -7233,7 +7248,7 @@ pub(crate) mod tests { } #[test] - fn test_tower_sync_from_bank() { + fn test_tower_sync_from_bank_failed_switch() { solana_logger::setup_with_default( "error,solana_core::replay_stage=info,solana_core::consensus=info", ); @@ -7252,9 +7267,10 @@ pub(crate) mod tests { slot 6 We had some point voted 0 - 6, while the rest of the network voted 0 - 4. - We are sitting with an oudated tower that has voted until 1. We see that 2 is the heaviest slot, + We are sitting with an oudated tower that has voted until 1. We see that 4 is the heaviest slot, however in the past we have voted up to 6. We must acknowledge the vote state present at 6, - adopt it as our own and *not* vote on 2 or 4, to respect slashing rules. + adopt it as our own and *not* vote on 2 or 4, to respect slashing rules as there is + not enough stake to switch */ let generate_votes = |pubkeys: Vec| { @@ -7273,7 +7289,7 @@ pub(crate) mod tests { tower.record_vote(0, bank_hash(0)); tower.record_vote(1, bank_hash(1)); - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, failures) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -7284,8 +7300,12 @@ pub(crate) mod tests { assert_eq!(vote_fork, None); assert_eq!(reset_fork, Some(6)); + assert_eq!( + failures, + vec![HeaviestForkFailures::FailedSwitchThreshold(4)], + ); - let (vote_fork, reset_fork) = run_compute_and_select_forks( + let (vote_fork, reset_fork, failures) = run_compute_and_select_forks( &bank_forks, &mut progress, &mut tower, @@ -7296,5 +7316,80 @@ pub(crate) mod tests { assert_eq!(vote_fork, None); assert_eq!(reset_fork, Some(6)); + assert_eq!( + failures, + vec![HeaviestForkFailures::FailedSwitchThreshold(4)], + ); + } + + #[test] + fn test_tower_sync_from_bank_failed_lockout() { + solana_logger::setup_with_default( + "error,solana_core::replay_stage=info,solana_core::consensus=info", + ); + /* + Fork structure: + + slot 0 + | + slot 1 + / \ + slot 3 | + | slot 2 + slot 4 | + slot 5 + | + slot 6 + + We had some point voted 0 - 6, while the rest of the network voted 0 - 4. + We are sitting with an oudated tower that has voted until 1. We see that 4 is the heaviest slot, + however in the past we have voted up to 6. We must acknowledge the vote state present at 6, + adopt it as our own and *not* vote on 3 or 4, to respect slashing rules as we are locked + out on 4, even though there is enough stake to switch. However we should still reset onto + 4. + */ + + let generate_votes = |pubkeys: Vec| { + pubkeys + .into_iter() + .zip(iter::once(vec![0, 1, 2, 5, 6]).chain(iter::repeat(vec![0, 1, 3, 4]).take(2))) + .collect() + }; + let tree = tr(0) / (tr(1) / (tr(3) / (tr(4))) / (tr(2) / (tr(5) / (tr(6))))); + let (mut vote_simulator, _blockstore) = + setup_forks_from_tree(tree, 3, Some(Box::new(generate_votes))); + let (bank_forks, mut progress) = (vote_simulator.bank_forks, vote_simulator.progress); + let bank_hash = |slot| bank_forks.read().unwrap().bank_hash(slot).unwrap(); + let my_vote_pubkey = vote_simulator.vote_pubkeys[0]; + let mut tower = Tower::default(); + tower.node_pubkey = vote_simulator.node_pubkeys[0]; + tower.record_vote(0, bank_hash(0)); + tower.record_vote(1, bank_hash(1)); + + let (vote_fork, reset_fork, failures) = run_compute_and_select_forks( + &bank_forks, + &mut progress, + &mut tower, + &mut vote_simulator.heaviest_subtree_fork_choice, + &mut vote_simulator.latest_validator_votes_for_frozen_banks, + Some(my_vote_pubkey), + ); + + assert_eq!(vote_fork, None); + assert_eq!(reset_fork, Some(4)); + assert_eq!(failures, vec![HeaviestForkFailures::LockedOut(4),]); + + let (vote_fork, reset_fork, failures) = run_compute_and_select_forks( + &bank_forks, + &mut progress, + &mut tower, + &mut vote_simulator.heaviest_subtree_fork_choice, + &mut vote_simulator.latest_validator_votes_for_frozen_banks, + Some(my_vote_pubkey), + ); + + assert_eq!(vote_fork, None); + assert_eq!(reset_fork, Some(4)); + assert_eq!(failures, vec![HeaviestForkFailures::LockedOut(4),]); } }