diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 54312baf3..4f129b182 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -210,6 +210,17 @@ impl TowerVersions { } } +#[derive(PartialEq, Eq, Debug, Default, Clone, Copy, AbiExample)] +pub(crate) enum BlockhashStatus { + /// No vote since restart + #[default] + Uninitialized, + /// Non voting validator + NonVoting, + /// Successfully generated vote tx with blockhash + Blockhash(Hash), +} + #[frozen_abi(digest = "iZi6s9BvytU3HbRsibrAD71jwMLvrqHdCjVk6qKcVvd")] #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, AbiExample)] pub struct Tower { @@ -223,8 +234,8 @@ pub struct Tower { // blockhash of the voted block itself, depending if the vote slot was refreshed. // For instance, a vote for slot 5, may be refreshed/resubmitted for inclusion in // block 10, in which case `last_vote_tx_blockhash` equals the blockhash of 10, not 5. - // For non voting validators this is None - last_vote_tx_blockhash: Option, + // For non voting validators this is NonVoting + last_vote_tx_blockhash: BlockhashStatus, last_timestamp: BlockTimestamp, #[serde(skip)] // Restored last voted slot which cannot be found in SlotHistory at replayed root @@ -247,7 +258,7 @@ impl Default for Tower { vote_state: VoteState::default(), last_vote: VoteTransaction::from(VoteStateUpdate::default()), last_timestamp: BlockTimestamp::default(), - last_vote_tx_blockhash: None, + last_vote_tx_blockhash: BlockhashStatus::default(), stray_restored_slot: Option::default(), last_switch_threshold_check: Option::default(), }; @@ -486,7 +497,7 @@ impl Tower { self.vote_state.tower() } - pub fn last_vote_tx_blockhash(&self) -> Option { + pub(crate) fn last_vote_tx_blockhash(&self) -> BlockhashStatus { self.last_vote_tx_blockhash } @@ -530,7 +541,11 @@ impl Tower { } pub fn refresh_last_vote_tx_blockhash(&mut self, new_vote_tx_blockhash: Hash) { - self.last_vote_tx_blockhash = Some(new_vote_tx_blockhash); + self.last_vote_tx_blockhash = BlockhashStatus::Blockhash(new_vote_tx_blockhash); + } + + pub(crate) fn mark_last_vote_tx_blockhash_non_voting(&mut self) { + self.last_vote_tx_blockhash = BlockhashStatus::NonVoting; } pub fn last_voted_slot_in_bank(bank: &Bank, vote_account_pubkey: &Pubkey) -> Option { diff --git a/core/src/consensus/tower1_14_11.rs b/core/src/consensus/tower1_14_11.rs index befce9350..22c396e09 100644 --- a/core/src/consensus/tower1_14_11.rs +++ b/core/src/consensus/tower1_14_11.rs @@ -1,6 +1,6 @@ use { - crate::consensus::SwitchForkDecision, - solana_sdk::{clock::Slot, hash::Hash, pubkey::Pubkey}, + crate::consensus::{BlockhashStatus, SwitchForkDecision}, + solana_sdk::{clock::Slot, pubkey::Pubkey}, solana_vote_program::vote_state::{ vote_state_1_14_11::VoteState1_14_11, BlockTimestamp, VoteTransaction, }, @@ -19,7 +19,7 @@ pub struct Tower1_14_11 { // blockhash of the voted block itself, depending if the vote slot was refreshed. // For instance, a vote for slot 5, may be refreshed/resubmitted for inclusion in // block 10, in which case `last_vote_tx_blockhash` equals the blockhash of 10, not 5. - pub(crate) last_vote_tx_blockhash: Option, + pub(crate) last_vote_tx_blockhash: BlockhashStatus, pub(crate) last_timestamp: BlockTimestamp, #[serde(skip)] // Restored last voted slot which cannot be found in SlotHistory at replayed root diff --git a/core/src/consensus/tower1_7_14.rs b/core/src/consensus/tower1_7_14.rs index 62e5870b4..725b78192 100644 --- a/core/src/consensus/tower1_7_14.rs +++ b/core/src/consensus/tower1_7_14.rs @@ -1,8 +1,7 @@ use { - crate::consensus::{Result, SwitchForkDecision, TowerError}, + crate::consensus::{BlockhashStatus, Result, SwitchForkDecision, TowerError}, solana_sdk::{ clock::Slot, - hash::Hash, pubkey::Pubkey, signature::{Signature, Signer}, }, @@ -22,7 +21,7 @@ pub struct Tower1_7_14 { // blockhash of the voted block itself, depending if the vote slot was refreshed. // For instance, a vote for slot 5, may be refreshed/resubmitted for inclusion in // block 10, in which case `last_vote_tx_blockhash` equals the blockhash of 10, not 5. - pub(crate) last_vote_tx_blockhash: Option, + pub(crate) last_vote_tx_blockhash: BlockhashStatus, pub(crate) last_timestamp: BlockTimestamp, #[serde(skip)] // Restored last voted slot which cannot be found in SlotHistory at replayed root diff --git a/core/src/consensus/tower_storage.rs b/core/src/consensus/tower_storage.rs index 61f3c0724..1e81f28f4 100644 --- a/core/src/consensus/tower_storage.rs +++ b/core/src/consensus/tower_storage.rs @@ -372,7 +372,7 @@ pub mod test { super::*, crate::consensus::{ tower1_7_14::{SavedTower1_7_14, Tower1_7_14}, - Tower, + BlockhashStatus, Tower, }, solana_sdk::{hash::Hash, signature::Keypair}, solana_vote_program::vote_state::{ @@ -403,7 +403,7 @@ pub mod test { vote_state: VoteState1_14_11::from(vote_state), last_vote: vote.clone(), last_timestamp: BlockTimestamp::default(), - last_vote_tx_blockhash: None, + last_vote_tx_blockhash: BlockhashStatus::Uninitialized, stray_restored_slot: Some(2), last_switch_threshold_check: Option::default(), }; diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 0ba4cdc50..27c30b9e5 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -15,8 +15,8 @@ use { latest_validator_votes_for_frozen_banks::LatestValidatorVotesForFrozenBanks, progress_map::{ForkProgress, ProgressMap, PropagatedStats, ReplaySlotStats}, tower_storage::{SavedTower, SavedTowerVersions, TowerStorage}, - ComputedBankState, Stake, SwitchForkDecision, ThresholdDecision, Tower, VotedStakes, - SWITCH_FORK_THRESHOLD, + BlockhashStatus, ComputedBankState, Stake, SwitchForkDecision, ThresholdDecision, + Tower, VotedStakes, SWITCH_FORK_THRESHOLD, }, cost_update_service::CostUpdate, repair::{ @@ -137,6 +137,20 @@ enum ConfirmationType { DuplicateConfirmed, } +enum GenerateVoteTxResult { + // non voting validator, not eligible for refresh + NonVoting, + // failed generation, eligible for refresh + Failed, + Tx(Transaction), +} + +impl GenerateVoteTxResult { + fn is_non_voting(&self) -> bool { + matches!(self, Self::NonVoting) + } +} + #[derive(PartialEq, Eq, Debug)] struct ConfirmedSlot { slot: Slot, @@ -2321,18 +2335,18 @@ impl ReplayStage { vote_signatures: &mut Vec, has_new_vote_been_rooted: bool, wait_to_vote_slot: Option, - ) -> Option { + ) -> GenerateVoteTxResult { if !bank.is_startup_verification_complete() { info!("startup verification incomplete, so unable to vote"); - return None; + return GenerateVoteTxResult::Failed; } if authorized_voter_keypairs.is_empty() { - return None; + return GenerateVoteTxResult::NonVoting; } if let Some(slot) = wait_to_vote_slot { if bank.slot() < slot { - return None; + return GenerateVoteTxResult::Failed; } } let vote_account = match bank.get_vote_account(vote_account_pubkey) { @@ -2341,7 +2355,7 @@ impl ReplayStage { "Vote account {} does not exist. Unable to vote", vote_account_pubkey, ); - return None; + return GenerateVoteTxResult::Failed; } Some(vote_account) => vote_account, }; @@ -2352,7 +2366,7 @@ impl ReplayStage { "Vote account {} is unreadable. Unable to vote", vote_account_pubkey, ); - return None; + return GenerateVoteTxResult::Failed; } Ok(vote_state) => vote_state, }; @@ -2363,7 +2377,7 @@ impl ReplayStage { vote_state.node_pubkey, node_keypair.pubkey() ); - return None; + return GenerateVoteTxResult::Failed; } let Some(authorized_voter_pubkey) = vote_state.get_authorized_voter(bank.epoch()) else { @@ -2372,7 +2386,7 @@ impl ReplayStage { vote_account_pubkey, bank.epoch() ); - return None; + return GenerateVoteTxResult::Failed; }; let authorized_voter_keypair = match authorized_voter_keypairs @@ -2382,7 +2396,7 @@ impl ReplayStage { None => { warn!("The authorized keypair {} for vote account {} is not available. Unable to vote", authorized_voter_pubkey, vote_account_pubkey); - return None; + return GenerateVoteTxResult::NonVoting; } Some(authorized_voter_keypair) => authorized_voter_keypair, }; @@ -2418,7 +2432,7 @@ impl ReplayStage { vote_signatures.clear(); } - Some(vote_tx) + GenerateVoteTxResult::Tx(vote_tx) } #[allow(clippy::too_many_arguments)] @@ -2457,13 +2471,23 @@ impl ReplayStage { // If we are a non voting validator or have an incorrect setup preventing us from // generating vote txs, no need to refresh - let Some(last_vote_tx_blockhash) = tower.last_vote_tx_blockhash() else { - return; + let last_vote_tx_blockhash = match tower.last_vote_tx_blockhash() { + // Since the checks in vote generation are deterministic, if we were non voting + // on the original vote, the refresh will also fail. No reason to refresh. + BlockhashStatus::NonVoting => return, + // In this case we have not voted since restart, it is unclear if we are non voting. + // Attempt to refresh. + BlockhashStatus::Uninitialized => None, + // Refresh if the blockhash is expired + BlockhashStatus::Blockhash(blockhash) => Some(blockhash), }; if my_latest_landed_vote >= last_voted_slot - || heaviest_bank_on_same_fork - .is_hash_valid_for_age(&last_vote_tx_blockhash, MAX_PROCESSING_AGE) + || { + last_vote_tx_blockhash.is_some() + && heaviest_bank_on_same_fork + .is_hash_valid_for_age(&last_vote_tx_blockhash.unwrap(), MAX_PROCESSING_AGE) + } || { // In order to avoid voting on multiple forks all past MAX_PROCESSING_AGE that don't // include the last voted blockhash @@ -2480,7 +2504,7 @@ impl ReplayStage { // Update timestamp for refreshed vote tower.refresh_last_vote_timestamp(heaviest_bank_on_same_fork.slot()); - let vote_tx = Self::generate_vote_tx( + let vote_tx_result = Self::generate_vote_tx( identity_keypair, heaviest_bank_on_same_fork, vote_account_pubkey, @@ -2492,7 +2516,7 @@ impl ReplayStage { wait_to_vote_slot, ); - if let Some(vote_tx) = vote_tx { + if let GenerateVoteTxResult::Tx(vote_tx) = vote_tx_result { let recent_blockhash = vote_tx.message.recent_blockhash; tower.refresh_last_vote_tx_blockhash(recent_blockhash); @@ -2511,6 +2535,8 @@ impl ReplayStage { }) .unwrap_or_else(|err| warn!("Error: {:?}", err)); last_vote_refresh_time.last_refresh_time = Instant::now(); + } else if vote_tx_result.is_non_voting() { + tower.mark_last_vote_tx_blockhash_non_voting(); } } @@ -2529,7 +2555,7 @@ impl ReplayStage { wait_to_vote_slot: Option, ) { let mut generate_time = Measure::start("generate_vote"); - let vote_tx = Self::generate_vote_tx( + let vote_tx_result = Self::generate_vote_tx( identity_keypair, bank, vote_account_pubkey, @@ -2542,7 +2568,7 @@ impl ReplayStage { ); generate_time.stop(); replay_timing.generate_vote_us += generate_time.as_us(); - if let Some(vote_tx) = vote_tx { + if let GenerateVoteTxResult::Tx(vote_tx) = vote_tx_result { tower.refresh_last_vote_tx_blockhash(vote_tx.message.recent_blockhash); let saved_tower = SavedTower::new(tower, identity_keypair).unwrap_or_else(|err| { @@ -2558,6 +2584,8 @@ impl ReplayStage { saved_tower: SavedTowerVersions::from(saved_tower), }) .unwrap_or_else(|err| warn!("Error: {:?}", err)); + } else if vote_tx_result.is_non_voting() { + tower.mark_last_vote_tx_blockhash_non_voting(); } } @@ -7480,8 +7508,8 @@ pub(crate) mod tests { let vote_tx = &votes[0]; assert_eq!(vote_tx.message.recent_blockhash, bank0.last_blockhash()); assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - bank0.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(bank0.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), 0); bank1.process_transaction(vote_tx).unwrap(); @@ -7517,8 +7545,8 @@ pub(crate) mod tests { assert!(votes.is_empty()); // Tower's latest vote tx blockhash hasn't changed either assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - bank0.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(bank0.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), 0); } @@ -7553,8 +7581,8 @@ pub(crate) mod tests { let vote_tx = &votes[0]; assert_eq!(vote_tx.message.recent_blockhash, bank1.last_blockhash()); assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - bank1.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(bank1.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), 1); @@ -7578,8 +7606,8 @@ pub(crate) mod tests { let votes = cluster_info.get_votes(&mut cursor); assert!(votes.is_empty()); assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - bank1.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(bank1.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), 1); @@ -7641,8 +7669,8 @@ pub(crate) mod tests { expired_bank.last_blockhash() ); assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - expired_bank.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(expired_bank.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), 1); @@ -7700,8 +7728,8 @@ pub(crate) mod tests { expired_bank.last_blockhash() ); assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - expired_bank.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(expired_bank.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), 1); } @@ -7758,8 +7786,8 @@ pub(crate) mod tests { parent_bank.last_blockhash() ); assert_eq!( - tower.last_vote_tx_blockhash().unwrap(), - parent_bank.last_blockhash() + tower.last_vote_tx_blockhash(), + BlockhashStatus::Blockhash(parent_bank.last_blockhash()) ); assert_eq!(tower.last_voted_slot().unwrap(), parent_bank.slot()); let bank = new_bank_from_parent_with_bank_forks(