From b527124147dc7f21212a64b48aa7ec43953a30b1 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 17:47:57 -0700 Subject: [PATCH 1/7] chore: refactore ibc memo handling --- x/uibc/gmp/gmp_middleware.go | 46 +++++------------ x/uibc/gmp/types.go | 8 --- x/uibc/uics20/ibc_module.go | 45 +++++++++++----- x/uibc/uics20/memo_handler.go | 83 +++++++++++++++++++----------- x/uibc/uics20/memo_handler_test.go | 5 +- 5 files changed, 102 insertions(+), 85 deletions(-) diff --git a/x/uibc/gmp/gmp_middleware.go b/x/uibc/gmp/gmp_middleware.go index b02fd6e899..1a0daa28c0 100644 --- a/x/uibc/gmp/gmp_middleware.go +++ b/x/uibc/gmp/gmp_middleware.go @@ -2,87 +2,69 @@ package gmp import ( "encoding/json" - "fmt" - "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" - ics20types "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" - channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" - - "github.com/umee-network/umee/v6/x/uibc" ) type Handler struct { } -var _ GeneralMessageHandler = Handler{} - func NewHandler() *Handler { return &Handler{} } -func (h Handler) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data ics20types.FungibleTokenPacketData, +func (h Handler) OnRecvPacket(ctx sdk.Context, coinReceived sdk.Coin, memo string, receiver sdk.AccAddress, ) error { logger := ctx.Logger().With("handler", "gmp_handler") var msg Message var err error - if err = json.Unmarshal([]byte(data.GetMemo()), &msg); err != nil { - logger.With(err).Error("cannot unmarshal memo") + if err = json.Unmarshal([]byte(memo), &msg); err != nil { + logger.Error("cannot unmarshal memo", "err", err) return err } switch msg.Type { case TypeGeneralMessage: - err := h.HandleGeneralMessage(ctx, msg.SourceAddress, msg.SourceAddress, data.Receiver, msg.Payload) + err := h.HandleGeneralMessage(ctx, msg.SourceAddress, msg.SourceAddress, receiver, msg.Payload) if err != nil { logger.Error("err at HandleGeneralMessage", err) } case TypeGeneralMessageWithToken: - // parse the transfer amount - amt, ok := sdk.NewIntFromString(data.Amount) - if !ok { - return errors.Wrapf( - ics20types.ErrInvalidAmount, - "unable to parse transfer amount (%s) into sdk.Int", - data.Amount, - ) - } - denom := uibc.ExtractDenomFromPacketOnRecv(packet, data.Denom) - err := h.HandleGeneralMessageWithToken(ctx, msg.SourceAddress, msg.SourceAddress, data.Receiver, - msg.Payload, sdk.NewCoin(denom, amt)) - + err := h.HandleGeneralMessageWithToken( + ctx, msg.SourceAddress, msg.SourceAddress, receiver, msg.Payload, coinReceived) if err != nil { logger.Error("err at HandleGeneralMessageWithToken", err) } default: - logger.With(fmt.Errorf("unrecognized message type: %d", msg.Type)).Error("unrecognized gmp message") + logger.Error("unrecognized gmp message type: %d", msg.Type) } return err } -func (h Handler) HandleGeneralMessage(ctx sdk.Context, srcChain, srcAddress string, destAddress string, +func (h Handler) HandleGeneralMessage(ctx sdk.Context, srcChain, srcAddress string, receiver sdk.AccAddress, payload []byte) error { ctx.Logger().Info("HandleGeneralMessage called", "srcChain", srcChain, "srcAddress", srcAddress, - "destAddress", destAddress, + "receiver", receiver, "payload", payload, "handler", "gmp-handler", ) return nil } -func (h Handler) HandleGeneralMessageWithToken(ctx sdk.Context, srcChain, srcAddress string, destAddress string, - payload []byte, coin sdk.Coin) error { +func (h Handler) HandleGeneralMessageWithToken(ctx sdk.Context, srcChain, srcAddress string, + receiver sdk.AccAddress, payload []byte, coin sdk.Coin) error { + ctx.Logger().Info("HandleGeneralMessageWithToken called", "srcChain", srcChain, "srcAddress", srcAddress, - "destAddress", destAddress, + "receiver", receiver, "payload", payload, "coin", coin, - "handler", "gmp-handler", + "handler", "gmp-token-handler", ) return nil } diff --git a/x/uibc/gmp/types.go b/x/uibc/gmp/types.go index 0b87dc82fc..358a3a81e1 100644 --- a/x/uibc/gmp/types.go +++ b/x/uibc/gmp/types.go @@ -1,13 +1,5 @@ package gmp -import sdk "github.com/cosmos/cosmos-sdk/types" - -type GeneralMessageHandler interface { - HandleGeneralMessage(ctx sdk.Context, srcChain, srcAddress string, destAddress string, payload []byte) error - HandleGeneralMessageWithToken(ctx sdk.Context, srcChain, srcAddress string, destAddress string, - payload []byte, coin sdk.Coin) error -} - const ( DefaultGMPAddress = "axelar1dv4u5k73pzqrxlzujxg3qp8kvc3pje7jtdvu72npnt5zhq05ejcsn5qme5" ) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index cf4a305d96..843dbb8b48 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -67,32 +67,51 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, // smaller than the amount originally sent (various fees). We need to be sure that there is // no other middleware that can change packet data or amounts. - mh := MemoHandler{im.cdc, im.leverage} - msgs, overwriteReceiver, events, err := mh.onRecvPacketPre(&ctx, packet, ftData) - if err != nil { + mh := MemoHandler{cdc: im.cdc, leverage: im.leverage} + fallbackReceiver, events, err := mh.onRecvPacketPrepare(&ctx, packet, ftData) + if err != nil && err != memoValidationErr { return channeltypes.NewErrorAcknowledgement(err) } - if overwriteReceiver != nil { - ftData.Receiver = overwriteReceiver.String() - msgs = nil // we don't want to execute hooks when we set fallback address. - events = append(events, "overwrite receiver to fallback_addr="+ftData.Receiver) - if packet.Data, err = json.Marshal(ftData); err != nil { - return channeltypes.NewErrorAcknowledgement(err) + var transferCtx = ctx + var ctxFlush func() + if fallbackReceiver != nil { + if err == memoValidationErr { + ftData.Receiver = fallbackReceiver.String() + events = append(events, "overwrite receiver to fallback_addr="+ftData.Receiver) + if packet.Data, err = json.Marshal(ftData); err != nil { + return channeltypes.NewErrorAcknowledgement(err) + } + } else { + // create a new cache context: we have a fallback receiver and memo is valid. + // we will discard it when the execution fails to move the funds to the fallbackAddress. + transferCtx, ctxFlush = ctx.CacheContext() } } + execCtx, execCtxFlush := transferCtx.CacheContext() // call transfer module app - ack := im.IBCModule.OnRecvPacket(ctx, packet, relayer) + ack := im.IBCModule.OnRecvPacket(transferCtx, packet, relayer) if !ack.Success() { - return ack + goto end } - if err := mh.dispatchMemoMsgs(&ctx, msgs); err != nil { + if err = mh.execute(&execCtx); err != nil { events = append(events, "can't handle ICS20 memo err = "+err.Error()) + // if we created a new cache context, then we can discard it, and repeate the transfer to + // the fallback address + if ctxFlush != nil { + ctxFlush = nil // discard the context + ack = im.IBCModule.OnRecvPacket(ctx, packet, relayer) + } + } else { + execCtxFlush() } +end: + if ctxFlush != nil { + ctxFlush() + } im.emitEvents(ctx.EventManager(), recvPacketLogger(&ctx), "ics20-memo-hook", events) - return ack } diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 1daee291cd..703e57839b 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -3,6 +3,7 @@ package uics20 import ( "errors" "fmt" + "strings" sdkerrors "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/codec" @@ -12,76 +13,96 @@ import ( ltypes "github.com/umee-network/umee/v6/x/leverage/types" "github.com/umee-network/umee/v6/x/uibc" + "github.com/umee-network/umee/v6/x/uibc/gmp" ) +var memoValidationErr = errors.New("ics20 memo validation error") + type MemoHandler struct { cdc codec.JSONCodec leverage ltypes.MsgServer + + isGMP bool + msgs []sdk.Msg + memo string + received sdk.Coin + + receiver sdk.AccAddress + fallbackReceiver sdk.AccAddress } +// onRecvPacketPre parses transfer Memo field and prepares the MemoHandler. // See ICS20Module.OnRecvPacket for the flow -func (mh MemoHandler) onRecvPacketPre( +func (mh *MemoHandler) onRecvPacketPrepare( ctx *sdk.Context, packet ibcexported.PacketI, ftData ics20types.FungibleTokenPacketData, -) ([]sdk.Msg, sdk.AccAddress, []string, error) { +) (sdk.AccAddress, []string, error) { var events []string + mh.memo = ftData.Memo + amount, ok := sdk.NewIntFromString(ftData.Amount) + if !ok { // must not happen + return nil, nil, fmt.Errorf("can't parse transfer amount: %s", ftData.Amount) + } + ibcDenom := uibc.ExtractDenomFromPacketOnRecv(packet, ftData.Denom) + mh.received = sdk.NewCoin(ibcDenom, amount) + + if strings.EqualFold(ftData.Sender, gmp.DefaultGMPAddress) { + events = append(events, "axelar GMP transaction") + mh.isGMP = true + return nil, events, nil + } + memo, err := deserializeMemo(mh.cdc, []byte(ftData.Memo)) if err != nil { recvPacketLogger(ctx).Debug("Not recognized ICS20 memo, ignoring hook execution", "err", err) - return nil, nil, nil, nil + return nil, nil, nil } - var msgs []sdk.Msg var fallbackReceiver sdk.AccAddress if memo.FallbackAddr != "" { if fallbackReceiver, err = sdk.AccAddressFromBech32(memo.FallbackAddr); err != nil { - return nil, nil, nil, + return nil, nil, sdkerrors.Wrap(err, "ICS20 memo fallback_addr defined, but not formatted correctly") } } - msgs, err = memo.GetMsgs() + mh.msgs, err = memo.GetMsgs() if err != nil { e := "ICS20 memo recognized, but can't unpack memo.messages: " + err.Error() events = append(events, e) - return nil, fallbackReceiver, events, nil + return fallbackReceiver, events, nil } - receiver, err := sdk.AccAddressFromBech32(ftData.Receiver) + mh.receiver, err = sdk.AccAddressFromBech32(ftData.Receiver) if err != nil { // must not happen - return nil, nil, nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") + return nil, nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") } - amount, ok := sdk.NewIntFromString(ftData.Amount) - if !ok { // must not happen - return nil, nil, nil, fmt.Errorf("can't parse transfer amount: %s [%w]", ftData.Amount, err) - } - ibcDenom := uibc.ExtractDenomFromPacketOnRecv(packet, ftData.Denom) - sentCoin := sdk.NewCoin(ibcDenom, amount) - if err := mh.validateMemoMsg(receiver, sentCoin, msgs); err != nil { + if err := mh.validateMemoMsg(); err != nil { events = append(events, "memo.messages are not valid, err: "+err.Error()) - return nil, fallbackReceiver, events, nil + return fallbackReceiver, events, memoValidationErr } - return msgs, fallbackReceiver, events, nil + return fallbackReceiver, events, nil } // runs messages encoded in the ICS20 memo. // NOTE: we fork the store and only commit if all messages pass. Otherwise the fork store // is discarded. -func (mh MemoHandler) dispatchMemoMsgs(ctx *sdk.Context, msgs []sdk.Msg) error { - if len(msgs) == 0 { +func (mh MemoHandler) execute(ctx *sdk.Context) error { + logger := recvPacketLogger(ctx) + if mh.isGMP { + gh := gmp.NewHandler() + return gh.OnRecvPacket(*ctx, mh.received, mh.memo, mh.receiver) + } + + if len(mh.msgs) == 0 { return nil // quick return - we have nothing to handle } - // Caching context so that we don't update the store in case of failure. - cacheCtx, flush := ctx.CacheContext() - logger := recvPacketLogger(ctx) - for _, m := range msgs { - if err := mh.handleMemoMsg(&cacheCtx, m); err != nil { - // ignore changes in cacheCtx and return + for _, m := range mh.msgs { + if err := mh.handleMemoMsg(ctx, m); err != nil { return sdkerrors.Wrapf(err, "error dispatching msg: %v", m) } logger.Debug("dispatching", "msg", m) } - flush() return nil } @@ -98,8 +119,8 @@ var ( // - [MsgLiquidate] // Signer of each message (account under charged with coins), must be the receiver of the ICS20 // transfer. -func (mh MemoHandler) validateMemoMsg(_receiver sdk.AccAddress, sent sdk.Coin, msgs []sdk.Msg) error { - msgLen := len(msgs) +func (mh MemoHandler) validateMemoMsg() error { + msgLen := len(mh.msgs) if msgLen == 0 { return nil } @@ -113,7 +134,7 @@ func (mh MemoHandler) validateMemoMsg(_receiver sdk.AccAddress, sent sdk.Coin, m asset sdk.Coin // collateral sdk.Coin ) - switch msg := msgs[0].(type) { + switch msg := mh.msgs[0].(type) { case *ltypes.MsgSupplyCollateral: asset = msg.Asset // collateral = asset @@ -125,7 +146,7 @@ func (mh MemoHandler) validateMemoMsg(_receiver sdk.AccAddress, sent sdk.Coin, m return errMsg0Type } - return assertSubCoins(sent, asset) + return assertSubCoins(mh.received, asset) /** TODO: handlers v2 diff --git a/x/uibc/uics20/memo_handler_test.go b/x/uibc/uics20/memo_handler_test.go index 1c2275960a..6e9cfd148e 100644 --- a/x/uibc/uics20/memo_handler_test.go +++ b/x/uibc/uics20/memo_handler_test.go @@ -93,7 +93,10 @@ func TestValidateMemoMsg(t *testing.T) { } for i, tc := range tcs { - err := mh.validateMemoMsg(receiver, sent, tc.msgs) + mh.receiver = receiver + mh.received = sent + mh.msgs = tc.msgs + err := mh.validateMemoMsg() if tc.errstr != "" { assert.ErrorContains(err, tc.errstr, "test: %d", i) } else { From 0990ff3b80eda09eae0afc2ae755d04341cd332a Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 18:26:28 -0700 Subject: [PATCH 2/7] adjust operated coins --- x/uibc/uics20/memo_handler.go | 19 ++++++++++++------- x/uibc/uics20/memo_handler_test.go | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 703e57839b..98fae5da5e 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -131,22 +131,22 @@ func (mh MemoHandler) validateMemoMsg() error { } var ( - asset sdk.Coin + asset *sdk.Coin // collateral sdk.Coin ) switch msg := mh.msgs[0].(type) { case *ltypes.MsgSupplyCollateral: - asset = msg.Asset + asset = &msg.Asset // collateral = asset case *ltypes.MsgSupply: - asset = msg.Asset + asset = &msg.Asset case *ltypes.MsgLiquidate: - asset = msg.Repayment + asset = &msg.Repayment default: return errMsg0Type } - return assertSubCoins(mh.received, asset) + return adjustOperatedCoin(mh.received, asset) /** TODO: handlers v2 @@ -192,10 +192,15 @@ func (mh MemoHandler) handleMemoMsg(ctx *sdk.Context, msg sdk.Msg) (err error) { return err } -func assertSubCoins(sent, operated sdk.Coin) error { - if sent.Denom != operated.Denom || sent.Amount.LT(operated.Amount) { +// adjustOperatedCoin assures that received and operated are of the same denom. Returns error if +// not. Moreover it updates operated amount if it is bigger than the received amount. +func adjustOperatedCoin(received sdk.Coin, operated *sdk.Coin) error { + if received.Denom != operated.Denom { return errNoSubCoins } + if received.Amount.LT(operated.Amount) { + operated.Amount = received.Amount + } return nil } diff --git a/x/uibc/uics20/memo_handler_test.go b/x/uibc/uics20/memo_handler_test.go index 6e9cfd148e..4ba7fd782e 100644 --- a/x/uibc/uics20/memo_handler_test.go +++ b/x/uibc/uics20/memo_handler_test.go @@ -140,3 +140,23 @@ func TestMsgMarshalling(t *testing.T) { memo2, err = deserializeMemo(cdc, bz) assert.Error(err) } + +func TestAdjustOperatedCoin(t *testing.T) { + received := sdk.NewInt64Coin("atom", 10) + tcs := []struct { + operated sdk.Coin + expectedAmount int64 + err error + }{ + {sdk.NewInt64Coin("other", 1), 1, errNoSubCoins}, + {sdk.NewInt64Coin("atom", 1), 1, nil}, + {sdk.NewInt64Coin("atom", 10), 10, nil}, + {sdk.NewInt64Coin("atom", 12), 10, nil}, + } + + for i, tc := range tcs { + err := adjustOperatedCoin(received, &tc.operated) + assert.ErrorIs(t, err, tc.err, "tc %d", i) + assert.Equal(t, tc.expectedAmount, tc.operated.Amount.Int64()) + } +} From eb455186958e078925778a2fb8cce065c0271c8c Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 18:47:57 -0700 Subject: [PATCH 3/7] check fallback receiver --- x/uibc/uics20/memo_handler.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 98fae5da5e..873b016fdf 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -44,6 +44,10 @@ func (mh *MemoHandler) onRecvPacketPrepare( } ibcDenom := uibc.ExtractDenomFromPacketOnRecv(packet, ftData.Denom) mh.received = sdk.NewCoin(ibcDenom, amount) + mh.receiver, err = sdk.AccAddressFromBech32(ftData.Receiver) + if err != nil { // must not happen + return nil, nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") + } if strings.EqualFold(ftData.Sender, gmp.DefaultGMPAddress) { events = append(events, "axelar GMP transaction") @@ -62,6 +66,9 @@ func (mh *MemoHandler) onRecvPacketPrepare( return nil, nil, sdkerrors.Wrap(err, "ICS20 memo fallback_addr defined, but not formatted correctly") } + if fallbackReceiver.Equals(mh.receiver) { + fallbackReceiver = nil + } } mh.msgs, err = memo.GetMsgs() @@ -71,10 +78,6 @@ func (mh *MemoHandler) onRecvPacketPrepare( return fallbackReceiver, events, nil } - mh.receiver, err = sdk.AccAddressFromBech32(ftData.Receiver) - if err != nil { // must not happen - return nil, nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") - } if err := mh.validateMemoMsg(); err != nil { events = append(events, "memo.messages are not valid, err: "+err.Error()) return fallbackReceiver, events, memoValidationErr From eaa051fad4d30b9bf4fcfeb016eac5b40e0beea0 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 23:28:28 -0700 Subject: [PATCH 4/7] fix compilation --- x/uibc/uics20/memo_handler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 873b016fdf..2538bc058b 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -37,6 +37,7 @@ func (mh *MemoHandler) onRecvPacketPrepare( ctx *sdk.Context, packet ibcexported.PacketI, ftData ics20types.FungibleTokenPacketData, ) (sdk.AccAddress, []string, error) { var events []string + var err error mh.memo = ftData.Memo amount, ok := sdk.NewIntFromString(ftData.Amount) if !ok { // must not happen From c50f6c9248aecab234c9e20784c4cd1e6f273950 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 23:32:46 -0700 Subject: [PATCH 5/7] update tests --- x/uibc/uics20/memo_handler_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/x/uibc/uics20/memo_handler_test.go b/x/uibc/uics20/memo_handler_test.go index 4ba7fd782e..7226ede427 100644 --- a/x/uibc/uics20/memo_handler_test.go +++ b/x/uibc/uics20/memo_handler_test.go @@ -56,11 +56,13 @@ func TestValidateMemoMsg(t *testing.T) { {[]sdk.Msg{goodMsgSupplyColl}, ""}, {[]sdk.Msg{goodMsgLiquidate}, ""}, // in handlers v2 this will be a good message - // messages[0] use more assets than the transfer - {[]sdk.Msg{goodMsgSupply11}, errNoSubCoins}, - {[]sdk.Msg{goodMsgSupplyColl11}, errNoSubCoins}, - {[]sdk.Msg{goodMsgSupplyColl11}, errNoSubCoins}, - {[]sdk.Msg{goodMsgLiquidate11}, errNoSubCoins}, + // messages[0] use more assets than the transfer -> OK + {[]sdk.Msg{goodMsgSupply11}, ""}, + {[]sdk.Msg{goodMsgSupplyColl11}, ""}, + {[]sdk.Msg{goodMsgSupplyColl11}, ""}, + {[]sdk.Msg{goodMsgLiquidate11}, ""}, + + {[]sdk.Msg{ltypes.NewMsgSupply(receiver, coin.New("other", 1))}, errNoSubCoins}, // wrong message types {[]sdk.Msg{goodMsgBorrow}, errMsg0type}, From e44fa5c06954f82cb3b3a911764805f988c42963 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 23:44:12 -0700 Subject: [PATCH 6/7] lint --- x/uibc/uics20/ibc_module.go | 4 ++-- x/uibc/uics20/memo_handler.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index 843dbb8b48..f59c6571bb 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -69,13 +69,13 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, mh := MemoHandler{cdc: im.cdc, leverage: im.leverage} fallbackReceiver, events, err := mh.onRecvPacketPrepare(&ctx, packet, ftData) - if err != nil && err != memoValidationErr { + if err != nil && err != errMemoValidation { return channeltypes.NewErrorAcknowledgement(err) } var transferCtx = ctx var ctxFlush func() if fallbackReceiver != nil { - if err == memoValidationErr { + if err == errMemoValidation { ftData.Receiver = fallbackReceiver.String() events = append(events, "overwrite receiver to fallback_addr="+ftData.Receiver) if packet.Data, err = json.Marshal(ftData); err != nil { diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 2538bc058b..80ac6803d2 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -16,7 +16,7 @@ import ( "github.com/umee-network/umee/v6/x/uibc/gmp" ) -var memoValidationErr = errors.New("ics20 memo validation error") +var errMemoValidation = errors.New("ics20 memo validation error") type MemoHandler struct { cdc codec.JSONCodec @@ -81,7 +81,7 @@ func (mh *MemoHandler) onRecvPacketPrepare( if err := mh.validateMemoMsg(); err != nil { events = append(events, "memo.messages are not valid, err: "+err.Error()) - return fallbackReceiver, events, memoValidationErr + return fallbackReceiver, events, errMemoValidation } return fallbackReceiver, events, nil From e829effbefb0cea5d1473ab8e130ba62914e1622 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 1 Mar 2024 00:48:57 -0700 Subject: [PATCH 7/7] fix --- x/uibc/uics20/ibc_module.go | 13 +++++++++---- x/uibc/uics20/memo_handler.go | 25 ++++++++++++------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index f59c6571bb..b067045927 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -68,15 +68,15 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, // no other middleware that can change packet data or amounts. mh := MemoHandler{cdc: im.cdc, leverage: im.leverage} - fallbackReceiver, events, err := mh.onRecvPacketPrepare(&ctx, packet, ftData) + events, err := mh.onRecvPacketPrepare(&ctx, packet, ftData) if err != nil && err != errMemoValidation { return channeltypes.NewErrorAcknowledgement(err) } var transferCtx = ctx var ctxFlush func() - if fallbackReceiver != nil { + if mh.fallbackReceiver != nil { if err == errMemoValidation { - ftData.Receiver = fallbackReceiver.String() + ftData.Receiver = mh.fallbackReceiver.String() events = append(events, "overwrite receiver to fallback_addr="+ftData.Receiver) if packet.Data, err = json.Marshal(ftData); err != nil { return channeltypes.NewErrorAcknowledgement(err) @@ -99,8 +99,13 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, events = append(events, "can't handle ICS20 memo err = "+err.Error()) // if we created a new cache context, then we can discard it, and repeate the transfer to // the fallback address - if ctxFlush != nil { + if mh.fallbackReceiver != nil { ctxFlush = nil // discard the context + ftData.Receiver = mh.fallbackReceiver.String() + events = append(events, "overwrite receiver to fallback_addr="+ftData.Receiver) + if packet.Data, err = json.Marshal(ftData); err != nil { + return channeltypes.NewErrorAcknowledgement(err) + } ack = im.IBCModule.OnRecvPacket(ctx, packet, relayer) } } else { diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 80ac6803d2..bbde77db6f 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -35,40 +35,39 @@ type MemoHandler struct { // See ICS20Module.OnRecvPacket for the flow func (mh *MemoHandler) onRecvPacketPrepare( ctx *sdk.Context, packet ibcexported.PacketI, ftData ics20types.FungibleTokenPacketData, -) (sdk.AccAddress, []string, error) { +) ([]string, error) { var events []string var err error mh.memo = ftData.Memo amount, ok := sdk.NewIntFromString(ftData.Amount) if !ok { // must not happen - return nil, nil, fmt.Errorf("can't parse transfer amount: %s", ftData.Amount) + return nil, fmt.Errorf("can't parse transfer amount: %s", ftData.Amount) } ibcDenom := uibc.ExtractDenomFromPacketOnRecv(packet, ftData.Denom) mh.received = sdk.NewCoin(ibcDenom, amount) mh.receiver, err = sdk.AccAddressFromBech32(ftData.Receiver) if err != nil { // must not happen - return nil, nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") + return nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") } if strings.EqualFold(ftData.Sender, gmp.DefaultGMPAddress) { events = append(events, "axelar GMP transaction") mh.isGMP = true - return nil, events, nil + return events, nil } memo, err := deserializeMemo(mh.cdc, []byte(ftData.Memo)) if err != nil { recvPacketLogger(ctx).Debug("Not recognized ICS20 memo, ignoring hook execution", "err", err) - return nil, nil, nil + return nil, nil } - var fallbackReceiver sdk.AccAddress if memo.FallbackAddr != "" { - if fallbackReceiver, err = sdk.AccAddressFromBech32(memo.FallbackAddr); err != nil { - return nil, nil, + if mh.fallbackReceiver, err = sdk.AccAddressFromBech32(memo.FallbackAddr); err != nil { + return nil, sdkerrors.Wrap(err, "ICS20 memo fallback_addr defined, but not formatted correctly") } - if fallbackReceiver.Equals(mh.receiver) { - fallbackReceiver = nil + if mh.fallbackReceiver.Equals(mh.receiver) { + mh.fallbackReceiver = nil } } @@ -76,15 +75,15 @@ func (mh *MemoHandler) onRecvPacketPrepare( if err != nil { e := "ICS20 memo recognized, but can't unpack memo.messages: " + err.Error() events = append(events, e) - return fallbackReceiver, events, nil + return events, nil } if err := mh.validateMemoMsg(); err != nil { events = append(events, "memo.messages are not valid, err: "+err.Error()) - return fallbackReceiver, events, errMemoValidation + return events, errMemoValidation } - return fallbackReceiver, events, nil + return events, nil } // runs messages encoded in the ICS20 memo.