From 2c9ad1678664c00a2ebd5ce9f15e1c8327e2b306 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Thu, 8 Feb 2024 11:09:36 +0000 Subject: [PATCH] Add testing for ICA events (#5687) --- e2e/tests/transfer/incentivized_test.go | 52 ++++++++----------- e2e/testsuite/sanitize/messages.go | 8 ++- e2e/testsuite/testsuite.go | 1 + .../controller/ibc_middleware_test.go | 16 +++++- .../27-interchain-accounts/host/ibc_module.go | 1 + .../host/ibc_module_test.go | 38 +++++++++++++- .../host/keeper/events.go | 14 +++++ 7 files changed, 95 insertions(+), 35 deletions(-) diff --git a/e2e/tests/transfer/incentivized_test.go b/e2e/tests/transfer/incentivized_test.go index 0fa493f24be..1902c9e0c66 100644 --- a/e2e/tests/transfer/incentivized_test.go +++ b/e2e/tests/transfer/incentivized_test.go @@ -22,6 +22,11 @@ import ( channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ) +const ( + legacyMessage = "balance should be lowered by sum of recv_fee, ack_fee and timeout_fee" + capitalEfficientMessage = "balance should be lowered by max(recv_fee + ack_fee, timeout_fee)" +) + type IncentivizedTransferTestSuite struct { TransferTestSuite } @@ -118,12 +123,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_AsyncSingleSender_Su s.Require().True(actualFee.TimeoutFee.Equal(testFee.TimeoutFee)) }) - msg := "balance should be lowered by max(recv_fee + ack_fee, timeout_fee)" - escrowTotalFee := testFee.Total() - if !testvalues.CapitalEfficientFeeEscrowFeatureReleases.IsSupported(chainAVersion) { - msg = "balance should be lowered by sum of recv_fee, ack_fee and timeout_fee" - escrowTotalFee = testFee.RecvFee.Add(testFee.AckFee...).Add(testFee.TimeoutFee...) - } + msg, escrowTotalFee := getMessageAndFee(testFee, chainAVersion) t.Run(msg, func(t *testing.T) { actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) s.Require().NoError(err) @@ -237,12 +237,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_InvalidReceiverAccou s.Require().True(actualFee.TimeoutFee.Equal(testFee.TimeoutFee)) }) - msg := "balance should be lowered by max(recv_fee + ack_fee, timeout_fee)" - escrowTotalFee := testFee.Total() - if !testvalues.CapitalEfficientFeeEscrowFeatureReleases.IsSupported(chainAVersion) { - msg = "balance should be lowered by sum of recv_fee, ack_fee and timeout_fee" - escrowTotalFee = testFee.RecvFee.Add(testFee.AckFee...).Add(testFee.TimeoutFee...) - } + msg, escrowTotalFee := getMessageAndFee(testFee, chainAVersion) t.Run(msg, func(t *testing.T) { actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) s.Require().NoError(err) @@ -342,12 +337,7 @@ func (s *IncentivizedTransferTestSuite) TestMultiMsg_MsgPayPacketFeeSingleSender s.Require().True(actualFee.TimeoutFee.Equal(testFee.TimeoutFee)) }) - msg := "balance should be lowered by max(recv_fee + ack_fee, timeout_fee)" - escrowTotalFee := testFee.Total() - if !testvalues.CapitalEfficientFeeEscrowFeatureReleases.IsSupported(chainAVersion) { - msg = "balance should be lowered by sum of recv_fee, ack_fee and timeout_fee" - escrowTotalFee = testFee.RecvFee.Add(testFee.AckFee...).Add(testFee.TimeoutFee...) - } + msg, escrowTotalFee := getMessageAndFee(testFee, chainAVersion) t.Run(msg, func(t *testing.T) { actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) s.Require().NoError(err) @@ -470,12 +460,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_SingleSender_TimesOu s.Require().True(actualFee.TimeoutFee.Equal(testFee.TimeoutFee)) }) - msg := "balance should be lowered by max(recv_fee + ack_fee, timeout_fee)" - escrowTotalFee := testFee.Total() - if !testvalues.CapitalEfficientFeeEscrowFeatureReleases.IsSupported(chainAVersion) { - msg = "balance should be lowered by sum of recv_fee, ack_fee and timeout_fee" - escrowTotalFee = testFee.RecvFee.Add(testFee.AckFee...).Add(testFee.TimeoutFee...) - } + msg, escrowTotalFee := getMessageAndFee(testFee, chainAVersion) t.Run(msg, func(t *testing.T) { actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) s.Require().NoError(err) @@ -574,12 +559,7 @@ func (s *IncentivizedTransferTestSuite) TestPayPacketFeeAsync_SingleSender_NoCou }) }) - msg := "balance should be lowered by max(recv_fee + ack_fee, timeout_fee)" - escrowTotalFee := testFee.Total() - if !testvalues.CapitalEfficientFeeEscrowFeatureReleases.IsSupported(chainAVersion) { - msg = "balance should be lowered by sum of recv_fee, ack_fee and timeout_fee" - escrowTotalFee = testFee.RecvFee.Add(testFee.AckFee...).Add(testFee.TimeoutFee...) - } + msg, escrowTotalFee := getMessageAndFee(testFee, chainAVersion) t.Run(msg, func(t *testing.T) { actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) s.Require().NoError(err) @@ -760,3 +740,15 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_AsyncMultipleSenders s.Require().Equal(expected2, actualBalance2) }) } + +// getMessageAndFee returns the message that should be used for t.Run as well as the expected fee based on +// whether or not capital efficient fee escrow is supported. +func getMessageAndFee(fee feetypes.Fee, chainVersion string) (string, sdk.Coins) { + msg := capitalEfficientMessage + escrowTotalFee := fee.Total() + if !testvalues.CapitalEfficientFeeEscrowFeatureReleases.IsSupported(chainVersion) { + msg = legacyMessage + escrowTotalFee = fee.RecvFee.Add(fee.AckFee...).Add(fee.TimeoutFee...) + } + return msg, escrowTotalFee +} diff --git a/e2e/testsuite/sanitize/messages.go b/e2e/testsuite/sanitize/messages.go index 7cf901d6913..55f3128c648 100644 --- a/e2e/testsuite/sanitize/messages.go +++ b/e2e/testsuite/sanitize/messages.go @@ -53,7 +53,9 @@ func removeUnknownFields(tag string, msg sdk.Msg) sdk.Msg { panic(err) } sanitizedMsgs := Messages(tag, msgs...) - msg.SetMsgs(sanitizedMsgs) + if err := msg.SetMsgs(sanitizedMsgs); err != nil { + panic(err) + } return msg case *grouptypes.MsgSubmitProposal: if !groupsv1ProposalTitleAndSummary.IsSupported(tag) { @@ -66,7 +68,9 @@ func removeUnknownFields(tag string, msg sdk.Msg) sdk.Msg { panic(err) } sanitizedMsgs := Messages(tag, msgs...) - msg.SetMsgs(sanitizedMsgs) + if err := msg.SetMsgs(sanitizedMsgs); err != nil { + panic(err) + } return msg case *icacontrollertypes.MsgRegisterInterchainAccount: if !icaUnorderedChannelFeatureReleases.IsSupported(tag) { diff --git a/e2e/testsuite/testsuite.go b/e2e/testsuite/testsuite.go index 3d3e9410438..138632982ba 100644 --- a/e2e/testsuite/testsuite.go +++ b/e2e/testsuite/testsuite.go @@ -18,6 +18,7 @@ import ( sdkmath "cosmossdk.io/math" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + "github.com/cosmos/ibc-go/e2e/relayer" "github.com/cosmos/ibc-go/e2e/testsuite/diagnostics" feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types" diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 8d76424a6ca..ac0a4fa35d3 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -564,8 +564,22 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { 0, ) - ack := cbs.OnRecvPacket(suite.chainA.GetContext(), packet, nil) + ctx := suite.chainA.GetContext() + ack := cbs.OnRecvPacket(ctx, packet, nil) suite.Require().Equal(tc.expPass, ack.Success()) + + expectedEvents := sdk.Events{ + sdk.NewEvent( + icatypes.EventTypePacket, + sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName), + sdk.NewAttribute(icatypes.AttributeKeyControllerChannelID, packet.GetDestChannel()), + sdk.NewAttribute(icatypes.AttributeKeyAckSuccess, fmt.Sprintf("%t", false)), + sdk.NewAttribute(icatypes.AttributeKeyAckError, "cannot receive packet on controller chain: invalid message sent to channel end"), + ), + }.ToABCIEvents() + + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) }) } } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 622bf284bd9..81dbb148679 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -119,6 +119,7 @@ func (im IBCModule) OnRecvPacket( logger := im.keeper.Logger(ctx) if !im.keeper.GetParams(ctx).HostEnabled { logger.Info("host submodule is disabled") + keeper.EmitHostDisabledEvent(ctx, packet) return channeltypes.NewErrorAcknowledgement(types.ErrHostSubModuleDisabled) } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index c0f05fb5c58..e63c7b0bcf1 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -400,14 +400,16 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { name string malleate func() expAckSuccess bool + eventErrorMsg string }{ { - "success", func() {}, true, + "success", func() {}, true, "", }, { "host submodule disabled", func() { suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false, []string{})) }, false, + types.ErrHostSubModuleDisabled.Error(), }, { "success with ICA auth module callback failure", func() { @@ -417,11 +419,13 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { return channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed OnRecvPacket mock callback")) } }, true, + "failed OnRecvPacket mock callback", }, { "ICA OnRecvPacket fails - cannot unmarshal packet data", func() { packetData = []byte("invalid data") }, false, + "cannot unmarshal ICS-27 interchain account packet data: unknown data type", }, } @@ -487,12 +491,42 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module) suite.Require().True(ok) - ack := cbs.OnRecvPacket(suite.chainB.GetContext(), packet, nil) + ctx := suite.chainB.GetContext() + ack := cbs.OnRecvPacket(ctx, packet, nil) + + expectedAttributes := []sdk.Attribute{ + sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName), + sdk.NewAttribute(icatypes.AttributeKeyHostChannelID, packet.GetDestChannel()), + sdk.NewAttribute(icatypes.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())), + } + if tc.expAckSuccess { suite.Require().True(ack.Success()) suite.Require().Equal(expectedAck, ack) + + expectedEvents := sdk.Events{ + sdk.NewEvent( + icatypes.EventTypePacket, + expectedAttributes..., + ), + }.ToABCIEvents() + + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) + } else { suite.Require().False(ack.Success()) + + expectedAttributes = append(expectedAttributes, sdk.NewAttribute(icatypes.AttributeKeyAckError, tc.eventErrorMsg)) + expectedEvents := sdk.Events{ + sdk.NewEvent( + icatypes.EventTypePacket, + expectedAttributes..., + ), + }.ToABCIEvents() + + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) } }) } diff --git a/modules/apps/27-interchain-accounts/host/keeper/events.go b/modules/apps/27-interchain-accounts/host/keeper/events.go index 3c5b4e0971f..ffb81cb6a93 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/events.go +++ b/modules/apps/27-interchain-accounts/host/keeper/events.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" "github.com/cosmos/ibc-go/v8/modules/core/exported" @@ -30,3 +31,16 @@ func EmitAcknowledgementEvent(ctx sdk.Context, packet channeltypes.Packet, ack e ), ) } + +// EmitHostDisabledEvent emits an event signalling that the host submodule is disabled. +func EmitHostDisabledEvent(ctx sdk.Context, packet channeltypes.Packet) { + ctx.EventManager().EmitEvent( + sdk.NewEvent( + icatypes.EventTypePacket, + sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName), + sdk.NewAttribute(icatypes.AttributeKeyHostChannelID, packet.GetDestChannel()), + sdk.NewAttribute(icatypes.AttributeKeyAckError, types.ErrHostSubModuleDisabled.Error()), + sdk.NewAttribute(icatypes.AttributeKeyAckSuccess, "false"), + ), + ) +}