From ce58998ea5cd93b9e0029bebf31e29d162ef550a Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 26 Feb 2024 20:11:49 -0700 Subject: [PATCH 1/6] feat(uics20 memo hook): add fallback address --- proto/umee/uibc/v1/uibc.proto | 8 +++ x/uibc/ics20.go | 6 ++ x/uibc/uibc.pb.go | 97 +++++++++++++++++++++++------- x/uibc/uics20/ibc_module.go | 2 +- x/uibc/uics20/memo_handler.go | 20 +++--- x/uibc/uics20/memo_handler_test.go | 14 ++++- 6 files changed, 113 insertions(+), 34 deletions(-) diff --git a/proto/umee/uibc/v1/uibc.proto b/proto/umee/uibc/v1/uibc.proto index 69186c30c6..0a2033ee1e 100644 --- a/proto/umee/uibc/v1/uibc.proto +++ b/proto/umee/uibc/v1/uibc.proto @@ -14,6 +14,14 @@ option (gogoproto.messagename_all) = true; message ICS20Memo { // messages is a list of `sdk.Msg`s that will be executed when handling ICS20 transfer. repeated google.protobuf.Any messages = 1; + // fallback_addr [optional] is an bech23 account address used to overwrite the original ICS20 + // recipient when: + // 1. it is defined + // 2. and memo is can be properly deserialized into this structure + // 3. and `messages` processes failed. + // When memo can't be properly deserialized, then the ICS20 processing will continue to other + // middlewares. + string fallback_addr = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; } // DecCoinSymbol extends the Cosmos SDK DecCoin type and adds symbol name. diff --git a/x/uibc/ics20.go b/x/uibc/ics20.go index 221e8bc7e0..c77c7ce029 100644 --- a/x/uibc/ics20.go +++ b/x/uibc/ics20.go @@ -2,6 +2,7 @@ package uibc import ( "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx" ) @@ -9,3 +10,8 @@ import ( func (m ICS20Memo) UnpackInterfaces(unpacker types.AnyUnpacker) error { return tx.UnpackInterfaces(unpacker, m.Messages) } + +// GetMsgs unpacks messages into []sdk.Msg +func (m ICS20Memo) GetMsgs() ([]sdk.Msg, error) { + return tx.GetMsgs(m.Messages, "memo messages") +} diff --git a/x/uibc/uibc.pb.go b/x/uibc/uibc.pb.go index 520583fc68..15decdcd84 100644 --- a/x/uibc/uibc.pb.go +++ b/x/uibc/uibc.pb.go @@ -30,6 +30,14 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package type ICS20Memo struct { // messages is a list of `sdk.Msg`s that will be executed when handling ICS20 transfer. Messages []*types.Any `protobuf:"bytes,1,rep,name=messages,proto3" json:"messages,omitempty"` + // fallback_addr [optional] is an bech23 account address used to overwrite the original ICS20 + // recipient when: + // 1. it is defined + // 2. and memo is can be properly deserialized into this structure + // 3. and `messages` processes failed. + // When memo can't be properly deserialized, then the ICS20 processing will continue to other + // middlewares. + FallbackAddr string `protobuf:"bytes,2,opt,name=fallback_addr,json=fallbackAddr,proto3" json:"fallback_addr,omitempty"` } func (m *ICS20Memo) Reset() { *m = ICS20Memo{} } @@ -121,28 +129,30 @@ func init() { func init() { proto.RegisterFile("umee/uibc/v1/uibc.proto", fileDescriptor_963b2b690b6cd9dd) } var fileDescriptor_963b2b690b6cd9dd = []byte{ - // 321 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x4c, 0x90, 0xb1, 0x4e, 0x02, 0x41, - 0x10, 0x86, 0x6f, 0x25, 0x12, 0x59, 0xb5, 0xb9, 0x10, 0x3d, 0x28, 0x16, 0x42, 0x61, 0x68, 0xd8, - 0x05, 0x4c, 0xac, 0xb4, 0x10, 0x28, 0xb4, 0xb0, 0x01, 0x2b, 0x1b, 0xc3, 0x1d, 0xe3, 0x49, 0x60, - 0x6f, 0x08, 0x7b, 0x87, 0xde, 0x5b, 0x98, 0xf8, 0x2a, 0x3e, 0xc4, 0x95, 0xc4, 0xca, 0x58, 0x10, - 0xe5, 0x5e, 0xc4, 0xdc, 0xde, 0x6a, 0xac, 0x66, 0xbf, 0x7f, 0x66, 0xf6, 0xcf, 0x3f, 0xf4, 0x38, - 0x92, 0x00, 0x22, 0x9a, 0xba, 0x9e, 0x58, 0x75, 0x74, 0xe5, 0x8b, 0x25, 0x86, 0x68, 0x1f, 0x64, - 0x0d, 0xae, 0x85, 0x55, 0xa7, 0x5a, 0xf1, 0x50, 0x49, 0x54, 0xf7, 0xba, 0x27, 0x72, 0xc8, 0x07, - 0xab, 0x65, 0x1f, 0x7d, 0xcc, 0xf5, 0xec, 0x65, 0xd4, 0x8a, 0x8f, 0xe8, 0xcf, 0x41, 0x68, 0x72, - 0xa3, 0x07, 0x31, 0x0e, 0xe2, 0xbc, 0xd5, 0xb8, 0xa0, 0xa5, 0xeb, 0xfe, 0xa8, 0xdb, 0xbe, 0x01, - 0x89, 0x76, 0x9b, 0xee, 0x49, 0x50, 0x6a, 0xec, 0x83, 0x72, 0x48, 0xbd, 0xd0, 0xdc, 0xef, 0x96, - 0x79, 0xbe, 0xca, 0x7f, 0x57, 0xf9, 0x65, 0x10, 0x0f, 0xff, 0xa6, 0x1a, 0xaf, 0x84, 0x1e, 0x0e, - 0xc0, 0xeb, 0xe3, 0x34, 0x18, 0xc5, 0xd2, 0xc5, 0xb9, 0x5d, 0xa6, 0xbb, 0x13, 0x08, 0x50, 0x3a, - 0xa4, 0x4e, 0x9a, 0xa5, 0x61, 0x0e, 0xf6, 0x2d, 0x2d, 0x8e, 0x25, 0x46, 0x41, 0xe8, 0xec, 0x64, - 0x72, 0xef, 0x3c, 0xd9, 0xd4, 0xac, 0xcf, 0x4d, 0xed, 0xc4, 0x9f, 0x86, 0x8f, 0x91, 0xcb, 0x3d, - 0x94, 0x26, 0x88, 0x29, 0x2d, 0x35, 0x99, 0x89, 0x30, 0x5e, 0x80, 0xe2, 0x03, 0xf0, 0xde, 0xdf, - 0x5a, 0xd4, 0xe4, 0x1c, 0x80, 0x37, 0x34, 0x7f, 0xd9, 0x47, 0xb4, 0xa8, 0xb4, 0xab, 0x53, 0xd0, - 0x66, 0x86, 0x7a, 0x57, 0xc9, 0x37, 0xb3, 0x92, 0x2d, 0x23, 0xeb, 0x2d, 0x23, 0x5f, 0x5b, 0x46, - 0x5e, 0x52, 0x66, 0x25, 0x29, 0x23, 0xeb, 0x94, 0x59, 0x1f, 0x29, 0xb3, 0xee, 0xfe, 0xfb, 0x66, - 0xb7, 0x6d, 0x05, 0x10, 0x3e, 0xe1, 0x72, 0xa6, 0x41, 0xac, 0xce, 0xc4, 0xb3, 0x3e, 0xbf, 0x5b, - 0xd4, 0xb9, 0x4f, 0x7f, 0x02, 0x00, 0x00, 0xff, 0xff, 0x46, 0xbc, 0x69, 0x2f, 0x9a, 0x01, 0x00, - 0x00, + // 364 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x4c, 0x51, 0xbb, 0x6e, 0xe2, 0x40, + 0x14, 0xf5, 0x2c, 0x5a, 0xb4, 0xcc, 0x42, 0x63, 0x59, 0xbb, 0x86, 0x62, 0x40, 0x14, 0x2b, 0x1a, + 0x7b, 0x80, 0x95, 0xb6, 0xda, 0x14, 0x3c, 0x8a, 0xa4, 0x48, 0x63, 0x52, 0xa5, 0x41, 0x7e, 0x0c, + 0x8e, 0x85, 0xc7, 0x83, 0x3c, 0x36, 0x89, 0xa5, 0x7c, 0x44, 0xa4, 0xfc, 0x0a, 0x1f, 0xe1, 0x12, + 0x51, 0x45, 0x29, 0x50, 0x82, 0x7f, 0x24, 0xf2, 0x78, 0x88, 0x52, 0xcd, 0x9c, 0x73, 0xcf, 0x3d, + 0xe7, 0xea, 0x5e, 0xf8, 0x3b, 0xa5, 0x84, 0xe0, 0x34, 0x70, 0x5c, 0xbc, 0x1d, 0x89, 0xd7, 0xdc, + 0xc4, 0x2c, 0x61, 0x6a, 0xb3, 0x2c, 0x98, 0x82, 0xd8, 0x8e, 0x3a, 0x6d, 0x97, 0x71, 0xca, 0xf8, + 0x52, 0xd4, 0x70, 0x05, 0x2a, 0x61, 0x47, 0xf3, 0x99, 0xcf, 0x2a, 0xbe, 0xfc, 0x49, 0xb6, 0xed, + 0x33, 0xe6, 0x87, 0x04, 0x0b, 0xe4, 0xa4, 0x2b, 0x6c, 0x47, 0x59, 0x55, 0xea, 0x3f, 0xc2, 0xc6, + 0xd5, 0x6c, 0x31, 0x1e, 0x5e, 0x13, 0xca, 0xd4, 0x21, 0xfc, 0x41, 0x09, 0xe7, 0xb6, 0x4f, 0xb8, + 0x0e, 0x7a, 0xb5, 0xc1, 0xcf, 0xb1, 0x66, 0x56, 0xad, 0xe6, 0xb9, 0xd5, 0x9c, 0x44, 0x99, 0xf5, + 0xa9, 0x52, 0x2f, 0x60, 0x6b, 0x65, 0x87, 0xa1, 0x63, 0xbb, 0xeb, 0xa5, 0xed, 0x79, 0xb1, 0xfe, + 0xad, 0x07, 0x06, 0x8d, 0xa9, 0x7e, 0xd8, 0x19, 0x9a, 0x1c, 0x6c, 0xe2, 0x79, 0x31, 0xe1, 0x7c, + 0x91, 0xc4, 0x41, 0xe4, 0x5b, 0xcd, 0xb3, 0xbc, 0xa4, 0xfb, 0xcf, 0x00, 0xb6, 0xe6, 0xc4, 0x9d, + 0xb1, 0x20, 0x5a, 0x64, 0xd4, 0x61, 0xa1, 0xaa, 0xc1, 0xef, 0x1e, 0x89, 0x18, 0xd5, 0x41, 0x69, + 0x64, 0x55, 0x40, 0xbd, 0x81, 0x75, 0x9b, 0xb2, 0x34, 0x4a, 0xa4, 0xff, 0xff, 0xfc, 0xd8, 0x55, + 0x5e, 0x8f, 0xdd, 0x3f, 0x7e, 0x90, 0xdc, 0xa5, 0x8e, 0xe9, 0x32, 0x2a, 0xf7, 0x20, 0x1f, 0x83, + 0x7b, 0x6b, 0x9c, 0x64, 0x1b, 0xc2, 0xcd, 0x39, 0x71, 0x0f, 0x3b, 0x03, 0xca, 0x69, 0xe6, 0xc4, + 0xb5, 0xa4, 0x97, 0xfa, 0x0b, 0xd6, 0xb9, 0x48, 0xd5, 0x6b, 0x22, 0x4c, 0xa2, 0xe9, 0x65, 0xfe, + 0x8e, 0x94, 0xfc, 0x84, 0xc0, 0xfe, 0x84, 0xc0, 0xdb, 0x09, 0x81, 0xa7, 0x02, 0x29, 0x79, 0x81, + 0xc0, 0xbe, 0x40, 0xca, 0x4b, 0x81, 0x94, 0xdb, 0xaf, 0xb9, 0xe5, 0x69, 0x8c, 0x88, 0x24, 0xf7, + 0x2c, 0x5e, 0x0b, 0x80, 0xb7, 0xff, 0xf0, 0x83, 0xb8, 0x9e, 0x53, 0x17, 0x6b, 0xfb, 0xfb, 0x11, + 0x00, 0x00, 0xff, 0xff, 0xb5, 0xa8, 0xd6, 0xef, 0xd9, 0x01, 0x00, 0x00, } func (m *ICS20Memo) Marshal() (dAtA []byte, err error) { @@ -165,6 +175,13 @@ func (m *ICS20Memo) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if len(m.FallbackAddr) > 0 { + i -= len(m.FallbackAddr) + copy(dAtA[i:], m.FallbackAddr) + i = encodeVarintUibc(dAtA, i, uint64(len(m.FallbackAddr))) + i-- + dAtA[i] = 0x12 + } if len(m.Messages) > 0 { for iNdEx := len(m.Messages) - 1; iNdEx >= 0; iNdEx-- { { @@ -252,6 +269,10 @@ func (m *ICS20Memo) Size() (n int) { n += 1 + l + sovUibc(uint64(l)) } } + l = len(m.FallbackAddr) + if l > 0 { + n += 1 + l + sovUibc(uint64(l)) + } return n } @@ -343,6 +364,38 @@ func (m *ICS20Memo) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field FallbackAddr", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowUibc + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthUibc + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthUibc + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.FallbackAddr = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipUibc(dAtA[iNdEx:]) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index e369905755..2009759308 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -57,7 +57,7 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, if ftData.Memo != "" { logger := recvPacketLogger(&ctx) mh := MemoHandler{im.cdc, im.leverage} - if err := mh.onRecvPacket(&ctx, ftData); err != nil { + if err := mh.onRecvPacketPost(&ctx, ftData); err != nil { logger.Error("can't handle ICS20 memo", "err", err) } } diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 2f1e797417..a544a03687 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -7,7 +7,6 @@ import ( sdkerrors "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/tx" ics20types "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ltypes "github.com/umee-network/umee/v6/x/leverage/types" @@ -19,8 +18,13 @@ type MemoHandler struct { leverage ltypes.MsgServer } -func (mh MemoHandler) onRecvPacket(ctx *sdk.Context, ftData ics20types.FungibleTokenPacketData) error { - msgs, err := deserializeMemoMsgs(mh.cdc, []byte(ftData.Memo)) +func (mh MemoHandler) onRecvPacketPost(ctx *sdk.Context, ftData ics20types.FungibleTokenPacketData) error { + memo, err := deserializeMemo(mh.cdc, []byte(ftData.Memo)) + if err != nil { + recvPacketLogger(ctx).Debug("Can't deserialize ICS20 memo for hook execution", "err", err) + return nil + } + msgs, err := memo.GetMsgs() if err != nil { recvPacketLogger(ctx).Debug("Can't deserialize ICS20 memo for hook execution", "err", err) return nil @@ -30,7 +34,7 @@ func (mh MemoHandler) onRecvPacket(ctx *sdk.Context, ftData ics20types.FungibleT receiver, err := sdk.AccAddressFromBech32(ftData.Receiver) if err != nil { - return sdkerrors.Wrap(err, "can't parse bech32 address") + return sdkerrors.Wrap(err, "can't parse bech32 address") // should not happen } amount, ok := sdk.NewIntFromString(ftData.Amount) if !ok { @@ -152,10 +156,6 @@ func assertSubCoins(sent, operated sdk.Coin) error { return nil } -func deserializeMemoMsgs(cdc codec.JSONCodec, data []byte) ([]sdk.Msg, error) { - var m uibc.ICS20Memo - if err := cdc.UnmarshalJSON(data, &m); err != nil { - return nil, err - } - return tx.GetMsgs(m.Messages, "memo messages") +func deserializeMemo(cdc codec.JSONCodec, data []byte) (m uibc.ICS20Memo, err error) { + return m, cdc.UnmarshalJSON(data, &m) } diff --git a/x/uibc/uics20/memo_handler_test.go b/x/uibc/uics20/memo_handler_test.go index e62bfa2125..e38cb1e5f6 100644 --- a/x/uibc/uics20/memo_handler_test.go +++ b/x/uibc/uics20/memo_handler_test.go @@ -111,9 +111,21 @@ func TestMsgMarshalling(t *testing.T) { bz, err := cdc.MarshalJSON(&memo) assert.NoError(err) - msgs2, err := deserializeMemoMsgs(cdc, bz) + memo2, err := deserializeMemo(cdc, bz) assert.NoError(err) + assert.Equal(memo2.FallbackAddr, "") + msgs2, err := memo2.GetMsgs() for i := range msgs2 { assert.Equal(msgs[i], msgs2[i], "idx=%d", i) } + + bz = []byte("{}") + memo2, err = deserializeMemo(cdc, bz) + assert.NoError(err) + assert.Equal(memo2, uibc.ICS20Memo{}) + + // we expect to fail deserialization if Any is not properly formatted + bz = []byte(`{"messages": ["any message"]}`) + memo2, err = deserializeMemo(cdc, bz) + assert.Error(err) } From b985f8fc60d00579c2bbf1427aaca01d28b3e818 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 26 Feb 2024 21:25:45 -0700 Subject: [PATCH 2/6] split memo handler in pre and post --- x/uibc/uics20/ibc_module.go | 13 ++++++++++--- x/uibc/uics20/memo_handler.go | 6 +++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index 2009759308..0e61f5de21 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -50,14 +50,21 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, return ackResp } + logger := recvPacketLogger(&ctx) + mh := MemoHandler{im.cdc, im.leverage} + var memo uibc.ICS20Memo + if ftData.Memo != "" { + if err := mh.onRecvPacketPre(&ctx, ftData); err != nil { + logger.Error("can't handle ICS20 memo", "err", err) + } + } + ack := im.IBCModule.OnRecvPacket(ctx, packet, relayer) if !ack.Success() { return ack } if ftData.Memo != "" { - logger := recvPacketLogger(&ctx) - mh := MemoHandler{im.cdc, im.leverage} - if err := mh.onRecvPacketPost(&ctx, ftData); err != nil { + if err := mh.onRecvPacketPre(&ctx, ftData); err != nil { logger.Error("can't handle ICS20 memo", "err", err) } } diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index a544a03687..961a1074e4 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -18,7 +18,7 @@ type MemoHandler struct { leverage ltypes.MsgServer } -func (mh MemoHandler) onRecvPacketPost(ctx *sdk.Context, ftData ics20types.FungibleTokenPacketData) error { +func (mh MemoHandler) onRecvPacketPre(ctx *sdk.Context, ftData ics20types.FungibleTokenPacketData) error { memo, err := deserializeMemo(mh.cdc, []byte(ftData.Memo)) if err != nil { recvPacketLogger(ctx).Debug("Can't deserialize ICS20 memo for hook execution", "err", err) @@ -29,9 +29,9 @@ func (mh MemoHandler) onRecvPacketPost(ctx *sdk.Context, ftData ics20types.Fungi recvPacketLogger(ctx).Debug("Can't deserialize ICS20 memo for hook execution", "err", err) return nil } - // TODO: need to handle fees! - // TODO: verify correctly if receiver and sender are the same (modulo hrp) +} +func (mh MemoHandler) onRecvPacketPost(ctx *sdk.Context, ftData ics20types.FungibleTokenPacketData, msgs []sdk.Msg) error { receiver, err := sdk.AccAddressFromBech32(ftData.Receiver) if err != nil { return sdkerrors.Wrap(err, "can't parse bech32 address") // should not happen From 9a48b30a29658677c07b441b70a4075676ef0a12 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 28 Feb 2024 11:23:20 -0700 Subject: [PATCH 3/6] rework memo handler pre/post onRecvPacket --- x/uibc/uics20/ibc_module.go | 44 +++++++++++++++++++++------- x/uibc/uics20/memo_handler.go | 54 ++++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 31 deletions(-) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index cc47a93a34..e5859e57ca 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -1,10 +1,13 @@ package uics20 import ( + "encoding/json" + sdkerrors "cosmossdk.io/errors" "github.com/cometbft/cometbft/libs/log" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/iavl/internal/logger" ics20types "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types" @@ -39,36 +42,55 @@ func NewICS20Module(app porttypes.IBCModule, cdc codec.JSONCodec, k quota.Keeper } // OnRecvPacket is called when a receiver chain receives a packet from SendPacket. +// 1. record IBC quota +// 2. Try to unpack and prepare memo. If memo has a correct structure, and fallback addr is +// defined but malformed, we cancel the transfer (otherwise would not be able to use it +// correctly). +// 3. If memo has a correct structure, but memo.messages can't be unpack or don't pass +// validation, then we continue with the transfer and overwrite the original receiver to +// fallback_addr if it's defined. +// 4. Execute the downstream middleware and the transfer app. +// 5. Execute hooks. If hook execution fails, we don't use the the fallback_addr nor ignore the +// transfer. This is because there could be other middlewares that are already executed. func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, ) exported.Acknowledgement { ftData, err := deserializeFTData(im.cdc, packet) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } - qk := im.kb.Keeper(&ctx) - if ackResp := qk.IBCOnRecvPacket(ftData, packet); ackResp != nil && !ackResp.Success() { + quotaKeeper := im.kb.Keeper(&ctx) + if ackResp := quotaKeeper.IBCOnRecvPacket(ftData, packet); ackResp != nil && !ackResp.Success() { return ackResp } - logger := recvPacketLogger(&ctx) + var events []string mh := MemoHandler{im.cdc, im.leverage} - var memo uibc.ICS20Memo - if ftData.Memo != "" { - if err := mh.onRecvPacketPre(&ctx, ftData); err != nil { - logger.Error("can't handle ICS20 memo", "err", err) + msgs, overwriteReceiver, err := mh.onRecvPacketPre(&ctx, packet, ftData) + if err != nil { + return channeltypes.NewErrorAcknowledgement(err) + } + if overwriteReceiver != nil { + ftData.Receiver = overwriteReceiver.String() + events = append(events, "overwrite receiver to fallback_addr="+ftData.Receiver) + if packet.Data, err = json.Marshal(ftData); err != nil { + return channeltypes.NewErrorAcknowledgement(err) } } + // call inner middeware and the app ack := im.IBCModule.OnRecvPacket(ctx, packet, relayer) if !ack.Success() { return ack } - if ftData.Memo != "" { - if err := mh.onRecvPacketPost(&ctx, packet, ftData); err != nil { - logger.Error("can't handle ICS20 memo", "err", err) - } + + if err := mh.dispatchMemoMsgs(&ctx, msgs); err != nil { + err := err.Error() + logger.Error("can't handle ICS20 memo", "err", err) + events = append(events, "can't handle ICS20 memo err = "+err) } + // TODO handle errors + return ack } diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 555652fe19..457761f547 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -19,47 +19,58 @@ type MemoHandler struct { leverage ltypes.MsgServer } +// See ICS20Module.OnRecvPacket for the flow func (mh MemoHandler) onRecvPacketPre( ctx *sdk.Context, packet ibcexported.PacketI, ftData ics20types.FungibleTokenPacketData, -) ([]sdk.Msg, error) { - - msgs, err := deserializeMemoMsgs(mh.cdc, []byte(ftData.Memo)) +) ([]sdk.Msg, sdk.AccAddress, error) { + logger := recvPacketLogger(ctx) + memo, err := deserializeMemo(mh.cdc, []byte(ftData.Memo)) if err != nil { - recvPacketLogger(ctx).Debug("Can't deserialize ICS20 memo for hook execution", "err", err) - return nil + logger.Debug("Not recognized ICS20 memo, ignoring hook execution", "err", err) + 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, + sdkerrors.Wrap(err, "ICS20 memo fallback_addr defined, but not formatted correctly") + } } - msgs, err := memo.GetMsgs() + + msgs, err = memo.GetMsgs() if err != nil { - recvPacketLogger(ctx).Debug("Can't deserialize ICS20 memo for hook execution", "err", err) - return nil + logger.Debug("Can't unpack ICS20 memo messages", + "err", err, "overwrite receiver to fallback_addr", fallbackReceiver != nil) + return nil, fallbackReceiver, nil } -} -func (mh MemoHandler) onRecvPacketPost(ctx *sdk.Context, ftData ics20types.FungibleTokenPacketData, msgs []sdk.Msg) error { receiver, err := sdk.AccAddressFromBech32(ftData.Receiver) - if err != nil { - return sdkerrors.Wrap(err, "can't parse bech32 address") // should not happen + if err != nil { // must not happen + return nil, nil, sdkerrors.Wrap(err, "can't parse ftData.Receiver bech32 address") } amount, ok := sdk.NewIntFromString(ftData.Amount) - if !ok { - return fmt.Errorf("can't parse transfer amount: %s [%w]", ftData.Amount, err) + if !ok { // must not happen + return nil, nil, fmt.Errorf("can't parse transfer amount: %s [%w]", ftData.Amount, err) } ibcDenom := uibc.ExtractDenomFromPacketOnRecv(packet, ftData.Denom) - return mh.dispatchMemoMsgs(ctx, receiver, sdk.NewCoin(ibcDenom, amount), msgs) + sentCoin := sdk.NewCoin(ibcDenom, amount) + if err := mh.validateMemoMsg(receiver, sentCoin, msgs); err != nil { + logger.Debug("memo.messages are not valid", "err", err) + return nil, fallbackReceiver, nil + } + + return msgs, fallbackReceiver, 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, receiver sdk.AccAddress, sent sdk.Coin, msgs []sdk.Msg) error { +func (mh MemoHandler) dispatchMemoMsgs(ctx *sdk.Context, msgs []sdk.Msg) error { if len(msgs) == 0 { return nil // quick return - we have nothing to handle } - if err := mh.validateMemoMsg(receiver, sent, msgs); err != nil { - return sdkerrors.Wrap(err, "ics20 memo messages are not valid.") - } - // Caching context so that we don't update the store in case of failure. cacheCtx, flush := ctx.CacheContext() logger := recvPacketLogger(ctx) @@ -89,6 +100,9 @@ var ( // transfer. func (mh MemoHandler) validateMemoMsg(_receiver sdk.AccAddress, sent sdk.Coin, msgs []sdk.Msg) error { msgLen := len(msgs) + if msgLen == 0 { + return nil + } // In this release we only support 1msg, and only messages that don't create or change // a borrow position if msgLen > 1 { From 027ea074b682710a923bedd0acc84ec1cc14fc32 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 28 Feb 2024 11:44:22 -0700 Subject: [PATCH 4/6] handle events --- x/uibc/uics20/ibc_module.go | 28 ++++++++++++++++++++-------- x/uibc/uics20/memo_handler.go | 26 +++++++++++++------------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index e5859e57ca..124e956dd3 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -7,7 +7,6 @@ import ( "github.com/cometbft/cometbft/libs/log" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/iavl/internal/logger" ics20types "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types" @@ -63,9 +62,8 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, return ackResp } - var events []string mh := MemoHandler{im.cdc, im.leverage} - msgs, overwriteReceiver, err := mh.onRecvPacketPre(&ctx, packet, ftData) + msgs, overwriteReceiver, events, err := mh.onRecvPacketPre(&ctx, packet, ftData) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } @@ -84,12 +82,10 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, } if err := mh.dispatchMemoMsgs(&ctx, msgs); err != nil { - err := err.Error() - logger.Error("can't handle ICS20 memo", "err", err) - events = append(events, "can't handle ICS20 memo err = "+err) + events = append(events, "can't handle ICS20 memo err = "+err.Error()) } - // TODO handle errors + im.emitEvents(ctx.EventManager(), recvPacketLogger(&ctx), "ics20-memo-hook", events) return ack } @@ -121,12 +117,28 @@ func (im ICS20Module) onAckErr(ctx *sdk.Context, packet channeltypes.Packet) { ftData, err := deserializeFTData(im.cdc, packet) if err != nil { // we only log error, because we want to propagate the ack to other layers. - ctx.Logger().Error("can't revert quota update", "err", err) + ctx.Logger().With("scope", "ics20-OnAckErr").Error("can't revert quota update", "err", err) } qk := im.kb.Keeper(ctx) qk.IBCRevertQuotaUpdate(ftData.Amount, ftData.Denom) } +func (im ICS20Module) emitEvents(em *sdk.EventManager, logger log.Logger, topic string, events []string) { + attributes := make([]sdk.Attribute, len(events)) + key := topic + "-context" + for i, s := range events { + attributes[i] = sdk.NewAttribute(key, s) + } + logger.Debug("Handle ICS20 memo", "events", events) + + em.EmitEvents(sdk.Events{ + sdk.NewEvent( + topic, + attributes..., + ), + }) +} + func deserializeFTData(cdc codec.JSONCodec, packet channeltypes.Packet, ) (d ics20types.FungibleTokenPacketData, err error) { if err = cdc.UnmarshalJSON(packet.GetData(), &d); err != nil { diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go index 457761f547..1daee291cd 100644 --- a/x/uibc/uics20/memo_handler.go +++ b/x/uibc/uics20/memo_handler.go @@ -22,45 +22,45 @@ type MemoHandler struct { // See ICS20Module.OnRecvPacket for the flow func (mh MemoHandler) onRecvPacketPre( ctx *sdk.Context, packet ibcexported.PacketI, ftData ics20types.FungibleTokenPacketData, -) ([]sdk.Msg, sdk.AccAddress, error) { - logger := recvPacketLogger(ctx) +) ([]sdk.Msg, sdk.AccAddress, []string, error) { + var events []string memo, err := deserializeMemo(mh.cdc, []byte(ftData.Memo)) if err != nil { - logger.Debug("Not recognized ICS20 memo, ignoring hook execution", "err", err) - return nil, nil, nil + recvPacketLogger(ctx).Debug("Not recognized ICS20 memo, ignoring hook execution", "err", err) + return nil, 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, + return nil, nil, nil, sdkerrors.Wrap(err, "ICS20 memo fallback_addr defined, but not formatted correctly") } } msgs, err = memo.GetMsgs() if err != nil { - logger.Debug("Can't unpack ICS20 memo messages", - "err", err, "overwrite receiver to fallback_addr", fallbackReceiver != nil) - return nil, fallbackReceiver, nil + e := "ICS20 memo recognized, but can't unpack memo.messages: " + err.Error() + events = append(events, e) + return nil, fallbackReceiver, events, nil } 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, 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, fmt.Errorf("can't parse transfer amount: %s [%w]", ftData.Amount, err) + 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 { - logger.Debug("memo.messages are not valid", "err", err) - return nil, fallbackReceiver, nil + events = append(events, "memo.messages are not valid, err: "+err.Error()) + return nil, fallbackReceiver, events, nil } - return msgs, fallbackReceiver, nil + return msgs, fallbackReceiver, events, nil } // runs messages encoded in the ICS20 memo. From 4605feb033db00c2122709e6c48a1fcb4d64d06f Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 16:33:39 -0700 Subject: [PATCH 5/6] cosmetic --- proto/umee/uibc/v1/uibc.proto | 2 +- x/uibc/uibc.pb.go | 2 +- x/uibc/uics20/ibc_module.go | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/proto/umee/uibc/v1/uibc.proto b/proto/umee/uibc/v1/uibc.proto index 0a2033ee1e..0baad7c15f 100644 --- a/proto/umee/uibc/v1/uibc.proto +++ b/proto/umee/uibc/v1/uibc.proto @@ -14,7 +14,7 @@ option (gogoproto.messagename_all) = true; message ICS20Memo { // messages is a list of `sdk.Msg`s that will be executed when handling ICS20 transfer. repeated google.protobuf.Any messages = 1; - // fallback_addr [optional] is an bech23 account address used to overwrite the original ICS20 + // fallback_addr [optional] is a bech23 account address used to overwrite the original ICS20 // recipient when: // 1. it is defined // 2. and memo is can be properly deserialized into this structure diff --git a/x/uibc/uibc.pb.go b/x/uibc/uibc.pb.go index 15decdcd84..79df39d46a 100644 --- a/x/uibc/uibc.pb.go +++ b/x/uibc/uibc.pb.go @@ -30,7 +30,7 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package type ICS20Memo struct { // messages is a list of `sdk.Msg`s that will be executed when handling ICS20 transfer. Messages []*types.Any `protobuf:"bytes,1,rep,name=messages,proto3" json:"messages,omitempty"` - // fallback_addr [optional] is an bech23 account address used to overwrite the original ICS20 + // fallback_addr [optional] is a bech23 account address used to overwrite the original ICS20 // recipient when: // 1. it is defined // 2. and memo is can be properly deserialized into this structure diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index 124e956dd3..61b67c8e2f 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -127,6 +127,7 @@ func (im ICS20Module) emitEvents(em *sdk.EventManager, logger log.Logger, topic attributes := make([]sdk.Attribute, len(events)) key := topic + "-context" for i, s := range events { + // it's ok that all events have the same key. This is how ibc-apps are dealing with events. attributes[i] = sdk.NewAttribute(key, s) } logger.Debug("Handle ICS20 memo", "events", events) From dce5adadd7de092e25a6a5c436338731d14c137d Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Feb 2024 16:41:25 -0700 Subject: [PATCH 6/6] docs: add comments assuring the ics20 hooks middleware is the last one in the middleware stack --- app/app.go | 3 +++ x/uibc/uics20/ibc_module.go | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/app.go b/app/app.go index ba85b70e32..f6569b9a9b 100644 --- a/app/app.go +++ b/app/app.go @@ -594,6 +594,9 @@ func New( packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp, // forward timeout packetforwardkeeper.DefaultRefundTransferPacketTimeoutTimestamp, // refund timeout ) + // NOTE: uics20 module must be the last middleware. We need to be sure there is no other code + // that will manipulate packet between the UICS20 middleware (which executes transfer hooks) + // and the transfer app. transferStack = uics20.NewICS20Module(transferStack, appCodec, app.UIbcQuotaKeeperB, leveragekeeper.NewMsgServerImpl(app.LeverageKeeper)) diff --git a/x/uibc/uics20/ibc_module.go b/x/uibc/uics20/ibc_module.go index 61b67c8e2f..cf4a305d96 100644 --- a/x/uibc/uics20/ibc_module.go +++ b/x/uibc/uics20/ibc_module.go @@ -62,6 +62,11 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, return ackResp } + // NOTE: IBC hooks must be the last middleware - just the transfer app. + // MemoHandler may update amoount in the message, because the received token amount may be + // 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 { @@ -69,13 +74,14 @@ func (im ICS20Module) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, } 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) } } - // call inner middeware and the app + // call transfer module app ack := im.IBCModule.OnRecvPacket(ctx, packet, relayer) if !ack.Success() { return ack