From bdf7207eecafd813365d25670eec398c2fce901a Mon Sep 17 00:00:00 2001 From: bji Date: Tue, 12 Sep 2023 10:40:04 -0700 Subject: [PATCH] Implement timely vote credits feature. (#32957) --- cli-output/src/cli_output.rs | 36 +- cli/src/vote.rs | 4 +- programs/vote/benches/process_vote.rs | 2 +- programs/vote/src/vote_state/mod.rs | 680 +++++++++++++++++++++++--- rpc/src/rpc.rs | 8 +- sdk/program/src/vote/state/mod.rs | 66 ++- sdk/src/feature_set.rs | 5 + 7 files changed, 704 insertions(+), 97 deletions(-) diff --git a/cli-output/src/cli_output.rs b/cli-output/src/cli_output.rs index 68e12e9c803d55..6fc394f6709530 100644 --- a/cli-output/src/cli_output.rs +++ b/cli-output/src/cli_output.rs @@ -43,9 +43,7 @@ use { }, solana_vote_program::{ authorized_voters::AuthorizedVoters, - vote_state::{ - BlockTimestamp, LandedVote, Lockout, MAX_EPOCH_CREDITS_HISTORY, MAX_LOCKOUT_HISTORY, - }, + vote_state::{BlockTimestamp, LandedVote, MAX_EPOCH_CREDITS_HISTORY, MAX_LOCKOUT_HISTORY}, }, std::{ collections::{BTreeMap, HashMap}, @@ -1047,7 +1045,7 @@ impl fmt::Display for CliKeyedEpochRewards { fn show_votes_and_credits( f: &mut fmt::Formatter, - votes: &[CliLockout], + votes: &[CliLandedVote], epoch_voting_history: &[CliEpochVotingHistory], ) -> fmt::Result { if votes.is_empty() { @@ -1070,11 +1068,16 @@ fn show_votes_and_credits( )?; for vote in votes.iter().rev() { - writeln!( + write!( f, "- slot: {} (confirmation count: {})", vote.slot, vote.confirmation_count )?; + if vote.latency == 0 { + writeln!(f)?; + } else { + writeln!(f, " (latency {})", vote.latency)?; + } } if let Some(newest) = newest_history_entry { writeln!( @@ -1555,7 +1558,7 @@ pub struct CliVoteAccount { pub commission: u8, pub root_slot: Option, pub recent_timestamp: BlockTimestamp, - pub votes: Vec, + pub votes: Vec, pub epoch_voting_history: Vec, #[serde(skip_serializing)] pub use_lamports_unit: bool, @@ -1637,25 +1640,18 @@ pub struct CliEpochVotingHistory { #[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct CliLockout { +pub struct CliLandedVote { + pub latency: u8, pub slot: Slot, pub confirmation_count: u32, } -impl From<&Lockout> for CliLockout { - fn from(lockout: &Lockout) -> Self { - Self { - slot: lockout.slot(), - confirmation_count: lockout.confirmation_count(), - } - } -} - -impl From<&LandedVote> for CliLockout { - fn from(vote: &LandedVote) -> Self { +impl From<&LandedVote> for CliLandedVote { + fn from(landed_vote: &LandedVote) -> Self { Self { - slot: vote.slot(), - confirmation_count: vote.confirmation_count(), + latency: landed_vote.latency, + slot: landed_vote.slot(), + confirmation_count: landed_vote.confirmation_count(), } } } diff --git a/cli/src/vote.rs b/cli/src/vote.rs index bde158295c3434..6c98e49c3bff42 100644 --- a/cli/src/vote.rs +++ b/cli/src/vote.rs @@ -23,7 +23,7 @@ use { offline::*, }, solana_cli_output::{ - return_signers_with_config, CliEpochVotingHistory, CliLockout, CliVoteAccount, + return_signers_with_config, CliEpochVotingHistory, CliLandedVote, CliVoteAccount, ReturnSignersConfig, }, solana_remote_wallet::remote_wallet::RemoteWalletManager, @@ -1215,7 +1215,7 @@ pub fn process_show_vote_account( let epoch_schedule = rpc_client.get_epoch_schedule()?; - let mut votes: Vec = vec![]; + let mut votes: Vec = vec![]; let mut epoch_voting_history: Vec = vec![]; if !vote_state.votes.is_empty() { for vote in &vote_state.votes { diff --git a/programs/vote/benches/process_vote.rs b/programs/vote/benches/process_vote.rs index c60fe5a68f4eba..6c9cb979c90484 100644 --- a/programs/vote/benches/process_vote.rs +++ b/programs/vote/benches/process_vote.rs @@ -48,7 +48,7 @@ fn create_accounts() -> (Slot, SlotHashes, Vec, Vec = vec![0; VoteState::size_of()]; let versioned = VoteStateVersions::new_current(vote_state); diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index f3a0904b13670f..e83171d06e0844 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -545,11 +545,12 @@ fn check_slots_are_valid( // popped off. pub fn process_new_vote_state( vote_state: &mut VoteState, - new_state: VecDeque, + mut new_state: VecDeque, new_root: Option, timestamp: Option, epoch: Epoch, - _feature_set: Option<&FeatureSet>, + current_slot: Slot, + feature_set: Option<&FeatureSet>, ) -> Result<(), VoteError> { assert!(!new_state.is_empty()); if new_state.len() > MAX_LOCKOUT_HISTORY { @@ -568,7 +569,7 @@ pub fn process_new_vote_state( _ => (), } - let mut previous_vote: Option<&Lockout> = None; + let mut previous_vote: Option<&LandedVote> = None; // Check that all the votes in the new proposed state are: // 1) Strictly sorted from oldest to newest vote @@ -597,7 +598,7 @@ pub fn process_new_vote_state( return Err(VoteError::SlotsNotOrdered); } else if previous_vote.confirmation_count() <= vote.confirmation_count() { return Err(VoteError::ConfirmationsNotOrdered); - } else if vote.slot() > previous_vote.last_locked_out_slot() { + } else if vote.slot() > previous_vote.lockout.last_locked_out_slot() { return Err(VoteError::NewVoteStateLockoutMismatch); } } @@ -609,27 +610,27 @@ pub fn process_new_vote_state( let mut current_vote_state_index: usize = 0; let mut new_vote_state_index = 0; - // Count the number of slots at and before the new root within the current vote state lockouts. Start with 1 - // for the new root. The purpose of this is to know how many slots were rooted by this state update: - // - The new root was rooted - // - As were any slots that were in the current state but are not in the new state. The only slots which - // can be in this set are those oldest slots in the current vote state that are not present in the - // new vote state; these have been "popped off the back" of the tower and thus represent finalized slots - let mut finalized_slot_count = 1_u64; + // Accumulate credits earned by newly rooted slots. The behavior changes with timely_vote_credits: prior to + // this feature, there was a bug that counted a new root slot as 1 credit even if it had never been voted on. + // timely_vote_credits fixes this bug by only awarding credits for slots actually voted on and finalized. + let timely_vote_credits = feature_set.map_or(false, |f| { + f.is_active(&feature_set::timely_vote_credits::id()) + }); + let mut earned_credits = if timely_vote_credits { 0_u64 } else { 1_u64 }; if let Some(new_root) = new_root { for current_vote in &vote_state.votes { // Find the first vote in the current vote state for a slot greater // than the new proposed root if current_vote.slot() <= new_root { + if timely_vote_credits || (current_vote.slot() != new_root) { + earned_credits = earned_credits + .checked_add(vote_state.credits_for_vote_at_index(current_vote_state_index)) + .expect("`earned_credits` does not overflow"); + } current_vote_state_index = current_vote_state_index .checked_add(1) .expect("`current_vote_state_index` is bounded by `MAX_LOCKOUT_HISTORY` when processing new root"); - if current_vote.slot() != new_root { - finalized_slot_count = finalized_slot_count - .checked_add(1) - .expect("`finalized_slot_count` is bounded by `MAX_LOCKOUT_HISTORY` when processing new root"); - } continue; } @@ -637,13 +638,30 @@ pub fn process_new_vote_state( } } + // For any slots newly added to the new vote state, the vote latency of that slot is not provided by the + // VoteStateUpdate instruction contents, but instead is computed from the actual latency of the VoteStateUpdate + // instruction. This prevents other validators from manipulating their own vote latencies within their vote states + // and forcing the rest of the cluster to accept these possibly fraudulent latency values. If the + // timly_vote_credits feature is not enabled then vote latency is set to 0 for new votes. + // + // For any slot that is in both the new state and the current state, the vote latency of the new state is taken + // from the current state. + // + // Thus vote latencies are set here for any newly vote-on slots when a VoteStateUpdate instruction is received. + // They are copied into the new vote state after every VoteStateUpdate for already voted-on slots. + // And when voted-on slots are rooted, the vote latencies stored in the vote state of all the rooted slots is used + // to compute credits earned. + // All validators compute the same vote latencies because all process the same VoteStateUpdate instruction at the + // same slot, and the only time vote latencies are ever computed is at the time that their slot is first voted on; + // after that, the latencies are retained unaltered until the slot is rooted. + // All the votes in our current vote state that are missing from the new vote state // must have been expired by later votes. Check that the lockouts match this assumption. while current_vote_state_index < vote_state.votes.len() && new_vote_state_index < new_state.len() { let current_vote = &vote_state.votes[current_vote_state_index]; - let new_vote = &new_state[new_vote_state_index]; + let new_vote = &mut new_state[new_vote_state_index]; // If the current slot is less than the new proposed slot, then the // new slot must have popped off the old slot, so check that the @@ -664,6 +682,9 @@ pub fn process_new_vote_state( return Err(VoteError::ConfirmationRollBack); } + // Copy the vote slot latency in from the current state to the new state + new_vote.latency = vote_state.votes[current_vote_state_index].latency; + current_vote_state_index = current_vote_state_index .checked_add(1) .expect("`current_vote_state_index` is bounded by `MAX_LOCKOUT_HISTORY` when slot is equal to proposed"); @@ -681,21 +702,32 @@ pub fn process_new_vote_state( // `new_vote_state` passed all the checks, finalize the change by rewriting // our state. + + // Now set the vote latencies on new slots not in the current state. New slots not in the current vote state will + // have had their latency initialized to 0 by the above loop. Those will now be updated to their actual latency. + // If the timely_vote_credits feature is not enabled, then the latency is left as 0 for such slots, which will + // result in 1 credit per slot when credits are calculated at the time that the slot is rooted. + if timely_vote_credits { + for new_vote in new_state.iter_mut() { + if new_vote.latency == 0 { + new_vote.latency = VoteState::compute_vote_latency(new_vote.slot(), current_slot); + } + } + } + if vote_state.root_slot != new_root { // Award vote credits based on the number of slots that were voted on and have reached finality // For each finalized slot, there was one voted-on slot in the new vote state that was responsible for // finalizing it. Each of those votes is awarded 1 credit. - vote_state.increment_credits(epoch, finalized_slot_count); + vote_state.increment_credits(epoch, earned_credits); } if let Some(timestamp) = timestamp { let last_slot = new_state.back().unwrap().slot(); vote_state.process_timestamp(last_slot, timestamp)?; } vote_state.root_slot = new_root; - vote_state.votes = new_state - .into_iter() - .map(|lockout| lockout.into()) - .collect(); + vote_state.votes = new_state; + Ok(()) } @@ -705,11 +737,12 @@ pub fn process_vote_unfiltered( vote: &Vote, slot_hashes: &[SlotHash], epoch: Epoch, + current_slot: Slot, ) -> 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)); + .for_each(|s| vote_state.process_next_vote_slot(*s, epoch, current_slot)); Ok(()) } @@ -718,6 +751,7 @@ pub fn process_vote( vote: &Vote, slot_hashes: &[SlotHash], epoch: Epoch, + current_slot: Slot, ) -> Result<(), VoteError> { if vote.slots.is_empty() { return Err(VoteError::EmptySlots); @@ -732,7 +766,14 @@ pub fn process_vote( if vote_slots.is_empty() { return Err(VoteError::VotesTooOldAllFiltered); } - process_vote_unfiltered(vote_state, &vote_slots, vote, slot_hashes, epoch) + process_vote_unfiltered( + vote_state, + &vote_slots, + vote, + slot_hashes, + epoch, + current_slot, + ) } /// "unchecked" functions used by tests and Tower @@ -747,6 +788,7 @@ pub fn process_vote_unchecked(vote_state: &mut VoteState, vote: Vote) -> Result< &vote, &slot_hashes, vote_state.current_epoch(), + 0, ) } @@ -994,7 +1036,7 @@ pub fn process_vote_with_account( ) -> Result<(), InstructionError> { let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?; - process_vote(&mut vote_state, vote, slot_hashes, clock.epoch)?; + process_vote(&mut vote_state, vote, slot_hashes, clock.epoch, clock.slot)?; if let Some(timestamp) = vote.timestamp { vote.slots .iter() @@ -1018,6 +1060,7 @@ pub fn process_vote_state_update( &mut vote_state, slot_hashes, clock.epoch, + clock.slot, vote_state_update, Some(feature_set), )?; @@ -1028,16 +1071,22 @@ pub fn do_process_vote_state_update( vote_state: &mut VoteState, slot_hashes: &[SlotHash], epoch: u64, + slot: u64, mut vote_state_update: VoteStateUpdate, feature_set: Option<&FeatureSet>, ) -> Result<(), VoteError> { check_update_vote_state_slots_are_valid(vote_state, &mut vote_state_update, slot_hashes)?; process_new_vote_state( vote_state, - vote_state_update.lockouts, + vote_state_update + .lockouts + .iter() + .map(|lockout| LandedVote::from(*lockout)) + .collect(), vote_state_update.root, vote_state_update.timestamp, epoch, + slot, feature_set, ) } @@ -1169,7 +1218,7 @@ mod tests { 134, 135, ] .into_iter() - .for_each(|v| vote_state.process_next_vote_slot(v, 4)); + .for_each(|v| vote_state.process_next_vote_slot(v, 4, 0)); let version1_14_11_serialized = bincode::serialize(&VoteStateVersions::V1_14_11(Box::new( VoteState1_14_11::from(vote_state.clone()), @@ -1461,11 +1510,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), + process_vote(&mut vote_state_a, &vote, &slot_hashes, 0, 0), Ok(()) ); assert_eq!( - process_vote(&mut vote_state_b, &vote, &slot_hashes, 0), + process_vote(&mut vote_state_b, &vote, &slot_hashes, 0, 0), Ok(()) ); assert_eq!(recent_votes(&vote_state_a), recent_votes(&vote_state_b)); @@ -1478,12 +1527,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), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0), Ok(()) ); let recent = recent_votes(&vote_state); assert_eq!( - process_vote(&mut vote_state, &vote, &slot_hashes, 0), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0), Err(VoteError::VoteTooOld) ); assert_eq!(recent, recent_votes(&vote_state)); @@ -1543,7 +1592,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), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0), Ok(()) ); assert_eq!( @@ -1559,7 +1608,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), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0), Ok(()) ); @@ -1578,7 +1627,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), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0), Ok(()) ); @@ -1595,14 +1644,14 @@ mod tests { let vote = Vote::new(vec![], Hash::default()); assert_eq!( - process_vote(&mut vote_state, &vote, &[], 0), + process_vote(&mut vote_state, &vote, &[], 0, 0), Err(VoteError::EmptySlots) ); } - pub fn process_new_vote_state_from_votes( + pub fn process_new_vote_state_from_lockouts( vote_state: &mut VoteState, - new_state: VecDeque, + new_state: VecDeque, new_root: Option, timestamp: Option, epoch: Epoch, @@ -1610,10 +1659,11 @@ mod tests { ) -> Result<(), VoteError> { process_new_vote_state( vote_state, - new_state.into_iter().map(|vote| vote.lockout).collect(), + new_state.into_iter().map(LandedVote::from).collect(), new_root, timestamp, epoch, + 0, feature_set, ) } @@ -1680,12 +1730,13 @@ mod tests { // Now use the resulting new vote state to perform a vote state update on vote_state assert_eq!( - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state, vote_state_after_vote.votes, vote_state_after_vote.root_slot, None, 0, + 0, Some(&feature_set) ), Ok(()) @@ -1699,6 +1750,304 @@ mod tests { } } + // Test vote credit updates after "timely vote credits" feature is enabled + #[test] + fn test_timely_credits() { + // Each of the following (Vec, Slot, u32) tuples gives a set of slots to cast votes on, a slot in which + // the vote was cast, and the number of credits that should have been earned by the vote account after this + // and all prior votes were cast. + let test_vote_groups: Vec<(Vec, Slot, u32)> = vec![ + // Initial set of votes that don't dequeue any slots, so no credits earned + ( + vec![1, 2, 3, 4, 5, 6, 7, 8], + 9, + // root: none, no credits earned + 0, + ), + ( + vec![ + 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, + 29, 30, 31, + ], + 34, + // lockouts full + // root: none, no credits earned + 0, + ), + // Now a single vote which should result in the first root and first credit earned + ( + vec![32], + 35, + // root: 1 + // when slot 1 was voted on in slot 9, it earned 2 credits + 2, + ), + // Now another vote, should earn one credit + ( + vec![33], + 36, + // root: 2 + // when slot 2 was voted on in slot 9, it earned 3 credits + 2 + 3, // 5 + ), + // Two votes in sequence + ( + vec![34, 35], + 37, + // root: 4 + // when slots 3 and 4 were voted on in slot 9, they earned 4 and 5 credits + 5 + 4 + 5, // 14 + ), + // 3 votes in sequence + ( + vec![36, 37, 38], + 39, + // root: 7 + // slots 5, 6, and 7 earned 6, 7, and 8 credits when voted in slot 9 + 14 + 6 + 7 + 8, // 35 + ), + ( + // 30 votes in sequence + vec![ + 39, 40, 41, 42, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, + 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, + ], + 69, + // root: 37 + // slot 8 was voted in slot 9, earning 8 credits + // slots 9 - 25 earned 1 credit when voted in slot 34 + // slot 26, 27, 28, 29, 30, 31 earned 2, 3, 4, 5, 6, 7 credits when voted in slot 34 + // slot 32 earned 7 credits when voted in slot 35 + // slot 33 earned 7 credits when voted in slot 36 + // slot 34 and 35 earned 7 and 8 credits when voted in slot 37 + // slot 36 and 37 earned 7 and 8 credits when voted in slot 39 + 35 + 8 + ((25 - 9) + 1) + 2 + 3 + 4 + 5 + 6 + 7 + 7 + 7 + 7 + 8 + 7 + 8, // 131 + ), + // 31 votes in sequence + ( + vec![ + 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, + 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, + ], + 100, + // root: 68 + // slot 38 earned 8 credits when voted in slot 39 + // slot 39 - 60 earned 1 credit each when voted in slot 69 + // slot 61, 62, 63, 64, 65, 66, 67, 68 earned 2, 3, 4, 5, 6, 7, 8, and 8 credits when + // voted in slot 69 + 131 + 8 + ((60 - 39) + 1) + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 8, // 204 + ), + // Votes with expiry + ( + vec![115, 116, 117, 118, 119, 120, 121, 122, 123, 124], + 130, + // root: 74 + // slots 96 - 114 expire + // slots 69 - 74 earned 1 credit when voted in slot 100 + 204 + ((74 - 69) + 1), // 210 + ), + // More votes with expiry of a large number of votes + ( + vec![200, 201], + 202, + // root: 74 + // slots 119 - 124 expire + 210, + ), + ( + vec![ + 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, + 218, 219, 220, 221, 222, 223, 224, 225, 226, + ], + 227, + // root: 95 + // slot 75 - 91 earned 1 credit each when voted in slot 100 + // slot 92, 93, 94, 95 earned 2, 3, 4, 5, credits when voted in slot 100 + 210 + ((91 - 75) + 1) + 2 + 3 + 4 + 5, // 241 + ), + ( + vec![227, 228, 229, 230, 231, 232, 233, 234, 235, 236], + 237, + // root: 205 + // slot 115 - 118 earned 1 credit when voted in slot 130 + // slot 200 and 201 earned 8 credits when voted in slot 202 + // slots 202 - 205 earned 1 credit when voted in slot 227 + 241 + 1 + 1 + 1 + 1 + 8 + 8 + 1 + 1 + 1 + 1, // 265 + ), + ]; + + let mut feature_set = FeatureSet::default(); + feature_set.activate(&feature_set::timely_vote_credits::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 + for i in 0..test_vote_groups.len() { + // Create a new VoteState for vote transaction + let mut vote_state_1 = VoteState::new(&VoteInit::default(), &Clock::default()); + // Create a new VoteState for vote state update transaction + let mut vote_state_2 = VoteState::new(&VoteInit::default(), &Clock::default()); + test_vote_groups.iter().take(i + 1).for_each(|vote_group| { + let vote = Vote { + slots: vote_group.0.clone(), //vote_group.0 is the set of slots to cast votes on + hash: Hash::new_unique(), + timestamp: None, + }; + let slot_hashes: Vec<_> = + vote.slots.iter().rev().map(|x| (*x, vote.hash)).collect(); + assert_eq!( + process_vote( + &mut vote_state_1, + &vote, + &slot_hashes, + 0, + vote_group.1 // vote_group.1 is the slot in which the vote was cast + ), + Ok(()) + ); + + assert_eq!( + process_new_vote_state( + &mut vote_state_2, + vote_state_1.votes.clone(), + vote_state_1.root_slot, + None, + 0, + vote_group.1, // vote_group.1 is the slot in which the vote was cast + Some(&feature_set) + ), + Ok(()) + ); + }); + + // Ensure that the credits earned is correct for both vote states + let vote_group = &test_vote_groups[i]; + assert_eq!(vote_state_1.credits(), vote_group.2 as u64); // vote_group.2 is the expected number of credits + assert_eq!(vote_state_2.credits(), vote_group.2 as u64); // vote_group.2 is the expected number of credits + } + } + + #[test] + fn test_retroactive_voting_timely_credits() { + // Each of the following (Vec<(Slot, int)>, Slot, Option, u32) tuples gives the following data: + // Vec<(Slot, int)> -- the set of slots and confirmation_counts that is the VoteStateUpdate + // Slot -- the slot in which the VoteStateUpdate occurred + // Option -- the root after processing the VoteStateUpdate + // u32 -- the credits after processing the VoteStateUpdate + #[allow(clippy::type_complexity)] + let test_vote_state_updates: Vec<(Vec<(Slot, u32)>, Slot, Option, u32)> = vec![ + // VoteStateUpdate to set initial vote state + ( + vec![(7, 4), (8, 3), (9, 2), (10, 1)], + 11, + // root: none + None, + // no credits earned + 0, + ), + // VoteStateUpdate to include the missing slots *prior to previously included slots* + ( + vec![ + (1, 10), + (2, 9), + (3, 8), + (4, 7), + (5, 6), + (6, 5), + (7, 4), + (8, 3), + (9, 2), + (10, 1), + ], + 12, + // root: none + None, + // no credits earned + 0, + ), + // Now a single VoteStateUpdate which roots all of the slots from 1 - 10 + ( + vec![ + (11, 31), + (12, 30), + (13, 29), + (14, 28), + (15, 27), + (16, 26), + (17, 25), + (18, 24), + (19, 23), + (20, 22), + (21, 21), + (22, 20), + (23, 19), + (24, 18), + (25, 17), + (26, 16), + (27, 15), + (28, 14), + (29, 13), + (30, 12), + (31, 11), + (32, 10), + (33, 9), + (34, 8), + (35, 7), + (36, 6), + (37, 5), + (38, 4), + (39, 3), + (40, 2), + (41, 1), + ], + 42, + // root: 10 + Some(10), + // when slots 1 - 6 were voted on in slot 12, they earned 1, 1, 1, 2, 3, and 4 credits + // when slots 7 - 10 were voted on in slot 11, they earned 6, 7, 8, and 8 credits + 1 + 1 + 1 + 2 + 3 + 4 + 6 + 7 + 8 + 8, + ), + ]; + + let mut feature_set = FeatureSet::default(); + feature_set.activate(&feature_set::timely_vote_credits::id(), 1); + + // Retroactive voting is only possible with VoteStateUpdate transactions, which is why Vote transactions are + // not tested here + + // Initial vote state + let mut vote_state = VoteState::new(&VoteInit::default(), &Clock::default()); + + // Process the vote state updates in sequence and ensure that the credits earned after each is processed is + // correct + test_vote_state_updates + .iter() + .for_each(|vote_state_update| { + let new_state = vote_state_update + .0 // vote_state_update.0 is the set of slots and confirmation_counts that is the VoteStateUpdate + .iter() + .map(|(slot, confirmation_count)| LandedVote { + latency: 0, + lockout: Lockout::new_with_confirmation_count(*slot, *confirmation_count), + }) + .collect::>(); + assert_eq!( + process_new_vote_state( + &mut vote_state, + new_state, + vote_state_update.2, // vote_state_update.2 is root after processing the VoteStateUpdate + None, + 0, + vote_state_update.1, // vote_state_update.1 is the slot in which the VoteStateUpdate occurred + Some(&feature_set) + ), + Ok(()) + ); + + // Ensure that the credits earned is correct + assert_eq!(vote_state.credits(), vote_state_update.3 as u64); + }); + } + #[test] fn test_process_new_vote_too_many_votes() { let mut vote_state1 = VoteState::default(); @@ -1713,7 +2062,14 @@ mod tests { let current_epoch = vote_state1.current_epoch(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::TooManyVotes) ); } @@ -1736,12 +2092,13 @@ mod tests { let current_epoch = vote_state2.current_epoch(); assert_eq!( - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), lesser_root, None, current_epoch, + 0, None, ), Err(VoteError::RootRollBack) @@ -1750,18 +2107,120 @@ mod tests { // Trying to set root to None should error let none_root = None; assert_eq!( - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), none_root, None, current_epoch, + 0, None, ), Err(VoteError::RootRollBack) ); } + fn process_new_vote_state_replaced_root_vote_credits( + feature_set: &FeatureSet, + expected_credits: u64, + ) { + let mut vote_state1 = VoteState::default(); + + // Initial vote state: as if 31 votes had occurred on slots 0 - 30 (inclusive) + assert_eq!( + process_new_vote_state_from_lockouts( + &mut vote_state1, + (0..MAX_LOCKOUT_HISTORY) + .enumerate() + .map(|(index, slot)| Lockout::new_with_confirmation_count( + slot as Slot, + (MAX_LOCKOUT_HISTORY.checked_sub(index).unwrap()) as u32 + )) + .collect(), + None, + None, + 0, + Some(feature_set), + ), + Ok(()) + ); + + // Now vote as if new votes on slots 31 and 32 had occurred, yielding a new Root of 1 + assert_eq!( + process_new_vote_state_from_lockouts( + &mut vote_state1, + (2..(MAX_LOCKOUT_HISTORY.checked_add(2).unwrap())) + .enumerate() + .map(|(index, slot)| Lockout::new_with_confirmation_count( + slot as Slot, + (MAX_LOCKOUT_HISTORY.checked_sub(index).unwrap()) as u32 + )) + .collect(), + Some(1), + None, + 0, + Some(feature_set), + ), + Ok(()) + ); + + // Vote credits should be 2, since two voted-on slots were "popped off the back" of the tower + assert_eq!(vote_state1.credits(), 2); + + // Create a new vote state that represents the validator having not voted for a long time, then voting on + // slots 10001 through 10032 (inclusive) with an entirely new root of 10000 that was never previously voted + // on. This is valid because a vote state can include a root that it never voted on (if it votes after a very + // long delinquency, the new votes will have a root much newer than its most recently voted slot). + assert_eq!( + process_new_vote_state_from_lockouts( + &mut vote_state1, + (10001..(MAX_LOCKOUT_HISTORY.checked_add(10001).unwrap())) + .enumerate() + .map(|(index, slot)| Lockout::new_with_confirmation_count( + slot as Slot, + (MAX_LOCKOUT_HISTORY.checked_sub(index).unwrap()) as u32 + )) + .collect(), + Some(10000), + None, + 0, + Some(feature_set), + ), + Ok(()) + ); + + // The vote is valid, but no vote credits should be awarded because although there is a new root, it does not + // represent a slot previously voted on. + assert_eq!(vote_state1.credits(), expected_credits) + } + + #[test] + fn test_process_new_vote_state_replaced_root_vote_credits() { + let mut feature_set = FeatureSet::default(); + + // Always use allow_votes_to_directly_update_vote_state feature because VoteStateUpdate is being tested + feature_set.activate( + &feature_set::allow_votes_to_directly_update_vote_state::id(), + 1, + ); + + // Test without the timely_vote_credits feature. The expected credits here of 34 is *incorrect* but is what + // is expected using vote_state_update_credit_per_dequeue. With this feature, the credits earned will be + // calculated as: + // 2 (from initial vote state) + // + 31 (for votes which were "popped off of the back of the tower" by the new vote + // + 1 (just because there is a new root, even though it was never voted on -- this is the flaw) + feature_set.activate(&feature_set::vote_state_update_credit_per_dequeue::id(), 1); + process_new_vote_state_replaced_root_vote_credits(&feature_set, 34); + + // Now test using the timely_vote_credits feature. The expected credits here of 33 is *correct*. With + // this feature, the credits earned will be calculated as: + // 2 (from initial vote state) + // + 31 (for votes which were "popped off of the back of the tower" by the new vote) + feature_set.activate(&feature_set::timely_vote_credits::id(), 1); + process_new_vote_state_replaced_root_vote_credits(&feature_set, 33); + } + #[test] fn test_process_new_vote_state_zero_confirmations() { let mut vote_state1 = VoteState::default(); @@ -1774,7 +2233,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::ZeroConfirmations) ); @@ -1785,7 +2251,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::ZeroConfirmations) ); } @@ -1802,7 +2275,7 @@ mod tests { .into_iter() .collect(); - process_new_vote_state( + process_new_vote_state_from_lockouts( &mut vote_state1, good_votes, None, @@ -1820,7 +2293,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::ConfirmationTooLarge) ); } @@ -1838,7 +2318,7 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state( + process_new_vote_state_from_lockouts( &mut vote_state1, bad_votes, Some(root_slot), @@ -1856,7 +2336,7 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state( + process_new_vote_state_from_lockouts( &mut vote_state1, bad_votes, Some(root_slot), @@ -1880,7 +2360,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::SlotsNotOrdered) ); @@ -1891,7 +2378,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::SlotsNotOrdered) ); } @@ -1908,7 +2402,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::ConfirmationsNotOrdered) ); @@ -1919,7 +2420,14 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::ConfirmationsNotOrdered) ); } @@ -1938,7 +2446,14 @@ mod tests { // Slot 7 should have expired slot 0 assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + None, + None, + current_epoch, + None + ), Err(VoteError::NewVoteStateLockoutMismatch) ); } @@ -1953,7 +2468,15 @@ mod tests { ] .into_iter() .collect(); - process_new_vote_state(&mut vote_state1, votes, None, None, current_epoch, None).unwrap(); + process_new_vote_state_from_lockouts( + &mut vote_state1, + votes, + None, + None, + current_epoch, + None, + ) + .unwrap(); let votes: VecDeque = vec![ Lockout::new_with_confirmation_count(0, 4), @@ -1966,7 +2489,14 @@ mod tests { // Should error because newer vote state should not have lower confirmation the same slot // 1 assert_eq!( - process_new_vote_state(&mut vote_state1, votes, None, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + votes, + None, + None, + current_epoch, + None + ), Err(VoteError::ConfirmationRollBack) ); } @@ -1991,12 +2521,13 @@ mod tests { process_slot_vote_unchecked(&mut vote_state2, new_vote as Slot); assert_ne!(vote_state1.root_slot, vote_state2.root_slot); - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), vote_state2.root_slot, None, vote_state2.current_epoch(), + 0, None, ) .unwrap(); @@ -2049,12 +2580,13 @@ mod tests { ); // See that on-chain vote state can update properly - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), vote_state2.root_slot, None, vote_state2.current_epoch(), + 0, None, ) .unwrap(); @@ -2091,12 +2623,13 @@ mod tests { // See that on-chain vote state can update properly assert_eq!( - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), vote_state2.root_slot, None, vote_state2.current_epoch(), + 0, None ), Err(VoteError::LockoutConflict) @@ -2133,12 +2666,13 @@ mod tests { // Both vote states contain `5`, but `5` is not part of the common prefix // of both vote states. However, the violation should still be detected. assert_eq!( - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), vote_state2.root_slot, None, vote_state2.current_epoch(), + 0, None ), Err(VoteError::LockoutConflict) @@ -2178,12 +2712,13 @@ mod tests { ); // Should be able to update vote_state1 - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, vote_state2.votes.clone(), vote_state2.root_slot, None, vote_state2.current_epoch(), + 0, None, ) .unwrap(); @@ -2215,7 +2750,14 @@ mod tests { let current_epoch = vote_state1.current_epoch(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, root, None, current_epoch, None), + process_new_vote_state_from_lockouts( + &mut vote_state1, + bad_votes, + root, + None, + current_epoch, + None + ), Err(VoteError::LockoutConflict) ); @@ -2227,12 +2769,13 @@ mod tests { .collect(); let current_epoch = vote_state1.current_epoch(); - process_new_vote_state_from_votes( + process_new_vote_state( &mut vote_state1, good_votes.clone(), root, None, current_epoch, + 0, None, ) .unwrap(); @@ -2249,7 +2792,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), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0), Err(VoteError::VotesTooOldAllFiltered) ); @@ -2263,7 +2806,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).unwrap(); + process_vote(&mut vote_state, &vote, &slot_hashes, 0, 0).unwrap(); assert_eq!( vote_state .votes @@ -2292,7 +2835,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).unwrap(); + process_vote_unfiltered(&mut vote_state, &vote.slots, &vote, slot_hashes, 0, 0) + .unwrap(); } vote_state @@ -2415,6 +2959,7 @@ mod tests { &mut vote_state, &slot_hashes, 0, + 0, vote_state_update.clone(), Some(&FeatureSet::all_enabled()), ) @@ -2634,6 +3179,7 @@ mod tests { &mut vote_state, &slot_hashes, 0, + 0, vote_state_update, Some(&FeatureSet::all_enabled()), ) @@ -2680,6 +3226,7 @@ mod tests { &mut vote_state, &slot_hashes, 0, + 0, vote_state_update, Some(&FeatureSet::all_enabled()), ) @@ -2739,6 +3286,7 @@ mod tests { &mut vote_state, &slot_hashes, 0, + 0, vote_state_update, Some(&FeatureSet::all_enabled()), ) @@ -2896,6 +3444,7 @@ mod tests { &mut vote_state, &slot_hashes, 0, + 0, vote_state_update, Some(&FeatureSet::all_enabled()), ) @@ -2944,6 +3493,7 @@ mod tests { &mut vote_state, &slot_hashes, 0, + 0, vote_state_update, Some(&FeatureSet::all_enabled()) ), diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index ff70bdee116263..997102b3e6564c 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -7274,12 +7274,16 @@ pub mod tests { .unwrap(); assert_ne!(leader_info.activated_stake, 0); // Subtract one because the last vote always carries over to the next epoch - let expected_credits = TEST_SLOTS_PER_EPOCH - MAX_LOCKOUT_HISTORY as u64 - 1; + // Each slot earned maximum credits + let credits_per_slot = + solana_vote_program::vote_state::VOTE_CREDITS_MAXIMUM_PER_SLOT as u64; + let expected_credits = + (TEST_SLOTS_PER_EPOCH - MAX_LOCKOUT_HISTORY as u64 - 1) * credits_per_slot; assert_eq!( leader_info.epoch_credits, vec![ (0, expected_credits, 0), - (1, expected_credits + 1, expected_credits) // one vote in current epoch + (1, expected_credits + credits_per_slot, expected_credits) // one vote in current epoch ] ); diff --git a/sdk/program/src/vote/state/mod.rs b/sdk/program/src/vote/state/mod.rs index 5d7e14a70cddfc..6d77d3ab5d9dda 100644 --- a/sdk/program/src/vote/state/mod.rs +++ b/sdk/program/src/vote/state/mod.rs @@ -35,6 +35,12 @@ pub const MAX_EPOCH_CREDITS_HISTORY: usize = 64; // Offset of VoteState::prior_voters, for determining initialization status without deserialization const DEFAULT_PRIOR_VOTERS_OFFSET: usize = 114; +// Number of slots of grace period for which maximum vote credits are awarded - votes landing within this number of slots of the slot that is being voted on are awarded full credits. +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 = 8; + #[frozen_abi(digest = "Ch2vVEwos2EjAVqSHCyJjnN2MNX1yrpapZTGhMSCjWUH")] #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone, AbiExample)] pub struct Vote { @@ -419,7 +425,12 @@ impl VoteState { } } - pub fn process_next_vote_slot(&mut self, next_vote_slot: Slot, epoch: Epoch) { + pub fn process_next_vote_slot( + &mut self, + next_vote_slot: Slot, + epoch: Epoch, + current_slot: Slot, + ) { // Ignore votes for slots earlier than we already have votes for if self .last_voted_slot() @@ -428,18 +439,22 @@ impl VoteState { return; } - let lockout = Lockout::new(next_vote_slot); - self.pop_expired_votes(next_vote_slot); + let landed_vote = LandedVote { + latency: Self::compute_vote_latency(next_vote_slot, current_slot), + lockout: Lockout::new(next_vote_slot), + }; + // Once the stack is full, pop the oldest lockout and distribute rewards if self.votes.len() == MAX_LOCKOUT_HISTORY { - let vote = self.votes.pop_front().unwrap(); - self.root_slot = Some(vote.slot()); + let credits = self.credits_for_vote_at_index(0); + let landed_vote = self.votes.pop_front().unwrap(); + self.root_slot = Some(landed_vote.slot()); - self.increment_credits(epoch, 1); + self.increment_credits(epoch, credits); } - self.votes.push_back(lockout.into()); + self.votes.push_back(landed_vote); self.double_lockouts(); } @@ -472,6 +487,43 @@ impl VoteState { self.epoch_credits.last().unwrap().1.saturating_add(credits); } + // Computes the vote latency for vote on voted_for_slot where the vote itself landed in current_slot + pub fn compute_vote_latency(voted_for_slot: Slot, current_slot: Slot) -> u8 { + std::cmp::min(current_slot.saturating_sub(voted_for_slot), u8::MAX as u64) as u8 + } + + /// Returns the credits to award for a vote at the given lockout slot index + pub fn credits_for_vote_at_index(&self, index: usize) -> u64 { + let latency = self + .votes + .get(index) + .map_or(0, |landed_vote| landed_vote.latency); + + // 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 { + 1 + } else { + match latency.checked_sub(VOTE_CREDITS_GRACE_SLOTS) { + None | Some(0) => { + // latency was <= VOTE_CREDITS_GRACE_SLOTS, so maximum credits are awarded + 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 VOTE_CREDITS_MAXIMUM_PER_SLOT.checked_sub(diff) { + // If diff >= VOTE_CREDITS_MAXIMUM_PER_SLOT, 1 credit is awarded + None | Some(0) => 1, + + Some(credits) => credits as u64, + } + } + } + } + } + pub fn nth_recent_lockout(&self, position: usize) -> Option<&Lockout> { if position < self.votes.len() { let pos = self diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 386a6969d2fd3c..2ce873fe5f3d91 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -687,6 +687,10 @@ pub mod enable_poseidon_syscall { solana_sdk::declare_id!("FL9RsQA6TVUoh5xJQ9d936RHSebA1NLQqe3Zv9sXZRpr"); } +pub mod timely_vote_credits { + solana_sdk::declare_id!("2oXpeh141pPZCTCFHBsvCwG2BtaHZZAtrVhwaxSy6brS"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -851,6 +855,7 @@ lazy_static! { (reduce_stake_warmup_cooldown::id(), "reduce stake warmup cooldown from 25% to 9%"), (revise_turbine_epoch_stakes::id(), "revise turbine epoch stakes"), (enable_poseidon_syscall::id(), "Enable Poseidon syscall"), + (timely_vote_credits::id(), "use timeliness of votes in determining credits to award"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()