Skip to content

Commit

Permalink
Add allow_commission_decrease_at_any_time feature which will allow (#…
Browse files Browse the repository at this point in the history 
…33847)

* 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 #33843.

SIMD: 0080

* Remove unused `feature_set` import

---------

Co-authored-by: Jon Cinque <[email protected]>
  • Loading branch information
@bji @joncinque
bji and joncinque authored Dec 8, 2023
1 parent 79739e1 commit 930ce1e
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 15 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 5 additions & 12 deletions programs/vote/src/vote_processor.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
//! 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::{
declare_process_instruction, invoke_context::InvokeContext,
sysvar_cache::get_sysvar_with_account_check,
},
solana_sdk::{
feature_set,
instruction::InstructionError,
program_utils::limited_deserialize,
pubkey::Pubkey,
Expand Down Expand Up @@ -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,
)
}
Expand Down
227 changes: 224 additions & 3 deletions programs/vote/src/vote_state/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,11 +877,41 @@ pub fn update_commission<S: std::hash::BuildHasher>(
vote_account: &mut BorrowedAccount,
commission: u8,
signers: &HashSet<Pubkey, S>,
epoch_schedule: &EpochSchedule,
clock: &Clock,
feature_set: &FeatureSet,
) -> Result<(), InstructionError> {
let mut vote_state: VoteState = vote_account
.get_state::<VoteStateVersions>()?
.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::<VoteStateVersions>() {
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::<VoteStateVersions>()?
.convert_to_current(),
};

// current authorized withdrawer must say "yay"
verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?;
Expand All @@ -891,6 +921,11 @@ 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_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 {
Expand Down Expand Up @@ -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<Pubkey> = vec![withdrawer_pubkey].into_iter().collect();

// Increase commission in first half of epoch -- allowed
assert_eq!(
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_eq!(
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_eq!(
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_eq!(
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_eq!(
borrowed_account
.get_state::<VoteStateVersions>()
.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::<VoteStateVersions>()
.unwrap()
.convert_to_current()
.commission,
9
);
}

#[test]
fn test_vote_double_lockout_after_expiration() {
let voter_pubkey = solana_sdk::pubkey::new_rand();
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pubkey, &'static str> = [
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 930ce1e

Please sign in to comment.