From 6b3007393fa98ecdefd806ced9cc7b6f11649e14 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Tue, 25 Apr 2023 22:28:15 +0200
Subject: [PATCH] feat: update the slashing and evidence modules to work with
 ICS (backport #15908) (#15947)

---
 CHANGELOG.md                     |  5 +++
 x/evidence/keeper/infraction.go  | 52 ++++++++++++--------------------
 x/slashing/keeper/infractions.go |  3 --
 3 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76b8d6f082f..309a7f085c0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+## Features
+
+* (x/evidence) [#15908](https://github.com/cosmos/cosmos-sdk/pull/15908) Update the equivocation handler to work with ICS by removing a pubkey check that was performing a no-op for consumer chains.
+* (x/slashing) [#15908](https://github.com/cosmos/cosmos-sdk/pull/15908) Remove the validators' pubkey check in the signature handler in order to work with ICS.
+
 ## Improvements
 
 * (store) [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has.
diff --git a/x/evidence/keeper/infraction.go b/x/evidence/keeper/infraction.go
index 3b6d1704284..f71f967b79d 100644
--- a/x/evidence/keeper/infraction.go
+++ b/x/evidence/keeper/infraction.go
@@ -27,19 +27,29 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi
 	logger := k.Logger(ctx)
 	consAddr := evidence.GetConsensusAddress()
 
-	if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil {
-		// Ignore evidence that cannot be handled.
-		//
-		// NOTE: We used to panic with:
-		// `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`,
-		// but this couples the expectations of the app to both Tendermint and
-		// the simulator.  Both are expected to provide the full range of
-		// allowable but none of the disallowed evidence types.  Instead of
-		// getting this coordination right, it is easier to relax the
-		// constraints and ignore evidence that cannot be handled.
+	validator := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr)
+	if validator == nil || validator.IsUnbonded() {
+		// Defensive: Simulation doesn't take unbonding periods into account, and
+		// CometBFT might break this assumption at some point.
 		return
 	}
 
+	if !validator.GetOperator().Empty() {
+		if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil {
+			// Ignore evidence that cannot be handled.
+			//
+			// NOTE: We used to panic with:
+			// `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`,
+			// but this couples the expectations of the app to both CometBFT and
+			// the simulator.  Both are expected to provide the full range of
+			// allowable but none of the disallowed evidence types.  Instead of
+			// getting this coordination right, it is easier to relax the
+			// constraints and ignore evidence that cannot be handled.
+			logger.Error(fmt.Sprintf("ignore evidence; expected public key for validator %s not found", consAddr))
+			return
+		}
+	}
+
 	// calculate the age of the evidence
 	infractionHeight := evidence.GetHeight()
 	infractionTime := evidence.GetTime()
@@ -64,28 +74,6 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi
 		}
 	}
 
-	validator := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr)
-	if validator == nil || validator.IsUnbonded() {
-		// Defensive: Simulation doesn't take unbonding periods into account, and
-		// Tendermint might break this assumption at some point.
-		return
-	}
-
-	if !validator.GetOperator().Empty() {
-		if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil {
-			// Ignore evidence that cannot be handled.
-			//
-			// NOTE: We used to panic with:
-			// `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`,
-			// but this couples the expectations of the app to both Tendermint and
-			// the simulator.  Both are expected to provide the full range of
-			// allowable but none of the disallowed evidence types.  Instead of
-			// getting this coordination right, it is easier to relax the
-			// constraints and ignore evidence that cannot be handled.
-			return
-		}
-	}
-
 	if ok := k.slashingKeeper.HasValidatorSigningInfo(ctx, consAddr); !ok {
 		panic(fmt.Sprintf("expected signing info for validator %s but not found", consAddr))
 	}
diff --git a/x/slashing/keeper/infractions.go b/x/slashing/keeper/infractions.go
index 17f56d04c6c..b4f473d6a37 100644
--- a/x/slashing/keeper/infractions.go
+++ b/x/slashing/keeper/infractions.go
@@ -16,9 +16,6 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre
 
 	// fetch the validator public key
 	consAddr := sdk.ConsAddress(addr)
-	if _, err := k.GetPubkey(ctx, addr); err != nil {
-		panic(fmt.Sprintf("Validator consensus-address %s not found", consAddr))
-	}
 
 	// don't update missed blocks when validator's jailed
 	if k.sk.IsValidatorJailed(ctx, consAddr) {