From 5db55dd116d0248b33412fd9985319a45f3849c5 Mon Sep 17 00:00:00 2001 From: Nikolas De Giorgis Date: Tue, 27 Aug 2024 09:55:22 +0100 Subject: [PATCH] (chore): use expected errors in 29fee (#7191) * chore: replace expected boolean errors with expected errors * use errors rather than strings * last tests * linter * introduce testing aux function * Update modules/apps/29-fee/ibc_middleware_test.go Co-authored-by: Carlos Rodriguez * Missing file * mark function as helper --------- Co-authored-by: Carlos Rodriguez --- modules/apps/29-fee/ibc_middleware_test.go | 190 ++++++------- modules/apps/29-fee/keeper/grpc_query_test.go | 246 +++++++++-------- modules/apps/29-fee/keeper/msg_server_test.go | 255 +++++++++--------- modules/apps/29-fee/keeper/relay_test.go | 10 +- modules/apps/29-fee/types/codec_test.go | 18 +- modules/apps/29-fee/types/fee_test.go | 45 ++-- modules/apps/29-fee/types/genesis_test.go | 110 ++++---- modules/apps/29-fee/types/keys_test.go | 98 +++---- modules/apps/29-fee/types/msgs_test.go | 153 ++++++----- testing/utils.go | 12 + 10 files changed, 601 insertions(+), 536 deletions(-) diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index 4e2a9c27280..d348c4de7e2 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -2,6 +2,7 @@ package fee_test import ( "encoding/json" + "errors" "fmt" errorsmod "cosmossdk.io/errors" @@ -17,6 +18,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" "github.com/cosmos/ibc-go/v9/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v9/testing" ibcmock "github.com/cosmos/ibc-go/v9/testing/mock" @@ -33,44 +35,44 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { testCases := []struct { name string version string - expPass bool isFeeEnabled bool + expErr error }{ { "success - valid fee middleware and mock version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version})), true, - true, + nil, }, { "success - fee version not included, only perform mock logic", ibcmock.Version, - true, false, + nil, + }, + { + "passing an empty string returns default version", + "", + true, + nil, }, { "invalid fee middleware version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: ibcmock.Version})), false, - false, + types.ErrInvalidVersion, }, { "invalid mock version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-mock-version"})), false, - false, + errors.New("incorrect mock version"), }, { "mock version not wrapped", types.Version, false, - false, - }, - { - "passing an empty string returns default version", - "", - true, - true, + errors.New("incorrect mock version"), }, } @@ -117,7 +119,7 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, channel.Version) - if tc.expPass { + if tc.expErr == nil { // check if the channel is fee enabled. If so version string should include metaData if tc.isFeeEnabled { versionMetadata := types.Metadata{ @@ -135,7 +137,7 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { suite.Require().NoError(err, "unexpected error from version: %s", tc.version) } else { - suite.Require().Error(err, "error not returned for version: %s", tc.version) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, "error not returned for version: %s or error %s is not %s", tc.version, err, tc.expErr) suite.Require().Equal("", version) } }) @@ -148,27 +150,27 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { testCases := []struct { name string cpVersion string - expPass bool + expErr error }{ { "success - valid fee middleware version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version})), - true, + nil, }, { "success - valid mock version", ibcmock.Version, - true, + nil, }, { "invalid fee middleware version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: ibcmock.Version})), - false, + types.ErrInvalidVersion, }, { "invalid mock version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-mock-version"})), - false, + errors.New("incorrect mock version"), }, } @@ -222,10 +224,10 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { _, err = cbs.OnChanOpenTry(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, tc.cpVersion) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) } }) } @@ -238,31 +240,31 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { name string cpVersion string malleate func(suite *FeeTestSuite) - expPass bool + expErr error }{ { "success", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version})), func(suite *FeeTestSuite) {}, - true, + nil, }, { "invalid fee version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: ibcmock.Version})), func(suite *FeeTestSuite) {}, - false, + types.ErrInvalidVersion, }, { "invalid mock version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-mock-version"})), func(suite *FeeTestSuite) {}, - false, + errors.New("incorrect mock version"), }, { "invalid version fails to unmarshal metadata", ibctesting.InvalidID, func(suite *FeeTestSuite) {}, - false, + errors.New("incorrect mock version"), }, { "previous INIT set without fee, however counterparty set fee version", // note this can only happen with incompetent or malicious counterparty chain @@ -272,7 +274,7 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { suite.path.EndpointA.ChannelConfig.Version = ibcmock.Version suite.path.EndpointB.ChannelConfig.Version = ibcmock.Version }, - false, + errors.New("incorrect mock version"), }, } @@ -307,10 +309,10 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { suite.Require().True(ok) err = cbs.OnChanOpenAck(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, suite.path.EndpointA.Counterparty.ChannelID, tc.cpVersion) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) } else { - suite.Require().Error(err, "%s expected error but returned none", tc.name) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) } }) } @@ -325,19 +327,16 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { - "success", func() {}, true, + "success", func() {}, nil, }, { - "application callback fails", func() { - suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanCloseInit = func( - ctx sdk.Context, portID, channelID string, - ) error { - return fmt.Errorf("application callback fails") - } - }, false, + "fee module is not enabled", func() { + suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + }, + nil, }, { "RefundFeesOnChannelClosure continues - invalid refund address", func() { @@ -349,19 +348,22 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) suite.Require().NoError(err) }, - true, + nil, }, { - "fee module locked", func() { - lockFeeModule(suite.chainA) - }, - false, + "application callback fails", func() { + suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanCloseInit = func( + ctx sdk.Context, portID, channelID string, + ) error { + return fmt.Errorf("application callback fails") + } + }, errors.New("application callback fails"), }, { - "fee module is not enabled", func() { - suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + "fee module locked", func() { + lockFeeModule(suite.chainA) }, - true, + types.ErrFeeModuleLocked, }, } @@ -395,10 +397,10 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { err = cbs.OnChanCloseInit(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) } }) } @@ -414,19 +416,16 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { - "success", func() {}, true, + "success", func() {}, nil, }, { - "application callback fails", func() { - suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanCloseConfirm = func( - ctx sdk.Context, portID, channelID string, - ) error { - return fmt.Errorf("application callback fails") - } - }, false, + "fee module is not enabled", func() { + suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + }, + nil, }, { "RefundChannelFeesOnClosure continues - refund address is invalid", func() { @@ -438,19 +437,22 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, fee.Total()) suite.Require().NoError(err) }, - true, + nil, }, { - "fee module locked", func() { - lockFeeModule(suite.chainA) - }, - false, + "application callback fails", func() { + suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanCloseConfirm = func( + ctx sdk.Context, portID, channelID string, + ) error { + return fmt.Errorf("application callback fails") + } + }, errors.New("application callback fails"), }, { - "fee module is not enabled", func() { - suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + "fee module locked", func() { + lockFeeModule(suite.chainA) }, - true, + types.ErrFeeModuleLocked, }, } @@ -485,10 +487,10 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { err = cbs.OnChanCloseConfirm(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) } }) } @@ -619,7 +621,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { testCases := []struct { name string malleate func() - expPass bool + expErr error expResult func() }{ { @@ -629,7 +631,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)) expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...) }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -654,7 +656,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)) expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...) }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -688,7 +690,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { payeeAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), payeeAddr, sdk.DefaultBondDenom)) expPayeeAccBalance = payeeAccBalance.Add(packetFee.Fee.RecvFee...).Add(packetFee.Fee.AckFee...) }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -713,7 +715,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { ForwardRelayerAddress: "", }.Acknowledgement() }, - true, + nil, func() { found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) suite.Require().False(found) @@ -725,7 +727,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) ack = ibcmock.MockAcknowledgement.Acknowledgement() }, - true, + nil, func() {}, }, { @@ -733,7 +735,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { func() { lockFeeModule(suite.chainA) }, - true, + nil, func() { suite.Require().Equal(true, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) }, @@ -750,7 +752,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { relayerAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, sdk.DefaultBondDenom)) expPayeeAccBalance = relayerAccBalance.Add(packetFee.Fee.AckFee...) }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -770,7 +772,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { func() { escrowAmount = sdk.NewCoins() }, - true, + nil, func() { suite.Require().Equal(true, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) }, @@ -780,7 +782,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { func() { ack = []byte("unsupported acknowledgement format") }, - false, + ibcerrors.ErrInvalidType, func() {}, }, { @@ -794,7 +796,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { suite.path.EndpointA.ChannelID, ) }, - false, + errors.New("failed to create sdk.Address from payee"), func() {}, }, { @@ -804,7 +806,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { return fmt.Errorf("mock fee app callback fails") } }, - false, + errors.New("mock fee app callback fails"), func() {}, }, } @@ -843,10 +845,10 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), suite.path.EndpointA.GetChannel().Version, packet, ack, relayerAddr) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) } tc.expResult() @@ -869,7 +871,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { testCases := []struct { name string malleate func() - expPass bool + expErr error expResult func() }{ { @@ -879,7 +881,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)) expRefundAccBalance = refundAccBalance }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -906,7 +908,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)) expRefundAccBalance = refundAccBalance.Add(refundCoins...) }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -939,7 +941,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { refundAccBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAddr, sdk.DefaultBondDenom)) expRefundAccBalance = refundAccBalance }, - true, + nil, func() { // assert that the packet fees have been distributed found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -958,7 +960,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { func() { suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) }, - true, + nil, func() {}, }, { @@ -966,7 +968,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { func() { lockFeeModule(suite.chainA) }, - true, + nil, func() { suite.Require().Equal(true, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) }, @@ -976,7 +978,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { func() { suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeesInEscrow(suite.chainA.GetContext(), packetID) }, - true, + nil, func() {}, }, { @@ -984,7 +986,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { func() { relayerAddr = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() }, - true, + nil, func() {}, }, { @@ -992,7 +994,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { func() { escrowAmount = sdk.NewCoins() }, - true, + nil, func() { suite.Require().Equal(true, suite.chainA.GetSimApp().IBCFeeKeeper.IsLocked(suite.chainA.GetContext())) }, @@ -1008,7 +1010,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { suite.path.EndpointA.ChannelID, ) }, - false, + errors.New("failed to create sdk.Address from payee"), func() {}, }, { @@ -1018,7 +1020,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { return fmt.Errorf("mock fee app callback fails") } }, - false, + errors.New("mock fee app callback fails"), func() {}, }, } @@ -1054,10 +1056,10 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), suite.path.EndpointA.GetChannel().Version, packet, relayerAddr) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) } tc.expResult() diff --git a/modules/apps/29-fee/keeper/grpc_query_test.go b/modules/apps/29-fee/keeper/grpc_query_test.go index bf93096744e..3732be86f01 100644 --- a/modules/apps/29-fee/keeper/grpc_query_test.go +++ b/modules/apps/29-fee/keeper/grpc_query_test.go @@ -24,7 +24,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", @@ -50,22 +50,22 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() { QueryHeight: 0, } }, - true, + "", }, { - "empty request", + "empty pagination", func() { - req = nil + expectedPackets = nil + req = &types.QueryIncentivizedPacketsRequest{} }, - false, + "", }, { - "empty pagination", + "empty request", func() { - expectedPackets = nil - req = &types.QueryIncentivizedPacketsRequest{} + req = nil }, - true, + "InvalidArgument", }, } @@ -81,12 +81,12 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() { res, err := suite.chainA.GetSimApp().IBCFeeKeeper.IncentivizedPackets(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(expectedPackets, res.IncentivizedPackets) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -98,19 +98,19 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacket() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, + "", }, { "empty request", func() { req = nil }, - false, + "InvalidArgument", }, { "fees not found for packet id", @@ -120,7 +120,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacket() { QueryHeight: 0, } }, - false, + "NotFound", }, } @@ -149,12 +149,12 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacket() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.IncentivizedPacket(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(types.NewIdentifiedPacketFees(packetID, []types.PacketFee{packetFee, packetFee, packetFee}), res.IncentivizedPacket) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -177,7 +177,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { testCases := []struct { msg string malleate func() - expPass bool + errMsg string }{ { "empty pagination", @@ -192,7 +192,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { QueryHeight: 0, } }, - true, + "", }, { "success", @@ -215,14 +215,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), identifiedPacketFees.PacketId, types.NewPacketFees(identifiedPacketFees.PacketFees)) } }, - true, - }, - { - "empty request", - func() { - req = nil - }, - false, + "", }, { "no packets for specified channel", @@ -240,7 +233,14 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { QueryHeight: 0, } }, - true, + "", + }, + { + "empty request", + func() { + req = nil + }, + "InvalidArgument", }, { "channel not found", @@ -250,7 +250,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { ChannelId: ibctesting.InvalidID, } }, - false, + "NotFound", }, { "invalid ID", @@ -260,7 +260,7 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { ChannelId: "test-channel-id", } }, - false, + "InvalidArgument", }, } @@ -294,12 +294,12 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketsForChannel() { res, err := suite.chainA.GetSimApp().IBCFeeKeeper.IncentivizedPacketsForChannel(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(expIdentifiedPacketFees, res.IncentivizedPackets) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -311,26 +311,26 @@ func (suite *KeeperTestSuite) TestQueryTotalRecvFees() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, + "", }, { "empty request", func() { req = nil }, - false, + "InvalidArgument", }, { "packet not found", func() { req.PacketId = channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 100) }, - false, + "NotFound", }, } @@ -359,7 +359,7 @@ func (suite *KeeperTestSuite) TestQueryTotalRecvFees() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.TotalRecvFees(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().NotNil(res) @@ -367,7 +367,7 @@ func (suite *KeeperTestSuite) TestQueryTotalRecvFees() { expectedFees := defaultRecvFee.Add(defaultRecvFee...).Add(defaultRecvFee...) suite.Require().Equal(expectedFees, res.RecvFees) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -379,26 +379,26 @@ func (suite *KeeperTestSuite) TestQueryTotalAckFees() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, + "", }, { "empty request", func() { req = nil }, - false, + "InvalidArgument", }, { "packet not found", func() { req.PacketId = channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 100) }, - false, + "NotFound", }, } @@ -427,7 +427,7 @@ func (suite *KeeperTestSuite) TestQueryTotalAckFees() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.TotalAckFees(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().NotNil(res) @@ -435,7 +435,7 @@ func (suite *KeeperTestSuite) TestQueryTotalAckFees() { expectedFees := defaultAckFee.Add(defaultAckFee...).Add(defaultAckFee...) suite.Require().Equal(expectedFees, res.AckFees) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -447,26 +447,26 @@ func (suite *KeeperTestSuite) TestQueryTotalTimeoutFees() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, + "", }, { "empty request", func() { req = nil }, - false, + "InvalidArgument", }, { "packet not found", func() { req.PacketId = channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 100) }, - false, + "NotFound", }, } @@ -495,7 +495,7 @@ func (suite *KeeperTestSuite) TestQueryTotalTimeoutFees() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.TotalTimeoutFees(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().NotNil(res) @@ -503,7 +503,7 @@ func (suite *KeeperTestSuite) TestQueryTotalTimeoutFees() { expectedFees := defaultTimeoutFee.Add(defaultTimeoutFee...).Add(defaultTimeoutFee...) suite.Require().Equal(expectedFees, res.TimeoutFees) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -515,33 +515,33 @@ func (suite *KeeperTestSuite) TestQueryPayee() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, + "", }, { "empty request", func() { req = nil }, - false, + "InvalidArgument", }, { "payee address not found: invalid channel", func() { req.ChannelId = "invalid-channel-id" //nolint:goconst }, - false, + "NotFound", }, { "payee address not found: invalid relayer address", func() { req.Relayer = "invalid-addr" //nolint:goconst }, - false, + "NotFound", }, } @@ -571,11 +571,11 @@ func (suite *KeeperTestSuite) TestQueryPayee() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.Payee(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().Equal(expPayeeAddr.String(), res.PayeeAddress) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -587,33 +587,33 @@ func (suite *KeeperTestSuite) TestQueryCounterpartyPayee() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, + "", }, { "empty request", func() { req = nil }, - false, + "InvalidArgument", }, { "counterparty address not found: invalid channel", func() { req.ChannelId = "invalid-channel-id" }, - false, + "NotFound", }, { "counterparty address not found: invalid address", func() { req.Relayer = "invalid-addr" }, - false, + "NotFound", }, } @@ -643,16 +643,61 @@ func (suite *KeeperTestSuite) TestQueryCounterpartyPayee() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.CounterpartyPayee(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().Equal(expCounterpartyPayeeAddr.String(), res.CounterpartyPayee) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } } +func (suite *KeeperTestSuite) TestQueryFeeEnabledChannelsWithPagination() { + suite.SetupTest() // reset + + suite.path.Setup() + + expChannel := types.FeeEnabledChannel{ + PortId: suite.path.EndpointA.ChannelConfig.PortID, + ChannelId: suite.path.EndpointA.ChannelID, + } + + expFeeEnabledChannels := []types.FeeEnabledChannel{expChannel} + + req := &types.QueryFeeEnabledChannelsRequest{ + Pagination: &query.PageRequest{ + Limit: 5, + CountTotal: false, + }, + QueryHeight: 0, + } + + // Extract the next available sequence number for channel IDs. + nextSeq := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextChannelSequence(suite.chainA.GetContext()) + for i := 0; i < 8; i++ { + channelID := channeltypes.FormatChannelIdentifier(uint64(i + int(nextSeq))) + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), ibctesting.MockFeePort, channelID) + + expChannel := types.FeeEnabledChannel{ + PortId: ibctesting.MockFeePort, + ChannelId: channelID, + } + + if i < 4 { // add only the first 5 channels, as our default pagination limit is 5 + expFeeEnabledChannels = append(expFeeEnabledChannels, expChannel) + } + } + + suite.chainA.NextBlock() + + ctx := suite.chainA.GetContext() + res, err := suite.chainA.GetSimApp().IBCFeeKeeper.FeeEnabledChannels(ctx, req) + + suite.Require().NoError(err) + suite.Require().Equal(expFeeEnabledChannels, res.FeeEnabledChannels) +} + func (suite *KeeperTestSuite) TestQueryFeeEnabledChannels() { var ( req *types.QueryFeeEnabledChannelsRequest @@ -662,26 +707,19 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannels() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, - }, - { - "empty request", - func() { - req = nil - }, - false, + "", }, { "success: empty pagination", func() { req = &types.QueryFeeEnabledChannelsRequest{} }, - true, + "", }, { "success: with multiple fee enabled channels", @@ -695,30 +733,14 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannels() { expFeeEnabledChannels = append(expFeeEnabledChannels, expChannel) }, - true, + "", }, { - "success: pagination with multiple fee enabled channels", + "failure: empty request", func() { - // Extract the next available sequence number for channel IDs. - nextSeq := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextChannelSequence(suite.chainA.GetContext()) - for i := 0; i < 9; i++ { - channelID := channeltypes.FormatChannelIdentifier(uint64(i + int(nextSeq))) - suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), ibctesting.MockFeePort, channelID) - - expChannel := types.FeeEnabledChannel{ - PortId: ibctesting.MockFeePort, - ChannelId: channelID, - } - - if i < 4 { // add only the first 5 channels, as our default pagination limit is 5 - expFeeEnabledChannels = append(expFeeEnabledChannels, expChannel) - } - } - - suite.chainA.NextBlock() + req = nil }, - true, + "InvalidArgument", }, { "empty response", @@ -728,7 +750,7 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannels() { suite.chainA.NextBlock() }, - true, + "", }, } @@ -760,11 +782,11 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannels() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.FeeEnabledChannels(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().Equal(expFeeEnabledChannels, res.FeeEnabledChannels) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } @@ -780,20 +802,12 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannel() { testCases := []struct { name string malleate func() - expPass bool + errMsg string }{ { "success", func() {}, - true, - }, - { - "empty request", - func() { - req = nil - expEnabled = false - }, - false, + "", }, { "fee not enabled on channel", @@ -807,14 +821,22 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannel() { ChannelId: path.EndpointA.ChannelID, } }, - true, + "", + }, + { + "empty request", + func() { + req = nil + expEnabled = false + }, + "InvalidArgument", }, { "channel not found", func() { req.ChannelId = ibctesting.InvalidID }, - false, + "NotFound", }, { "invalid ID", @@ -824,7 +846,7 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannel() { ChannelId: "test-channel-id", } }, - false, + "InvalidArgument", }, } @@ -848,11 +870,11 @@ func (suite *KeeperTestSuite) TestQueryFeeEnabledChannel() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().IBCFeeKeeper.FeeEnabledChannel(ctx, req) - if tc.expPass { + if tc.errMsg == "" { suite.Require().NoError(err) suite.Require().Equal(expEnabled, res.FeeEnabled) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.errMsg) } }) } diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index ee1658f16ab..c2c629e95cc 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -1,17 +1,20 @@ package keeper_test import ( + "errors" "fmt" sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v9/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v9/testing" ibcmock "github.com/cosmos/ibc-go/v9/testing/mock" ) @@ -21,90 +24,92 @@ func (suite *KeeperTestSuite) TestRegisterPayee() { testCases := []struct { name string - expPass bool malleate func() + expErr error }{ { "success", - true, func() {}, + nil, }, { "channel does not exist", - false, func() { msg.ChannelId = "channel-100" //nolint:goconst }, + channeltypes.ErrChannelNotFound, }, { "channel is not fee enabled", - false, func() { suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) }, + types.ErrFeeNotEnabled, }, { "given payee is not an sdk address", - false, func() { msg.Payee = "invalid-addr" }, + errors.New("decoding bech32 failed: invalid separator index -1"), }, { "payee is a blocked address", - false, func() { msg.Payee = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(transfertypes.ModuleName).String() }, + ibcerrors.ErrUnauthorized, }, } for _, tc := range testCases { tc := tc - suite.SetupTest() - suite.path.Setup() + suite.Run(tc.name, func() { + suite.SetupTest() + suite.path.Setup() - msg = types.NewMsgRegisterPayee( - suite.path.EndpointA.ChannelConfig.PortID, - suite.path.EndpointA.ChannelID, - suite.chainA.SenderAccounts[0].SenderAccount.GetAddress().String(), - suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String(), - ) + msg = types.NewMsgRegisterPayee( + suite.path.EndpointA.ChannelConfig.PortID, + suite.path.EndpointA.ChannelID, + suite.chainA.SenderAccounts[0].SenderAccount.GetAddress().String(), + suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String(), + ) - tc.malleate() + tc.malleate() - ctx := suite.chainA.GetContext() - res, err := suite.chainA.GetSimApp().IBCFeeKeeper.RegisterPayee(ctx, msg) + ctx := suite.chainA.GetContext() + res, err := suite.chainA.GetSimApp().IBCFeeKeeper.RegisterPayee(ctx, msg) - if tc.expPass { - suite.Require().NoError(err) - suite.Require().NotNil(res) + if tc.expErr == nil { + suite.Require().NoError(err) + suite.Require().NotNil(res) - payeeAddr, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetPayeeAddress( - suite.chainA.GetContext(), - suite.chainA.SenderAccount.GetAddress().String(), - suite.path.EndpointA.ChannelID, - ) + payeeAddr, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetPayeeAddress( + suite.chainA.GetContext(), + suite.chainA.SenderAccount.GetAddress().String(), + suite.path.EndpointA.ChannelID, + ) - suite.Require().True(found) - suite.Require().Equal(suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String(), payeeAddr) + suite.Require().True(found) + suite.Require().Equal(suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String(), payeeAddr) - expectedEvents := sdk.Events{ - sdk.NewEvent( - types.EventTypeRegisterPayee, - sdk.NewAttribute(types.AttributeKeyRelayer, suite.chainA.SenderAccount.GetAddress().String()), - sdk.NewAttribute(types.AttributeKeyPayee, payeeAddr), - sdk.NewAttribute(types.AttributeKeyChannelID, suite.path.EndpointA.ChannelID), - ), - }.ToABCIEvents() + expectedEvents := sdk.Events{ + sdk.NewEvent( + types.EventTypeRegisterPayee, + sdk.NewAttribute(types.AttributeKeyRelayer, suite.chainA.SenderAccount.GetAddress().String()), + sdk.NewAttribute(types.AttributeKeyPayee, payeeAddr), + sdk.NewAttribute(types.AttributeKeyChannelID, suite.path.EndpointA.ChannelID), + ), + }.ToABCIEvents() - expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) - ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) - } else { - suite.Require().Error(err) - } + } else { + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) + } + }) } } @@ -116,85 +121,87 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyPayee() { testCases := []struct { name string - expPass bool malleate func() + expErr error }{ { "success", - true, func() {}, + nil, }, { "counterparty payee is an arbitrary string", - true, func() { msg.CounterpartyPayee = "arbitrary-string" expCounterpartyPayee = "arbitrary-string" }, + nil, }, { "channel does not exist", - false, func() { msg.ChannelId = "channel-100" }, + channeltypes.ErrChannelNotFound, }, { "channel is not fee enabled", - false, func() { suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) }, + types.ErrFeeNotEnabled, }, } for _, tc := range testCases { tc := tc - suite.SetupTest() - suite.path.Setup() // setup channel + suite.Run(tc.name, func() { + suite.SetupTest() + suite.path.Setup() // setup channel - expCounterpartyPayee = suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String() - msg = types.NewMsgRegisterCounterpartyPayee( - suite.path.EndpointA.ChannelConfig.PortID, - suite.path.EndpointA.ChannelID, - suite.chainA.SenderAccounts[0].SenderAccount.GetAddress().String(), - expCounterpartyPayee, - ) + expCounterpartyPayee = suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String() + msg = types.NewMsgRegisterCounterpartyPayee( + suite.path.EndpointA.ChannelConfig.PortID, + suite.path.EndpointA.ChannelID, + suite.chainA.SenderAccounts[0].SenderAccount.GetAddress().String(), + expCounterpartyPayee, + ) - tc.malleate() + tc.malleate() - ctx := suite.chainA.GetContext() - res, err := suite.chainA.GetSimApp().IBCFeeKeeper.RegisterCounterpartyPayee(ctx, msg) + ctx := suite.chainA.GetContext() + res, err := suite.chainA.GetSimApp().IBCFeeKeeper.RegisterCounterpartyPayee(ctx, msg) - if tc.expPass { - suite.Require().NoError(err) - suite.Require().NotNil(res) + if tc.expErr == nil { + suite.Require().NoError(err) + suite.Require().NotNil(res) - counterpartyPayee, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetCounterpartyPayeeAddress( - suite.chainA.GetContext(), - suite.chainA.SenderAccount.GetAddress().String(), - suite.path.EndpointA.ChannelID, - ) + counterpartyPayee, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetCounterpartyPayeeAddress( + suite.chainA.GetContext(), + suite.chainA.SenderAccount.GetAddress().String(), + suite.path.EndpointA.ChannelID, + ) - suite.Require().True(found) - suite.Require().Equal(expCounterpartyPayee, counterpartyPayee) + suite.Require().True(found) + suite.Require().Equal(expCounterpartyPayee, counterpartyPayee) - expectedEvents := sdk.Events{ - sdk.NewEvent( - types.EventTypeRegisterCounterpartyPayee, - sdk.NewAttribute(types.AttributeKeyRelayer, suite.chainA.SenderAccount.GetAddress().String()), - sdk.NewAttribute(types.AttributeKeyCounterpartyPayee, counterpartyPayee), - sdk.NewAttribute(types.AttributeKeyChannelID, suite.path.EndpointA.ChannelID), - ), - }.ToABCIEvents() + expectedEvents := sdk.Events{ + sdk.NewEvent( + types.EventTypeRegisterCounterpartyPayee, + sdk.NewAttribute(types.AttributeKeyRelayer, suite.chainA.SenderAccount.GetAddress().String()), + sdk.NewAttribute(types.AttributeKeyCounterpartyPayee, counterpartyPayee), + sdk.NewAttribute(types.AttributeKeyChannelID, suite.path.EndpointA.ChannelID), + ), + }.ToABCIEvents() - expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) - ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) - } else { - suite.Require().Error(err) - } + } else { + suite.Require().ErrorIs(err, tc.expErr) + } + }) } } @@ -210,12 +217,12 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "success with existing packet fees in escrow", @@ -235,7 +242,17 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { eventFee = types.NewFee(defaultRecvFee.Add(escrowFee.RecvFee...), defaultAckFee.Add(escrowFee.AckFee...), defaultTimeoutFee.Add(escrowFee.TimeoutFee...)) }, - true, + nil, + }, + { + "refund account is module account", + func() { + suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibcmock.ModuleName, fee.Total()) //nolint:errcheck // ignore error for testing + msg.Signer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(ibcmock.ModuleName).String() + expPacketFee := types.NewPacketFee(fee, msg.Signer, nil) + expFeesInEscrow = []types.PacketFee{expPacketFee} + }, + nil, }, { "bank send enabled for fee denom", @@ -247,24 +264,14 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { ) suite.Require().NoError(err) }, - true, - }, - { - "refund account is module account", - func() { - suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibcmock.ModuleName, fee.Total()) //nolint:errcheck // ignore error for testing - msg.Signer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(ibcmock.ModuleName).String() - expPacketFee := types.NewPacketFee(fee, msg.Signer, nil) - expFeesInEscrow = []types.PacketFee{expPacketFee} - }, - true, + nil, }, { "fee module is locked", func() { lockFeeModule(suite.chainA) }, - false, + types.ErrFeeModuleLocked, }, { "fee module disabled on channel", @@ -272,21 +279,21 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { msg.SourcePortId = "invalid-port" msg.SourceChannelId = "invalid-channel" }, - false, + types.ErrFeeNotEnabled, }, { "invalid refund address", func() { msg.Signer = "invalid-address" }, - false, + errors.New("decoding bech32 failed"), }, { "refund account does not exist", func() { msg.Signer = suite.chainB.SenderAccount.GetAddress().String() }, - false, + types.ErrRefundAccNotFound, }, { "refund account is a blocked address", @@ -294,7 +301,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() msg.Signer = blockedAddr.String() }, - false, + ibcerrors.ErrUnauthorized, }, { "bank send disabled for fee denom", @@ -306,28 +313,28 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { ) suite.Require().NoError(err) }, - false, + banktypes.ErrSendDisabled, }, { "acknowledgement fee balance not found", func() { msg.Fee.AckFee = invalidCoins }, - false, + sdkerrors.ErrInsufficientFunds, }, { "receive fee balance not found", func() { msg.Fee.RecvFee = invalidCoins }, - false, + sdkerrors.ErrInsufficientFunds, }, { "timeout fee balance not found", func() { msg.Fee.TimeoutFee = invalidCoins }, - false, + sdkerrors.ErrInsufficientFunds, }, } @@ -357,7 +364,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { ctx := suite.chainA.GetContext() _, err := suite.chainA.GetSimApp().IBCFeeKeeper.PayPacketFee(ctx, msg) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) // message committed packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) @@ -384,7 +391,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) suite.Require().Equal(sdkmath.NewInt(0), escrowBalance.Amount) @@ -404,12 +411,12 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "success with existing packet fees in escrow", @@ -427,7 +434,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { expEscrowBalance = expEscrowBalance.Add(fee.Total()...) expFeesInEscrow = append(expFeesInEscrow, packetFee) }, - true, + nil, }, { "bank send enabled for fee denom", @@ -439,14 +446,14 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { ) suite.Require().NoError(err) }, - true, + nil, }, { "fee module is locked", func() { lockFeeModule(suite.chainA) }, - false, + types.ErrFeeModuleLocked, }, { "fee module disabled on channel", @@ -454,7 +461,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { msg.PacketId.PortId = "invalid-port" msg.PacketId.ChannelId = "invalid-channel" }, - false, + types.ErrFeeNotEnabled, }, { "channel does not exist", @@ -465,14 +472,14 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { // NOTE: the channel doesn't exist in 04-channel keeper, but we will add a mapping within ics29 anyways suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), msg.PacketId.PortId, msg.PacketId.ChannelId) }, - false, + channeltypes.ErrSequenceSendNotFound, }, { "packet not sent", func() { msg.PacketId.Sequence++ }, - false, + channeltypes.ErrPacketNotSent, }, { "packet already acknowledged", @@ -480,7 +487,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { err := suite.path.RelayPacket(packet) suite.Require().NoError(err) }, - false, + channeltypes.ErrPacketCommitmentNotFound, }, { "packet already timed out", @@ -502,21 +509,21 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { packetID := channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, sequence) msg.PacketId = packetID }, - false, + channeltypes.ErrPacketCommitmentNotFound, }, { "invalid refund address", func() { msg.PacketFee.RefundAddress = "invalid-address" }, - false, + errors.New("decoding bech32 failed"), }, { "refund account does not exist", func() { msg.PacketFee.RefundAddress = suite.chainB.SenderAccount.GetAddress().String() }, - false, + types.ErrRefundAccNotFound, }, { "refund account is a blocked address", @@ -524,7 +531,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() msg.PacketFee.RefundAddress = blockedAddr.String() }, - false, + ibcerrors.ErrUnauthorized, }, { "bank send disabled for fee denom", @@ -536,28 +543,28 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { ) suite.Require().NoError(err) }, - false, + banktypes.ErrSendDisabled, }, { "acknowledgement fee balance not found", func() { msg.PacketFee.Fee.AckFee = invalidCoins }, - false, + sdkerrors.ErrInsufficientFunds, }, { "receive fee balance not found", func() { msg.PacketFee.Fee.RecvFee = invalidCoins }, - false, + sdkerrors.ErrInsufficientFunds, }, { "timeout fee balance not found", func() { msg.PacketFee.Fee.TimeoutFee = invalidCoins }, - false, + sdkerrors.ErrInsufficientFunds, }, } @@ -587,7 +594,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { _, err = suite.chainA.GetSimApp().IBCFeeKeeper.PayPacketFeeAsync(suite.chainA.GetContext(), msg) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) // message committed feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) @@ -597,7 +604,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) suite.Require().Equal(expEscrowBalance.AmountOf(sdk.DefaultBondDenom), escrowBalance.Amount) } else { - suite.Require().Error(err) + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error()) escrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeModuleAddress(), sdk.DefaultBondDenom) suite.Require().Equal(sdkmath.NewInt(0), escrowBalance.Amount) diff --git a/modules/apps/29-fee/keeper/relay_test.go b/modules/apps/29-fee/keeper/relay_test.go index 7b98c66b5cc..1902c3fa5f5 100644 --- a/modules/apps/29-fee/keeper/relay_test.go +++ b/modules/apps/29-fee/keeper/relay_test.go @@ -12,7 +12,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", @@ -20,12 +20,12 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() { suite.chainB.GetSimApp().IBCFeeKeeper.SetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketID(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID, 1), suite.chainA.SenderAccount.GetAddress().String()) suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyPayeeAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID) }, - true, + nil, }, { "relayer address not set for async WriteAcknowledgement", func() {}, - false, + types.ErrRelayerNotFoundForAsyncAck, }, } @@ -61,7 +61,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() { err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) _, found := suite.chainB.GetSimApp().IBCFeeKeeper.GetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1)) suite.Require().False(found) @@ -70,7 +70,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() { committedAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1) suite.Require().Equal(committedAck, channeltypes.CommitAcknowledgement(expectedAck.Acknowledgement())) } else { - suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } diff --git a/modules/apps/29-fee/types/codec_test.go b/modules/apps/29-fee/types/codec_test.go index 1e435f472a6..56666abab64 100644 --- a/modules/apps/29-fee/types/codec_test.go +++ b/modules/apps/29-fee/types/codec_test.go @@ -1,6 +1,7 @@ package types_test import ( + "errors" "testing" "github.com/stretchr/testify/require" @@ -10,38 +11,39 @@ import ( fee "github.com/cosmos/ibc-go/v9/modules/apps/29-fee" "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types" + ibctesting "github.com/cosmos/ibc-go/v9/testing" ) func TestCodecTypeRegistration(t *testing.T) { testCases := []struct { name string typeURL string - expPass bool + expErr error }{ { "success: MsgPayPacketFee", sdk.MsgTypeURL(&types.MsgPayPacketFee{}), - true, + nil, }, { "success: MsgPayPacketFeeAsync", sdk.MsgTypeURL(&types.MsgPayPacketFeeAsync{}), - true, + nil, }, { "success: MsgRegisterPayee", sdk.MsgTypeURL(&types.MsgRegisterPayee{}), - true, + nil, }, { "success: MsgRegisterCounterpartyPayee", sdk.MsgTypeURL(&types.MsgRegisterCounterpartyPayee{}), - true, + nil, }, { "type not registered on codec", "ibc.invalid.MsgTypeURL", - false, + errors.New("unable to resolve type URL ibc.invalid.MsgTypeURL"), }, } @@ -52,12 +54,12 @@ func TestCodecTypeRegistration(t *testing.T) { encodingCfg := moduletestutil.MakeTestEncodingConfig(fee.AppModuleBasic{}) msg, err := encodingCfg.Codec.InterfaceRegistry().Resolve(tc.typeURL) - if tc.expPass { + if tc.expErr == nil { require.NotNil(t, msg) require.NoError(t, err) } else { require.Nil(t, msg) - require.Error(t, err) + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) } }) } diff --git a/modules/apps/29-fee/types/fee_test.go b/modules/apps/29-fee/types/fee_test.go index fc3e712e1b8..0168b0e2275 100644 --- a/modules/apps/29-fee/types/fee_test.go +++ b/modules/apps/29-fee/types/fee_test.go @@ -1,6 +1,7 @@ package types_test import ( + "errors" "testing" "github.com/stretchr/testify/require" @@ -12,6 +13,8 @@ import ( "github.com/cometbft/cometbft/crypto/secp256k1" "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" + ibctesting "github.com/cosmos/ibc-go/v9/testing" ) var ( @@ -131,26 +134,26 @@ func TestPacketFeeValidation(t *testing.T) { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "success with empty slice for Relayers", func() { packetFee.Relayers = []string{} }, - true, + nil, }, { "should fail when refund address is invalid", func() { packetFee.RefundAddress = invalidAddress }, - false, + errors.New("failed to convert RefundAddress into sdk.AccAddress"), }, { "should fail when all fees are invalid", @@ -159,14 +162,14 @@ func TestPacketFeeValidation(t *testing.T) { packetFee.Fee.RecvFee = invalidFee packetFee.Fee.TimeoutFee = invalidFee }, - false, + ibcerrors.ErrInvalidCoins, }, { "should fail with single invalid fee", func() { packetFee.Fee.AckFee = invalidFee }, - false, + ibcerrors.ErrInvalidCoins, }, { "should fail with two invalid fees", @@ -174,7 +177,7 @@ func TestPacketFeeValidation(t *testing.T) { packetFee.Fee.TimeoutFee = invalidFee packetFee.Fee.AckFee = invalidFee }, - false, + ibcerrors.ErrInvalidCoins, }, { "should pass with two empty fees", @@ -182,14 +185,14 @@ func TestPacketFeeValidation(t *testing.T) { packetFee.Fee.TimeoutFee = sdk.Coins{} packetFee.Fee.AckFee = sdk.Coins{} }, - true, + nil, }, { "should pass with one empty fee", func() { packetFee.Fee.TimeoutFee = sdk.Coins{} }, - true, + nil, }, { "should fail if all fees are empty", @@ -198,31 +201,33 @@ func TestPacketFeeValidation(t *testing.T) { packetFee.Fee.RecvFee = sdk.Coins{} packetFee.Fee.TimeoutFee = sdk.Coins{} }, - false, + ibcerrors.ErrInvalidCoins, }, { "should fail with non empty Relayers", func() { packetFee.Relayers = []string{"relayer"} }, - false, + types.ErrRelayersNotEmpty, }, } for _, tc := range testCases { tc := tc - fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - packetFee = types.NewPacketFee(fee, defaultAccAddress, nil) + t.Run(tc.name, func(t *testing.T) { + fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + packetFee = types.NewPacketFee(fee, defaultAccAddress, nil) - tc.malleate() // malleate mutates test data + tc.malleate() // malleate mutates test data - err := packetFee.Validate() + err := packetFee.Validate() - if tc.expPass { - require.NoError(t, err, tc.name) - } else { - require.Error(t, err, tc.name) - } + if tc.expErr == nil { + require.NoError(t, err, tc.name) + } else { + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) + } + }) } } diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 633a3e80c00..93e063745b9 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -1,6 +1,7 @@ package types_test import ( + "errors" "testing" "github.com/stretchr/testify/require" @@ -11,6 +12,8 @@ import ( "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" + host "github.com/cosmos/ibc-go/v9/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v9/testing" ) @@ -25,159 +28,160 @@ func TestValidateGenesis(t *testing.T) { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success - valid genesis", func() {}, - true, + nil, }, { "invalid packetID: invalid port ID", func() { genState.IdentifiedFees[0].PacketId = channeltypes.NewPacketID("", ibctesting.FirstChannelID, 1) }, - false, + host.ErrInvalidID, }, { "invalid packetID: invalid channel ID", func() { genState.IdentifiedFees[0].PacketId = channeltypes.NewPacketID(ibctesting.MockFeePort, "", 1) }, - false, + host.ErrInvalidID, }, { "invalid packetID: invalid sequence", func() { genState.IdentifiedFees[0].PacketId = channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 0) }, - false, + host.ErrInvalidPacket, }, { "invalid packet fee: invalid fee", func() { genState.IdentifiedFees[0].PacketFees[0].Fee = types.NewFee(sdk.Coins{}, sdk.Coins{}, sdk.Coins{}) }, - false, + ibcerrors.ErrInvalidCoins, }, { "invalid packet fee: invalid refund address", func() { genState.IdentifiedFees[0].PacketFees[0].RefundAddress = "" }, - false, + errors.New("failed to convert RefundAddress into sdk.AccAddress"), }, { "invalid fee enabled channel: invalid port ID", func() { genState.FeeEnabledChannels[0].PortId = "" }, - false, + host.ErrInvalidID, }, { "invalid fee enabled channel: invalid channel ID", func() { genState.FeeEnabledChannels[0].ChannelId = "" }, - false, + host.ErrInvalidID, }, { "invalid registered payee: invalid relayer address", func() { genState.RegisteredPayees[0].Relayer = "" }, - false, + errors.New("failed to convert relayer address into sdk.AccAddress"), }, { "invalid registered payee: invalid payee address", func() { genState.RegisteredPayees[0].Payee = "" }, - false, + errors.New("failed to convert payee address into sdk.AccAddress"), }, { "invalid registered payee: invalid channel ID", func() { genState.RegisteredPayees[0].ChannelId = "" }, - false, + host.ErrInvalidID, }, { "invalid registered counterparty payees: invalid relayer address", func() { genState.RegisteredCounterpartyPayees[0].Relayer = "" }, - false, + errors.New("failed to convert relayer address into sdk.AccAddress"), }, { "invalid registered counterparty payees: invalid counterparty payee", func() { genState.RegisteredCounterpartyPayees[0].CounterpartyPayee = "" }, - false, + types.ErrCounterpartyPayeeEmpty, }, { "invalid forward relayer address: invalid forward address", func() { genState.ForwardRelayers[0].Address = "" }, - false, + errors.New("failed to convert forward relayer address into sdk.AccAddress"), }, { "invalid forward relayer address: invalid packet", func() { genState.ForwardRelayers[0].PacketId = channeltypes.PacketId{} }, - false, + host.ErrInvalidID, }, } for _, tc := range testCases { tc := tc - - genState = &types.GenesisState{ - IdentifiedFees: []types.IdentifiedPacketFees{ - { - PacketId: channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 1), - PacketFees: []types.PacketFee{types.NewPacketFee(types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee), defaultAccAddress, nil)}, + t.Run(tc.name, func(t *testing.T) { + genState = &types.GenesisState{ + IdentifiedFees: []types.IdentifiedPacketFees{ + { + PacketId: channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 1), + PacketFees: []types.PacketFee{types.NewPacketFee(types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee), defaultAccAddress, nil)}, + }, }, - }, - FeeEnabledChannels: []types.FeeEnabledChannel{ - { - PortId: ibctesting.MockFeePort, - ChannelId: ibctesting.FirstChannelID, + FeeEnabledChannels: []types.FeeEnabledChannel{ + { + PortId: ibctesting.MockFeePort, + ChannelId: ibctesting.FirstChannelID, + }, }, - }, - RegisteredCounterpartyPayees: []types.RegisteredCounterpartyPayee{ - { - Relayer: defaultAccAddress, - CounterpartyPayee: defaultAccAddress, - ChannelId: ibctesting.FirstChannelID, + RegisteredCounterpartyPayees: []types.RegisteredCounterpartyPayee{ + { + Relayer: defaultAccAddress, + CounterpartyPayee: defaultAccAddress, + ChannelId: ibctesting.FirstChannelID, + }, }, - }, - ForwardRelayers: []types.ForwardRelayerAddress{ - { - Address: defaultAccAddress, - PacketId: channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 1), + ForwardRelayers: []types.ForwardRelayerAddress{ + { + Address: defaultAccAddress, + PacketId: channeltypes.NewPacketID(ibctesting.MockFeePort, ibctesting.FirstChannelID, 1), + }, }, - }, - RegisteredPayees: []types.RegisteredPayee{ - { - Relayer: sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String(), - Payee: sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String(), - ChannelId: ibctesting.FirstChannelID, + RegisteredPayees: []types.RegisteredPayee{ + { + Relayer: sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String(), + Payee: sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String(), + ChannelId: ibctesting.FirstChannelID, + }, }, - }, - } + } - tc.malleate() + tc.malleate() - err := genState.Validate() + err := genState.Validate() - if tc.expPass { - require.NoError(t, err, tc.name) - } else { - require.Error(t, err, tc.name) - } + if tc.expErr == nil { + require.NoError(t, err, tc.name) + } else { + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) + } + }) } } diff --git a/modules/apps/29-fee/types/keys_test.go b/modules/apps/29-fee/types/keys_test.go index edefc87c83d..ec95afab64b 100644 --- a/modules/apps/29-fee/types/keys_test.go +++ b/modules/apps/29-fee/types/keys_test.go @@ -1,6 +1,7 @@ package types_test import ( + "errors" "fmt" "testing" @@ -8,6 +9,7 @@ import ( "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v9/testing" ) @@ -20,19 +22,19 @@ func TestKeyPayee(t *testing.T) { func TestParseKeyPayee(t *testing.T) { testCases := []struct { - name string - key string - expPass bool + name string + key string + expErr error }{ { "success", string(types.KeyPayee("relayer-address", ibctesting.FirstChannelID)), - true, + nil, }, { "incorrect key - key split has incorrect length", "payeeAddress/relayer_address/transfer/channel-0", - false, + ibcerrors.ErrLogic, }, } @@ -41,12 +43,12 @@ func TestParseKeyPayee(t *testing.T) { address, channelID, err := types.ParseKeyPayeeAddress(tc.key) - if tc.expPass { + if tc.expErr == nil { require.NoError(t, err) require.Equal(t, "relayer-address", address) require.Equal(t, ibctesting.FirstChannelID, channelID) } else { - require.Error(t, err) + require.ErrorIs(t, err, tc.expErr) } } } @@ -68,64 +70,66 @@ func TestKeyFeesInEscrow(t *testing.T) { func TestParseKeyFeeEnabled(t *testing.T) { testCases := []struct { - name string - key string - expPass bool + name string + key string + expErr error }{ { "success", string(types.KeyFeeEnabled(ibctesting.MockPort, ibctesting.FirstChannelID)), - true, + nil, }, { "incorrect key - key split has incorrect length", string(types.KeyFeesInEscrow(validPacketID)), - false, + ibcerrors.ErrLogic, }, { "incorrect key - key split has incorrect length", fmt.Sprintf("%s/%s/%s", "fee", ibctesting.MockPort, ibctesting.FirstChannelID), - false, + ibcerrors.ErrLogic, }, } for _, tc := range testCases { tc := tc - portID, channelID, err := types.ParseKeyFeeEnabled(tc.key) - - if tc.expPass { - require.NoError(t, err) - require.Equal(t, ibctesting.MockPort, portID) - require.Equal(t, ibctesting.FirstChannelID, channelID) - } else { - require.Error(t, err) - require.Empty(t, portID) - require.Empty(t, channelID) - } + t.Run(tc.name, func(t *testing.T) { + portID, channelID, err := types.ParseKeyFeeEnabled(tc.key) + + if tc.expErr == nil { + require.NoError(t, err) + require.Equal(t, ibctesting.MockPort, portID) + require.Equal(t, ibctesting.FirstChannelID, channelID) + } else { + require.ErrorIs(t, err, tc.expErr) + require.Empty(t, portID) + require.Empty(t, channelID) + } + }) } } func TestParseKeyFeesInEscrow(t *testing.T) { testCases := []struct { - name string - key string - expPass bool + name string + key string + expErr error }{ { "success", string(types.KeyFeesInEscrow(validPacketID)), - true, + nil, }, { "incorrect key - key split has incorrect length", string(types.KeyFeeEnabled(validPacketID.PortId, validPacketID.ChannelId)), - false, + ibcerrors.ErrLogic, }, { "incorrect key - sequence cannot be parsed", fmt.Sprintf("%s/%s", types.KeyFeesInEscrowChannelPrefix(validPacketID.PortId, validPacketID.ChannelId), "sequence"), - false, + errors.New("invalid syntax"), }, } @@ -134,35 +138,35 @@ func TestParseKeyFeesInEscrow(t *testing.T) { packetID, err := types.ParseKeyFeesInEscrow(tc.key) - if tc.expPass { + if tc.expErr == nil { require.NoError(t, err) require.Equal(t, validPacketID, packetID) } else { - require.Error(t, err) + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) } } } func TestParseKeyForwardRelayerAddress(t *testing.T) { testCases := []struct { - name string - key string - expPass bool + name string + key string + expErr error }{ { "success", string(types.KeyRelayerAddressForAsyncAck(validPacketID)), - true, + nil, }, { "incorrect key - key split has incorrect length", "forwardRelayer/transfer/channel-0", - false, + ibcerrors.ErrLogic, }, { "incorrect key - sequence is not correct", "forwardRelayer/transfer/channel-0/sequence", - false, + errors.New("invalid syntax"), }, } @@ -171,11 +175,11 @@ func TestParseKeyForwardRelayerAddress(t *testing.T) { packetID, err := types.ParseKeyRelayerAddressForAsyncAck(tc.key) - if tc.expPass { + if tc.expErr == nil { require.NoError(t, err) require.Equal(t, validPacketID, packetID) } else { - require.Error(t, err) + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) } } } @@ -184,19 +188,19 @@ func TestParseKeyCounterpartyPayee(t *testing.T) { relayerAddress := "relayer_address" testCases := []struct { - name string - key string - expPass bool + name string + key string + expErr error }{ { "success", string(types.KeyCounterpartyPayee(relayerAddress, ibctesting.FirstChannelID)), - true, + nil, }, { "incorrect key - key split has incorrect length", "relayerAddress/relayer_address/transfer/channel-0", - false, + ibcerrors.ErrLogic, }, } @@ -205,12 +209,12 @@ func TestParseKeyCounterpartyPayee(t *testing.T) { address, channelID, err := types.ParseKeyCounterpartyPayee(tc.key) - if tc.expPass { + if tc.expErr == nil { require.NoError(t, err) require.Equal(t, relayerAddress, address) require.Equal(t, ibctesting.FirstChannelID, channelID) } else { - require.Error(t, err) + require.ErrorIs(t, err, tc.expErr) } } } diff --git a/modules/apps/29-fee/types/msgs_test.go b/modules/apps/29-fee/types/msgs_test.go index e952ee80907..7699b94e990 100644 --- a/modules/apps/29-fee/types/msgs_test.go +++ b/modules/apps/29-fee/types/msgs_test.go @@ -1,6 +1,7 @@ package types_test import ( + "errors" "testing" "github.com/stretchr/testify/require" @@ -13,6 +14,8 @@ import ( modulefee "github.com/cosmos/ibc-go/v9/modules/apps/29-fee" "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" + host "github.com/cosmos/ibc-go/v9/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v9/testing" ) @@ -22,12 +25,12 @@ func TestMsgRegisterPayeeValidation(t *testing.T) { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "success: relayer and payee are equal", @@ -35,55 +38,57 @@ func TestMsgRegisterPayeeValidation(t *testing.T) { msg.Relayer = defaultAccAddress msg.Payee = defaultAccAddress }, - true, + nil, }, { "invalid portID", func() { msg.PortId = "" }, - false, + host.ErrInvalidID, }, { "invalid channelID", func() { msg.ChannelId = "" }, - false, + host.ErrInvalidID, }, { "invalid relayer address", func() { msg.Relayer = invalidAddress }, - false, + errors.New("failed to create sdk.AccAddress from relayer address"), }, { "invalid payee address", func() { msg.Payee = invalidAddress }, - false, + errors.New("failed to create sdk.AccAddress from payee address"), }, } for i, tc := range testCases { tc := tc - relayerAddr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - payeeAddr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + t.Run(tc.name, func(t *testing.T) { + relayerAddr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + payeeAddr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - msg = types.NewMsgRegisterPayee(ibctesting.MockPort, ibctesting.FirstChannelID, relayerAddr.String(), payeeAddr.String()) + msg = types.NewMsgRegisterPayee(ibctesting.MockPort, ibctesting.FirstChannelID, relayerAddr.String(), payeeAddr.String()) - tc.malleate() + tc.malleate() - err := msg.ValidateBasic() + err := msg.ValidateBasic() - if tc.expPass { - require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) - } else { - require.Error(t, err, "invalid test case %d passed: %s", i, tc.name) - } + if tc.expErr == nil { + require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) + } else { + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) + } + }) } } @@ -103,73 +108,75 @@ func TestMsgRegisterCountepartyPayeeValidation(t *testing.T) { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "invalid portID", func() { msg.PortId = "" }, - false, + host.ErrInvalidID, }, { "invalid channelID", func() { msg.ChannelId = "" }, - false, + host.ErrInvalidID, }, { "validate with incorrect destination relayer address", func() { msg.Relayer = invalidAddress }, - false, + errors.New("failed to create sdk.AccAddress from relayer address"), }, { "invalid counterparty payee address", func() { msg.CounterpartyPayee = "" }, - false, + types.ErrCounterpartyPayeeEmpty, }, { "invalid counterparty payee address: whitespaced empty string", func() { msg.CounterpartyPayee = " " }, - false, + types.ErrCounterpartyPayeeEmpty, }, { "invalid counterparty payee address: too long", func() { msg.CounterpartyPayee = ibctesting.GenerateString(types.MaximumCounterpartyPayeeLength + 1) }, - false, + ibcerrors.ErrInvalidAddress, }, } for i, tc := range testCases { i, tc := i, tc - payeeAddr, err := sdk.AccAddressFromBech32(ibctesting.TestAccAddress) - require.NoError(t, err) - msg = types.NewMsgRegisterCounterpartyPayee(ibctesting.MockPort, ibctesting.FirstChannelID, defaultAccAddress, payeeAddr.String()) + t.Run(tc.name, func(t *testing.T) { + payeeAddr, err := sdk.AccAddressFromBech32(ibctesting.TestAccAddress) + require.NoError(t, err) + msg = types.NewMsgRegisterCounterpartyPayee(ibctesting.MockPort, ibctesting.FirstChannelID, defaultAccAddress, payeeAddr.String()) - tc.malleate() + tc.malleate() - err = msg.ValidateBasic() + err = msg.ValidateBasic() - if tc.expPass { - require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) - } else { - require.Error(t, err, "invalid test case %d passed: %s", i, tc.name) - } + if tc.expErr == nil { + require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) + } else { + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) + } + }) } } @@ -189,47 +196,47 @@ func TestMsgPayPacketFeeValidation(t *testing.T) { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "success with empty relayers", func() { msg.Relayers = []string{} }, - true, + nil, }, { "invalid channelID", func() { msg.SourceChannelId = "" }, - false, + host.ErrInvalidID, }, { "invalid portID", func() { msg.SourcePortId = "" }, - false, + host.ErrInvalidID, }, { "relayers is not nil", func() { msg.Relayers = []string{defaultAccAddress} }, - false, + types.ErrRelayersNotEmpty, }, { "invalid signer address", func() { msg.Signer = invalidAddress }, - false, + errors.New("failed to convert msg.Signer into sdk.AccAddress"), }, } @@ -243,10 +250,10 @@ func TestMsgPayPacketFeeValidation(t *testing.T) { err := msg.ValidateBasic() - if tc.expPass { + if tc.expErr == nil { require.NoError(t, err, tc.name) } else { - require.Error(t, err, tc.name) + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) } } } @@ -268,54 +275,69 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", func() {}, - true, + nil, }, { "success with empty relayers", func() { msg.PacketFee.Relayers = []string{} }, - true, + nil, + }, + { + "should pass with two empty fees", + func() { + msg.PacketFee.Fee.AckFee = sdk.Coins{} + msg.PacketFee.Fee.TimeoutFee = sdk.Coins{} + }, + nil, + }, + { + "should pass with one empty fee", + func() { + msg.PacketFee.Fee.TimeoutFee = sdk.Coins{} + }, + nil, }, { "invalid channelID", func() { msg.PacketId.ChannelId = "" }, - false, + host.ErrInvalidID, }, { "invalid portID", func() { msg.PacketId.PortId = "" }, - false, + host.ErrInvalidID, }, { "invalid sequence", func() { msg.PacketId.Sequence = 0 }, - false, + channeltypes.ErrInvalidPacket, }, { "relayers is not nil", func() { msg.PacketFee.Relayers = []string{defaultAccAddress} }, - false, + types.ErrRelayersNotEmpty, }, { "invalid signer address", func() { msg.PacketFee.RefundAddress = "invalid-addr" }, - false, + errors.New("failed to convert RefundAddress into sdk.AccAddress"), }, { "should fail when all fees are invalid", @@ -324,14 +346,14 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) { msg.PacketFee.Fee.RecvFee = invalidFee msg.PacketFee.Fee.TimeoutFee = invalidFee }, - false, + ibcerrors.ErrInvalidCoins, }, { "should fail with single invalid fee", func() { msg.PacketFee.Fee.AckFee = invalidFee }, - false, + ibcerrors.ErrInvalidCoins, }, { "should fail with two invalid fees", @@ -339,22 +361,7 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) { msg.PacketFee.Fee.AckFee = invalidFee msg.PacketFee.Fee.TimeoutFee = invalidFee }, - false, - }, - { - "should pass with two empty fees", - func() { - msg.PacketFee.Fee.AckFee = sdk.Coins{} - msg.PacketFee.Fee.TimeoutFee = sdk.Coins{} - }, - true, - }, - { - "should pass with one empty fee", - func() { - msg.PacketFee.Fee.TimeoutFee = sdk.Coins{} - }, - true, + ibcerrors.ErrInvalidCoins, }, { "should fail if all fees are empty", @@ -363,7 +370,7 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) { msg.PacketFee.Fee.RecvFee = sdk.Coins{} msg.PacketFee.Fee.TimeoutFee = sdk.Coins{} }, - false, + ibcerrors.ErrInvalidCoins, }, } @@ -380,10 +387,10 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) { err := msg.ValidateBasic() - if tc.expPass { + if tc.expErr == nil { require.NoError(t, err, tc.name) } else { - require.Error(t, err, tc.name) + ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error()) } } } diff --git a/testing/utils.go b/testing/utils.go index 773c39b420d..53d84ab559a 100644 --- a/testing/utils.go +++ b/testing/utils.go @@ -1,8 +1,10 @@ package ibctesting import ( + "errors" "fmt" "math/rand" + "strings" "testing" "github.com/stretchr/testify/require" @@ -81,3 +83,13 @@ func UnmarshalMsgResponses(cdc codec.Codec, data []byte, msgs ...codec.ProtoMars return nil } + +// RequireErrorIsOrContains verifies that the passed error is either a target error or contains its error message. +func RequireErrorIsOrContains(t *testing.T, err, targetError error, msgAndArgs ...interface{}) { + t.Helper() + require.True( + t, + errors.Is(err, targetError) || + strings.Contains(err.Error(), targetError.Error()), + msgAndArgs...) +}