diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 78469cfd3a1..af50f6c6c15 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -1,6 +1,7 @@ package ibccallbacks_test import ( + "encoding/json" "fmt" errorsmod "cosmossdk.io/errors" @@ -477,7 +478,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { s.Require().NoError(err) s.Require().NotNil(packet) - err = transfertypes.ModuleCdc.UnmarshalJSON(packet.Data, &packetData) + err = json.Unmarshal(packet.Data, &packetData) s.Require().NoError(err) ctx = s.chainA.GetContext() diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index b73dc6e8edb..4cfd55ffdef 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -1,6 +1,7 @@ package transfer import ( + "encoding/json" "fmt" "math" "strings" @@ -182,7 +183,7 @@ func (im IBCModule) OnRecvPacket( var data types.FungibleTokenPacketData var ackErr error - if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + if err := json.Unmarshal(packet.GetData(), &data); err != nil { ackErr = errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-20 transfer packet data") logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence)) ack = channeltypes.NewErrorAcknowledgement(ackErr) @@ -238,7 +239,7 @@ func (im IBCModule) OnAcknowledgementPacket( return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) } var data types.FungibleTokenPacketData - if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + if err := json.Unmarshal(packet.GetData(), &data); err != nil { return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) } @@ -286,7 +287,7 @@ func (im IBCModule) OnTimeoutPacket( relayer sdk.AccAddress, ) error { var data types.FungibleTokenPacketData - if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + if err := json.Unmarshal(packet.GetData(), &data); err != nil { return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) } // refund tokens @@ -352,7 +353,7 @@ func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, pr // PacketDataUnmarshaler interface required for ADR 008 support. func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) { var packetData types.FungibleTokenPacketData - if err := types.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil { + if err := json.Unmarshal(bz, &packetData); err != nil { return nil, err } diff --git a/modules/apps/transfer/types/codec.go b/modules/apps/transfer/types/codec.go index aef5c464632..0b5d63b145b 100644 --- a/modules/apps/transfer/types/codec.go +++ b/modules/apps/transfer/types/codec.go @@ -1,11 +1,6 @@ package types import ( - "bytes" - - "github.com/cosmos/gogoproto/jsonpb" - "github.com/cosmos/gogoproto/proto" - "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/legacy" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -39,32 +34,3 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { // The actual codec used for serialization should be provided to x/ibc transfer and // defined at the application level. var ModuleCdc = codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) - -// mustProtoMarshalJSON provides an auxiliary function to return Proto3 JSON encoded -// bytes of a message. -// NOTE: Copied from https://github.com/cosmos/cosmos-sdk/blob/971c542453e0972ef1dfc5a80159ad5049c7211c/codec/json.go -// and modified in order to allow `EmitDefaults` to be set to false for ics20 packet marshalling. -// This allows for the introduction of the memo field to be backwards compatible. -func mustProtoMarshalJSON(msg proto.Message) []byte { - anyResolver := codectypes.NewInterfaceRegistry() - - // EmitDefaults is set to false to prevent marshalling of unpopulated fields (memo) - // OrigName and the anyResovler match the fields the original SDK function would expect - // in order to minimize changes. - - // OrigName is true since there is no particular reason to use camel case - // The any resolver is empty, but provided anyways. - jm := &jsonpb.Marshaler{OrigName: true, EmitDefaults: false, AnyResolver: anyResolver} - - err := codectypes.UnpackInterfaces(msg, codectypes.ProtoJSONPacker{JSONPBMarshaler: jm}) - if err != nil { - panic(err) - } - - buf := new(bytes.Buffer) - if err := jm.Marshal(buf, msg); err != nil { - panic(err) - } - - return buf.Bytes() -} diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index 7a00da24f86..7478d4d0967 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -2,14 +2,13 @@ package types import ( "encoding/json" + "errors" "strings" "time" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" - sdk "github.com/cosmos/cosmos-sdk/types" - ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -67,9 +66,16 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { return ValidatePrefixedDenom(ftpd.Denom) } -// GetBytes is a helper for serialising +// GetBytes is a helper for serialising the packet to bytes. +// The memo field of FungibleTokenPacketData is marked with the JSON omitempty tag +// ensuring that the memo field is not included in the marshalled bytes if one is not specified. func (ftpd FungibleTokenPacketData) GetBytes() []byte { - return sdk.MustSortJSON(mustProtoMarshalJSON(&ftpd)) + bz, err := json.Marshal(ftpd) + if err != nil { + panic(errors.New("cannot marshal FungibleTokenPacketData into bytes")) + } + + return bz } // GetPacketSender returns the sender address embedded in the packet data. diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index bd7b1054ea6..d06d0774c7d 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -1,6 +1,7 @@ package types_test import ( + "encoding/json" "fmt" "testing" @@ -134,3 +135,27 @@ func (suite *TypesTestSuite) TestPacketDataProvider() { suite.Require().Equal(tc.expCustomData, customData) } } + +func (suite *TypesTestSuite) TestFungibleTokenPacketDataOmitEmpty() { + // check that omitempty is present for the memo field + packetData := types.FungibleTokenPacketData{ + Denom: denom, + Amount: amount, + Sender: sender, + Receiver: receiver, + // Default value for non-specified memo field is empty string + } + + bz, err := json.Marshal(packetData) + suite.Require().NoError(err) + + // check that the memo field is not present in the marshalled bytes + suite.Require().NotContains(string(bz), "memo") + + packetData.Memo = "abc" + bz, err = json.Marshal(packetData) + suite.Require().NoError(err) + + // check that the memo field is present in the marshalled bytes + suite.Require().Contains(string(bz), "memo") +}