From 0425db0396f433dc2c26fd7a5d2c29b094950ae7 Mon Sep 17 00:00:00 2001 From: Coach Chuck <169060940+coachchucksol@users.noreply.github.com> Date: Tue, 23 Jul 2024 07:32:19 -0600 Subject: [PATCH] FEATURE: Mark delinquent validators for instant removal (#57) To cover the edge case when some validators can be removed on the same epoch, we treat them differently. If there are any validators that are marked for instant removal - they have to be removed before anything else can happen. This could be further cleaned by running epoch maintenance over all validators. But for now, it should cover us. --------- Co-authored-by: Christian Krueger --- .github/workflows/build.yaml | 2 +- programs/steward/idl/steward.json | 85 +++++++++++++- programs/steward/src/errors.rs | 8 ++ programs/steward/src/events.rs | 1 + .../auto_add_validator_to_pool.rs | 15 ++- .../auto_remove_validator_from_pool.rs | 111 ++++++++++++------ .../src/instructions/compute_delegations.rs | 29 +++-- .../instructions/compute_instant_unstake.rs | 35 ++++-- .../steward/src/instructions/compute_score.rs | 26 ++-- .../src/instructions/epoch_maintenance.rs | 46 +++----- programs/steward/src/instructions/idle.rs | 31 +++-- .../instructions/instant_remove_validator.rs | 81 +++++++++++++ programs/steward/src/instructions/mod.rs | 2 + .../steward/src/instructions/realloc_state.rs | 1 + .../steward/src/instructions/rebalance.rs | 34 ++++-- .../src/instructions/reset_steward_state.rs | 1 + programs/steward/src/lib.rs | 8 ++ programs/steward/src/state/steward_state.rs | 51 ++++++-- tests/src/steward_fixtures.rs | 5 + tests/tests/steward/test_spl_passthrough.rs | 17 +-- tests/tests/steward/test_steward.rs | 39 +++++- 21 files changed, 475 insertions(+), 153 deletions(-) create mode 100644 programs/steward/src/instructions/instant_remove_validator.rs diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index f61fdef7..4e118e11 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -23,7 +23,7 @@ jobs: uses: baptiste0928/cargo-install@v3 with: crate: cargo-audit - - run: cargo audit --ignore RUSTSEC-2022-0093 --ignore RUSTSEC-2023-0065 --ignore RUSTSEC-2024-0336 --ignore RUSTSEC-2024-0344 + - run: cargo audit --ignore RUSTSEC-2022-0093 --ignore RUSTSEC-2023-0065 --ignore RUSTSEC-2024-0336 --ignore RUSTSEC-2024-0344 --ignore RUSTSEC-2024-0357 lint: name: lint diff --git a/programs/steward/idl/steward.json b/programs/steward/idl/steward.json index de1fb130..9378d755 100644 --- a/programs/steward/idl/steward.json +++ b/programs/steward/idl/steward.json @@ -881,6 +881,43 @@ } ] }, + { + "name": "instant_remove_validator", + "docs": [ + "Housekeeping, run at the start of any new epoch before any other instructions" + ], + "discriminator": [ + 119, + 127, + 216, + 135, + 24, + 63, + 229, + 242 + ], + "accounts": [ + { + "name": "config" + }, + { + "name": "state_account", + "writable": true + }, + { + "name": "validator_list" + }, + { + "name": "stake_pool" + } + ], + "args": [ + { + "name": "validator_index_to_remove", + "type": "u64" + } + ] + }, { "name": "pause_steward", "discriminator": [ @@ -1624,36 +1661,56 @@ }, { "code": 6022, + "name": "ValidatorMarkedActive", + "msg": "Validator was marked active when it should be deactivating" + }, + { + "code": 6023, "name": "MaxValidatorsReached", "msg": "Max validators reached" }, { - "code": 6023, + "code": 6024, "name": "ValidatorHistoryMismatch", "msg": "Validator history account does not match vote account" }, { - "code": 6024, + "code": 6025, "name": "EpochMaintenanceNotComplete", "msg": "Epoch Maintenance must be called before continuing" }, { - "code": 6025, + "code": 6026, "name": "StakePoolNotUpdated", "msg": "The stake pool must be updated before continuing" }, { - "code": 6026, + "code": 6027, + "name": "EpochMaintenanceAlreadyComplete", + "msg": "Epoch Maintenance has already been completed" + }, + { + "code": 6028, + "name": "ValidatorsNeedToBeRemoved", + "msg": "Validators are marked for immediate removal" + }, + { + "code": 6029, + "name": "NoValidatorsNeedToBeRemoved", + "msg": "No validators are marked for immediate removal" + }, + { + "code": 6030, "name": "ValidatorNotMarkedForRemoval", "msg": "Validator not marked for removal" }, { - "code": 6027, + "code": 6031, "name": "ValidatorsHaveNotBeenRemoved", "msg": "Validators have not been removed" }, { - "code": 6028, + "code": 6032, "name": "ListStateMismatch", "msg": "Validator List count does not match state machine" } @@ -1736,6 +1793,10 @@ { "name": "stake_account_deactivated", "type": "bool" + }, + { + "name": "marked_for_immediate_removal", + "type": "bool" } ] } @@ -2690,6 +2751,18 @@ } } }, + { + "name": "validators_for_immediate_removal", + "docs": [ + "Marks a validator for immediate removal after `remove_validator_from_pool` has been called on the stake pool", + "This happens when a validator is able to be removed within the same epoch as it was marked" + ], + "type": { + "defined": { + "name": "BitMask" + } + } + }, { "name": "start_computing_scores_slot", "docs": [ diff --git a/programs/steward/src/errors.rs b/programs/steward/src/errors.rs index de36d4ad..ea80d06f 100644 --- a/programs/steward/src/errors.rs +++ b/programs/steward/src/errors.rs @@ -48,6 +48,8 @@ pub enum StewardError { ArithmeticError, #[msg("Validator not eligible for removal. Must be delinquent or have closed vote account")] ValidatorNotRemovable, + #[msg("Validator was marked active when it should be deactivating")] + ValidatorMarkedActive, #[msg("Max validators reached")] MaxValidatorsReached, #[msg("Validator history account does not match vote account")] @@ -56,6 +58,12 @@ pub enum StewardError { EpochMaintenanceNotComplete, #[msg("The stake pool must be updated before continuing")] StakePoolNotUpdated, + #[msg("Epoch Maintenance has already been completed")] + EpochMaintenanceAlreadyComplete, + #[msg("Validators are marked for immediate removal")] + ValidatorsNeedToBeRemoved, + #[msg("No validators are marked for immediate removal")] + NoValidatorsNeedToBeRemoved, #[msg("Validator not marked for removal")] ValidatorNotMarkedForRemoval, #[msg("Validators have not been removed")] diff --git a/programs/steward/src/events.rs b/programs/steward/src/events.rs index c1f57228..8de1d7ae 100644 --- a/programs/steward/src/events.rs +++ b/programs/steward/src/events.rs @@ -12,6 +12,7 @@ pub struct AutoRemoveValidatorEvent { pub vote_account: Pubkey, pub vote_account_closed: bool, pub stake_account_deactivated: bool, + pub marked_for_immediate_removal: bool, } #[event] diff --git a/programs/steward/src/instructions/auto_add_validator_to_pool.rs b/programs/steward/src/instructions/auto_add_validator_to_pool.rs index b9b2ff76..91d7382a 100644 --- a/programs/steward/src/instructions/auto_add_validator_to_pool.rs +++ b/programs/steward/src/instructions/auto_add_validator_to_pool.rs @@ -107,10 +107,17 @@ pub fn handler(ctx: Context) -> Result<()> { let epoch = Clock::get()?.epoch; // Should not be able to add a validator if update is not complete - require!( - epoch == state_account.state.current_epoch, - StewardError::EpochMaintenanceNotComplete - ); + { + require!( + epoch == state_account.state.current_epoch, + StewardError::EpochMaintenanceNotComplete + ); + + require!( + state_account.state.validators_for_immediate_removal.count() == 0, + StewardError::ValidatorsNeedToBeRemoved + ); + } let validator_list_len = { let validator_list_data = &mut ctx.accounts.validator_list.try_borrow_mut_data()?; diff --git a/programs/steward/src/instructions/auto_remove_validator_from_pool.rs b/programs/steward/src/instructions/auto_remove_validator_from_pool.rs index 70a205e1..1b0ee4bb 100644 --- a/programs/steward/src/instructions/auto_remove_validator_from_pool.rs +++ b/programs/steward/src/instructions/auto_remove_validator_from_pool.rs @@ -12,6 +12,7 @@ use anchor_lang::solana_program::{program::invoke_signed, stake, sysvar, vote}; use anchor_lang::{prelude::*, system_program}; use spl_pod::solana_program::borsh1::try_from_slice_unchecked; use spl_pod::solana_program::stake::state::StakeStateV2; +use spl_stake_pool::state::StakeStatus; use spl_stake_pool::{find_stake_program_address, find_transient_stake_program_address}; use validator_history::state::ValidatorHistory; @@ -124,8 +125,11 @@ pub struct AutoRemoveValidator<'info> { */ pub fn handler(ctx: Context, validator_list_index: usize) -> Result<()> { + let stake_account_deactivated; + let vote_account_closed; + { - let mut state_account = ctx.accounts.state_account.load_mut()?; + let state_account = ctx.accounts.state_account.load()?; let validator_list = &ctx.accounts.validator_list; let epoch = Clock::get()?.epoch; @@ -143,7 +147,7 @@ pub fn handler(ctx: Context, validator_list_index: usize) - ); // Checks state for deactivate delinquent status, preventing pool from merging stake with activating - let stake_account_deactivated = { + stake_account_deactivated = { let stake_account_data = &mut ctx.accounts.stake_account.data.borrow_mut(); let stake_state: StakeStateV2 = try_from_slice_unchecked::(stake_account_data)?; @@ -158,57 +162,86 @@ pub fn handler(ctx: Context, validator_list_index: usize) - }; // Check if vote account closed - let vote_account_closed = ctx.accounts.vote_account.owner == &system_program::ID; + vote_account_closed = ctx.accounts.vote_account.owner == &system_program::ID; require!( stake_account_deactivated || vote_account_closed, StewardError::ValidatorNotRemovable ); + } + + { + invoke_signed( + &spl_stake_pool::instruction::remove_validator_from_pool( + &ctx.accounts.stake_pool_program.key(), + &ctx.accounts.stake_pool.key(), + &ctx.accounts.state_account.key(), + &ctx.accounts.withdraw_authority.key(), + &ctx.accounts.validator_list.key(), + &ctx.accounts.stake_account.key(), + &ctx.accounts.transient_stake_account.key(), + ), + &[ + ctx.accounts.stake_pool.to_account_info(), + ctx.accounts.state_account.to_account_info(), + ctx.accounts.reserve_stake.to_owned(), + ctx.accounts.withdraw_authority.to_owned(), + ctx.accounts.validator_list.to_account_info(), + ctx.accounts.stake_account.to_account_info(), + ctx.accounts.transient_stake_account.to_account_info(), + ctx.accounts.vote_account.to_account_info(), + ctx.accounts.rent.to_account_info(), + ctx.accounts.clock.to_account_info(), + ctx.accounts.stake_history.to_account_info(), + ctx.accounts.stake_config.to_account_info(), + ctx.accounts.system_program.to_account_info(), + ctx.accounts.stake_program.to_account_info(), + ], + &[&[ + StewardStateAccount::SEED, + &ctx.accounts.config.key().to_bytes(), + &[ctx.bumps.state_account], + ]], + )?; + } + + { + // Read the state account again + let mut state_account = ctx.accounts.state_account.load_mut()?; + let validator_list = &ctx.accounts.validator_list; + let validator_stake_info = + get_validator_stake_info_at_index(validator_list, validator_list_index)?; - state_account - .state - .mark_validator_for_removal(validator_list_index)?; + let stake_status = StakeStatus::try_from(validator_stake_info.status)?; + let marked_for_immediate_removal: bool; + + match stake_status { + StakeStatus::Active => { + // Should never happen + return Err(StewardError::ValidatorMarkedActive.into()); + } + StakeStatus::DeactivatingValidator | StakeStatus::ReadyForRemoval => { + marked_for_immediate_removal = true; + state_account + .state + .mark_validator_for_immediate_removal(validator_list_index)?; + } + StakeStatus::DeactivatingAll | StakeStatus::DeactivatingTransient => { + marked_for_immediate_removal = false; + state_account + .state + .mark_validator_for_removal(validator_list_index)?; + } + } emit!(AutoRemoveValidatorEvent { vote_account: ctx.accounts.vote_account.key(), validator_list_index: validator_list_index as u64, stake_account_deactivated, vote_account_closed, + marked_for_immediate_removal, }); } - invoke_signed( - &spl_stake_pool::instruction::remove_validator_from_pool( - &ctx.accounts.stake_pool_program.key(), - &ctx.accounts.stake_pool.key(), - &ctx.accounts.state_account.key(), - &ctx.accounts.withdraw_authority.key(), - &ctx.accounts.validator_list.key(), - &ctx.accounts.stake_account.key(), - &ctx.accounts.transient_stake_account.key(), - ), - &[ - ctx.accounts.stake_pool.to_account_info(), - ctx.accounts.state_account.to_account_info(), - ctx.accounts.reserve_stake.to_owned(), - ctx.accounts.withdraw_authority.to_owned(), - ctx.accounts.validator_list.to_account_info(), - ctx.accounts.stake_account.to_account_info(), - ctx.accounts.transient_stake_account.to_account_info(), - ctx.accounts.vote_account.to_account_info(), - ctx.accounts.rent.to_account_info(), - ctx.accounts.clock.to_account_info(), - ctx.accounts.stake_history.to_account_info(), - ctx.accounts.stake_config.to_account_info(), - ctx.accounts.system_program.to_account_info(), - ctx.accounts.stake_program.to_account_info(), - ], - &[&[ - StewardStateAccount::SEED, - &ctx.accounts.config.key().to_bytes(), - &[ctx.bumps.state_account], - ]], - )?; - Ok(()) } diff --git a/programs/steward/src/instructions/compute_delegations.rs b/programs/steward/src/instructions/compute_delegations.rs index 005fe540..27729ead 100644 --- a/programs/steward/src/instructions/compute_delegations.rs +++ b/programs/steward/src/instructions/compute_delegations.rs @@ -1,5 +1,5 @@ use crate::errors::StewardError; -use crate::{maybe_transition_and_emit, Config, StewardStateAccount}; +use crate::{maybe_transition_and_emit, Config, StewardStateAccount, StewardStateEnum}; use anchor_lang::prelude::*; #[derive(Accounts)] @@ -25,13 +25,28 @@ pub fn handler(ctx: Context) -> Result<()> { let clock = Clock::get()?; let epoch_schedule = EpochSchedule::get()?; - require!( - clock.epoch == state_account.state.current_epoch, - StewardError::EpochMaintenanceNotComplete - ); + { + if config.is_paused() { + return Err(StewardError::StateMachinePaused.into()); + } - if config.is_paused() { - return Err(StewardError::StateMachinePaused.into()); + require!( + matches!( + state_account.state.state_tag, + StewardStateEnum::ComputeDelegations + ), + StewardError::InvalidState + ); + + require!( + clock.epoch == state_account.state.current_epoch, + StewardError::EpochMaintenanceNotComplete + ); + + require!( + state_account.state.validators_for_immediate_removal.count() == 0, + StewardError::ValidatorsNeedToBeRemoved + ); } state_account diff --git a/programs/steward/src/instructions/compute_instant_unstake.rs b/programs/steward/src/instructions/compute_instant_unstake.rs index bfb50f44..576d0606 100644 --- a/programs/steward/src/instructions/compute_instant_unstake.rs +++ b/programs/steward/src/instructions/compute_instant_unstake.rs @@ -2,7 +2,7 @@ use crate::{ errors::StewardError, maybe_transition_and_emit, utils::{get_validator_list, get_validator_stake_info_at_index}, - Config, StewardStateAccount, + Config, StewardStateAccount, StewardStateEnum, }; use anchor_lang::prelude::*; use validator_history::{ClusterHistory, ValidatorHistory}; @@ -41,6 +41,30 @@ pub fn handler(ctx: Context, validator_list_index: usize) let clock = Clock::get()?; let epoch_schedule = EpochSchedule::get()?; + { + if config.is_paused() { + return Err(StewardError::StateMachinePaused.into()); + } + + require!( + matches!( + state_account.state.state_tag, + StewardStateEnum::ComputeInstantUnstake + ), + StewardError::InvalidState + ); + + require!( + clock.epoch == state_account.state.current_epoch, + StewardError::EpochMaintenanceNotComplete + ); + + require!( + state_account.state.validators_for_immediate_removal.count() == 0, + StewardError::ValidatorsNeedToBeRemoved + ); + } + let validator_stake_info = get_validator_stake_info_at_index(validator_list, validator_list_index)?; require!( @@ -48,15 +72,6 @@ pub fn handler(ctx: Context, validator_list_index: usize) StewardError::ValidatorNotInList ); - require!( - clock.epoch == state_account.state.current_epoch, - StewardError::EpochMaintenanceNotComplete - ); - - if config.is_paused() { - return Err(StewardError::StateMachinePaused.into()); - } - if let Some(instant_unstake) = state_account.state.compute_instant_unstake( &clock, &epoch_schedule, diff --git a/programs/steward/src/instructions/compute_score.rs b/programs/steward/src/instructions/compute_score.rs index 585420ae..04bad5f9 100644 --- a/programs/steward/src/instructions/compute_score.rs +++ b/programs/steward/src/instructions/compute_score.rs @@ -42,6 +42,22 @@ pub fn handler(ctx: Context, validator_list_index: usize) -> Resul let clock: Clock = Clock::get()?; let epoch_schedule = EpochSchedule::get()?; + { + if config.is_paused() { + return Err(StewardError::StateMachinePaused.into()); + } + + require!( + clock.epoch == state_account.state.current_epoch, + StewardError::EpochMaintenanceNotComplete + ); + + require!( + state_account.state.validators_for_immediate_removal.count() == 0, + StewardError::ValidatorsNeedToBeRemoved + ); + } + let validator_stake_info = get_validator_stake_info_at_index(validator_list, validator_list_index)?; require!( @@ -51,15 +67,6 @@ pub fn handler(ctx: Context, validator_list_index: usize) -> Resul let num_pool_validators = get_validator_list_length(validator_list)?; - require!( - clock.epoch == state_account.state.current_epoch, - StewardError::EpochMaintenanceNotComplete - ); - - if config.is_paused() { - return Err(StewardError::StateMachinePaused.into()); - } - // May need to force an extra transition here in case cranking got stuck in any previous state // and it's now the start of a new scoring cycle if !matches!( @@ -73,6 +80,7 @@ pub fn handler(ctx: Context, validator_list_index: usize) -> Resul &epoch_schedule, )?; } + require!( matches!( state_account.state.state_tag, diff --git a/programs/steward/src/instructions/epoch_maintenance.rs b/programs/steward/src/instructions/epoch_maintenance.rs index 8c75fea0..4cd72e2e 100644 --- a/programs/steward/src/instructions/epoch_maintenance.rs +++ b/programs/steward/src/instructions/epoch_maintenance.rs @@ -3,7 +3,7 @@ use crate::{ events::EpochMaintenanceEvent, utils::{ check_validator_list_has_stake_status_other_than, deserialize_stake_pool, - get_stake_pool_address, get_validator_list_length, + get_stake_pool_address, get_validator_list, get_validator_list_length, }, Config, StewardStateAccount, CHECKED_VALIDATORS_REMOVED_FROM_LIST, COMPUTE_INSTANT_UNSTAKES, EPOCH_MAINTENANCE, POST_LOOP_IDLE, PRE_LOOP_IDLE, REBALANCE, RESET_TO_IDLE, @@ -23,7 +23,7 @@ pub struct EpochMaintenance<'info> { pub state_account: AccountLoader<'info, StewardStateAccount>, /// CHECK: Correct account guaranteed if address is correct - #[account(address = deserialize_stake_pool(&stake_pool)?.validator_list)] + #[account(address = get_validator_list(&config)?)] pub validator_list: AccountInfo<'info>, /// CHECK: Correct account guaranteed if address is correct @@ -50,35 +50,31 @@ pub fn handler( StewardError::StakePoolNotUpdated ); - // Keep this unset until we have completed all maintenance tasks - state_account.state.unset_flag(EPOCH_MAINTENANCE); + require!( + state_account.state.current_epoch < clock.epoch, + StewardError::EpochMaintenanceAlreadyComplete + ); - // We only need to check this once per maintenance cycle - if !state_account - .state - .has_flag(CHECKED_VALIDATORS_REMOVED_FROM_LIST) - { - // Ensure there are no validators in the list that have not been removed, that should be - require!( - !check_validator_list_has_stake_status_other_than( - &ctx.accounts.validator_list, - StakeStatus::Active - )?, - StewardError::ValidatorsHaveNotBeenRemoved - ); + // Ensure there are no validators in the list that have not been removed, that should be + require!( + !check_validator_list_has_stake_status_other_than( + &ctx.accounts.validator_list, + StakeStatus::Active + )?, + StewardError::ValidatorsHaveNotBeenRemoved + ); - state_account - .state - .set_flag(CHECKED_VALIDATORS_REMOVED_FROM_LIST); - } + state_account.state.unset_flag(EPOCH_MAINTENANCE); { // Routine - Remove marked validators // We still want these checks to run even if we don't specify a validator to remove let validators_in_list = get_validator_list_length(&ctx.accounts.validator_list)?; - let validators_to_remove = state_account.state.validators_to_remove.count(); + let validators_to_remove = state_account.state.validators_to_remove.count() + + state_account.state.validators_for_immediate_removal.count(); // Ensure we have a 1-1 mapping between the number of validators in the list and the number of validators in the state + // If we don't have this mapping, everything needs to be removed require!( state_account.state.num_pool_validators as usize + state_account.state.validators_added as usize @@ -95,10 +91,7 @@ pub fn handler( { // Routine - Update state - let okay_to_update = state_account.state.validators_to_remove.is_empty() - && state_account - .state - .has_flag(CHECKED_VALIDATORS_REMOVED_FROM_LIST); + let okay_to_update = state_account.state.validators_to_remove.is_empty(); if okay_to_update { state_account.state.current_epoch = clock.epoch; @@ -115,7 +108,6 @@ pub fn handler( .state .set_flag(RESET_TO_IDLE | EPOCH_MAINTENANCE); } - emit!(EpochMaintenanceEvent { validator_index_to_remove: validator_index_to_remove.map(|x| x as u64), validator_list_length: get_validator_list_length(&ctx.accounts.validator_list)? as u64, diff --git a/programs/steward/src/instructions/idle.rs b/programs/steward/src/instructions/idle.rs index 73173e5c..be16761c 100644 --- a/programs/steward/src/instructions/idle.rs +++ b/programs/steward/src/instructions/idle.rs @@ -25,18 +25,25 @@ pub fn handler(ctx: Context) -> Result<()> { let clock = Clock::get()?; let epoch_schedule = EpochSchedule::get()?; - require!( - matches!(state_account.state.state_tag, StewardStateEnum::Idle), - StewardError::InvalidState - ); - - require!( - clock.epoch == state_account.state.current_epoch, - StewardError::EpochMaintenanceNotComplete - ); - - if config.is_paused() { - return Err(StewardError::StateMachinePaused.into()); + { + if config.is_paused() { + return Err(StewardError::StateMachinePaused.into()); + } + + require!( + matches!(state_account.state.state_tag, StewardStateEnum::Idle), + StewardError::InvalidState + ); + + require!( + clock.epoch == state_account.state.current_epoch, + StewardError::EpochMaintenanceNotComplete + ); + + require!( + state_account.state.validators_for_immediate_removal.count() == 0, + StewardError::ValidatorsNeedToBeRemoved + ); } maybe_transition_and_emit( diff --git a/programs/steward/src/instructions/instant_remove_validator.rs b/programs/steward/src/instructions/instant_remove_validator.rs new file mode 100644 index 00000000..63d76057 --- /dev/null +++ b/programs/steward/src/instructions/instant_remove_validator.rs @@ -0,0 +1,81 @@ +use crate::{ + errors::StewardError, + utils::{ + check_validator_list_has_stake_status_other_than, deserialize_stake_pool, + get_stake_pool_address, get_validator_list, get_validator_list_length, + }, + Config, StewardStateAccount, +}; +use anchor_lang::prelude::*; +use spl_stake_pool::state::StakeStatus; + +#[derive(Accounts)] +pub struct InstantRemoveValidator<'info> { + pub config: AccountLoader<'info, Config>, + + #[account( + mut, + seeds = [StewardStateAccount::SEED, config.key().as_ref()], + bump + )] + pub state_account: AccountLoader<'info, StewardStateAccount>, + + /// CHECK: Correct account guaranteed if address is correct + #[account(address = get_validator_list(&config)?)] + pub validator_list: AccountInfo<'info>, + + /// CHECK: Correct account guaranteed if address is correct + #[account( + address = get_stake_pool_address(&config)? + )] + pub stake_pool: AccountInfo<'info>, +} + +/// Removes validators from the pool that have been marked for immediate removal +pub fn handler( + ctx: Context, + validator_index_to_remove: usize, +) -> Result<()> { + let stake_pool = deserialize_stake_pool(&ctx.accounts.stake_pool)?; + let mut state_account = ctx.accounts.state_account.load_mut()?; + + let clock = Clock::get()?; + let validators_to_remove = state_account.state.validators_for_immediate_removal.count(); + let validators_in_list = get_validator_list_length(&ctx.accounts.validator_list)?; + + require!( + clock.epoch == stake_pool.last_update_epoch, + StewardError::StakePoolNotUpdated + ); + + require!( + state_account + .state + .validators_for_immediate_removal + .get(validator_index_to_remove)?, + StewardError::ValidatorNotInList + ); + + require!( + state_account.state.num_pool_validators as usize + + state_account.state.validators_added as usize + - validators_to_remove + == validators_in_list, + StewardError::ListStateMismatch + ); + + // Ensure there are no validators in the list that have not been removed, that should be + require!( + !check_validator_list_has_stake_status_other_than( + &ctx.accounts.validator_list, + StakeStatus::Active + )?, + StewardError::ValidatorsHaveNotBeenRemoved + ); + + state_account + .state + .remove_validator(validator_index_to_remove)?; + + Ok(()) +} diff --git a/programs/steward/src/instructions/mod.rs b/programs/steward/src/instructions/mod.rs index fdef8aa5..27337283 100644 --- a/programs/steward/src/instructions/mod.rs +++ b/programs/steward/src/instructions/mod.rs @@ -9,6 +9,7 @@ pub mod compute_score; pub mod epoch_maintenance; pub mod idle; pub mod initialize_steward; +pub mod instant_remove_validator; pub mod pause_steward; pub mod realloc_state; pub mod rebalance; @@ -29,6 +30,7 @@ pub use compute_score::*; pub use epoch_maintenance::*; pub use idle::*; pub use initialize_steward::*; +pub use instant_remove_validator::*; pub use pause_steward::*; pub use realloc_state::*; pub use rebalance::*; diff --git a/programs/steward/src/instructions/realloc_state.rs b/programs/steward/src/instructions/realloc_state.rs index b2e6ef8d..0d5057e8 100644 --- a/programs/steward/src/instructions/realloc_state.rs +++ b/programs/steward/src/instructions/realloc_state.rs @@ -88,6 +88,7 @@ pub fn handler(ctx: Context) -> Result<()> { state_account.state.instant_unstake = BitMask::default(); state_account.state.start_computing_scores_slot = clock.slot; state_account.state.validators_to_remove = BitMask::default(); + state_account.state.validators_for_immediate_removal = BitMask::default(); state_account.state.validators_added = 0; state_account.state.clear_flags(); state_account.state._padding0 = [0; STATE_PADDING_0_SIZE]; diff --git a/programs/steward/src/instructions/rebalance.rs b/programs/steward/src/instructions/rebalance.rs index 3bf09d4d..9aba351a 100644 --- a/programs/steward/src/instructions/rebalance.rs +++ b/programs/steward/src/instructions/rebalance.rs @@ -24,7 +24,7 @@ use crate::{ events::{DecreaseComponents, RebalanceEvent, RebalanceTypeTag}, maybe_transition_and_emit, utils::{deserialize_stake_pool, get_stake_pool_address, get_validator_stake_info_at_index}, - Config, StewardStateAccount, + Config, StewardStateAccount, StewardStateEnum, }; #[derive(Accounts)] @@ -111,7 +111,7 @@ pub struct Rebalance<'info> { pub transient_stake_account: AccountInfo<'info>, /// CHECK: passing through, checks are done by spl-stake-pool - #[account(owner = vote::program::ID)] + #[account(constraint = (vote_account.owner == &vote::program::ID || vote_account.owner == &system_program::ID))] pub vote_account: AccountInfo<'info>, /// CHECK: passing through, checks are done by spl-stake-pool @@ -146,6 +146,27 @@ pub fn handler(ctx: Context, validator_list_index: usize) -> Result<( let clock = Clock::get()?; let epoch_schedule = EpochSchedule::get()?; + { + if config.is_paused() { + return Err(StewardError::StateMachinePaused.into()); + } + + require!( + matches!(state_account.state.state_tag, StewardStateEnum::Rebalance), + StewardError::InvalidState + ); + + require!( + clock.epoch == state_account.state.current_epoch, + StewardError::EpochMaintenanceNotComplete + ); + + require!( + state_account.state.validators_for_immediate_removal.count() == 0, + StewardError::ValidatorsNeedToBeRemoved + ); + } + let validator_stake_info = get_validator_stake_info_at_index(validator_list, validator_list_index)?; require!( @@ -154,15 +175,6 @@ pub fn handler(ctx: Context, validator_list_index: usize) -> Result<( ); let transient_seed = u64::from(validator_stake_info.transient_seed_suffix); - require!( - clock.epoch == state_account.state.current_epoch, - StewardError::EpochMaintenanceNotComplete - ); - - if config.is_paused() { - return Err(StewardError::StateMachinePaused.into()); - } - let minimum_delegation = minimum_delegation(get_minimum_delegation()?); let stake_rent = Rent::get()?.minimum_balance(StakeStateV2::size_of()); diff --git a/programs/steward/src/instructions/reset_steward_state.rs b/programs/steward/src/instructions/reset_steward_state.rs index b647f4c7..0588b014 100644 --- a/programs/steward/src/instructions/reset_steward_state.rs +++ b/programs/steward/src/instructions/reset_steward_state.rs @@ -61,6 +61,7 @@ pub fn handler(ctx: Context) -> Result<()> { state_account.state.instant_unstake = BitMask::default(); state_account.state.start_computing_scores_slot = clock.slot; state_account.state.validators_to_remove = BitMask::default(); + state_account.state.validators_for_immediate_removal = BitMask::default(); state_account.state.validators_added = 0; state_account.state.clear_flags(); state_account.state._padding0 = [0; STATE_PADDING_0_SIZE]; diff --git a/programs/steward/src/lib.rs b/programs/steward/src/lib.rs index 74b8f938..574fd923 100644 --- a/programs/steward/src/lib.rs +++ b/programs/steward/src/lib.rs @@ -94,6 +94,14 @@ pub mod steward { instructions::epoch_maintenance::handler(ctx, validator_index_to_remove.map(|x| x as usize)) } + /// Housekeeping, run at the start of any new epoch before any other instructions + pub fn instant_remove_validator( + ctx: Context, + validator_index_to_remove: u64, + ) -> Result<()> { + instructions::instant_remove_validator::handler(ctx, validator_index_to_remove as usize) + } + /// Computes score for a the validator at `validator_list_index` for the current cycle. pub fn compute_score(ctx: Context, validator_list_index: u64) -> Result<()> { instructions::compute_score::handler(ctx, validator_list_index as usize) diff --git a/programs/steward/src/state/steward_state.rs b/programs/steward/src/state/steward_state.rs index 8e782992..b0c070b3 100644 --- a/programs/steward/src/state/steward_state.rs +++ b/programs/steward/src/state/steward_state.rs @@ -87,6 +87,10 @@ pub struct StewardState { /// This is cleaned up in the next epoch pub validators_to_remove: BitMask, + /// Marks a validator for immediate removal after `remove_validator_from_pool` has been called on the stake pool + /// This happens when a validator is able to be removed within the same epoch as it was marked + pub validators_for_immediate_removal: BitMask, + ////// Cycle metadata fields ////// /// Slot of the first ComputeScores instruction in the current cycle pub start_computing_scores_slot: u64, @@ -463,15 +467,27 @@ impl StewardState { /// Update internal state when a validator is removed from the pool pub fn remove_validator(&mut self, index: usize) -> Result<()> { + let marked_for_regular_removal = self.validators_to_remove.get(index)?; + let marked_for_immediate_removal = self.validators_for_immediate_removal.get(index)?; + require!( - self.validators_to_remove.get(index)?, + marked_for_regular_removal || marked_for_immediate_removal, StewardError::ValidatorNotMarkedForRemoval ); - self.num_pool_validators = self - .num_pool_validators - .checked_sub(1) - .ok_or(StewardError::ArithmeticError)?; + // If the validator was marked for removal in the current cycle, decrement validators_added + if index >= self.num_pool_validators as usize { + self.validators_added = self + .validators_added + .checked_sub(1) + .ok_or(StewardError::ArithmeticError)?; + } else { + self.num_pool_validators = self + .num_pool_validators + .checked_sub(1) + .ok_or(StewardError::ArithmeticError)?; + } + let num_pool_validators = self.num_pool_validators as usize; // Shift all validator state to the left @@ -486,6 +502,8 @@ impl StewardState { self.progress.set(i, self.progress.get(next_i)?)?; self.validators_to_remove .set(i, self.validators_to_remove.get(next_i)?)?; + self.validators_for_immediate_removal + .set(i, self.validators_for_immediate_removal.get(next_i)?)?; } // Update score indices @@ -533,9 +551,14 @@ impl StewardState { self.sorted_yield_score_indices[num_pool_validators] = SORTED_INDEX_DEFAULT; self.delegations[num_pool_validators] = Delegation::default(); self.instant_unstake.set(num_pool_validators, false)?; - self.validators_to_remove.set(num_pool_validators, false)?; self.progress.set(num_pool_validators, false)?; + if marked_for_regular_removal { + self.validators_to_remove.set(index, false)?; + } else { + self.validators_for_immediate_removal.set(index, false)?; + } + Ok(()) } @@ -546,6 +569,10 @@ impl StewardState { self.validators_to_remove.set(index, true) } + pub fn mark_validator_for_immediate_removal(&mut self, index: usize) -> Result<()> { + self.validators_for_immediate_removal.set(index, true) + } + /// Called when adding a validator to the pool so that we can ensure a 1-1 mapping between /// the validator list and the steward state pub fn increment_validator_to_add(&mut self) -> Result<()> { @@ -610,7 +637,9 @@ impl StewardState { } // Skip scoring if marked for deletion - if self.validators_to_remove.get(index)? { + if self.validators_to_remove.get(index)? + || self.validators_for_immediate_removal.get(index)? + { self.scores[index] = 0_u32; self.yield_scores[index] = 0_u32; @@ -755,7 +784,9 @@ impl StewardState { } // Skip if marked for deletion - if self.validators_to_remove.get(index)? { + if self.validators_to_remove.get(index)? + || self.validators_for_immediate_removal.get(index)? + { self.progress.set(index, true)?; return Ok(None); } @@ -835,7 +866,9 @@ impl StewardState { } // Skip if marked for deletion - if self.validators_to_remove.get(index)? { + if self.validators_to_remove.get(index)? + || self.validators_for_immediate_removal.get(index)? + { self.progress.set(index, true)?; return Ok(RebalanceType::None); } diff --git a/tests/src/steward_fixtures.rs b/tests/src/steward_fixtures.rs index 395466e3..bc376121 100644 --- a/tests/src/steward_fixtures.rs +++ b/tests/src/steward_fixtures.rs @@ -526,6 +526,10 @@ impl TestFixture { }; if let Err(e) = process_transaction_result { + if !e.to_string().contains(error_message) { + panic!("Error: {}\n\nDoes not match {}", e, error_message); + } + assert!(e.to_string().contains(error_message)); } else { panic!("Error: Transaction succeeded. Expected {}", error_message); @@ -937,6 +941,7 @@ impl Default for StateMachineFixtures { status_flags: 0, validators_added: 0, validators_to_remove: BitMask::default(), + validators_for_immediate_removal: BitMask::default(), _padding0: [0; STATE_PADDING_0_SIZE], }; diff --git a/tests/tests/steward/test_spl_passthrough.rs b/tests/tests/steward/test_spl_passthrough.rs index 1fc9e51f..141bb451 100644 --- a/tests/tests/steward/test_spl_passthrough.rs +++ b/tests/tests/steward/test_spl_passthrough.rs @@ -161,21 +161,6 @@ async fn _add_test_validator(fixture: &TestFixture, vote_account: Pubkey) { fixture.simulate_stake_pool_update().await; - let epoch_maintenance_ix = Instruction { - program_id: jito_steward::id(), - accounts: jito_steward::accounts::EpochMaintenance { - config: fixture.steward_config.pubkey(), - state_account: fixture.steward_state, - validator_list: fixture.stake_pool_meta.validator_list, - stake_pool: fixture.stake_pool_meta.stake_pool, - } - .to_account_metas(None), - data: jito_steward::instruction::EpochMaintenance { - validator_index_to_remove: None, - } - .data(), - }; - // Add Validator let instruction = Instruction { program_id: jito_steward::id(), @@ -207,7 +192,7 @@ async fn _add_test_validator(fixture: &TestFixture, vote_account: Pubkey) { let latest_blockhash = _get_latest_blockhash(fixture).await; let transaction = Transaction::new_signed_with_payer( - &[epoch_maintenance_ix, instruction], + &[instruction], Some(&fixture.keypair.pubkey()), &[&fixture.keypair], latest_blockhash, diff --git a/tests/tests/steward/test_steward.rs b/tests/tests/steward/test_steward.rs index 4126bd3e..60a941fa 100644 --- a/tests/tests/steward/test_steward.rs +++ b/tests/tests/steward/test_steward.rs @@ -135,10 +135,8 @@ async fn test_auto_remove() { fixture.stake_accounts_for_validator(vote_account).await; // Add vote account - println!("DID STEWARD STATE"); _auto_add_validator_to_pool(&fixture, &vote_account).await; - println!("ADDED VALIDATOR"); let auto_remove_validator_ix = Instruction { program_id: jito_steward::id(), @@ -200,6 +198,43 @@ async fn test_auto_remove() { fixture.submit_transaction_assert_success(tx).await; + let steward_state_account: StewardStateAccount = + fixture.load_and_deserialize(&fixture.steward_state).await; + + assert!( + steward_state_account + .state + .validators_for_immediate_removal + .count() + == 1 + ); + + let instant_remove_validator_ix = Instruction { + program_id: jito_steward::id(), + accounts: jito_steward::accounts::InstantRemoveValidator { + config: fixture.steward_config.pubkey(), + state_account: fixture.steward_state, + validator_list: fixture.stake_pool_meta.validator_list, + stake_pool: fixture.stake_pool_meta.stake_pool, + } + .to_account_metas(None), + data: jito_steward::instruction::InstantRemoveValidator { + validator_index_to_remove: 0, + } + .data(), + }; + + let tx = Transaction::new_signed_with_payer( + &[instant_remove_validator_ix.clone()], + Some(&fixture.keypair.pubkey()), + &[&fixture.keypair], + fixture.ctx.borrow().last_blockhash, + ); + + fixture + .submit_transaction_assert_error(tx, "ValidatorsHaveNotBeenRemoved") + .await; + drop(fixture); }