fix: make SlashPacketData backward compatible when sending data over the wire

proto/interchain_security/ccv/v1/ccv.proto

@@ -84,4 +84,42 @@ enum ConsumerPacketDataType {
   // VSCMatured packet
   CONSUMER_PACKET_TYPE_VSCM = 2
       [ (gogoproto.enumvalue_customname) = "VscMaturedPacket" ];
-}
+}
+
+// ConsumerPacketData contains a consumer packet data and a type tag
+// that is compatible with ICS v1 and v2 over the wire. It is not used for internal storage.
+message ConsumerPacketDataV1 {
+  ConsumerPacketDataType type = 1;
+
+  oneof data {
+    SlashPacketDataV1 slashPacketData = 2;
+    VSCMaturedPacketData vscMaturedPacketData = 3;
+  }
+}
+
+// This packet is sent from the consumer chain to the provider chain
+// It is backward compatible with the ICS v1 and v2 version of the packet.
+message SlashPacketDataV1 {
+  tendermint.abci.Validator validator = 1 [
+    (gogoproto.nullable) = false,
+    (gogoproto.moretags) = "yaml:\"validator\""
+  ];
+  // map to the infraction block height on the provider
+  uint64 valset_update_id = 2;
+  // tell if the slashing is for a downtime or a double-signing infraction
+  InfractionType infraction = 3;
+}
+
+// InfractionType indicates the infraction type a validator commited.
+// NOTE: ccv.InfractionType to maintain compatibility between ICS versions
+// using different versions of the cosmos-sdk and ibc-go modules.
+enum InfractionType {
+  option (gogoproto.goproto_enum_prefix) = false;
+
+  // UNSPECIFIED defines an empty infraction type.
+  INFRACTION_TYPE_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "InfractionEmpty"];
+  // DOUBLE_SIGN defines a validator that double-signs a block.
+  INFRACTION_TYPE_DOUBLE_SIGN = 1 [(gogoproto.enumvalue_customname) = "DoubleSign"];
+  // DOWNTIME defines a validator that missed signing too many blocks.
+  INFRACTION_TYPE_DOWNTIME = 2 [(gogoproto.enumvalue_customname) = "Downtime"];
+}

tests/integration/throttle.go

@@ -315,8 +315,14 @@ func (s *CCVTestSuite) TestPacketSpam() {
 	// Recv 500 packets from consumer to provider in same block
 	for _, packet := range packets {
 		consumerPacketData := ccvtypes.ConsumerPacketData{}
-		ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		consumerPacketDataV1 := ccvtypes.ConsumerPacketDataV1{}
+		err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacketData)
+		if err == nil {
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		} else {
+			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketDataV1)
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketDataV1.GetSlashPacketData().FromV1())
+		}
 	}
 
 	// Execute block to handle packets in endblock
@@ -369,8 +375,14 @@ func (s *CCVTestSuite) TestDoubleSignDoesNotAffectThrottling() {
 	// Recv 500 packets from consumer to provider in same block
 	for _, packet := range packets {
 		consumerPacketData := ccvtypes.ConsumerPacketData{}
-		ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		consumerPacketDataV1 := ccvtypes.ConsumerPacketDataV1{}
+		err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacketData)
+		if err == nil {
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		} else {
+			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketDataV1)
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketDataV1.GetSlashPacketData().FromV1())
+		}
 	}
 
 	// Execute block to handle packets in endblock
@@ -465,7 +477,17 @@ func (s *CCVTestSuite) TestQueueOrdering() {
 	// Recv 500 packets from consumer to provider in same block
 	for i, packet := range packets {
 		consumerPacketData := ccvtypes.ConsumerPacketData{}
-		ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
+		consumerPacketDataV1 := ccvtypes.ConsumerPacketDataV1{}
+		err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacketData)
+		if err != nil {
+			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketDataV1)
+			consumerPacketData = ccvtypes.ConsumerPacketData{
+				Type: consumerPacketDataV1.Type,
+				Data: &ccvtypes.ConsumerPacketData_SlashPacketData{
+					SlashPacketData: consumerPacketDataV1.GetSlashPacketData().FromV1(),
+				},
+			}
+		}
 		// Type depends on index packets were appended from above
 		if (i+5)%10 == 0 {
 			vscMaturedPacketData := consumerPacketData.GetVscMaturedPacketData()
@@ -679,8 +701,14 @@ func (s *CCVTestSuite) TestSlashSameValidator() {
 	// Recv and queue all slash packets.
 	for _, packet := range packets {
 		consumerPacketData := ccvtypes.ConsumerPacketData{}
-		ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		consumerPacketDataV1 := ccvtypes.ConsumerPacketDataV1{}
+		err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacketData)
+		if err == nil {
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		} else {
+			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketDataV1)
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketDataV1.GetSlashPacketData().FromV1())
+		}
 	}
 
 	// We should have 6 pending slash packet entries queued.
@@ -740,8 +768,14 @@ func (s CCVTestSuite) TestSlashAllValidators() { //nolint:govet // this is a tes
 	// Recv and queue all slash packets.
 	for _, packet := range packets {
 		consumerPacketData := ccvtypes.ConsumerPacketData{}
-		ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
-		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		consumerPacketDataV1 := ccvtypes.ConsumerPacketDataV1{}
+		err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacketData)
+		if err == nil {
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
+		} else {
+			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketDataV1)
+			providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketDataV1.GetSlashPacketData().FromV1())
+		}
 	}
 
 	// We should have 24 pending slash packet entries queued.
@@ -787,9 +821,14 @@ func (s *CCVTestSuite) TestLeadingVSCMaturedAreDequeued() {
 			packet := s.constructSlashPacketFromConsumer(*bundle,
 				*s.providerChain.Vals.Validators[0], stakingtypes.Infraction_INFRACTION_DOWNTIME, ibcSeqNum)
 			packetData := ccvtypes.ConsumerPacketData{}
-			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
-			providerKeeper.OnRecvSlashPacket(s.providerCtx(),
-				packet, *packetData.GetSlashPacketData())
+			packetDataV1 := ccvtypes.ConsumerPacketDataV1{}
+			err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &packetData)
+			if err == nil {
+				providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *packetData.GetSlashPacketData())
+			} else {
+				ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetDataV1)
+				providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *packetDataV1.GetSlashPacketData().FromV1())
+			}
 		}
 	}
 
@@ -878,9 +917,14 @@ func (s *CCVTestSuite) TestVscMaturedHandledPerBlockLimit() {
 			packet := s.constructSlashPacketFromConsumer(*bundle,
 				*s.providerChain.Vals.Validators[0], stakingtypes.Infraction_INFRACTION_DOWNTIME, ibcSeqNum)
 			packetData := ccvtypes.ConsumerPacketData{}
-			ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
-			providerKeeper.OnRecvSlashPacket(s.providerCtx(),
-				packet, *packetData.GetSlashPacketData())
+			packetDataV1 := ccvtypes.ConsumerPacketDataV1{}
+			err := ccvtypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &packetData)
+			if err == nil {
+				providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *packetData.GetSlashPacketData())
+			} else {
+				ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetDataV1)
+				providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *packetDataV1.GetSlashPacketData().FromV1())
+			}
 		}
 	}
 

x/ccv/provider/ibc_module.go

@@ -175,28 +175,49 @@ func (am AppModule) OnRecvPacket(
 	_ sdk.AccAddress,
 ) ibcexported.Acknowledgement {
 	var (
-		ack            ibcexported.Acknowledgement
-		consumerPacket ccv.ConsumerPacketData
+		ack              ibcexported.Acknowledgement
+		consumerPacket   ccv.ConsumerPacketData
+		consumerPacketV1 ccv.ConsumerPacketDataV1
+		isV1Packet       bool
 	)
+
 	// unmarshall consumer packet
 	if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacket); err != nil {
-		errAck := channeltypes.NewErrorAcknowledgement(fmt.Errorf("cannot unmarshal CCV packet data"))
-		ack = &errAck
-	} else {
-		// TODO: call ValidateBasic method on consumer packet data
-		// See: https://github.com/cosmos/interchain-security/issues/634
-
-		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 := channeltypes.NewErrorAcknowledgement(fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
+		// retry for v1 packet type
+		errV1 := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacketV1)
+		if errV1 != nil {
+			errAck := channeltypes.NewErrorAcknowledgement(fmt.Errorf("cannot unmarshal CCV packet data"))
 			ack = &errAck
+
+			ctx.EventManager().EmitEvent(
+				sdk.NewEvent(
+					ccv.EventTypePacket,
+					sdk.NewAttribute(sdk.AttributeKeyModule, providertypes.ModuleName),
+					sdk.NewAttribute(ccv.AttributeKeyAckSuccess, "true"),
+				),
+			)
+			return ack
+		}
+		isV1Packet = true
+	}
+
+	// TODO: call ValidateBasic method on consumer packet data
+	// See: https://github.com/cosmos/interchain-security/issues/634
+
+	switch consumerPacket.Type {
+	case ccv.VscMaturedPacket:
+		// handle VSCMaturedPacket
+		ack = am.keeper.OnRecvVSCMaturedPacket(ctx, packet, *consumerPacket.GetVscMaturedPacketData())
+	case ccv.SlashPacket:
+		// handle SlashPacket
+		if isV1Packet {
+			ack = am.keeper.OnRecvSlashPacket(ctx, packet, *consumerPacketV1.GetSlashPacketData().FromV1())
+		} else {
+			ack = am.keeper.OnRecvSlashPacket(ctx, packet, *consumerPacket.GetSlashPacketData())
 		}
+	default:
+		errAck := channeltypes.NewErrorAcknowledgement(fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
+		ack = &errAck
 	}
 
 	ctx.EventManager().EmitEvent(

x/ccv/provider/keeper/relay.go

@@ -400,6 +400,26 @@ func (k Keeper) ValidateSlashPacket(ctx sdk.Context, chainID string,
 	return nil
 }
 
+// ValidateV1SlashPacket validates a recv slash packet compatible with v1 before it is
+// handled or persisted in store. An error is returned if the packet is invalid,
+// and an error ack should be relayed to the sender.
+func (k Keeper) ValidateV1SlashPacket(ctx sdk.Context, chainID string,
+	packet channeltypes.Packet, data ccv.SlashPacketDataV1,
+) error {
+	_, found := k.getMappedInfractionHeight(ctx, chainID, data.ValsetUpdateId)
+	// return error if we cannot find infraction height matching the validator update id
+	if !found {
+		return fmt.Errorf("cannot find infraction height matching "+
+			"the validator update id %d for chain %s", data.ValsetUpdateId, chainID)
+	}
+
+	if data.Infraction != ccv.DoubleSign && data.Infraction != ccv.Downtime {
+		return fmt.Errorf("invalid infraction type: %s", data.Infraction)
+	}
+
+	return nil
+}
+
 // HandleSlashPacket potentially jails a misbehaving validator for a downtime infraction.
 // This method should NEVER be called with a double-sign infraction.
 func (k Keeper) HandleSlashPacket(ctx sdk.Context, chainID string, data ccv.SlashPacketData) {

x/ccv/provider/proposal_handler_test.go

@@ -81,7 +81,8 @@ func TestProviderProposalHandler(t *testing.T) {
 		{
 			name: "unsupported proposal type",
 			// lint rule disabled because this is a test case for an unsupported proposal type
-			content: &distributiontypes.CommunityPoolSpendProposal{ // nolint:staticcheck
+			// nolint:staticcheck
+			content: &distributiontypes.CommunityPoolSpendProposal{
 				Title:       "title",
 				Description: "desc",
 				Recipient:   "",

x/ccv/types/ccv.go

@@ -59,6 +59,23 @@ func NewSlashPacketData(validator abci.Validator, valUpdateId uint64, infraction
 	}
 }
 
+// NewSlashPacketDataV1 creates a new SlashPacketDataV1 that uses ccv.InfractionTypes to maintain backward compatibility.
+func NewSlashPacketDataV1(validator abci.Validator, valUpdateId uint64, infractionType stakingtypes.Infraction) *SlashPacketDataV1 {
+	v1Type := InfractionEmpty
+	switch infractionType {
+	case stakingtypes.Infraction_INFRACTION_DOWNTIME:
+		v1Type = Downtime
+	case stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN:
+		v1Type = DoubleSign
+	}
+
+	return &SlashPacketDataV1{
+		Validator:      validator,
+		ValsetUpdateId: valUpdateId,
+		Infraction:     v1Type,
+	}
+}
+
 func (vdt SlashPacketData) ValidateBasic() error {
 	if len(vdt.Validator.Address) == 0 || vdt.Validator.Power == 0 {
 		return errorsmod.Wrap(ErrInvalidPacketData, "validator fields cannot be empty")
@@ -76,6 +93,10 @@ func (vdt SlashPacketData) GetBytes() []byte {
 	return valDowntimeBytes
 }
 
+func (vdt SlashPacketData) ToV1() *SlashPacketDataV1 {
+	return NewSlashPacketDataV1(vdt.Validator, vdt.ValsetUpdateId, vdt.Infraction)
+}
+
 func (cp ConsumerPacketData) ValidateBasic() (err error) {
 	switch cp.Type {
 	case VscMaturedPacket:
@@ -99,7 +120,45 @@ func (cp ConsumerPacketData) ValidateBasic() (err error) {
 	return
 }
 
+// Convert to bytes while maintaining over the wire compatibility with previous versions.
 func (cp ConsumerPacketData) GetBytes() []byte {
-	bytes := ModuleCdc.MustMarshalJSON(&cp)
+	return cp.ToV1Bytes()
+}
+
+// ToV1Bytes converts the ConsumerPacketData to JSON byte array compatible
+// with the format used by ICS versions using cosmos-sdk v45 (ICS v1 and ICS v2).
+func (cp ConsumerPacketData) ToV1Bytes() []byte {
+	if cp.Type != SlashPacket {
+		bytes := ModuleCdc.MustMarshalJSON(&cp)
+		return bytes
+	}
+
+	sp := cp.GetSlashPacketData()
+	spdv1 := NewSlashPacketDataV1(sp.Validator, sp.ValsetUpdateId, sp.Infraction)
+	cpv1 := ConsumerPacketDataV1{
+		Type: cp.Type,
+		Data: &ConsumerPacketDataV1_SlashPacketData{
+			SlashPacketData: spdv1,
+		},
+	}
+	bytes := ModuleCdc.MustMarshalJSON(&cpv1)
 	return bytes
 }
+
+// FromV1 converts SlashPacketDataV1 to SlashPacketData.
+// Provider must handle both V1 and later versions of the SlashPacketData.
+func (vdt1 SlashPacketDataV1) FromV1() *SlashPacketData {
+	newType := stakingtypes.Infraction_INFRACTION_UNSPECIFIED
+	switch vdt1.Infraction {
+	case Downtime:
+		newType = stakingtypes.Infraction_INFRACTION_DOWNTIME
+	case DoubleSign:
+		newType = stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN
+	}
+
+	return &SlashPacketData{
+		Validator:      vdt1.Validator,
+		ValsetUpdateId: vdt1.ValsetUpdateId,
+		Infraction:     newType,
+	}
+}