Skip to content

Commit

Permalink
fix: disable ibc OnRecvPacket packetData router and fee
Browse files Browse the repository at this point in the history
  • Loading branch information
fx0x55 committed Dec 9, 2022
1 parent 6da6b3b commit 6a3c531
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 94 deletions.
7 changes: 2 additions & 5 deletions app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ import (
"github.com/cosmos/cosmos-sdk/x/feegrant"
feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper"

pundixtransfer "github.com/pundix/pundix/x/ibc/applications/transfer"
pundixtransferkeeper "github.com/pundix/pundix/x/ibc/applications/transfer/keeper"
pundixtransfertypes "github.com/pundix/pundix/x/ibc/applications/transfer/types"

govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
pundixtransfer "github.com/pundix/pundix/x/ibc/applications/transfer"
pundixtransferkeeper "github.com/pundix/pundix/x/ibc/applications/transfer/keeper"

minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
"github.com/cosmos/cosmos-sdk/x/params"
Expand Down Expand Up @@ -283,7 +281,6 @@ func NewAppKeeper(
appKeepers.TransferModule = transfer.NewAppModule(appKeepers.TransferKeeper)
transferIBCModule := transfer.NewIBCModule(appKeepers.TransferKeeper)

appKeepers.PundixTransferKeeper.SetRouter(pundixtransfertypes.NewRouter())
pundixIBCMiddleware := pundixtransfer.NewIBCModule(appKeepers.PundixTransferKeeper, transferIBCModule)
appKeepers.PundixTransferModule = pundixtransfer.NewAppModule(appKeepers.PundixTransferKeeper)

Expand Down
20 changes: 0 additions & 20 deletions x/ibc/applications/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ type Keeper struct {
authKeeper types.AccountKeeper
bankKeeper types.BankKeeper
scopedKeeper capabilitykeeper.ScopedKeeper
Router *types.Router
RefundHook types.RefundHook
}

// NewKeeper creates a new IBC transfer Keeper instance
Expand Down Expand Up @@ -56,24 +54,6 @@ func NewKeeper(keeper ibctransferkeeper.Keeper,
}
}

// SetRouter sets the Router in IBC Transfer Keeper and seals it. The method panics if
// there is an existing router that's already sealed.
func (k Keeper) SetRouter(rtr *types.Router) {
if k.Router != nil && k.Router.Sealed() {
panic("cannot reset a sealed router")
}
k.Router = rtr
k.Router.Seal()
}

func (k Keeper) GetRouter() *types.Router {
return k.Router
}

func (k *Keeper) SetRefundHook(hook types.RefundHook) {
k.RefundHook = hook
}

// Logger returns a module-specific logger.
func (k Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", "x/"+host.ModuleName+"-"+types.CompatibleModuleName)
Expand Down
85 changes: 17 additions & 68 deletions x/ibc/applications/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,14 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
return err
}

if len(data.Router) > 0 {
return sdkerrors.Wrapf(types.ErrNotSupportRouter, "router:(%s)", data.Router)
}

if data.Fee != sdk.ZeroInt().String() {
return sdkerrors.Wrapf(types.ErrNotSupportFee, "fee:(%s)", data.Router)
}

receiver, transferAmount, feeAmount, err := parseReceiveAndAmountByPacket(data)
if err != nil {
return err
Expand All @@ -223,33 +231,6 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
),
)

if data.Router == "" || !k.Router.HasRoute(data.Router) {
return nil
}
if route, exists := k.Router.GetRoute(data.Router); exists {
ibcAmount := sdk.NewCoin(receiveDenom, transferAmount)
ibcFee := sdk.NewCoin(receiveDenom, feeAmount)
ctx.Logger().Info("IBCTransfer", "transfer route sourceChannel", packet.GetSourceChannel(),
"destChannel", packet.GetDestChannel(), "sequence", packet.GetSequence(), "sender", receiver.String(),
"receive", data.Receiver, "amount", ibcAmount, "fee", ibcFee, "router", data.Router)
cacheCtx, writeFn := ctx.CacheContext()
err = route.TransferAfter(cacheCtx, receiver.String(), data.Receiver, ibcAmount, ibcFee)
routerEvent := sdk.NewEvent(types.EventTypeReceiveRoute,
sdk.NewAttribute(types.AttributeKeyRoute, data.Router),
sdk.NewAttribute(types.AttributeKeyRouteSuccess, fmt.Sprintf("%t", err == nil)),
)
switch err {
case nil:
writeFn()
ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
default:
ctx.Logger().Error("IBCTransfer", "transfer after route err!!!sourceChannel", packet.GetSourceChannel(), "destChannel", packet.GetDestChannel(), "sequence", packet.GetSequence(), "err", err)
routerEvent = routerEvent.AppendAttributes(sdk.NewAttribute(types.AttributeKeyRouteError, err.Error()))
}
ctx.EventManager().EmitEvent(routerEvent)

return nil
}
return nil
}

Expand All @@ -260,10 +241,13 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData, ack channeltypes.Acknowledgement) error {
switch ack.Response.(type) {
case *channeltypes.Acknowledgement_Error:
if err := k.Keeper.OnAcknowledgementPacket(ctx, packet, data.ToIBCPacketData(), ack); err != nil {
_, amount, fee, err := parseReceiveAndAmountByPacket(data)
if err != nil {
return err
}
return k.refundPacketTokenHook(ctx, packet, data)
ibcPacketData := data.ToIBCPacketData()
ibcPacketData.Amount = amount.Add(fee).String()
return k.Keeper.OnAcknowledgementPacket(ctx, packet, ibcPacketData, ack)
default:
// the acknowledgement succeeded on the receiving chain so nothing
// needs to be executed and no error needs to be returned
Expand All @@ -274,46 +258,11 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac
// OnTimeoutPacket refunds the sender since the original packet sent was
// never received and has been timed out.
func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error {
if err := k.Keeper.OnTimeoutPacket(ctx, packet, data.ToIBCPacketData()); err != nil {
return err
}
return k.refundPacketTokenHook(ctx, packet, data)
}

// refundPacketToken will unescrow and send back the tokens back to sender
// if the sending chain was the source chain. Otherwise, the sent tokens
// were burnt in the original send so new tokens are minted and sent to
// the sending address.
func (k Keeper) refundPacketTokenHook(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error {
// parse the denomination from the full denom path
trace := transfertypes.ParseDenomTrace(data.Denom)

amount, ok := sdk.NewIntFromString(data.Amount)
if !ok {
return sdkerrors.Wrapf(transfertypes.ErrInvalidAmount, "unable to parse transfer amount (%s) into sdk.Int", data.Amount)
}
// If the IBC router is not empty, the feeAmount refund is added.
if data.Router != "" {
feeAmount, ok := sdk.NewIntFromString(data.Fee)
if !ok {
return sdkerrors.Wrapf(transfertypes.ErrInvalidAmount, "unable to parse transfer fee (%s) into sdk.Int", data.Amount)
}
amount = amount.Add(feeAmount)
}
token := sdk.NewCoin(trace.IBCDenom(), amount)

// decode the sender address
sender, err := sdk.AccAddressFromBech32(data.Sender)
_, amount, fee, err := parseReceiveAndAmountByPacket(data)
if err != nil {
return err
}

if k.RefundHook != nil {
ctx.Logger().Info("ibc refund hook", "sourcePort", packet.SourcePort, "sourceChannel",
packet.SourceChannel, "sequence", fmt.Sprintf("%d", packet.Sequence), "sender", sender.String(), "token", token.String())
if err = k.RefundHook.RefundAfter(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence, sender, data.Receiver, token); err != nil {
ctx.Logger().Error("refundPacketToken", "refund hook err!!!sourceChannel", packet.GetSourceChannel(), "destChannel", packet.GetDestChannel(), "sequence", packet.GetSequence(), "err", err)
}
}
return nil
ibcPacketData := data.ToIBCPacketData()
ibcPacketData.Amount = amount.Add(fee).String()
return k.Keeper.OnTimeoutPacket(ctx, packet, ibcPacketData)
}
133 changes: 133 additions & 0 deletions x/ibc/applications/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package keeper_test

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/ibc-go/v3/modules/apps/transfer"
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
ibcgotesting "github.com/cosmos/ibc-go/v3/testing"
pxtypes "github.com/pundix/pundix/types"
pundixtransfer "github.com/pundix/pundix/x/ibc/applications/transfer"
pxtransfertypes "github.com/pundix/pundix/x/ibc/applications/transfer/types"
"github.com/tendermint/tendermint/libs/rand"
)

func (suite *KeeperTestSuite) TestOnRecvPacket() {
baseDenom := "stake"
senderAddr := sdk.AccAddress([]byte{0x1}).String()
receiveAddr := sdk.AccAddress([]byte{0x2}).String()
ibcDenomTrace := transfertypes.DenomTrace{
Path: "transfer/channel-0",
BaseDenom: pxtypes.MintDenom(),
}
ibcAmount := sdk.NewInt(100)
testCases := []struct {
name string
malleate func(pxIbcTransferMsg *channeltypes.Packet)
expPass bool
errorStr string
checkBalance bool
expCoins sdk.Coins
}{
{
"pass - normal - ibc transfer packet",
func(packet *channeltypes.Packet) {
},
true,
"",
true,
sdk.NewCoins(sdk.NewCoin(ibcDenomTrace.IBCDenom(), ibcAmount)),
},
{
"pass - normal - px ibc transfer packet",
func(packet *channeltypes.Packet) {
packetData := pxtransfertypes.FungibleTokenPacketData{}
pxtransfertypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
packetData.Router = ""
packetData.Fee = sdk.ZeroInt().String()
packet.Data = packetData.GetBytes()
},
true,
"",
true,
sdk.NewCoins(sdk.NewCoin(ibcDenomTrace.IBCDenom(), ibcAmount)),
},
{
"pass - normal - ibc purse token - router is empty",
func(packet *channeltypes.Packet) {
packetData := pxtransfertypes.FungibleTokenPacketData{}
pxtransfertypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
packetData.Denom = ibcDenomTrace.IBCDenom()
packetData.Router = ""
packetData.Fee = sdk.ZeroInt().String()
packet.Data = packetData.GetBytes()
},
true,
"",
true,
sdk.NewCoins(sdk.NewCoin(ibcDenomTrace.IBCDenom(), ibcAmount)),
},
{
"error - not support router",
func(packet *channeltypes.Packet) {
packetData := pxtransfertypes.FungibleTokenPacketData{}
pxtransfertypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
packetData.Denom = ibcDenomTrace.IBCDenom()
packetData.Router = rand.Str(4)
packet.Data = packetData.GetBytes()
},
false,
"not support router",
true,
sdk.NewCoins(sdk.NewCoin(ibcDenomTrace.IBCDenom(), ibcAmount)),
},
{
"error - not support fee",
func(packet *channeltypes.Packet) {
packetData := pxtransfertypes.FungibleTokenPacketData{}
pxtransfertypes.ModuleCdc.MustUnmarshalJSON(packet.GetData(), &packetData)
packetData.Denom = ibcDenomTrace.IBCDenom()
packetData.Router = rand.Str(4)
packetData.Fee = sdk.NewInt(rand.Int64()).String()
packet.Data = packetData.GetBytes()
},
false,
"not support router",
true,
sdk.NewCoins(sdk.NewCoin(ibcDenomTrace.IBCDenom(), ibcAmount)),
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
app := suite.GetApp(suite.chainPx.App)
transferIBCModule := transfer.NewIBCModule(app.TransferKeeper)
pundixIBCMiddleware := pundixtransfer.NewIBCModule(app.PundixTransferKeeper, transferIBCModule)
packetData := transfertypes.NewFungibleTokenPacketData(baseDenom, ibcAmount.String(), senderAddr, receiveAddr)
// only use timeout height
packet := channeltypes.NewPacket(packetData.GetBytes(), 1, ibcgotesting.TransferPort, "channel-0", ibcgotesting.TransferPort, "channel-0", clienttypes.Height{
RevisionNumber: 100,
RevisionHeight: 100000,
}, 0)
tc.malleate(&packet)

ack := pundixIBCMiddleware.OnRecvPacket(suite.chainPx.GetContext(), packet, nil)
suite.Require().NotNil(ack)
if tc.expPass {
suite.Require().True(ack.Success())
} else {
suite.Require().False(ack.Success())
_, ok := ack.(channeltypes.Acknowledgement)
suite.Require().True(ok)
// suite.Require().Equal(tc.errorStr, acknowledgement.GetError())
}
})
}
}

func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() {
}

func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
}
6 changes: 5 additions & 1 deletion x/ibc/applications/transfer/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

var ErrFeeDenomNotMatchTokenDenom = sdkerrors.Register(ModuleName, 100, "invalid fee denom, must match token denom")
var (
ErrFeeDenomNotMatchTokenDenom = sdkerrors.Register(ModuleName, 100, "invalid fee denom, must match token denom")
ErrNotSupportRouter = sdkerrors.Register(ModuleName, 101, "not support router")
ErrNotSupportFee = sdkerrors.Register(ModuleName, 102, "not support fee")
)

0 comments on commit 6a3c531

Please sign in to comment.