Skip to content

Commit 3d64169

Browse files
mergify[bot]MSalopek
mergify[bot]
and
MSalopek
authored
fix: make SlashPacketData backward compatible when sending data over the wire (backport #1093) (#1106)
fix: make SlashPacketData backward compatible when sending data over 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 <marius.poke@posteo.de> Co-authored-by: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> (cherry picked from commit 7a33cb0) Co-authored-by: MSalopek <matija.salopek994@gmail.com>
1 parent 81f5cc9 commit 3d64169

File tree

7 files changed

+919
-93
lines changed

7 files changed

+919
-93
lines changed

‎proto/interchain_security/ccv/v1/ccv.proto

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,42 @@ enum ConsumerPacketDataType {
8484
// VSCMatured packet
8585
CONSUMER_PACKET_TYPE_VSCM = 2
8686
[ (gogoproto.enumvalue_customname) = "VscMaturedPacket" ];
87-
}
87+
}
88+
89+
// ConsumerPacketData contains a consumer packet data and a type tag
90+
// that is compatible with ICS v1 and v2 over the wire. It is not used for internal storage.
91+
message ConsumerPacketDataV1 {
92+
ConsumerPacketDataType type = 1;
93+
94+
oneof data {
95+
SlashPacketDataV1 slashPacketData = 2;
96+
VSCMaturedPacketData vscMaturedPacketData = 3;
97+
}
98+
}
99+
100+
// This packet is sent from the consumer chain to the provider chain
101+
// It is backward compatible with the ICS v1 and v2 version of the packet.
102+
message SlashPacketDataV1 {
103+
tendermint.abci.Validator validator = 1 [
104+
(gogoproto.nullable) = false,
105+
(gogoproto.moretags) = "yaml:\"validator\""
106+
];
107+
// map to the infraction block height on the provider
108+
uint64 valset_update_id = 2;
109+
// tell if the slashing is for a downtime or a double-signing infraction
110+
InfractionType infraction = 3;
111+
}
112+
113+
// InfractionType indicates the infraction type a validator commited.
114+
// NOTE: ccv.InfractionType to maintain compatibility between ICS versions
115+
// using different versions of the cosmos-sdk and ibc-go modules.
116+
enum InfractionType {
117+
option (gogoproto.goproto_enum_prefix) = false;
118+
119+
// UNSPECIFIED defines an empty infraction type.
120+
INFRACTION_TYPE_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "InfractionEmpty"];
121+
// DOUBLE_SIGN defines a validator that double-signs a block.
122+
INFRACTION_TYPE_DOUBLE_SIGN = 1 [(gogoproto.enumvalue_customname) = "DoubleSign"];
123+
// DOWNTIME defines a validator that missed signing too many blocks.
124+
INFRACTION_TYPE_DOWNTIME = 2 [(gogoproto.enumvalue_customname) = "Downtime"];
125+
}

‎tests/integration/throttle.go

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
1010

1111
icstestingutils "github.com/cosmos/interchain-security/v3/testutil/ibc_testing"
12+
"github.com/cosmos/interchain-security/v3/x/ccv/provider"
1213
providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
1314
ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types"
1415
)
@@ -314,8 +315,8 @@ func (s *CCVTestSuite) TestPacketSpam() {
314315

315316
// Recv 500 packets from consumer to provider in same block
316317
for _, packet := range packets {
317-
consumerPacketData := ccvtypes.ConsumerPacketData{}
318-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
318+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
319+
s.Require().NoError(err)
319320
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
320321
}
321322

@@ -368,8 +369,8 @@ func (s *CCVTestSuite) TestDoubleSignDoesNotAffectThrottling() {
368369

369370
// Recv 500 packets from consumer to provider in same block
370371
for _, packet := range packets {
371-
consumerPacketData := ccvtypes.ConsumerPacketData{}
372-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
372+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
373+
s.Require().NoError(err)
373374
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
374375
}
375376

@@ -464,8 +465,10 @@ func (s *CCVTestSuite) TestQueueOrdering() {
464465

465466
// Recv 500 packets from consumer to provider in same block
466467
for i, packet := range packets {
467-
consumerPacketData := ccvtypes.ConsumerPacketData{}
468-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
468+
469+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
470+
s.Require().NoError(err)
471+
469472
// Type depends on index packets were appended from above
470473
if (i+5)%10 == 0 {
471474
vscMaturedPacketData := consumerPacketData.GetVscMaturedPacketData()
@@ -678,8 +681,8 @@ func (s *CCVTestSuite) TestSlashSameValidator() {
678681

679682
// Recv and queue all slash packets.
680683
for _, packet := range packets {
681-
consumerPacketData := ccvtypes.ConsumerPacketData{}
682-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
684+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
685+
s.Require().NoError(err)
683686
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
684687
}
685688

@@ -739,8 +742,8 @@ func (s CCVTestSuite) TestSlashAllValidators() { //nolint:govet // this is a tes
739742

740743
// Recv and queue all slash packets.
741744
for _, packet := range packets {
742-
consumerPacketData := ccvtypes.ConsumerPacketData{}
743-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacketData)
745+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
746+
s.Require().NoError(err)
744747
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
745748
}
746749

@@ -786,10 +789,9 @@ func (s *CCVTestSuite) TestLeadingVSCMaturedAreDequeued() {
786789
ibcSeqNum := uint64(i)
787790
packet := s.constructSlashPacketFromConsumer(*bundle,
788791
*s.providerChain.Vals.Validators[0], stakingtypes.Infraction_INFRACTION_DOWNTIME, ibcSeqNum)
789-
packetData := ccvtypes.ConsumerPacketData{}
790-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
791-
providerKeeper.OnRecvSlashPacket(s.providerCtx(),
792-
packet, *packetData.GetSlashPacketData())
792+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
793+
s.Require().NoError(err)
794+
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
793795
}
794796
}
795797

@@ -877,10 +879,9 @@ func (s *CCVTestSuite) TestVscMaturedHandledPerBlockLimit() {
877879
ibcSeqNum := uint64(i)
878880
packet := s.constructSlashPacketFromConsumer(*bundle,
879881
*s.providerChain.Vals.Validators[0], stakingtypes.Infraction_INFRACTION_DOWNTIME, ibcSeqNum)
880-
packetData := ccvtypes.ConsumerPacketData{}
881-
ccvtypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
882-
providerKeeper.OnRecvSlashPacket(s.providerCtx(),
883-
packet, *packetData.GetSlashPacketData())
882+
consumerPacketData, err := provider.UnmarshalConsumerPacket(packet) // Same func used by provider's OnRecvPacket
883+
s.Require().NoError(err)
884+
providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, *consumerPacketData.GetSlashPacketData())
884885
}
885886
}
886887

‎x/ccv/provider/ibc_module.go

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -174,29 +174,26 @@ func (am AppModule) OnRecvPacket(
174174
packet channeltypes.Packet,
175175
_ sdk.AccAddress,
176176
) ibcexported.Acknowledgement {
177-
var (
178-
ack ibcexported.Acknowledgement
179-
consumerPacket ccv.ConsumerPacketData
180-
)
181-
// unmarshall consumer packet
182-
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacket); err != nil {
183-
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("cannot unmarshal CCV packet data"))
177+
consumerPacket, err := UnmarshalConsumerPacket(packet)
178+
if err != nil {
179+
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, err)
180+
return &errAck
181+
}
182+
183+
// TODO: call ValidateBasic method on consumer packet data
184+
// See: https://github.com/cosmos/interchain-security/issues/634
185+
186+
var ack ibcexported.Acknowledgement
187+
switch consumerPacket.Type {
188+
case ccv.VscMaturedPacket:
189+
// handle VSCMaturedPacket
190+
ack = am.keeper.OnRecvVSCMaturedPacket(ctx, packet, *consumerPacket.GetVscMaturedPacketData())
191+
case ccv.SlashPacket:
192+
// handle SlashPacket
193+
ack = am.keeper.OnRecvSlashPacket(ctx, packet, *consumerPacket.GetSlashPacketData())
194+
default:
195+
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
184196
ack = &errAck
185-
} else {
186-
// TODO: call ValidateBasic method on consumer packet data
187-
// See: https://github.com/cosmos/interchain-security/issues/634
188-
189-
switch consumerPacket.Type {
190-
case ccv.VscMaturedPacket:
191-
// handle VSCMaturedPacket
192-
ack = am.keeper.OnRecvVSCMaturedPacket(ctx, packet, *consumerPacket.GetVscMaturedPacketData())
193-
case ccv.SlashPacket:
194-
// handle SlashPacket
195-
ack = am.keeper.OnRecvSlashPacket(ctx, packet, *consumerPacket.GetSlashPacketData())
196-
default:
197-
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
198-
ack = &errAck
199-
}
200197
}
201198

202199
ctx.EventManager().EmitEvent(
@@ -210,6 +207,33 @@ func (am AppModule) OnRecvPacket(
210207
return ack
211208
}
212209

210+
func UnmarshalConsumerPacket(packet channeltypes.Packet) (consumerPacket ccv.ConsumerPacketData, err error) {
211+
// First try unmarshaling into ccv.ConsumerPacketData type
212+
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacket); err != nil {
213+
// If failed, packet should be a v1 slash packet, retry for ConsumerPacketDataV1 packet type
214+
var v1Packet ccv.ConsumerPacketDataV1
215+
errV1 := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &v1Packet)
216+
if errV1 != nil {
217+
// If neither worked, return error
218+
return ccv.ConsumerPacketData{}, errV1
219+
}
220+
221+
// VSC matured packets should not be unmarshaled as v1 packets
222+
if v1Packet.Type == ccv.VscMaturedPacket {
223+
return ccv.ConsumerPacketData{}, fmt.Errorf("VSC matured packets should be correctly unmarshaled")
224+
}
225+
226+
// Convert from v1 packet type
227+
consumerPacket = ccv.ConsumerPacketData{
228+
Type: v1Packet.Type,
229+
Data: &ccv.ConsumerPacketData_SlashPacketData{
230+
SlashPacketData: v1Packet.GetSlashPacketData().FromV1(),
231+
},
232+
}
233+
}
234+
return consumerPacket, nil
235+
}
236+
213237
// OnAcknowledgementPacket implements the IBCModule interface
214238
func (am AppModule) OnAcknowledgementPacket(
215239
ctx sdk.Context,

‎x/ccv/provider/ibc_module_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
88
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
99

10+
"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
1011
conntypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
1112
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
1213
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
@@ -338,3 +339,62 @@ func TestOnChanOpenConfirm(t *testing.T) {
338339
ctrl.Finish()
339340
}
340341
}
342+
343+
func TestUnmarshalConsumerPacket(t *testing.T) {
344+
testCases := []struct {
345+
name string
346+
packet channeltypes.Packet
347+
expectedPacketData ccv.ConsumerPacketData
348+
}{
349+
{
350+
name: "vsc matured",
351+
packet: channeltypes.NewPacket(
352+
ccv.ConsumerPacketData{
353+
Type: ccv.VscMaturedPacket,
354+
Data: &ccv.ConsumerPacketData_VscMaturedPacketData{
355+
VscMaturedPacketData: &ccv.VSCMaturedPacketData{
356+
ValsetUpdateId: 420,
357+
},
358+
},
359+
}.GetBytes(),
360+
342, "sourcePort", "sourceChannel", "destinationPort", "destinationChannel", types.Height{}, 0,
361+
),
362+
expectedPacketData: ccv.ConsumerPacketData{
363+
Type: ccv.VscMaturedPacket,
364+
Data: &ccv.ConsumerPacketData_VscMaturedPacketData{
365+
VscMaturedPacketData: &ccv.VSCMaturedPacketData{
366+
ValsetUpdateId: 420,
367+
},
368+
},
369+
},
370+
},
371+
{
372+
name: "slash packet",
373+
packet: channeltypes.NewPacket(
374+
ccv.ConsumerPacketData{
375+
Type: ccv.SlashPacket,
376+
Data: &ccv.ConsumerPacketData_SlashPacketData{
377+
SlashPacketData: &ccv.SlashPacketData{
378+
ValsetUpdateId: 789,
379+
},
380+
},
381+
}.GetBytes(), // Note packet data is converted to v1 bytes here
382+
342, "sourcePort", "sourceChannel", "destinationPort", "destinationChannel", types.Height{}, 0,
383+
),
384+
expectedPacketData: ccv.ConsumerPacketData{
385+
Type: ccv.SlashPacket,
386+
Data: &ccv.ConsumerPacketData_SlashPacketData{
387+
SlashPacketData: &ccv.SlashPacketData{
388+
ValsetUpdateId: 789,
389+
},
390+
},
391+
},
392+
},
393+
}
394+
395+
for _, tc := range testCases {
396+
actualConsumerPacketData, err := provider.UnmarshalConsumerPacket(tc.packet)
397+
require.NoError(t, err)
398+
require.Equal(t, tc.expectedPacketData, actualConsumerPacketData)
399+
}
400+
}

‎x/ccv/provider/proposal_handler_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ func TestProviderProposalHandler(t *testing.T) {
8181
{
8282
name: "unsupported proposal type",
8383
// lint rule disabled because this is a test case for an unsupported proposal type
84-
content: &distributiontypes.CommunityPoolSpendProposal{ // nolint:staticcheck
84+
// nolint:staticcheck
85+
content: &distributiontypes.CommunityPoolSpendProposal{
8586
Title: "title",
8687
Description: "desc",
8788
Recipient: "",

‎x/ccv/types/ccv.go

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,23 @@ func NewSlashPacketData(validator abci.Validator, valUpdateId uint64, infraction
5959
}
6060
}
6161

62+
// NewSlashPacketDataV1 creates a new SlashPacketDataV1 that uses ccv.InfractionTypes to maintain backward compatibility.
63+
func NewSlashPacketDataV1(validator abci.Validator, valUpdateId uint64, infractionType stakingtypes.Infraction) *SlashPacketDataV1 {
64+
v1Type := InfractionEmpty
65+
switch infractionType {
66+
case stakingtypes.Infraction_INFRACTION_DOWNTIME:
67+
v1Type = Downtime
68+
case stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN:
69+
v1Type = DoubleSign
70+
}
71+
72+
return &SlashPacketDataV1{
73+
Validator: validator,
74+
ValsetUpdateId: valUpdateId,
75+
Infraction: v1Type,
76+
}
77+
}
78+
6279
func (vdt SlashPacketData) ValidateBasic() error {
6380
if len(vdt.Validator.Address) == 0 || vdt.Validator.Power == 0 {
6481
return errorsmod.Wrap(ErrInvalidPacketData, "validator fields cannot be empty")
@@ -76,6 +93,10 @@ func (vdt SlashPacketData) GetBytes() []byte {
7693
return valDowntimeBytes
7794
}
7895

96+
func (vdt SlashPacketData) ToV1() *SlashPacketDataV1 {
97+
return NewSlashPacketDataV1(vdt.Validator, vdt.ValsetUpdateId, vdt.Infraction)
98+
}
99+
79100
func (cp ConsumerPacketData) ValidateBasic() (err error) {
80101
switch cp.Type {
81102
case VscMaturedPacket:
@@ -99,7 +120,45 @@ func (cp ConsumerPacketData) ValidateBasic() (err error) {
99120
return
100121
}
101122

123+
// Convert to bytes while maintaining over the wire compatibility with previous versions.
102124
func (cp ConsumerPacketData) GetBytes() []byte {
103-
bytes := ModuleCdc.MustMarshalJSON(&cp)
125+
return cp.ToV1Bytes()
126+
}
127+
128+
// ToV1Bytes converts the ConsumerPacketData to JSON byte array compatible
129+
// with the format used by ICS versions using cosmos-sdk v45 (ICS v1 and ICS v2).
130+
func (cp ConsumerPacketData) ToV1Bytes() []byte {
131+
if cp.Type != SlashPacket {
132+
bytes := ModuleCdc.MustMarshalJSON(&cp)
133+
return bytes
134+
}
135+
136+
sp := cp.GetSlashPacketData()
137+
spdv1 := NewSlashPacketDataV1(sp.Validator, sp.ValsetUpdateId, sp.Infraction)
138+
cpv1 := ConsumerPacketDataV1{
139+
Type: cp.Type,
140+
Data: &ConsumerPacketDataV1_SlashPacketData{
141+
SlashPacketData: spdv1,
142+
},
143+
}
144+
bytes := ModuleCdc.MustMarshalJSON(&cpv1)
104145
return bytes
105146
}
147+
148+
// FromV1 converts SlashPacketDataV1 to SlashPacketData.
149+
// Provider must handle both V1 and later versions of the SlashPacketData.
150+
func (vdt1 SlashPacketDataV1) FromV1() *SlashPacketData {
151+
newType := stakingtypes.Infraction_INFRACTION_UNSPECIFIED
152+
switch vdt1.Infraction {
153+
case Downtime:
154+
newType = stakingtypes.Infraction_INFRACTION_DOWNTIME
155+
case DoubleSign:
156+
newType = stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN
157+
}
158+
159+
return &SlashPacketData{
160+
Validator: vdt1.Validator,
161+
ValsetUpdateId: vdt1.ValsetUpdateId,
162+
Infraction: newType,
163+
}
164+
}

0 commit comments

Comments
 (0)