From d882b43ccdb784b4cac08b402232d45956999588 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 13 Jan 2022 12:48:43 +0100 Subject: [PATCH] refactor: move ica connection identifiers from port to version metadata bytestring (#700) * define and generate new metadata proto type * refactor types pkg, remove version string parsers, add PortPrefix * refactor ica entrypoint and handshake to handle connection ids in metadata * fixing broken test cases * adding controller port and metadata validation, adding new testcases * updating proto doc and removing commented out code * updating testcase names, adding metadata constructor func, updating PortPrefix and removing Delimiter * adding ErrInvalidControllerPort and ErrInvalidHostPort * Apply suggestions from code review Co-authored-by: Carlos Rodriguez * updating error msgs to use expected, got format * adding inline metadata documentation as per review, updating bz to versionBytes Co-authored-by: Carlos Rodriguez --- docs/ibc/proto-docs.md | 38 ++ .../controller/ibc_module_test.go | 27 +- .../controller/keeper/account.go | 18 +- .../controller/keeper/account_test.go | 4 +- .../controller/keeper/handshake.go | 71 ++- .../controller/keeper/handshake_test.go | 124 +++-- .../controller/keeper/keeper_test.go | 21 +- .../host/ibc_module_test.go | 26 +- .../host/keeper/account_test.go | 3 +- .../host/keeper/handshake.go | 60 +-- .../host/keeper/handshake_test.go | 55 +- .../host/keeper/keeper_test.go | 23 +- .../host/keeper/relay_test.go | 2 +- .../27-interchain-accounts/types/account.go | 22 + .../types/account_test.go | 3 +- .../27-interchain-accounts/types/errors.go | 2 + .../apps/27-interchain-accounts/types/keys.go | 14 +- .../27-interchain-accounts/types/metadata.go | 11 + .../types/metadata.pb.go | 487 ++++++++++++++++++ .../apps/27-interchain-accounts/types/port.go | 68 +-- .../27-interchain-accounts/types/port_test.go | 116 +---- .../27-interchain-accounts/types/version.go | 67 --- .../types/version_test.go | 106 ---- .../interchain_accounts/v1/metadata.proto | 21 + 24 files changed, 870 insertions(+), 519 deletions(-) create mode 100644 modules/apps/27-interchain-accounts/types/metadata.go create mode 100644 modules/apps/27-interchain-accounts/types/metadata.pb.go delete mode 100644 modules/apps/27-interchain-accounts/types/version.go delete mode 100644 modules/apps/27-interchain-accounts/types/version_test.go create mode 100644 proto/ibc/applications/interchain_accounts/v1/metadata.proto diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index 921a3326522..9ac5b0eac5c 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -14,6 +14,9 @@ - [HostGenesisState](#ibc.applications.interchain_accounts.v1.HostGenesisState) - [RegisteredInterchainAccount](#ibc.applications.interchain_accounts.v1.RegisteredInterchainAccount) +- [ibc/applications/interchain_accounts/v1/metadata.proto](#ibc/applications/interchain_accounts/v1/metadata.proto) + - [Metadata](#ibc.applications.interchain_accounts.v1.Metadata) + - [ibc/applications/interchain_accounts/v1/packet.proto](#ibc/applications/interchain_accounts/v1/packet.proto) - [CosmosTx](#ibc.applications.interchain_accounts.v1.CosmosTx) - [InterchainAccountPacketData](#ibc.applications.interchain_accounts.v1.InterchainAccountPacketData) @@ -388,6 +391,41 @@ RegisteredInterchainAccount contains a pairing of controller port ID and associa + + + + + + + + + + + +

Top

+ +## ibc/applications/interchain_accounts/v1/metadata.proto + + + + + +### Metadata +Metadata defines a set of protocol specific data encoded into the ICS27 channel version bytestring +See ICS004: https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#Versioning + + +| Field | Type | Label | Description | +| ----- | ---- | ----- | ----------- | +| `version` | [string](#string) | | version defines the ICS27 protocol version | +| `controller_connection_id` | [string](#string) | | controller_connection_id is the connection identifier associated with the controller chain | +| `host_connection_id` | [string](#string) | | host_connection_id is the connection identifier associated with the host chain | +| `address` | [string](#string) | | address defines the interchain account address to be fulfilled upon the OnChanOpenTry handshake step NOTE: the address field is empty on the OnChanOpenInit handshake step | + + + + + diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go index 5137606e764..68e534cda74 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go @@ -18,15 +18,24 @@ import ( ) var ( + // TODO: Cosmos-SDK ADR-28: Update crypto.AddressHash() when sdk uses address.Module() + // https://github.com/cosmos/cosmos-sdk/issues/10225 + // // TestAccAddress defines a resuable bech32 address for testing purposes - // TODO: update crypto.AddressHash() when sdk uses address.Module() TestAccAddress = icatypes.GenerateAddress(sdk.AccAddress(crypto.AddressHash([]byte(icatypes.ModuleName))), TestPortID) + // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes - TestPortID, _ = icatypes.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + TestPortID, _ = icatypes.NewControllerPortID(TestOwnerAddress) + // TestVersion defines a resuable interchainaccounts version string for testing purposes - TestVersion = icatypes.NewAppVersion(icatypes.VersionPrefix, TestAccAddress.String()) + TestVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: ibctesting.FirstConnectionID, + HostConnectionId: ibctesting.FirstConnectionID, + })) ) type InterchainAccountsTestSuite struct { @@ -55,21 +64,21 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path.EndpointB.ChannelConfig.PortID = icatypes.PortID path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointA.ChannelConfig.Version = icatypes.VersionPrefix + path.EndpointA.ChannelConfig.Version = TestVersion path.EndpointB.ChannelConfig.Version = TestVersion return path } func InitInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error { - portID, err := icatypes.GeneratePortID(owner, endpoint.ConnectionID, endpoint.Counterparty.ConnectionID) + portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err } channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, endpoint.Counterparty.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { return err } @@ -150,7 +159,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { suite.coordinator.SetupConnections(path) // mock init interchain account - portID, err := icatypes.GeneratePortID(TestOwnerAddress, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + portID, err := icatypes.NewControllerPortID(TestOwnerAddress) suite.Require().NoError(err) portCap := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), portID) @@ -166,7 +175,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointA.ConnectionID}, - Version: icatypes.VersionPrefix, + Version: path.EndpointA.ChannelConfig.Version, } tc.malleate() // malleate mutates test data @@ -219,7 +228,7 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenTry() { proofInit, proofHeight := path.EndpointB.Chain.QueryProof(channelKey) // use chainA (controller) for ChanOpenTry - msg := channeltypes.NewMsgChannelOpenTry(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, TestVersion, channeltypes.ORDERED, []string{path.EndpointA.ConnectionID}, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, icatypes.VersionPrefix, proofInit, proofHeight, icatypes.ModuleName) + msg := channeltypes.NewMsgChannelOpenTry(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, TestVersion, channeltypes.ORDERED, []string{path.EndpointA.ConnectionID}, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, TestVersion, proofInit, proofHeight, icatypes.ModuleName) handler := suite.chainA.GetSimApp().MsgServiceRouter().Handler(msg) _, err = handler(suite.chainA.GetContext(), msg) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account.go b/modules/apps/27-interchain-accounts/controller/keeper/account.go index 4e4527a8af6..c0c61ab75f7 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account.go @@ -15,8 +15,8 @@ import ( // call 04-channel 'ChanOpenInit'. An error is returned if the port identifier is // already in use. Gaining access to interchain accounts whose channels have closed // cannot be done with this function. A regular MsgChanOpenInit must be used. -func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionID, counterpartyConnectionID, owner string) error { - portID, err := icatypes.GeneratePortID(owner, connectionID, counterpartyConnectionID) +func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionID, owner string) error { + portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err } @@ -30,7 +30,19 @@ func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionID, counterpart return sdkerrors.Wrap(err, "unable to bind to newly generated portID") } - msg := channeltypes.NewMsgChannelOpenInit(portID, icatypes.VersionPrefix, channeltypes.ORDERED, []string{connectionID}, icatypes.PortID, icatypes.ModuleName) + connectionEnd, err := k.channelKeeper.GetConnection(ctx, connectionID) + if err != nil { + return err + } + + // NOTE: An empty string is provided for accAddress, to be fulfilled upon OnChanOpenTry handshake step + metadata := icatypes.NewMetadata(icatypes.Version, connectionID, connectionEnd.GetCounterparty().GetConnectionID(), "") + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + if err != nil { + return err + } + + msg := channeltypes.NewMsgChannelOpenInit(portID, string(versionBytes), channeltypes.ORDERED, []string{connectionID}, icatypes.PortID, icatypes.ModuleName) handler := k.msgRouter.Handler(msg) res, err := handler(ctx, msg) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go index 0cbc9f4e281..967f7511aa2 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go @@ -37,7 +37,7 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() { { "MsgChanOpenInit fails - channel is already active", func() { - portID, err := icatypes.GeneratePortID(owner, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + portID, err := icatypes.NewControllerPortID(TestOwnerAddress) suite.Require().NoError(err) suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), portID, path.EndpointA.ChannelID) @@ -59,7 +59,7 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() { tc.malleate() // malleate mutates test data - err = suite.chainA.GetSimApp().ICAControllerKeeper.InitInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, path.EndpointB.ConnectionID, owner) + err = suite.chainA.GetSimApp().ICAControllerKeeper.InitInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index ea0b907569a..1dedbc228c4 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -1,6 +1,8 @@ package keeper import ( + "strings" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" @@ -32,26 +34,25 @@ func (k Keeper) OnChanOpenInit( return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order) } - connSequence, err := icatypes.ParseControllerConnSequence(portID) - if err != nil { - return sdkerrors.Wrapf(err, "expected format %s, got %s", icatypes.ControllerPortFormat, portID) + if !strings.HasPrefix(portID, icatypes.PortPrefix) { + return sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, portID) } - counterpartyConnSequence, err := icatypes.ParseHostConnSequence(portID) - if err != nil { - return sdkerrors.Wrapf(err, "expected format %s, got %s", icatypes.ControllerPortFormat, portID) + if counterparty.PortId != icatypes.PortID { + return sdkerrors.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.PortID, counterparty.PortId) } - if err := k.validateControllerPortParams(ctx, connectionHops, connSequence, counterpartyConnSequence); err != nil { - return sdkerrors.Wrapf(err, "failed to validate controller port %s", portID) + var metadata icatypes.Metadata + if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil { + return sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") } - if counterparty.PortId != icatypes.PortID { - return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", icatypes.PortID, counterparty.PortId) + if err := k.validateConnectionParams(ctx, connectionHops, metadata.ControllerConnectionId, metadata.HostConnectionId); err != nil { + return err } - if version != icatypes.VersionPrefix { - return sdkerrors.Wrapf(icatypes.ErrInvalidVersion, "expected %s, got %s", icatypes.VersionPrefix, version) + if metadata.Version != icatypes.Version { + return sdkerrors.Wrapf(icatypes.ErrInvalidVersion, "expected %s, got %s", icatypes.Version, metadata.Version) } activeChannelID, found := k.GetActiveChannelID(ctx, portID) @@ -71,21 +72,28 @@ func (k Keeper) OnChanOpenAck( counterpartyVersion string, ) error { if portID == icatypes.PortID { - return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "portID cannot be host chain port ID: %s", icatypes.PortID) + return sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "portID cannot be host chain port ID: %s", icatypes.PortID) } - if err := icatypes.ValidateVersion(counterpartyVersion); err != nil { - return sdkerrors.Wrap(err, "counterparty version validation failed") + if !strings.HasPrefix(portID, icatypes.PortPrefix) { + return sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, portID) } - k.SetActiveChannelID(ctx, portID, channelID) + var metadata icatypes.Metadata + if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(counterpartyVersion), &metadata); err != nil { + return sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") + } - accAddr, err := icatypes.ParseAddressFromVersion(counterpartyVersion) - if err != nil { - return sdkerrors.Wrapf(err, "expected format , got %s", icatypes.Delimiter, counterpartyVersion) + if err := icatypes.ValidateAccountAddress(metadata.Address); err != nil { + return err } - k.SetInterchainAccountAddress(ctx, portID, accAddr) + if metadata.Version != icatypes.Version { + return sdkerrors.Wrapf(icatypes.ErrInvalidVersion, "expected %s, got %s", icatypes.Version, metadata.Version) + } + + k.SetActiveChannelID(ctx, portID, channelID) + k.SetInterchainAccountAddress(ctx, portID, metadata.Address) return nil } @@ -102,31 +110,20 @@ func (k Keeper) OnChanCloseConfirm( return nil } -// validateControllerPortParams asserts the provided connection sequence and counterparty connection sequence -// match that of the associated connection stored in state -func (k Keeper) validateControllerPortParams(ctx sdk.Context, connectionHops []string, connectionSeq, counterpartyConnectionSeq uint64) error { +// validateConnectionParams asserts the provided controller and host connection identifiers match that of the associated connection stored in state +func (k Keeper) validateConnectionParams(ctx sdk.Context, connectionHops []string, controllerConnectionID, hostConnectionID string) error { connectionID := connectionHops[0] connection, err := k.channelKeeper.GetConnection(ctx, connectionID) if err != nil { return err } - connSeq, err := connectiontypes.ParseConnectionSequence(connectionID) - if err != nil { - return sdkerrors.Wrapf(err, "failed to parse connection sequence %s", connectionID) - } - - counterpartyConnSeq, err := connectiontypes.ParseConnectionSequence(connection.GetCounterparty().GetConnectionID()) - if err != nil { - return sdkerrors.Wrapf(err, "failed to parse counterparty connection sequence %s", connection.GetCounterparty().GetConnectionID()) - } - - if connSeq != connectionSeq { - return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "sequence mismatch, expected %d, got %d", connSeq, connectionSeq) + if controllerConnectionID != connectionID { + return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "expected %s, got %s", connectionID, controllerConnectionID) } - if counterpartyConnSeq != counterpartyConnectionSeq { - return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "counterparty sequence mismatch, expected %d, got %d", counterpartyConnSeq, counterpartyConnectionSeq) + if hostConnectionID != connection.GetCounterparty().GetConnectionID() { + return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "expected %s, got %s", connection.GetCounterparty().GetConnectionID(), hostConnectionID) } return nil diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 3df200e51a3..9e902c41084 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -11,9 +11,10 @@ import ( func (suite *KeeperTestSuite) TestOnChanOpenInit() { var ( - channel *channeltypes.Channel - path *ibctesting.Path - chanCap *capabilitytypes.Capability + channel *channeltypes.Channel + path *ibctesting.Path + chanCap *capabilitytypes.Capability + metadata icatypes.Metadata ) testCases := []struct { @@ -52,10 +53,10 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { false, }, { - "invalid version", + "invalid metadata bytestring", func() { path.EndpointA.SetChannel(*channel) - channel.Version = "version" + channel.Version = "invalid-metadata-bytestring" }, false, }, @@ -68,23 +69,40 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { false, }, { - "invalid connection sequence", + "invalid controller connection ID", + func() { + metadata.ControllerConnectionId = "invalid-connnection-id" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + + channel.Version = string(versionBytes) + path.EndpointA.SetChannel(*channel) + }, + false, + }, + { + "invalid host connection ID", func() { - portID, err := icatypes.GeneratePortID(TestOwnerAddress, "connection-1", "connection-0") + metadata.HostConnectionId = "invalid-connnection-id" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) suite.Require().NoError(err) - path.EndpointA.ChannelConfig.PortID = portID + channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, false, }, { - "invalid counterparty connection sequence", + "invalid version", func() { - portID, err := icatypes.GeneratePortID(TestOwnerAddress, "connection-0", "connection-1") + metadata.Version = "invalid-version" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) suite.Require().NoError(err) - path.EndpointA.ChannelConfig.PortID = portID + channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, false, @@ -108,7 +126,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { suite.coordinator.SetupConnections(path) // mock init interchain account - portID, err := icatypes.GeneratePortID(TestOwnerAddress, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + portID, err := icatypes.NewControllerPortID(TestOwnerAddress) suite.Require().NoError(err) portCap := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), portID) @@ -116,13 +134,17 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.ChannelConfig.PortID = portID // default values + metadata = icatypes.NewMetadata(icatypes.Version, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID, "") + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.INIT, Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointA.ConnectionID}, - Version: icatypes.VersionPrefix, + Version: string(versionBytes), } chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) @@ -131,7 +153,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { tc.malleate() // malleate mutates test data err = suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), - path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(), + path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version, ) if tc.expPass { @@ -146,9 +168,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { func (suite *KeeperTestSuite) TestOnChanOpenAck() { var ( - path *ibctesting.Path - expectedChannelID string - counterpartyVersion string + path *ibctesting.Path + metadata icatypes.Metadata ) testCases := []struct { @@ -160,18 +181,49 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { "success", func() {}, true, }, { - "invalid counterparty version", + "invalid port ID - host chain", func() { - expectedChannelID = "" - counterpartyVersion = "version" + path.EndpointA.ChannelConfig.PortID = icatypes.PortID }, false, }, { - "invalid portID", func() { - path.EndpointA.ChannelConfig.PortID = icatypes.PortID - expectedChannelID = "" - }, false, + "invalid port ID - unexpected prefix", + func() { + path.EndpointA.ChannelConfig.PortID = "invalid-port-id" + }, + false, + }, + { + "invalid metadata bytestring", + func() { + path.EndpointA.Counterparty.ChannelConfig.Version = "invalid-metadata-bytestring" + }, + false, + }, + { + "invalid account address", + func() { + metadata.Address = "invalid-account-address" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + + path.EndpointA.Counterparty.ChannelConfig.Version = string(versionBytes) + }, + false, + }, + { + "invalid counterparty version", + func() { + metadata.Version = "invalid-version" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + + path.EndpointA.Counterparty.ChannelConfig.Version = string(versionBytes) + }, + false, }, } @@ -182,7 +234,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) - counterpartyVersion = TestVersion suite.coordinator.SetupConnections(path) err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) @@ -190,20 +241,31 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { err = path.EndpointB.ChanOpenTry() suite.Require().NoError(err) - expectedChannelID = path.EndpointA.ChannelID + + metadata = icatypes.NewMetadata(icatypes.Version, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID, TestAccAddress.String()) + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + + path.EndpointB.ChannelConfig.Version = string(versionBytes) tc.malleate() // malleate mutates test data err = suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenAck(suite.chainA.GetContext(), - path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, counterpartyVersion, + path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointA.Counterparty.ChannelConfig.Version, ) - activeChannelID, _ := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) - - suite.Require().Equal(activeChannelID, expectedChannelID) - if tc.expPass { suite.Require().NoError(err) + + activeChannelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + suite.Require().True(found) + + suite.Require().Equal(path.EndpointA.ChannelID, activeChannelID) + + interchainAccAddress, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + suite.Require().True(found) + + suite.Require().Equal(metadata.Address, interchainAccAddress) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go index e1678689509..75cec2c45d9 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go @@ -14,15 +14,24 @@ import ( ) var ( + // TODO: Cosmos-SDK ADR-28: Update crypto.AddressHash() when sdk uses address.Module() + // https://github.com/cosmos/cosmos-sdk/issues/10225 + // // TestAccAddress defines a resuable bech32 address for testing purposes - // TODO: update crypto.AddressHash() when sdk uses address.Module() TestAccAddress = icatypes.GenerateAddress(sdk.AccAddress(crypto.AddressHash([]byte(icatypes.ModuleName))), TestPortID) + // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes - TestPortID, _ = icatypes.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + TestPortID, _ = icatypes.NewControllerPortID(TestOwnerAddress) + // TestVersion defines a resuable interchainaccounts version string for testing purposes - TestVersion = icatypes.NewAppVersion(icatypes.VersionPrefix, TestAccAddress.String()) + TestVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: ibctesting.FirstConnectionID, + HostConnectionId: ibctesting.FirstConnectionID, + })) ) type KeeperTestSuite struct { @@ -49,7 +58,7 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path.EndpointB.ChannelConfig.PortID = icatypes.PortID path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointA.ChannelConfig.Version = icatypes.VersionPrefix + path.EndpointA.ChannelConfig.Version = TestVersion path.EndpointB.ChannelConfig.Version = TestVersion return path @@ -78,14 +87,14 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { // InitInterchainAccount is a helper function for starting the channel handshake func InitInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error { - portID, err := icatypes.GeneratePortID(owner, endpoint.ConnectionID, endpoint.Counterparty.ConnectionID) + portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err } channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, endpoint.Counterparty.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { return err } 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 1f76d661a6c..0556f5b4100 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -20,15 +20,24 @@ import ( ) var ( + // TODO: Cosmos-SDK ADR-28: Update crypto.AddressHash() when sdk uses address.Module() + // https://github.com/cosmos/cosmos-sdk/issues/10225 + // // TestAccAddress defines a resuable bech32 address for testing purposes - // TODO: update crypto.AddressHash() when sdk uses address.Module() TestAccAddress = icatypes.GenerateAddress(sdk.AccAddress(crypto.AddressHash([]byte(icatypes.ModuleName))), TestPortID) + // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes - TestPortID, _ = icatypes.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + TestPortID, _ = icatypes.NewControllerPortID(TestOwnerAddress) + // TestVersion defines a resuable interchainaccounts version string for testing purposes - TestVersion = icatypes.NewAppVersion(icatypes.VersionPrefix, TestAccAddress.String()) + TestVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: ibctesting.FirstConnectionID, + HostConnectionId: ibctesting.FirstConnectionID, + })) ) type InterchainAccountsTestSuite struct { @@ -57,21 +66,21 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path.EndpointB.ChannelConfig.PortID = icatypes.PortID path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointA.ChannelConfig.Version = icatypes.VersionPrefix + path.EndpointA.ChannelConfig.Version = TestVersion path.EndpointB.ChannelConfig.Version = TestVersion return path } func InitInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error { - portID, err := icatypes.GeneratePortID(owner, endpoint.ConnectionID, endpoint.Counterparty.ConnectionID) + portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err } channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, endpoint.Counterparty.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { return err } @@ -115,7 +124,7 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenInit() { suite.coordinator.SetupConnections(path) // use chainB (host) for ChanOpenInit - msg := channeltypes.NewMsgChannelOpenInit(path.EndpointB.ChannelConfig.PortID, icatypes.VersionPrefix, channeltypes.ORDERED, []string{path.EndpointB.ConnectionID}, path.EndpointA.ChannelConfig.PortID, icatypes.ModuleName) + msg := channeltypes.NewMsgChannelOpenInit(path.EndpointB.ChannelConfig.PortID, icatypes.Version, channeltypes.ORDERED, []string{path.EndpointB.ConnectionID}, path.EndpointA.ChannelConfig.PortID, icatypes.ModuleName) handler := suite.chainB.GetSimApp().MsgServiceRouter().Handler(msg) _, err := handler(suite.chainB.GetContext(), msg) @@ -181,7 +190,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointB.ConnectionID}, - Version: "", + Version: path.EndpointB.ChannelConfig.Version, } tc.malleate() @@ -204,7 +213,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { if tc.expPass { suite.Require().NoError(err) - suite.Require().Equal(TestVersion, version) } else { suite.Require().Error(err) suite.Require().Equal("", version) diff --git a/modules/apps/27-interchain-accounts/host/keeper/account_test.go b/modules/apps/27-interchain-accounts/host/keeper/account_test.go index 0dce100888f..67828c79886 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/account_test.go @@ -4,7 +4,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" - ibctesting "github.com/cosmos/ibc-go/v3/testing" ) func (suite *KeeperTestSuite) TestRegisterInterchainAccount() { @@ -17,7 +16,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() { err := SetupICAPath(path, TestOwnerAddress) suite.Require().NoError(err) - portID, err := icatypes.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + portID, err := icatypes.NewControllerPortID(TestOwnerAddress) suite.Require().NoError(err) // Get the address of the interchain account stored in state during handshake step diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go index bb65da4a33b..5d95f605076 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go @@ -1,6 +1,8 @@ package keeper import ( + "strings" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" @@ -8,7 +10,6 @@ import ( icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" connectiontypes "github.com/cosmos/ibc-go/v3/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" ) @@ -31,25 +32,24 @@ func (k Keeper) OnChanOpenTry( } if portID != icatypes.PortID { - return "", sdkerrors.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", icatypes.PortID, portID) + return "", sdkerrors.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.PortID, portID) } - connSequence, err := icatypes.ParseHostConnSequence(counterparty.PortId) - if err != nil { - return "", sdkerrors.Wrapf(err, "expected format %s, got %s", icatypes.ControllerPortFormat, counterparty.PortId) + if !strings.HasPrefix(counterparty.PortId, icatypes.PortPrefix) { + return "", sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, counterparty.PortId) } - counterpartyConnSequence, err := icatypes.ParseControllerConnSequence(counterparty.PortId) - if err != nil { - return "", sdkerrors.Wrapf(err, "expected format %s, got %s", icatypes.ControllerPortFormat, counterparty.PortId) + var metadata icatypes.Metadata + if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(counterpartyVersion), &metadata); err != nil { + return "", sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") } - if err := k.validateControllerPortParams(ctx, connectionHops, connSequence, counterpartyConnSequence); err != nil { - return "", sdkerrors.Wrapf(err, "failed to validate controller port %s", counterparty.PortId) + if err := k.validateConnectionParams(ctx, connectionHops, metadata.ControllerConnectionId, metadata.HostConnectionId); err != nil { + return "", err } - if counterpartyVersion != icatypes.VersionPrefix { - return "", sdkerrors.Wrapf(icatypes.ErrInvalidVersion, "expected %s, got %s", icatypes.VersionPrefix, counterpartyVersion) + if metadata.Version != icatypes.Version { + return "", sdkerrors.Wrapf(icatypes.ErrInvalidVersion, "expected %s, got %s", icatypes.Version, metadata.Version) } // On the host chain the capability may only be claimed during the OnChanOpenTry @@ -58,13 +58,18 @@ func (k Keeper) OnChanOpenTry( return "", sdkerrors.Wrapf(err, "failed to claim capability for channel %s on port %s", channelID, portID) } - accAddr := icatypes.GenerateAddress(k.accountKeeper.GetModuleAddress(icatypes.ModuleName), counterparty.PortId) + accAddress := icatypes.GenerateAddress(k.accountKeeper.GetModuleAddress(icatypes.ModuleName), counterparty.PortId) // Register interchain account if it does not already exist - k.RegisterInterchainAccount(ctx, accAddr, counterparty.PortId) - version := icatypes.NewAppVersion(icatypes.VersionPrefix, accAddr.String()) + k.RegisterInterchainAccount(ctx, accAddress, counterparty.PortId) + + metadata.Address = accAddress.String() + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + if err != nil { + return "", err + } - return version, nil + return string(versionBytes), nil } // OnChanOpenConfirm completes the handshake process by setting the active channel in state on the host chain @@ -91,31 +96,20 @@ func (k Keeper) OnChanCloseConfirm( return nil } -// validateControllerPortParams asserts the provided connection sequence and counterparty connection sequence -// match that of the associated connection stored in state -func (k Keeper) validateControllerPortParams(ctx sdk.Context, connectionHops []string, connectionSeq, counterpartyConnectionSeq uint64) error { +// validateConnectionParams asserts the provided controller and host connection identifiers match that of the associated connection stored in state +func (k Keeper) validateConnectionParams(ctx sdk.Context, connectionHops []string, controllerConnectionID, hostConnectionID string) error { connectionID := connectionHops[0] connection, err := k.channelKeeper.GetConnection(ctx, connectionID) if err != nil { return err } - connSeq, err := connectiontypes.ParseConnectionSequence(connectionID) - if err != nil { - return sdkerrors.Wrapf(err, "failed to parse connection sequence %s", connectionID) - } - - counterpartyConnSeq, err := connectiontypes.ParseConnectionSequence(connection.GetCounterparty().GetConnectionID()) - if err != nil { - return sdkerrors.Wrapf(err, "failed to parse counterparty connection sequence %s", connection.GetCounterparty().GetConnectionID()) - } - - if connSeq != connectionSeq { - return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "sequence mismatch, expected %d, got %d", connSeq, connectionSeq) + if hostConnectionID != connectionID { + return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "expected %s, got %s", connectionID, controllerConnectionID) } - if counterpartyConnSeq != counterpartyConnectionSeq { - return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "counterparty sequence mismatch, expected %d, got %d", counterpartyConnSeq, counterpartyConnectionSeq) + if controllerConnectionID != connection.GetCounterparty().GetConnectionID() { + return sdkerrors.Wrapf(connectiontypes.ErrInvalidConnection, "expected %s, got %s", connection.GetCounterparty().GetConnectionID(), hostConnectionID) } return nil diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go index 48179f1a8a0..52ca9dfb319 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go @@ -11,10 +11,10 @@ import ( func (suite *KeeperTestSuite) TestOnChanOpenTry() { var ( - channel *channeltypes.Channel - path *ibctesting.Path - chanCap *capabilitytypes.Capability - counterpartyVersion string + channel *channeltypes.Channel + path *ibctesting.Path + chanCap *capabilitytypes.Capability + metadata icatypes.Metadata ) testCases := []struct { @@ -38,14 +38,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { false, }, { - "invalid port", + "invalid port ID", func() { path.EndpointB.ChannelConfig.PortID = "invalid-port-id" }, false, }, { - "invalid counterparty port", + "invalid counterparty port ID", func() { channel.Counterparty.PortId = "invalid-port-id" }, @@ -60,32 +60,45 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { false, }, { - "invalid connection sequence", + "invalid metadata bytestring", func() { - portID, err := icatypes.GeneratePortID(TestOwnerAddress, "connection-0", "connection-1") + path.EndpointA.ChannelConfig.Version = "invalid-metadata-bytestring" + }, + false, + }, + { + "invalid controller connection ID", + func() { + metadata.ControllerConnectionId = "invalid-connnection-id" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) suite.Require().NoError(err) - channel.Counterparty.PortId = portID - path.EndpointB.SetChannel(*channel) + path.EndpointA.ChannelConfig.Version = string(versionBytes) }, false, }, { - "invalid counterparty connection sequence", + "invalid host connection ID", func() { - portID, err := icatypes.GeneratePortID(TestOwnerAddress, "connection-1", "connection-0") + metadata.HostConnectionId = "invalid-connnection-id" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) suite.Require().NoError(err) - channel.Counterparty.PortId = portID - path.EndpointB.SetChannel(*channel) + path.EndpointA.ChannelConfig.Version = string(versionBytes) }, false, }, { "invalid counterparty version", func() { - counterpartyVersion = "version" - path.EndpointB.SetChannel(*channel) + metadata.Version = "invalid-version" + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + + path.EndpointA.ChannelConfig.Version = string(versionBytes) }, false, }, @@ -107,7 +120,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) - counterpartyVersion = icatypes.VersionPrefix suite.coordinator.SetupConnections(path) err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) @@ -118,13 +130,17 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { path.EndpointB.ChannelID = channeltypes.FormatChannelIdentifier(channelSequence) // default values + metadata = icatypes.NewMetadata(icatypes.Version, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID, "") + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.TRYOPEN, Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointB.ConnectionID}, - Version: TestVersion, + Version: string(versionBytes), } chanCap, err = suite.chainB.App.GetScopedIBCKeeper().NewCapability(suite.chainB.GetContext(), host.ChannelCapabilityPath(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)) @@ -133,12 +149,11 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { tc.malleate() // malleate mutates test data version, err := suite.chainB.GetSimApp().ICAHostKeeper.OnChanOpenTry(suite.chainB.GetContext(), channel.Ordering, channel.GetConnectionHops(), - path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chanCap, channel.Counterparty, counterpartyVersion, + path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chanCap, channel.Counterparty, path.EndpointA.ChannelConfig.Version, ) if tc.expPass { suite.Require().NoError(err) - suite.Require().Equal(TestVersion, version) } else { suite.Require().Error(err) suite.Require().Equal("", version) diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go index 102cb278ebe..e9824f6dcb2 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go @@ -14,15 +14,24 @@ import ( ) var ( + // TODO: Cosmos-SDK ADR-28: Update crypto.AddressHash() when sdk uses address.Module() + // https://github.com/cosmos/cosmos-sdk/issues/10225 + // // TestAccAddress defines a resuable bech32 address for testing purposes - // TODO: update crypto.AddressHash() when sdk uses address.Module() TestAccAddress = icatypes.GenerateAddress(sdk.AccAddress(crypto.AddressHash([]byte(icatypes.ModuleName))), TestPortID) + // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes - TestPortID, _ = icatypes.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + TestPortID, _ = icatypes.NewControllerPortID(TestOwnerAddress) + // TestVersion defines a resuable interchainaccounts version string for testing purposes - TestVersion = icatypes.NewAppVersion(icatypes.VersionPrefix, TestAccAddress.String()) + TestVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: ibctesting.FirstConnectionID, + HostConnectionId: ibctesting.FirstConnectionID, + })) ) type KeeperTestSuite struct { @@ -49,8 +58,8 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path.EndpointB.ChannelConfig.PortID = icatypes.PortID path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointA.ChannelConfig.Version = icatypes.VersionPrefix - path.EndpointB.ChannelConfig.Version = icatypes.VersionPrefix + path.EndpointA.ChannelConfig.Version = TestVersion + path.EndpointB.ChannelConfig.Version = TestVersion return path } @@ -78,14 +87,14 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { // InitInterchainAccount is a helper function for starting the channel handshake func InitInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error { - portID, err := icatypes.GeneratePortID(owner, endpoint.ConnectionID, endpoint.Counterparty.ConnectionID) + portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err } channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, endpoint.Counterparty.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go index ef8709b5ce1..37cefe34bba 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go @@ -395,7 +395,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { err := SetupICAPath(path, TestOwnerAddress) suite.Require().NoError(err) - portID, err := icatypes.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + portID, err := icatypes.NewControllerPortID(TestOwnerAddress) suite.Require().NoError(err) // Get the address of the interchain account stored in state during handshake step diff --git a/modules/apps/27-interchain-accounts/types/account.go b/modules/apps/27-interchain-accounts/types/account.go index e55c57f3b35..5ad990dd34d 100644 --- a/modules/apps/27-interchain-accounts/types/account.go +++ b/modules/apps/27-interchain-accounts/types/account.go @@ -2,6 +2,7 @@ package types import ( "encoding/json" + "regexp" "strings" crypto "github.com/cosmos/cosmos-sdk/crypto/types" @@ -17,6 +18,13 @@ var ( _ InterchainAccountI = (*InterchainAccount)(nil) ) +// DefaultMaxAddrLength defines the default maximum character length used in validation of addresses +var DefaultMaxAddrLength = 128 + +// IsValidAddr defines a regular expression to check if the provided string consists of +// strictly alphanumeric characters +var IsValidAddr = regexp.MustCompile("^[a-zA-Z0-9]*$").MatchString + // InterchainAccountI wraps the authtypes.AccountI interface type InterchainAccountI interface { authtypes.AccountI @@ -37,6 +45,20 @@ func GenerateAddress(moduleAccAddr sdk.AccAddress, portID string) sdk.AccAddress return sdk.AccAddress(sdkaddress.Derive(moduleAccAddr, []byte(portID))) } +// ValidateAccountAddress performs basic validation of interchain account addresses, enforcing constraints +// on address length and character set +func ValidateAccountAddress(addr string) error { + if !IsValidAddr(addr) || len(addr) == 0 || len(addr) > DefaultMaxAddrLength { + return sdkerrors.Wrapf( + ErrInvalidAccountAddress, + "address must contain strictly alphanumeric characters, not exceeding %d characters in length", + DefaultMaxAddrLength, + ) + } + + return nil +} + // NewInterchainAccount creates and returns a new InterchainAccount type func NewInterchainAccount(ba *authtypes.BaseAccount, accountOwner string) *InterchainAccount { return &InterchainAccount{ diff --git a/modules/apps/27-interchain-accounts/types/account_test.go b/modules/apps/27-interchain-accounts/types/account_test.go index 265a74d09af..d2098a3bdbb 100644 --- a/modules/apps/27-interchain-accounts/types/account_test.go +++ b/modules/apps/27-interchain-accounts/types/account_test.go @@ -17,8 +17,9 @@ import ( var ( // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes - TestPortID, _ = types.GeneratePortID(TestOwnerAddress, "connection-0", "connection-0") + TestPortID, _ = types.NewControllerPortID(TestOwnerAddress) ) type TypesTestSuite struct { diff --git a/modules/apps/27-interchain-accounts/types/errors.go b/modules/apps/27-interchain-accounts/types/errors.go index 6061c6ae243..575d619d516 100644 --- a/modules/apps/27-interchain-accounts/types/errors.go +++ b/modules/apps/27-interchain-accounts/types/errors.go @@ -17,4 +17,6 @@ var ( ErrInvalidVersion = sdkerrors.Register(ModuleName, 11, "invalid interchain accounts version") ErrInvalidAccountAddress = sdkerrors.Register(ModuleName, 12, "invalid account address") ErrUnsupported = sdkerrors.Register(ModuleName, 13, "interchain account does not support this action") + ErrInvalidControllerPort = sdkerrors.Register(ModuleName, 14, "invalid controller port") + ErrInvalidHostPort = sdkerrors.Register(ModuleName, 15, "invalid host port") ) diff --git a/modules/apps/27-interchain-accounts/types/keys.go b/modules/apps/27-interchain-accounts/types/keys.go index 3f535a01489..a855d878d30 100644 --- a/modules/apps/27-interchain-accounts/types/keys.go +++ b/modules/apps/27-interchain-accounts/types/keys.go @@ -8,12 +8,15 @@ const ( // ModuleName defines the interchain accounts module name ModuleName = "interchainaccounts" - // VersionPrefix defines the current version for interchain accounts - VersionPrefix = "ics27-1" - - // PortID is the default port id that the interchain accounts module binds to + // PortID is the default port id that the interchain accounts host submodule binds to PortID = "interchain-account" + // PortPrefix is the default port prefix that the interchain accounts controller submodule binds to + PortPrefix = "ics27-" + + // Version defines the current version for interchain accounts + Version = "ics27-1" + // StoreKey is the store key string for interchain accounts StoreKey = ModuleName @@ -22,9 +25,6 @@ const ( // QuerierRoute is the querier route for interchain accounts QuerierRoute = ModuleName - - // Delimiter is the delimiter used for the interchain accounts version string - Delimiter = "." ) var ( diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go new file mode 100644 index 00000000000..c770b74ca3a --- /dev/null +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -0,0 +1,11 @@ +package types + +// NewMetadata creates and returns a new ICS27 Metadata instance +func NewMetadata(version, controllerConnectionID, hostConnectionID, accAddress string) Metadata { + return Metadata{ + Version: version, + ControllerConnectionId: controllerConnectionID, + HostConnectionId: hostConnectionID, + Address: accAddress, + } +} diff --git a/modules/apps/27-interchain-accounts/types/metadata.pb.go b/modules/apps/27-interchain-accounts/types/metadata.pb.go new file mode 100644 index 00000000000..d3cc1214361 --- /dev/null +++ b/modules/apps/27-interchain-accounts/types/metadata.pb.go @@ -0,0 +1,487 @@ +// Code generated by protoc-gen-gogo. DO NOT EDIT. +// source: ibc/applications/interchain_accounts/v1/metadata.proto + +package types + +import ( + fmt "fmt" + _ "github.com/gogo/protobuf/gogoproto" + proto "github.com/gogo/protobuf/proto" + io "io" + math "math" + math_bits "math/bits" +) + +// Reference imports to suppress errors if they are not otherwise used. +var _ = proto.Marshal +var _ = fmt.Errorf +var _ = math.Inf + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the proto package it is being compiled against. +// A compilation error at this line likely means your copy of the +// proto package needs to be updated. +const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package + +// Metadata defines a set of protocol specific data encoded into the ICS27 channel version bytestring +// See ICS004: https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#Versioning +type Metadata struct { + // version defines the ICS27 protocol version + Version string `protobuf:"bytes,1,opt,name=version,proto3" json:"version,omitempty"` + // controller_connection_id is the connection identifier associated with the controller chain + ControllerConnectionId string `protobuf:"bytes,2,opt,name=controller_connection_id,json=controllerConnectionId,proto3" json:"controller_connection_id,omitempty" yaml:"controller_connection_id"` + // host_connection_id is the connection identifier associated with the host chain + HostConnectionId string `protobuf:"bytes,3,opt,name=host_connection_id,json=hostConnectionId,proto3" json:"host_connection_id,omitempty" yaml:"host_connection_id"` + // address defines the interchain account address to be fulfilled upon the OnChanOpenTry handshake step + // NOTE: the address field is empty on the OnChanOpenInit handshake step + Address string `protobuf:"bytes,4,opt,name=address,proto3" json:"address,omitempty"` +} + +func (m *Metadata) Reset() { *m = Metadata{} } +func (m *Metadata) String() string { return proto.CompactTextString(m) } +func (*Metadata) ProtoMessage() {} +func (*Metadata) Descriptor() ([]byte, []int) { + return fileDescriptor_c29c32e397d1f21e, []int{0} +} +func (m *Metadata) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *Metadata) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_Metadata.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *Metadata) XXX_Merge(src proto.Message) { + xxx_messageInfo_Metadata.Merge(m, src) +} +func (m *Metadata) XXX_Size() int { + return m.Size() +} +func (m *Metadata) XXX_DiscardUnknown() { + xxx_messageInfo_Metadata.DiscardUnknown(m) +} + +var xxx_messageInfo_Metadata proto.InternalMessageInfo + +func (m *Metadata) GetVersion() string { + if m != nil { + return m.Version + } + return "" +} + +func (m *Metadata) GetControllerConnectionId() string { + if m != nil { + return m.ControllerConnectionId + } + return "" +} + +func (m *Metadata) GetHostConnectionId() string { + if m != nil { + return m.HostConnectionId + } + return "" +} + +func (m *Metadata) GetAddress() string { + if m != nil { + return m.Address + } + return "" +} + +func init() { + proto.RegisterType((*Metadata)(nil), "ibc.applications.interchain_accounts.v1.Metadata") +} + +func init() { + proto.RegisterFile("ibc/applications/interchain_accounts/v1/metadata.proto", fileDescriptor_c29c32e397d1f21e) +} + +var fileDescriptor_c29c32e397d1f21e = []byte{ + // 316 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x91, 0xcf, 0x4b, 0xc3, 0x30, + 0x14, 0xc7, 0x57, 0x15, 0xa7, 0x3d, 0x49, 0x11, 0x89, 0x82, 0x99, 0xd4, 0x83, 0x5e, 0xd6, 0x30, + 0x07, 0x0a, 0x1e, 0x27, 0x1e, 0x44, 0xbc, 0xec, 0x28, 0x48, 0x49, 0x5f, 0x42, 0x17, 0x68, 0xf3, + 0x4a, 0x92, 0x15, 0xf6, 0x5f, 0xf8, 0x67, 0x79, 0xdc, 0xd1, 0xd3, 0x90, 0xed, 0xe6, 0x71, 0x7f, + 0x81, 0x74, 0x9d, 0x9b, 0x3f, 0x6f, 0x79, 0x79, 0xdf, 0xcf, 0x27, 0xe4, 0x3d, 0xff, 0x52, 0x25, + 0xc0, 0x78, 0x51, 0x64, 0x0a, 0xb8, 0x53, 0xa8, 0x2d, 0x53, 0xda, 0x49, 0x03, 0x03, 0xae, 0x74, + 0xcc, 0x01, 0x70, 0xa8, 0x9d, 0x65, 0x65, 0x87, 0xe5, 0xd2, 0x71, 0xc1, 0x1d, 0x8f, 0x0a, 0x83, + 0x0e, 0x83, 0x33, 0x95, 0x40, 0xf4, 0x95, 0x8b, 0xfe, 0xe0, 0xa2, 0xb2, 0x73, 0xb4, 0x9f, 0x62, + 0x8a, 0x0b, 0x86, 0x55, 0xa7, 0x1a, 0x0f, 0xdf, 0x3d, 0x7f, 0xe7, 0x61, 0x69, 0x0c, 0x88, 0xdf, + 0x2c, 0xa5, 0xb1, 0x0a, 0x35, 0xf1, 0x4e, 0xbc, 0xf3, 0xdd, 0xfe, 0x67, 0x19, 0x3c, 0xf9, 0x04, + 0x50, 0x3b, 0x83, 0x59, 0x26, 0x4d, 0x0c, 0xa8, 0xb5, 0x84, 0xea, 0xb5, 0x58, 0x09, 0xb2, 0x51, + 0x45, 0x7b, 0xa7, 0xf3, 0x49, 0xab, 0x35, 0xe2, 0x79, 0x76, 0x1d, 0xfe, 0x97, 0x0c, 0xfb, 0x07, + 0xeb, 0xd6, 0xcd, 0xaa, 0x73, 0x27, 0x82, 0x7b, 0x3f, 0x18, 0xa0, 0x75, 0x3f, 0xc4, 0x9b, 0x0b, + 0xf1, 0xf1, 0x7c, 0xd2, 0x3a, 0xac, 0xc5, 0xbf, 0x33, 0x61, 0x7f, 0xaf, 0xba, 0xfc, 0x26, 0x23, + 0x7e, 0x93, 0x0b, 0x61, 0xa4, 0xb5, 0x64, 0xab, 0xfe, 0xc5, 0xb2, 0xec, 0xc5, 0x2f, 0x53, 0xea, + 0x8d, 0xa7, 0xd4, 0x7b, 0x9b, 0x52, 0xef, 0x79, 0x46, 0x1b, 0xe3, 0x19, 0x6d, 0xbc, 0xce, 0x68, + 0xe3, 0xf1, 0x36, 0x55, 0x6e, 0x30, 0x4c, 0x22, 0xc0, 0x9c, 0x01, 0xda, 0x1c, 0x2d, 0x53, 0x09, + 0xb4, 0x53, 0x64, 0x65, 0x97, 0xe5, 0x28, 0x86, 0x99, 0xb4, 0xd5, 0x76, 0x2c, 0xbb, 0xb8, 0x6a, + 0xaf, 0x07, 0xdc, 0x5e, 0x2d, 0xc6, 0x8d, 0x0a, 0x69, 0x93, 0xed, 0xc5, 0x50, 0xbb, 0x1f, 0x01, + 0x00, 0x00, 0xff, 0xff, 0xb0, 0xff, 0x53, 0x4e, 0xcd, 0x01, 0x00, 0x00, +} + +func (m *Metadata) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *Metadata) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *Metadata) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + if len(m.Address) > 0 { + i -= len(m.Address) + copy(dAtA[i:], m.Address) + i = encodeVarintMetadata(dAtA, i, uint64(len(m.Address))) + i-- + dAtA[i] = 0x22 + } + if len(m.HostConnectionId) > 0 { + i -= len(m.HostConnectionId) + copy(dAtA[i:], m.HostConnectionId) + i = encodeVarintMetadata(dAtA, i, uint64(len(m.HostConnectionId))) + i-- + dAtA[i] = 0x1a + } + if len(m.ControllerConnectionId) > 0 { + i -= len(m.ControllerConnectionId) + copy(dAtA[i:], m.ControllerConnectionId) + i = encodeVarintMetadata(dAtA, i, uint64(len(m.ControllerConnectionId))) + i-- + dAtA[i] = 0x12 + } + if len(m.Version) > 0 { + i -= len(m.Version) + copy(dAtA[i:], m.Version) + i = encodeVarintMetadata(dAtA, i, uint64(len(m.Version))) + i-- + dAtA[i] = 0xa + } + return len(dAtA) - i, nil +} + +func encodeVarintMetadata(dAtA []byte, offset int, v uint64) int { + offset -= sovMetadata(v) + base := offset + for v >= 1<<7 { + dAtA[offset] = uint8(v&0x7f | 0x80) + v >>= 7 + offset++ + } + dAtA[offset] = uint8(v) + return base +} +func (m *Metadata) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + l = len(m.Version) + if l > 0 { + n += 1 + l + sovMetadata(uint64(l)) + } + l = len(m.ControllerConnectionId) + if l > 0 { + n += 1 + l + sovMetadata(uint64(l)) + } + l = len(m.HostConnectionId) + if l > 0 { + n += 1 + l + sovMetadata(uint64(l)) + } + l = len(m.Address) + if l > 0 { + n += 1 + l + sovMetadata(uint64(l)) + } + return n +} + +func sovMetadata(x uint64) (n int) { + return (math_bits.Len64(x|1) + 6) / 7 +} +func sozMetadata(x uint64) (n int) { + return sovMetadata(uint64((x << 1) ^ uint64((int64(x) >> 63)))) +} +func (m *Metadata) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowMetadata + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: Metadata: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: Metadata: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Version", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowMetadata + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthMetadata + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthMetadata + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Version = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field ControllerConnectionId", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowMetadata + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthMetadata + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthMetadata + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.ControllerConnectionId = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 3: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field HostConnectionId", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowMetadata + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthMetadata + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthMetadata + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.HostConnectionId = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Address", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowMetadata + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthMetadata + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthMetadata + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Address = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipMetadata(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthMetadata + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func skipMetadata(dAtA []byte) (n int, err error) { + l := len(dAtA) + iNdEx := 0 + depth := 0 + for iNdEx < l { + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowMetadata + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + wireType := int(wire & 0x7) + switch wireType { + case 0: + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowMetadata + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + iNdEx++ + if dAtA[iNdEx-1] < 0x80 { + break + } + } + case 1: + iNdEx += 8 + case 2: + var length int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowMetadata + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + length |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if length < 0 { + return 0, ErrInvalidLengthMetadata + } + iNdEx += length + case 3: + depth++ + case 4: + if depth == 0 { + return 0, ErrUnexpectedEndOfGroupMetadata + } + depth-- + case 5: + iNdEx += 4 + default: + return 0, fmt.Errorf("proto: illegal wireType %d", wireType) + } + if iNdEx < 0 { + return 0, ErrInvalidLengthMetadata + } + if depth == 0 { + return iNdEx, nil + } + } + return 0, io.ErrUnexpectedEOF +} + +var ( + ErrInvalidLengthMetadata = fmt.Errorf("proto: negative length found during unmarshaling") + ErrIntOverflowMetadata = fmt.Errorf("proto: integer overflow") + ErrUnexpectedEndOfGroupMetadata = fmt.Errorf("proto: unexpected end of group") +) diff --git a/modules/apps/27-interchain-accounts/types/port.go b/modules/apps/27-interchain-accounts/types/port.go index 9b5c3b5f53f..54949b8a1c4 100644 --- a/modules/apps/27-interchain-accounts/types/port.go +++ b/modules/apps/27-interchain-accounts/types/port.go @@ -2,78 +2,16 @@ package types import ( "fmt" - "strconv" "strings" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - - connectiontypes "github.com/cosmos/ibc-go/v3/modules/core/03-connection/types" - porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" -) - -const ( - // ControllerPortFormat is the expected port identifier format to which controller chains must conform - // See (TODO: Link to spec when updated) - ControllerPortFormat = "..." ) -// GeneratePortID generates an interchain accounts controller port identifier for the provided owner -// in the following format: -// -// 'ics-27---' -// https://github.com/seantking/ibc/tree/sean/ics-27-updates/spec/app/ics-027-interchain-accounts#registering--controlling-flows -// TODO: update link to spec -func GeneratePortID(owner, connectionID, counterpartyConnectionID string) (string, error) { +// NewControllerPortID creates and returns a new prefixed controller port identifier using the provided owner string +func NewControllerPortID(owner string) (string, error) { if strings.TrimSpace(owner) == "" { return "", sdkerrors.Wrap(ErrInvalidAccountAddress, "owner address cannot be empty") } - connectionSeq, err := connectiontypes.ParseConnectionSequence(connectionID) - if err != nil { - return "", sdkerrors.Wrap(err, "invalid connection identifier") - } - - counterpartyConnectionSeq, err := connectiontypes.ParseConnectionSequence(counterpartyConnectionID) - if err != nil { - return "", sdkerrors.Wrap(err, "invalid counterparty connection identifier") - } - - return fmt.Sprint( - VersionPrefix, Delimiter, - connectionSeq, Delimiter, - counterpartyConnectionSeq, Delimiter, - owner, - ), nil -} - -// ParseControllerConnSequence attempts to parse the controller connection sequence from the provided port identifier -// The port identifier must match the controller chain format outlined in (TODO: link spec), otherwise an empty string is returned -func ParseControllerConnSequence(portID string) (uint64, error) { - s := strings.Split(portID, Delimiter) - if len(s) != 4 { - return 0, sdkerrors.Wrap(porttypes.ErrInvalidPort, "failed to parse port identifier") - } - - seq, err := strconv.ParseUint(s[1], 10, 64) - if err != nil { - return 0, sdkerrors.Wrapf(err, "failed to parse connection sequence (%s)", s[1]) - } - - return seq, nil -} - -// ParseHostConnSequence attempts to parse the host connection sequence from the provided port identifier -// The port identifier must match the controller chain format outlined in (TODO: link spec), otherwise an empty string is returned -func ParseHostConnSequence(portID string) (uint64, error) { - s := strings.Split(portID, Delimiter) - if len(s) != 4 { - return 0, sdkerrors.Wrap(porttypes.ErrInvalidPort, "failed to parse port identifier") - } - - seq, err := strconv.ParseUint(s[2], 10, 64) - if err != nil { - return 0, sdkerrors.Wrapf(err, "failed to parse connection sequence (%s)", s[2]) - } - - return seq, nil + return fmt.Sprint(PortPrefix, owner), nil } diff --git a/modules/apps/27-interchain-accounts/types/port_test.go b/modules/apps/27-interchain-accounts/types/port_test.go index 3a01b79391a..bdef740cd98 100644 --- a/modules/apps/27-interchain-accounts/types/port_test.go +++ b/modules/apps/27-interchain-accounts/types/port_test.go @@ -7,7 +7,7 @@ import ( ibctesting "github.com/cosmos/ibc-go/v3/testing" ) -func (suite *TypesTestSuite) TestGeneratePortID() { +func (suite *TypesTestSuite) TestNewControllerPortID() { var ( path *ibctesting.Path owner = TestOwnerAddress @@ -22,33 +22,9 @@ func (suite *TypesTestSuite) TestGeneratePortID() { { "success", func() {}, - fmt.Sprint(types.VersionPrefix, types.Delimiter, "0", types.Delimiter, "0", types.Delimiter, TestOwnerAddress), + fmt.Sprint(types.PortPrefix, TestOwnerAddress), true, }, - { - "success with non matching connection sequences", - func() { - path.EndpointA.ConnectionID = "connection-1" - }, - fmt.Sprint(types.VersionPrefix, types.Delimiter, "1", types.Delimiter, "0", types.Delimiter, TestOwnerAddress), - true, - }, - { - "invalid connectionID", - func() { - path.EndpointA.ConnectionID = "connection" - }, - "", - false, - }, - { - "invalid counterparty connectionID", - func() { - path.EndpointB.ConnectionID = "connection" - }, - "", - false, - }, { "invalid owner address", func() { @@ -69,7 +45,7 @@ func (suite *TypesTestSuite) TestGeneratePortID() { tc.malleate() // malleate mutates test data - portID, err := types.GeneratePortID(owner, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + portID, err := types.NewControllerPortID(owner) if tc.expPass { suite.Require().NoError(err, tc.name) @@ -81,89 +57,3 @@ func (suite *TypesTestSuite) TestGeneratePortID() { }) } } - -func (suite *TypesTestSuite) TestParseControllerConnSequence() { - - testCases := []struct { - name string - portID string - expValue uint64 - expPass bool - }{ - { - "success", - TestPortID, - 0, - true, - }, - { - "failed to parse port identifier", - "invalid-port-id", - 0, - false, - }, - { - "failed to parse connection sequence", - "ics27-1.x.y.cosmos1", - 0, - false, - }, - } - - for _, tc := range testCases { - suite.Run(tc.name, func() { - connSeq, err := types.ParseControllerConnSequence(tc.portID) - - if tc.expPass { - suite.Require().Equal(tc.expValue, connSeq) - suite.Require().NoError(err, tc.name) - } else { - suite.Require().Zero(connSeq) - suite.Require().Error(err, tc.name) - } - }) - } -} - -func (suite *TypesTestSuite) TestParseHostConnSequence() { - - testCases := []struct { - name string - portID string - expValue uint64 - expPass bool - }{ - { - "success", - TestPortID, - 0, - true, - }, - { - "failed to parse port identifier", - "invalid-port-id", - 0, - false, - }, - { - "failed to parse connection sequence", - "ics27-1.x.y.cosmos1", - 0, - false, - }, - } - - for _, tc := range testCases { - suite.Run(tc.name, func() { - connSeq, err := types.ParseHostConnSequence(tc.portID) - - if tc.expPass { - suite.Require().Equal(tc.expValue, connSeq) - suite.Require().NoError(err, tc.name) - } else { - suite.Require().Zero(connSeq) - suite.Require().Error(err, tc.name) - } - }) - } -} diff --git a/modules/apps/27-interchain-accounts/types/version.go b/modules/apps/27-interchain-accounts/types/version.go deleted file mode 100644 index ffa33aa0017..00000000000 --- a/modules/apps/27-interchain-accounts/types/version.go +++ /dev/null @@ -1,67 +0,0 @@ -package types - -import ( - "fmt" - "regexp" - "strings" - - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" -) - -// DefaultMaxAddrLength defines the default maximum character length used in validation of addresses -var DefaultMaxAddrLength = 128 - -// IsValidAddr defines a regular expression to check if the provided string consists of -// strictly alphanumeric characters -var IsValidAddr = regexp.MustCompile("^[a-zA-Z0-9]*$").MatchString - -// NewVersion returns a complete version string in the format: VersionPrefix + Delimter + AccAddress -func NewAppVersion(versionPrefix, accAddr string) string { - return fmt.Sprint(versionPrefix, Delimiter, accAddr) -} - -// ParseAddressFromVersion attempts to extract the associated account address from the provided version string -func ParseAddressFromVersion(version string) (string, error) { - s := strings.Split(version, Delimiter) - if len(s) != 2 { - return "", sdkerrors.Wrap(ErrInvalidVersion, "failed to parse version") - } - - return s[1], nil -} - -// ValidateVersion performs basic validation of the provided ics27 version string. -// An ics27 version string may include an optional account address as per [TODO: Add spec when available] -// ValidateVersion first attempts to split the version string using the standard delimiter, then asserts a supported -// version prefix is included, followed by additional checks which enforce constraints on the account address. -func ValidateVersion(version string) error { - s := strings.Split(version, Delimiter) - - if len(s) != 2 { - return sdkerrors.Wrapf(ErrInvalidVersion, "expected format , got %s", Delimiter, version) - } - - if s[0] != VersionPrefix { - return sdkerrors.Wrapf(ErrInvalidVersion, "expected %s, got %s", VersionPrefix, s[0]) - } - - if err := ValidateAccountAddress(s[1]); err != nil { - return err - } - - return nil -} - -// ValidateAccountAddress performs basic validation of interchain account addresses, enforcing constraints -// on address length and character set -func ValidateAccountAddress(addr string) error { - if !IsValidAddr(addr) || len(addr) == 0 || len(addr) > DefaultMaxAddrLength { - return sdkerrors.Wrapf( - ErrInvalidAccountAddress, - "address must contain strictly alphanumeric characters, not exceeding %d characters in length", - DefaultMaxAddrLength, - ) - } - - return nil -} diff --git a/modules/apps/27-interchain-accounts/types/version_test.go b/modules/apps/27-interchain-accounts/types/version_test.go deleted file mode 100644 index 6da0c780c3e..00000000000 --- a/modules/apps/27-interchain-accounts/types/version_test.go +++ /dev/null @@ -1,106 +0,0 @@ -package types_test - -import ( - "fmt" - - "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" -) - -func (suite *TypesTestSuite) TestParseAddressFromVersion() { - - testCases := []struct { - name string - version string - expValue string - expPass bool - }{ - { - "success", - types.NewAppVersion(types.VersionPrefix, TestOwnerAddress), - TestOwnerAddress, - true, - }, - { - "failed to parse address from version", - "invalid-version-string", - "", - false, - }, - { - "failure with multiple delimiters", - fmt.Sprint(types.NewAppVersion(types.VersionPrefix, TestOwnerAddress), types.Delimiter, types.NewAppVersion(types.VersionPrefix, TestOwnerAddress)), - "", - false, - }, - } - - for _, tc := range testCases { - suite.Run(tc.name, func() { - addr, err := types.ParseAddressFromVersion(tc.version) - - if tc.expPass { - suite.Require().Equal(tc.expValue, addr) - suite.Require().NoError(err, tc.name) - } else { - suite.Require().Empty(addr) - suite.Require().Error(err, tc.name) - } - }) - } -} - -func (suite *TypesTestSuite) TestValidateVersion() { - testCases := []struct { - name string - version string - expPass bool - }{ - { - "success", - types.NewAppVersion(types.VersionPrefix, TestOwnerAddress), - true, - }, - { - "unexpected version string format", - "invalid-version-string-format", - false, - }, - { - "unexpected version string format, additional delimiter", - types.NewAppVersion(types.VersionPrefix, "cosmos17dtl0mjt3t77kpu.hg2edqzjpszulwhgzuj9ljs"), - false, - }, - { - "invalid version", - types.NewAppVersion("ics27-5", TestOwnerAddress), - false, - }, - { - "invalid account address - empty", - types.NewAppVersion(types.VersionPrefix, ""), - false, - }, - { - "invalid account address - exceeded character length", - types.NewAppVersion(types.VersionPrefix, "ofwafxhdmqcdbpzvrccxkidbunrwyyoboyctignpvthxbwxtmnzyfwhhywobaatltfwafxhdmqcdbpzvrccxkidbunrwyyoboyctignpvthxbwxtmnzyfwhhywobaatlt"), - false, - }, - { - "invalid account address - non alphanumeric characters", - types.NewAppVersion(types.VersionPrefix, "-_-"), - false, - }, - } - - for _, tc := range testCases { - suite.Run(tc.name, func() { - err := types.ValidateVersion(tc.version) - - if tc.expPass { - suite.Require().NoError(err, tc.name) - } else { - suite.Require().Error(err, tc.name) - } - }) - } -} diff --git a/proto/ibc/applications/interchain_accounts/v1/metadata.proto b/proto/ibc/applications/interchain_accounts/v1/metadata.proto new file mode 100644 index 00000000000..acc338466af --- /dev/null +++ b/proto/ibc/applications/interchain_accounts/v1/metadata.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package ibc.applications.interchain_accounts.v1; + +option go_package = "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types"; + +import "gogoproto/gogo.proto"; + +// Metadata defines a set of protocol specific data encoded into the ICS27 channel version bytestring +// See ICS004: https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#Versioning +message Metadata { + // version defines the ICS27 protocol version + string version = 1; + // controller_connection_id is the connection identifier associated with the controller chain + string controller_connection_id = 2 [(gogoproto.moretags) = "yaml:\"controller_connection_id\""]; + // host_connection_id is the connection identifier associated with the host chain + string host_connection_id = 3 [(gogoproto.moretags) = "yaml:\"host_connection_id\""]; + // address defines the interchain account address to be fulfilled upon the OnChanOpenTry handshake step + // NOTE: the address field is empty on the OnChanOpenInit handshake step + string address = 4; +}