Skip to content

Commit

Permalink
fix: make SlashPacketData backward compatible when sending data over …
Browse files Browse the repository at this point in the history 
…the wire (#1093)

* fix: use v1 slash types on consumer side

* fix: update provider ibc_module to also handle v1 slash packets

* chore: update linter

* fix problematic packet handling on provider

* rm unused function

* refactor/test: 1093 continued (#1104)

* UnmarshalConsumerPacket func, use it in tests

* added test

* format

---------

Co-authored-by: Marius Poke <[email protected]>
Co-authored-by: Shawn <[email protected]>
  • Loading branch information
@MSalopek @mpoke @shaspitz
3 people authored and Karolos Antoniadis committed Jul 24, 2023
1 parent 119b6bc commit b71d71a
Show file tree
Hide file tree
Showing 7 changed files with 919 additions and 93 deletions.
40 changes: 39 additions & 1 deletion proto/interchain_security/ccv/v1/ccv.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
}
37 changes: 19 additions & 18 deletions tests/integration/throttle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"

icstestingutils "github.com/cosmos/interchain-security/v3/testutil/ibc_testing"
"github.com/cosmos/interchain-security/v3/x/ccv/provider"
providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types"
)
Expand Down Expand Up @@ -314,8 +315,8 @@ 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)
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
}

Expand Down Expand Up @@ -368,8 +369,8 @@ 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)
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
}

Expand Down Expand Up @@ -464,8 +465,10 @@ 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)

consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)

// Type depends on index packets were appended from above
if (i+5)%10 == 0 {
vscMaturedPacketData := consumerPacketData.GetVscMaturedPacketData()
Expand Down Expand Up @@ -678,8 +681,8 @@ func (s *CCVTestSuite) TestSlashSameValidator() {

// Recv and queue all slash packets.
for _, packet := range packets {
consumerPacketData := ccvtypes.ConsumerPacketData{}
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
}

Expand Down Expand Up @@ -739,8 +742,8 @@ 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)
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
}

Expand Down Expand Up @@ -786,10 +789,9 @@ func (s *CCVTestSuite) TestLeadingVSCMaturedAreDequeued() {
ibcSeqNum := uint64(i)
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())
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
}
}

Expand Down Expand Up @@ -877,10 +879,9 @@ func (s *CCVTestSuite) TestVscMaturedHandledPerBlockLimit() {
ibcSeqNum := uint64(i)
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())
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
s.Require().NoError(err)
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
}
}

Expand Down
68 changes: 46 additions & 22 deletions x/ccv/provider/ibc_module.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,29 +174,26 @@ func (am AppModule) OnRecvPacket(
packet channeltypes.Packet,
_ sdk.AccAddress,
) ibcexported.Acknowledgement {
var (
ack ibcexported.Acknowledgement
consumerPacket ccv.ConsumerPacketData
)
// unmarshall consumer packet
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacket); err != nil {
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("cannot unmarshal CCV packet data"))
consumerPacket, err := UnmarshalConsumerPacket(packet)
if err != nil {
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, err)
return &errAck
}

// TODO: call ValidateBasic method on consumer packet data
// See: https://github.com/cosmos/interchain-security/issues/634

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
} 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 := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
ack = &errAck
}
}

ctx.EventManager().EmitEvent(
Expand All @@ -210,6 +207,33 @@ func (am AppModule) OnRecvPacket(
return ack
}

func UnmarshalConsumerPacket(packet channeltypes.Packet) (consumerPacket ccv.ConsumerPacketData, err error) {
// First try unmarshaling into ccv.ConsumerPacketData type
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacket); err != nil {
// If failed, packet should be a v1 slash packet, retry for ConsumerPacketDataV1 packet type
var v1Packet ccv.ConsumerPacketDataV1
errV1 := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &v1Packet)
if errV1 != nil {
// If neither worked, return error
return ccv.ConsumerPacketData{}, errV1
}

// VSC matured packets should not be unmarshaled as v1 packets
if v1Packet.Type == ccv.VscMaturedPacket {
return ccv.ConsumerPacketData{}, fmt.Errorf("VSC matured packets should be correctly unmarshaled")
}

// Convert from v1 packet type
consumerPacket = ccv.ConsumerPacketData{
Type: v1Packet.Type,
Data: &ccv.ConsumerPacketData_SlashPacketData{
SlashPacketData: v1Packet.GetSlashPacketData().FromV1(),
},
}
}
return consumerPacket, nil
}

// OnAcknowledgementPacket implements the IBCModule interface
func (am AppModule) OnAcknowledgementPacket(
ctx sdk.Context,
Expand Down
60 changes: 60 additions & 0 deletions x/ccv/provider/ibc_module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
conntypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
Expand Down Expand Up @@ -338,3 +339,62 @@ func TestOnChanOpenConfirm(t *testing.T) {
ctrl.Finish()
}
}

func TestUnmarshalConsumerPacket(t *testing.T) {
testCases := []struct {
name string
packet channeltypes.Packet
expectedPacketData ccv.ConsumerPacketData
}{
{
name: "vsc matured",
packet: channeltypes.NewPacket(
ccv.ConsumerPacketData{
Type: ccv.VscMaturedPacket,
Data: &ccv.ConsumerPacketData_VscMaturedPacketData{
VscMaturedPacketData: &ccv.VSCMaturedPacketData{
ValsetUpdateId: 420,
},
},
}.GetBytes(),
342, "sourcePort", "sourceChannel", "destinationPort", "destinationChannel", types.Height{}, 0,
),
expectedPacketData: ccv.ConsumerPacketData{
Type: ccv.VscMaturedPacket,
Data: &ccv.ConsumerPacketData_VscMaturedPacketData{
VscMaturedPacketData: &ccv.VSCMaturedPacketData{
ValsetUpdateId: 420,
},
},
},
},
{
name: "slash packet",
packet: channeltypes.NewPacket(
ccv.ConsumerPacketData{
Type: ccv.SlashPacket,
Data: &ccv.ConsumerPacketData_SlashPacketData{
SlashPacketData: &ccv.SlashPacketData{
ValsetUpdateId: 789,
},
},
}.GetBytes(), // Note packet data is converted to v1 bytes here
342, "sourcePort", "sourceChannel", "destinationPort", "destinationChannel", types.Height{}, 0,
),
expectedPacketData: ccv.ConsumerPacketData{
Type: ccv.SlashPacket,
Data: &ccv.ConsumerPacketData_SlashPacketData{
SlashPacketData: &ccv.SlashPacketData{
ValsetUpdateId: 789,
},
},
},
},
}

for _, tc := range testCases {
actualConsumerPacketData, err := provider.UnmarshalConsumerPacket(tc.packet)
require.NoError(t, err)
require.Equal(t, tc.expectedPacketData, actualConsumerPacketData)
}
}
3 changes: 2 additions & 1 deletion x/ccv/provider/proposal_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand Down
61 changes: 60 additions & 1 deletion x/ccv/types/ccv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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:
Expand All @@ -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,
}
}
Loading

0 comments on commit b71d71a

Please sign in to comment.