From 56d565d9b58642a8f7b2d647cccd3107e51fbd19 Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 18 May 2022 16:53:49 +0200 Subject: [PATCH 1/3] refactor: using SendCoins & writing test case for module account incentivizing packet --- modules/apps/29-fee/keeper/msg_server_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index 84c7588a6b2..782c0342646 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" @@ -76,6 +77,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { expEscrowBalance sdk.Coins expFeesInEscrow []types.PacketFee msg *types.MsgPayPacketFee + fee types.Fee ) testCases := []struct { @@ -106,6 +108,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() { @@ -165,7 +176,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, From 9b3f30d0f45edd030604e0219a1f47284b74f561 Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 22 Jun 2022 11:32:40 +0200 Subject: [PATCH 2/3] updating ModuleAccountAddrs helper fn and adding additional test for refunding to module acc --- modules/apps/29-fee/keeper/escrow_test.go | 50 +++++++++++++++++++---- testing/simapp/app.go | 8 ++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 4ac6e2ed03f..040f3d25b1a 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -7,6 +7,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" ) func (suite *KeeperTestSuite) TestDistributeFee() { @@ -19,6 +20,7 @@ func (suite *KeeperTestSuite) TestDistributeFee() { refundAccBal sdk.Coin packetFee types.PacketFee packetFees []types.PacketFee + fee types.Fee ) testCases := []struct { @@ -28,7 +30,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) @@ -57,8 +62,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) }, @@ -77,6 +104,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() { @@ -89,6 +119,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() { @@ -101,6 +134,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() { @@ -113,6 +149,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() }, @@ -138,18 +177,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/testing/simapp/app.go b/testing/simapp/app.go index f707ba6087b..5442a3d6c9b 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, } ) @@ -648,6 +650,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 } From 4f02f66212c55d0439b97451f38eb8e90f300443 Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 22 Jun 2022 16:47:12 +0200 Subject: [PATCH 3/3] chore: changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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