From d04c1fdc04b21babcb695aa65c8464b4a8aea9f3 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 3 Feb 2023 13:51:45 +0100 Subject: [PATCH 1/3] fix: correct ICS20 middleware handlers --- app/app.go | 2 +- x/uibc/quota/ibc_module.go | 154 ++++++++++--------------------------- 2 files changed, 42 insertions(+), 114 deletions(-) diff --git a/app/app.go b/app/app.go index fef82c9e21..aa67ebe562 100644 --- a/app/app.go +++ b/app/app.go @@ -554,7 +554,7 @@ func New( ) if Experimental { - transferStack = uibcquota.NewIBCMiddleware(transferStack, app.UIbcQuotaKeeper) + transferStack = uibcquota.NewIBCMiddleware(transferStack, app.UIbcQuotaKeeper, appCodec) } // Create IBC Router diff --git a/x/uibc/quota/ibc_module.go b/x/uibc/quota/ibc_module.go index b5cbe64917..e9d2805204 100644 --- a/x/uibc/quota/ibc_module.go +++ b/x/uibc/quota/ibc_module.go @@ -4,150 +4,66 @@ import ( "encoding/json" sdkmath "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" transfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v5/modules/core/05-port/types" - "github.com/cosmos/ibc-go/v5/modules/core/exported" "github.com/umee-network/umee/v4/x/uibc" "github.com/umee-network/umee/v4/x/uibc/quota/keeper" ) type IBCMiddleware struct { - app porttypes.IBCModule + porttypes.IBCModule keeper keeper.Keeper + cdc codec.JSONCodec } // NewIBCMiddleware creates a new IBCMiddlware given the keeper and underlying application -func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { +func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper, cdc codec.JSONCodec) IBCMiddleware { return IBCMiddleware{ - app: app, - keeper: k, + IBCModule: app, + keeper: k, + cdc: cdc, } } -// OnChanOpenInit implements types.Middleware -func (im IBCMiddleware) OnChanOpenInit(ctx sdk.Context, order channeltypes.Order, connectionHops []string, - portID string, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, - version string, -) (string, error) { - return im.app.OnChanOpenInit( - ctx, - order, - connectionHops, - portID, - channelID, - channelCap, - counterparty, - version, - ) -} - -// OnChanOpenTry implements types.Middleware -func (im IBCMiddleware) OnChanOpenTry( - ctx sdk.Context, - order channeltypes.Order, - connectionHops []string, - portID string, channelID string, - channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, - counterpartyVersion string) (version string, err error) { - return im.app.OnChanOpenTry( - ctx, order, connectionHops, portID, channelID, channelCap, - counterparty, counterpartyVersion, - ) -} - -// OnChanOpenAck implements types.Middleware -func (im IBCMiddleware) OnChanOpenAck(ctx sdk.Context, portID string, channelID string, counterpartyChannelID string, - counterpartyVersion string, -) error { - return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) -} - -// OnChanOpenConfirm implements types.Middleware -func (im IBCMiddleware) OnChanOpenConfirm(ctx sdk.Context, portID string, channelID string) error { - return im.app.OnChanOpenConfirm(ctx, portID, channelID) -} - -// OnChanCloseInit implements types.Middleware -func (im IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID string, channelID string) error { - return im.app.OnChanCloseInit(ctx, portID, channelID) -} - -// OnChanCloseConfirm implements types.Middleware -func (im IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID string, channelID string) error { - return im.app.OnChanCloseConfirm(ctx, portID, channelID) -} - -// OnRecvPacket implements types.Middleware -func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, -) exported.Acknowledgement { - // if this returns an Acknowledgement that isn't successful, all state changes are discarded - return im.app.OnRecvPacket(ctx, packet, relayer) -} - // OnAcknowledgementPacket implements types.Middleware func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, ) error { - // TODO: recheck the ibc acknowledgement error handling var ack channeltypes.Acknowledgement - if err := json.Unmarshal(acknowledgement, &ack); err != nil { - return sdkerrors.ErrUnknownRequest.Wrapf("cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) + if err := im.cdc.UnmarshalJSON(acknowledgement, &ack); err != nil { + return sdkerrors.Wrap(err, "cannot unmarshal ICS-20 transfer packet acknowledgement") } - - if isAckError(acknowledgement) { - err := im.RevertSentPacket(ctx, packet) // If there is an error here we should still handle the timeout - if err != nil { - _ = ctx.EventManager().EmitTypedEvent(&uibc.EventBadRevert{ - Module: uibc.ModuleName, - FailureType: "acknowledgment", - Packet: string(packet.GetData()), - }) - } + switch ack.Response.(type) { + case *channeltypes.Acknowledgement_Error: + err := im.RevertQuotaUpdate(ctx, packet.Data) + emitOnRevertQuota(ctx, "acknowledgement", packet.Data, err) } - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) -} - -// IsAckError checks an IBC acknowledgement to see if it's an error. -// This is a replacement for ack.Success() which is currently not working on some circumstances -func isAckError(acknowledgement []byte) bool { - var ackErr channeltypes.Acknowledgement_Error - if err := json.Unmarshal(acknowledgement, &ackErr); err == nil && len(ackErr.Error) > 0 { - return true - } - return false + return im.IBCModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) } // OnTimeoutPacket implements types.Middleware func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { - err := im.RevertSentPacket(ctx, packet) // If there is an error here we should still handle the timeout - if err != nil { - if err = ctx.EventManager().EmitTypedEvent(&uibc.EventBadRevert{ - Module: uibc.ModuleName, - FailureType: "timeout", - Packet: string(packet.GetData()), - }); err != nil { - return err - } - } - return im.app.OnTimeoutPacket(ctx, packet, relayer) + err := im.RevertQuotaUpdate(ctx, packet.Data) + emitOnRevertQuota(ctx, "timeout", packet.Data, err) + + return im.IBCModule.OnTimeoutPacket(ctx, packet, relayer) } -// RevertSentPacket Notifies the contract that a sent packet wasn't properly received -func (im IBCMiddleware) RevertSentPacket( +// RevertQuotaUpdate Notifies the contract that a sent packet wasn't properly received +func (im IBCMiddleware) RevertQuotaUpdate( ctx sdk.Context, - packet channeltypes.Packet, + packetData []byte, ) error { var data transfertypes.FungibleTokenPacketData - if err := json.Unmarshal(packet.GetData(), &data); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, - "cannot unmarshal ICS-20 transfer packet data: %s", err.Error(), - ) + if err := im.cdc.UnmarshalJSON(packetData, &data); err != nil { + return sdkerrors.Wrap(err, + "cannot unmarshal ICS-20 transfer packet data") } amount, ok := sdkmath.NewIntFromString(data.Amount) @@ -155,11 +71,7 @@ func (im IBCMiddleware) RevertSentPacket( return sdkerrors.ErrInvalidRequest.Wrapf("invalid transfer amount %s", data.Amount) } - return im.keeper.UndoUpdateQuota( - ctx, - data.Denom, - amount, - ) + return im.keeper.UndoUpdateQuota(ctx, data.Denom, amount) } func ValidateReceiverAddress(packet channeltypes.Packet) error { @@ -174,3 +86,19 @@ func ValidateReceiverAddress(packet channeltypes.Packet) error { } return nil } + +// emitOnRevertQuota emits events related to quota update revert. +// packetData is ICS 20 packet data bytes. +func emitOnRevertQuota(ctx sdk.Context, failureType string, packetData []byte, err error) { + if err == nil { + return + } + logger := ctx.Logger() + logger.Error("revert quota update error", "err", err) + err = ctx.EventManager().EmitTypedEvent(&uibc.EventBadRevert{ + Module: uibc.ModuleName, + FailureType: failureType, + Packet: string(packetData), + }) + logger.Error("emit evenet error", "err", err) +} From fc03174cfaf38bf3d5fd30445ade5b4fa9ac2cbc Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 3 Feb 2023 14:24:41 +0100 Subject: [PATCH 2/3] Update x/uibc/quota/ibc_module.go Co-authored-by: Sai Kumar <17549398+gsk967@users.noreply.github.com> --- x/uibc/quota/ibc_module.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/uibc/quota/ibc_module.go b/x/uibc/quota/ibc_module.go index e9d2805204..ce1cfe9450 100644 --- a/x/uibc/quota/ibc_module.go +++ b/x/uibc/quota/ibc_module.go @@ -95,10 +95,11 @@ func emitOnRevertQuota(ctx sdk.Context, failureType string, packetData []byte, e } logger := ctx.Logger() logger.Error("revert quota update error", "err", err) - err = ctx.EventManager().EmitTypedEvent(&uibc.EventBadRevert{ + if err = ctx.EventManager().EmitTypedEvent(&uibc.EventBadRevert{ Module: uibc.ModuleName, FailureType: failureType, Packet: string(packetData), - }) - logger.Error("emit evenet error", "err", err) + }); err != nil { + logger.Error("emit event error", "err", err) + } } From 8347c6ee4645b96bb4f34338cd9883738f01bf57 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 3 Feb 2023 14:59:46 +0100 Subject: [PATCH 3/3] lint --- x/uibc/quota/ibc_module.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/uibc/quota/ibc_module.go b/x/uibc/quota/ibc_module.go index ce1cfe9450..a7c5b565fd 100644 --- a/x/uibc/quota/ibc_module.go +++ b/x/uibc/quota/ibc_module.go @@ -38,8 +38,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet channelt if err := im.cdc.UnmarshalJSON(acknowledgement, &ack); err != nil { return sdkerrors.Wrap(err, "cannot unmarshal ICS-20 transfer packet acknowledgement") } - switch ack.Response.(type) { - case *channeltypes.Acknowledgement_Error: + if _, ok := ack.Response.(*channeltypes.Acknowledgement_Error); ok { err := im.RevertQuotaUpdate(ctx, packet.Data) emitOnRevertQuota(ctx, "acknowledgement", packet.Data, err) }