feat(core, apps): 'PacketDataUnmarshaler' interface added and impleme…

…nted (backport #4188) (#4211) * 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 * fix: fixed backport conflicts --------- Co-authored-by: srdtrk <[email protected]> Co-authored-by: srdtrk <[email protected]>
cosmos · Aug 1, 2023 · 8802471 · 8802471
1 parent 001236b
commit 8802471
Show file tree

Hide file tree

Showing 10 changed files with 201 additions and 2 deletions.
diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -15,7 +15,10 @@ import (
 	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
-var _ porttypes.Middleware = &IBCMiddleware{}
+var (
+	_ porttypes.Middleware            = (*IBCMiddleware)(nil)
+	_ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil)
+)
 
 // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
 // ICA controller keeper and the underlying application.
@@ -251,3 +254,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
+}
diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/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)
+}
diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go
@@ -15,7 +15,10 @@ import (
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
-var _ porttypes.Middleware = &IBCMiddleware{}
+var (
+	_ porttypes.Middleware            = (*IBCMiddleware)(nil)
+	_ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil)
+)
 
 // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
 // fee keeper and the underlying application.
@@ -361,3 +364,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, sdkerrors.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil))
+	}
+
+	return unmarshaler.UnmarshalPacketData(bz)
+}
diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go
@@ -4,12 +4,15 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	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 +1081,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 := sdkerrors.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil))
+	suite.Require().ErrorIs(err, expError)
+}
diff --git a/modules/apps/29-fee/types/errors.go b/modules/apps/29-fee/types/errors.go
@@ -16,4 +16,5 @@ 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")
+	ErrUnsupportedAction             = sdkerrors.Register(ModuleName, 12, "unsupported action")
 )
diff --git a/modules/apps/transfer/ibc_module.go b/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
+}
diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go
@@ -3,6 +3,8 @@ package transfer_test
 import (
 	"math"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
 	"github.com/cosmos/ibc-go/v7/modules/apps/transfer"
@@ -239,3 +241,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)
+		}
+	}
+}
diff --git a/modules/core/05-port/types/module.go b/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)
+}
diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go
@@ -3,17 +3,31 @@ 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"
 )
 
+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"
+}
+
 // IBCModule implements the ICS26 callbacks for testing/mock.
 type IBCModule struct {
 	appModule *AppModule
@@ -162,6 +176,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())))

diff --git a/testing/mock/mock.go b/testing/mock/mock.go
@@ -36,6 +36,9 @@ var (
 	MockRecvCanaryCapabilityName    = "mock receive canary capability name"
 	MockAckCanaryCapabilityName     = "mock acknowledgement canary capability name"
 	MockTimeoutCanaryCapabilityName = "mock timeout canary capability name"
+	// MockApplicationCallbackError should be returned when an application callback should fail. It is possible to
+	// test that this error was returned using ErrorIs.
+	MockApplicationCallbackError error = &applicationCallbackError{}
 )
 
 var _ porttypes.IBCModule = IBCModule{}