Skip to content

Commit

Permalink
Added Arnab's changes in for the custom handler. Also made SendTransf…
Browse files Browse the repository at this point in the history
…er public again so that we can use it.
  • Loading branch information
Matthew Witkowski committed Dec 22, 2022
1 parent d34cef7 commit f18fd1a
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 17 deletions.
4 changes: 2 additions & 2 deletions modules/apps/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type Keeper struct {
channelKeeper types.ChannelKeeper
portKeeper types.PortKeeper
authKeeper types.AccountKeeper
bankKeeper types.BankKeeper
BankKeeper types.BankKeeper
scopedKeeper exported.ScopedKeeper
}

Expand Down Expand Up @@ -54,7 +54,7 @@ func NewKeeper(
channelKeeper: channelKeeper,
portKeeper: portKeeper,
authKeeper: authKeeper,
bankKeeper: bankKeeper,
BankKeeper: bankKeeper,
scopedKeeper: scopedKeeper,
}
}
Expand Down
8 changes: 4 additions & 4 deletions modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
return nil, err
}

if !k.bankKeeper.IsSendEnabledCoin(ctx, msg.Token) {
if !k.BankKeeper.IsSendEnabledCoin(ctx, msg.Token) {
return nil, sdkerrors.Wrapf(types.ErrSendDisabled, "%s transfers are currently disabled", msg.Token.Denom)
}

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

sequence, err := k.sendTransfer(
sequence, err := k.SendTransfer(
ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp,
msg.Memo)
msg.Memo, nil)
if err != nil {
return nil, err
}
Expand Down
36 changes: 25 additions & 11 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (
coretypes "github.com/cosmos/ibc-go/v6/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)

// 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 @@ -48,7 +51,7 @@ import (
// 4. A -> C : sender chain is sink zone. Denom upon receiving: 'C/B/denom'
// 5. C -> B : sender chain is sink zone. Denom upon receiving: 'B/denom'
// 6. B -> A : sender chain is sink zone. Denom upon receiving: 'denom'
func (k Keeper) sendTransfer(
func (k Keeper) SendTransfer(
ctx sdk.Context,
sourcePort,
sourceChannel string,
Expand All @@ -58,7 +61,18 @@ func (k Keeper) sendTransfer(
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
memo string,
// check restrictions condition met for this address and coin.
checkRestrictionsHandler CheckRestrictionsHandler,

) (uint64, error) {
// if nil then apply default checks
if checkRestrictionsHandler != nil {
res, err := checkRestrictionsHandler(ctx, k, sender, token)
if !res {
return 0, err
}
}

channel, found := k.channelKeeper.GetChannel(ctx, sourcePort, sourceChannel)
if !found {
return 0, sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", sourcePort, sourceChannel)
Expand Down Expand Up @@ -104,7 +118,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 0, err
Expand All @@ -113,13 +127,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 0, 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 @@ -215,13 +229,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 @@ -276,14 +290,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 @@ -357,7 +371,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 @@ -369,13 +383,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
182 changes: 182 additions & 0 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,196 @@ 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/v6/modules/apps/transfer/keeper"
"github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v6/testing"
"github.com/cosmos/ibc-go/v6/testing/simapp"
)

// test sending from chainA to chainB using both coin that orignate on
// chainA and coin that orignate on chainB
func (suite *KeeperTestSuite) TestSendTransferOriginal() {
var (
amount sdk.Coin
path *ibctesting.Path
sender sdk.AccAddress
err error
)

testCases := []struct {
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, 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, nil,
},
{
"source channel not found",
func() {
// channel references wrong ID
suite.coordinator.CreateTransferChannels(path)
path.EndpointA.ChannelID = ibctesting.InvalidID
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, true, false, nil,
},
{
"next seq send not found",
func() {
path.EndpointA.ChannelID = "channel-0"
path.EndpointB.ChannelID = "channel-0"
// manually create channel so next seq send is never set
suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetChannel(
suite.chainA.GetContext(),
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{path.EndpointA.ConnectionID}, ibctesting.DefaultChannelVersion),
)
suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, 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, nil,
},
// createOutgoingPacket tests
// - source chain
{
"send coin failed",
func() {
suite.coordinator.CreateTransferChannels(path)
amount = sdk.NewCoin("randomdenom", sdk.NewInt(100))
}, 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, nil,
},
{
"channel capability not found",
func() {
suite.coordinator.CreateTransferChannels(path)
cap := suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)

// Release channel capability
suite.chainA.GetSimApp().ScopedTransferKeeper.ReleaseCapability(suite.chainA.GetContext(), cap)
amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
}, true, false, nil,
},
{
"send coin success,even though send coin is disabled, by passing in custom checkRestrictionHandler(for restricted 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
},
},
}

for _, tc := range testCases {
tc := tc

suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset
path = NewTransferPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
sender = suite.chainA.SenderAccount.GetAddress()

tc.malleate()

if !tc.sendFromSource {
// send coin from chainB to chainA
coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(0, 110), 0, "")
_, err = suite.chainB.SendMsgs(transferMsg)
suite.Require().NoError(err) // message committed

// receive coin on chainA from chainB
fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA.Denom, coinFromBToA.Amount.String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), "")
packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, clienttypes.NewHeight(0, 110), 0)

// get proof of packet commitment from chainB
err = path.EndpointA.UpdateClient()
suite.Require().NoError(err)
packetKey := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
proof, proofHeight := path.EndpointB.QueryProof(packetKey)

recvMsg := channeltypes.NewMsgRecvPacket(packet, proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String())
_, err = suite.chainA.SendMsgs(recvMsg)
suite.Require().NoError(err) // message committed
}

_, 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, "", tc.checkRestrictionsHandler,
)

if tc.expPass {
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
}
})
}
}

// test sending from chainA to chainB using both coin that orignate on
// chainA and coin that orignate on chainB
func (suite *KeeperTestSuite) TestSendTransfer() {
Expand Down
1 change: 1 addition & 0 deletions modules/apps/transfer/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type BankKeeper interface {
SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
BlockedAddr(addr sdk.AccAddress) bool
IsSendEnabledCoin(ctx sdk.Context, coin sdk.Coin) bool
IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error
}

// ChannelKeeper defines the expected IBC channel keeper
Expand Down

0 comments on commit f18fd1a

Please sign in to comment.