vote: Prevent commission update in the second half of epochs (#29362)

* vote: Prevent commission update in the second half of epochs

* Address feedback

* Fix tests

* Make the feature enabled by single-contributor

* Use a cooler pubkey

Cargo.lock

@@ -6899,6 +6899,7 @@ dependencies = [
  "solana-program 1.15.0",
  "solana-program-runtime",
  "solana-sdk 1.15.0",
+ "test-case",
  "thiserror",
 ]
 

programs/vote/Cargo.toml

@@ -26,6 +26,7 @@ thiserror = "1.0"
 
 [dev-dependencies]
 solana-logger = { path = "../../logger", version = "=1.15.0" }
+test-case = "2.2.2"
 
 [build-dependencies]
 rustc_version = "0.4"

programs/vote/src/vote_processor.rs

@@ -1,7 +1,7 @@
 //! Vote program processor
 
 use {
-    crate::vote_state,
+    crate::{vote_error::VoteError, vote_state},
     log::*,
     solana_program::vote::{instruction::VoteInstruction, program::id, state::VoteAuthorize},
     solana_program_runtime::{
@@ -135,6 +135,16 @@ pub fn process_instruction(
             vote_state::update_validator_identity(&mut me, node_pubkey, &signers)
         }
         VoteInstruction::UpdateCommission(commission) => {
+            if invoke_context.feature_set.is_active(
+                &feature_set::commission_updates_only_allowed_in_first_half_of_epoch::id(),
+            ) {
+                let sysvar_cache = invoke_context.get_sysvar_cache();
+                let epoch_schedule = sysvar_cache.get_epoch_schedule()?;
+                let clock = sysvar_cache.get_clock()?;
+                if !vote_state::is_commission_update_allowed(clock.slot, &epoch_schedule) {
+                    return Err(VoteError::CommissionUpdateTooLate.into());
+                }
+            }
             vote_state::update_commission(&mut me, commission, &signers)
         }
         VoteInstruction::Vote(vote) | VoteInstruction::VoteSwitch(vote, _) => {
@@ -276,7 +286,10 @@ mod tests {
             hash::Hash,
             instruction::{AccountMeta, Instruction},
             pubkey::Pubkey,
-            sysvar::{self, clock::Clock, rent::Rent, slot_hashes::SlotHashes},
+            sysvar::{
+                self, clock::Clock, epoch_schedule::EpochSchedule, rent::Rent,
+                slot_hashes::SlotHashes,
+            },
         },
         std::{collections::HashSet, str::FromStr},
     };
@@ -344,6 +357,7 @@ mod tests {
             .map(|meta| meta.pubkey)
             .collect();
         pubkeys.insert(sysvar::clock::id());
+        pubkeys.insert(sysvar::epoch_schedule::id());
         pubkeys.insert(sysvar::rent::id());
         pubkeys.insert(sysvar::slot_hashes::id());
         let transaction_accounts: Vec<_> = pubkeys
@@ -353,6 +367,10 @@ mod tests {
                     *pubkey,
                     if sysvar::clock::check_id(pubkey) {
                         account::create_account_shared_data_for_test(&Clock::default())
+                    } else if sysvar::epoch_schedule::check_id(pubkey) {
+                        account::create_account_shared_data_for_test(
+                            &EpochSchedule::without_warmup(),
+                        )
                     } else if sysvar::slot_hashes::check_id(pubkey) {
                         account::create_account_shared_data_for_test(&SlotHashes::default())
                     } else if sysvar::rent::check_id(pubkey) {
@@ -667,6 +685,15 @@ mod tests {
         let transaction_accounts = vec![
             (vote_pubkey, vote_account),
             (authorized_withdrawer, AccountSharedData::default()),
+            // Add the sysvar accounts so they're in the cache for mock processing
+            (
+                sysvar::clock::id(),
+                account::create_account_shared_data_for_test(&Clock::default()),
+            ),
+            (
+                sysvar::epoch_schedule::id(),
+                account::create_account_shared_data_for_test(&EpochSchedule::without_warmup()),
+            ),
         ];
         let mut instruction_accounts = vec![
             AccountMeta {

programs/vote/src/vote_state/mod.rs

@@ -9,6 +9,7 @@ use {
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount, WritableAccount},
         clock::{Epoch, Slot, UnixTimestamp},
+        epoch_schedule::EpochSchedule,
         feature_set::{self, filter_votes_outside_slot_hashes, FeatureSet},
         hash::Hash,
         instruction::InstructionError,
@@ -797,6 +798,22 @@ pub fn update_commission<S: std::hash::BuildHasher>(
     vote_account.set_state(&VoteStateVersions::new_current(vote_state))
 }
 
+/// Given the current slot and epoch schedule, determine if a commission change
+/// is allowed
+pub fn is_commission_update_allowed(slot: Slot, epoch_schedule: &EpochSchedule) -> bool {
+    // always allowed during warmup epochs
+    if let Some(relative_slot) = slot
+        .saturating_sub(epoch_schedule.first_normal_slot)
+        .checked_rem(epoch_schedule.slots_per_epoch)
+    {
+        // allowed up to the midpoint of the epoch
+        relative_slot.saturating_mul(2) <= epoch_schedule.slots_per_epoch
+    } else {
+        // no slots per epoch, just allow it, even though this should never happen
+        true
+    }
+}
+
 fn verify_authorized_signer<S: std::hash::BuildHasher>(
     authorized: &Pubkey,
     signers: &HashSet<Pubkey, S>,
@@ -1020,8 +1037,12 @@ mod tests {
     use {
         super::*,
         crate::vote_state,
-        solana_sdk::{account::AccountSharedData, account_utils::StateMut, hash::hash},
+        solana_sdk::{
+            account::AccountSharedData, account_utils::StateMut, clock::DEFAULT_SLOTS_PER_EPOCH,
+            hash::hash,
+        },
         std::cell::RefCell,
+        test_case::test_case,
     };
 
     const MAX_RECENT_VOTES: usize = 16;
@@ -2955,4 +2976,45 @@ mod tests {
             Err(VoteError::SlotHashMismatch),
         );
     }
+
+    #[test_case(0, true; "first slot")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH / 2, true; "halfway through epoch")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH / 2 + 1, false; "halfway through epoch plus one")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH - 1, false; "last slot in epoch")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH, true; "first slot in second epoch")]
+    fn test_epoch_half_check(slot: Slot, expected_allowed: bool) {
+        let epoch_schedule = EpochSchedule::without_warmup();
+        assert_eq!(
+            is_commission_update_allowed(slot, &epoch_schedule),
+            expected_allowed
+        );
+    }
+
+    #[test]
+    fn test_warmup_epoch_half_check_with_warmup() {
+        let epoch_schedule = EpochSchedule::default();
+        let first_normal_slot = epoch_schedule.first_normal_slot;
+        // first slot works
+        assert!(is_commission_update_allowed(0, &epoch_schedule));
+        // right before first normal slot works, since all warmup slots allow
+        // commission updates
+        assert!(is_commission_update_allowed(
+            first_normal_slot - 1,
+            &epoch_schedule
+        ));
+    }
+
+    #[test_case(0, true; "first slot")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH / 2, true; "halfway through epoch")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH / 2 + 1, false; "halfway through epoch plus one")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH - 1, false; "last slot in epoch")]
+    #[test_case(DEFAULT_SLOTS_PER_EPOCH, true; "first slot in second epoch")]
+    fn test_epoch_half_check_with_warmup(slot: Slot, expected_allowed: bool) {
+        let epoch_schedule = EpochSchedule::default();
+        let first_normal_slot = epoch_schedule.first_normal_slot;
+        assert_eq!(
+            is_commission_update_allowed(first_normal_slot + slot, &epoch_schedule),
+            expected_allowed
+        );
+    }
 }

sdk/program/src/vote/error.rs

@@ -66,6 +66,9 @@ pub enum VoteError {
 
     #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")]
     ActiveVoteAccountClose,
+
+    #[error("Cannot update commission at this point in the epoch")]
+    CommissionUpdateTooLate,
 }
 
 impl<E> DecodeError<E> for VoteError {

sdk/src/feature_set.rs

@@ -554,6 +554,10 @@ pub mod enable_program_redeployment_cooldown {
     solana_sdk::declare_id!("J4HFT8usBxpcF63y46t1upYobJgChmKyZPm5uTBRg25Z");
 }
 
+pub mod commission_updates_only_allowed_in_first_half_of_epoch {
+    solana_sdk::declare_id!("noRuG2kzACwgaY7TVmLRnUNPLKNVQE1fb7X55YWBehp");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -687,6 +691,7 @@ lazy_static! {
         (cap_transaction_accounts_data_size::id(), "cap transaction accounts data size up to its compute unit limits #27839"),
         (enable_alt_bn128_syscall::id(), "add alt_bn128 syscalls #27961"),
         (enable_program_redeployment_cooldown::id(), "enable program redeployment cooldown #29135"),
+        (commission_updates_only_allowed_in_first_half_of_epoch::id(), "validator commission updates are only allowed in the first half of an epoch #29362"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()