PoS: delay slash validator set update when next epoch is done in TM

.changelog/unreleased/bug-fixes/1582-slash-validator-delay-set-update.md

@@ -0,0 +1,3 @@
+- PoS: Ensure that when a validator is slashed, it gets removed from
+  validator set in the same epoch in Namada state as in CometBFT's state.
+  ([\#1582](https://github.com/anoma/namada/pull/1582))

apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -107,8 +107,10 @@ where
         }
 
         // Invariant: This has to be applied after
-        // `copy_validator_sets_and_positions` if we're starting a new epoch
+        // `copy_validator_sets_and_positions` and before `self.update_epoch`.
         self.record_slashes_from_evidence();
+        // Invariant: This has to be applied after
+        // `copy_validator_sets_and_positions` if we're starting a new epoch
         if new_epoch {
             self.process_slashes();
         }

apps/src/lib/node/ledger/shell/mod.rs

@@ -31,6 +31,7 @@ use namada::ledger::pos::namada_proof_of_stake::types::{
 use namada::ledger::storage::write_log::WriteLog;
 use namada::ledger::storage::{
     DBIter, Sha256Hasher, Storage, StorageHasher, TempWlStorage, WlStorage, DB,
+    EPOCH_SWITCH_BLOCKS_DELAY,
 };
 use namada::ledger::storage_api::{self, StorageRead, StorageWrite};
 use namada::ledger::{ibc, pos, protocol, replay_protection};
@@ -591,9 +592,28 @@ where
                             continue;
                         }
                     };
+                // Check if we're gonna switch to a new epoch after a delay
+                let validator_set_update_epoch = if let Some(delay) =
+                    self.wl_storage.storage.update_epoch_blocks_delay
+                {
+                    if delay == EPOCH_SWITCH_BLOCKS_DELAY {
+                        // If we're about to update validator sets for the
+                        // upcoming epoch, we can still remove the validator
+                        current_epoch.next()
+                    } else {
+                        // If we're waiting to switch to a new epoch, it's too
+                        // late to update validator sets
+                        // on the next epoch, so we need to
+                        // wait for the one after.
+                        current_epoch.next().next()
+                    }
+                } else {
+                    current_epoch.next()
+                };
                 tracing::info!(
                     "Slashing {} for {} in epoch {}, block height {} (current \
-                     epoch = {})",
+                     epoch = {}, validator set update epoch = \
+                     {validator_set_update_epoch})",
                     validator,
                     slash_type,
                     evidence_epoch,
@@ -608,6 +628,7 @@ where
                     evidence_height,
                     slash_type,
                     &validator,
+                    validator_set_update_epoch,
                 ) {
                     tracing::error!("Error in slashing: {}", err);
                 }

proof_of_stake/src/lib.rs

@@ -2974,6 +2974,7 @@ where
 /// Record a slash for a misbehavior that has been received from Tendermint and
 /// then jail the validator, removing it from the validator set. The slash rate
 /// will be computed at a later epoch.
+#[allow(clippy::too_many_arguments)]
 pub fn slash<S>(
     storage: &mut S,
     params: &PosParams,
@@ -2982,6 +2983,7 @@ pub fn slash<S>(
     evidence_block_height: impl Into<u64>,
     slash_type: SlashType,
     validator: &Address,
+    validator_set_update_epoch: Epoch,
 ) -> storage_api::Result<()>
 where
     S: StorageRead + StorageWrite,
@@ -3017,7 +3019,7 @@ where
     // Remove the validator from the set starting at the next epoch and up thru
     // the pipeline epoch.
     for epoch in
-        Epoch::iter_bounds_inclusive(current_epoch.next(), pipeline_epoch)
+        Epoch::iter_bounds_inclusive(validator_set_update_epoch, pipeline_epoch)
     {
         let prev_state = validator_state_handle(validator)
             .get(storage, epoch, params)?

proof_of_stake/src/tests.rs

@@ -941,6 +941,7 @@ fn test_slashes_with_unbonding_aux(
         evidence_block_height,
         slash_0_type,
         val_addr,
+        current_epoch.next(),
     )
     .unwrap();
 
@@ -978,6 +979,7 @@ fn test_slashes_with_unbonding_aux(
         evidence_block_height,
         slash_1_type,
         val_addr,
+        current_epoch.next(),
     )
     .unwrap();
 

proof_of_stake/src/tests/state_machine.rs

@@ -421,6 +421,7 @@ impl StateMachineTest for ConcretePosState {
                     height,
                     slash_type,
                     &address,
+                    current_epoch.next(),
                 )
                 .unwrap();