From a13cfa71b517a16e382f7e4de4f52f39c4757f94 Mon Sep 17 00:00:00 2001 From: beer-1 <147697694+beer-1@users.noreply.github.com> Date: Thu, 14 Dec 2023 16:48:54 +0900 Subject: [PATCH] fix move middleware to use strict json decoder --- x/move/ibc-middleware/README.md | 58 ++++++++++++------------- x/move/ibc-middleware/ibc_middleware.go | 4 +- x/move/ibc-middleware/util.go | 13 ++++-- x/move/ibc-middleware/util_test.go | 39 +++++++++++++++++ 4 files changed, 79 insertions(+), 35 deletions(-) diff --git a/x/move/ibc-middleware/README.md b/x/move/ibc-middleware/README.md index 837485b4..0181cf97 100644 --- a/x/move/ibc-middleware/README.md +++ b/x/move/ibc-middleware/README.md @@ -19,21 +19,21 @@ The move `MsgExecute` is defined [here](https://github.com/initia-labs/initia/bl ```go type MsgExecute struct { - // Sender is the that actor that signed the messages - Sender string `protobuf:"bytes,1,opt,name=sender,proto3" json:"sender,omitempty"` - // ModuleAddress is the address of the module deployer - ModuleAddress string `protobuf:"bytes,2,opt,name=module_address,json=moduleAddress,proto3" json:"module_address,omitempty"` - // ModuleName is the name of module to execute - ModuleName string `protobuf:"bytes,3,opt,name=module_name,json=moduleName,proto3" json:"module_name,omitempty"` - // FunctionName is the name of a function to execute - FunctionName string `protobuf:"bytes,4,opt,name=function_name,json=functionName,proto3" json:"function_name,omitempty"` - // TypeArgs is the type arguments of a function to execute - // ex) "0x1::BasicCoin::Initia", "bool", "u8", "u64" - TypeArgs []string `protobuf:"bytes,5,rep,name=type_args,json=typeArgs,proto3" json:"type_args,omitempty"` - // Args is the arguments of a function to execute - // - number: little endian - // - string: base64 bytes - Args [][]byte `protobuf:"bytes,6,rep,name=args,proto3" json:"args,omitempty"` + // Sender is the that actor that signed the messages + Sender string `protobuf:"bytes,1,opt,name=sender,proto3" json:"sender,omitempty"` + // ModuleAddress is the address of the module deployer + ModuleAddress string `protobuf:"bytes,2,opt,name=module_address,json=moduleAddress,proto3" json:"module_address,omitempty"` + // ModuleName is the name of module to execute + ModuleName string `protobuf:"bytes,3,opt,name=module_name,json=moduleName,proto3" json:"module_name,omitempty"` + // FunctionName is the name of a function to execute + FunctionName string `protobuf:"bytes,4,opt,name=function_name,json=functionName,proto3" json:"function_name,omitempty"` + // TypeArgs is the type arguments of a function to execute + // ex) "0x1::BasicCoin::Initia", "bool", "u8", "u64" + TypeArgs []string `protobuf:"bytes,5,rep,name=type_args,json=typeArgs,proto3" json:"type_args,omitempty"` + // Args is the arguments of a function to execute + // - number: little endian + // - string: base64 bytes + Args [][]byte `protobuf:"bytes,6,rep,name=args,proto3" json:"args,omitempty"` } ``` @@ -53,21 +53,21 @@ So our constructed move message that we execute will look like: ```go msg := MsgExecuteContract{ - // Sender is the that actor that signed the messages - Sender: "init1-hash-of-channel-and-sender", - // ModuleAddress is the address of the module deployer - ModuleAddress: packet.data.memo["move"]["module_address"], + // Sender is the that actor that signed the messages + Sender: "init1-hash-of-channel-and-sender", + // ModuleAddress is the address of the module deployer + ModuleAddress: packet.data.memo["move"]["module_address"], // ModuleName is the name of module to execute - ModuleName: packet.data.memo["move"]["module_name"], + ModuleName: packet.data.memo["move"]["module_name"], // FunctionName is the name of a function to execute - FunctionName: packet.data.memo["move"]["function_name"], - // TypeArgs is the type arguments of a function to execute - // ex) "0x1::BasicCoin::Initia", "bool", "u8", "u64" - TypeArgs: packet.data.memo["move"]["type_args"], - // Args is the arguments of a function to execute - // - number: little endian - // - string: base64 bytes - Args: packet.data.memo["move"]["args"]} + FunctionName: packet.data.memo["move"]["function_name"], + // TypeArgs is the type arguments of a function to execute + // ex) "0x1::BasicCoin::Initia", "bool", "u8", "u64" + TypeArgs: packet.data.memo["move"]["type_args"], + // Args is the arguments of a function to execute + // - number: little endian + // - string: base64 bytes + Args: packet.data.memo["move"]["args"]} ``` ### ICS20 packet structure @@ -132,6 +132,6 @@ In move hooks, post packet execution: - if move message has error, return ErrAck - otherwise continue through middleware -# Testing strategy +### Testing strategy See go tests. diff --git a/x/move/ibc-middleware/ibc_middleware.go b/x/move/ibc-middleware/ibc_middleware.go index 4e196fa1..b8c63660 100644 --- a/x/move/ibc-middleware/ibc_middleware.go +++ b/x/move/ibc-middleware/ibc_middleware.go @@ -155,12 +155,12 @@ func (im IBCMiddleware) OnRecvPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) ibcexported.Acknowledgement { - isIcs20, ics20Data := isIcs20Packet(packet) + isIcs20, ics20Data := isIcs20Packet(packet.GetData()) if isIcs20 { return im.handleIcs20Packet(ctx, packet, relayer, ics20Data) } - isIcs721, ics721Data := isIcs721Packet(packet) + isIcs721, ics721Data := isIcs721Packet(packet.GetData()) if isIcs721 { return im.handleIcs721Packet(ctx, packet, relayer, ics721Data) } diff --git a/x/move/ibc-middleware/util.go b/x/move/ibc-middleware/util.go index 503dda87..c1ccc7e6 100644 --- a/x/move/ibc-middleware/util.go +++ b/x/move/ibc-middleware/util.go @@ -3,6 +3,7 @@ package ibc_middleware import ( "encoding/json" "fmt" + "strings" "cosmossdk.io/errors" @@ -26,17 +27,21 @@ func deriveIntermediateSender(channel, originalSender string) string { return senderAddr.String() } -func isIcs20Packet(packet channeltypes.Packet) (isIcs20 bool, ics20data transfertypes.FungibleTokenPacketData) { +func isIcs20Packet(packetData []byte) (isIcs20 bool, ics20data transfertypes.FungibleTokenPacketData) { var data transfertypes.FungibleTokenPacketData - if err := json.Unmarshal(packet.GetData(), &data); err != nil { + decoder := json.NewDecoder(strings.NewReader(string(packetData))) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&data); err != nil { return false, data } return true, data } -func isIcs721Packet(packet channeltypes.Packet) (isIcs721 bool, ics721data nfttransfertypes.NonFungibleTokenPacketData) { +func isIcs721Packet(packetData []byte) (isIcs721 bool, ics721data nfttransfertypes.NonFungibleTokenPacketData) { var data nfttransfertypes.NonFungibleTokenPacketData - if err := json.Unmarshal(packet.GetData(), &data); err != nil { + decoder := json.NewDecoder(strings.NewReader(string(packetData))) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&data); err != nil { return false, data } return true, data diff --git a/x/move/ibc-middleware/util_test.go b/x/move/ibc-middleware/util_test.go index f2bc9cb5..9b1b3e5d 100644 --- a/x/move/ibc-middleware/util_test.go +++ b/x/move/ibc-middleware/util_test.go @@ -2,14 +2,53 @@ package ibc_middleware import ( "encoding/base64" + "encoding/json" "fmt" "testing" + nfttransfertypes "github.com/initia-labs/initia/x/ibc/nft-transfer/types" movetypes "github.com/initia-labs/initia/x/move/types" vmtypes "github.com/initia-labs/initiavm/types" + + transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" + "github.com/stretchr/testify/require" ) +func Test_isIcs20Packet(t *testing.T) { + transferMsg := transfertypes.NewFungibleTokenPacketData("denom", "1000000", "0x1", "0x2", "memo") + bz, err := json.Marshal(transferMsg) + require.NoError(t, err) + + ok, _transferMsg := isIcs20Packet(bz) + require.True(t, ok) + require.Equal(t, transferMsg, _transferMsg) + + nftTransferMsg := nfttransfertypes.NewNonFungibleTokenPacketData("class_id", "uri", "data", []string{"1", "2", "3"}, []string{"uri1", "uri2", "uri3"}, []string{"data1", "data2", "data3"}, "sender", "receiver", "memo") + bz, err = json.Marshal(nftTransferMsg) + require.NoError(t, err) + + ok, _ = isIcs20Packet(bz) + require.False(t, ok) +} + +func Test_isIcs721Packet(t *testing.T) { + nftTransferMsg := nfttransfertypes.NewNonFungibleTokenPacketData("class_id", "uri", "data", []string{"1", "2", "3"}, []string{"uri1", "uri2", "uri3"}, []string{"data1", "data2", "data3"}, "sender", "receiver", "memo") + bz, err := json.Marshal(nftTransferMsg) + require.NoError(t, err) + + ok, _nftTransferMsg := isIcs721Packet(bz) + require.True(t, ok) + require.Equal(t, nftTransferMsg, _nftTransferMsg) + + transferMsg := transfertypes.NewFungibleTokenPacketData("denom", "1000000", "0x1", "0x2", "memo") + bz, err = json.Marshal(transferMsg) + require.NoError(t, err) + + ok, _ = isIcs721Packet(bz) + require.False(t, ok) +} + func Test_validateAndParseMemo(t *testing.T) { argBz, err := vmtypes.SerializeUint64(100)