From 3bc3ae735c3812fde75ae5dfba77a7b564e748f3 Mon Sep 17 00:00:00 2001 From: Evan B Date: Mon, 3 Jun 2024 20:59:41 -0400 Subject: [PATCH] Add historical_commission_threshold and fixes instant unstaking bug (#42) * Adds `historical_commission_threshold` as described in the Steward README. Requires a compiler feature when building the main image to specify VALIDATOR_HISTORY_FIRST_RELIABLE_EPOCH for the given network this is running on. * Fixes a bug in which validators with stake that are not in the delegation set would not be checked for instant unstake status, which was wrong because we still need to be able to instant unstake these validators if they commission rug or go delinquent. * Re-enables testing of Steward tests in CI. --- .github/workflows/build.yaml | 20 +++++- programs/steward/src/constants.rs | 3 + programs/steward/src/score.rs | 26 ++++++- programs/steward/src/state/parameters.rs | 17 ++++- programs/steward/src/state/steward_state.rs | 5 +- tests/src/steward_fixtures.rs | 3 +- tests/tests/steward/test_algorithms.rs | 77 +++++++++++++++++++++ tests/tests/steward/test_integration.rs | 9 ++- tests/tests/steward/test_state_methods.rs | 71 +++++++++++++++++++ 9 files changed, 218 insertions(+), 13 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 7519e824..6dc936b8 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -81,7 +81,7 @@ jobs: run: echo "/home/runner/.local/share/solana/install/active_release/bin" >> $GITHUB_PATH # build the program and IDL; exit if error - - run: anchor build --no-idl --program-name validator_history + - run: anchor build --no-idl # - name: Check for diff on IDL # run: git diff --exit-code @@ -94,6 +94,12 @@ jobs: with: name: validator_history.so path: target/deploy/validator_history.so + + - name: Upload jito_steward.so + uses: actions/upload-artifact@v4 + with: + name: jito_steward.so + path: target/deploy/jito_steward.so # - name: Upload IDL # uses: actions/upload-artifact@v4 # with: @@ -115,13 +121,23 @@ jobs: with: name: validator_history.so path: target/deploy/ + - uses: actions/download-artifact@v4 + with: + name: jito_steward.so + path: target/deploy/ - name: cargo test - run: cargo test --package tests --test mod validator_history --all-features --color auto + run: cargo test --package tests --all-features --color auto -- --skip steward::test_state_methods shell: bash env: RUST_LOG: trace SBF_OUT_DIR: ${{ github.workspace }}/target/deploy RUST_MIN_STACK: 5000000 + - name: cargo test steward::test_state_methods + run: cargo test --package tests --test mod steward::test_state_methods + shell: bash + env: + RUST_LOG: trace + RUST_MIN_STACK: 5000000 # release only runs on tagged commits # it should wait for all the other steps to finish, to ensure releases are the highest quality diff --git a/programs/steward/src/constants.rs b/programs/steward/src/constants.rs index 8cd71de1..8957cf82 100644 --- a/programs/steward/src/constants.rs +++ b/programs/steward/src/constants.rs @@ -11,4 +11,7 @@ pub const EPOCH_PROGRESS_MAX: f64 = 0.99; pub const NUM_EPOCHS_BETWEEN_SCORING_MAX: u64 = 100; // Cannot score validators in under 100 slots, to submit 1 instruction per validator pub const COMPUTE_SCORE_SLOT_RANGE_MIN: usize = 100; +#[cfg(feature = "mainnet-beta")] pub const VALIDATOR_HISTORY_FIRST_RELIABLE_EPOCH: usize = 520; +#[cfg(not(feature = "mainnet-beta"))] +pub const VALIDATOR_HISTORY_FIRST_RELIABLE_EPOCH: usize = 0; diff --git a/programs/steward/src/score.rs b/programs/steward/src/score.rs index 3fec0d89..762344aa 100644 --- a/programs/steward/src/score.rs +++ b/programs/steward/src/score.rs @@ -4,7 +4,7 @@ use anchor_lang::{ use validator_history::{ClusterHistory, ValidatorHistory}; use crate::{ - constants::{BASIS_POINTS_MAX, COMMISSION_MAX}, + constants::{BASIS_POINTS_MAX, COMMISSION_MAX, VALIDATOR_HISTORY_FIRST_RELIABLE_EPOCH}, errors::StewardError::{self, ArithmeticError}, Config, }; @@ -33,9 +33,12 @@ pub struct ScoreComponents { /// If validator has a mev commission in the last 10 epochs, score is 1.0, else 0.0 pub running_jito_score: f64, - /// If max commission in commission_range epochs is less than threshold, score is 1.0, else 0.0 + /// If max commission in commission_range epochs is less than commission_threshold, score is 1.0, else 0.0 pub commission_score: f64, + /// If max commission in all validator history epochs is less than historical_commission_threshold, score is 1.0, else 0.0 + pub historical_commission_score: f64, + /// Average vote credits in last epoch_credits_range epochs / average blocks in last epoch_credits_range epochs /// Excluding current epoch pub vote_credits_ratio: f64, @@ -140,6 +143,23 @@ pub fn validator_score( }; let commission = commission_u8 as f64 / COMMISSION_MAX as f64; + /////// Historical Commission /////// + + let historical_commission_max = validator + .history + .commission_range(VALIDATOR_HISTORY_FIRST_RELIABLE_EPOCH as u16, current_epoch) + .iter() + .filter_map(|&i| i) + .max() + .unwrap_or(0); + + let historical_commission_score: f64 = + if historical_commission_max <= params.historical_commission_threshold { + 1.0 + } else { + 0.0 + }; + /////// Superminority /////// /* If epoch credits exist, we expect the validator to have a superminority flag set. If not, scoring fails and we wait for @@ -186,6 +206,7 @@ pub fn validator_score( let score = mev_commission_score * commission_score + * historical_commission_score * blacklisted_score * superminority_score * delinquency_score @@ -201,6 +222,7 @@ pub fn validator_score( delinquency_score, running_jito_score, commission_score, + historical_commission_score, vote_credits_ratio: average_vote_credits / average_blocks, vote_account: validator.vote_account, epoch: current_epoch, diff --git a/programs/steward/src/state/parameters.rs b/programs/steward/src/state/parameters.rs index 82c69e7d..8d573899 100644 --- a/programs/steward/src/state/parameters.rs +++ b/programs/steward/src/state/parameters.rs @@ -20,6 +20,7 @@ pub struct UpdateParametersArgs { pub instant_unstake_delinquency_threshold_ratio: Option, pub mev_commission_bps_threshold: Option, pub commission_threshold: Option, + pub historical_commission_threshold: Option, // Delegation parameters pub num_delegation_validators: Option, pub scoring_unstake_cap_bps: Option, @@ -56,12 +57,15 @@ pub struct Parameters { /// Proportion of delinquent slots to total slots to trigger instant unstake pub instant_unstake_delinquency_threshold_ratio: f64, - /// Highest commission rate allowed in percent + /// Highest commission rate allowed in commission_range epochs, in percent pub commission_threshold: u8, + /// Highest commission rate allowed in tracked history + pub historical_commission_threshold: u8, + /// Required so that the struct is 8-byte aligned /// https://doc.rust-lang.org/reference/type-layout.html#reprc-structs - pub padding0: [u8; 7], + pub padding0: [u8; 6], /////// Delegation parameters /////// /// Number of validators to delegate to @@ -113,6 +117,7 @@ impl Parameters { instant_unstake_delinquency_threshold_ratio, mev_commission_bps_threshold, commission_threshold, + historical_commission_threshold, num_delegation_validators, scoring_unstake_cap_bps, instant_unstake_cap_bps, @@ -159,6 +164,10 @@ impl Parameters { new_parameters.commission_threshold = commission_threshold; } + if let Some(historical_commission_threshold) = historical_commission_threshold { + new_parameters.historical_commission_threshold = historical_commission_threshold; + } + if let Some(num_delegation_validators) = num_delegation_validators { new_parameters.num_delegation_validators = num_delegation_validators; } @@ -244,6 +253,10 @@ impl Parameters { return Err(StewardError::InvalidParameterValue.into()); } + if self.historical_commission_threshold > COMMISSION_MAX { + return Err(StewardError::InvalidParameterValue.into()); + } + if self.num_delegation_validators == 0 || self.num_delegation_validators > MAX_VALIDATORS as u32 { diff --git a/programs/steward/src/state/steward_state.rs b/programs/steward/src/state/steward_state.rs index f41e39c1..60c975d0 100644 --- a/programs/steward/src/state/steward_state.rs +++ b/programs/steward/src/state/steward_state.rs @@ -636,9 +636,8 @@ impl StewardState { clock.epoch as u16, )?; emit!(instant_unstake_result); - if self.delegations[index].numerator > 0 && instant_unstake_result.instant_unstake { - self.instant_unstake.set(index, true)?; - } + self.instant_unstake + .set(index, instant_unstake_result.instant_unstake)?; self.progress.set(index, true)?; return Ok(()); } diff --git a/tests/src/steward_fixtures.rs b/tests/src/steward_fixtures.rs index f76d0035..72ce9014 100644 --- a/tests/src/steward_fixtures.rs +++ b/tests/src/steward_fixtures.rs @@ -762,7 +762,8 @@ impl Default for StateMachineFixtures { scoring_delinquency_threshold_ratio: 0.875, instant_unstake_delinquency_threshold_ratio: 0.1, commission_threshold: 10, - padding0: [0; 7], + historical_commission_threshold: 10, + padding0: [0; 6], num_delegation_validators: 3, scoring_unstake_cap_bps: 1000, instant_unstake_cap_bps: 1000, diff --git a/tests/tests/steward/test_algorithms.rs b/tests/tests/steward/test_algorithms.rs index c07785c7..c5c5ba6e 100644 --- a/tests/tests/steward/test_algorithms.rs +++ b/tests/tests/steward/test_algorithms.rs @@ -54,6 +54,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: good_validator.vote_account, epoch: current_epoch as u16 } @@ -83,6 +84,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -110,6 +112,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -136,6 +139,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -167,6 +171,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -196,6 +201,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -226,6 +232,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -257,6 +264,7 @@ fn test_compute_score() { running_jito_score: 0.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -285,6 +293,71 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 0.0, + historical_commission_score: 0.0, + vote_account: validator.vote_account, + epoch: current_epoch as u16 + } + ); + + //// historical commission //// + let mut validator = good_validator; + let mut config = default_fixture.config; + + config.parameters.historical_commission_threshold = 15; + config.parameters.commission_threshold = 10; + config.parameters.commission_range = 10; + + // commission above regular threshold, below historical threshold, outside of regular threshold window + validator.history.arr[0].commission = 14; + + let components = validator_score( + &validator, + validator.index as usize, + &cluster_history, + &config, + current_epoch as u16, + ) + .unwrap(); + assert_eq!( + components, + ScoreComponents { + score: 1.0, + yield_score: 1.0, + mev_commission_score: 1.0, + blacklisted_score: 1.0, + superminority_score: 1.0, + delinquency_score: 1.0, + running_jito_score: 1.0, + vote_credits_ratio: 1.0, + commission_score: 1.0, + historical_commission_score: 1.0, + vote_account: validator.vote_account, + epoch: current_epoch as u16 + } + ); + + validator.history.arr[0].commission = 16; + let components = validator_score( + &validator, + validator.index as usize, + &cluster_history, + &config, + current_epoch as u16, + ) + .unwrap(); + assert_eq!( + components, + ScoreComponents { + score: 0.0, + yield_score: 1.0, + mev_commission_score: 1.0, + blacklisted_score: 1.0, + superminority_score: 1.0, + delinquency_score: 1.0, + running_jito_score: 1.0, + vote_credits_ratio: 1.0, + commission_score: 1.0, + historical_commission_score: 0.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -318,6 +391,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 0.88, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -346,6 +420,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 0.95, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -379,6 +454,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 0.9, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } @@ -410,6 +486,7 @@ fn test_compute_score() { running_jito_score: 1.0, vote_credits_ratio: 1.0, commission_score: 1.0, + historical_commission_score: 1.0, vote_account: validator.vote_account, epoch: current_epoch as u16 } diff --git a/tests/tests/steward/test_integration.rs b/tests/tests/steward/test_integration.rs index cd536aec..ea3ded46 100644 --- a/tests/tests/steward/test_integration.rs +++ b/tests/tests/steward/test_integration.rs @@ -304,7 +304,8 @@ async fn test_compute_scores() { let tx = Transaction::new_signed_with_payer( &[ // Only high because we are averaging 512 epochs - ComputeBudgetInstruction::set_compute_unit_limit(1_400_000), + ComputeBudgetInstruction::set_compute_unit_limit(600_000), + ComputeBudgetInstruction::request_heap_frame(128 * 1024), compute_scores_ix.clone(), ], Some(&fixture.keypair.pubkey()), @@ -349,7 +350,8 @@ async fn test_compute_scores() { let tx = Transaction::new_signed_with_payer( &[ - ComputeBudgetInstruction::set_compute_unit_limit(1_400_000), + ComputeBudgetInstruction::set_compute_unit_limit(600_000), + ComputeBudgetInstruction::request_heap_frame(128 * 1024), compute_scores_ix.clone(), ], Some(&fixture.keypair.pubkey()), @@ -377,7 +379,8 @@ async fn test_compute_scores() { let blockhash = fixture.get_latest_blockhash().await; let tx = Transaction::new_signed_with_payer( &[ - ComputeBudgetInstruction::set_compute_unit_limit(1_400_000), + ComputeBudgetInstruction::set_compute_unit_limit(600_000), + ComputeBudgetInstruction::request_heap_frame(128 * 1024), compute_scores_ix.clone(), ], Some(&fixture.keypair.pubkey()), diff --git a/tests/tests/steward/test_state_methods.rs b/tests/tests/steward/test_state_methods.rs index 62e7dde5..6ec24a1e 100644 --- a/tests/tests/steward/test_state_methods.rs +++ b/tests/tests/steward/test_state_methods.rs @@ -447,6 +447,23 @@ fn test_compute_instant_unstake_success() { .instant_unstake .get(validators[0].index as usize) .unwrap()); + + // Instant unstakeable validator with no delegation amount + state.delegations[validators[0].index as usize] = Delegation::new(0, 1); + state.instant_unstake.reset(); + let res = state.compute_instant_unstake( + clock, + epoch_schedule, + &validators[0], + validators[0].index as usize, + cluster_history, + config, + ); + assert!(res.is_ok()); + assert!(state + .instant_unstake + .get(validators[0].index as usize) + .unwrap()); } #[test] @@ -554,6 +571,60 @@ fn test_rebalance() { _ => panic!("Expected RebalanceType::Decrease"), } + // Instant unstake validator, but no delegation, so other delegations are not affected + // Same scenario as above but out-of-band validator + state.delegations[0..3].copy_from_slice(&[ + Delegation::new(1, 2), + Delegation::new(0, 1), + Delegation::new(1, 2), + ]); + state.scores[0..3].copy_from_slice(&[1_000_000_000, 500_000_000, 0]); + state.sorted_score_indices[0..3].copy_from_slice(&[0, 1, 2]); + state.sorted_yield_score_indices[0..3].copy_from_slice(&[0, 1, 2]); + // Second validator is instant unstakeable + state.instant_unstake.set(1, true).unwrap(); + state.validator_lamport_balances[1] = 1000 * LAMPORTS_PER_SOL; + + // Validator index 0: 1000 SOL, 1 score, 1 delegation -> Keeps its stake + // Validator index 1: 1000 SOL, 0.5 score, 0 delegation, -> Decrease stake, from "instant unstake" category, and set delegation to 0 + // Validator index 2: 1000 SOL, 0 score, 0 delegation -> Decrease stake, from "regular unstake" category + + let res = state.rebalance( + fixtures.current_epoch, + 1, + &validator_list_bigvec, + 4000 * LAMPORTS_PER_SOL, + 1000 * LAMPORTS_PER_SOL, + 0, + 0, + &fixtures.config.parameters, + ); + assert!(res.is_ok()); + match res.unwrap() { + RebalanceType::Decrease(decrease_components) => { + assert_eq!( + decrease_components.total_unstake_lamports, + 1000 * LAMPORTS_PER_SOL + ); + assert_eq!( + decrease_components.instant_unstake_lamports, + 1000 * LAMPORTS_PER_SOL + ); + assert_eq!(decrease_components.scoring_unstake_lamports, 0); + assert_eq!(decrease_components.stake_deposit_unstake_lamports, 0); + + assert!( + state.delegations[0..3] + == [ + Delegation::new(1, 2), + Delegation::new(0, 1), + Delegation::new(1, 2) + ] + ); + } + _ => panic!("Expected RebalanceType::Decrease"), + } + // Decrease, instant unstake on the last eligible validator state.instant_unstake_total = 0; state.scoring_unstake_total = 0;