From fb9c600487bd5db2f5e73b88df20962968fee402 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 19 Dec 2022 13:20:22 +0100 Subject: [PATCH] fix: check `x/bank` send enabled before escrowing fees (backport #2942) (#2953) --- CHANGELOG.md | 2 + modules/apps/29-fee/keeper/msg_server.go | 8 ++++ modules/apps/29-fee/keeper/msg_server_test.go | 45 +++++++++++++++++++ modules/apps/29-fee/types/expected_keepers.go | 1 + 4 files changed, 56 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5fb3857ffd..c2726941482 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (apps/29-fee) [\#2942](https://github.com/cosmos/ibc-go/pull/2942) Check `x/bank` send enabled before escrowing fees. + ### Improvements * (apps/29-fee) [\#2786](https://github.com/cosmos/ibc-go/pull/2786) Save gas by checking key existence with `KVStore`'s `Has` method. diff --git a/modules/apps/29-fee/keeper/msg_server.go b/modules/apps/29-fee/keeper/msg_server.go index c86b258d19b..8460e73e048 100644 --- a/modules/apps/29-fee/keeper/msg_server.go +++ b/modules/apps/29-fee/keeper/msg_server.go @@ -92,6 +92,10 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee) return nil, err } + if err := k.bankKeeper.IsSendEnabledCoins(ctx, msg.Fee.Total()...); err != nil { + return nil, err + } + if k.bankKeeper.BlockedAddr(refundAcc) { return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to escrow fees", refundAcc) } @@ -133,6 +137,10 @@ func (k Keeper) PayPacketFeeAsync(goCtx context.Context, msg *types.MsgPayPacket return nil, err } + if err := k.bankKeeper.IsSendEnabledCoins(ctx, msg.PacketFee.Fee.Total()...); err != nil { + return nil, err + } + if k.bankKeeper.BlockedAddr(refundAcc) { return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to escrow fees", refundAcc) } diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index 2623f1b2e5b..6fb3690eaa5 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -2,6 +2,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/cosmos/ibc-go/v5/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" @@ -197,6 +198,17 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { }, true, }, + { + "bank send enabled for fee denom", + func() { + suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), + banktypes.Params{ + SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: true}}, + }, + ) + }, + true, + }, { "refund account is module account", func() { @@ -244,6 +256,17 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { }, false, }, + { + "bank send disabled for fee denom", + func() { + suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), + banktypes.Params{ + SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: false}}, + }, + ) + }, + false, + }, { "acknowledgement fee balance not found", func() { @@ -347,6 +370,17 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { }, true, }, + { + "bank send enabled for fee denom", + func() { + suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), + banktypes.Params{ + SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: true}}, + }, + ) + }, + true, + }, { "fee module is locked", func() { @@ -431,6 +465,17 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { }, false, }, + { + "bank send disabled for fee denom", + func() { + suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), + banktypes.Params{ + SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: false}}, + }, + ) + }, + false, + }, { "acknowledgement fee balance not found", func() { diff --git a/modules/apps/29-fee/types/expected_keepers.go b/modules/apps/29-fee/types/expected_keepers.go index 9373d24e920..0915796e89b 100644 --- a/modules/apps/29-fee/types/expected_keepers.go +++ b/modules/apps/29-fee/types/expected_keepers.go @@ -41,4 +41,5 @@ type BankKeeper interface { SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error BlockedAddr(sdk.AccAddress) bool + IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error }