Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding missing integration tests #87

Merged
merged 14 commits into from
Dec 4, 2024
3 changes: 2 additions & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ jobs:
uses: baptiste0928/cargo-install@v3
with:
crate: solana-verify
version: "0.2.11"
- name: Install anchor-cli from crates.io
uses: baptiste0928/cargo-install@v3
with:
Expand Down Expand Up @@ -156,7 +157,7 @@ jobs:
env:
RUST_LOG: trace
SBF_OUT_DIR: ${{ github.workspace }}/target/deploy
RUST_MIN_STACK: 5000000
RUST_MIN_STACK: 10000000

# release only runs on tagged commits
# it should wait for all the other steps to finish, to ensure releases are the highest quality
Expand Down
6 changes: 3 additions & 3 deletions programs/steward/src/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ impl UnstakeState {
})
}

fn stake_deposit_unstake(
pub fn stake_deposit_unstake(
&self,
state: &StewardState,
index: usize,
Expand Down Expand Up @@ -276,7 +276,7 @@ impl UnstakeState {
Ok(0)
}

fn instant_unstake(
pub fn instant_unstake(
&self,
state: &StewardState,
index: usize,
Expand All @@ -303,7 +303,7 @@ impl UnstakeState {
Ok(0)
}

fn scoring_unstake(&self, current_lamports: u64, target_lamports: u64) -> Result<u64> {
pub fn scoring_unstake(&self, current_lamports: u64, target_lamports: u64) -> Result<u64> {
// If there are additional lamports to unstake on this validator and the total unstaked lamports is below the cap, destake to the target
if self.scoring_unstake_total < self.scoring_unstake_cap {
let lamports_above_target = current_lamports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ use crate::events::AutoRemoveValidatorEvent;
use crate::state::Config;
use crate::utils::{
deserialize_stake_pool, get_stake_pool_address, get_validator_stake_info_at_index,
remove_validator_check,
remove_validator_check, stake_is_inactive_without_history, stake_is_usable_by_pool,
};
use crate::StewardStateAccount;
use anchor_lang::prelude::*;
use anchor_lang::solana_program::{clock::Epoch, program::invoke_signed, stake, sysvar, vote};
use anchor_lang::solana_program::{program::invoke_signed, stake, sysvar, vote};
use spl_pod::solana_program::borsh1::try_from_slice_unchecked;
use spl_pod::solana_program::stake::state::StakeStateV2;
use spl_stake_pool::state::StakeStatus;
Expand Down Expand Up @@ -243,12 +243,14 @@ pub fn handler(ctx: Context<AutoRemoveValidator>, validator_list_index: usize) -
}
}
StakeStatus::ReadyForRemoval => {
// Should never happen but this is logical action
marked_for_immediate_removal = true;
state_account
.state
.mark_validator_for_immediate_removal(validator_list_index)?;
}
StakeStatus::DeactivatingAll | StakeStatus::DeactivatingTransient => {
// DeactivatingTransient should not be possible but this is the logical action
marked_for_immediate_removal = false;
state_account
.state
Expand All @@ -267,23 +269,3 @@ pub fn handler(ctx: Context<AutoRemoveValidator>, validator_list_index: usize) -

Ok(())
}

// CHECKS FROM spl_stake_pool::processor::update_validator_list_balance

/// Checks if a stake account can be managed by the pool
fn stake_is_usable_by_pool(
meta: &stake::state::Meta,
expected_authority: &Pubkey,
expected_lockup: &stake::state::Lockup,
) -> bool {
meta.authorized.staker == *expected_authority
&& meta.authorized.withdrawer == *expected_authority
&& meta.lockup == *expected_lockup
}

/// Checks if a stake account is active, without taking into account cooldowns
fn stake_is_inactive_without_history(stake: &stake::state::Stake, epoch: Epoch) -> bool {
stake.delegation.deactivation_epoch < epoch
|| (stake.delegation.activation_epoch == epoch
&& stake.delegation.deactivation_epoch == epoch)
}
2 changes: 1 addition & 1 deletion programs/steward/src/instructions/epoch_maintenance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub fn handler(
if let Some(validator_index_to_remove) = validator_index_to_remove {
state_account
.state
.remove_validator(validator_index_to_remove, validators_in_list)?;
.remove_validator(validator_index_to_remove)?;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub fn handler(

state_account
.state
.remove_validator(validator_index_to_remove, validators_in_list)?;
.remove_validator(validator_index_to_remove)?;

Ok(())
}
33 changes: 30 additions & 3 deletions programs/steward/src/instructions/spl_passthrough.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ use crate::errors::StewardError;
use crate::state::Config;
use crate::utils::{
deserialize_stake_pool, get_config_admin, get_stake_pool_address,
get_validator_stake_info_at_index,
get_validator_stake_info_at_index, stake_is_inactive_without_history, stake_is_usable_by_pool,
};
use crate::StewardStateAccount;
use anchor_lang::prelude::*;
use anchor_lang::solana_program::{program::invoke_signed, stake, sysvar, vote};
use spl_pod::solana_program::borsh1::try_from_slice_unchecked;
use spl_pod::solana_program::stake::state::StakeStateV2;
use spl_stake_pool::find_stake_program_address;
use spl_stake_pool::instruction::PreferredValidatorType;
use spl_stake_pool::state::{StakeStatus, ValidatorListHeader};
Expand Down Expand Up @@ -180,9 +182,9 @@ pub fn remove_validator_from_pool_handler(
ctx: Context<RemoveValidatorFromPool>,
validator_list_index: usize,
) -> Result<()> {
let epoch = Clock::get()?.epoch;
{
let state_account = ctx.accounts.state_account.load_mut()?;
let epoch = Clock::get()?.epoch;

// Should not be able to remove a validator if update is not complete
require!(
Expand Down Expand Up @@ -245,12 +247,37 @@ pub fn remove_validator_from_pool_handler(

let stake_status = StakeStatus::try_from(validator_stake_info.status)?;

let stake_pool = deserialize_stake_pool(&ctx.accounts.stake_pool)?;

match stake_status {
StakeStatus::Active => {
// Should never happen
return Err(StewardError::ValidatorMarkedActive.into());
}
StakeStatus::DeactivatingValidator | StakeStatus::ReadyForRemoval => {
StakeStatus::DeactivatingValidator => {
let stake_account_data = &mut ctx.accounts.stake_account.data.borrow_mut();
let (meta, stake) =
match try_from_slice_unchecked::<StakeStateV2>(stake_account_data)? {
StakeStateV2::Stake(meta, stake, _stake_flags) => (meta, stake),
_ => return Err(StewardError::StakeStateIsNotStake.into()),
};

if stake_is_usable_by_pool(
&meta,
ctx.accounts.withdraw_authority.key,
&stake_pool.lockup,
) && stake_is_inactive_without_history(&stake, epoch)
{
state_account
.state
.mark_validator_for_immediate_removal(validator_list_index)?;
} else {
state_account
.state
.mark_validator_for_removal(validator_list_index)?;
}
}
StakeStatus::ReadyForRemoval => {
coachchucksol marked this conversation as resolved.
Show resolved Hide resolved
state_account
.state
.mark_validator_for_immediate_removal(validator_list_index)?;
Expand Down
21 changes: 14 additions & 7 deletions programs/steward/src/state/steward_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ impl StewardState {
}

/// Update internal state when a validator is removed from the pool
pub fn remove_validator(&mut self, index: usize, validator_list_len: usize) -> Result<()> {
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)?;

Expand All @@ -474,8 +474,16 @@ impl StewardState {
StewardError::ValidatorNotMarkedForRemoval
);

let num_pool_validators = self.num_pool_validators as usize;
let num_pool_validators_plus_added = num_pool_validators + self.validators_added as usize;

require!(
index < num_pool_validators_plus_added,
StewardError::ValidatorIndexOutOfBounds
);

// If the validator was marked for removal in the current cycle, decrement validators_added
if index >= self.num_pool_validators as usize {
if index >= num_pool_validators {
self.validators_added = self
.validators_added
.checked_sub(1)
Expand All @@ -487,8 +495,6 @@ impl StewardState {
.ok_or(StewardError::ArithmeticError)?;
}

let num_pool_validators = self.num_pool_validators as usize;

// Shift all validator state to the left
for i in index..num_pool_validators {
let next_i = i.checked_add(1).ok_or(StewardError::ArithmeticError)?;
Expand All @@ -502,7 +508,7 @@ impl StewardState {
}

// For state that can be valid past num_pool_validators, we still need to shift the values
for i in index..validator_list_len {
for i in index..num_pool_validators_plus_added {
let next_i = i.checked_add(1).ok_or(StewardError::ArithmeticError)?;
self.validators_to_remove
.set(i, self.validators_to_remove.get(next_i)?)?;
Expand Down Expand Up @@ -556,9 +562,10 @@ impl StewardState {
self.delegations[num_pool_validators] = Delegation::default();
self.instant_unstake.set(num_pool_validators, false)?;
self.progress.set(num_pool_validators, false)?;
self.validators_to_remove.set(validator_list_len, false)?;
self.validators_to_remove
.set(num_pool_validators_plus_added, false)?;
self.validators_for_immediate_removal
.set(validator_list_len, false)?;
.set(num_pool_validators_plus_added, false)?;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the changes here to properly shift indices even if validator_list_len < number of steward-tracked validators


Ok(())
}
Expand Down
27 changes: 26 additions & 1 deletion programs/steward/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ use std::ops::{Deref, Not};
use anchor_lang::idl::types::*;
use anchor_lang::prelude::*;
use borsh::{BorshDeserialize, BorshSerialize};
use spl_pod::{bytemuck::pod_from_bytes, primitives::PodU64, solana_program::program_pack::Pack};
use spl_pod::solana_program::clock::Epoch;
use spl_pod::{
bytemuck::pod_from_bytes,
primitives::PodU64,
solana_program::{program_pack::Pack, stake},
};
use spl_stake_pool::{
big_vec::BigVec,
state::{StakeStatus, ValidatorListHeader, ValidatorStakeInfo},
Expand Down Expand Up @@ -312,6 +317,26 @@ pub fn check_validator_list_has_stake_status_other_than(
Ok(false)
}

/// Checks if a stake account can be managed by the pool
/// FROM spl_stake_pool::processor::update_validator_list_balance
pub fn stake_is_usable_by_pool(
meta: &stake::state::Meta,
expected_authority: &Pubkey,
expected_lockup: &stake::state::Lockup,
) -> bool {
meta.authorized.staker == *expected_authority
&& meta.authorized.withdrawer == *expected_authority
&& meta.lockup == *expected_lockup
}

/// Checks if a stake account is active, without taking into account cooldowns
/// FROM spl_stake_pool::processor::update_validator_list_balance
pub fn stake_is_inactive_without_history(stake: &stake::state::Stake, epoch: Epoch) -> bool {
stake.delegation.deactivation_epoch < epoch
|| (stake.delegation.activation_epoch == epoch
&& stake.delegation.deactivation_epoch == epoch)
}

pub fn get_validator_list_length(validator_list_account_info: &AccountInfo) -> Result<usize> {
let mut validator_list_data = validator_list_account_info.try_borrow_mut_data()?;
let (header, validator_list) = ValidatorListHeader::deserialize_vec(&mut validator_list_data)?;
Expand Down
2 changes: 1 addition & 1 deletion run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ cargo build-sbf --manifest-path programs/steward/Cargo.toml;
cargo build-sbf --manifest-path programs/validator-history/Cargo.toml;

# Run all tests
SBF_OUT_DIR=$(pwd)/target/deploy RUST_MIN_STACK=5000000 cargo test --package tests --all-features --color auto
SBF_OUT_DIR=$(pwd)/target/deploy RUST_MIN_STACK=10000000 cargo test --package tests --all-features --color auto

Loading
Loading