chore: use module account instead of custom forward address (#6688)

* chore: use module account instead of custom forward address

* pull blocked addr checker into keeper method

* lint

* clean up IsBlockedAddr

modules/apps/transfer/keeper/forwarding.go

@@ -20,8 +20,8 @@ func (k Keeper) forwardPacket(ctx sdk.Context, data types.FungibleTokenPacketDat
 		nextForwardingPath = types.NewForwarding(false, data.Forwarding.Hops[1:]...)
 	}
 
-	// sending from the forward escrow address to the original receiver address.
-	sender := types.GetForwardAddress(packet.DestinationPort, packet.DestinationChannel)
+	// sending from module account (used as a temporary forward escrow) to the original receiver address.
+	sender := k.authKeeper.GetModuleAddress(types.ModuleName)
 
 	msg := types.NewMsgTransfer(
 		data.Forwarding.Hops[0].PortId,
@@ -101,7 +101,7 @@ func (k Keeper) revertForwardedPacket(ctx sdk.Context, prevPacket channeltypes.P
 			2. Burning voucher tokens if the funds are foreign
 	*/
 
-	forwardingAddr := types.GetForwardAddress(prevPacket.DestinationPort, prevPacket.DestinationChannel)
+	forwardingAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
 	escrow := types.GetEscrowAddress(prevPacket.DestinationPort, prevPacket.DestinationChannel)
 
 	// we can iterate over the received tokens of prevPacket by iterating over the sent tokens of failedPacketData
@@ -133,10 +133,10 @@ func (k Keeper) revertForwardedPacket(ctx sdk.Context, prevPacket channeltypes.P
 
 // getReceiverFromPacketData returns either the sender specified in the packet data or the forwarding address
 // if there are still hops left to perform.
-func getReceiverFromPacketData(data types.FungibleTokenPacketDataV2, portID, channelID string) (sdk.AccAddress, error) {
+func (k Keeper) getReceiverFromPacketData(data types.FungibleTokenPacketDataV2) (sdk.AccAddress, error) {
 	if data.ShouldBeForwarded() {
 		// since data.Receiver can potentially be a non-CosmosSDK AccAddress, we return early if the packet should be forwarded
-		return types.GetForwardAddress(portID, channelID), nil
+		return k.authKeeper.GetModuleAddress(types.ModuleName), nil
 	}
 
 	receiver, err := sdk.AccAddressFromBech32(data.Receiver)

modules/apps/transfer/keeper/keeper.go

@@ -336,3 +336,14 @@ func (k Keeper) deleteForwardedPacket(ctx sdk.Context, portID, channelID string,
 
 	store.Delete(packetKey)
 }
+
+// IsBlockedAddr checks if the given address is allowed to send or receive tokens.
+// The module account is always allowed to send and receive tokens.
+func (k Keeper) IsBlockedAddr(addr sdk.AccAddress) bool {
+	moduleAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
+	if addr.Equals(moduleAddr) {
+		return false
+	}
+
+	return k.bankKeeper.BlockedAddr(addr)
+}

modules/apps/transfer/keeper/keeper_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
+	minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
 
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper"
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
@@ -350,3 +351,37 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {
 
 	suite.Require().IsType((*channelkeeper.Keeper)(nil), ics4Wrapper)
 }
+
+func (suite *KeeperTestSuite) TestIsBlockedAddr() {
+	suite.SetupTest()
+
+	testCases := []struct {
+		name     string
+		addr     sdk.AccAddress
+		expBlock bool
+	}{
+		{
+			"transfer module account address",
+			suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName),
+			false,
+		},
+		{
+			"regular address",
+			suite.chainA.SenderAccount.GetAddress(),
+			false,
+		},
+		{
+			"blocked address",
+			suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName),
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		suite.Run(tc.name, func() {
+			suite.Require().Equal(tc.expBlock, suite.chainA.GetSimApp().TransferKeeper.IsBlockedAddr(tc.addr))
+		})
+	}
+}

modules/apps/transfer/keeper/msg_server.go

@@ -32,7 +32,7 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 		return nil, errorsmod.Wrapf(types.ErrSendDisabled, err.Error())
 	}
 
-	if k.bankKeeper.BlockedAddr(sender) {
+	if k.IsBlockedAddr(sender) {
 		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
 	}
 

modules/apps/transfer/keeper/msg_server_test.go

@@ -9,6 +9,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
 
 	abci "github.com/cometbft/cometbft/abci/types"
 
@@ -80,7 +81,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 		{
 			"failure: sender is a blocked address",
 			func() {
-				msg.Sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName).String()
+				msg.Sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName).String()
 			},
 			ibcerrors.ErrUnauthorized,
 		},

modules/apps/transfer/keeper/relay.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/telemetry"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/events"
 	internaltelemetry "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/telemetry"
@@ -179,7 +180,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
 		return types.ErrReceiveDisabled
 	}
 
-	receiver, err := getReceiverFromPacketData(data, packet.DestinationPort, packet.DestinationChannel)
+	receiver, err := k.getReceiverFromPacketData(data)
 	if err != nil {
 		return err
 	}
@@ -212,7 +213,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
 
 			coin := sdk.NewCoin(token.Denom.IBCDenom(), transferAmount)
 
-			if k.bankKeeper.BlockedAddr(receiver) {
+			if k.IsBlockedAddr(receiver) {
 				return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver)
 			}
 
@@ -259,8 +260,15 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
 		}
 
 		// send to receiver
-		if err := k.bankKeeper.SendCoinsFromModuleToAccount(
-			ctx, types.ModuleName, receiver, sdk.NewCoins(voucher),
+		if k.IsBlockedAddr(receiver) {
+			return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver)
+		}
+		moduleAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
+		if moduleAddr == nil {
+			return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", types.ModuleName)
+		}
+		if err := k.bankKeeper.SendCoins(
+			ctx, moduleAddr, receiver, sdk.NewCoins(voucher),
 		); err != nil {
 			return errorsmod.Wrapf(err, "failed to send coins to receiver %s", receiver.String())
 		}
@@ -339,6 +347,7 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, dat
 func (k Keeper) refundPacketTokens(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketDataV2) error {
 	// NOTE: packet data type already checked in handler.go
 
+	moduleAccountAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
 	for _, token := range data.Tokens {
 		coin, err := token.ToCoin()
 		if err != nil {
@@ -368,7 +377,10 @@ func (k Keeper) refundPacketTokens(ctx sdk.Context, packet channeltypes.Packet,
 			return err
 		}
 
-		if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sender, sdk.NewCoins(coin)); err != nil {
+		if k.IsBlockedAddr(sender) {
+			return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
+		}
+		if err := k.bankKeeper.SendCoins(ctx, moduleAccountAddr, sender, sdk.NewCoins(coin)); err != nil {
 			panic(fmt.Errorf("unable to send coins from module to account despite previously minting coins to module account: %v", err))
 		}
 	}

modules/apps/transfer/keeper/relay_forwarding_test.go

@@ -951,7 +951,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() {
 	suite.Require().True(found, "Chain B has no forwarded packet")
 	suite.Require().Equal(packet, forwardedPacket, "ForwardedPacket stored in ChainB is not the same that was sent")
 
-	address := types.GetForwardAddress(packet.DestinationPort, packet.DestinationChannel).String()
+	address := suite.chainB.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName).String()
 	data := types.NewFungibleTokenPacketDataV2(
 		[]types.Token{
 			{

modules/apps/transfer/keeper/relay_test.go

@@ -11,6 +11,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
 
 	transferkeeper "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper"
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
@@ -124,7 +125,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 		{
 			"failure: sender account is blocked",
 			func() {
-				sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName)
+				sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName)
 			},
 			ibcerrors.ErrUnauthorized,
 		},
@@ -364,9 +365,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket_ReceiverIsNotSource() {
 		{
 			"failure: receiver is module account",
 			func() {
-				receiver = suite.chainB.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName).String()
+				receiver = suite.chainB.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName).String()
 			},
-			sdkerrors.ErrUnauthorized,
+			ibcerrors.ErrUnauthorized,
 		},
 		{
 			"failure: receive is disabled",
@@ -522,7 +523,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket_ReceiverIsSource() {
 		{
 			"failure: receiver is module account",
 			func() {
-				receiver = suite.chainB.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName).String()
+				receiver = suite.chainB.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName).String()
 				expEscrowAmount = sdkmath.NewInt(100)
 			},
 			ibcerrors.ErrUnauthorized,

modules/apps/transfer/types/keys.go

@@ -49,9 +49,6 @@ const (
 	// this address has been reasoned about to avoid collisions with other addresses
 	// https://github.com/cosmos/cosmos-sdk/issues/7737#issuecomment-735671951
 	escrowAddressVersion = V1
-
-	// this new address is introduced specifically with ics20-2.
-	forwardAddressVersion = "forwardingAddress"
 )
 
 var (
@@ -82,20 +79,6 @@ func GetEscrowAddress(portID, channelID string) sdk.AccAddress {
 	return hash[:20]
 }
 
-// GetForwardAddress returns the escrow address for the specified channel.
-func GetForwardAddress(portID, channelID string) sdk.AccAddress {
-	// a slash is used to create domain separation between port and channel identifiers to
-	// prevent address collisions between escrow addresses created for different channels
-	contents := fmt.Sprintf("%s/%s", portID, channelID)
-
-	// ADR 028 AddressHash construction
-	preImage := []byte(forwardAddressVersion)
-	preImage = append(preImage, 0)
-	preImage = append(preImage, contents...)
-	hash := sha256.Sum256(preImage)
-	return hash[:20]
-}
-
 // TotalEscrowForDenomKey returns the store key of under which the total amount of
 // source chain tokens in escrow is stored.
 func TotalEscrowForDenomKey(denom string) []byte {