feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188)

* feat(core/port): added PacketDataUnmarshaler interface

* feat(transfer): implemented PacketDataUnmarshaler interface in transfer

* feat(ica/controller): implemented PacketDataUnmarshaler interface

* feat(fee): implemented PacketDataUnmarshaler interface

* feat(mock): implemented PacketDataUnmarshaler interface

* imp(transfer_test): removed explicit use of callbacks in tests

* style(transfer_test): ran golangci-lint

* imp(testing/mock): removed ErrorMock for the upstreamed error

* style(fee_test): improved test readability

* imp(transfer_test): improved test styling and added new test case

(cherry picked from commit 2ac5506)

# Conflicts:
#	modules/apps/27-interchain-accounts/controller/ibc_middleware.go
#	modules/apps/29-fee/ibc_middleware.go
#	modules/apps/29-fee/ibc_middleware_test.go
#	modules/apps/29-fee/types/errors.go
#	modules/apps/transfer/ibc_module_test.go
#	testing/mock/ibc_module.go

Author: srdtrk

modules/apps/27-interchain-accounts/controller/ibc_middleware.go

@@ -15,7 +15,14 @@ import (
 	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+<<<<<<< HEAD
 var _ porttypes.Middleware = &IBCMiddleware{}
+=======
+var (
+	_ porttypes.Middleware            = (*IBCMiddleware)(nil)
+	_ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil)
+)
+>>>>>>> 2ac55069 (feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188))
 
 // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
 // ICA controller keeper and the underlying application.
@@ -251,3 +258,15 @@ func (im IBCMiddleware) WriteAcknowledgement(
 func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
 	return im.keeper.GetAppVersion(ctx, portID, channelID)
 }
+
+// UnmarshalPacketData attempts to unmarshal the provided packet data bytes
+// into an InterchainAccountPacketData. This function implements the optional
+// PacketDataUnmarshaler interface required for ADR 008 support.
+func (im IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) {
+	var packetData icatypes.InterchainAccountPacketData
+	if err := icatypes.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil {
+		return nil, err
+	}
+
+	return packetData, nil
+}

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -921,3 +921,26 @@ func (suite *InterchainAccountsTestSuite) TestClosedChannelReopensWithMsgServer(
 	err = path.EndpointB.ChanOpenConfirm()
 	suite.Require().NoError(err)
 }
+
+func (suite *InterchainAccountsTestSuite) TestPacketDataUnmarshalerInterface() {
+	path := NewICAPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupConnections(path)
+	err := SetupICAPath(path, TestOwnerAddress)
+	suite.Require().NoError(err)
+
+	expPacketData := icatypes.InterchainAccountPacketData{
+		Type: icatypes.EXECUTE_TX,
+		Data: []byte("data"),
+		Memo: "",
+	}
+
+	packetData, err := controller.IBCMiddleware{}.UnmarshalPacketData(expPacketData.GetBytes())
+	suite.Require().NoError(err)
+	suite.Require().Equal(expPacketData, packetData)
+
+	// test invalid packet data
+	invalidPacketData := []byte("invalid packet data")
+	packetData, err = controller.IBCMiddleware{}.UnmarshalPacketData(invalidPacketData)
+	suite.Require().Error(err)
+	suite.Require().Nil(packetData)
+}

modules/apps/29-fee/ibc_middleware.go

@@ -15,7 +15,14 @@ import (
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+<<<<<<< HEAD
 var _ porttypes.Middleware = &IBCMiddleware{}
+=======
+var (
+	_ porttypes.Middleware            = (*IBCMiddleware)(nil)
+	_ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil)
+)
+>>>>>>> 2ac55069 (feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188))
 
 // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
 // fee keeper and the underlying application.
@@ -361,3 +368,15 @@ func (im IBCMiddleware) WriteAcknowledgement(
 func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
 	return im.keeper.GetAppVersion(ctx, portID, channelID)
 }
+
+// UnmarshalPacketData attempts to use the underlying app to unmarshal the packet data.
+// If the underlying app does not support the PacketDataUnmarshaler interface, an error is returned.
+// This function implements the optional PacketDataUnmarshaler interface required for ADR 008 support.
+func (im IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) {
+	unmarshaler, ok := im.app.(porttypes.PacketDataUnmarshaler)
+	if !ok {
+		return nil, errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil))
+	}
+
+	return unmarshaler.UnmarshalPacketData(bz)
+}

modules/apps/29-fee/ibc_middleware_test.go

@@ -3,13 +3,21 @@ package fee_test
 import (
 	"fmt"
 
+<<<<<<< HEAD
+=======
+	errorsmod "cosmossdk.io/errors"
+	sdkmath "cosmossdk.io/math"
+
+>>>>>>> 2ac55069 (feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188))
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
 	fee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee"
+	feekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper"
 	"github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types"
 	transfertypes "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"
 	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
@@ -1078,3 +1086,27 @@ func (suite *FeeTestSuite) TestGetAppVersion() {
 		})
 	}
 }
+
+func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterface() {
+	module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
+	suite.Require().NoError(err)
+
+	cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
+	suite.Require().True(ok)
+
+	feeModule, ok := cbs.(porttypes.PacketDataUnmarshaler)
+	suite.Require().True(ok)
+
+	packetData, err := feeModule.UnmarshalPacketData(ibcmock.MockPacketData)
+	suite.Require().NoError(err)
+	suite.Require().Equal(ibcmock.MockPacketData, packetData)
+}
+
+func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterfaceError() {
+	// test the case when the underlying application cannot be casted to a PacketDataUnmarshaler
+	mockFeeMiddleware := fee.NewIBCMiddleware(nil, feekeeper.Keeper{})
+
+	_, err := mockFeeMiddleware.UnmarshalPacketData(ibcmock.MockPacketData)
+	expError := errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil))
+	suite.Require().ErrorIs(err, expError)
+}

modules/apps/29-fee/types/errors.go

@@ -6,6 +6,7 @@ import (
 
 // 29-fee sentinel errors
 var (
+<<<<<<< HEAD
 	ErrInvalidVersion                = sdkerrors.Register(ModuleName, 2, "invalid ICS29 middleware version")
 	ErrRefundAccNotFound             = sdkerrors.Register(ModuleName, 3, "no account found for given refund address")
 	ErrBalanceNotFound               = sdkerrors.Register(ModuleName, 4, "balance not found for given account address")
@@ -16,4 +17,17 @@ var (
 	ErrFeeNotEnabled                 = sdkerrors.Register(ModuleName, 9, "fee module is not enabled for this channel. If this error occurs after channel setup, fee module may not be enabled")
 	ErrRelayerNotFoundForAsyncAck    = sdkerrors.Register(ModuleName, 10, "relayer address must be stored for async WriteAcknowledgement")
 	ErrFeeModuleLocked               = sdkerrors.Register(ModuleName, 11, "the fee module is currently locked, a severe bug has been detected")
+=======
+	ErrInvalidVersion                = errorsmod.Register(ModuleName, 2, "invalid ICS29 middleware version")
+	ErrRefundAccNotFound             = errorsmod.Register(ModuleName, 3, "no account found for given refund address")
+	ErrBalanceNotFound               = errorsmod.Register(ModuleName, 4, "balance not found for given account address")
+	ErrFeeNotFound                   = errorsmod.Register(ModuleName, 5, "there is no fee escrowed for the given packetID")
+	ErrRelayersNotEmpty              = errorsmod.Register(ModuleName, 6, "relayers must not be set. This feature is not supported")
+	ErrCounterpartyPayeeEmpty        = errorsmod.Register(ModuleName, 7, "counterparty payee must not be empty")
+	ErrForwardRelayerAddressNotFound = errorsmod.Register(ModuleName, 8, "forward relayer address not found")
+	ErrFeeNotEnabled                 = errorsmod.Register(ModuleName, 9, "fee module is not enabled for this channel. If this error occurs after channel setup, fee module may not be enabled")
+	ErrRelayerNotFoundForAsyncAck    = errorsmod.Register(ModuleName, 10, "relayer address must be stored for async WriteAcknowledgement")
+	ErrFeeModuleLocked               = errorsmod.Register(ModuleName, 11, "the fee module is currently locked, a severe bug has been detected")
+	ErrUnsupportedAction             = errorsmod.Register(ModuleName, 12, "unsupported action")
+>>>>>>> 2ac55069 (feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188))
 )

modules/apps/transfer/ibc_module.go

@@ -17,6 +17,11 @@ import (
 	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+var (
+	_ porttypes.IBCModule             = (*IBCModule)(nil)
+	_ porttypes.PacketDataUnmarshaler = (*IBCModule)(nil)
+)
+
 // IBCModule implements the ICS26 interface for transfer given the transfer keeper.
 type IBCModule struct {
 	keeper keeper.Keeper
@@ -294,3 +299,15 @@ func (im IBCModule) OnTimeoutPacket(
 
 	return nil
 }
+
+// UnmarshalPacketData attempts to unmarshal the provided packet data bytes
+// into a FungibleTokenPacketData. This function implements the optional
+// PacketDataUnmarshaler interface required for ADR 008 support.
+func (im IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
+	var packetData types.FungibleTokenPacketData
+	if err := types.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil {
+		return nil, err
+	}
+
+	return packetData, nil
+}

modules/apps/transfer/ibc_module_test.go

@@ -3,8 +3,15 @@ package transfer_test
 import (
 	"math"
 
+<<<<<<< HEAD
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
+=======
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
+>>>>>>> 2ac55069 (feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188))
 	"github.com/cosmos/ibc-go/v7/modules/apps/transfer"
 	"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
@@ -239,3 +246,69 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() {
 		})
 	}
 }
+
+func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
+	var (
+		sender   = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
+		receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
+
+		data          []byte
+		expPacketData types.FungibleTokenPacketData
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+		{
+			"success: valid packet data with memo",
+			func() {
+				expPacketData = types.FungibleTokenPacketData{
+					Denom:    ibctesting.TestCoin.Denom,
+					Amount:   ibctesting.TestCoin.Amount.String(),
+					Sender:   sender,
+					Receiver: receiver,
+					Memo:     "some memo",
+				}
+				data = expPacketData.GetBytes()
+			},
+			true,
+		},
+		{
+			"success: valid packet data without memo",
+			func() {
+				expPacketData = types.FungibleTokenPacketData{
+					Denom:    ibctesting.TestCoin.Denom,
+					Amount:   ibctesting.TestCoin.Amount.String(),
+					Sender:   sender,
+					Receiver: receiver,
+					Memo:     "",
+				}
+				data = expPacketData.GetBytes()
+			},
+			true,
+		},
+		{
+			"failure: invalid packet data",
+			func() {
+				data = []byte("invalid packet data")
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc.malleate()
+
+		packetData, err := transfer.IBCModule{}.UnmarshalPacketData(data)
+
+		if tc.expPass {
+			suite.Require().NoError(err)
+			suite.Require().Equal(expPacketData, packetData)
+		} else {
+			suite.Require().Error(err)
+			suite.Require().Nil(packetData)
+		}
+	}
+}

modules/core/05-port/types/module.go

@@ -138,3 +138,10 @@ type Middleware interface {
 	IBCModule
 	ICS4Wrapper
 }
+
+// PacketDataUnmarshaler defines an optional interface which allows a middleware to
+// request the packet data to be unmarshaled by the base application.
+type PacketDataUnmarshaler interface {
+	// UnmarshalPacketData unmarshals the packet data into a concrete type
+	UnmarshalPacketData([]byte) (interface{}, error)
+}

testing/mock/ibc_module.go

@@ -3,17 +3,34 @@ package mock
 import (
 	"bytes"
 	"fmt"
+	"reflect"
 	"strconv"
 	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/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"
 	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+<<<<<<< HEAD
+=======
+var (
+	_ porttypes.IBCModule             = (*IBCModule)(nil)
+	_ porttypes.PacketDataUnmarshaler = (*IBCModule)(nil)
+)
+
+// applicationCallbackError is a custom error type that will be unique for testing purposes.
+type applicationCallbackError struct{}
+
+func (e applicationCallbackError) Error() string {
+	return "mock application callback failed"
+}
+
+>>>>>>> 2ac55069 (feat(core, apps): 'PacketDataUnmarshaler' interface added and implemented (#4188))
 // IBCModule implements the ICS26 callbacks for testing/mock.
 type IBCModule struct {
 	appModule *AppModule
@@ -162,6 +179,15 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet,
 	return nil
 }
 
+// UnmarshalPacketData returns the MockPacketData. This function implements the optional
+// PacketDataUnmarshaler interface required for ADR 008 support.
+func (im IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
+	if reflect.DeepEqual(bz, MockPacketData) {
+		return MockPacketData, nil
+	}
+	return nil, MockApplicationCallbackError
+}
+
 // GetMockRecvCanaryCapabilityName generates a capability name for testing OnRecvPacket functionality.
 func GetMockRecvCanaryCapabilityName(packet channeltypes.Packet) string {
 	return fmt.Sprintf("%s%s%s%s", MockRecvCanaryCapabilityName, packet.GetDestPort(), packet.GetDestChannel(), strconv.Itoa(int(packet.GetSequence())))