From 6a3c5319fd375b63a817b696f4053c0e26f6c306 Mon Sep 17 00:00:00 2001 From: fx0x55 Date: Fri, 9 Dec 2022 16:06:15 +0800 Subject: [PATCH] fix: disable ibc OnRecvPacket packetData router and fee --- app/keepers/keepers.go | 7 +- x/ibc/applications/transfer/keeper/keeper.go | 20 --- x/ibc/applications/transfer/keeper/relay.go | 85 +++-------- .../transfer/keeper/relay_test.go | 133 ++++++++++++++++++ x/ibc/applications/transfer/types/errors.go | 6 +- 5 files changed, 157 insertions(+), 94 deletions(-) create mode 100644 x/ibc/applications/transfer/keeper/relay_test.go diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 10c7482..a713400 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -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" @@ -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) diff --git a/x/ibc/applications/transfer/keeper/keeper.go b/x/ibc/applications/transfer/keeper/keeper.go index 36c6dd2..c4b0257 100644 --- a/x/ibc/applications/transfer/keeper/keeper.go +++ b/x/ibc/applications/transfer/keeper/keeper.go @@ -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 @@ -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) diff --git a/x/ibc/applications/transfer/keeper/relay.go b/x/ibc/applications/transfer/keeper/relay.go index 8eef7c1..0c8f6bb 100644 --- a/x/ibc/applications/transfer/keeper/relay.go +++ b/x/ibc/applications/transfer/keeper/relay.go @@ -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 @@ -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 } @@ -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 @@ -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) } diff --git a/x/ibc/applications/transfer/keeper/relay_test.go b/x/ibc/applications/transfer/keeper/relay_test.go new file mode 100644 index 0000000..4d1c6c0 --- /dev/null +++ b/x/ibc/applications/transfer/keeper/relay_test.go @@ -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() { +} diff --git a/x/ibc/applications/transfer/types/errors.go b/x/ibc/applications/transfer/types/errors.go index 798208e..d9d2465 100644 --- a/x/ibc/applications/transfer/types/errors.go +++ b/x/ibc/applications/transfer/types/errors.go @@ -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") +)