From 855c281ab50c6983011cad126f914adb84a795a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 29 Mar 2023 12:10:26 +0200 Subject: [PATCH] imp: add UnmarshalPacketData interface function (#3353) * adr 8 with 20/27 implementation * change interface name and register codecs * documentation * add comma before new line * fix return arg and dest for ica * add ica tests * adr 8 callback packet data impl followups (#3325) * remove query client (#3227) * remove query client * merge main * go mod tidy * Rename ``IsBound`` to ``HasCapability`` (#3253) --- .../controller/ibc_middleware.go | 17 ++++++++- .../controller/ibc_middleware_test.go | 17 +++++++++ .../27-interchain-accounts/host/ibc_module.go | 18 ++++++++++ .../host/ibc_module_test.go | 19 ++++++++++ modules/apps/27-interchain-accounts/module.go | 2 -- modules/apps/29-fee/ibc_middleware.go | 17 ++++++++- modules/apps/29-fee/ibc_middleware_test.go | 14 ++++++++ modules/apps/29-fee/types/errors.go | 1 + modules/apps/transfer/ibc_module.go | 17 +++++++++ modules/apps/transfer/ibc_module_test.go | 35 +++++++++++++++++++ modules/apps/transfer/module.go | 2 -- modules/core/05-port/types/module.go | 6 ++++ testing/mock/ibc_module.go | 6 ++++ 13 files changed, 165 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 53adf6a090e..530fe056e6c 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -13,7 +13,10 @@ import ( ibcexported "github.com/cosmos/ibc-go/v4/modules/core/exported" ) -var _ porttypes.Middleware = &IBCMiddleware{} +var ( + _ porttypes.Middleware = &IBCMiddleware{} + _ porttypes.PacketDataUnmarshaler = &IBCMiddleware{} +) // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the // ICA controller keeper and the underlying application. @@ -199,3 +202,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 index 256ee56afb2..716d9dc1ed2 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -712,3 +712,20 @@ func (suite *InterchainAccountsTestSuite) TestGetAppVersion() { suite.Require().True(found) suite.Require().Equal(path.EndpointA.ChannelConfig.Version, appVersion) } + +func (suite *InterchainAccountsTestSuite) TestUnmarshalPacketData() { + expPacketData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: []byte("data"), + Memo: `{"callbacks": {"src_callback_address": "testAddr"}}`, + } + + packetData, err := controller.IBCMiddleware{}.UnmarshalPacketData(expPacketData.GetBytes()) + suite.Require().NoError(err) + suite.Require().Equal(expPacketData, packetData) + + invalidPacketData := []byte("invalid packet data") + packetData, err = controller.IBCMiddleware{}.UnmarshalPacketData(invalidPacketData) + suite.Require().Error(err) + suite.Require().Nil(packetData) +} diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 7cf0deca969..6cb50a89a45 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -9,9 +9,15 @@ import ( "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/v4/modules/core/05-port/types" ibcexported "github.com/cosmos/ibc-go/v4/modules/core/exported" ) +var ( + _ porttypes.IBCModule = &IBCModule{} + _ porttypes.PacketDataUnmarshaler = &IBCModule{} +) + // IBCModule implements the ICS26 interface for interchain accounts host chains type IBCModule struct { keeper keeper.Keeper @@ -140,3 +146,15 @@ func (im IBCModule) OnTimeoutPacket( ) error { return sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "cannot cause a packet timeout on a host channel end, a host chain does not send a packet over the channel") } + +// 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 IBCModule) 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/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index b8001747d14..33bc4b85e9b 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -10,8 +10,10 @@ import ( "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/suite" + icahost "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/host" "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/types" + feetypes "github.com/cosmos/ibc-go/v4/modules/apps/29-fee/types" clienttypes "github.com/cosmos/ibc-go/v4/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v4/modules/core/24-host" @@ -702,6 +704,23 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose() suite.assertBalance(icaAddr, expBalAfterSecondSend) } +func (suite *InterchainAccountsTestSuite) TestUnmarshalPacketData() { + expPacketData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: []byte("data"), + Memo: `{"callbacks": {"src_callback_address": "testAddr"}}`, + } + + packetData, err := icahost.IBCModule{}.UnmarshalPacketData(expPacketData.GetBytes()) + suite.Require().NoError(err) + suite.Require().Equal(expPacketData, packetData) + + invalidPacketData := []byte("invalid packet data") + packetData, err = icahost.IBCModule{}.UnmarshalPacketData(invalidPacketData) + suite.Require().Error(err) + suite.Require().Nil(packetData) +} + // assertBalance asserts that the provided address has exactly the expected balance. // CONTRACT: the expected balance must only contain one coin denom. func (suite *InterchainAccountsTestSuite) assertBalance(addr sdk.AccAddress, expBalance sdk.Coins) { diff --git a/modules/apps/27-interchain-accounts/module.go b/modules/apps/27-interchain-accounts/module.go index 23a89b2f919..53b33172f92 100644 --- a/modules/apps/27-interchain-accounts/module.go +++ b/modules/apps/27-interchain-accounts/module.go @@ -29,8 +29,6 @@ import ( var ( _ module.AppModule = AppModule{} _ module.AppModuleBasic = AppModuleBasic{} - - _ porttypes.IBCModule = host.IBCModule{} ) // AppModuleBasic is the IBC interchain accounts AppModuleBasic diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 2cbce3d2f9d..6e7572cb053 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -14,7 +14,10 @@ import ( "github.com/cosmos/ibc-go/v4/modules/core/exported" ) -var _ porttypes.Middleware = &IBCMiddleware{} +var ( + _ porttypes.Middleware = &IBCMiddleware{} + _ porttypes.PacketDataUnmarshaler = &IBCMiddleware{} +) // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the // fee keeper and the underlying application. @@ -356,3 +359,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 PacketDataUnmarshaler") + } + + return unmarshaler.UnmarshalPacketData(bz) +} diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index ea7cdcae8b6..e62a2732777 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -1076,3 +1076,17 @@ func (suite *FeeTestSuite) TestGetAppVersion() { }) } } + +func (suite *FeeTestSuite) TestUnmarshalPacketData() { + 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 := cbs.(fee.IBCMiddleware) + + packetData, err := feeModule.UnmarshalPacketData(ibcmock.MockPacketData) + suite.Require().NoError(err) + suite.Require().Equal(ibcmock.MockPacketData, packetData) +} diff --git a/modules/apps/29-fee/types/errors.go b/modules/apps/29-fee/types/errors.go index 700864b9a33..213c4c4f1fd 100644 --- 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 index 6ecd7f1a6b4..a1799f1bc2e 100644 --- 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/v4/modules/core/exported" ) +var ( + _ porttypes.IBCModule = IBCModule{} + _ porttypes.PacketDataUnmarshaler = IBCModule{} +) + // 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 index 3ff7b25679e..fecb1979e7c 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -1,14 +1,18 @@ package transfer_test import ( + "fmt" "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/v4/modules/apps/transfer" "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v4/modules/core/24-host" + ibcexported "github.com/cosmos/ibc-go/v4/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v4/testing" ) @@ -239,3 +243,34 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() { }) } } + +func (suite *TransferTestSuite) TestUnmarshalPacketData() { + var ( + sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() + receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() + denom = "transfer/channel-0/atom" + amount = "100" + ) + + expPacketData := types.FungibleTokenPacketData{ + Denom: denom, + Amount: amount, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"callbacks": {"src_callback_address": "%s", "dest_callback_address": "%s"}}`, sender, receiver), + } + + packetData, err := transfer.IBCModule{}.UnmarshalPacketData(expPacketData.GetBytes()) + suite.Require().NoError(err) + suite.Require().Equal(expPacketData, packetData) + + callbackPacketData, ok := packetData.(ibcexported.CallbackPacketData) + suite.Require().True(ok) + suite.Require().Equal(sender, callbackPacketData.GetSourceCallbackAddress(), "incorrect source callback address") + suite.Require().Equal(receiver, callbackPacketData.GetDestCallbackAddress(), "incorrect destination callback address") + + invalidPacketData := []byte("invalid packet data") + packetData, err = transfer.IBCModule{}.UnmarshalPacketData(invalidPacketData) + suite.Require().Error(err) + suite.Require().Nil(packetData) +} diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index 9503a06d926..e6c932a5264 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -21,13 +21,11 @@ import ( "github.com/cosmos/ibc-go/v4/modules/apps/transfer/keeper" "github.com/cosmos/ibc-go/v4/modules/apps/transfer/simulation" "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types" - porttypes "github.com/cosmos/ibc-go/v4/modules/core/05-port/types" ) var ( _ module.AppModule = AppModule{} _ module.AppModuleBasic = AppModuleBasic{} - _ porttypes.IBCModule = IBCModule{} ) // AppModuleBasic is the IBC Transfer AppModuleBasic diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 3f71e1d0339..bf2f73b4474 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -133,3 +133,9 @@ 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([]byte) (interface{}, error) +} diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index 12d35564005..c5cb087a37e 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -158,6 +158,12 @@ 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) { + return MockPacketData, nil +} + // 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())))