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.

Sign up for GitHub

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

Jump to bottom

v1.14: tower: when syncing from vote state, update last_vote (backport of #32944) #32959

Merged
merged 2 commits into from
Aug 24, 2023
Merged

v1.14: tower: when syncing from vote state, update last_vote (backport of #32944) #32959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2edb25f
tower: when syncing from vote state, update last_vote (#32944)
AshwinSekar Aug 23, 2023
b96790a
Fix conflicts
AshwinSekar Aug 23, 2023
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
46 changes: 31 additions & 15 deletions core/src/consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ impl Tower {
last_voted_slot_in_bank: Option<Slot>,
) -> VoteTransaction {
let vote = Vote::new(vec![slot], hash);
local_vote_state.process_vote_unchecked(vote);
let _ignored = local_vote_state.process_vote_unchecked(vote);
let slots = if let Some(last_voted_slot) = last_voted_slot_in_bank {
local_vote_state
.votes
Expand Down Expand Up @@ -511,6 +511,19 @@ impl Tower {
)
}

/// If we've recently updated the vote state by applying a new vote
/// or syncing from a bank, generate the proper last_vote.
pub(crate) fn update_last_vote_from_vote_state(&mut self, vote_hash: Hash) {
let mut new_vote = VoteTransaction::from(VoteStateUpdate::new(
self.vote_state.votes.clone(),
self.vote_state.root_slot,
vote_hash,
));

new_vote.set_timestamp(self.maybe_timestamp(self.last_voted_slot().unwrap_or_default()));
self.last_vote = new_vote;
}

fn record_bank_vote_and_update_lockouts(
&mut self,
vote_slot: Slot,
Expand All @@ -521,25 +534,28 @@ impl Tower {
trace!("{} record_vote for {}", self.node_pubkey, vote_slot);
let old_root = self.root();

let mut new_vote = if is_direct_vote_state_update_enabled {
if is_direct_vote_state_update_enabled {
let vote = Vote::new(vec![vote_slot], vote_hash);
self.vote_state.process_vote_unchecked(vote);
VoteTransaction::from(VoteStateUpdate::new(
self.vote_state.votes.clone(),
self.vote_state.root_slot,
vote_hash,
))
let result = self.vote_state.process_vote_unchecked(vote);
if result.is_err() {
error!(
"Error while recording vote {} {} in local tower {:?}",
vote_slot, vote_hash, result
);
}
self.update_last_vote_from_vote_state(vote_hash);
} else {
Self::apply_vote_and_generate_vote_diff(
let mut new_vote = Self::apply_vote_and_generate_vote_diff(
&mut self.vote_state,
vote_slot,
vote_hash,
last_voted_slot_in_bank,
)
};
);

new_vote.set_timestamp(self.maybe_timestamp(self.last_voted_slot().unwrap_or_default()));
self.last_vote = new_vote;
new_vote
.set_timestamp(self.maybe_timestamp(self.last_voted_slot().unwrap_or_default()));
self.last_vote = new_vote;
};

let new_root = self.root();

Expand Down Expand Up @@ -2473,7 +2489,7 @@ pub mod test {
hash: Hash::default(),
timestamp: None,
};
local.process_vote_unchecked(vote);
let _ = local.process_vote_unchecked(vote);
assert_eq!(local.votes.len(), 1);
let vote =
Tower::apply_vote_and_generate_vote_diff(&mut local, 1, Hash::default(), Some(0));
Expand All @@ -2489,7 +2505,7 @@ pub mod test {
hash: Hash::default(),
timestamp: None,
};
local.process_vote_unchecked(vote);
let _ = local.process_vote_unchecked(vote);
assert_eq!(local.votes.len(), 1);

// First vote expired, so should be evicted from tower. Thus even with
Expand Down
108 changes: 107 additions & 1 deletion core/src/replay_stage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2906,6 +2906,37 @@ impl ReplayStage {

tower.vote_state.root_slot = bank_vote_state.root_slot;
tower.vote_state.votes = bank_vote_state.votes;

let last_voted_slot =
tower.vote_state.last_voted_slot().unwrap_or_else(||
// If our local root is higher than the highest slot in `bank_vote_state` due to
// supermajority roots, then it's expected that the vote state will be empty.
// In this case we use the root as our last vote. This root cannot be None, because
// `tower.vote_state.last_voted_slot()` is None only if `tower.vote_state.root_slot`
// is Some.
tower
.vote_state
.root_slot
.expect("root_slot cannot be None here"));
// This is safe because `last_voted_slot` is now equal to
// `bank_vote_state.last_voted_slot()` or `local_root`.
// Since this vote state is contained in `bank`, which we have frozen,
// we must have frozen all slots contained in `bank_vote_state`,
// and by definition we must have frozen `local_root`.
//
// If `bank` is a duplicate, since we are able to replay it successfully, any slots
// in its vote state must also be part of the duplicate fork, and thus present in our
// progress map.
//
// Finally if both `bank` and `bank_vote_state.last_voted_slot()` are duplicate,
// we must have the compatible versions of both duplicates in order to replay `bank`
// successfully, so we are once again guaranteed that `bank_vote_state.last_voted_slot()`
// is present in progress map.
tower.update_last_vote_from_vote_state(
progress
.get_hash(last_voted_slot)
.expect("Must exist for us to have frozen descendant"),
);
}
}
}
Expand Down Expand Up @@ -5800,6 +5831,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
assert_eq!(vote_fork.unwrap(), 4);
assert_eq!(reset_fork.unwrap(), 4);
Expand All @@ -5818,6 +5850,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
assert!(vote_fork.is_none());
assert_eq!(reset_fork, Some(5));
Expand Down Expand Up @@ -5860,6 +5893,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
assert!(vote_fork.is_none());
assert!(reset_fork.is_none());
Expand Down Expand Up @@ -5895,6 +5929,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
// Should now pick 5 as the heaviest fork from last vote again.
assert!(vote_fork.is_none());
Expand Down Expand Up @@ -5946,6 +5981,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
assert_eq!(vote_fork.unwrap(), 4);
assert_eq!(reset_fork.unwrap(), 4);
Expand Down Expand Up @@ -5992,6 +6028,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
assert!(vote_fork.is_none());
assert_eq!(reset_fork, Some(3));
Expand Down Expand Up @@ -6026,6 +6063,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);

// Should now pick the next heaviest fork that is not a descendant of 2, which is 6.
Expand Down Expand Up @@ -6065,6 +6103,7 @@ pub(crate) mod tests {
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
None,
);
// Should now pick the heaviest fork 4 again, but lockouts apply so fork 4
// is not votable, which avoids voting for 4 again.
Expand Down Expand Up @@ -7083,6 +7122,7 @@ pub(crate) mod tests {
tower: &mut Tower,
heaviest_subtree_fork_choice: &mut HeaviestSubtreeForkChoice,
latest_validator_votes_for_frozen_banks: &mut LatestValidatorVotesForFrozenBanks,
my_vote_pubkey: Option<Pubkey>,
) -> (Option<Slot>, Option<Slot>) {
let mut frozen_banks: Vec<_> = bank_forks
.read()
Expand All @@ -7094,7 +7134,7 @@ pub(crate) mod tests {
let ancestors = &bank_forks.read().unwrap().ancestors();
let descendants = &bank_forks.read().unwrap().descendants();
ReplayStage::compute_bank_stats(
&Pubkey::default(),
&my_vote_pubkey.unwrap_or_default(),
&bank_forks.read().unwrap().ancestors(),
&mut frozen_banks,
tower,
Expand Down Expand Up @@ -7191,4 +7231,70 @@ pub(crate) mod tests {
ReplayStage::check_for_vote_only_mode(10, 0, &in_vote_only_mode, &bank_forks);
assert!(!in_vote_only_mode.load(Ordering::Relaxed));
}

#[test]
fn test_tower_sync_from_bank() {
solana_logger::setup_with_default(
"error,solana_core::replay_stage=info,solana_core::consensus=info",
);
/*
Fork structure:

slot 0
|
slot 1
/ \
slot 2 |
| slot 3
slot 4 |
slot 5
|
slot 6

We had some point voted 0 - 6, while the rest of the network voted 0 - 4.
We are sitting with an oudated tower that has voted until 1. We see that 2 is the heaviest slot,
however in the past we have voted up to 6. We must acknowledge the vote state present at 6,
adopt it as our own and *not* vote on 2 or 4, to respect slashing rules.
*/

let generate_votes = |pubkeys: Vec<Pubkey>| {
pubkeys
.into_iter()
.zip(iter::once(vec![0, 1, 3, 5, 6]).chain(iter::repeat(vec![0, 1, 2, 4]).take(2)))
.collect()
};
let (mut vote_simulator, _blockstore) =
setup_default_forks(3, Some(Box::new(generate_votes)));
let (bank_forks, mut progress) = (vote_simulator.bank_forks, vote_simulator.progress);
let bank_hash = |slot| bank_forks.read().unwrap().bank_hash(slot).unwrap();
let my_vote_pubkey = vote_simulator.vote_pubkeys[0];
let mut tower = Tower::default();
tower.node_pubkey = vote_simulator.node_pubkeys[0];
tower.record_vote(0, bank_hash(0));
tower.record_vote(1, bank_hash(1));

let (vote_fork, reset_fork) = run_compute_and_select_forks(
&bank_forks,
&mut progress,
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
Some(my_vote_pubkey),
);

assert_eq!(vote_fork, None);
assert_eq!(reset_fork, Some(6));

let (vote_fork, reset_fork) = run_compute_and_select_forks(
&bank_forks,
&mut progress,
&mut tower,
&mut vote_simulator.heaviest_subtree_fork_choice,
&mut vote_simulator.latest_validator_votes_for_frozen_banks,
Some(my_vote_pubkey),
);

assert_eq!(vote_fork, None);
assert_eq!(reset_fork, Some(6));
}
}
18 changes: 10 additions & 8 deletions programs/vote/src/vote_state/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,9 +1071,9 @@ impl VoteState {
}

/// "unchecked" functions used by tests and Tower
pub fn process_vote_unchecked(&mut self, vote: Vote) {
pub fn process_vote_unchecked(&mut self, vote: Vote) -> Result<(), VoteError> {
let slot_hashes: Vec<_> = vote.slots.iter().rev().map(|x| (*x, vote.hash)).collect();
let _ignored = self.process_vote(&vote, &slot_hashes, self.current_epoch(), None);
self.process_vote(&vote, &slot_hashes, self.current_epoch(), None)
}

#[cfg(test)]
Expand All @@ -1084,7 +1084,7 @@ impl VoteState {
}

pub fn process_slot_vote_unchecked(&mut self, slot: Slot) {
self.process_vote_unchecked(Vote::new(vec![slot], Hash::default()));
let _ = self.process_vote_unchecked(Vote::new(vec![slot], Hash::default()));
}

pub fn nth_recent_vote(&self, position: usize) -> Option<&Lockout> {
Expand Down Expand Up @@ -2180,11 +2180,13 @@ mod tests {
// Duplicate vote_state so that the new vote can be applied
let mut vote_state_after_vote = vote_state.clone();

vote_state_after_vote.process_vote_unchecked(Vote {
slots: vote_group.clone(),
hash: Hash::new_unique(),
timestamp: None,
});
vote_state_after_vote
.process_vote_unchecked(Vote {
slots: vote_group.clone(),
hash: Hash::new_unique(),
timestamp: None,
})
.unwrap();

// Now use the resulting new vote state to perform a vote state update on vote_state
assert_eq!(
Expand Down