replay: votes made before restart are eligible for refresh (#34737)

* replay: votes made before restart are eligible for refresh

* pr feedback: rename to mark

* pr feedback: limit scope to non voting validators
This commit is contained in:
Ashwin Sekar 2024-02-06 11:09:59 -08:00 committed by GitHub
parent 99760e519a
commit 3e24b410fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 89 additions and 47 deletions

View File

@ -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<Hash>,
// 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<Hash> {
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<Slot> {

View File

@ -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<Hash>,
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

View File

@ -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<Hash>,
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

View File

@ -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(),
};

View File

@ -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<Signature>,
has_new_vote_been_rooted: bool,
wait_to_vote_slot: Option<Slot>,
) -> Option<Transaction> {
) -> 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<Slot>,
) {
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(