Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add {Vote, Tower}::last_voted_slot() #10734

Merged
merged 1 commit into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/src/bank_weight_fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
31 changes: 16 additions & 15 deletions core/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,14 +336,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<Slot> {
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
}

Expand Down Expand Up @@ -407,14 +410,12 @@ impl Tower {
total_stake: u64,
epoch_vote_accounts: &HashMap<Pubkey, (u64, Account)>,
) -> 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;
Expand All @@ -437,14 +438,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;
}
Expand All @@ -465,7 +466,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`
Expand Down Expand Up @@ -1831,7 +1832,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]
Expand Down
4 changes: 2 additions & 2 deletions core/src/heaviest_subtree_fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,8 @@ impl ForkChoice for HeaviestSubtreeForkChoice {
_ancestors: &HashMap<u64, HashSet<u64>>,
bank_forks: &RwLock<BankForks>,
) -> (Arc<Bank>, Option<Arc<Bank>>) {
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 {
Expand Down
4 changes: 4 additions & 0 deletions programs/vote/src/vote_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ impl Vote {
timestamp: None,
}
}

pub fn last_voted_slot(&self) -> Option<Slot> {
self.slots.last().copied()
Copy link
Contributor

@carllin carllin Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to start being more mindful to do this over .cloned() when possible, good reminder!

}
}

#[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone)]
Expand Down