From 88f6a3ff6203eb4803d0b53a5cf476574a7f2275 Mon Sep 17 00:00:00 2001 From: chandiniv1 <117723967+chandiniv1@users.noreply.github.com> Date: Thu, 22 Aug 2024 14:54:20 +0530 Subject: [PATCH 1/2] add submodule to logger (#7196) * add submodule for logger * add submodule in keys.go --- modules/core/packet-server/keeper/keeper.go | 3 +-- modules/core/packet-server/types/keys.go | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 modules/core/packet-server/types/keys.go diff --git a/modules/core/packet-server/keeper/keeper.go b/modules/core/packet-server/keeper/keeper.go index 926905228b9..b816df12fd8 100644 --- a/modules/core/packet-server/keeper/keeper.go +++ b/modules/core/packet-server/keeper/keeper.go @@ -38,8 +38,7 @@ func NewKeeper(cdc codec.BinaryCodec, channelKeeper types.ChannelKeeper, clientK // Logger returns a module-specific logger. func (Keeper) Logger(ctx sdk.Context) log.Logger { - // TODO: prefix some submodule identifier? - return ctx.Logger().With("module", "x/"+exported.ModuleName) + return ctx.Logger().With("module", "x/"+exported.ModuleName+"/"+types.SubModuleName) } // SendPacket implements the packet sending logic required by a packet handler. diff --git a/modules/core/packet-server/types/keys.go b/modules/core/packet-server/types/keys.go new file mode 100644 index 00000000000..1dbfabd45e8 --- /dev/null +++ b/modules/core/packet-server/types/keys.go @@ -0,0 +1,3 @@ +package types + +const SubModuleName = "packetserver" From ad5748eaf27a02d8d9f4db7aaca2aa58f8670808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 22 Aug 2024 14:23:16 +0200 Subject: [PATCH 2/2] tests: msg server tests for eureka (#7162) * test: TestAcknowledgePacketV2 * fix up recv tests * add timeout v2 tests * imp: use expErr instead of expPass --------- Co-authored-by: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com> Co-authored-by: Carlos Rodriguez --- modules/core/keeper/msg_server_test.go | 437 +++++++++++++++++++++++++ 1 file changed, 437 insertions(+) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 73bcb53dd65..43e59b1c130 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -223,6 +223,179 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { } } +// tests the IBC handler receiving a packet using V2 protocol +func (suite *KeeperTestSuite) TestRecvPacketV2() { + var ( + packet channeltypes.Packet + path *ibctesting.Path + ) + + testCases := []struct { + name string + malleate func() + expErr error + expRevert bool + async bool // indicate no ack written + replay bool // indicate replay (no-op) + }{ + { + "success", + func() { + path.SetupV2() + + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, "", ibctesting.MockPacketData) + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + }, + nil, + false, + false, + false, + }, + { + "success: OnRecvPacket callback returns error acknowledgement", + func() { + path.SetupV2() + + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, "", ibctesting.MockFailPacketData) + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibctesting.MockFailPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + }, + nil, + true, + false, + false, + }, + { + "success: async acknowledgement", + func() { + path.SetupV2() + + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, "", ibcmock.MockAsyncPacketData) + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibcmock.MockAsyncPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + }, + nil, + false, + true, + false, + }, + { + "success no-op: packet already received (replay)", + func() { + // mock will panic if application callback is called twice on the same packet + path.SetupV2() + + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, "", ibctesting.MockPacketData) + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + err = path.EndpointB.RecvPacket(packet) + suite.Require().NoError(err) + }, + nil, + false, + false, + true, + }, + { + "channel does not exist", + func() { + // any non-nil value of packet is valid + suite.Require().NotNil(packet) + }, + channeltypes.ErrChannelNotFound, + false, + false, + false, + }, + { + "packet not sent", + func() { + path.SetupV2() + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + }, + fmt.Errorf("receive packet verification failed"), + false, + false, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + path = ibctesting.NewPath(suite.chainA, suite.chainB) + + tc.malleate() + + var ( + proof []byte + proofHeight clienttypes.Height + ) + // get proof of packet commitment from chainA + packetKey := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + if path.EndpointA.ClientID != "" { + proof, proofHeight = path.EndpointA.QueryProof(packetKey) + } + + ctx := suite.chainB.GetContext() + msg := channeltypes.NewMsgRecvPacket(packet, proof, proofHeight, suite.chainB.SenderAccount.GetAddress().String()) + res, err := suite.chainB.App.GetIBCKeeper().RecvPacket(ctx, msg) + + events := ctx.EventManager().Events() + + if tc.expErr == nil { + suite.Require().NoError(err) + + // replay should not fail since it will be treated as a no-op + _, err := suite.chainB.App.GetIBCKeeper().RecvPacket(suite.chainB.GetContext(), msg) + suite.Require().NoError(err) + + // check that callback state was handled correctly + _, exists := suite.chainB.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainB.GetContext(), ibcmock.GetMockRecvCanaryCapabilityName(packet)) + if tc.expRevert { + suite.Require().False(exists, "capability exists in store even after callback reverted") + + // context events should contain error events + suite.Require().Contains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) + suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent()) + } else { + suite.Require().True(exists, "callback state not persisted when revert is false") + + if tc.replay { + // context should not contain application events + suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent()) + suite.Require().NotContains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) + } else { + // context events should contain application events + suite.Require().Contains(events, ibcmock.NewMockRecvPacketEvent()) + } + } + + // verify if ack was written + ack, found := suite.chainB.App.GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + + if tc.async { + suite.Require().Nil(ack) + suite.Require().False(found) + } else { + suite.Require().NotNil(ack) + suite.Require().True(found) + } + } else { + suite.Require().ErrorContains(err, tc.expErr.Error()) + suite.Require().Nil(res) + } + }) + } +} + func (suite *KeeperTestSuite) TestRecoverClient() { var msg *clienttypes.MsgRecoverClient @@ -456,6 +629,133 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { } } +// tests the IBC handler acknowledgement of a packet using protocol version V2 +func (suite *KeeperTestSuite) TestAcknowledgePacketV2() { + var ( + packet channeltypes.Packet + signer string + path *ibctesting.Path + ) + + testCases := []struct { + name string + malleate func() + expError error + replay bool // indicate replay (no-op) + }{ + { + "success", + func() {}, + nil, + false, + }, + { + "invalid signer", + func() { + signer = "invalid-signer" + }, + errors.New("Invalid address for msg Signer"), + false, + }, + { + "port route does not exist", + func() { + packet.SourcePort = "invalid-port" + }, + porttypes.ErrInvalidRoute, + false, + }, + { + "acknowledge packet fails in packet handler", + func() { + packet.SourceChannel = "invalid-client" + }, + channeltypes.ErrChannelNotFound, + false, + }, + { + "successful no-op: - packet already acknowledged (replay)", + func() { + err := path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + suite.Require().NoError(err) + }, + nil, + true, + }, + { + "application callback returns error", + func() { + suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnAcknowledgementPacket = func(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { + return fmt.Errorf("mock app callback failed") + } + }, + fmt.Errorf("mock app callback failed"), + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + signer = suite.chainA.SenderAccount.GetAddress().String() + + path = ibctesting.NewPath(suite.chainA, suite.chainB) + path.SetupV2() + + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, "", ibctesting.MockPacketData) + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + err = path.EndpointB.RecvPacket(packet) + suite.Require().NoError(err) + + tc.malleate() + + var ( + proof []byte + proofHeight clienttypes.Height + ) + packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + proof, proofHeight = path.EndpointB.QueryProof(packetKey) + + msg := channeltypes.NewMsgAcknowledgement(packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight, signer) + + ctx := suite.chainA.GetContext() + res, err := suite.chainA.App.GetIBCKeeper().Acknowledgement(ctx, msg) + + events := ctx.EventManager().Events() + + if tc.expError == nil { + suite.Require().NoError(err) + + // verify packet commitment was deleted on source chain + has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + suite.Require().False(has) + + if tc.replay { + // context should not contain application events + suite.Require().NotContains(events, ibcmock.NewMockAckPacketEvent()) + suite.Require().Equal(channeltypes.NOOP, res.Result) + } else { + // context events should contain application events + suite.Require().Contains(events, ibcmock.NewMockAckPacketEvent()) + suite.Require().Equal(channeltypes.SUCCESS, res.Result) + + // replay should not error as it is treated as a no-op + res, err = suite.chainA.App.GetIBCKeeper().Acknowledgement(suite.chainA.GetContext(), msg) + suite.Require().NoError(err) + suite.Require().Equal(channeltypes.NOOP, res.Result) + } + } else { + suite.Require().ErrorContains(err, tc.expError.Error()) + suite.Require().Nil(res) + } + }) + } +} + // tests the IBC handler timing out a packet on ordered and unordered channels. // It verifies that the deletion of a packet commitment occurs. It tests // high level properties like ordering and basic sanity checks. More @@ -614,6 +914,143 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { } } +// tests the IBC handler timing out a packet for the V2 protocol. +// It verifies that the deletion of a packet commitment occurs. More +// rigorous testing of 'TimeoutPacket' and 'TimeoutExecuted' can be found in +// the packet-server/keeper/keeper_test.go. +func (suite *KeeperTestSuite) TestTimeoutPacketV2() { + var ( + packet channeltypes.Packet + packetKey []byte + path *ibctesting.Path + ) + + testCases := []struct { + name string + malleate func() + expErr error + noop bool // indicate no-op + }{ + { + "success", + func() { + path.SetupV2() + + timeoutHeight := clienttypes.GetSelfHeight(suite.chainB.GetContext()) + timeoutTimestamp := uint64(suite.chainB.GetContext().BlockTime().UnixNano()) + + // create packet commitment + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, timeoutTimestamp, "", ibctesting.MockPacketData) + suite.Require().NoError(err) + + // need to update chainA client to prove missing ack + err = path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, timeoutTimestamp, "") + packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + }, + nil, + false, + }, + { + "success: timeout out of order packet", + func() { + // setup uses an UNORDERED channel + path.SetupV2() + + // attempts to timeout the last packet sent without timing out the first packet + // packet sequences begin at 1 + for i := uint64(1); i < maxSequence; i++ { + timeoutHeight := clienttypes.GetSelfHeight(suite.chainB.GetContext()) + + // create packet commitment + sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, "", ibctesting.MockPacketData) + suite.Require().NoError(err) + + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, "") + } + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + }, + nil, + false, + }, + { + "success no-op: packet not sent", func() { + path.SetupV2() + packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, clienttypes.NewHeight(0, 1), 0, "") + packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + }, + nil, + true, + }, + { + "channel does not exist", + func() { + // any non-nil value of packet is valid + suite.Require().NotNil(packet) + + packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + }, + channeltypes.ErrChannelNotFound, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + path = ibctesting.NewPath(suite.chainA, suite.chainB) + + tc.malleate() + + var ( + proof []byte + proofHeight clienttypes.Height + ) + if path.EndpointB.ClientID != "" { + proof, proofHeight = path.EndpointB.QueryProof(packetKey) + } + + ctx := suite.chainA.GetContext() + msg := channeltypes.NewMsgTimeout(packet, 1, proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String()) + res, err := suite.chainA.App.GetIBCKeeper().Timeout(ctx, msg) + + events := ctx.EventManager().Events() + + if tc.expErr == nil { + suite.Require().NoError(err) + + // replay should not return an error as it is treated as a no-op + _, err := suite.chainA.App.GetIBCKeeper().Timeout(suite.chainA.GetContext(), msg) + suite.Require().NoError(err) + + // verify packet commitment was deleted on source chain + has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + suite.Require().False(has) + + if tc.noop { + // context should not contain application events + suite.Require().NotContains(events, ibcmock.NewMockTimeoutPacketEvent()) + } else { + // context should contain application events + suite.Require().Contains(events, ibcmock.NewMockTimeoutPacketEvent()) + } + + } else { + suite.Require().ErrorIs(err, tc.expErr) + suite.Require().Nil(res) + } + }) + } +} + // tests the IBC handler timing out a packet via channel closure on ordered // and unordered channels. It verifies that the deletion of a packet // commitment occurs. It tests high level properties like ordering and basic