refactor: writing test case for module account incentivizing packet (cosmos#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

Author: seantking

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
 

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)

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,

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
 	}