From 71d7480c923f4227453e8a80f51be01ae7ee845e Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 22 Jun 2022 16:56:29 +0200 Subject: [PATCH] refactor: writing test case for module account incentivizing packet (#1397) * refactor: using SendCoins & writing test case for module account incentivizing packet * updating ModuleAccountAddrs helper fn and adding additional test for refunding to module acc * chore: changelog --- CHANGELOG.md | 1 + modules/apps/29-fee/keeper/escrow_test.go | 50 ++++++++++++++++--- modules/apps/29-fee/keeper/msg_server_test.go | 13 ++++- testing/simapp/app.go | 8 +++ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e33c591d130..cb6e080f5f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/04-channel) [\#1232](https://github.com/cosmos/ibc-go/pull/1232) Updating params on `NewPacketId` and moving to bottom of file. * (app/29-fee) [\#1305](https://github.com/cosmos/ibc-go/pull/1305) Change version string for fee module to `ics29-1` * (app/29-fee) [\#1341](https://github.com/cosmos/ibc-go/pull/1341) Check if the fee module is locked and if the fee module is enabled before refunding all fees +* (testing/simapp) [\#1397](https://github.com/cosmos/ibc-go/pull/1397) Adding mock module to maccperms and adding check to ensure mock module is not a blocked account address. ### Features diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 7ccf87aa91b..0ab829877ea 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -5,6 +5,7 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" + "github.com/cosmos/ibc-go/v3/testing/mock" "github.com/tendermint/tendermint/crypto/secp256k1" ) @@ -18,6 +19,7 @@ func (suite *KeeperTestSuite) TestDistributeFee() { refundAccBal sdk.Coin packetFee types.PacketFee packetFees []types.PacketFee + fee types.Fee ) testCases := []struct { @@ -27,7 +29,10 @@ func (suite *KeeperTestSuite) TestDistributeFee() { }{ { "success", - func() {}, + func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + }, func() { // check if fees has been deleted packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) @@ -56,8 +61,30 @@ func (suite *KeeperTestSuite) TestDistributeFee() { suite.Require().Equal(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0)), balance) }, }, + { + "success: refund account is module account", + func() { + refundAcc = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(mock.ModuleName) + + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + + // fund mock account + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), mock.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...)) + suite.Require().NoError(err) + }, + func() { + // check if the refund acc has been refunded the timeoutFee + expectedRefundAccBal := defaultTimeoutFee[0].Add(defaultTimeoutFee[0]) + balance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) + suite.Require().Equal(expectedRefundAccBal, balance) + }, + }, { "escrow account out of balance, fee module becomes locked - no distribution", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + // pass in an extra packet fee packetFees = append(packetFees, packetFee) }, @@ -76,6 +103,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid forward address", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + forwardRelayer = "invalid address" }, func() { @@ -88,6 +118,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid forward address: blocked address", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + forwardRelayer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() }, func() { @@ -100,6 +133,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid receiver address: ack fee returned to sender", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + reverseRelayer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() }, func() { @@ -112,6 +148,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid refund address: no-op, timeout fee remains in escrow", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + packetFees[0].RefundAddress = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() packetFees[1].RefundAddress = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() }, @@ -137,18 +176,15 @@ func (suite *KeeperTestSuite) TestDistributeFee() { refundAcc = suite.chainA.SenderAccount.GetAddress() packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) - fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + fee = types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - // escrow the packet fees & store the fees in state - packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) - packetFees = []types.PacketFee{packetFee, packetFee} + tc.malleate() + // escrow the packet fees & store the fees in state suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...)) suite.Require().NoError(err) - tc.malleate() - // fetch the account balances before fee distribution (forward, reverse, refund) forwardAccAddress, _ := sdk.AccAddressFromBech32(forwardRelayer) forwardRelayerBal = suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), forwardAccAddress, sdk.DefaultBondDenom) diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index 9a05f294c9d..cdcbe574d1c 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -3,6 +3,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types" "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" @@ -152,6 +153,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { expEscrowBalance sdk.Coins expFeesInEscrow []types.PacketFee msg *types.MsgPayPacketFee + fee types.Fee ) testCases := []struct { @@ -182,6 +184,15 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { }, true, }, + { + "refund account is module account", + func() { + msg.Signer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(disttypes.ModuleName).String() + expPacketFee := types.NewPacketFee(fee, msg.Signer, nil) + expFeesInEscrow = []types.PacketFee{expPacketFee} + }, + true, + }, { "fee module is locked", func() { @@ -241,7 +252,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { suite.SetupTest() suite.coordinator.Setup(suite.path) // setup channel - fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + fee = types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) msg = types.NewMsgPayPacketFee( fee, suite.path.EndpointA.ChannelConfig.PortID, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 94d5640aa17..cb6c1cdccb3 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -43,6 +43,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/capability" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/cosmos/ibc-go/v3/testing/mock" simappparams "github.com/cosmos/ibc-go/v3/testing/simapp/params" "github.com/cosmos/cosmos-sdk/x/crisis" @@ -164,6 +165,7 @@ var ( ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner}, ibcfeetypes.ModuleName: nil, icatypes.ModuleName: nil, + mock.ModuleName: nil, } ) @@ -654,6 +656,12 @@ func (app *SimApp) LoadHeight(height int64) error { func (app *SimApp) ModuleAccountAddrs() map[string]bool { modAccAddrs := make(map[string]bool) for acc := range maccPerms { + // do not add mock module to blocked addresses + // this is only used for testing + if acc == mock.ModuleName { + continue + } + modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true }