Merge pull request #37 from initia-labs/fix/move-middleware

fix move middleware to use strict json decoder

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.

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)
 	}

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

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)