feat(ica)!: support json tx encoding for interchain accounts (cosmos#…

…3796)

* feat(ica): added EncodingJson to supported encodings

* imp(ica): changed the type of cdc to Codec in ica/host

* imp(ica): changed the type of cdc to Codec in ica/controller

* imp(ica.test): added a test cases for EncodingJSON

* imp(ica): created invalid encoding err

* feat(ica)!: first prototype of json supporting DeserializeCosmosTx

* docs(ica): updated godoc of DeserializeCosmosTx

* docs(ica): added comments to DeserializeCosmosTx

* fix(ica.test): fixed tests for DeserializeCosmosTx

* fix(ica): fixed 'OnRecvPacket' in relay.go

* fix(ica): fixed unhandled error

* style(ica): made DeserializeCosmosTx more compact

* fix(ica/host.cli.test): fixed a cli test

* style(ica): ran gofumpt

* style(ica): changed err message

* feat(ica): first prototype of SerializeCosmosTx is implemented

* fix(ica): fixed codec tests

* fix(ica/host.test): fix test

* fix(ica/host.test): fix test

* fix(ica/host.cli): cli always uses protobuf

* nil(ica/host.test): removed unneeded comment

* fix(ica/controller.test): fix test

* fix(ica/controller.test): fix test

* fix(ica/controller.test): fix test

* fix(fee.test): fix test

* nit: temporary save commit

* fix(ica): fixed json serde tests not passing

* fix(ica): fix panic if message does not implement sdk.Msg

* imp(ica): improved json serde functions

* style(ica): pleased the linter

* style(ica): ran gofumpt

* fix(e2e): fix compilation errors by adding icatypes.EncodingProtobuf arg to serde functions

* feat(ica.test): added important wip test for deserializing directly from cosmwasm

* imp(ica.test): added a new test case to cw codec unit test

* imp(ica): added another test case

* imp(ica.test): added another test case

* imp(ica.test): added another test case

* style(ica.test): improved test style

* style(ica.test): improved test style

* style(ica.test): ran gofumpt

* imp(ica.test): added json encoding version string for testing

* imp(ica.test): added new 'NewJSONICAPath' function

* imp(ica.test): added encoding field to ica test setup functions

* fix(ica.test): fixed test setups using the new encoding field

* feat(ica.test): added json test case

* style(ica.test): ran gofumpt

* feat(ica.test): got two cases of cosmwasm tests working in relay

* style(ica.test): ran gofumpt

* feat(ica): started progress on recursive handling of Anys

* imp(ica.test): added a new test case for ica json encoding, this fails

* feat(ica): achieved total json serialization (excluding any lists)

* refactor(ica): made function shorter and removed duplicated code

* style(ica): ran gofumpt

* imp(ica): added more err handling code

* refactor(ica): made deserialize code shorter

* style(ica): made linter a bit more happy

* fix(ica.test): fixed one codec test case

* feat(ica): added []Any handling code

* fix(ica): added more safety

* nit: deleted testing codec.go

* feat(ica): all works

* style(ica): ran gofumpt

* style(ica): made linter happy

* refactor(ica): reduced code duplication

* nit(ica): uncommented some test cases

* imp(ica.test): added more test cases

* feat(ica.test): finished test cases

* style(ica.test): reorganized test cases

* refactor(ica.test): combined the two test cases into one

* style(ica.test): ran gofumpt

* style(ica.test): renamed wallet address

* fix(ica.test): fixed test case names

* imp(ica.test): added more test cases

* style(ica.test): ran gofumpt

* test(ica): added more codec test cases

* style(ica.test): ran gofumpt

* feat(ica): removed JSONAny and JSONCosmosTx types

* feat(ica): implemented json encoding using module codec

* fix(ica.test): tests now match the new codec implementation

* fix(ica.test): fixed the tests to the new implementation

* style(ica.test): reorgenized the order of tests so that git diff makes sense

* imp(ica/controller): controller codec need not be codec.Codec

* imp(ica): replaced BinaryCodec with Codec

* test(ica): fixed codec test

* docs(ica.test): codec comment updated

* docs(ica.test): updated comments

* style(ica.test): removed 'from cosmwasm' from test case name as it is aparent from test name

* style(ica.test): ran gofumpt

* fix: fix merge error

* deps(ica): replaced sdk.NewInt with sdkmath.NewInt

* style(ica): ran 'gofumpt'

* imp(ica): removed redundant cosmwasm tests

* revert: "imp(ica): removed redundant cosmwasm tests"

This reverts commit 5123fba.

* imp(ica.test): made codec_test  human readable

* imp(ica.test): made relay_test human readable

* style(ica.test): ran 'golanci-lint run --fix'

* imp(ica/host): created 'GetAppMetadata' function

* refactor(ica/host): used GetAppMetadata function

* imp(ica.test): removed unneeded encoding argument

* imp(ica): removed ErrUnsupportedEncoding

* imp(ica.test): used suite chainB height instead of clienttypes.NewHeight(1, 100)

* imp(ica.test): add nil check for unsupported encoding

* imp(ica.test): added a empty/nil checks

* style(ica.test): renamed version variable to TestVersionWithJSONEncoding

* imp(ica): wrapped some errors

* style(ica): ran 'golanci-lint run --fix'

* style(ica)!: renamed EncodingJSON to EncodingProto3JSON

* docs(ica): improved godocs

* imp(ica): passing codec instead of binary codec

* style(ica): improved error messages and godocs

* docs(ica.test): improved godocs for tests

* imp(ica.test): improved unsupported encoding test case slightly

* style(ica.test): test style improvements

* imp(ica.test): added expError to some codec tests

* imp(ica.test): added more error type checks to codec tests

* style(ica.test): ran 'golangci-lint run --fix'

* imp(ica/host.test): added 'TestMetadataNotFound'

* imp(ica/host.test): reduce test size

* docs(ica/host.test): updated godocs for test

* docs(ica/host): improved godoc

* imp(ica/host): made GetAppMetadata private

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

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

@@ -26,7 +26,7 @@ import (
 // Keeper defines the IBC interchain accounts controller keeper
 type Keeper struct {
 	storeKey       storetypes.StoreKey
-	cdc            codec.BinaryCodec
+	cdc            codec.Codec
 	legacySubspace paramtypes.Subspace
 	ics4Wrapper    porttypes.ICS4Wrapper
 	channelKeeper  icatypes.ChannelKeeper
@@ -43,7 +43,7 @@ type Keeper struct {
 
 // NewKeeper creates a new interchain accounts controller Keeper instance
 func NewKeeper(
-	cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace,
+	cdc codec.Codec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace,
 	ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
 	scopedKeeper exported.ScopedKeeper, msgRouter icatypes.MessageRouter, authority string,
 ) Keeper {

modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go

@@ -171,7 +171,7 @@ func (suite *KeeperTestSuite) TestSubmitTx() {
 				Amount:      sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))),
 			}
 
-			data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []proto.Message{icaMsg})
+			data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{icaMsg}, icatypes.EncodingProtobuf)
 			suite.Require().NoError(err)
 
 			packetData := icatypes.InterchainAccountPacketData{

modules/apps/27-interchain-accounts/host/client/cli/tx_test.go

@@ -125,7 +125,8 @@ func TestGeneratePacketData(t *testing.T) {
 				require.Equal(t, tc.memo, packetData.Memo)
 
 				data := packetData.Data
-				messages, err := icatypes.DeserializeCosmosTx(cdc, data)
+				// cli tx commands always use protobuf encoding
+				messages, err := icatypes.DeserializeCosmosTx(cdc, data, icatypes.EncodingProtobuf)
 
 				require.NoError(t, err)
 				require.NotNil(t, messages)

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -446,7 +446,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
 				ToAddress:   suite.chainB.SenderAccount.GetAddress().String(),
 				Amount:      amount,
 			}
-			data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []proto.Message{msg})
+			data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, icatypes.EncodingProtobuf)
 			suite.Require().NoError(err)
 
 			icaPacketData := icatypes.InterchainAccountPacketData{
@@ -655,7 +655,7 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
 		Amount:      tokenAmt,
 	}
 
-	data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg})
+	data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, icatypes.EncodingProtobuf)
 	suite.Require().NoError(err)
 
 	icaPacketData := icatypes.InterchainAccountPacketData{

modules/apps/27-interchain-accounts/host/keeper/export_test.go

@@ -0,0 +1,16 @@
+package keeper
+
+/*
+	This file is to allow for unexported functions to be accessible to the testing package.
+*/
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
+)
+
+// GetAppMetadata is a wrapper around getAppMetadata to allow the function to be directly called in tests.
+func (k Keeper) GetAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
+	return k.getAppMetadata(ctx, portID, channelID)
+}

modules/apps/27-interchain-accounts/host/keeper/genesis_test.go

@@ -109,7 +109,7 @@ func (suite *KeeperTestSuite) TestGenesisParams() {
 func (suite *KeeperTestSuite) TestExportGenesis() {
 	suite.SetupTest()
 
-	path := NewICAPath(suite.chainA, suite.chainB)
+	path := NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 	suite.coordinator.SetupConnections(path)
 
 	err := SetupICAPath(path, TestOwnerAddress)

modules/apps/27-interchain-accounts/host/keeper/handshake_test.go

@@ -278,7 +278,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
 
-			path = NewICAPath(suite.chainA, suite.chainB)
+			path = NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 			suite.coordinator.SetupConnections(path)
 
 			err := RegisterInterchainAccount(path.EndpointA, TestOwnerAddress)
@@ -358,7 +358,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenConfirm() {
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
 
-			path = NewICAPath(suite.chainA, suite.chainB)
+			path = NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 			suite.coordinator.SetupConnections(path)
 
 			err := RegisterInterchainAccount(path.EndpointA, TestOwnerAddress)
@@ -401,7 +401,7 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() {
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
 
-			path = NewICAPath(suite.chainA, suite.chainB)
+			path = NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 			suite.coordinator.SetupConnections(path)
 
 			err := SetupICAPath(path, TestOwnerAddress)

modules/apps/27-interchain-accounts/host/keeper/keeper.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"strings"
 
+	errorsmod "cosmossdk.io/errors"
 	"cosmossdk.io/log"
 	storetypes "cosmossdk.io/store/types"
 
@@ -18,6 +19,7 @@ import (
 	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"
+	ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
@@ -106,6 +108,22 @@ func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string
 	return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
 }
 
+// GetAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID
+func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
+	appVersion, found := k.GetAppVersion(ctx, portID, channelID)
+	if !found {
+		return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID)
+	}
+
+	var metadata icatypes.Metadata
+	if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(appVersion), &metadata); err != nil {
+		// UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks
+		return icatypes.Metadata{}, errorsmod.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata")
+	}
+
+	return metadata, nil
+}
+
 // GetActiveChannelID retrieves the active channelID from the store keyed by the provided connectionID and portID
 func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string) (string, bool) {
 	store := ctx.KVStore(k.storeKey)

modules/apps/27-interchain-accounts/host/keeper/keeper_test.go

@@ -1,6 +1,7 @@
 package keeper_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
@@ -9,6 +10,7 @@ import (
 	"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
+	ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 )
 
@@ -27,6 +29,15 @@ var (
 		Encoding:               icatypes.EncodingProtobuf,
 		TxType:                 icatypes.TxTypeSDKMultiMsg,
 	}))
+
+	// TestVersionWithJSONEncoding defines a reusable interchainaccounts version string that uses JSON encoding for testing purposes
+	TestVersionWithJSONEncoding = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{
+		Version:                icatypes.Version,
+		ControllerConnectionId: ibctesting.FirstConnectionID,
+		HostConnectionId:       ibctesting.FirstConnectionID,
+		Encoding:               icatypes.EncodingProto3JSON,
+		TxType:                 icatypes.TxTypeSDKMultiMsg,
+	}))
 )
 
 type KeeperTestSuite struct {
@@ -47,14 +58,25 @@ func (suite *KeeperTestSuite) SetupTest() {
 	suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3))
 }
 
-func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path {
+func NewICAPath(chainA, chainB *ibctesting.TestChain, encoding string) *ibctesting.Path {
 	path := ibctesting.NewPath(chainA, chainB)
+
+	var version string
+	switch encoding {
+	case icatypes.EncodingProtobuf:
+		version = TestVersion
+	case icatypes.EncodingProto3JSON:
+		version = TestVersionWithJSONEncoding
+	default:
+		panic(fmt.Sprintf("unsupported encoding type: %s", encoding))
+	}
+
 	path.EndpointA.ChannelConfig.PortID = icatypes.HostPortID
 	path.EndpointB.ChannelConfig.PortID = icatypes.HostPortID
 	path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED
 	path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED
-	path.EndpointA.ChannelConfig.Version = TestVersion
-	path.EndpointB.ChannelConfig.Version = TestVersion
+	path.EndpointA.ChannelConfig.Version = version
+	path.EndpointB.ChannelConfig.Version = version
 
 	return path
 }
@@ -85,7 +107,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
 		return err
 	}
 
@@ -106,7 +128,7 @@ func TestKeeperTestSuite(t *testing.T) {
 func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() {
 	suite.SetupTest()
 
-	path := NewICAPath(suite.chainA, suite.chainB)
+	path := NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 	suite.coordinator.SetupConnections(path)
 
 	err := SetupICAPath(path, TestOwnerAddress)
@@ -131,7 +153,7 @@ func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
 
 	suite.SetupTest()
 
-	path := NewICAPath(suite.chainA, suite.chainB)
+	path := NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 	suite.coordinator.SetupConnections(path)
 
 	err := SetupICAPath(path, TestOwnerAddress)
@@ -165,7 +187,7 @@ func (suite *KeeperTestSuite) TestGetAllInterchainAccounts() {
 
 	suite.SetupTest()
 
-	path := NewICAPath(suite.chainA, suite.chainB)
+	path := NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 	suite.coordinator.SetupConnections(path)
 
 	err := SetupICAPath(path, TestOwnerAddress)
@@ -197,7 +219,7 @@ func (suite *KeeperTestSuite) TestGetAllInterchainAccounts() {
 func (suite *KeeperTestSuite) TestIsActiveChannel() {
 	suite.SetupTest()
 
-	path := NewICAPath(suite.chainA, suite.chainB)
+	path := NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
 	suite.coordinator.SetupConnections(path)
 
 	err := SetupICAPath(path, TestOwnerAddress)
@@ -220,6 +242,17 @@ func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
 	suite.Require().Equal(expectedAccAddr, retrievedAddr)
 }
 
+func (suite *KeeperTestSuite) TestMetadataNotFound() {
+	var (
+		invalidPortID    = "invalid-port"
+		invalidChannelID = "invalid-channel"
+	)
+
+	_, err := suite.chainB.GetSimApp().ICAHostKeeper.GetAppMetadata(suite.chainB.GetContext(), invalidPortID, invalidChannelID)
+	suite.Require().ErrorIs(err, ibcerrors.ErrNotFound)
+	suite.Require().Contains(err.Error(), fmt.Sprintf("app version not found for port %s and channel %s", invalidPortID, invalidChannelID))
+}
+
 func (suite *KeeperTestSuite) TestParams() {
 	expParams := types.DefaultParams()
 

modules/apps/27-interchain-accounts/host/keeper/relay.go

@@ -24,9 +24,14 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byt
 		return nil, errorsmod.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain account packet data")
 	}
 
+	metadata, err := k.getAppMetadata(ctx, packet.DestinationPort, packet.DestinationChannel)
+	if err != nil {
+		return nil, err
+	}
+
 	switch data.Type {
 	case icatypes.EXECUTE_TX:
-		msgs, err := icatypes.DeserializeCosmosTx(k.cdc, data.Data)
+		msgs, err := icatypes.DeserializeCosmosTx(k.cdc, data.Data, metadata.Encoding)
 		if err != nil {
 			return nil, errorsmod.Wrapf(err, "failed to deserialize interchain account transaction")
 		}