Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
v1.16: tower: when syncing from vote state, update last_vote (backpor…
Browse files Browse the repository at this point in the history 
…t of #32944) (#32960)

* tower: when syncing from vote state, update last_vote (#32944)

* tower: when syncing from vote state, update last_vote

* pr: bubble error through unchecked

(cherry picked from commit 329c6f1)

# Conflicts:
#	programs/vote/src/vote_state/mod.rs

* fix conflicts

* fix bad merge

---------

Co-authored-by: Ashwin Sekar <[email protected]>
  • Loading branch information
@mergify @AshwinSekar
mergify[bot] and AshwinSekar authored Aug 24, 2023
1 parent 45dbe2b commit 9b06ca8
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 25 deletions.
54 changes: 35 additions & 19 deletions core/src/consensus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl Tower {
last_voted_slot_in_bank: Option<Slot>,
) -> VoteTransaction {
let vote = Vote::new(vec![slot], hash);
process_vote_unchecked(local_vote_state, vote);
let _ignored = process_vote_unchecked(local_vote_state, vote);
let slots = if let Some(last_voted_slot) = last_voted_slot_in_bank {
local_vote_state
.votes
Expand Down Expand Up @@ -544,6 +544,23 @@ 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
.iter()
.map(|vote| vote.lockout)
.collect(),
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 @@ -554,29 +571,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);
process_vote_unchecked(&mut self.vote_state, vote);
VoteTransaction::from(VoteStateUpdate::new(
self.vote_state
.votes
.iter()
.map(|vote| vote.lockout)
.collect(),
self.vote_state.root_slot,
vote_hash,
))
let result = process_vote_unchecked(&mut self.vote_state, 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 @@ -2529,7 +2545,7 @@ pub mod test {
hash: Hash::default(),
timestamp: None,
};
vote_state::process_vote_unchecked(&mut local, vote);
let _ = vote_state::process_vote_unchecked(&mut local, 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 @@ -2545,7 +2561,7 @@ pub mod test {
hash: Hash::default(),
timestamp: None,
};
vote_state::process_vote_unchecked(&mut local, vote);
let _ = vote_state::process_vote_unchecked(&mut local, 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
Original file line number Diff line number Diff line change
Expand Up @@ -3058,6 +3058,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(
// 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 @@ -5977,6 +6008,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 @@ -5995,6 +6027,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 @@ -6037,6 +6070,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 @@ -6072,6 +6106,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 @@ -6123,6 +6158,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 @@ -6169,6 +6205,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 @@ -6203,6 +6240,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 @@ -6242,6 +6280,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 @@ -7316,6 +7355,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 @@ -7327,7 +7367,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 @@ -7424,4 +7464,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));
}
}
11 changes: 6 additions & 5 deletions programs/vote/src/vote_state/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,15 +762,15 @@ pub fn process_vote(
}

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

#[cfg(test)]
Expand All @@ -781,7 +781,7 @@ pub fn process_slot_votes_unchecked(vote_state: &mut VoteState, slots: &[Slot])
}

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

/// Authorize the given pubkey to withdraw or sign votes. This may be called multiple times,
Expand Down Expand Up @@ -1751,7 +1751,8 @@ mod tests {
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

0 comments on commit 9b06ca8

Please sign in to comment.