diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e8517a4c9e5b5..4e568de857c674 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ Release channels have their own copy of this changelog: * The default for `solana-ledger-tool`, however, remains `always` (#34228) * Added `central-scheduler` option for `--block-production-method` (#33890) * Updated to Borsh v1 + * 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 * `solana-program` and `solana-sdk` default to support for Borsh v1, with limited backward compatibility for v0.10 and v0.9. Please upgrade to Borsh v1. diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index 3cfde9ed8e26cb..443aeb391b8c13 100644 --- a/programs/vote/src/vote_processor.rs +++ b/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::{ @@ -9,7 +9,6 @@ use { sysvar_cache::get_sysvar_with_account_check, }, solana_sdk::{ - feature_set, instruction::InstructionError, program_utils::limited_deserialize, pubkey::Pubkey, @@ -140,20 +139,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()?.as_ref(), + sysvar_cache.get_clock()?.as_ref(), &invoke_context.feature_set, ) } diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 775bfef326ed13..79d131583ae467 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -877,11 +877,41 @@ pub fn update_commission( vote_account: &mut BorrowedAccount, commission: u8, signers: &HashSet, + epoch_schedule: &EpochSchedule, + clock: &Clock, feature_set: &FeatureSet, ) -> Result<(), InstructionError> { - let mut vote_state: VoteState = vote_account - .get_state::()? - .convert_to_current(); + // Decode vote state only once, and only if needed + let mut vote_state = None; + + let enforce_commission_update_rule = + if feature_set.is_active(&feature_set::allow_commission_decrease_at_any_time::id()) { + if let Ok(decoded_vote_state) = vote_account.get_state::() { + vote_state = Some(decoded_vote_state.convert_to_current()); + is_commission_increase(vote_state.as_ref().unwrap(), commission) + } else { + true + } + } 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 = match vote_state { + Some(vote_state) => vote_state, + None => vote_account + .get_state::()? + .convert_to_current(), + }; // current authorized withdrawer must say "yay" verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; @@ -891,6 +921,11 @@ pub fn update_commission( 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_state: &VoteState, commission: u8) -> bool { + commission > vote_state.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 +1400,192 @@ mod tests { assert_eq!(vote_state.votes.len(), 2); } + #[test] + fn test_update_commission() { + let node_pubkey = Pubkey::new_unique(); + let withdrawer_pubkey = Pubkey::new_unique(); + 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 = vec![withdrawer_pubkey].into_iter().collect(); + + // Increase commission in first half of epoch -- allowed + assert_eq!( + borrowed_account + .get_state::() + .unwrap() + .convert_to_current() + .commission, + 10 + ); + assert_matches!( + update_commission( + &mut borrowed_account, + 11, + &signers, + &epoch_schedule, + &first_half_clock, + &feature_set + ), + Ok(()) + ); + assert_eq!( + borrowed_account + .get_state::() + .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_eq!( + borrowed_account + .get_state::() + .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_eq!( + borrowed_account + .get_state::() + .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_eq!( + borrowed_account + .get_state::() + .unwrap() + .convert_to_current() + .commission, + 10 + ); + + // Decrease commission in second half of epoch -- allowed because feature_set allows 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_eq!( + borrowed_account + .get_state::() + .unwrap() + .convert_to_current() + .commission, + 9 + ); + } + #[test] fn test_vote_double_lockout_after_expiration() { let voter_pubkey = solana_sdk::pubkey::new_rand(); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 5f3eb547244208..2373eebf7f6d09 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -736,6 +736,10 @@ pub mod drop_legacy_shreds { solana_sdk::declare_id!("GV49KKQdBNaiv2pgqhS2Dy3GWYJGXMTVYbYkdk91orRy"); } +pub mod allow_commission_decrease_at_any_time { + solana_sdk::declare_id!("decoMktMcnmiq6t3u7g5BfgcQu91nKZr6RvMYf9z1Jb"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -915,6 +919,7 @@ lazy_static! { (disable_rent_fees_collection::id(), "Disable rent fees collection #33945"), (enable_zk_transfer_with_fee::id(), "enable Zk Token proof program transfer with fee"), (drop_legacy_shreds::id(), "drops legacy shreds #34328"), + (allow_commission_decrease_at_any_time::id(), "Allow commission decrease at any time in epoch #33843"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()