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

Ibc marker transfer #1

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/migrations/v2-to-v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ For example, if a chain chooses not to integrate a controller submodule, it may

#### Add `StoreUpgrades` for ICS27 module

For ICS27 it is also necessary to [manually add store upgrades](https://docs.cosmos.network/v0.44/core/upgrade.html#add-storeupgrades-for-new-modules) for the new ICS27 module and then configure the store loader to apply those upgrades in `app.go`:
For ICS27 it is also necessary to [manually add store upgrades](https://docs.cosmos.network/v0.46/core/upgrade.html#add-storeupgrades-for-new-modules) for the new ICS27 module and then configure the store loader to apply those upgrades in `app.go`:

```go
if upgradeInfo.Name == "v3" && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type Keeper struct {
channelKeeper types.ChannelKeeper
portKeeper types.PortKeeper
authKeeper types.AccountKeeper
bankKeeper types.BankKeeper
BankKeeper types.BankKeeper
scopedKeeper capabilitykeeper.ScopedKeeper
}

Expand Down Expand Up @@ -53,7 +53,7 @@ func NewKeeper(
channelKeeper: channelKeeper,
portKeeper: portKeeper,
authKeeper: authKeeper,
bankKeeper: bankKeeper,
BankKeeper: bankKeeper,
scopedKeeper: scopedKeeper,
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/mbt_relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() {
sender,
tc.packet.Data.Receiver,
clienttypes.NewHeight(0, 110),
0)
0, nil)
}
case "OnRecvPacket":
err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data)
Expand Down
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 @@ -20,7 +20,7 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
}

if err := k.SendTransfer(
ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp,
ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp, nil,
); err != nil {
return nil, err
}
Expand Down
65 changes: 44 additions & 21 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,27 @@ import (
coretypes "github.com/cosmos/ibc-go/v5/modules/core/types"
)

// CheckRestrictionsHandler checks if the ibc transfer should happen.
type CheckRestrictionsHandler func(ctx sdk.Context, k Keeper, sender sdk.AccAddress, token sdk.Coin) (canTransfer bool, err error)

// DefaultCheckRestrictionsHandler if no custom CheckRestrictionsHandler is provided, this should be used.
func DefaultCheckRestrictionsHandler(ctx sdk.Context, k Keeper, sender sdk.AccAddress, token sdk.Coin) (bool, error) {
// This is IBC modules sendEnabled flag
if !k.GetSendEnabled(ctx) {
return false, types.ErrSendDisabled
}

// Check if the address is blocked by the bank module
if k.BankKeeper.BlockedAddr(sender) {
return false, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
}
// Check if the coin transfer is disabled by the bank module.
if err := k.BankKeeper.IsSendEnabledCoins(ctx, sdk.NewCoins(token)...); err != nil {
return false, err
}
return true, nil
}

// SendTransfer handles transfer sending logic. There are 2 possible cases:
//
// 1. Sender chain is acting as the source zone. The coins are transferred
Expand Down Expand Up @@ -59,19 +80,21 @@ func (k Keeper) SendTransfer(
receiver string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
// check restrictions condition met for this address and coin.
checkRestrictionsHandler CheckRestrictionsHandler,
) error {
if !k.GetSendEnabled(ctx) {
return types.ErrSendDisabled
}

if k.bankKeeper.BlockedAddr(sender) {
return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
}

if err := k.bankKeeper.IsSendEnabledCoins(ctx, sdk.NewCoins(token)...); err != nil {
return err
// if nil then apply default checks
if checkRestrictionsHandler != nil {
res, err := checkRestrictionsHandler(ctx, k, sender, token)
if !res {
return err
}
} else {
res, err := DefaultCheckRestrictionsHandler(ctx, k, sender, token)
if !res {
return err
}
}

sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, sourcePort, sourceChannel)
if !found {
return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", sourcePort, sourceChannel)
Expand Down Expand Up @@ -126,7 +149,7 @@ func (k Keeper) SendTransfer(
escrowAddress := types.GetEscrowAddress(sourcePort, sourceChannel)

// escrow source tokens. It fails if balance insufficient.
if err := k.bankKeeper.SendCoins(
if err := k.BankKeeper.SendCoins(
ctx, sender, escrowAddress, sdk.NewCoins(token),
); err != nil {
return err
Expand All @@ -136,13 +159,13 @@ func (k Keeper) SendTransfer(
labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "false"))

// transfer the coins to the module account and burn them
if err := k.bankKeeper.SendCoinsFromAccountToModule(
if err := k.BankKeeper.SendCoinsFromAccountToModule(
ctx, sender, types.ModuleName, sdk.NewCoins(token),
); err != nil {
return err
}

if err := k.bankKeeper.BurnCoins(
if err := k.BankKeeper.BurnCoins(
ctx, types.ModuleName, sdk.NewCoins(token),
); err != nil {
// NOTE: should not happen as the module account was
Expand Down Expand Up @@ -248,13 +271,13 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
}
token := sdk.NewCoin(denom, transferAmount)

if k.bankKeeper.BlockedAddr(receiver) {
if k.BankKeeper.BlockedAddr(receiver) {
return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver)
}

// unescrow tokens
escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
if err := k.bankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(token)); err != nil {
if err := k.BankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(token)); err != nil {
// NOTE: this error is only expected to occur given an unexpected bug or a malicious
// counterparty module. The bug may occur in bank or any part of the code that allows
// the escrow address to be drained. A malicious counterparty module could drain the
Expand Down Expand Up @@ -309,14 +332,14 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
voucher := sdk.NewCoin(voucherDenom, transferAmount)

// mint new tokens if the source of the transfer is the same chain
if err := k.bankKeeper.MintCoins(
if err := k.BankKeeper.MintCoins(
ctx, types.ModuleName, sdk.NewCoins(voucher),
); err != nil {
return err
}

// send to receiver
if err := k.bankKeeper.SendCoinsFromModuleToAccount(
if err := k.BankKeeper.SendCoinsFromModuleToAccount(
ctx, types.ModuleName, receiver, sdk.NewCoins(voucher),
); err != nil {
return err
Expand Down Expand Up @@ -390,7 +413,7 @@ func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, d
if types.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) {
// unescrow tokens back to sender
escrowAddress := types.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel())
if err := k.bankKeeper.SendCoins(ctx, escrowAddress, sender, sdk.NewCoins(token)); err != nil {
if err := k.BankKeeper.SendCoins(ctx, escrowAddress, sender, sdk.NewCoins(token)); err != nil {
// NOTE: this error is only expected to occur given an unexpected bug or a malicious
// counterparty module. The bug may occur in bank or any part of the code that allows
// the escrow address to be drained. A malicious counterparty module could drain the
Expand All @@ -402,13 +425,13 @@ func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, d
}

// mint vouchers back to sender
if err := k.bankKeeper.MintCoins(
if err := k.BankKeeper.MintCoins(
ctx, types.ModuleName, sdk.NewCoins(token),
); err != nil {
return err
}

if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sender, sdk.NewCoins(token)); err != nil {
if err := k.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sender, sdk.NewCoins(token)); err != nil {
panic(fmt.Sprintf("unable to send coins from module to account despite previously minting coins to module account: %v", err))
}

Expand Down
67 changes: 53 additions & 14 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (

"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/cosmos/ibc-go/v5/modules/apps/transfer/keeper"

"github.com/cosmos/ibc-go/v5/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
Expand All @@ -26,25 +28,26 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
)

testCases := []struct {
msg string
malleate func()
sendFromSource bool
expPass bool
msg string
malleate func()
sendFromSource bool
expPass bool
checkRestrictionsHandler keeper.CheckRestrictionsHandler
}{
{
"successful transfer from source chain",
func() {
suite.coordinator.CreateTransferChannels(path)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, true, true,
}, true, true, nil,
},
{
"successful transfer with coin from counterparty chain",
func() {
// send coin from chainA back to chainB
suite.coordinator.CreateTransferChannels(path)
amount = types.GetTransferCoin(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom, sdk.NewInt(100))
}, false, true,
}, false, true, nil,
},
{
"source channel not found",
Expand All @@ -53,7 +56,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
suite.coordinator.CreateTransferChannels(path)
path.EndpointA.ChannelID = ibctesting.InvalidID
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, true, false,
}, true, false, nil,
},
{
"next seq send not found",
Expand All @@ -68,15 +71,15 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
)
suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, true, false,
}, true, false, nil,
},
{
"transfer failed - sender account is blocked",
func() {
suite.coordinator.CreateTransferChannels(path)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName)
}, true, false,
}, true, false, nil,
},
// createOutgoingPacket tests
// - source chain
Expand All @@ -85,15 +88,15 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
func() {
suite.coordinator.CreateTransferChannels(path)
amount = sdk.NewCoin("randomdenom", sdk.NewInt(100))
}, true, false,
}, true, false, nil,
},
// - receiving chain
{
"send from module account failed",
func() {
suite.coordinator.CreateTransferChannels(path)
amount = types.GetTransferCoin(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, " randomdenom", sdk.NewInt(100))
}, false, false,
}, false, false, nil,
},
{
"channel capability not found",
Expand All @@ -104,7 +107,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
// Release channel capability
suite.chainA.GetSimApp().ScopedTransferKeeper.ReleaseCapability(suite.chainA.GetContext(), cap)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, true, false,
}, true, false, nil,
},
{
"send coin failed - send coin is disabled",
Expand All @@ -116,7 +119,43 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
params := suite.chainA.GetSimApp().BankKeeper.GetParams(suite.chainA.GetContext())
params.SendEnabled = append(params.SendEnabled, banktypes.NewSendEnabled(sdk.DefaultBondDenom, false))
suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), params)
}, true, false,
}, true, false, nil,
}, {
"send coin success,even though send coin is disabled, by passing in custom checkRestrictionHandler(for marker) ",
func() {
suite.coordinator.CreateTransferChannels(path)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))

// Disable SendEnabled
params := suite.chainA.GetSimApp().BankKeeper.GetParams(suite.chainA.GetContext())
params.SendEnabled = append(params.SendEnabled, banktypes.NewSendEnabled(sdk.DefaultBondDenom, false))
suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), params)
}, true, true, func(ctx sdk.Context, k keeper.Keeper, sender sdk.AccAddress, token sdk.Coin) (canTransfer bool, err error) {
if !k.GetSendEnabled(ctx) {
return false, types.ErrSendDisabled
}
if k.BankKeeper.BlockedAddr(sender) {
return false, sdkerrors.ErrUnauthorized.Wrapf("%s is not allowed to send funds", sender)
}
return true, nil
},
},
{
"send coin failed, send coin is disabled, pass in customHandler that checks sendEnabled ",
func() {
suite.coordinator.CreateTransferChannels(path)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))

// Disable SendEnabled
params := suite.chainA.GetSimApp().BankKeeper.GetParams(suite.chainA.GetContext())
params.SendEnabled = append(params.SendEnabled, banktypes.NewSendEnabled(sdk.DefaultBondDenom, false))
suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), params)
}, true, false, func(ctx sdk.Context, k keeper.Keeper, sender sdk.AccAddress, token sdk.Coin) (canTransfer bool, err error) {
if err := k.BankKeeper.IsSendEnabledCoins(ctx, sdk.NewCoins(token)...); err != nil {
return false, err
}
return true, nil
},
},
}

Expand Down Expand Up @@ -155,7 +194,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() {

err = suite.chainA.GetSimApp().TransferKeeper.SendTransfer(
suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, amount,
sender, suite.chainB.SenderAccount.GetAddress().String(), suite.chainB.GetTimeoutHeight(), 0,
sender, suite.chainB.SenderAccount.GetAddress().String(), suite.chainB.GetTimeoutHeight(), 0, tc.checkRestrictionsHandler,
)

if tc.expPass {
Expand Down