From 0952b76f02fd684753dba1029446584ea9dacd5f Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 23 Jun 2020 11:59:54 +0900 Subject: [PATCH] Add {Vote, Tower}::last_voted_slot() (#10734) --- core/src/bank_weight_fork_choice.rs | 8 +++--- core/src/consensus.rs | 31 ++++++++++++------------ core/src/heaviest_subtree_fork_choice.rs | 4 +-- programs/vote/src/vote_state/mod.rs | 4 +++ 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/core/src/bank_weight_fork_choice.rs b/core/src/bank_weight_fork_choice.rs index 43bb026455c352..124f40cde91609 100644 --- a/core/src/bank_weight_fork_choice.rs +++ b/core/src/bank_weight_fork_choice.rs @@ -63,7 +63,7 @@ impl ForkChoice for BankWeightForkChoice { .filter(|b| b.slot() < tower.root().unwrap_or(0)) .count(); - let last_vote = tower.last_vote().slots.last().cloned(); + let last_voted_slot = tower.last_voted_slot(); let mut heaviest_bank_on_same_fork = None; let mut heaviest_same_fork_weight = 0; let stats: Vec<&ForkStats> = frozen_banks @@ -76,18 +76,18 @@ impl ForkChoice for BankWeightForkChoice { .get_fork_stats(bank.slot()) .expect("All frozen banks must exist in the Progress map"); - if let Some(last_vote) = last_vote { + if let Some(last_voted_slot) = last_voted_slot { if ancestors .get(&bank.slot()) .expect("Entry in frozen banks must exist in ancestors") - .contains(&last_vote) + .contains(&last_voted_slot) { // Descendant of last vote cannot be locked out assert!(!stats.is_locked_out); // ancestors(slot) should not contain the slot itself, // so we should never get the same bank as the last vote - assert_ne!(bank.slot(), last_vote); + assert_ne!(bank.slot(), last_voted_slot); // highest weight, lowest slot first. frozen_banks is sorted // from least slot to greatest slot, so if two banks have // the same fork weight, the lower slot will be picked diff --git a/core/src/consensus.rs b/core/src/consensus.rs index ee4276d2613fbe..2ad90b36c176b0 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -326,14 +326,17 @@ impl Tower { self.record_bank_vote(vote) } - pub fn last_vote(&self) -> Vote { - self.last_vote.clone() + pub fn last_vote(&self) -> &Vote { + &self.last_vote + } + + pub fn last_voted_slot(&self) -> Option { + self.last_vote().last_voted_slot() } pub fn last_vote_and_timestamp(&mut self) -> Vote { - let mut last_vote = self.last_vote(); - let current_slot = last_vote.slots.iter().max().unwrap_or(&0); - last_vote.timestamp = self.maybe_timestamp(*current_slot); + let mut last_vote = self.last_vote.clone(); + last_vote.timestamp = self.maybe_timestamp(last_vote.last_voted_slot().unwrap_or(0)); last_vote } @@ -397,14 +400,12 @@ impl Tower { total_stake: u64, epoch_vote_accounts: &HashMap, ) -> SwitchForkDecision { - self.last_vote() - .slots - .last() - .map(|last_vote| { - let last_vote_ancestors = ancestors.get(&last_vote).unwrap(); + self.last_voted_slot() + .map(|last_voted_slot| { + let last_vote_ancestors = ancestors.get(&last_voted_slot).unwrap(); let switch_slot_ancestors = ancestors.get(&switch_slot).unwrap(); - if switch_slot == *last_vote || switch_slot_ancestors.contains(last_vote) { + if switch_slot == last_voted_slot || switch_slot_ancestors.contains(&last_voted_slot) { // If the `switch_slot is a descendant of the last vote, // no switching proof is necessary return SwitchForkDecision::NoSwitch; @@ -427,14 +428,14 @@ impl Tower { // 3) Don't consider lockouts on any descendants of // `last_vote` if !descendants.is_empty() - || candidate_slot == last_vote + || *candidate_slot == last_voted_slot || ancestors .get(&candidate_slot) .expect( "empty descendants implies this is a child, not parent of root, so must exist in the ancestors map", ) - .contains(last_vote) + .contains(&last_voted_slot) { continue; } @@ -455,7 +456,7 @@ impl Tower { .lockout_intervals; // Find any locked out intervals in this bank with endpoint >= last_vote, // implies they are locked out at last_vote - for (_, value) in lockout_intervals.range((Included(last_vote), Unbounded)) { + for (_, value) in lockout_intervals.range((Included(last_voted_slot), Unbounded)) { for (lockout_interval_start, vote_account_pubkey) in value { // Only count lockouts on slots that are: // 1) Not ancestors of `last_vote` @@ -1747,7 +1748,7 @@ pub mod test { for i in 0..num_votes { tower.record_vote(i as u64, Hash::default()); } - assert_eq!(expected, tower.last_vote()) + assert_eq!(expected, tower.last_vote) } #[test] diff --git a/core/src/heaviest_subtree_fork_choice.rs b/core/src/heaviest_subtree_fork_choice.rs index 6881bdab6b1bfb..4edfd692f73a78 100644 --- a/core/src/heaviest_subtree_fork_choice.rs +++ b/core/src/heaviest_subtree_fork_choice.rs @@ -439,8 +439,8 @@ impl ForkChoice for HeaviestSubtreeForkChoice { _ancestors: &HashMap>, bank_forks: &RwLock, ) -> (Arc, Option>) { - let last_vote = tower.last_vote().slots.last().cloned(); - let heaviest_slot_on_same_voted_fork = last_vote.map(|last_vote| { + let last_voted_slot = tower.last_voted_slot(); + let heaviest_slot_on_same_voted_fork = last_voted_slot.map(|last_vote| { let heaviest_slot_on_same_voted_fork = self.best_slot(last_vote).expect("last_vote is a frozen bank so must have been added to heaviest_subtree_fork_choice at time of freezing"); if heaviest_slot_on_same_voted_fork == last_vote { diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index dc2686d2e9b7c5..0858f331450ed5 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -54,6 +54,10 @@ impl Vote { timestamp: None, } } + + pub fn last_voted_slot(&self) -> Option { + self.slots.last().copied() + } } #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone)]