From 59176f822205ecc8de64a6642db9e1ace89b77b9 Mon Sep 17 00:00:00 2001 From: Evan B Date: Mon, 16 Sep 2024 11:11:37 -0400 Subject: [PATCH] Fix for removed validator past boundary (#84) Problem: A bug was found where validators who are added to the pool outside of the boundary of the Steward state, then removed before they begin getting tracked (validators added during cycle N are tracked in steward state in cycle N+1) were not getting properly removed. This caused the state machine to get stuck because the invariant used to keep the state aligned was failing (`num_pool_validators + validators_added - validators_to_remove == validator_list.len()`) Resolution: Make sure we're shifting values for steward state fields that can be relevant past `num_pool_validators` (which is just the validators_to_remove and validators_for_immediate_removal). We also don't want to clear those values at `num_pool_validators` because that might be a relevant value. --- .../src/entries/crank_steward.rs | 2 +- .../src/instructions/epoch_maintenance.rs | 2 +- .../instructions/instant_remove_validator.rs | 2 +- programs/steward/src/state/steward_state.rs | 20 +++++++++---------- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/keepers/stakenet-keeper/src/entries/crank_steward.rs b/keepers/stakenet-keeper/src/entries/crank_steward.rs index ef0d8194..77c967e7 100644 --- a/keepers/stakenet-keeper/src/entries/crank_steward.rs +++ b/keepers/stakenet-keeper/src/entries/crank_steward.rs @@ -266,7 +266,7 @@ async fn _handle_instant_removal_validators( while validators_to_remove.count() != 0 { let mut validator_index_to_remove = None; - for i in 0..num_validators { + for i in 0..all_steward_accounts.validator_list_account.validators.len() as u64 { if validators_to_remove.get(i as usize).map_err(|e| { JitoTransactionError::Custom(format!( "Error fetching bitmask index for immediate removed validator: {}/{} - {}", diff --git a/programs/steward/src/instructions/epoch_maintenance.rs b/programs/steward/src/instructions/epoch_maintenance.rs index 2b3cafe7..893be118 100644 --- a/programs/steward/src/instructions/epoch_maintenance.rs +++ b/programs/steward/src/instructions/epoch_maintenance.rs @@ -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)?; + .remove_validator(validator_index_to_remove, validators_in_list)?; } } diff --git a/programs/steward/src/instructions/instant_remove_validator.rs b/programs/steward/src/instructions/instant_remove_validator.rs index bc4dc1a0..5daadf4f 100644 --- a/programs/steward/src/instructions/instant_remove_validator.rs +++ b/programs/steward/src/instructions/instant_remove_validator.rs @@ -88,7 +88,7 @@ pub fn handler( state_account .state - .remove_validator(validator_index_to_remove)?; + .remove_validator(validator_index_to_remove, validators_in_list)?; Ok(()) } diff --git a/programs/steward/src/state/steward_state.rs b/programs/steward/src/state/steward_state.rs index 6d76a21f..88da95f7 100644 --- a/programs/steward/src/state/steward_state.rs +++ b/programs/steward/src/state/steward_state.rs @@ -460,7 +460,7 @@ impl StewardState { } /// Update internal state when a validator is removed from the pool - pub fn remove_validator(&mut self, index: usize) -> Result<()> { + pub fn remove_validator(&mut self, index: usize, validator_list_len: 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)?; @@ -494,6 +494,11 @@ impl StewardState { self.instant_unstake .set(i, self.instant_unstake.get(next_i)?)?; self.progress.set(i, self.progress.get(next_i)?)?; + } + + // For state that can be valid past num_pool_validators, we still need to shift the values + for i in index..validator_list_len { + let next_i = i.checked_add(1).ok_or(StewardError::ArithmeticError)?; self.validators_to_remove .set(i, self.validators_to_remove.get(next_i)?)?; self.validators_for_immediate_removal @@ -538,7 +543,7 @@ impl StewardState { } // Clear values on empty last index - self.validator_lamport_balances[num_pool_validators] = 0; + self.validator_lamport_balances[num_pool_validators] = LAMPORT_BALANCE_DEFAULT; self.scores[num_pool_validators] = 0; self.yield_scores[num_pool_validators] = 0; self.sorted_score_indices[num_pool_validators] = SORTED_INDEX_DEFAULT; @@ -546,16 +551,9 @@ 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(num_pool_validators, false)?; + self.validators_to_remove.set(validator_list_len, false)?; self.validators_for_immediate_removal - .set(num_pool_validators, false)?; - - if marked_for_regular_removal { - self.validators_to_remove.set(num_pool_validators, false)?; - } else { - self.validators_for_immediate_removal - .set(num_pool_validators, false)?; - } + .set(validator_list_len, false)?; Ok(()) }