Skip to content

Commit

Permalink
feat(ica)!: support json tx encoding for interchain accounts (backport
Browse files Browse the repository at this point in the history
…#3796) (#4006)

* feat(ica)!: support json tx encoding for interchain accounts (#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 <[email protected]>
(cherry picked from commit e5b057d)

# Conflicts:
#	modules/apps/27-interchain-accounts/controller/keeper/keeper.go
#	modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go
#	modules/apps/27-interchain-accounts/host/client/cli/tx_test.go
#	modules/apps/27-interchain-accounts/host/ibc_module_test.go
#	modules/apps/27-interchain-accounts/host/keeper/keeper.go
#	modules/apps/27-interchain-accounts/host/keeper/keeper_test.go
#	modules/apps/27-interchain-accounts/host/keeper/relay.go
#	modules/apps/27-interchain-accounts/host/keeper/relay_test.go
#	modules/apps/27-interchain-accounts/types/codec.go
#	modules/apps/27-interchain-accounts/types/codec_test.go

* fix(ica): fixed backport conflicts

* style(ica/host): Update modules/apps/27-interchain-accounts/host/keeper/keeper.go

Co-authored-by: colin axnér <[email protected]>

* style(ica): ran gofumpt

---------

Co-authored-by: srdtrk <[email protected]>
Co-authored-by: srdtrk <[email protected]>
Co-authored-by: colin axnér <[email protected]>
  • Loading branch information
4 people committed Jul 6, 2023
1 parent 06e33a1 commit 6257a7f
Show file tree
Hide file tree
Showing 11 changed files with 978 additions and 137 deletions.
16 changes: 16 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/export_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,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)
Expand Down Expand Up @@ -354,7 +354,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)
Expand Down Expand Up @@ -397,7 +397,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)
Expand Down
19 changes: 19 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import (
"fmt"
"strings"

errorsmod "cosmossdk.io/errors"
"github.com/cometbft/cometbft/libs/log"

"github.com/cosmos/cosmos-sdk/codec"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

ibcerrors "github.com/cosmos/ibc-go/v7/internal/errors"
genesistypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/genesis/types"
"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"
Expand Down Expand Up @@ -99,6 +102,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)
Expand Down
51 changes: 42 additions & 9 deletions modules/apps/27-interchain-accounts/host/keeper/keeper_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package keeper_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/suite"

ibcerrors "github.com/cosmos/ibc-go/v7/internal/errors"
genesistypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/genesis/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"
Expand All @@ -26,6 +28,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 {
Expand All @@ -46,14 +57,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
}
Expand Down Expand Up @@ -88,7 +110,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
}

Expand All @@ -109,7 +131,7 @@ func TestKeeperTestSuite(t *testing.T) {
func (suite *KeeperTestSuite) TestIsBound() {
suite.SetupTest()

path := NewICAPath(suite.chainA, suite.chainB)
path := NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
suite.coordinator.SetupConnections(path)

err := SetupICAPath(path, TestOwnerAddress)
Expand All @@ -122,7 +144,7 @@ func (suite *KeeperTestSuite) TestIsBound() {
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)
Expand All @@ -147,7 +169,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)
Expand Down Expand Up @@ -181,7 +203,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)
Expand Down Expand Up @@ -213,7 +235,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)
Expand All @@ -235,3 +257,14 @@ func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
suite.Require().True(found)
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))
}
7 changes: 6 additions & 1 deletion modules/apps/27-interchain-accounts/host/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byt
return nil, sdkerrors.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.DeserializeCosmosTxWithEncoding(k.cdc, data.Data, metadata.Encoding)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 6257a7f

Please sign in to comment.