Add allow_commission_decrease_at_any_time feature which will allow

vote account commission to be lowered at any time during the epoch
regardless of the commission_updates_only_allowed_in_first_half_of_epoch
feature.  Fixes solana-labs#33843.

CHANGELOG.md

@@ -15,6 +15,9 @@ Release channels have their own copy of this changelog:
 ## [1.18.0] - Unreleased
 * Changes
   * Added a github check to support `changelog` label
+  * Added allow_commission_decrease_at_any_time feature which will allow commission on a vote account to be
+    decreased even in the second half of epochs when the commission_updates_only_allowed_in_first_half_of_epoch
+    feature would have prevented it
 * Upgrade Notes
 
 ## [1.17.0]

programs/vote/src/vote_processor.rs

@@ -1,7 +1,7 @@
 //! Vote program processor
 
 use {
-    crate::{vote_error::VoteError, vote_state},
+    crate::vote_state,
     log::*,
     solana_program::vote::{instruction::VoteInstruction, program::id, state::VoteAuthorize},
     solana_program_runtime::{
@@ -140,20 +140,14 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context|
             )
         }
         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());
-                }
-            }
+            let sysvar_cache = invoke_context.get_sysvar_cache();
+
             vote_state::update_commission(
                 &mut me,
                 commission,
                 &signers,
+                &sysvar_cache.get_epoch_schedule()?,
+                &sysvar_cache.get_clock()?,
                 &invoke_context.feature_set,
             )
         }

programs/vote/src/vote_state/mod.rs

@@ -877,8 +877,27 @@ pub fn update_commission<S: std::hash::BuildHasher>(
     vote_account: &mut BorrowedAccount,
     commission: u8,
     signers: &HashSet<Pubkey, S>,
+    epoch_schedule: &std::sync::Arc<EpochSchedule>,
+    clock: &std::sync::Arc<Clock>,
     feature_set: &FeatureSet,
 ) -> Result<(), InstructionError> {
+    let enforce_commission_update_rule =
+        if feature_set.is_active(&feature_set::allow_commission_decrease_at_any_time::id()) {
+            is_commission_increase(vote_account, commission)?
+        } else {
+            true
+        };
+
+    #[allow(clippy::collapsible_if)]
+    if enforce_commission_update_rule
+        && feature_set
+            .is_active(&feature_set::commission_updates_only_allowed_in_first_half_of_epoch::id())
+    {
+        if !is_commission_update_allowed(clock.slot, epoch_schedule) {
+            return Err(VoteError::CommissionUpdateTooLate.into());
+        }
+    }
+
     let mut vote_state: VoteState = vote_account
         .get_state::<VoteStateVersions>()?
         .convert_to_current();
@@ -891,6 +910,18 @@ pub fn update_commission<S: std::hash::BuildHasher>(
     set_vote_account_state(vote_account, vote_state, feature_set)
 }
 
+/// Given a proposed new commission, returns true if this would be a commission increase, false otherwise
+pub fn is_commission_increase(
+    vote_account: &BorrowedAccount,
+    commission: u8,
+) -> Result<bool, InstructionError> {
+    Ok(commission
+        > vote_account
+            .get_state::<VoteStateVersions>()?
+            .convert_to_current()
+            .commission)
+}
+
 /// 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 {
@@ -1365,6 +1396,192 @@ mod tests {
         assert_eq!(vote_state.votes.len(), 2);
     }
 
+    #[test]
+    fn test_update_commission() {
+        let node_pubkey = solana_sdk::pubkey::new_rand();
+        let withdrawer_pubkey = solana_sdk::pubkey::new_rand();
+        let clock = Clock::default();
+        let vote_state = VoteState::new(
+            &VoteInit {
+                node_pubkey,
+                authorized_voter: withdrawer_pubkey,
+                authorized_withdrawer: withdrawer_pubkey,
+                commission: 10,
+            },
+            &clock,
+        );
+
+        let serialized =
+            bincode::serialize(&VoteStateVersions::Current(Box::new(vote_state.clone()))).unwrap();
+        let serialized_len = serialized.len();
+        let rent = Rent::default();
+        let lamports = rent.minimum_balance(serialized_len);
+        let mut vote_account = AccountSharedData::new(lamports, serialized_len, &id());
+        vote_account.set_data_from_slice(&serialized);
+
+        // Create a fake TransactionContext with a fake InstructionContext with a single account which is the
+        // vote account that was just created
+        let processor_account = AccountSharedData::new(0, 0, &solana_sdk::native_loader::id());
+        let transaction_context = TransactionContext::new(
+            vec![(id(), processor_account), (node_pubkey, vote_account)],
+            rent,
+            0,
+            0,
+        );
+        let mut instruction_context = InstructionContext::default();
+        instruction_context.configure(
+            &[0],
+            &[InstructionAccount {
+                index_in_transaction: 1,
+                index_in_caller: 1,
+                index_in_callee: 0,
+                is_signer: false,
+                is_writable: true,
+            }],
+            &[],
+        );
+
+        // Get the BorrowedAccount from the InstructionContext which is what is used to manipulate and inspect account
+        // state
+        let mut borrowed_account = instruction_context
+            .try_borrow_instruction_account(&transaction_context, 0)
+            .unwrap();
+
+        let epoch_schedule = std::sync::Arc::new(EpochSchedule::without_warmup());
+
+        let first_half_clock = std::sync::Arc::new(Clock {
+            slot: epoch_schedule.slots_per_epoch / 4,
+            ..Clock::default()
+        });
+
+        let second_half_clock = std::sync::Arc::new(Clock {
+            slot: (epoch_schedule.slots_per_epoch * 3) / 4,
+            ..Clock::default()
+        });
+
+        let mut feature_set = FeatureSet::default();
+        feature_set.activate(
+            &feature_set::commission_updates_only_allowed_in_first_half_of_epoch::id(),
+            1,
+        );
+
+        let signers: HashSet<Pubkey> = vec![withdrawer_pubkey].into_iter().collect();
+
+        // Increase commission in first half of epoch -- allowed
+        assert!(
+            borrowed_account
+                .get_state::<VoteStateVersions>()
+                .unwrap()
+                .convert_to_current()
+                .commission
+                == 10
+        );
+        assert_matches!(
+            update_commission(
+                &mut borrowed_account,
+                11,
+                &signers,
+                &epoch_schedule,
+                &first_half_clock,
+                &feature_set
+            ),
+            Ok(())
+        );
+        assert!(
+            borrowed_account
+                .get_state::<VoteStateVersions>()
+                .unwrap()
+                .convert_to_current()
+                .commission
+                == 11
+        );
+
+        // Increase commission in second half of epoch -- disallowed
+        assert_matches!(
+            update_commission(
+                &mut borrowed_account,
+                12,
+                &signers,
+                &epoch_schedule,
+                &second_half_clock,
+                &feature_set
+            ),
+            Err(_)
+        );
+        assert!(
+            borrowed_account
+                .get_state::<VoteStateVersions>()
+                .unwrap()
+                .convert_to_current()
+                .commission
+                == 11
+        );
+
+        // Decrease commission in first half of epoch -- allowed
+        assert_matches!(
+            update_commission(
+                &mut borrowed_account,
+                10,
+                &signers,
+                &epoch_schedule,
+                &first_half_clock,
+                &feature_set
+            ),
+            Ok(())
+        );
+        assert!(
+            borrowed_account
+                .get_state::<VoteStateVersions>()
+                .unwrap()
+                .convert_to_current()
+                .commission
+                == 10
+        );
+
+        // Decrease commission in second half of epoch -- disallowed because feature_set does not allow it
+        assert_matches!(
+            update_commission(
+                &mut borrowed_account,
+                9,
+                &signers,
+                &epoch_schedule,
+                &second_half_clock,
+                &feature_set
+            ),
+            Err(_)
+        );
+        assert!(
+            borrowed_account
+                .get_state::<VoteStateVersions>()
+                .unwrap()
+                .convert_to_current()
+                .commission
+                == 10
+        );
+
+        // Decrease commission in second half of epoch -- allowed because feature_set does not allow it
+        feature_set.activate(&feature_set::allow_commission_decrease_at_any_time::id(), 1);
+        assert_matches!(
+            update_commission(
+                &mut borrowed_account,
+                9,
+                &signers,
+                &epoch_schedule,
+                &second_half_clock,
+                &feature_set
+            ),
+            Ok(())
+        );
+        assert!(
+            borrowed_account
+                .get_state::<VoteStateVersions>()
+                .unwrap()
+                .convert_to_current()
+                .commission
+                == 9
+        );
+    }
+
     #[test]
     fn test_vote_double_lockout_after_expiration() {
         let voter_pubkey = solana_sdk::pubkey::new_rand();

sdk/src/feature_set.rs

@@ -724,6 +724,10 @@ pub mod update_hashes_per_tick6 {
     solana_sdk::declare_id!("FKu1qYwLQSiehz644H6Si65U5ZQ2cp9GxsyFUfYcuADv");
 }
 
+pub mod allow_commission_decrease_at_any_time {
+    solana_sdk::declare_id!("9wWpKE7vL7EmZcDTPm8n9pPVGJLynedAdAxh3BzZnAoF");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -900,6 +904,7 @@ lazy_static! {
         (update_hashes_per_tick4::id(), "Update desired hashes per tick to 7.6M"),
         (update_hashes_per_tick5::id(), "Update desired hashes per tick to 9.2M"),
         (update_hashes_per_tick6::id(), "Update desired hashes per tick to 10M"),
+        (allow_commission_decrease_at_any_time::id(), "Allow commission decrease at any time in epoch #33843"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()