From d82114df2b18c43bf8989ad3443999964592651c Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Wed, 17 Jun 2020 11:36:09 +0200 Subject: [PATCH] ibc/04-channel: move UNORDERED channel ack to RecvPacket (#6457) --- x/ibc/04-channel/keeper/packet.go | 18 +++++---- x/ibc/04-channel/keeper/packet_test.go | 56 ++++++++++++++++++-------- 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/x/ibc/04-channel/keeper/packet.go b/x/ibc/04-channel/keeper/packet.go index 9bb63d53f6..125a1a7827 100644 --- a/x/ibc/04-channel/keeper/packet.go +++ b/x/ibc/04-channel/keeper/packet.go @@ -201,6 +201,17 @@ func (k Keeper) RecvPacket( ) } + // check if the packet acknowledgement has been received already for unordered channels + if channel.Ordering == types.UNORDERED { + _, found := k.GetPacketAcknowledgement(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + if found { + return nil, sdkerrors.Wrapf( + types.ErrInvalidPacket, + "packet sequence (%d) already has been received", packet.GetSequence(), + ) + } + } + if err := k.connectionKeeper.VerifyPacketCommitment( ctx, connectionEnd, proofHeight, proof, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), @@ -245,13 +256,6 @@ func (k Keeper) PacketExecuted( } if len(acknowledgement) > 0 || channel.Ordering == types.UNORDERED { - if _, found := k.GetPacketAcknowledgement(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()); found { - return sdkerrors.Wrapf( - types.ErrInvalidPacket, - "packet sequence (%d) already has been received", packet.GetSequence(), - ) - } - k.SetPacketAcknowledgement( ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), types.CommitAcknowledgement(acknowledgement), diff --git a/x/ibc/04-channel/keeper/packet_test.go b/x/ibc/04-channel/keeper/packet_test.go index c92a5f4f27..fa43996174 100644 --- a/x/ibc/04-channel/keeper/packet_test.go +++ b/x/ibc/04-channel/keeper/packet_test.go @@ -123,7 +123,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { var packet exported.PacketI testCases := []testCase{ - {"success", func() { + {"success ordered channel", func() { suite.chainB.CreateClient(suite.chainA) suite.chainA.CreateClient(suite.chainB) suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.OPEN) @@ -133,40 +133,64 @@ func (suite *KeeperTestSuite) TestRecvPacket() { suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceSend(suite.chainA.GetContext(), testPort2, testChannel2, 1) packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), testPort2, testChannel2, 1, types.CommitPacket(packet)) - + }, true}, + {"success unordered channel", func() { + suite.chainB.CreateClient(suite.chainA) + suite.chainA.CreateClient(suite.chainB) + suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.OPEN) + suite.chainA.createConnection(testConnectionIDB, testConnectionIDA, testClientIDB, testClientIDA, connection.OPEN) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.UNORDERED, testConnectionIDA) + suite.chainA.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.OPEN, types.UNORDERED, testConnectionIDB) + suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceSend(suite.chainA.GetContext(), testPort2, testChannel2, 1) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), testPort2, testChannel2, 1, types.CommitPacket(packet)) }, true}, {"channel not found", func() {}, false}, {"channel not open", func() { packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) - suite.chainB.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.INIT, types.ORDERED, testConnectionIDA) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.CLOSED, types.ORDERED, testConnectionIDA) }, false}, {"packet source port ≠ channel counterparty port", func() { - packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) - suite.chainB.createChannel(testPort2, testChannel2, testPort3, testChannel1, types.OPEN, types.ORDERED, testConnectionIDA) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + suite.chainB.createChannel(testPort1, testChannel1, testPort3, testChannel2, types.OPEN, types.ORDERED, testConnectionIDA) }, false}, {"packet source channel ID ≠ channel counterparty channel ID", func() { - packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) - suite.chainB.createChannel(testPort2, testChannel2, testPort1, testChannel3, types.OPEN, types.ORDERED, testConnectionIDA) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel3, types.OPEN, types.ORDERED, testConnectionIDA) }, false}, {"connection not found", func() { - packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) - suite.chainB.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.OPEN, types.ORDERED, testConnectionIDA) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.ORDERED, testConnectionIDA) }, false}, {"connection not OPEN", func() { - packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.INIT) - suite.chainB.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.OPEN, types.ORDERED, testConnectionIDA) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.ORDERED, testConnectionIDA) }, false}, {"timeout height passed", func() { - commitNBlocks(suite.chainB, 10) - packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), 1, disabledTimeoutTimestamp) suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.OPEN) - suite.chainB.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.OPEN, types.ORDERED, testConnectionIDA) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.ORDERED, testConnectionIDA) + }, false}, + {"timeout timestamp passed", func() { + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, 1) + suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.OPEN) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.ORDERED, testConnectionIDA) + }, false}, + {"acknowledgement already received", func() { + suite.chainB.CreateClient(suite.chainA) + suite.chainA.CreateClient(suite.chainB) + suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.OPEN) + suite.chainA.createConnection(testConnectionIDB, testConnectionIDA, testClientIDB, testClientIDA, connection.OPEN) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.UNORDERED, testConnectionIDA) + suite.chainA.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.OPEN, types.UNORDERED, testConnectionIDB) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + suite.chainB.storeAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) }, false}, {"validation failed", func() { - packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort2, testChannel2, counterparty.GetPortID(), counterparty.GetChannelID(), timeoutHeight, disabledTimeoutTimestamp) suite.chainB.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, connection.OPEN) - suite.chainB.createChannel(testPort2, testChannel2, testPort1, testChannel1, types.OPEN, types.ORDERED, testConnectionIDA) + suite.chainB.createChannel(testPort1, testChannel1, testPort2, testChannel2, types.OPEN, types.ORDERED, testConnectionIDA) }, false}, }