From 03a46181c6edfdad5e74654f88f789fcbb3c178b Mon Sep 17 00:00:00 2001 From: Aditya Date: Thu, 5 Nov 2020 14:07:15 +0000 Subject: [PATCH] Merge Packet Functions (#7813) * start implementation and debugging tests * fix some issues, continue debugging * fix IBC tests * remove print statement and fix docs Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../core/03-connection/keeper/verify_test.go | 12 +- x/ibc/core/04-channel/keeper/packet.go | 192 +++++------ x/ibc/core/04-channel/keeper/packet_test.go | 297 +++++++----------- x/ibc/core/keeper/msg_server.go | 15 +- x/ibc/core/keeper/msg_server_test.go | 47 +-- .../07-tendermint/types/client_state_test.go | 16 +- x/ibc/testing/chain.go | 43 +-- x/ibc/testing/coordinator.go | 38 --- 8 files changed, 225 insertions(+), 435 deletions(-) diff --git a/x/ibc/core/03-connection/keeper/verify_test.go b/x/ibc/core/03-connection/keeper/verify_test.go index 53fd3be8ff..7f9705a80b 100644 --- a/x/ibc/core/03-connection/keeper/verify_test.go +++ b/x/ibc/core/03-connection/keeper/verify_test.go @@ -11,6 +11,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" + ibcmock "github.com/cosmos/cosmos-sdk/x/ibc/testing/mock" ) var defaultTimeoutHeight = clienttypes.NewHeight(0, 100000) @@ -336,16 +337,13 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) packetAckKey := host.KeyPacketAcknowledgement(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) proof, proofHeight := suite.chainB.QueryProof(packetAckKey) - ack := ibctesting.TestHash + ack := ibcmock.MockAcknowledgement if tc.changeAcknowledgement { ack = []byte(ibctesting.InvalidID) } @@ -399,7 +397,7 @@ func (suite *KeeperTestSuite) TestVerifyPacketReceiptAbsence() { suite.Require().NoError(err) if tc.recvAck { - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) } else { // need to update height to prove absence @@ -458,7 +456,7 @@ func (suite *KeeperTestSuite) TestVerifyNextSequenceRecv() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) nextSeqRecvKey := host.KeyNextSequenceRecv(packet.GetDestPort(), packet.GetDestChannel()) diff --git a/x/ibc/core/04-channel/keeper/packet.go b/x/ibc/core/04-channel/keeper/packet.go index d1d7b63920..87ba9ce337 100644 --- a/x/ibc/core/04-channel/keeper/packet.go +++ b/x/ibc/core/04-channel/keeper/packet.go @@ -137,6 +137,7 @@ func (k Keeper) SendPacket( // sent on the corresponding channel end on the counterparty chain. func (k Keeper) RecvPacket( ctx sdk.Context, + chanCap *capabilitytypes.Capability, packet exported.PacketI, proof []byte, proofHeight exported.Height, @@ -153,8 +154,14 @@ func (k Keeper) RecvPacket( ) } - // NOTE: RecvPacket is called by the AnteHandler which acts upon the packet.Route(), - // so the capability authentication can be omitted here + // Authenticate capability to ensure caller has authority to receive packet on this channel + capName := host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel()) + if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { + return sdkerrors.Wrapf( + types.ErrInvalidChannelCapability, + "channel capability failed authentication for capability name %s", capName, + ) + } // packet must come from the channel's counterparty if packet.GetSourcePort() != channel.Counterparty.PortId { @@ -171,6 +178,9 @@ func (k Keeper) RecvPacket( ) } + // Connection must be OPEN to receive a packet. It is possible for connection to not yet be open if packet was + // sent optimistically before connection and channel handshake completed. However, to receive a packet, + // connection and channel must both be open connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0]) if !found { return sdkerrors.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) @@ -201,6 +211,15 @@ func (k Keeper) RecvPacket( ) } + // verify that the counterparty did commit to sending this packet + if err := k.connectionKeeper.VerifyPacketCommitment( + ctx, connectionEnd, proofHeight, proof, + packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), + types.CommitPacket(packet), + ); err != nil { + return sdkerrors.Wrap(err, "couldn't verify counterparty packet commitment") + } + switch channel.Ordering { case types.UNORDERED: // check if the packet receipt has been received already for unordered channels @@ -212,6 +231,12 @@ func (k Keeper) RecvPacket( ) } + // All verification complete, update state + // For unordered channels we must set the receipt so it can be verified on the other side. + // This receipt does not contain any data, since the packet has not yet been processed, + // it's just a single store key set to an empty string to indicate that the packet has been received + k.SetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + case types.ORDERED: // check if the packet is being received in order nextSequenceRecv, found := k.GetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel()) @@ -228,79 +253,15 @@ func (k Keeper) RecvPacket( "packet sequence ≠ next receive sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceRecv, ) } - } - - if err := k.connectionKeeper.VerifyPacketCommitment( - ctx, connectionEnd, proofHeight, proof, - packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), - types.CommitPacket(packet), - ); err != nil { - return sdkerrors.Wrap(err, "couldn't verify counterparty packet commitment") - } - - // NOTE: the remaining code is located in the WriteReceipt function - return nil -} - -// WriteReceipt updates the receive sequence in the case of an ordered channel or sets an empty receipt -// if the channel is unordered. -// -// CONTRACT: this function must be called in the IBC handler -func (k Keeper) WriteReceipt( - ctx sdk.Context, - chanCap *capabilitytypes.Capability, - packet exported.PacketI, -) error { - channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel()) - if !found { - return sdkerrors.Wrapf(types.ErrChannelNotFound, packet.GetDestChannel()) - } - - // sanity check - if channel.State != types.OPEN { - return sdkerrors.Wrapf( - types.ErrInvalidChannelState, - "channel state is not OPEN (got %s)", channel.State.String(), - ) - } - - capName := host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel()) - if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { - return sdkerrors.Wrapf( - types.ErrInvalidChannelCapability, - "channel capability failed authentication for capability name %s", capName, - ) - } - - switch channel.Ordering { - case types.ORDERED: - nextSequenceRecv, found := k.GetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel()) - if !found { - return sdkerrors.Wrapf( - types.ErrSequenceReceiveNotFound, - "destination port: %s, destination channel: %s", packet.GetDestPort(), packet.GetDestChannel(), - ) - } + // All verification complete, update state + // In ordered case, we must increment nextSequenceRecv nextSequenceRecv++ // incrementing nextSequenceRecv and storing under this chain's channelEnd identifiers // Since this is the receiving chain, our channelEnd is packet's destination port and channel k.SetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel(), nextSequenceRecv) - case types.UNORDERED: - // For unordered channels we must set the receipt so it can be verified on the other side. - // This receipt does not contain any data, since the packet has not yet been processed, - // it's just a single store key set to an empty string to indicate that the packet has been received - _, found := k.GetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) - if found { - return sdkerrors.Wrapf( - types.ErrPacketReceived, - "destination port: %s, destination channel: %s, sequence: %d", packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), - ) - } - - k.SetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) } // log that a packet has been received & executed @@ -338,12 +299,35 @@ func (k Keeper) WriteReceipt( // For async handling, it needs to be called directly by the module which originally // processed the packet. // -// 2) Assumes that packet receipt has been writted previously by WriteReceipt. +// 2) Assumes that packet receipt has been written (unordered), or nextSeqRecv was incremented (ordered) +// previously by RecvPacket. func (k Keeper) WriteAcknowledgement( ctx sdk.Context, + chanCap *capabilitytypes.Capability, packet exported.PacketI, acknowledgement []byte, ) error { + channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel()) + if !found { + return sdkerrors.Wrap(types.ErrChannelNotFound, packet.GetDestChannel()) + } + + if channel.State != types.OPEN { + return sdkerrors.Wrapf( + types.ErrInvalidChannelState, + "channel state is not OPEN (got %s)", channel.State.String(), + ) + } + + // Authenticate capability to ensure caller has authority to receive packet on this channel + capName := host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel()) + if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { + return sdkerrors.Wrapf( + types.ErrInvalidChannelCapability, + "channel capability failed authentication for capability name %s", capName, + ) + } + // NOTE: IBC app modules might have written the acknowledgement synchronously on // the OnRecvPacket callback so we need to check if the acknowledgement is already // set on the store and return an error if so. @@ -355,14 +339,14 @@ func (k Keeper) WriteAcknowledgement( return sdkerrors.Wrap(types.ErrInvalidAcknowledgement, "acknowledgement cannot be empty") } - // always set the acknowledgement so that it can be verified on the other side + // set the acknowledgement so that it can be verified on the other side k.SetPacketAcknowledgement( ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), types.CommitAcknowledgement(acknowledgement), ) - // log that a packet has been acknowledged - k.Logger(ctx).Info("packet acknowledged", "packet", fmt.Sprintf("%v", packet)) + // log that a packet acknowledgement has been written + k.Logger(ctx).Info("acknowledged written", "packet", fmt.Sprintf("%v", packet)) // emit an event that the relayer can query for ctx.EventManager().EmitEvents(sdk.Events{ @@ -390,11 +374,12 @@ func (k Keeper) WriteAcknowledgement( // AcknowledgePacket is called by a module to process the acknowledgement of a // packet previously sent by the calling module on a channel to a counterparty // module on the counterparty chain. Its intended usage is within the ante -// handler. A subsequent call to AcknowledgementExecuted will clean up the -// packet commitment, which is no longer necessary since the packet has been -// received and acted upon. +// handler. AcknowledgePacket will clean up the packet commitment, +// which is no longer necessary since the packet has been received and acted upon. +// It will also increment NextSequenceAck in case of ORDERED channels. func (k Keeper) AcknowledgePacket( ctx sdk.Context, + chanCap *capabilitytypes.Capability, packet exported.PacketI, acknowledgement []byte, proof []byte, @@ -415,8 +400,14 @@ func (k Keeper) AcknowledgePacket( ) } - // NOTE: AcknowledgePacket is called by the AnteHandler which acts upon the packet.Route(), - // so the capability authentication can be omitted here + // Authenticate capability to ensure caller has authority to receive packet on this channel + capName := host.ChannelCapabilityPath(packet.GetSourcePort(), packet.GetSourceChannel()) + if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { + return sdkerrors.Wrapf( + types.ErrInvalidChannelCapability, + "channel capability failed authentication for capability name %s", capName, + ) + } // packet must have been sent to the channel's counterparty if packet.GetDestPort() != channel.Counterparty.PortId { @@ -475,56 +466,19 @@ func (k Keeper) AcknowledgePacket( "packet sequence ≠ next ack sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceAck, ) } - } - - // NOTE: the remaining code is located in the AcknowledgementExecuted function - return nil -} - -// AcknowledgementExecuted deletes the packet commitment from this chain. -// It is assumed that the acknowledgement verification has already occurred. -// -// CONTRACT: this function must be called in the IBC handler -func (k Keeper) AcknowledgementExecuted( - ctx sdk.Context, - chanCap *capabilitytypes.Capability, - packet exported.PacketI, -) error { - channel, found := k.GetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) - if !found { - return sdkerrors.Wrapf( - types.ErrChannelNotFound, - "port ID (%s) channel ID (%s)", packet.GetSourcePort(), packet.GetSourceChannel(), - ) - } - - capName := host.ChannelCapabilityPath(packet.GetSourcePort(), packet.GetSourceChannel()) - if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { - return sdkerrors.Wrapf( - types.ErrInvalidChannelCapability, - "channel capability failed authentication for capability name %s", capName, - ) - } - - k.deletePacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) - - // increment NextSequenceAck - if channel.Ordering == types.ORDERED { - nextSequenceAck, found := k.GetNextSequenceAck(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) - if !found { - return sdkerrors.Wrapf( - types.ErrSequenceAckNotFound, - "source port: %s, source channel: %s", packet.GetSourcePort(), packet.GetSourceChannel(), - ) - } + // All verification complete, in the case of ORDERED channels we must increment nextSequenceAck nextSequenceAck++ // incrementing NextSequenceAck and storing under this chain's channelEnd identifiers // Since this is the original sending chain, our channelEnd is packet's source port and channel k.SetNextSequenceAck(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), nextSequenceAck) + } + // Delete packet commitment, since the packet has been acknowledged, the commitement is no longer necessary + k.deletePacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + // log that a packet has been acknowledged k.Logger(ctx).Info("packet acknowledged", "packet", fmt.Sprintf("%v", packet)) diff --git a/x/ibc/core/04-channel/keeper/packet_test.go b/x/ibc/core/04-channel/keeper/packet_test.go index e2cc85459f..3d942ce12e 100644 --- a/x/ibc/core/04-channel/keeper/packet_test.go +++ b/x/ibc/core/04-channel/keeper/packet_test.go @@ -9,6 +9,7 @@ import ( host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" + ibcmock "github.com/cosmos/cosmos-sdk/x/ibc/testing/mock" ) var ( @@ -179,7 +180,8 @@ func (suite *KeeperTestSuite) TestSendPacket() { // verification tests need to simulate sending a packet from chainA to chainB. func (suite *KeeperTestSuite) TestRecvPacket() { var ( - packet exported.PacketI + packet exported.PacketI + channelCap *capabilitytypes.Capability ) testCases := []testCase{ @@ -188,6 +190,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, true}, {"success UNORDERED channel", func() { // setup uses an UNORDERED channel @@ -195,6 +198,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, true}, {"success with out of order packet: UNORDERED channel", func() { // setup uses an UNORDERED channel @@ -209,6 +213,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { err = suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) // attempts to receive packet 2 without receiving packet 1 + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, true}, {"out of order packet failure with ORDERED channel", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) @@ -222,11 +227,13 @@ func (suite *KeeperTestSuite) TestRecvPacket() { err = suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) // attempts to receive packet 2 without receiving packet 1 + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"channel not found", func() { // use wrong channel naming - _, _, _, _, channelA, _ := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) + _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"channel not open", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) @@ -234,16 +241,26 @@ func (suite *KeeperTestSuite) TestRecvPacket() { err := suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) suite.Require().NoError(err) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) + }, false}, + {"capability cannot authenticate", func() { + _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) + packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + suite.Require().NoError(err) + channelCap = capabilitytypes.NewCapability(3) }, false}, {"packet source port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"packet source channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest packet = types.NewPacket(validPacketData, 1, channelA.PortID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"connection not found", func() { channelA := ibctesting.TestChannel{PortID: portID, ID: channelIDA} @@ -255,6 +272,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() { types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connIDB}, channelB.Version), ) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"connection not OPEN", func() { clientA, clientB := suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint) @@ -271,14 +290,18 @@ func (suite *KeeperTestSuite) TestRecvPacket() { types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connB.ID}, channelB.Version), ) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"timeout height passed", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"timeout timestamp passed", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, disabledTimeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"next receive sequence is not found", func() { _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, ibctesting.Tendermint) @@ -296,11 +319,22 @@ func (suite *KeeperTestSuite) TestRecvPacket() { // manually set packet commitment suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.TestHash) + suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) + + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) + }, false}, + {"receipt already stored", func() { + _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) + packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketReceipt(suite.chainB.GetContext(), channelB.PortID, channelB.ID, 1) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"validation failed", func() { // packet commitment not set resulting in invalid proof _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, } @@ -314,10 +348,24 @@ func (suite *KeeperTestSuite) TestRecvPacket() { packetKey := host.KeyPacketCommitment(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) proof, proofHeight := suite.chainA.QueryProof(packetKey) - err := suite.chainB.App.IBCKeeper.ChannelKeeper.RecvPacket(suite.chainB.GetContext(), packet, proof, proofHeight) + err := suite.chainB.App.IBCKeeper.ChannelKeeper.RecvPacket(suite.chainB.GetContext(), channelCap, packet, proof, proofHeight) if tc.expPass { suite.Require().NoError(err) + + channelB, _ := suite.chainB.App.IBCKeeper.ChannelKeeper.GetChannel(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel()) + nextSeqRecv, found := suite.chainB.App.IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel()) + suite.Require().True(found) + receipt, receiptStored := suite.chainB.App.IBCKeeper.ChannelKeeper.GetPacketReceipt(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + + if channelB.Ordering == types.ORDERED { + suite.Require().Equal(packet.GetSequence()+1, nextSeqRecv, "sequence not incremented in ordered channel") + suite.Require().False(receiptStored, "packet receipt stored on ORDERED channel") + } else { + suite.Require().Equal(uint64(1), nextSeqRecv, "sequence incremented for UNORDERED channel") + suite.Require().True(receiptStored, "packet receipt not stored after RecvPacket in UNORDERED channel") + suite.Require().Equal("", receipt, "packet receipt is not empty string") + } } else { suite.Require().Error(err) } @@ -326,92 +374,11 @@ func (suite *KeeperTestSuite) TestRecvPacket() { } -// TestWriteReceipt tests the WriteReceipt call on chainB. -func (suite *KeeperTestSuite) TestWriteReceipt() { - var ( - packet types.Packet - channelCap *capabilitytypes.Capability - ) - - testCases := []testCase{ - {"success: UNORDERED", func() { - // setup uses an UNORDERED channel - _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) - channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) - }, true}, - {"success: ORDERED", func() { - _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) - channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) - }, true}, - {"channel not found", func() { - // use wrong channel naming - _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) - channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) - }, false}, - {"channel not OPEN", func() { - _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - - err := suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) - suite.Require().NoError(err) - }, false}, - {"next sequence receive not found", func() { - _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, ibctesting.Tendermint) - channelA := connA.NextTestChannel(ibctesting.TransferPort) - channelB := connB.NextTestChannel(ibctesting.TransferPort) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - // manually creating channel prevents next sequence receive from being set - suite.chainB.App.IBCKeeper.ChannelKeeper.SetChannel( - suite.chainB.GetContext(), - channelB.PortID, channelB.ID, - types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connB.ID}, channelB.Version), - ) - suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) - channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) - }, false}, - {"capability not found", func() { - _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) - - channelCap = capabilitytypes.NewCapability(3) - }, false}, - {"receipt already stored", func() { - _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) - suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketReceipt(suite.chainB.GetContext(), channelB.PortID, channelB.ID, 1) - channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) - }, false}, - } - - for i, tc := range testCases { - tc := tc - suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() { - suite.SetupTest() // reset - - tc.malleate() - - err := suite.chainB.App.IBCKeeper.ChannelKeeper.WriteReceipt(suite.chainB.GetContext(), channelCap, packet) - - if tc.expPass { - suite.Require().NoError(err) - } else { - suite.Require().Error(err) - } - }) - } -} - func (suite *KeeperTestSuite) TestWriteAcknowledgement() { var ( - ack []byte - packet exported.PacketI + ack []byte + packet exported.PacketI + channelCap *capabilitytypes.Capability ) testCases := []testCase{ @@ -421,9 +388,36 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) ack = ibctesting.TestHash + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, true, }, + {"channel not found", func() { + // use wrong channel naming + _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) + packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.TestHash + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) + }, false}, + {"channel not open", func() { + _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) + packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.TestHash + + err := suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) + suite.Require().NoError(err) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) + }, false}, + { + "capability authentication failed", + func() { + _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) + packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.TestHash + channelCap = capabilitytypes.NewCapability(3) + }, + false, + }, { "no-op, already acked", func() { @@ -431,6 +425,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) ack = ibctesting.TestHash suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack) + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false, }, @@ -440,6 +435,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) ack = nil + channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false, }, @@ -451,7 +447,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { tc.malleate() - err := suite.chainB.App.IBCKeeper.ChannelKeeper.WriteAcknowledgement(suite.chainB.GetContext(), packet, ack) + err := suite.chainB.App.IBCKeeper.ChannelKeeper.WriteAcknowledgement(suite.chainB.GetContext(), channelCap, packet, ack) if tc.expPass { suite.Require().NoError(err) @@ -466,7 +462,9 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { func (suite *KeeperTestSuite) TestAcknowledgePacket() { var ( packet types.Packet - ack = ibctesting.TestHash + ack = ibcmock.MockAcknowledgement + + channelCap *capabilitytypes.Capability ) testCases := []testCase{ @@ -478,11 +476,10 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) // create packet receipt and acknowledgement - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, true}, {"success on unordered channel", func() { // setup uses an UNORDERED channel @@ -494,11 +491,10 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) // create packet receipt and acknowledgement - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, true}, {"channel not found", func() { // use wrong channel naming @@ -511,16 +507,32 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { err := suite.coordinator.SetChannelClosed(suite.chainA, suite.chainB, channelA) suite.Require().NoError(err) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) + }, false}, + {"capability authentication failed", func() { + clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) + packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + // create packet commitment + err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + suite.Require().NoError(err) + + // create packet receipt and acknowledgement + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) + suite.Require().NoError(err) + + channelCap = capabilitytypes.NewCapability(3) }, false}, {"packet destination port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet destination channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong channel for dest packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"connection not found", func() { channelA := ibctesting.TestChannel{PortID: portID, ID: channelIDA} @@ -532,6 +544,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connIDB}, channelB.Version), ) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"connection not OPEN", func() { clientA, clientB := suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint) @@ -548,11 +562,14 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelB.PortID, channelB.ID), []string{connA.ID}, channelA.Version), ) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet hasn't been sent", func() { // packet commitment never written _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet ack verification failed", func() { // ack never written @@ -561,6 +578,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { // create packet commitment suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"next ack sequence not found", func() { _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, ibctesting.Tendermint) @@ -576,8 +594,10 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { // manually set packet commitment suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.TestHash) - // manually set packet acknowledgement + // manually set packet acknowledgement and capability suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), channelB.PortID, channelB.ID, packet.GetSequence(), ibctesting.TestHash) + suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"next ack sequence mismatch", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) @@ -587,14 +607,12 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) // create packet acknowledgement - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) // set next sequence ack wrong suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceAck(suite.chainA.GetContext(), channelA.PortID, channelA.ID, 10) + channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, } @@ -607,84 +625,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { packetKey := host.KeyPacketAcknowledgement(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) proof, proofHeight := suite.chainB.QueryProof(packetKey) - err := suite.chainA.App.IBCKeeper.ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), packet, ack, proof, proofHeight) - - if tc.expPass { - suite.Require().NoError(err) - } else { - suite.Require().Error(err) - } - }) - } -} - -// TestAcknowledgementExectued verifies that packet commitments are deleted on chainA -// after capabilities are verified. -func (suite *KeeperTestSuite) TestAcknowledgementExecuted() { - var ( - packet types.Packet - chanCap *capabilitytypes.Capability - ) - - testCases := []testCase{ - {"success ORDERED", func() { - clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - - // create packet commitment - err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) - suite.Require().NoError(err) - - // create packet receipt and acknowledgement - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) - - }, true}, - {"success UNORDERED", func() { - // setup uses an UNORDERED channel - clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - - // create packet commitment - err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) - suite.Require().NoError(err) - - // create packet receipt and acknowledgement - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) - }, true}, - {"channel not found", func() { - // use wrong channel naming - _, _, _, _, _, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - }, false}, - {"incorrect capability", func() { - _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - - chanCap = capabilitytypes.NewCapability(100) - }, false}, - } - - for i, tc := range testCases { - tc := tc - suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() { - suite.SetupTest() // reset - - tc.malleate() - - err := suite.chainA.App.IBCKeeper.ChannelKeeper.AcknowledgementExecuted(suite.chainA.GetContext(), chanCap, packet) + err := suite.chainA.App.IBCKeeper.ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ack, proof, proofHeight) pc := suite.chainA.App.IBCKeeper.ChannelKeeper.GetPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) channelA, _ := suite.chainA.App.IBCKeeper.ChannelKeeper.GetChannel(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel()) diff --git a/x/ibc/core/keeper/msg_server.go b/x/ibc/core/keeper/msg_server.go index d26ea627b0..01d3dae311 100644 --- a/x/ibc/core/keeper/msg_server.go +++ b/x/ibc/core/keeper/msg_server.go @@ -431,7 +431,7 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke } // Perform TAO verification - if err := k.ChannelKeeper.RecvPacket(ctx, msg.Packet, msg.Proof, msg.ProofHeight); err != nil { + if err := k.ChannelKeeper.RecvPacket(ctx, cap, msg.Packet, msg.Proof, msg.ProofHeight); err != nil { return nil, sdkerrors.Wrap(err, "receive packet verification failed") } @@ -441,15 +441,11 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke return nil, sdkerrors.Wrap(err, "receive packet callback failed") } - if err := k.ChannelKeeper.WriteReceipt(ctx, cap, msg.Packet); err != nil { - return nil, err - } - // Set packet acknowledgement only if the acknowledgement is not nil. // NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the // acknowledgement is nil. if ack != nil { - if err := k.ChannelKeeper.WriteAcknowledgement(ctx, msg.Packet, ack); err != nil { + if err := k.ChannelKeeper.WriteAcknowledgement(ctx, cap, msg.Packet, ack); err != nil { return nil, err } } @@ -586,7 +582,7 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn } // Perform TAO verification - if err := k.ChannelKeeper.AcknowledgePacket(ctx, msg.Packet, msg.Acknowledgement, msg.Proof, msg.ProofHeight); err != nil { + if err := k.ChannelKeeper.AcknowledgePacket(ctx, cap, msg.Packet, msg.Acknowledgement, msg.Proof, msg.ProofHeight); err != nil { return nil, sdkerrors.Wrap(err, "acknowledge packet verification failed") } @@ -596,11 +592,6 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn return nil, sdkerrors.Wrap(err, "acknowledge packet callback failed") } - // Delete packet commitment - if err = k.ChannelKeeper.AcknowledgementExecuted(ctx, cap, msg.Packet); err != nil { - return nil, err - } - defer func() { telemetry.IncrCounterWithLabels( []string{"tx", "msg", "ibc", msg.Type()}, diff --git a/x/ibc/core/keeper/msg_server_test.go b/x/ibc/core/keeper/msg_server_test.go index 0e1f6aab77..0bed10ae7a 100644 --- a/x/ibc/core/keeper/msg_server_test.go +++ b/x/ibc/core/keeper/msg_server_test.go @@ -15,6 +15,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/core/keeper" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" + ibcmock "github.com/cosmos/cosmos-sdk/x/ibc/testing/mock" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" ) @@ -49,7 +50,7 @@ func TestIBCTestSuite(t *testing.T) { // tests the IBC handler receiving a packet on ordered and unordered channels. // It verifies that the storing of an acknowledgement on success occurs. It // tests high level properties like ordering and basic sanity checks. More -// rigorous testing of 'RecvPacket' and 'WriteReceipt' can be found in the +// rigorous testing of 'RecvPacket' can be found in the // 04-channel/keeper/packet_test.go. func (suite *KeeperTestSuite) TestHandleRecvPacket() { var ( @@ -113,7 +114,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) }, false}, {"UNORDERED: packet already received (replay)", func() { @@ -124,7 +125,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) }, false}, } @@ -167,7 +168,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { // tests the IBC handler acknowledgement of a packet on ordered and unordered // channels. It verifies that the deletion of packet commitments from state // occurs. It test high level properties like ordering and basic sanity -// checks. More rigorous testing of 'AcknowledgePacket' and 'AcknowledgementExecuted' +// checks. More rigorous testing of 'AcknowledgePacket' // can be found in the 04-channel/keeper/packet_test.go. func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { var ( @@ -186,10 +187,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) }, true}, {"success: UNORDERED", func() { @@ -199,10 +197,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) }, true}, {"success: UNORDERED acknowledge out of order packet", func() { @@ -216,10 +211,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) } }, true}, @@ -233,10 +225,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) } }, false}, @@ -258,16 +247,13 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.AcknowledgementExecuted(suite.chainA, suite.chainB, packet, clientB) + err = suite.coordinator.AcknowledgePacket(suite.chainA, suite.chainB, clientB, packet, ibctesting.TestHash) suite.Require().NoError(err) }, false}, - {"UNORDERED: packet already received (replay)", func() { + {"UNORDERED: packet already acknowledged (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) @@ -275,13 +261,10 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.AcknowledgementExecuted(suite.chainA, suite.chainB, packet, clientB) + err = suite.coordinator.AcknowledgePacket(suite.chainA, suite.chainB, clientB, packet, ibctesting.TestHash) suite.Require().NoError(err) }, false}, } @@ -298,7 +281,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { packetKey := host.KeyPacketAcknowledgement(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) proof, proofHeight := suite.chainB.QueryProof(packetKey) - msg := channeltypes.NewMsgAcknowledgement(packet, ibctesting.MockAcknowledgement, proof, proofHeight, suite.chainA.SenderAccount.GetAddress()) + msg := channeltypes.NewMsgAcknowledgement(packet, ibcmock.MockAcknowledgement, proof, proofHeight, suite.chainA.SenderAccount.GetAddress()) _, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.IBCKeeper, sdk.WrapSDKContext(suite.chainA.GetContext()), msg) diff --git a/x/ibc/light-clients/07-tendermint/types/client_state_test.go b/x/ibc/light-clients/07-tendermint/types/client_state_test.go index b032e68b12..4f82044664 100644 --- a/x/ibc/light-clients/07-tendermint/types/client_state_test.go +++ b/x/ibc/light-clients/07-tendermint/types/client_state_test.go @@ -10,6 +10,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" + ibcmock "github.com/cosmos/cosmos-sdk/x/ibc/testing/mock" ) const ( @@ -465,10 +466,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { suite.Require().NoError(err) // write receipt and ack - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) - suite.Require().NoError(err) - - err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA) + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) var ok bool @@ -488,7 +486,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { err = clientState.VerifyPacketAcknowledgement( store, suite.chainA.Codec, proofHeight, &prefix, proof, - packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ibctesting.TestHash, + packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ibcmock.MockAcknowledgement, ) if tc.expPass { @@ -635,15 +633,15 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { suite.SetupTest() // reset // setup testing conditions - clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) + clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) // send packet err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - // write receipt, next seq recv incremented - err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA) + // next seq recv incremented + err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) // need to update chainA's client representing chainB @@ -666,7 +664,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { err = clientState.VerifyNextSequenceRecv( store, suite.chainA.Codec, proofHeight, &prefix, proof, - packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), + packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()+1, ) if tc.expPass { diff --git a/x/ibc/testing/chain.go b/x/ibc/testing/chain.go index 4ef4676eef..2c0554c6aa 100644 --- a/x/ibc/testing/chain.go +++ b/x/ibc/testing/chain.go @@ -881,51 +881,14 @@ func (chain *TestChain) SendPacket( return nil } -// WriteReceipt simulates receiving and writing a receipt to the chain. -func (chain *TestChain) WriteReceipt( +// WriteAcknowledgement simulates writing an acknowledgement to the chain. +func (chain *TestChain) WriteAcknowledgement( packet exported.PacketI, ) error { channelCap := chain.GetChannelCapability(packet.GetDestPort(), packet.GetDestChannel()) // no need to send message, acting as a handler - err := chain.App.IBCKeeper.ChannelKeeper.WriteReceipt(chain.GetContext(), channelCap, packet) - if err != nil { - return err - } - - // commit changes - chain.App.Commit() - chain.NextBlock() - - return nil -} - -// WriteAcknowledgement simulates writing an acknowledgement to the chain. -func (chain *TestChain) WriteAcknowledgement( - packet exported.PacketI, -) error { - // no need to send message, acting as a handler - err := chain.App.IBCKeeper.ChannelKeeper.WriteAcknowledgement(chain.GetContext(), packet, TestHash) - if err != nil { - return err - } - - // commit changes - chain.App.Commit() - chain.NextBlock() - - return nil -} - -// AcknowledgementExecuted simulates deleting a packet commitment with the -// given packet sequence. -func (chain *TestChain) AcknowledgementExecuted( - packet exported.PacketI, -) error { - channelCap := chain.GetChannelCapability(packet.GetSourcePort(), packet.GetSourceChannel()) - - // no need to send message, acting as a handler - err := chain.App.IBCKeeper.ChannelKeeper.AcknowledgementExecuted(chain.GetContext(), channelCap, packet) + err := chain.App.IBCKeeper.ChannelKeeper.WriteAcknowledgement(chain.GetContext(), channelCap, packet, TestHash) if err != nil { return err } diff --git a/x/ibc/testing/coordinator.go b/x/ibc/testing/coordinator.go index 6dbacee50f..7ead436abe 100644 --- a/x/ibc/testing/coordinator.go +++ b/x/ibc/testing/coordinator.go @@ -247,25 +247,6 @@ func (coord *Coordinator) RecvPacket( return coord.SendMsgs(counterparty, source, sourceClient, []sdk.Msg{recvMsg}) } -// WriteReceipt receives a packet through the channel keeper on the source chain, writes a receipt, and updates the -// counterparty client for the source chain. -func (coord *Coordinator) WriteReceipt( - source, counterparty *TestChain, - packet exported.PacketI, - counterpartyClientID string, -) error { - if err := source.WriteReceipt(packet); err != nil { - return err - } - coord.IncrementTime() - - // update source client on counterparty connection - return coord.UpdateClient( - counterparty, source, - counterpartyClientID, Tendermint, - ) -} - // WriteAcknowledgement writes an acknowledgement to the channel keeper on the source chain and updates the // counterparty client for the source chain. func (coord *Coordinator) WriteAcknowledgement( @@ -303,25 +284,6 @@ func (coord *Coordinator) AcknowledgePacket( return coord.SendMsgs(source, counterparty, counterpartyClient, []sdk.Msg{ackMsg}) } -// AcknowledgementExecuted deletes the packet commitment with the given -// packet sequence since the acknowledgement has been verified. -func (coord *Coordinator) AcknowledgementExecuted( - source, counterparty *TestChain, - packet exported.PacketI, - counterpartyClientID string, -) error { - if err := source.AcknowledgementExecuted(packet); err != nil { - return err - } - coord.IncrementTime() - - // update source client on counterparty connection - return coord.UpdateClient( - counterparty, source, - counterpartyClientID, Tendermint, - ) -} - // RelayPacket receives a channel packet on counterparty, queries the ack // and acknowledges the packet on source. The clients are updated as needed. func (coord *Coordinator) RelayPacket(