From c8e52aa816d2583fc9c147fa453a42a06c13b4a3 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Tue, 28 Nov 2023 11:19:43 +0100
Subject: [PATCH 01/14] validate ValidatorSetChangePacketData

---
 tests/integration/valset_update.go  | 15 ++++----
 x/ccv/consumer/ibc_module.go        | 44 +++++++++++++++++------
 x/ccv/consumer/keeper/relay.go      | 11 +++---
 x/ccv/consumer/keeper/relay_test.go | 55 +++++++++++++++++++++++------
 x/ccv/types/wire.go                 | 13 +++++--
 x/ccv/types/wire_test.go            | 54 +++++++++++++++++++++++++++-
 6 files changed, 154 insertions(+), 38 deletions(-)

diff --git a/tests/integration/valset_update.go b/tests/integration/valset_update.go
index b12066afa8..89236dbd0c 100644
--- a/tests/integration/valset_update.go
+++ b/tests/integration/valset_update.go
@@ -71,9 +71,8 @@ func (suite *CCVTestSuite) TestQueueAndSendVSCMaturedPackets() {
 
 	// send first packet
 	packet := suite.newPacketFromProvider(pd.GetBytes(), 1, suite.path, clienttypes.NewHeight(1, 0), 0)
-	ack := consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
-	suite.Require().NotNil(ack, "OnRecvVSCPacket did not return ack")
-	suite.Require().True(ack.Success(), "OnRecvVSCPacket did not return a Success Acknowledgment")
+	err = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
+	suite.Require().Nil(err, "OnRecvVSCPacket did return non-nil error")
 
 	// increase time
 	incrementTime(suite, time.Hour)
@@ -83,9 +82,8 @@ func (suite *CCVTestSuite) TestQueueAndSendVSCMaturedPackets() {
 	pd.ValsetUpdateId = 2
 	packet.Data = pd.GetBytes()
 	packet.Sequence = 2
-	ack = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
-	suite.Require().NotNil(ack, "OnRecvVSCPacket did not return ack")
-	suite.Require().True(ack.Success(), "OnRecvVSCPacket did not return a Success Acknowledgment")
+	err = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
+	suite.Require().Nil(err, "OnRecvVSCPacket did return non-nil error")
 
 	// increase time
 	incrementTime(suite, 24*time.Hour)
@@ -95,9 +93,8 @@ func (suite *CCVTestSuite) TestQueueAndSendVSCMaturedPackets() {
 	pd.ValsetUpdateId = 3
 	packet.Data = pd.GetBytes()
 	packet.Sequence = 3
-	ack = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
-	suite.Require().NotNil(ack, "OnRecvVSCPacket did not return ack")
-	suite.Require().True(ack.Success(), "OnRecvVSCPacket did not return a Success Acknowledgment")
+	err = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
+	suite.Require().Nil(err, "OnRecvVSCPacket did return non-nil error")
 
 	packetMaturities := consumerKeeper.GetAllPacketMaturityTimes(suite.consumerChain.GetContext())
 
diff --git a/x/ccv/consumer/ibc_module.go b/x/ccv/consumer/ibc_module.go
index 99f0141e64..d8f25b2173 100644
--- a/x/ccv/consumer/ibc_module.go
+++ b/x/ccv/consumer/ibc_module.go
@@ -2,6 +2,7 @@ package consumer
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
@@ -225,25 +226,48 @@ func (am AppModule) OnRecvPacket(
 	packet channeltypes.Packet,
 	_ sdk.AccAddress,
 ) ibcexported.Acknowledgement {
-	var (
-		ack  ibcexported.Acknowledgement
-		data types.ValidatorSetChangePacketData
-	)
+	logger := am.keeper.Logger(ctx)
+	ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
+
+	var data types.ValidatorSetChangePacketData
+	var ackErr error
 	if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
-		errAck := types.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("cannot unmarshal CCV packet data"))
-		ack = &errAck
-	} else {
-		ack = am.keeper.OnRecvVSCPacket(ctx, packet, data)
+		ackErr = errorsmod.Wrapf(sdkerrors.ErrInvalidType, "cannot unmarshal VSCPacket data")
+		logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
+		ack = channeltypes.NewErrorAcknowledgement(ackErr)
+	}
+
+	// only attempt the application logic if the packet data
+	// was successfully decoded
+	if ack.Success() {
+		err := am.keeper.OnRecvVSCPacket(ctx, packet, data)
+		if err != nil {
+			ack = channeltypes.NewErrorAcknowledgement(err)
+			ackErr = err
+			logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
+		} else {
+			logger.Info("successfully handled VSCPacket sequence: %d", packet.Sequence)
+		}
+	}
+
+	eventAttributes := []sdk.Attribute{
+		sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+		sdk.NewAttribute(types.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))),
+		sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())),
+	}
+
+	if ackErr != nil {
+		eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeKeyAckError, ackErr.Error()))
 	}
 
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
 			types.EventTypePacket,
-			sdk.NewAttribute(sdk.AttributeKeyModule, consumertypes.ModuleName),
-			sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack != nil)),
+			eventAttributes...,
 		),
 	)
 
+	// NOTE: acknowledgement will be written synchronously during IBC handler execution.
 	return ack
 }
 
diff --git a/x/ccv/consumer/keeper/relay.go b/x/ccv/consumer/keeper/relay.go
index 3afeb33188..4772dfc2e8 100644
--- a/x/ccv/consumer/keeper/relay.go
+++ b/x/ccv/consumer/keeper/relay.go
@@ -6,7 +6,6 @@ import (
 
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
-	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -25,7 +24,12 @@ import (
 //
 // Note: CCV uses an ordered IBC channel, meaning VSC packet changes will be accumulated (and later
 // processed by ApplyCCValidatorChanges) s.t. more recent val power changes overwrite older ones.
-func (k Keeper) OnRecvVSCPacket(ctx sdk.Context, packet channeltypes.Packet, newChanges ccv.ValidatorSetChangePacketData) exported.Acknowledgement {
+func (k Keeper) OnRecvVSCPacket(ctx sdk.Context, packet channeltypes.Packet, newChanges ccv.ValidatorSetChangePacketData) error {
+	// validate packet data upon receiving
+	if err := newChanges.ValidateBasic(); err != nil {
+		return errorsmod.Wrapf(err, "error validating VSCPacket data")
+	}
+
 	// get the provider channel
 	providerChannel, found := k.GetProviderChannel(ctx)
 	if found && providerChannel != packet.DestinationChannel {
@@ -87,8 +91,7 @@ func (k Keeper) OnRecvVSCPacket(ctx sdk.Context, packet channeltypes.Packet, new
 		"len updates", len(newChanges.ValidatorUpdates),
 		"len slash acks", len(newChanges.SlashAcks),
 	)
-	ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
-	return ack
+	return nil
 }
 
 // QueueVSCMaturedPackets appends matured VSCs to an internal queue.
diff --git a/x/ccv/consumer/keeper/relay_test.go b/x/ccv/consumer/keeper/relay_test.go
index d681a2fdc4..f9d03c68f0 100644
--- a/x/ccv/consumer/keeper/relay_test.go
+++ b/x/ccv/consumer/keeper/relay_test.go
@@ -74,31 +74,59 @@ func TestOnRecvVSCPacket(t *testing.T) {
 		nil,
 	)
 
+	pd3 := types.NewValidatorSetChangePacketData(
+		[]abci.ValidatorUpdate{},
+		3,
+		[]string{
+			"invalid_slash_ack",
+		},
+	)
+
 	testCases := []struct {
 		name                   string
+		expError               bool
 		packet                 channeltypes.Packet
-		newChanges             types.ValidatorSetChangePacketData
 		expectedPendingChanges types.ValidatorSetChangePacketData
 	}{
 		{
 			"success on first packet",
+			false,
 			channeltypes.NewPacket(pd.GetBytes(), 1, types.ProviderPortID, providerCCVChannelID, types.ConsumerPortID, consumerCCVChannelID,
 				clienttypes.NewHeight(1, 0), 0),
 			types.ValidatorSetChangePacketData{ValidatorUpdates: changes1},
-			types.ValidatorSetChangePacketData{ValidatorUpdates: changes1},
 		},
 		{
 			"success on subsequent packet",
+			false,
 			channeltypes.NewPacket(pd.GetBytes(), 2, types.ProviderPortID, providerCCVChannelID, types.ConsumerPortID, consumerCCVChannelID,
 				clienttypes.NewHeight(1, 0), 0),
 			types.ValidatorSetChangePacketData{ValidatorUpdates: changes1},
-			types.ValidatorSetChangePacketData{ValidatorUpdates: changes1},
 		},
 		{
 			"success on packet with more changes",
+			false,
 			channeltypes.NewPacket(pd2.GetBytes(), 3, types.ProviderPortID, providerCCVChannelID, types.ConsumerPortID, consumerCCVChannelID,
 				clienttypes.NewHeight(1, 0), 0),
-			types.ValidatorSetChangePacketData{ValidatorUpdates: changes2},
+			types.ValidatorSetChangePacketData{ValidatorUpdates: []abci.ValidatorUpdate{
+				{
+					PubKey: pk1,
+					Power:  30,
+				},
+				{
+					PubKey: pk2,
+					Power:  40,
+				},
+				{
+					PubKey: pk3,
+					Power:  10,
+				},
+			}},
+		},
+		{
+			"failure on packet with invalid slash acks",
+			true,
+			channeltypes.NewPacket(pd3.GetBytes(), 4, types.ProviderPortID, providerCCVChannelID, types.ConsumerPortID, consumerCCVChannelID,
+				clienttypes.NewHeight(1, 0), 0),
 			types.ValidatorSetChangePacketData{ValidatorUpdates: []abci.ValidatorUpdate{
 				{
 					PubKey: pk1,
@@ -128,10 +156,16 @@ func TestOnRecvVSCPacket(t *testing.T) {
 	consumerKeeper.SetParams(ctx, moduleParams)
 
 	for _, tc := range testCases {
-		ack := consumerKeeper.OnRecvVSCPacket(ctx, tc.packet, tc.newChanges)
+		var newChanges types.ValidatorSetChangePacketData
+		err := types.ModuleCdc.UnmarshalJSON(tc.packet.GetData(), &newChanges)
+		require.Nil(t, err, "invalid test case: %s - cannot unmarshal VSCPacket data", tc.name)
+		err = consumerKeeper.OnRecvVSCPacket(ctx, tc.packet, newChanges)
+		if tc.expError {
+			require.Error(t, err, "%s - invalid but OnRecvVSCPacket did not return error", tc.name)
+			continue
+		}
+		require.NoError(t, err, "%s - valid but OnRecvVSCPacket returned error: %w", tc.name, err)
 
-		require.NotNil(t, ack, "invalid test case: %s did not return ack", tc.name)
-		require.True(t, ack.Success(), "invalid test case: %s did not return a Success Acknowledgment", tc.name)
 		providerChannel, ok := consumerKeeper.GetProviderChannel(ctx)
 		require.True(t, ok)
 		require.Equal(t, tc.packet.DestinationChannel, providerChannel,
@@ -152,7 +186,7 @@ func TestOnRecvVSCPacket(t *testing.T) {
 		expectedTime := ctx.BlockTime().Add(consumerKeeper.GetUnbondingPeriod(ctx))
 		require.True(
 			t,
-			consumerKeeper.PacketMaturityTimeExists(ctx, tc.newChanges.ValsetUpdateId, expectedTime),
+			consumerKeeper.PacketMaturityTimeExists(ctx, newChanges.ValsetUpdateId, expectedTime),
 			"no packet maturity time for case: %s", tc.name,
 		)
 	}
@@ -199,9 +233,8 @@ func TestOnRecvVSCPacketDuplicateUpdates(t *testing.T) {
 	require.False(t, ok)
 
 	// Execute OnRecvVSCPacket
-	ack := consumerKeeper.OnRecvVSCPacket(ctx, packet, vscData)
-	require.NotNil(t, ack)
-	require.True(t, ack.Success())
+	err := consumerKeeper.OnRecvVSCPacket(ctx, packet, vscData)
+	require.Nil(t, err)
 
 	// Confirm pending changes are queued by OnRecvVSCPacket
 	gotPendingChanges, ok := consumerKeeper.GetPendingChanges(ctx)
diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index 5b4e57994f..551ee24f8f 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -5,6 +5,7 @@ import (
 
 	errorsmod "cosmossdk.io/errors"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	abci "github.com/cometbft/cometbft/abci/types"
@@ -20,12 +21,18 @@ func NewValidatorSetChangePacketData(valUpdates []abci.ValidatorUpdate, valUpdat
 
 // ValidateBasic is used for validating the CCV packet data.
 func (vsc ValidatorSetChangePacketData) ValidateBasic() error {
-	if len(vsc.ValidatorUpdates) == 0 {
-		return errorsmod.Wrap(ErrInvalidPacketData, "validator updates cannot be empty")
-	}
+	// Note that vsc.ValidatorUpdate can be empty in the case of unbonding
+	// operations w/o changes in the voting power of the validators in the validator set
 	if vsc.ValsetUpdateId == 0 {
 		return errorsmod.Wrap(ErrInvalidPacketData, "valset update id cannot be equal to zero")
 	}
+	// Validate the slash acks - must be consensus addresses
+	for _, ack := range vsc.SlashAcks {
+		_, err := sdk.ConsAddressFromBech32(ack)
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
 
diff --git a/x/ccv/types/wire_test.go b/x/ccv/types/wire_test.go
index 50164fcf73..0eb1c8a1ed 100644
--- a/x/ccv/types/wire_test.go
+++ b/x/ccv/types/wire_test.go
@@ -22,6 +22,10 @@ func TestPacketDataValidateBasic(t *testing.T) {
 	pk2, err := cryptocodec.ToTmProtoPublicKey(ed25519.GenPrivKey().PubKey())
 	require.NoError(t, err)
 
+	cId := crypto.NewCryptoIdentityFromIntSeed(4732894342)
+	validSlashAck := cId.SDKValConsAddress().String()
+	tooLongSlashAck := string(make([]byte, 1024))
+
 	cases := []struct {
 		name       string
 		expError   bool
@@ -37,6 +41,54 @@ func TestPacketDataValidateBasic(t *testing.T) {
 			true,
 			types.NewValidatorSetChangePacketData([]abci.ValidatorUpdate{}, 2, nil),
 		},
+		{
+			"invalid slash ack",
+			true,
+			types.NewValidatorSetChangePacketData(
+				[]abci.ValidatorUpdate{
+					{
+						PubKey: pk1,
+						Power:  30,
+					},
+				},
+				3,
+				[]string{
+					"some_string",
+				},
+			),
+		},
+		{
+			"valid packet data with valid slash ack",
+			false,
+			types.NewValidatorSetChangePacketData(
+				[]abci.ValidatorUpdate{
+					{
+						PubKey: pk2,
+						Power:  20,
+					},
+				},
+				4,
+				[]string{
+					validSlashAck,
+				},
+			),
+		},
+		{
+			"too long slash ack",
+			true,
+			types.NewValidatorSetChangePacketData(
+				[]abci.ValidatorUpdate{
+					{
+						PubKey: pk2,
+						Power:  20,
+					},
+				},
+				5,
+				[]string{
+					tooLongSlashAck,
+				},
+			),
+		},
 		{
 			"valid packet data",
 			false,
@@ -51,7 +103,7 @@ func TestPacketDataValidateBasic(t *testing.T) {
 						Power:  20,
 					},
 				},
-				3,
+				6,
 				nil,
 			),
 		},

From 5467c1a0695c89bb7a757397b965c5679c31816d Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Tue, 28 Nov 2023 11:27:26 +0100
Subject: [PATCH 02/14] update ValidateBasic for ValidatorSetChangePacketData

---
 x/ccv/types/wire.go      |  5 ++++-
 x/ccv/types/wire_test.go | 14 +++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index 551ee24f8f..2bae13b6c6 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -21,8 +21,11 @@ func NewValidatorSetChangePacketData(valUpdates []abci.ValidatorUpdate, valUpdat
 
 // ValidateBasic is used for validating the CCV packet data.
 func (vsc ValidatorSetChangePacketData) ValidateBasic() error {
-	// Note that vsc.ValidatorUpdate can be empty in the case of unbonding
+	// Note that vsc.ValidatorUpdates can be empty in the case of unbonding
 	// operations w/o changes in the voting power of the validators in the validator set
+	if vsc.ValidatorUpdates == nil {
+		return errorsmod.Wrap(ErrInvalidPacketData, "validator updates cannot be nil")
+	}
 	if vsc.ValsetUpdateId == 0 {
 		return errorsmod.Wrap(ErrInvalidPacketData, "valset update id cannot be equal to zero")
 	}
diff --git a/x/ccv/types/wire_test.go b/x/ccv/types/wire_test.go
index 0eb1c8a1ed..806e89edb3 100644
--- a/x/ccv/types/wire_test.go
+++ b/x/ccv/types/wire_test.go
@@ -32,17 +32,17 @@ func TestPacketDataValidateBasic(t *testing.T) {
 		packetData types.ValidatorSetChangePacketData
 	}{
 		{
-			"nil packet data",
+			"invalid: nil packet data",
 			true,
 			types.NewValidatorSetChangePacketData(nil, 1, nil),
 		},
 		{
-			"empty packet data",
-			true,
+			"valid: empty packet data",
+			false,
 			types.NewValidatorSetChangePacketData([]abci.ValidatorUpdate{}, 2, nil),
 		},
 		{
-			"invalid slash ack",
+			"invalid: slash ack not consensus address",
 			true,
 			types.NewValidatorSetChangePacketData(
 				[]abci.ValidatorUpdate{
@@ -58,7 +58,7 @@ func TestPacketDataValidateBasic(t *testing.T) {
 			),
 		},
 		{
-			"valid packet data with valid slash ack",
+			"valid: packet data with valid slash ack",
 			false,
 			types.NewValidatorSetChangePacketData(
 				[]abci.ValidatorUpdate{
@@ -74,7 +74,7 @@ func TestPacketDataValidateBasic(t *testing.T) {
 			),
 		},
 		{
-			"too long slash ack",
+			"invalid: slash ack is too long",
 			true,
 			types.NewValidatorSetChangePacketData(
 				[]abci.ValidatorUpdate{
@@ -90,7 +90,7 @@ func TestPacketDataValidateBasic(t *testing.T) {
 			),
 		},
 		{
-			"valid packet data",
+			"valid: packet data with nil slash ack",
 			false,
 			types.NewValidatorSetChangePacketData(
 				[]abci.ValidatorUpdate{

From 656781add50dda2e0860cdc1b62197e06aab123d Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Wed, 29 Nov 2023 17:15:43 +0100
Subject: [PATCH 03/14] update ConsumerPacketData validation

---
 tests/integration/slashing.go       | 42 +++++++----------
 tests/integration/throttle.go       |  9 ++--
 x/ccv/provider/ibc_module.go        | 70 ++++++++++++++++++++++-------
 x/ccv/provider/keeper/relay.go      | 30 +++++++++----
 x/ccv/provider/keeper/relay_test.go | 33 +++++++-------
 x/ccv/types/wire.go                 | 18 +++++---
 6 files changed, 128 insertions(+), 74 deletions(-)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index 4058494974..ebbd222909 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -270,12 +270,14 @@ func (s *CCVTestSuite) TestSlashPacketAcknowledgement() {
 	// Map infraction height on provider so validation passes and provider returns valid ack result
 	providerKeeper.SetValsetUpdateBlockHeight(s.providerCtx(), spd.ValsetUpdateId, 47923)
 
-	exportedAck := providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, spd)
-	s.Require().NotNil(exportedAck)
+	ackResult, err := providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, spd)
+	s.Require().NotNil(ackResult)
+	s.Require().NoError(err)
+	exportedAck := channeltypes.NewResultAcknowledgement(ackResult)
 
 	// Unmarshal ack to struct that's compatible with consumer. IBC does this automatically
 	ack := channeltypes.Acknowledgement{}
-	err := channeltypes.SubModuleCdc.UnmarshalJSON(exportedAck.Acknowledgement(), &ack)
+	err = channeltypes.SubModuleCdc.UnmarshalJSON(exportedAck.Acknowledgement(), &ack)
 	s.Require().NoError(err)
 
 	err = consumerKeeper.OnAcknowledgementPacket(s.consumerCtx(), packet, ack)
@@ -353,22 +355,16 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	providerKeeper.DeleteInitChainHeight(ctx, consumerChainID)
 
 	packetData := ccv.SlashPacketData{ValsetUpdateId: 0}
-	errAck := providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
-	suite.Require().False(errAck.Success())
-	errAckCast := errAck.(channeltypes.Acknowledgement)
-	// Error strings in err acks are now thrown out by IBC core to prevent app hash.
-	// Hence a generic error string is expected.
-	suite.Require().Equal("ABCI code: 1: error handling packet: see events for details", errAckCast.GetError())
+	_, err := providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
+	suite.Require().Error(err)
 
 	// Restore init chain height
 	providerKeeper.SetInitChainHeight(ctx, consumerChainID, initChainHeight)
 
 	// now the method will fail at infraction height check.
 	packetData.Infraction = stakingtypes.Infraction_INFRACTION_UNSPECIFIED
-	errAck = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
-	suite.Require().False(errAck.Success())
-	errAckCast = errAck.(channeltypes.Acknowledgement)
-	suite.Require().Equal("ABCI code: 1: error handling packet: see events for details", errAckCast.GetError())
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
+	suite.Require().Error(err)
 
 	// save current VSC ID
 	vscID := providerKeeper.GetValidatorSetUpdateId(ctx)
@@ -380,10 +376,8 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	packetData = ccv.SlashPacketData{ValsetUpdateId: vscID}
 
 	// expect an error if mapped block height is not found
-	errAck = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
-	suite.Require().False(errAck.Success())
-	errAckCast = errAck.(channeltypes.Acknowledgement)
-	suite.Require().Equal("ABCI code: 1: error handling packet: see events for details", errAckCast.GetError())
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
+	suite.Require().Error(err)
 
 	// construct slashing packet with non existing validator
 	slashingPkt := ccv.NewSlashPacketData(
@@ -397,10 +391,8 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	providerKeeper.SetInitChainHeight(ctx, consumerChainID, uint64(ctx.BlockHeight()))
 
 	// Expect no error ack if validator does not exist
-	// TODO: this behavior should be changed to return an error ack,
-	// see: https://github.com/cosmos/interchain-security/issues/546
-	ack := providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
-	suite.Require().True(ack.Success())
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
+	suite.Require().NoError(err)
 
 	val := suite.providerChain.Vals.Validators[0]
 
@@ -427,13 +419,13 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	valInfo.Address = sdk.ConsAddress(tmAddr).String()
 	providerSlashingKeeper.SetValidatorSigningInfo(ctx, sdk.ConsAddress(tmAddr), valInfo)
 
-	errAck = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
-	suite.Require().False(errAck.Success())
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
+	suite.Require().Error(err)
 
 	// expect to queue entries for the slash request
 	slashingPkt.Infraction = stakingtypes.Infraction_INFRACTION_DOWNTIME
-	ack = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
-	suite.Require().True(ack.Success())
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
+	suite.Require().NoError(err)
 }
 
 // TestValidatorDowntime tests if a slash packet is sent
diff --git a/tests/integration/throttle.go b/tests/integration/throttle.go
index 71ee3f606b..eb04d1ce35 100644
--- a/tests/integration/throttle.go
+++ b/tests/integration/throttle.go
@@ -330,7 +330,8 @@ func (s *CCVTestSuite) TestPacketSpam() {
 		consumerPacketData, err := provider.UnmarshalConsumerPacketData(data) // Same func used by provider's OnRecvPacket
 		s.Require().NoError(err)
 		packet := s.newPacketFromConsumer(data, uint64(sequence), firstBundle.Path, timeoutHeight, timeoutTimestamp)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		_, err = providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		s.Require().NoError(err)
 	}
 
 	// Execute block
@@ -387,7 +388,8 @@ func (s *CCVTestSuite) TestDoubleSignDoesNotAffectThrottling() {
 		consumerPacketData, err := provider.UnmarshalConsumerPacketData(data) // Same func used by provider's OnRecvPacket
 		s.Require().NoError(err)
 		packet := s.newPacketFromConsumer(data, uint64(sequence), firstBundle.Path, timeoutHeight, timeoutTimestamp)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		_, err = providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		s.Require().NoError(err)
 	}
 
 	// Execute block to handle packets in endblock
@@ -557,7 +559,8 @@ func (s CCVTestSuite) TestSlashAllValidators() { //nolint:govet // this is a tes
 		consumerPacketData, err := provider.UnmarshalConsumerPacketData(data) // Same func used by provider's OnRecvPacket
 		s.Require().NoError(err)
 		packet := s.newPacketFromConsumer(data, ibcSeqNum, s.getFirstBundle().Path, timeoutHeight, timeoutTimestamp)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		_, err = providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		s.Require().NoError(err)
 	}
 
 	// Check that all validators are jailed.
diff --git a/x/ccv/provider/ibc_module.go b/x/ccv/provider/ibc_module.go
index 75da9588d6..7697606f7a 100644
--- a/x/ccv/provider/ibc_module.go
+++ b/x/ccv/provider/ibc_module.go
@@ -2,6 +2,7 @@ package provider
 
 import (
 	"fmt"
+	"strconv"
 
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
@@ -16,6 +17,7 @@ import (
 
 	"github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper"
 	providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
+	"github.com/cosmos/interchain-security/v3/x/ccv/types"
 	ccv "github.com/cosmos/interchain-security/v3/x/ccv/types"
 )
 
@@ -175,37 +177,71 @@ func (am AppModule) OnRecvPacket(
 	packet channeltypes.Packet,
 	_ sdk.AccAddress,
 ) ibcexported.Acknowledgement {
+	logger := am.keeper.Logger(ctx)
+	ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
+
+	var ackErr error
 	consumerPacket, err := UnmarshalConsumerPacket(packet)
 	if err != nil {
-		errAck := ccv.NewErrorAcknowledgementWithLog(ctx, err)
-		return &errAck
+		ackErr = errorsmod.Wrapf(sdkerrors.ErrInvalidType, "cannot unmarshal ConsumerPacket data")
+		logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
+		ack = channeltypes.NewErrorAcknowledgement(ackErr)
 	}
 
-	// TODO: call ValidateBasic method on consumer packet data
-	// See: https://github.com/cosmos/interchain-security/issues/634
+	eventAttributes := []sdk.Attribute{
+		sdk.NewAttribute(sdk.AttributeKeyModule, providertypes.ModuleName),
+	}
+
+	// only attempt the application logic if the packet data
+	// was successfully decoded
+	if ack.Success() {
+		var err error = nil
+		switch consumerPacket.Type {
+		case ccv.VscMaturedPacket:
+			// handle VSCMaturedPacket
+			data := *consumerPacket.GetVscMaturedPacketData()
+			err = am.keeper.OnRecvVSCMaturedPacket(ctx, packet, data)
+			if err == nil {
+				logger.Info("successfully handled VSCMaturedPacket sequence: %d", packet.Sequence)
+				eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))))
+			}
+		case ccv.SlashPacket:
+			// handle SlashPacket
+			var ackResult ccv.PacketAckResult
+			data := *consumerPacket.GetSlashPacketData()
+			ackResult, err = am.keeper.OnRecvSlashPacket(ctx, packet, data)
+			if err == nil {
+				ack = channeltypes.NewResultAcknowledgement(ackResult)
+				logger.Info("successfully handled SlashPacket sequence: %d", packet.Sequence)
+				eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))))
+			}
+		default:
+			err = fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type)
+		}
+		if err != nil {
+			ack = channeltypes.NewErrorAcknowledgement(err)
+			ackErr = err
+			logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
+		}
+	}
 
-	var ack ibcexported.Acknowledgement
-	switch consumerPacket.Type {
-	case ccv.VscMaturedPacket:
-		// handle VSCMaturedPacket
-		ack = am.keeper.OnRecvVSCMaturedPacket(ctx, packet, *consumerPacket.GetVscMaturedPacketData())
-	case ccv.SlashPacket:
-		// handle SlashPacket
-		ack = am.keeper.OnRecvSlashPacket(ctx, packet, *consumerPacket.GetSlashPacketData())
-	default:
-		errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
-		ack = &errAck
+	eventAttributes = append(eventAttributes, sdk.NewAttribute(ccv.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())))
+	if ackErr != nil {
+		eventAttributes = append(eventAttributes, sdk.NewAttribute(ccv.AttributeKeyAckError, ackErr.Error()))
 	}
 
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
 			ccv.EventTypePacket,
-			sdk.NewAttribute(sdk.AttributeKeyModule, providertypes.ModuleName),
-			sdk.NewAttribute(ccv.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack != nil)),
+			eventAttributes...,
 		),
 	)
 
+	// NOTE: acknowledgement will be written synchronously during IBC handler execution.
 	return ack
+
+	// TODO: call ValidateBasic method on consumer packet data
+	// See: https://github.com/cosmos/interchain-security/issues/634
 }
 
 func UnmarshalConsumerPacket(packet channeltypes.Packet) (consumerPacket ccv.ConsumerPacketData, err error) {
diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go
index 0cbce718a7..0c11556d8c 100644
--- a/x/ccv/provider/keeper/relay.go
+++ b/x/ccv/provider/keeper/relay.go
@@ -6,7 +6,6 @@ import (
 
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
-	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -22,7 +21,12 @@ func (k Keeper) OnRecvVSCMaturedPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	data ccv.VSCMaturedPacketData,
-) exported.Acknowledgement {
+) error {
+	// validate packet data upon receiving
+	if err := data.ValidateBasic(); err != nil {
+		return errorsmod.Wrapf(err, "error validating VSCMaturedPacket data")
+	}
+
 	// check that the channel is established, panic if not
 	chainID, found := k.GetChannelToChain(ctx, packet.DestinationChannel)
 	if !found {
@@ -41,8 +45,7 @@ func (k Keeper) OnRecvVSCMaturedPacket(
 		"vscID", data.ValsetUpdateId,
 	)
 
-	ack := channeltypes.NewResultAcknowledgement(ccv.V1Result)
-	return ack
+	return nil
 }
 
 // HandleVSCMaturedPacket handles a VSCMatured packet.
@@ -265,7 +268,16 @@ func (k Keeper) EndBlockCIS(ctx sdk.Context) {
 
 // OnRecvSlashPacket delivers a received slash packet, validates it and
 // then queues the slash packet as pending if valid.
-func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, data ccv.SlashPacketData) exported.Acknowledgement {
+func (k Keeper) OnRecvSlashPacket(
+	ctx sdk.Context,
+	packet channeltypes.Packet,
+	data ccv.SlashPacketData,
+) (ccv.PacketAckResult, error) {
+	// validate packet data upon receiving
+	if err := data.ValidateBasic(); err != nil {
+		return nil, errorsmod.Wrapf(err, "error validating SlashPacket data")
+	}
+
 	// check that the channel is established, panic if not
 	chainID, found := k.GetChannelToChain(ctx, packet.DestinationChannel)
 	if !found {
@@ -285,7 +297,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
 			"vscID", data.ValsetUpdateId,
 			"infractionType", data.Infraction,
 		)
-		return ccv.NewErrorAcknowledgementWithLog(ctx, err)
+		return nil, err
 	}
 
 	// The slash packet validator address may be known only on the consumer chain,
@@ -308,7 +320,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
 
 		// return successful ack, as an error would result
 		// in the consumer closing the CCV channel
-		return channeltypes.NewResultAcknowledgement(ccv.V1Result)
+		return ccv.V1Result, nil
 	}
 
 	meter := k.GetSlashMeter(ctx)
@@ -321,7 +333,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
 			"vscID", data.ValsetUpdateId,
 			"infractionType", data.Infraction,
 		)
-		return channeltypes.NewResultAcknowledgement(ccv.SlashPacketBouncedResult)
+		return ccv.SlashPacketBouncedResult, nil
 	}
 
 	// Subtract voting power that will be jailed/tombstoned from the slash meter,
@@ -340,7 +352,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
 	)
 
 	// Return result ack that the packet was handled successfully
-	return channeltypes.NewResultAcknowledgement(ccv.SlashPacketHandledResult)
+	return ccv.SlashPacketHandledResult, nil
 }
 
 // ValidateSlashPacket validates a recv slash packet before it is
diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go
index bc482d30a2..b92282bff9 100644
--- a/x/ccv/provider/keeper/relay_test.go
+++ b/x/ccv/provider/keeper/relay_test.go
@@ -6,7 +6,6 @@ import (
 
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
-	exported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
@@ -106,12 +105,12 @@ func TestOnRecvVSCMaturedPacket(t *testing.T) {
 	providerKeeper.SetChannelToChain(ctx, "channel-2", "chain-2")
 
 	// Execute on recv for chain-1, confirm v1 result ack is returned
-	ack := executeOnRecvVSCMaturedPacket(t, &providerKeeper, ctx, "channel-1", 1)
-	require.Equal(t, channeltypes.NewResultAcknowledgement([]byte{byte(1)}), ack)
+	err := executeOnRecvVSCMaturedPacket(t, &providerKeeper, ctx, "channel-1", 1)
+	require.NoError(t, err)
 
 	// Now queue a slash packet data instance for chain-2, confirm v1 result ack is returned
-	ack = executeOnRecvVSCMaturedPacket(t, &providerKeeper, ctx, "channel-2", 2)
-	require.Equal(t, channeltypes.NewResultAcknowledgement([]byte{byte(1)}), ack)
+	err = executeOnRecvVSCMaturedPacket(t, &providerKeeper, ctx, "channel-2", 2)
+	require.NoError(t, err)
 }
 
 // TestOnRecvDowntimeSlashPacket tests the OnRecvSlashPacket method specifically for downtime slash packets.
@@ -133,12 +132,14 @@ func TestOnRecvDowntimeSlashPacket(t *testing.T) {
 
 	// Set slash meter to negative value and assert a bounce ack is returned
 	providerKeeper.SetSlashMeter(ctx, math.NewInt(-5))
-	ack := executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
-	require.Equal(t, channeltypes.NewResultAcknowledgement(ccv.SlashPacketBouncedResult), ack)
+	ackResult, err := executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
+	require.Equal(t, ccv.SlashPacketBouncedResult, ackResult)
+	require.NoError(t, err)
 
 	// Also bounced for chain-2
-	ack = executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-2", 2, packetData)
-	require.Equal(t, channeltypes.NewResultAcknowledgement(ccv.SlashPacketBouncedResult), ack)
+	ackResult, err = executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-2", 2, packetData)
+	require.Equal(t, ccv.SlashPacketBouncedResult, ackResult)
+	require.NoError(t, err)
 
 	// Now set slash meter to positive value and assert slash packet handled result is returned
 	providerKeeper.SetSlashMeter(ctx, math.NewInt(5))
@@ -160,8 +161,9 @@ func TestOnRecvDowntimeSlashPacket(t *testing.T) {
 	gomock.InOrder(calls...)
 
 	// Execute on recv and confirm slash packet handled result is returned
-	ack = executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
-	require.Equal(t, channeltypes.NewResultAcknowledgement(ccv.SlashPacketHandledResult), ack)
+	ackResult, err = executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
+	require.Equal(t, ccv.SlashPacketHandledResult, ackResult)
+	require.NoError(t, err)
 
 	// Require slash meter was decremented appropriately, 5-2=3
 	require.Equal(t, int64(3), providerKeeper.GetSlashMeter(ctx).Int64())
@@ -185,8 +187,9 @@ func TestOnRecvDoubleSignSlashPacket(t *testing.T) {
 	providerKeeper.SetValsetUpdateBlockHeight(ctx, packetData.ValsetUpdateId, uint64(15))
 
 	// Receive the double-sign slash packet for chain-1 and confirm the expected acknowledgement
-	ack := executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
-	require.Equal(t, channeltypes.NewResultAcknowledgement(ccv.V1Result), ack)
+	ackResult, err := executeOnRecvSlashPacket(t, &providerKeeper, ctx, "channel-1", 1, packetData)
+	require.Equal(t, ccv.V1Result, ackResult)
+	require.NoError(t, err)
 
 	require.True(t, providerKeeper.GetSlashLog(ctx,
 		providertypes.NewProviderConsAddress(packetData.Validator.Address)))
@@ -198,7 +201,7 @@ func TestOnRecvDoubleSignSlashPacket(t *testing.T) {
 
 func executeOnRecvVSCMaturedPacket(t *testing.T, providerKeeper *keeper.Keeper, ctx sdk.Context,
 	channelID string, ibcSeqNum uint64,
-) exported.Acknowledgement {
+) error {
 	t.Helper()
 	// Instantiate vsc matured packet data and bytes
 	data := testkeeper.GetNewVSCMaturedPacketData()
@@ -214,7 +217,7 @@ func executeOnRecvVSCMaturedPacket(t *testing.T, providerKeeper *keeper.Keeper,
 
 func executeOnRecvSlashPacket(t *testing.T, providerKeeper *keeper.Keeper, ctx sdk.Context,
 	channelID string, ibcSeqNum uint64, packetData ccv.SlashPacketData,
-) exported.Acknowledgement {
+) (ccv.PacketAckResult, error) {
 	t.Helper()
 	// Instantiate slash packet data and bytes
 	dataBz, err := packetData.Marshal()
diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index 2bae13b6c6..084ceb4189 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -26,6 +26,7 @@ func (vsc ValidatorSetChangePacketData) ValidateBasic() error {
 	if vsc.ValidatorUpdates == nil {
 		return errorsmod.Wrap(ErrInvalidPacketData, "validator updates cannot be nil")
 	}
+	// ValsetUpdateId is strictly positive
 	if vsc.ValsetUpdateId == 0 {
 		return errorsmod.Wrap(ErrInvalidPacketData, "valset update id cannot be equal to zero")
 	}
@@ -54,6 +55,7 @@ func NewVSCMaturedPacketData(valUpdateID uint64) *VSCMaturedPacketData {
 
 // ValidateBasic is used for validating the VSCMatured packet data.
 func (mat VSCMaturedPacketData) ValidateBasic() error {
+	// ValsetUpdateId is strictly positive
 	if mat.ValsetUpdateId == 0 {
 		return errorsmod.Wrap(ErrInvalidPacketData, "vscId cannot be equal to zero")
 	}
@@ -86,9 +88,15 @@ func NewSlashPacketDataV1(validator abci.Validator, valUpdateId uint64, infracti
 }
 
 func (vdt SlashPacketData) ValidateBasic() error {
-	if len(vdt.Validator.Address) == 0 || vdt.Validator.Power == 0 {
-		return errorsmod.Wrap(ErrInvalidPacketData, "validator fields cannot be empty")
+	// vdt.Validator.Address must be a consensus address
+	if err := sdk.VerifyAddressFormat(vdt.Validator.Address); err != nil {
+		return errorsmod.Wrap(ErrInvalidPacketData, fmt.Sprintf("invalid validator: %s", err.Error()))
 	}
+	// vdt.Validator.Power must be positive
+	if vdt.Validator.Power == 0 {
+		return errorsmod.Wrap(ErrInvalidPacketData, "validator power cannot be zero")
+	}
+	// Note that ValsetUpdateId can be zero due to the vscID mapping
 
 	if vdt.Infraction == stakingtypes.Infraction_INFRACTION_UNSPECIFIED {
 		return errorsmod.Wrap(ErrInvalidPacketData, "invalid infraction type")
@@ -105,11 +113,11 @@ func (cp ConsumerPacketData) ValidateBasic() (err error) {
 	switch cp.Type {
 	case VscMaturedPacket:
 		// validate VSCMaturedPacket
-		vscPacket := cp.GetVscMaturedPacketData()
-		if vscPacket == nil {
+		vscMaturedPacket := cp.GetVscMaturedPacketData()
+		if vscMaturedPacket == nil {
 			return fmt.Errorf("invalid consumer packet data: VscMaturePacketData data cannot be empty")
 		}
-		err = vscPacket.ValidateBasic()
+		err = vscMaturedPacket.ValidateBasic()
 	case SlashPacket:
 		// validate SlashPacket
 		slashPacket := cp.GetSlashPacketData()

From 629dfae0a1e849670864f4fd899b7d5e3caffa86 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 11:22:03 +0100
Subject: [PATCH 04/14] fix TestConsumerPacketSendExpiredClient

---
 tests/integration/expired_client.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/integration/expired_client.go b/tests/integration/expired_client.go
index f196f16ce1..81654895eb 100644
--- a/tests/integration/expired_client.go
+++ b/tests/integration/expired_client.go
@@ -129,7 +129,7 @@ func (s *CCVTestSuite) TestConsumerPacketSendExpiredClient() {
 
 	// try to send slash packet for downtime infraction
 	addr := ed25519.GenPrivKey().PubKey().Address()
-	val := abci.Validator{Address: addr}
+	val := abci.Validator{Address: addr, Power: 1}
 	consumerKeeper.QueueSlashPacket(s.consumerCtx(), val, 2, stakingtypes.Infraction_INFRACTION_DOWNTIME)
 	// try to send slash packet for the same downtime infraction
 	consumerKeeper.QueueSlashPacket(s.consumerCtx(), val, 3, stakingtypes.Infraction_INFRACTION_DOWNTIME)

From 421c0224a5d60827856b374590090fb08a6d94da Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 12:31:33 +0100
Subject: [PATCH 05/14] update TestOnRecvSlashPacketErrors

---
 tests/integration/slashing.go       | 142 +++++++++++++---------------
 x/ccv/provider/keeper/relay.go      |  14 +--
 x/ccv/provider/keeper/relay_test.go |  10 --
 x/ccv/types/wire.go                 |   4 +-
 4 files changed, 73 insertions(+), 97 deletions(-)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index ebbd222909..42393b83d2 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -9,6 +9,8 @@ import (
 
 	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdktypes "github.com/cosmos/cosmos-sdk/types"
+	sdkaddress "github.com/cosmos/cosmos-sdk/types/address"
 	evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"
 	slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
@@ -331,9 +333,7 @@ func (suite *CCVTestSuite) TestHandleSlashPacketDowntime() {
 // TestOnRecvSlashPacketErrors tests errors for the OnRecvSlashPacket method in an integration testing setting
 func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	providerKeeper := suite.providerApp.GetProviderKeeper()
-	providerSlashingKeeper := suite.providerApp.GetTestSlashingKeeper()
 	firstBundle := suite.getFirstBundle()
-	consumerChainID := firstBundle.Chain.ChainID
 
 	suite.SetupAllCCVChannels()
 
@@ -347,85 +347,75 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 
 	// Add correct channelID to packet. Now we will not panic anymore.
 	packet := channeltypes.Packet{DestinationChannel: firstBundle.Path.EndpointB.ChannelID}
+	suite.NotPanics(func() {
+		providerKeeper.OnRecvSlashPacket(ctx, packet, ccv.SlashPacketData{})
+	})
 
-	// Init chain height is set by established CCV channel
-	// Delete init chain height and confirm expected error
-	initChainHeight, found := providerKeeper.GetInitChainHeight(ctx, consumerChainID)
-	suite.Require().True(found)
-	providerKeeper.DeleteInitChainHeight(ctx, consumerChainID)
-
-	packetData := ccv.SlashPacketData{ValsetUpdateId: 0}
-	_, err := providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
-	suite.Require().Error(err)
-
-	// Restore init chain height
-	providerKeeper.SetInitChainHeight(ctx, consumerChainID, initChainHeight)
-
-	// now the method will fail at infraction height check.
-	packetData.Infraction = stakingtypes.Infraction_INFRACTION_UNSPECIFIED
-	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
-	suite.Require().Error(err)
-
-	// save current VSC ID
-	vscID := providerKeeper.GetValidatorSetUpdateId(ctx)
-
-	// remove block height value mapped to current VSC ID
-	providerKeeper.DeleteValsetUpdateBlockHeight(ctx, vscID)
-
-	// Instantiate packet data with current VSC ID
-	packetData = ccv.SlashPacketData{ValsetUpdateId: vscID}
-
-	// expect an error if mapped block height is not found
-	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
-	suite.Require().Error(err)
-
-	// construct slashing packet with non existing validator
-	slashingPkt := ccv.NewSlashPacketData(
+	// Check ValidateBasic for SlashPacket data
+	validAddress := ed25519.GenPrivKey().PubKey().Address()
+	slashPacketData := ccv.NewSlashPacketData(
 		abci.Validator{
-			Address: ed25519.GenPrivKey().PubKey().Address(),
-			Power:   int64(0),
+			Address: validAddress,
+			Power:   int64(1),
 		}, uint64(0), stakingtypes.Infraction_INFRACTION_DOWNTIME,
 	)
 
-	// Set initial block height for consumer chain
-	providerKeeper.SetInitChainHeight(ctx, consumerChainID, uint64(ctx.BlockHeight()))
-
-	// Expect no error ack if validator does not exist
-	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
-	suite.Require().NoError(err)
-
-	val := suite.providerChain.Vals.Validators[0]
-
-	// commit block to set VSC ID
-	suite.coordinator.CommitBlock(suite.providerChain)
-	// Update suite.ctx bc CommitBlock updates only providerChain's current header block height
-	ctx = suite.providerChain.GetContext()
-	suite.Require().NotZero(providerKeeper.GetValsetUpdateBlockHeight(ctx, vscID))
-
-	// create validator signing info
-	valInfo := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(val.Address), ctx.BlockHeight(),
-		ctx.BlockHeight()-1, time.Time{}.UTC(), false, int64(0))
-	providerSlashingKeeper.SetValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address), valInfo)
-
-	// update validator address and VSC ID
-	slashingPkt.Validator.Address = val.Address
-	slashingPkt.ValsetUpdateId = vscID
-
-	// expect error ack when infraction type in unspecified
-	tmAddr := suite.providerChain.Vals.Validators[1].Address
-	slashingPkt.Validator.Address = tmAddr
-	slashingPkt.Infraction = stakingtypes.Infraction_INFRACTION_UNSPECIFIED
-
-	valInfo.Address = sdk.ConsAddress(tmAddr).String()
-	providerSlashingKeeper.SetValidatorSigningInfo(ctx, sdk.ConsAddress(tmAddr), valInfo)
-
-	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
-	suite.Require().Error(err)
-
-	// expect to queue entries for the slash request
-	slashingPkt.Infraction = stakingtypes.Infraction_INFRACTION_DOWNTIME
-	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashingPkt)
-	suite.Require().NoError(err)
+	// Expect an error if validator address is too long
+	slashPacketData.Validator.Address = make([]byte, sdkaddress.MaxAddrLen+1)
+	_, err := providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().Error(err, "validating SlashPacket data should fail - invalid validator address")
+
+	// Expect an error if validator power is zero
+	slashPacketData.Validator.Address = validAddress
+	slashPacketData.Validator.Power = 0
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().Error(err, "validating SlashPacket data should fail - invalid validator power")
+
+	// Expect an error if the infraction type is unspecified
+	slashPacketData.Validator.Power = 1
+	slashPacketData.Infraction = stakingtypes.Infraction_INFRACTION_UNSPECIFIED
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().Error(err, "validating SlashPacket data should fail - invalid infraction type")
+
+	// Restore slashPacketData to be valid
+	slashPacketData.Infraction = stakingtypes.Infraction_INFRACTION_DOWNTIME
+
+	// Check ValidateSlashPacket
+	// Expect an error if a mapping of the infraction height cannot be found;
+	// just set the vscID of the slash packet to the latest mapped vscID +1
+	valsetUpdateBlockHeights := providerKeeper.GetAllValsetUpdateBlockHeights(ctx)
+	latestMappedValsetUpdateId := valsetUpdateBlockHeights[len(valsetUpdateBlockHeights)-1].ValsetUpdateId
+	slashPacketData.ValsetUpdateId = latestMappedValsetUpdateId + 1
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().Error(err, "ValidateSlashPacket should fail - no infraction height mapping")
+
+	// Restore slashPacketData to be valid
+	slashPacketData.ValsetUpdateId = latestMappedValsetUpdateId
+
+	// Expect no error if validator does not exist
+	_, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().NoError(err, "no error expected")
+
+	// Check expected behavior for handling SlashPackets for double signing infractions
+	slashPacketData.Infraction = stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN
+	ackResult, err := providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().NoError(err, "no error expected")
+	suite.Require().Equal(ccv.V1Result, ackResult, "expected successful ack")
+
+	// Check expected behavior for handling SlashPackets for downtime infractions
+	slashPacketData.Infraction = stakingtypes.Infraction_INFRACTION_DOWNTIME
+
+	// Expect the packet to bounce if the slash meter is negative
+	providerKeeper.SetSlashMeter(ctx, sdktypes.NewInt(-1))
+	ackResult, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().NoError(err, "no error expected")
+	suite.Require().Equal(ccv.SlashPacketBouncedResult, ackResult, "expected successful ack")
+
+	// Expect the packet to be handled if the slash meter is positive
+	providerKeeper.SetSlashMeter(ctx, sdktypes.NewInt(0))
+	ackResult, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
+	suite.Require().NoError(err, "no error expected")
+	suite.Require().Equal(ccv.SlashPacketHandledResult, ackResult, "expected successful ack")
 }
 
 // TestValidatorDowntime tests if a slash packet is sent
diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go
index 0c11556d8c..648ccdc07a 100644
--- a/x/ccv/provider/keeper/relay.go
+++ b/x/ccv/provider/keeper/relay.go
@@ -273,11 +273,6 @@ func (k Keeper) OnRecvSlashPacket(
 	packet channeltypes.Packet,
 	data ccv.SlashPacketData,
 ) (ccv.PacketAckResult, error) {
-	// validate packet data upon receiving
-	if err := data.ValidateBasic(); err != nil {
-		return nil, errorsmod.Wrapf(err, "error validating SlashPacket data")
-	}
-
 	// check that the channel is established, panic if not
 	chainID, found := k.GetChannelToChain(ctx, packet.DestinationChannel)
 	if !found {
@@ -289,6 +284,11 @@ func (k Keeper) OnRecvSlashPacket(
 		panic(fmt.Errorf("SlashPacket received on unknown channel %s", packet.DestinationChannel))
 	}
 
+	// validate packet data upon receiving
+	if err := data.ValidateBasic(); err != nil {
+		return nil, errorsmod.Wrapf(err, "error validating SlashPacket data")
+	}
+
 	if err := k.ValidateSlashPacket(ctx, chainID, packet, data); err != nil {
 		k.Logger(ctx).Error("invalid slash packet",
 			"error", err.Error(),
@@ -368,10 +368,6 @@ func (k Keeper) ValidateSlashPacket(ctx sdk.Context, chainID string,
 			"the validator update id %d for chain %s", data.ValsetUpdateId, chainID)
 	}
 
-	if data.Infraction != stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN && data.Infraction != stakingtypes.Infraction_INFRACTION_DOWNTIME {
-		return fmt.Errorf("invalid infraction type: %s", data.Infraction)
-	}
-
 	return nil
 }
 
diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go
index b92282bff9..fdb9c7cc2f 100644
--- a/x/ccv/provider/keeper/relay_test.go
+++ b/x/ccv/provider/keeper/relay_test.go
@@ -244,16 +244,6 @@ func TestValidateSlashPacket(t *testing.T) {
 			ccv.SlashPacketData{ValsetUpdateId: 61},
 			true,
 		},
-		{
-			"non-set infraction type",
-			ccv.SlashPacketData{ValsetUpdateId: validVscID},
-			true,
-		},
-		{
-			"invalid infraction type",
-			ccv.SlashPacketData{ValsetUpdateId: validVscID, Infraction: stakingtypes.MaxMonikerLength},
-			true,
-		},
 		{
 			"valid double sign packet with non-zero vscID",
 			ccv.SlashPacketData{ValsetUpdateId: validVscID, Infraction: stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN},
diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index 084ceb4189..7a225f73d7 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -98,8 +98,8 @@ func (vdt SlashPacketData) ValidateBasic() error {
 	}
 	// Note that ValsetUpdateId can be zero due to the vscID mapping
 
-	if vdt.Infraction == stakingtypes.Infraction_INFRACTION_UNSPECIFIED {
-		return errorsmod.Wrap(ErrInvalidPacketData, "invalid infraction type")
+	if vdt.Infraction != stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN && vdt.Infraction != stakingtypes.Infraction_INFRACTION_DOWNTIME {
+		return errorsmod.Wrap(ErrInvalidPacketData, fmt.Sprintf("invalid infraction type: %s", vdt.Infraction.String()))
 	}
 
 	return nil

From aab0e98fe8f0c2c3d76f240299b570a0da929c74 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 12:38:54 +0100
Subject: [PATCH 06/14] fix TestQueueAndSendSlashPacket

---
 tests/integration/slashing.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index 42393b83d2..51e959437b 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -631,6 +631,7 @@ func (suite *CCVTestSuite) TestQueueAndSendSlashPacket() {
 			addr := ed25519.GenPrivKey().PubKey().Address()
 			val := abci.Validator{
 				Address: addr,
+				Power:   int64(1),
 			}
 			consumerKeeper.QueueSlashPacket(ctx, val, 0, infraction)
 			slashedVals = append(slashedVals, slashedVal{validator: val, infraction: infraction})

From ac5c37914d84c787b18fd634c3b929e52ba98fd4 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 12:41:44 +0100
Subject: [PATCH 07/14] remove TODO

---
 x/ccv/provider/ibc_module.go   |  3 ---
 x/ccv/provider/keeper/relay.go | 10 +++++-----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/x/ccv/provider/ibc_module.go b/x/ccv/provider/ibc_module.go
index 7697606f7a..41f8305148 100644
--- a/x/ccv/provider/ibc_module.go
+++ b/x/ccv/provider/ibc_module.go
@@ -239,9 +239,6 @@ func (am AppModule) OnRecvPacket(
 
 	// NOTE: acknowledgement will be written synchronously during IBC handler execution.
 	return ack
-
-	// TODO: call ValidateBasic method on consumer packet data
-	// See: https://github.com/cosmos/interchain-security/issues/634
 }
 
 func UnmarshalConsumerPacket(packet channeltypes.Packet) (consumerPacket ccv.ConsumerPacketData, err error) {
diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go
index 648ccdc07a..064a498782 100644
--- a/x/ccv/provider/keeper/relay.go
+++ b/x/ccv/provider/keeper/relay.go
@@ -22,11 +22,6 @@ func (k Keeper) OnRecvVSCMaturedPacket(
 	packet channeltypes.Packet,
 	data ccv.VSCMaturedPacketData,
 ) error {
-	// validate packet data upon receiving
-	if err := data.ValidateBasic(); err != nil {
-		return errorsmod.Wrapf(err, "error validating VSCMaturedPacket data")
-	}
-
 	// check that the channel is established, panic if not
 	chainID, found := k.GetChannelToChain(ctx, packet.DestinationChannel)
 	if !found {
@@ -38,6 +33,11 @@ func (k Keeper) OnRecvVSCMaturedPacket(
 		panic(fmt.Errorf("VSCMaturedPacket received on unknown channel %s", packet.DestinationChannel))
 	}
 
+	// validate packet data upon receiving
+	if err := data.ValidateBasic(); err != nil {
+		return errorsmod.Wrapf(err, "error validating VSCMaturedPacket data")
+	}
+
 	k.HandleVSCMaturedPacket(ctx, chainID, data)
 
 	k.Logger(ctx).Info("VSCMaturedPacket handled",

From 5a4b6d80a7ef9e217db280a233448b700976ecc9 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 12:52:32 +0100
Subject: [PATCH 08/14] nit: validate MsgAssignConsumerKey

---
 x/ccv/provider/types/errors.go | 2 +-
 x/ccv/provider/types/msg.go    | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go
index 66d7a9a3b8..6c19a7b396 100644
--- a/x/ccv/provider/types/errors.go
+++ b/x/ccv/provider/types/errors.go
@@ -11,7 +11,7 @@ var (
 	ErrUnknownConsumerChainId           = errorsmod.Register(ModuleName, 3, "no consumer chain with this chain id")
 	ErrUnknownConsumerChannelId         = errorsmod.Register(ModuleName, 4, "no consumer chain with this channel id")
 	ErrInvalidConsumerConsensusPubKey   = errorsmod.Register(ModuleName, 5, "empty consumer consensus public key")
-	ErrBlankConsumerChainID             = errorsmod.Register(ModuleName, 6, "consumer chain id must not be blank")
+	ErrInvalidConsumerChainID           = errorsmod.Register(ModuleName, 6, "invalid consumer chain id")
 	ErrConsumerKeyNotFound              = errorsmod.Register(ModuleName, 7, "consumer key not found")
 	ErrNoValidatorConsumerAddress       = errorsmod.Register(ModuleName, 8, "error getting validator consumer address")
 	ErrNoValidatorProviderAddress       = errorsmod.Register(ModuleName, 9, "error getting validator provider address")
diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go
index f7ee11325c..73fb7f5b74 100644
--- a/x/ccv/provider/types/msg.go
+++ b/x/ccv/provider/types/msg.go
@@ -72,14 +72,13 @@ func (msg MsgAssignConsumerKey) GetSignBytes() []byte {
 // ValidateBasic implements the sdk.Msg interface.
 func (msg MsgAssignConsumerKey) ValidateBasic() error {
 	if strings.TrimSpace(msg.ChainId) == "" {
-		return ErrBlankConsumerChainID
+		return errorsmod.Wrapf(ErrInvalidConsumerChainID, "chainId cannot be blank")
 	}
 	// It is possible to assign keys for consumer chains that are not yet approved.
 	// This can only be done by a signing validator, but it is still sensible
 	// to limit the chainID size to prevent abuse.
-
 	if 128 < len(msg.ChainId) {
-		return ErrBlankConsumerChainID
+		return errorsmod.Wrapf(ErrInvalidConsumerChainID, "chainId cannot exceed 128 length")
 	}
 	_, err := sdk.ValAddressFromBech32(msg.ProviderAddr)
 	if err != nil {

From 6ca0ebc532f04dcb0e158cf6c769874765b303d6 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 13:03:45 +0100
Subject: [PATCH 09/14] add changelog entries

---
 .changelog/unreleased/bug-fixes/1460-msg-validation.md      | 3 +++
 .changelog/unreleased/state-breaking/1460-msg-validation.md | 3 +++
 2 files changed, 6 insertions(+)
 create mode 100644 .changelog/unreleased/bug-fixes/1460-msg-validation.md
 create mode 100644 .changelog/unreleased/state-breaking/1460-msg-validation.md

diff --git a/.changelog/unreleased/bug-fixes/1460-msg-validation.md b/.changelog/unreleased/bug-fixes/1460-msg-validation.md
new file mode 100644
index 0000000000..46d18bd4c9
--- /dev/null
+++ b/.changelog/unreleased/bug-fixes/1460-msg-validation.md
@@ -0,0 +1,3 @@
+- Improve validation of IBC packet data and provider messages. Also,
+  enable the provider to validate consumer packets before handling them.
+  ([\#1460](https://github.com/cosmos/interchain-security/pull/1460))
\ No newline at end of file
diff --git a/.changelog/unreleased/state-breaking/1460-msg-validation.md b/.changelog/unreleased/state-breaking/1460-msg-validation.md
new file mode 100644
index 0000000000..46d18bd4c9
--- /dev/null
+++ b/.changelog/unreleased/state-breaking/1460-msg-validation.md
@@ -0,0 +1,3 @@
+- Improve validation of IBC packet data and provider messages. Also,
+  enable the provider to validate consumer packets before handling them.
+  ([\#1460](https://github.com/cosmos/interchain-security/pull/1460))
\ No newline at end of file

From 72bc1f281842599cae74a4de5764d682631a3ffb Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 13:10:55 +0100
Subject: [PATCH 10/14] fix linter

---
 tests/integration/slashing.go | 5 ++---
 x/ccv/provider/ibc_module.go  | 7 +++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index 51e959437b..93ab8d2d25 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -9,7 +9,6 @@ import (
 
 	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdktypes "github.com/cosmos/cosmos-sdk/types"
 	sdkaddress "github.com/cosmos/cosmos-sdk/types/address"
 	evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"
 	slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
@@ -406,13 +405,13 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 	slashPacketData.Infraction = stakingtypes.Infraction_INFRACTION_DOWNTIME
 
 	// Expect the packet to bounce if the slash meter is negative
-	providerKeeper.SetSlashMeter(ctx, sdktypes.NewInt(-1))
+	providerKeeper.SetSlashMeter(ctx, sdk.NewInt(-1))
 	ackResult, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
 	suite.Require().NoError(err, "no error expected")
 	suite.Require().Equal(ccv.SlashPacketBouncedResult, ackResult, "expected successful ack")
 
 	// Expect the packet to be handled if the slash meter is positive
-	providerKeeper.SetSlashMeter(ctx, sdktypes.NewInt(0))
+	providerKeeper.SetSlashMeter(ctx, sdk.NewInt(0))
 	ackResult, err = providerKeeper.OnRecvSlashPacket(ctx, packet, *slashPacketData)
 	suite.Require().NoError(err, "no error expected")
 	suite.Require().Equal(ccv.SlashPacketHandledResult, ackResult, "expected successful ack")
diff --git a/x/ccv/provider/ibc_module.go b/x/ccv/provider/ibc_module.go
index 41f8305148..8d342b0c6f 100644
--- a/x/ccv/provider/ibc_module.go
+++ b/x/ccv/provider/ibc_module.go
@@ -17,7 +17,6 @@ import (
 
 	"github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper"
 	providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
-	"github.com/cosmos/interchain-security/v3/x/ccv/types"
 	ccv "github.com/cosmos/interchain-security/v3/x/ccv/types"
 )
 
@@ -195,7 +194,7 @@ func (am AppModule) OnRecvPacket(
 	// only attempt the application logic if the packet data
 	// was successfully decoded
 	if ack.Success() {
-		var err error = nil
+		var err error
 		switch consumerPacket.Type {
 		case ccv.VscMaturedPacket:
 			// handle VSCMaturedPacket
@@ -203,7 +202,7 @@ func (am AppModule) OnRecvPacket(
 			err = am.keeper.OnRecvVSCMaturedPacket(ctx, packet, data)
 			if err == nil {
 				logger.Info("successfully handled VSCMaturedPacket sequence: %d", packet.Sequence)
-				eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))))
+				eventAttributes = append(eventAttributes, sdk.NewAttribute(ccv.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))))
 			}
 		case ccv.SlashPacket:
 			// handle SlashPacket
@@ -213,7 +212,7 @@ func (am AppModule) OnRecvPacket(
 			if err == nil {
 				ack = channeltypes.NewResultAcknowledgement(ackResult)
 				logger.Info("successfully handled SlashPacket sequence: %d", packet.Sequence)
-				eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))))
+				eventAttributes = append(eventAttributes, sdk.NewAttribute(ccv.AttributeValSetUpdateID, strconv.Itoa(int(data.ValsetUpdateId))))
 			}
 		default:
 			err = fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type)

From 4b26974926651c862afd6db2cefc5245431e5b91 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 13:15:35 +0100
Subject: [PATCH 11/14] fix gosec

---
 tests/integration/slashing.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index 93ab8d2d25..24d2c2c33b 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -341,13 +341,13 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 
 	// Expect panic if ccv channel is not established via dest channel of packet
 	suite.Panics(func() {
-		providerKeeper.OnRecvSlashPacket(ctx, channeltypes.Packet{}, ccv.SlashPacketData{})
+		_, _ = providerKeeper.OnRecvSlashPacket(ctx, channeltypes.Packet{}, ccv.SlashPacketData{})
 	})
 
 	// Add correct channelID to packet. Now we will not panic anymore.
 	packet := channeltypes.Packet{DestinationChannel: firstBundle.Path.EndpointB.ChannelID}
 	suite.NotPanics(func() {
-		providerKeeper.OnRecvSlashPacket(ctx, packet, ccv.SlashPacketData{})
+		_, _ = providerKeeper.OnRecvSlashPacket(ctx, packet, ccv.SlashPacketData{})
 	})
 
 	// Check ValidateBasic for SlashPacket data

From 5488484374096426ed33c0cb76aacba3313f15b6 Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 13:36:06 +0100
Subject: [PATCH 12/14] rename ValidateBasic to Validate (IBC packets)

---
 tests/integration/slashing.go  |  2 +-
 x/ccv/consumer/keeper/relay.go |  2 +-
 x/ccv/provider/keeper/relay.go |  4 ++--
 x/ccv/types/wire.go            | 16 ++++++++--------
 x/ccv/types/wire_test.go       |  6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
index 24d2c2c33b..6e0269e268 100644
--- a/tests/integration/slashing.go
+++ b/tests/integration/slashing.go
@@ -350,7 +350,7 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
 		_, _ = providerKeeper.OnRecvSlashPacket(ctx, packet, ccv.SlashPacketData{})
 	})
 
-	// Check ValidateBasic for SlashPacket data
+	// Check Validate for SlashPacket data
 	validAddress := ed25519.GenPrivKey().PubKey().Address()
 	slashPacketData := ccv.NewSlashPacketData(
 		abci.Validator{
diff --git a/x/ccv/consumer/keeper/relay.go b/x/ccv/consumer/keeper/relay.go
index 4772dfc2e8..68c911f255 100644
--- a/x/ccv/consumer/keeper/relay.go
+++ b/x/ccv/consumer/keeper/relay.go
@@ -26,7 +26,7 @@ import (
 // processed by ApplyCCValidatorChanges) s.t. more recent val power changes overwrite older ones.
 func (k Keeper) OnRecvVSCPacket(ctx sdk.Context, packet channeltypes.Packet, newChanges ccv.ValidatorSetChangePacketData) error {
 	// validate packet data upon receiving
-	if err := newChanges.ValidateBasic(); err != nil {
+	if err := newChanges.Validate(); err != nil {
 		return errorsmod.Wrapf(err, "error validating VSCPacket data")
 	}
 
diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go
index 064a498782..b07ba3b8ff 100644
--- a/x/ccv/provider/keeper/relay.go
+++ b/x/ccv/provider/keeper/relay.go
@@ -34,7 +34,7 @@ func (k Keeper) OnRecvVSCMaturedPacket(
 	}
 
 	// validate packet data upon receiving
-	if err := data.ValidateBasic(); err != nil {
+	if err := data.Validate(); err != nil {
 		return errorsmod.Wrapf(err, "error validating VSCMaturedPacket data")
 	}
 
@@ -285,7 +285,7 @@ func (k Keeper) OnRecvSlashPacket(
 	}
 
 	// validate packet data upon receiving
-	if err := data.ValidateBasic(); err != nil {
+	if err := data.Validate(); err != nil {
 		return nil, errorsmod.Wrapf(err, "error validating SlashPacket data")
 	}
 
diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index 7a225f73d7..c7cbe9e126 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -19,8 +19,8 @@ func NewValidatorSetChangePacketData(valUpdates []abci.ValidatorUpdate, valUpdat
 	}
 }
 
-// ValidateBasic is used for validating the CCV packet data.
-func (vsc ValidatorSetChangePacketData) ValidateBasic() error {
+// Validate is used for validating the CCV packet data.
+func (vsc ValidatorSetChangePacketData) Validate() error {
 	// Note that vsc.ValidatorUpdates can be empty in the case of unbonding
 	// operations w/o changes in the voting power of the validators in the validator set
 	if vsc.ValidatorUpdates == nil {
@@ -53,8 +53,8 @@ func NewVSCMaturedPacketData(valUpdateID uint64) *VSCMaturedPacketData {
 	}
 }
 
-// ValidateBasic is used for validating the VSCMatured packet data.
-func (mat VSCMaturedPacketData) ValidateBasic() error {
+// Validate is used for validating the VSCMatured packet data.
+func (mat VSCMaturedPacketData) Validate() error {
 	// ValsetUpdateId is strictly positive
 	if mat.ValsetUpdateId == 0 {
 		return errorsmod.Wrap(ErrInvalidPacketData, "vscId cannot be equal to zero")
@@ -87,7 +87,7 @@ func NewSlashPacketDataV1(validator abci.Validator, valUpdateId uint64, infracti
 	}
 }
 
-func (vdt SlashPacketData) ValidateBasic() error {
+func (vdt SlashPacketData) Validate() error {
 	// vdt.Validator.Address must be a consensus address
 	if err := sdk.VerifyAddressFormat(vdt.Validator.Address); err != nil {
 		return errorsmod.Wrap(ErrInvalidPacketData, fmt.Sprintf("invalid validator: %s", err.Error()))
@@ -109,7 +109,7 @@ func (vdt SlashPacketData) ToV1() *SlashPacketDataV1 {
 	return NewSlashPacketDataV1(vdt.Validator, vdt.ValsetUpdateId, vdt.Infraction)
 }
 
-func (cp ConsumerPacketData) ValidateBasic() (err error) {
+func (cp ConsumerPacketData) Validate() (err error) {
 	switch cp.Type {
 	case VscMaturedPacket:
 		// validate VSCMaturedPacket
@@ -117,14 +117,14 @@ func (cp ConsumerPacketData) ValidateBasic() (err error) {
 		if vscMaturedPacket == nil {
 			return fmt.Errorf("invalid consumer packet data: VscMaturePacketData data cannot be empty")
 		}
-		err = vscMaturedPacket.ValidateBasic()
+		err = vscMaturedPacket.Validate()
 	case SlashPacket:
 		// validate SlashPacket
 		slashPacket := cp.GetSlashPacketData()
 		if slashPacket == nil {
 			return fmt.Errorf("invalid consumer packet data: SlashPacketData data cannot be empty")
 		}
-		err = slashPacket.ValidateBasic()
+		err = slashPacket.Validate()
 	default:
 		err = fmt.Errorf("invalid consumer packet type: %q", cp.Type)
 	}
diff --git a/x/ccv/types/wire_test.go b/x/ccv/types/wire_test.go
index 806e89edb3..f97d1af1d2 100644
--- a/x/ccv/types/wire_test.go
+++ b/x/ccv/types/wire_test.go
@@ -110,11 +110,11 @@ func TestPacketDataValidateBasic(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		err := c.packetData.ValidateBasic()
+		err := c.packetData.Validate()
 		if c.expError {
-			require.Error(t, err, "%s invalid but passed ValidateBasic", c.name)
+			require.Error(t, err, "%s invalid but passed Validate", c.name)
 		} else {
-			require.NoError(t, err, "%s valid but ValidateBasic returned error: %w", c.name, err)
+			require.NoError(t, err, "%s valid but Validate returned error: %w", c.name, err)
 		}
 	}
 }

From d3424fc03d536f6ac7a6ec31bbfa5606c00a0598 Mon Sep 17 00:00:00 2001
From: Marius Poke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 14:57:52 +0100
Subject: [PATCH 13/14] Update x/ccv/types/wire.go

Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
---
 x/ccv/types/wire.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index c7cbe9e126..aa598d5f49 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -89,7 +89,7 @@ func NewSlashPacketDataV1(validator abci.Validator, valUpdateId uint64, infracti
 
 func (vdt SlashPacketData) Validate() error {
 	// vdt.Validator.Address must be a consensus address
-	if err := sdk.VerifyAddressFormat(vdt.Validator.Address); err != nil {
+	if err := sdk.ConsAddressFromBech32(vdt.Validator.Address.String()); err != nil {
 		return errorsmod.Wrap(ErrInvalidPacketData, fmt.Sprintf("invalid validator: %s", err.Error()))
 	}
 	// vdt.Validator.Power must be positive

From a6609d76284c915c2a57de5d1d00020cf00443ad Mon Sep 17 00:00:00 2001
From: mpoke <marius.poke@posteo.de>
Date: Thu, 30 Nov 2023 15:00:37 +0100
Subject: [PATCH 14/14] revert SlashPacketData address validation

---
 x/ccv/types/wire.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x/ccv/types/wire.go b/x/ccv/types/wire.go
index aa598d5f49..c7cbe9e126 100644
--- a/x/ccv/types/wire.go
+++ b/x/ccv/types/wire.go
@@ -89,7 +89,7 @@ func NewSlashPacketDataV1(validator abci.Validator, valUpdateId uint64, infracti
 
 func (vdt SlashPacketData) Validate() error {
 	// vdt.Validator.Address must be a consensus address
-	if err := sdk.ConsAddressFromBech32(vdt.Validator.Address.String()); err != nil {
+	if err := sdk.VerifyAddressFormat(vdt.Validator.Address); err != nil {
 		return errorsmod.Wrap(ErrInvalidPacketData, fmt.Sprintf("invalid validator: %s", err.Error()))
 	}
 	// vdt.Validator.Power must be positive