From 31a7aa3ded362844ba37d51e37a908160cfcfc7a Mon Sep 17 00:00:00 2001
From: Simon Noetzlin <simon.ntz@gmail.com>
Date: Wed, 1 May 2024 11:05:58 +0200
Subject: [PATCH] fix!: fix slashing in PSS (#1838)

* drop slash packet for opted-out validators before updating slash meter

* fix integration test

* fix ut

* update UT
---
 tests/integration/slashing.go       |  9 +++++++++
 x/ccv/provider/keeper/relay.go      | 19 ++++++++++---------
 x/ccv/provider/keeper/relay_test.go | 27 ++++++++++-----------------
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index e7f585f756..19805a49ad 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -406,6 +406,15 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	// Check expected behavior for handling SlashPackets for downtime infractions
 	slashPacketData.Infraction = stakingtypes.Infraction_INFRACTION_DOWNTIME
 
+	// Expect packet to be handled if the validator didn't opt in
+	ackResult, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().NoError(err, "no error expected")
+	suite.Require().Equal(ccv.SlashPacketHandledResult, ackResult, "expected successful ack")
+
+	providerKeeper.SetConsumerValidator(ctx, firstBundle.Chain.ChainID, providertypes.ConsumerValidator{
+		ProviderConsAddr: validAddress,
+	})
+
 	// Expect the packet to bounce if the slash meter is negative
 	providerKeeper.SetSlashMeter(ctx, sdk.NewInt(-1))
 	ackResult, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go
index 35c98aa088..5dde197289 100644
--- a/x/ccv/provider/keeper/relay.go
+++ b/x/ccv/provider/keeper/relay.go
@@ -338,6 +338,16 @@ func (k Keeper) OnRecvSlashPacket(
 		return ccv.V1Result, nil
 	}
 
+	// Check that the validator belongs to the consumer chain valset
+	if !k.IsConsumerValidator(ctx, chainID, providerConsAddr) {
+		k.Logger(ctx).Error("cannot jail validator %s that does not belong to consumer %s valset",
+			providerConsAddr.String(), chainID)
+		// drop packet but return a slash ack so that the consumer can send another slash packet
+		k.AppendSlashAck(ctx, chainID, consumerConsAddr.String())
+
+		return ccv.SlashPacketHandledResult, nil
+	}
+
 	meter := k.GetSlashMeter(ctx)
 	// Return bounce ack if meter is negative in value
 	if meter.IsNegative() {
@@ -401,15 +411,6 @@ func (k Keeper) HandleSlashPacket(ctx sdk.Context, chainID string, data ccv.Slas
 		"infractionType", data.Infraction,
 	)
 
-	// Check that the validator belongs to the consumer chain valset
-	if !k.IsConsumerValidator(ctx, chainID, providerConsAddr) {
-		k.Logger(ctx).Error("cannot jail validator %s that does not belong to consumer %s valset",
-			providerConsAddr.String(), chainID)
-		// drop packet but return a slash ack so that the consumer can send another slash packet
-		k.AppendSlashAck(ctx, chainID, consumerConsAddr.String())
-		return
-	}
-
 	// Obtain validator from staking keeper
 	validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerConsAddr.ToSdkConsAddr())
 
diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go
index 5afbf70248..b300916a2e 100644
--- a/x/ccv/provider/keeper/relay_test.go
+++ b/x/ccv/provider/keeper/relay_test.go
@@ -125,12 +125,22 @@ func TestOnRecvDowntimeSlashPacket(t *testing.T) {
 	// Set a block height for the valset update id in the generated packet data
 	providerKeeper.SetValsetUpdateBlockHeight(ctx, packetData.ValsetUpdateId, uint64(15))
 
+	// Set consumer validator
+	providerKeeper.SetConsumerValidator(ctx, "chain-1", providertypes.ConsumerValidator{
+		ProviderConsAddr: packetData.Validator.Address,
+	})
+
 	// Set slash meter to negative value and assert a bounce ack is returned
 	providerKeeper.SetSlashMeter(ctx, math.NewInt(-5))
 	ackResult, err := executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
 	require.Equal(t, ccv.SlashPacketBouncedResult, ackResult)
 	require.NoError(t, err)
 
+	// Set consumer validator
+	providerKeeper.SetConsumerValidator(ctx, "chain-2", providertypes.ConsumerValidator{
+		ProviderConsAddr: packetData.Validator.Address,
+	})
+
 	// Also bounced for chain-2
 	ackResult, err = executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-2", 2, packetData)
 	require.Equal(t, ccv.SlashPacketBouncedResult, ackResult)
@@ -298,8 +308,6 @@ func TestHandleSlashPacket(t *testing.T) {
 
 	providerConsAddr := cryptotestutil.NewCryptoIdentityFromIntSeed(7842334).ProviderConsAddress()
 	consumerConsAddr := cryptotestutil.NewCryptoIdentityFromIntSeed(784987634).ConsumerConsAddress()
-	// this "dummy" consensus address won't be stored on the provider states
-	dummyConsAddr := cryptotestutil.NewCryptoIdentityFromIntSeed(784987639).ConsumerConsAddress()
 
 	testCases := []struct {
 		name       string
@@ -309,21 +317,6 @@ func TestHandleSlashPacket(t *testing.T) {
 		expectedSlashAcksLen                int
 		expectedSlashAckConsumerConsAddress types.ConsumerConsAddress
 	}{
-		{
-			"validator isn't a consumer validator",
-			ccv.SlashPacketData{
-				Validator:      abci.Validator{Address: dummyConsAddr.ToSdkConsAddr()},
-				ValsetUpdateId: validVscID,
-				Infraction:     stakingtypes.Infraction_INFRACTION_DOWNTIME,
-			},
-			func(ctx sdk.Context, mocks testkeeper.MockedKeepers,
-				expectedPacketData ccv.SlashPacketData,
-			) []*gomock.Call {
-				return []*gomock.Call{}
-			},
-			1,
-			dummyConsAddr,
-		},
 		{
 			"unfound validator",
 			ccv.SlashPacketData{