From e1576b5352168cd857772526d39f1fb1698d2839 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Fri, 30 Jun 2023 17:53:06 -0700 Subject: [PATCH] Don't attempt to refresh votes on non voting validators (#32315) --- core/src/consensus.rs | 9 ++++---- core/src/consensus/tower1_14_11.rs | 2 +- core/src/consensus/tower1_7_14.rs | 2 +- core/src/consensus/tower_storage.rs | 2 +- core/src/replay_stage.rs | 36 ++++++++++++++++++++++------- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 777dba5e04..db03bb7a49 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -224,7 +224,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. - last_vote_tx_blockhash: Hash, + // For non voting validators this is None + last_vote_tx_blockhash: Option, last_timestamp: BlockTimestamp, #[serde(skip)] // Restored last voted slot which cannot be found in SlotHistory at replayed root @@ -247,7 +248,7 @@ impl Default for Tower { vote_state: VoteState::default(), last_vote: VoteTransaction::from(VoteStateUpdate::default()), last_timestamp: BlockTimestamp::default(), - last_vote_tx_blockhash: Hash::default(), + last_vote_tx_blockhash: None, stray_restored_slot: Option::default(), last_switch_threshold_check: Option::default(), }; @@ -460,7 +461,7 @@ impl Tower { self.vote_state.tower() } - pub fn last_vote_tx_blockhash(&self) -> Hash { + pub fn last_vote_tx_blockhash(&self) -> Option { self.last_vote_tx_blockhash } @@ -504,7 +505,7 @@ impl Tower { } pub fn refresh_last_vote_tx_blockhash(&mut self, new_vote_tx_blockhash: Hash) { - self.last_vote_tx_blockhash = new_vote_tx_blockhash; + self.last_vote_tx_blockhash = Some(new_vote_tx_blockhash); } // Returns true if we have switched the new vote instruction that directly sets vote state diff --git a/core/src/consensus/tower1_14_11.rs b/core/src/consensus/tower1_14_11.rs index 3d13efbd5a..befce93503 100644 --- a/core/src/consensus/tower1_14_11.rs +++ b/core/src/consensus/tower1_14_11.rs @@ -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: Hash, + pub(crate) last_vote_tx_blockhash: Option, 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 982ffffb60..62e5870b4e 100644 --- a/core/src/consensus/tower1_7_14.rs +++ b/core/src/consensus/tower1_7_14.rs @@ -22,7 +22,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: Hash, + pub(crate) last_vote_tx_blockhash: Option, 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 cfaf89299a..2e03d2006e 100644 --- a/core/src/consensus/tower_storage.rs +++ b/core/src/consensus/tower_storage.rs @@ -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: Hash::default(), + last_vote_tx_blockhash: None, 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 047aa9d784..a5e8516876 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -2368,9 +2368,14 @@ impl ReplayStage { last_voted_slot ); } + + // 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 }; + if my_latest_landed_vote >= last_voted_slot || heaviest_bank_on_same_fork - .is_hash_valid_for_age(&tower.last_vote_tx_blockhash(), MAX_PROCESSING_AGE) + .is_hash_valid_for_age(&last_vote_tx_blockhash, MAX_PROCESSING_AGE) || { // In order to avoid voting on multiple forks all past MAX_PROCESSING_AGE that don't // include the last voted blockhash @@ -7064,7 +7069,10 @@ pub(crate) mod tests { assert_eq!(votes.len(), 1); let vote_tx = &votes[0]; assert_eq!(vote_tx.message.recent_blockhash, bank0.last_blockhash()); - assert_eq!(tower.last_vote_tx_blockhash(), bank0.last_blockhash()); + assert_eq!( + tower.last_vote_tx_blockhash().unwrap(), + bank0.last_blockhash() + ); assert_eq!(tower.last_voted_slot().unwrap(), 0); bank1.process_transaction(vote_tx).unwrap(); bank1.freeze(); @@ -7093,7 +7101,10 @@ pub(crate) mod tests { let votes = cluster_info.get_votes(&mut cursor); assert!(votes.is_empty()); // Tower's latest vote tx blockhash hasn't changed either - assert_eq!(tower.last_vote_tx_blockhash(), bank0.last_blockhash()); + assert_eq!( + tower.last_vote_tx_blockhash().unwrap(), + bank0.last_blockhash() + ); assert_eq!(tower.last_voted_slot().unwrap(), 0); } @@ -7126,7 +7137,10 @@ pub(crate) mod tests { assert_eq!(votes.len(), 1); let vote_tx = &votes[0]; assert_eq!(vote_tx.message.recent_blockhash, bank1.last_blockhash()); - assert_eq!(tower.last_vote_tx_blockhash(), bank1.last_blockhash()); + assert_eq!( + tower.last_vote_tx_blockhash().unwrap(), + bank1.last_blockhash() + ); assert_eq!(tower.last_voted_slot().unwrap(), 1); // Trying to refresh the vote for bank 1 in bank 2 won't succeed because @@ -7148,7 +7162,10 @@ pub(crate) mod tests { // No new votes have been submitted to gossip let votes = cluster_info.get_votes(&mut cursor); assert!(votes.is_empty()); - assert_eq!(tower.last_vote_tx_blockhash(), bank1.last_blockhash()); + assert_eq!( + tower.last_vote_tx_blockhash().unwrap(), + bank1.last_blockhash() + ); assert_eq!(tower.last_voted_slot().unwrap(), 1); // Create a bank where the last vote transaction will have expired @@ -7207,7 +7224,7 @@ pub(crate) mod tests { expired_bank.last_blockhash() ); assert_eq!( - tower.last_vote_tx_blockhash(), + tower.last_vote_tx_blockhash().unwrap(), expired_bank.last_blockhash() ); assert_eq!(tower.last_voted_slot().unwrap(), 1); @@ -7263,7 +7280,7 @@ pub(crate) mod tests { expired_bank.last_blockhash() ); assert_eq!( - tower.last_vote_tx_blockhash(), + tower.last_vote_tx_blockhash().unwrap(), expired_bank.last_blockhash() ); assert_eq!(tower.last_voted_slot().unwrap(), 1); @@ -7320,7 +7337,10 @@ pub(crate) mod tests { vote_tx.message.recent_blockhash, parent_bank.last_blockhash() ); - assert_eq!(tower.last_vote_tx_blockhash(), parent_bank.last_blockhash()); + assert_eq!( + tower.last_vote_tx_blockhash().unwrap(), + parent_bank.last_blockhash() + ); assert_eq!(tower.last_voted_slot().unwrap(), parent_bank.slot()); let bank = Bank::new_from_parent(parent_bank, &Pubkey::default(), my_slot); bank.fill_bank_with_ticks_for_tests();