Skip to content

Commit

Permalink
ff cleanup: deprecate_unused_legacy_vote_plumbing (#2585)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AshwinSekar
AshwinSekar authored Aug 15, 2024
1 parent de4cb11 commit 86d6020
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 68 deletions.
2 changes: 1 addition & 1 deletion programs/vote/benches/process_vote.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn create_accounts() -> (Slot, SlotHashes, Vec<TransactionAccount>, Vec<AccountM
);

for next_vote_slot in 0..num_initial_votes {
vote_state.process_next_vote_slot(next_vote_slot, 0, 0, true, true);
vote_state.process_next_vote_slot(next_vote_slot, 0, 0, true);
}
let mut vote_account_data: Vec<u8> = vec![0; VoteState::size_of()];
let versioned = VoteStateVersions::new_current(vote_state);
Expand Down
57 changes: 14 additions & 43 deletions programs/vote/src/vote_state/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,9 +626,6 @@ pub fn process_new_vote_state(
let timely_vote_credits = feature_set.map_or(false, |f| {
f.is_active(&feature_set::timely_vote_credits::id())
});
let deprecate_unused_legacy_vote_plumbing = feature_set.map_or(false, |f| {
f.is_active(&feature_set::deprecate_unused_legacy_vote_plumbing::id())
});
let mut earned_credits = if timely_vote_credits { 0_u64 } else { 1_u64 };

if let Some(new_root) = new_root {
Expand All @@ -641,7 +638,6 @@ pub fn process_new_vote_state(
.checked_add(vote_state.credits_for_vote_at_index(
current_vote_state_index,
timely_vote_credits,
deprecate_unused_legacy_vote_plumbing,
))
.expect("`earned_credits` does not overflow");
}
Expand Down Expand Up @@ -756,17 +752,10 @@ pub fn process_vote_unfiltered(
epoch: Epoch,
current_slot: Slot,
timely_vote_credits: bool,
deprecate_unused_legacy_vote_plumbing: bool,
) -> Result<(), VoteError> {
check_slots_are_valid(vote_state, vote_slots, &vote.hash, slot_hashes)?;
vote_slots.iter().for_each(|s| {
vote_state.process_next_vote_slot(
*s,
epoch,
current_slot,
timely_vote_credits,
deprecate_unused_legacy_vote_plumbing,
)
vote_state.process_next_vote_slot(*s, epoch, current_slot, timely_vote_credits)
});
Ok(())
}
Expand All @@ -778,7 +767,6 @@ pub fn process_vote(
epoch: Epoch,
current_slot: Slot,
timely_vote_credits: bool,
deprecate_unused_legacy_vote_plumbing: bool,
) -> Result<(), VoteError> {
if vote.slots.is_empty() {
return Err(VoteError::EmptySlots);
Expand All @@ -801,7 +789,6 @@ pub fn process_vote(
epoch,
current_slot,
timely_vote_credits,
deprecate_unused_legacy_vote_plumbing,
)
}

Expand All @@ -819,7 +806,6 @@ pub fn process_vote_unchecked(vote_state: &mut VoteState, vote: Vote) -> Result<
vote_state.current_epoch(),
0,
true,
true,
)
}

Expand Down Expand Up @@ -1095,16 +1081,13 @@ pub fn process_vote_with_account<S: std::hash::BuildHasher>(
let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?;

let timely_vote_credits = feature_set.is_active(&feature_set::timely_vote_credits::id());
let deprecate_unused_legacy_vote_plumbing =
feature_set.is_active(&feature_set::deprecate_unused_legacy_vote_plumbing::id());
process_vote(
&mut vote_state,
vote,
slot_hashes,
clock.epoch,
clock.slot,
timely_vote_credits,
deprecate_unused_legacy_vote_plumbing,
)?;
if let Some(timestamp) = vote.timestamp {
vote.slots
Expand Down Expand Up @@ -1342,7 +1325,7 @@ mod tests {
134, 135,
]
.into_iter()
.for_each(|v| vote_state.process_next_vote_slot(v, 4, 0, false, true));
.for_each(|v| vote_state.process_next_vote_slot(v, 4, 0, false));

let version1_14_11_serialized = bincode::serialize(&VoteStateVersions::V1_14_11(Box::new(
VoteState1_14_11::from(vote_state.clone()),
Expand Down Expand Up @@ -1823,11 +1806,11 @@ mod tests {
let slot_hashes: Vec<_> = vote.slots.iter().rev().map(|x| (*x, vote.hash)).collect();

assert_eq!(
process_vote(&mut vote_state_a, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state_a, &vote, &slot_hashes, 0, 0, true),
Ok(())
);
assert_eq!(
process_vote(&mut vote_state_b, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state_b, &vote, &slot_hashes, 0, 0, true),
Ok(())
);
assert_eq!(recent_votes(&vote_state_a), recent_votes(&vote_state_b));
Expand All @@ -1840,12 +1823,12 @@ mod tests {
let vote = Vote::new(vec![0], Hash::default());
let slot_hashes: Vec<_> = vec![(0, vote.hash)];
assert_eq!(
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true),
Ok(())
);
let recent = recent_votes(&vote_state);
assert_eq!(
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true),
Err(VoteError::VoteTooOld)
);
assert_eq!(recent, recent_votes(&vote_state));
Expand Down Expand Up @@ -1905,7 +1888,7 @@ mod tests {
let vote = Vote::new(vec![0], Hash::default());
let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)];
assert_eq!(
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true),
Ok(())
);
assert_eq!(
Expand All @@ -1921,7 +1904,7 @@ mod tests {
let vote = Vote::new(vec![0], Hash::default());
let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)];
assert_eq!(
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true),
Ok(())
);

Expand All @@ -1940,7 +1923,7 @@ mod tests {
let vote = Vote::new(vec![0], Hash::default());
let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)];
assert_eq!(
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true),
Ok(())
);

Expand All @@ -1957,7 +1940,7 @@ mod tests {

let vote = Vote::new(vec![], Hash::default());
assert_eq!(
process_vote(&mut vote_state, &vote, &[], 0, 0, true, true),
process_vote(&mut vote_state, &vote, &[], 0, 0, true),
Err(VoteError::EmptySlots)
);
}
Expand Down Expand Up @@ -2232,7 +2215,6 @@ mod tests {

let mut feature_set = FeatureSet::default();
feature_set.activate(&feature_set::timely_vote_credits::id(), 1);
feature_set.activate(&feature_set::deprecate_unused_legacy_vote_plumbing::id(), 1);

// For each vote group, process all vote groups leading up to it and it itself, and ensure that the number of
// credits earned is correct for both regular votes and vote state updates
Expand All @@ -2257,7 +2239,6 @@ mod tests {
0,
vote_group.1, // vote_group.1 is the slot in which the vote was cast
true,
true
),
Ok(())
);
Expand Down Expand Up @@ -2367,7 +2348,6 @@ mod tests {

let mut feature_set = FeatureSet::default();
feature_set.activate(&feature_set::timely_vote_credits::id(), 1);
feature_set.activate(&feature_set::deprecate_unused_legacy_vote_plumbing::id(), 1);

// Initial vote state
let mut vote_state = VoteState::new(&VoteInit::default(), &Clock::default());
Expand Down Expand Up @@ -3141,7 +3121,7 @@ mod tests {
// error with `VotesTooOldAllFiltered`
let slot_hashes = vec![(3, Hash::new_unique()), (2, Hash::new_unique())];
assert_eq!(
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true),
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true),
Err(VoteError::VotesTooOldAllFiltered)
);

Expand All @@ -3155,7 +3135,7 @@ mod tests {
.1;

let vote = Vote::new(vec![old_vote_slot, vote_slot], vote_slot_hash);
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true, true).unwrap();
process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0, true).unwrap();
assert_eq!(
vote_state
.votes
Expand Down Expand Up @@ -3184,17 +3164,8 @@ mod tests {
.unwrap()
.1;
let vote = Vote::new(vote_slots, vote_hash);
process_vote_unfiltered(
&mut vote_state,
&vote.slots,
&vote,
slot_hashes,
0,
0,
true,
true,
)
.unwrap();
process_vote_unfiltered(&mut vote_state, &vote.slots, &vote, slot_hashes, 0, 0, true)
.unwrap();
}

vote_state
Expand Down
30 changes: 6 additions & 24 deletions sdk/program/src/vote/state/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ pub const VOTE_CREDITS_GRACE_SLOTS: u8 = 2;
// Maximum number of credits to award for a vote; this number of credits is awarded to votes on slots that land within the grace period. After that grace period, vote credits are reduced.
pub const VOTE_CREDITS_MAXIMUM_PER_SLOT: u8 = 16;

// Previous max per slot
pub const VOTE_CREDITS_MAXIMUM_PER_SLOT_OLD: u8 = 8;

#[cfg_attr(
feature = "frozen-abi",
frozen_abi(digest = "Ch2vVEwos2EjAVqSHCyJjnN2MNX1yrpapZTGhMSCjWUH"),
Expand Down Expand Up @@ -668,7 +665,6 @@ impl VoteState {
epoch: Epoch,
current_slot: Slot,
timely_vote_credits: bool,
deprecate_unused_legacy_vote_plumbing: bool,
) {
// Ignore votes for slots earlier than we already have votes for
if self
Expand All @@ -681,7 +677,7 @@ impl VoteState {
self.pop_expired_votes(next_vote_slot);

let landed_vote = LandedVote {
latency: if timely_vote_credits || !deprecate_unused_legacy_vote_plumbing {
latency: if timely_vote_credits {
Self::compute_vote_latency(next_vote_slot, current_slot)
} else {
0
Expand All @@ -691,11 +687,7 @@ impl VoteState {

// Once the stack is full, pop the oldest lockout and distribute rewards
if self.votes.len() == MAX_LOCKOUT_HISTORY {
let credits = self.credits_for_vote_at_index(
0,
timely_vote_credits,
deprecate_unused_legacy_vote_plumbing,
);
let credits = self.credits_for_vote_at_index(0, timely_vote_credits);
let landed_vote = self.votes.pop_front().unwrap();
self.root_slot = Some(landed_vote.slot());

Expand Down Expand Up @@ -740,37 +732,27 @@ impl VoteState {
}

/// Returns the credits to award for a vote at the given lockout slot index
pub fn credits_for_vote_at_index(
&self,
index: usize,
timely_vote_credits: bool,
deprecate_unused_legacy_vote_plumbing: bool,
) -> u64 {
pub fn credits_for_vote_at_index(&self, index: usize, timely_vote_credits: bool) -> u64 {
let latency = self
.votes
.get(index)
.map_or(0, |landed_vote| landed_vote.latency);
let max_credits = if deprecate_unused_legacy_vote_plumbing {
VOTE_CREDITS_MAXIMUM_PER_SLOT
} else {
VOTE_CREDITS_MAXIMUM_PER_SLOT_OLD
};

// If latency is 0, this means that the Lockout was created and stored from a software version that did not
// store vote latencies; in this case, 1 credit is awarded
if latency == 0 || (deprecate_unused_legacy_vote_plumbing && !timely_vote_credits) {
if latency == 0 || !timely_vote_credits {
1
} else {
match latency.checked_sub(VOTE_CREDITS_GRACE_SLOTS) {
None | Some(0) => {
// latency was <= VOTE_CREDITS_GRACE_SLOTS, so maximum credits are awarded
max_credits as u64
VOTE_CREDITS_MAXIMUM_PER_SLOT as u64
}

Some(diff) => {
// diff = latency - VOTE_CREDITS_GRACE_SLOTS, and diff > 0
// Subtract diff from VOTE_CREDITS_MAXIMUM_PER_SLOT which is the number of credits to award
match max_credits.checked_sub(diff) {
match VOTE_CREDITS_MAXIMUM_PER_SLOT.checked_sub(diff) {
// If diff >= VOTE_CREDITS_MAXIMUM_PER_SLOT, 1 credit is awarded
None | Some(0) => 1,

Expand Down

0 comments on commit 86d6020

Please sign in to comment.