From 46d49e13e7e09891bd35e0b9f15ac54481d475c8 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 14 Jul 2023 13:52:37 -0700 Subject: [PATCH] handle vsc matured acks --- x/ccv/consumer/keeper/relay.go | 29 +++++++++++++++++------------ x/ccv/consumer/keeper/relay_test.go | 11 ++++++++--- x/ccv/types/ccv.go | 5 +---- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/x/ccv/consumer/keeper/relay.go b/x/ccv/consumer/keeper/relay.go index dce5ca5909..3a11cdb6b1 100644 --- a/x/ccv/consumer/keeper/relay.go +++ b/x/ccv/consumer/keeper/relay.go @@ -239,26 +239,31 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac if len(res) != 1 { k.Logger(ctx).Error("recv invalid ack; expected length 1", "channel", packet.SourceChannel, "ack", res) } + + // Unmarshal into V1 consumer packet data type. We trust data is formed correctly + // as it was originally marshalled by this module, and consumers must trust the provider + // did not tamper with the data. Note ConsumerPacketData.GetBytes() always JSON marshals to the + // ConsumerPacketDataV1 type which is sent over the wire. + var consumerPacket ccv.ConsumerPacketDataV1 + ccv.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &consumerPacket) + // If this ack is regarding a provider handling a vsc matured packet, there's nothing to do. + // As vsc matured packets are popped from the consumer pending packets queue on send. + if consumerPacket.Type == ccv.VscMaturedPacket { + return nil + } + + // Otherwise we handle the result of the slash packet acknowledgement. switch res[0] { + // We treat a v1 result as the provider successfully queuing the slash packet w/o need for retry. case ccv.V1Result[0]: - // If slash record is found, slash packet is at head of queue. - // But provider is running v1 throttling and has queued the slash packet itself. - // Therefore we can clear the slash record and delete the slash packet from the queue, unblocking packet sending. - // TODO: tests for this scenario with vsc matured. - _, found := k.GetSlashRecord(ctx) - if found { - k.ClearSlashRecord(ctx) - k.DeleteHeadOfPendingPackets(ctx) - } - k.Logger(ctx).Info("recv no-op ack", "channel", packet.SourceChannel, "ack", res) + k.ClearSlashRecord(ctx) // Clears slash record state, unblocks sending of pending packets. + k.DeleteHeadOfPendingPackets(ctx) // Remove slash from head of queue. It's been handled. case ccv.SlashPacketHandledResult[0]: k.ClearSlashRecord(ctx) // Clears slash record state, unblocks sending of pending packets. k.DeleteHeadOfPendingPackets(ctx) // Remove slash from head of queue. It's been handled. case ccv.SlashPacketBouncedResult[0]: k.UpdateSlashRecordOnBounce(ctx) // Note slash is still at head of queue and will now be retried after appropriate delay period. - case ccv.VSCMaturedPacketHandledResult[0]: - // VSC matured are deleted upon sending, nothing to do here. default: k.Logger(ctx).Error("recv invalid result ack; expected 1, 2, or 3", "channel", packet.SourceChannel, "ack", res) } diff --git a/x/ccv/consumer/keeper/relay_test.go b/x/ccv/consumer/keeper/relay_test.go index 1a3305f559..7d3decd3a2 100644 --- a/x/ccv/consumer/keeper/relay_test.go +++ b/x/ccv/consumer/keeper/relay_test.go @@ -381,18 +381,19 @@ func TestOnAcknowledgementPacketResult(t *testing.T) { // Setup consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - packet := channeltypes.Packet{} setupSlashBeforeVscMatured(ctx, &consumerKeeper) // Slash record found, 2 pending packets, slash is at head of queue _, found := consumerKeeper.GetSlashRecord(ctx) require.True(t, found) - require.Len(t, consumerKeeper.GetPendingPackets(ctx), 2) - require.Equal(t, types.SlashPacket, consumerKeeper.GetPendingPackets(ctx)[0].Type) + pendingPackets := consumerKeeper.GetPendingPackets(ctx) + require.Len(t, pendingPackets, 2) + require.Equal(t, types.SlashPacket, pendingPackets[0].Type) // v1 result should delete slash record and head of pending packets. Vsc matured remains ack := channeltypes.NewResultAcknowledgement(types.V1Result) + packet := channeltypes.Packet{Data: pendingPackets[0].GetBytes()} err := consumerKeeper.OnAcknowledgementPacket(ctx, packet, ack) require.Nil(t, err) _, found = consumerKeeper.GetSlashRecord(ctx) @@ -402,6 +403,8 @@ func TestOnAcknowledgementPacketResult(t *testing.T) { // refresh state setupSlashBeforeVscMatured(ctx, &consumerKeeper) + pendingPackets = consumerKeeper.GetPendingPackets(ctx) + packet = channeltypes.Packet{Data: pendingPackets[0].GetBytes()} // Slash packet handled result should delete slash record and head of pending packets ack = channeltypes.NewResultAcknowledgement(types.SlashPacketHandledResult) @@ -414,6 +417,8 @@ func TestOnAcknowledgementPacketResult(t *testing.T) { // refresh state setupSlashBeforeVscMatured(ctx, &consumerKeeper) + pendingPackets = consumerKeeper.GetPendingPackets(ctx) + packet = channeltypes.Packet{Data: pendingPackets[0].GetBytes()} slashRecordBefore, found := consumerKeeper.GetSlashRecord(ctx) require.True(t, found) diff --git a/x/ccv/types/ccv.go b/x/ccv/types/ccv.go index 93820b9d6d..25ef502dc0 100644 --- a/x/ccv/types/ccv.go +++ b/x/ccv/types/ccv.go @@ -170,16 +170,13 @@ type PacketAckResult []byte var ( // slice types can't be const // The result ack that has historically been sent from the provider. - // A provider with v1 throttling sends these acks for both slash and vsc matured packets. + // A provider with v1 throttling sends these acks for all successfully recv packets. V1Result = PacketAckResult([]byte{byte(1)}) // Slash packet handled result ack, sent by a throttling v2 provider to indicate that a slash packet was handled. SlashPacketHandledResult = PacketAckResult([]byte{byte(2)}) // Slash packet bounced result ack, sent by a throttling v2 provider to indicate that a slash packet was NOT handled // and should eventually be retried. SlashPacketBouncedResult = PacketAckResult([]byte{byte(3)}) - // VSC matured packet handled result ack, sent by a throttling v2 provider - // to indicate that a vsc matured packet was handled. - VSCMaturedPacketHandledResult = PacketAckResult([]byte{byte(4)}) ) // An exported wrapper around the auto generated isConsumerPacketData_Data interface, only for