Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ca75dd7
chore: use module account instead of custom forward address
gjermundgaraba Jun 24, 2024
b7c0777
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 25, 2024
cdb2298
pull blocked addr checker into keeper method
gjermundgaraba Jun 25, 2024
7b567fd
lint
gjermundgaraba Jun 25, 2024
7c2851d
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 25, 2024
cbbe776
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 25, 2024
4741ab0
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 25, 2024
e626e94
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 25, 2024
b1a7a19
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 25, 2024
a1afba0
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
e186040
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
0bcfc75
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
7973b24
clean up IsBlockedAddr
gjermundgaraba Jun 26, 2024
c3b4cba
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
27cba65
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
c6486a5
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
031e0e9
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
f23dac8
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6561-use-a…
gjermundgaraba Jun 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions modules/apps/transfer/keeper/forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved

msg := types.NewMsgTransfer(
data.Forwarding.Hops[0].PortId,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -133,14 +133,14 @@ 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) {
receiver, err := sdk.AccAddressFromBech32(data.Receiver)
if err != nil {
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "failed to decode receiver address %s: %v", data.Receiver, err)
}

if data.ShouldBeForwarded() {
receiver = types.GetForwardAddress(portID, channelID)
receiver = k.authKeeper.GetModuleAddress(types.ModuleName)
}

return receiver, nil
Expand Down
11 changes: 11 additions & 0 deletions modules/apps/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) && k.bankKeeper.BlockedAddr(addr) {
return true
}
gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved

return false
}
35 changes: 35 additions & 0 deletions modules/apps/transfer/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
})
}
}
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
3 changes: 2 additions & 1 deletion modules/apps/transfer/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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,
},
Expand Down
22 changes: 17 additions & 5 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -176,7 +177,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
}
Expand Down Expand Up @@ -209,7 +210,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)
}

Expand Down Expand Up @@ -256,8 +257,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())
}
Expand Down Expand Up @@ -336,6 +344,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 {
Expand Down Expand Up @@ -365,7 +374,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))
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/relay_forwarding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,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{
{
Expand Down
9 changes: 5 additions & 4 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 0 additions & 17 deletions modules/apps/transfer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down
Loading