From b7cb1ebe37f666d0830a14156071b21f1b0740d3 Mon Sep 17 00:00:00 2001
From: srdtrk <59252793+srdtrk@users.noreply.github.com>
Date: Mon, 21 Aug 2023 17:28:43 +0300
Subject: [PATCH] refactor(ica): packet data unmarshaling logic refactored
 (#4232)

* refactor(ica): refactored packet data's  unmarshal logic

* fix(ica_test): made tests pass

* imp(ica): changed to UnmarshalJSON api

* docs(ica): added godocs to iapd's 'UnmarshalJSON' method
---
 .../controller/ibc_middleware.go              |  8 +++----
 .../host/keeper/relay.go                      |  4 ++--
 .../27-interchain-accounts/types/packet.go    |  5 +++++
 .../types/packet_test.go                      | 21 +++++++++++++++++++
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
index 1ccd5206604..5bfc6ca16c9 100644
--- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
+++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -261,10 +261,10 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string)
 // into an InterchainAccountPacketData. This function implements the optional
 // PacketDataUnmarshaler interface required for ADR 008 support.
 func (IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) {
-	var packetData icatypes.InterchainAccountPacketData
-	if err := icatypes.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil {
+	var data icatypes.InterchainAccountPacketData
+	err := data.UnmarshalJSON(bz)
+	if err != nil {
 		return nil, err
 	}
-
-	return packetData, nil
+	return data, nil
 }
diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay.go b/modules/apps/27-interchain-accounts/host/keeper/relay.go
index 74dd7c6f832..a91a89a185d 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/relay.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/relay.go
@@ -18,8 +18,8 @@ import (
 // If the transaction is successfully executed, the transaction response bytes will be returned.
 func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byte, error) {
 	var data icatypes.InterchainAccountPacketData
-
-	if err := icatypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
+	err := data.UnmarshalJSON(packet.GetData())
+	if err != nil {
 		// UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks
 		return nil, errorsmod.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain account packet data")
 	}
diff --git a/modules/apps/27-interchain-accounts/types/packet.go b/modules/apps/27-interchain-accounts/types/packet.go
index 3091e6ba33c..f4e0efc9958 100644
--- a/modules/apps/27-interchain-accounts/types/packet.go
+++ b/modules/apps/27-interchain-accounts/types/packet.go
@@ -57,6 +57,11 @@ func (iapd InterchainAccountPacketData) GetBytes() []byte {
 	return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&iapd))
 }
 
+// UnmarshalJSON unmarshals raw JSON bytes into an InterchainAccountPacketData.
+func (iapd *InterchainAccountPacketData) UnmarshalJSON(bz []byte) error {
+	return ModuleCdc.UnmarshalJSON(bz, iapd)
+}
+
 // GetBytes returns the JSON marshalled interchain account CosmosTx.
 func (ct CosmosTx) GetBytes() []byte {
 	return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&ct))
diff --git a/modules/apps/27-interchain-accounts/types/packet_test.go b/modules/apps/27-interchain-accounts/types/packet_test.go
index c1ff4753652..6708ff2adaf 100644
--- a/modules/apps/27-interchain-accounts/types/packet_test.go
+++ b/modules/apps/27-interchain-accounts/types/packet_test.go
@@ -180,3 +180,24 @@ func (suite *TypesTestSuite) TestPacketDataProvider() {
 		suite.Require().Equal(tc.expCustomData, customData)
 	}
 }
+
+func (suite *TypesTestSuite) TestPacketDataUnmarshalerInterface() {
+	expPacketData := types.InterchainAccountPacketData{
+		Type: types.EXECUTE_TX,
+		Data: []byte("data"),
+		Memo: "some memo",
+	}
+
+	var packetData types.InterchainAccountPacketData
+	err := packetData.UnmarshalJSON(expPacketData.GetBytes())
+	suite.Require().NoError(err)
+	suite.Require().Equal(expPacketData, packetData)
+
+	// test invalid packet data
+	invalidPacketDataBytes := []byte("invalid packet data")
+
+	var invalidPacketData types.InterchainAccountPacketData
+	err = packetData.UnmarshalJSON(invalidPacketDataBytes)
+	suite.Require().Error(err)
+	suite.Require().Equal(types.InterchainAccountPacketData{}, invalidPacketData)
+}