diff --git a/modules/apps/27-interchain-accounts/keeper/account.go b/modules/apps/27-interchain-accounts/keeper/account.go index 43e3bc4b143..814e96d6dad 100644 --- a/modules/apps/27-interchain-accounts/keeper/account.go +++ b/modules/apps/27-interchain-accounts/keeper/account.go @@ -31,7 +31,7 @@ func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionID, counterpart return sdkerrors.Wrap(err, "unable to bind to newly generated portID") } - msg := channeltypes.NewMsgChannelOpenInit(portID, types.Version, channeltypes.ORDERED, []string{connectionID}, types.PortID, types.ModuleName) + msg := channeltypes.NewMsgChannelOpenInit(portID, types.VersionPrefix, channeltypes.ORDERED, []string{connectionID}, types.PortID, types.ModuleName) handler := k.msgRouter.Handler(msg) if _, err := handler(ctx, msg); err != nil { return err @@ -40,17 +40,15 @@ func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionID, counterpart return nil } -// Register interchain account if it has not already been created -func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, portID string) { - address := types.GenerateAddress(portID) - - account := k.accountKeeper.GetAccount(ctx, address) - if account != nil { +// RegisterInterchainAccount attempts to create a new account using the provided address and stores it in state keyed by the provided port identifier +// If an account for the provided address already exists this function returns early (no-op) +func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, accAddr sdk.AccAddress, portID string) { + if acc := k.accountKeeper.GetAccount(ctx, accAddr); acc != nil { return } interchainAccount := types.NewInterchainAccount( - authtypes.NewBaseAccountWithAddress(address), + authtypes.NewBaseAccountWithAddress(accAddr), portID, ) diff --git a/modules/apps/27-interchain-accounts/keeper/account_test.go b/modules/apps/27-interchain-accounts/keeper/account_test.go index 3d9cd9bae49..69ac5a5dc87 100644 --- a/modules/apps/27-interchain-accounts/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/keeper/account_test.go @@ -44,8 +44,8 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() { tc := tc suite.Run(tc.name, func() { - suite.SetupTest() // reset - owner = "owner" // must be explicitly changed + suite.SetupTest() // reset + owner = TestOwnerAddress // must be explicitly changed path = NewICAPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path) diff --git a/modules/apps/27-interchain-accounts/keeper/handshake.go b/modules/apps/27-interchain-accounts/keeper/handshake.go index e4677b72188..bccd1be1a16 100644 --- a/modules/apps/27-interchain-accounts/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/keeper/handshake.go @@ -36,8 +36,9 @@ func (k Keeper) OnChanOpenInit( if counterparty.PortId != types.PortID { return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "counterparty port-id must be '%s', (%s != %s)", types.PortID, counterparty.PortId, types.PortID) } - if version != types.Version { - return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelVersion, "channel version must be '%s' (%s != %s)", types.Version, version, types.Version) + + if err := types.ValidateVersion(version); err != nil { + return sdkerrors.Wrap(err, "version validation failed") } existingChannelID, found := k.GetActiveChannel(ctx, portID) @@ -71,11 +72,13 @@ func (k Keeper) OnChanOpenTry( if order != channeltypes.ORDERED { return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "invalid channel ordering: %s, expected %s", order.String(), channeltypes.ORDERED.String()) } - if version != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "got: %s, expected %s", version, types.Version) + + if err := types.ValidateVersion(version); err != nil { + return sdkerrors.Wrap(err, "version validation failed") } - if counterpartyVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: %s, expected %s", counterpartyVersion, types.Version) + + if err := types.ValidateVersion(counterpartyVersion); err != nil { + return sdkerrors.Wrap(err, "counterparty version validation failed") } // On the host chain the capability may only be claimed during the OnChanOpenTry @@ -84,23 +87,38 @@ func (k Keeper) OnChanOpenTry( return err } + // Check to ensure that the version string contains the expected address generated from the Counterparty portID + accAddr := types.GenerateAddress(counterparty.PortId) + parsedAddr := types.ParseAddressFromVersion(version) + if parsedAddr != accAddr.String() { + return sdkerrors.Wrapf(types.ErrInvalidAccountAddress, "version contains invalid account address: expected %s, got %s", parsedAddr, accAddr) + } + // Register interchain account if it does not already exist - k.RegisterInterchainAccount(ctx, counterparty.PortId) + k.RegisterInterchainAccount(ctx, accAddr, counterparty.PortId) + return nil } +// OnChanOpenAck sets the active channel for the interchain account/owner pair +// and stores the associated interchain account address in state keyed by it's corresponding port identifier +// +// Controller Chain func (k Keeper) OnChanOpenAck( ctx sdk.Context, portID, channelID string, counterpartyVersion string, ) error { - if counterpartyVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: %s, expected %s", counterpartyVersion, types.Version) + if err := types.ValidateVersion(counterpartyVersion); err != nil { + return sdkerrors.Wrap(err, "counterparty version validation failed") } k.SetActiveChannel(ctx, portID, channelID) + accAddr := types.ParseAddressFromVersion(counterpartyVersion) + k.SetInterchainAccountAddress(ctx, portID, accAddr) + return nil } diff --git a/modules/apps/27-interchain-accounts/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/keeper/handshake_test.go index 35f09372f76..d3ba9b20a31 100644 --- a/modules/apps/27-interchain-accounts/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/keeper/handshake_test.go @@ -62,7 +62,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { suite.coordinator.SetupConnections(path) // mock init interchain account - portID, err := types.GeneratePortID("owner", path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + portID, err := types.GeneratePortID(TestOwnerAddress, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) suite.Require().NoError(err) portCap := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), portID) suite.chainA.GetSimApp().ICAKeeper.ClaimCapability(suite.chainA.GetContext(), portCap, host.PortPath(portID)) @@ -75,7 +75,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointA.ConnectionID}, - Version: types.Version, + Version: types.VersionPrefix, } chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(portID, path.EndpointA.ChannelID)) @@ -136,6 +136,11 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { suite.Require().NoError(err) }, false, }, + { + "invalid account address", func() { + channel.Counterparty.PortId = "invalid-port-id" + }, false, + }, } for _, tc := range testCases { @@ -144,11 +149,10 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { suite.Run(tc.name, func() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) - owner := "owner" - counterpartyVersion = types.Version + counterpartyVersion = types.VersionPrefix suite.coordinator.SetupConnections(path) - err := InitInterchainAccount(path.EndpointA, owner) + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) suite.Require().NoError(err) // default values @@ -158,7 +162,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointB.ConnectionID}, - Version: types.Version, + Version: TestVersion, } chanCap, err = suite.chainB.App.GetScopedIBCKeeper().NewCapability(suite.chainB.GetContext(), host.ChannelCapabilityPath(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)) @@ -211,11 +215,10 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { suite.Run(tc.name, func() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) - owner := "owner" - counterpartyVersion = types.Version + counterpartyVersion = TestVersion suite.coordinator.SetupConnections(path) - err := InitInterchainAccount(path.EndpointA, owner) + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) suite.Require().NoError(err) err = path.EndpointB.ChanOpenTry() @@ -264,10 +267,9 @@ func (suite *KeeperTestSuite) TestOnChanOpenConfirm() { suite.Run(tc.name, func() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) - owner := "owner" suite.coordinator.SetupConnections(path) - err := InitInterchainAccount(path.EndpointA, owner) + err := InitInterchainAccount(path.EndpointA, TestOwnerAddress) suite.Require().NoError(err) err = path.EndpointB.ChanOpenTry() diff --git a/modules/apps/27-interchain-accounts/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/keeper/keeper_test.go index 7064d6ebc07..0f7c12d8189 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "testing" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -11,6 +12,15 @@ import ( ibctesting "github.com/cosmos/ibc-go/testing" ) +var ( + // TestOwnerAddress defines a reusable bech32 address for testing purposes + TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes + TestPortID = fmt.Sprintf("ics-27-0-0-%s", TestOwnerAddress) + // TestVersion defines a resuable interchainaccounts version string for testing purposes + TestVersion = types.NewAppVersion(types.VersionPrefix, types.GenerateAddress(TestPortID).String()) +) + type KeeperTestSuite struct { suite.Suite @@ -35,8 +45,8 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path.EndpointB.ChannelConfig.PortID = types.PortID path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointA.ChannelConfig.Version = types.Version - path.EndpointB.ChannelConfig.Version = types.Version + path.EndpointA.ChannelConfig.Version = types.VersionPrefix + path.EndpointB.ChannelConfig.Version = TestVersion return path } @@ -104,7 +114,7 @@ func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() { path := NewICAPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path) - err := SetupICAPath(path, "testing") + err := SetupICAPath(path, TestOwnerAddress) suite.Require().NoError(err) counterpartyPortID := path.EndpointA.ChannelConfig.PortID diff --git a/modules/apps/27-interchain-accounts/keeper/relay_test.go b/modules/apps/27-interchain-accounts/keeper/relay_test.go index 66c3b22d51a..ea836fb8c05 100644 --- a/modules/apps/27-interchain-accounts/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/keeper/relay_test.go @@ -74,7 +74,7 @@ func (suite *KeeperTestSuite) TestTrySendTx() { suite.Run(tc.name, func() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) - owner := "owner" + owner := TestOwnerAddress suite.coordinator.SetupConnections(path) err := suite.SetupICAPath(path, owner) diff --git a/modules/apps/27-interchain-accounts/module.go b/modules/apps/27-interchain-accounts/module.go index 740eea4ad6b..efa808f8d01 100644 --- a/modules/apps/27-interchain-accounts/module.go +++ b/modules/apps/27-interchain-accounts/module.go @@ -255,6 +255,18 @@ func (am AppModule) OnTimeoutPacket( return nil } -func (am AppModule) NegotiateAppVersion(ctx sdk.Context, order channeltypes.Order, connectionID, portID string, counterparty channeltypes.Counterparty, proposedVersion string) (string, error) { - return "", nil +// NegotiateAppVersion implements the IBCModule interface +func (am AppModule) NegotiateAppVersion( + ctx sdk.Context, + order channeltypes.Order, + connectionID string, + portID string, + counterparty channeltypes.Counterparty, + proposedVersion string, +) (string, error) { + if proposedVersion != types.VersionPrefix { + return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "failed to negotiate app version: expected %s, got %s", types.VersionPrefix, proposedVersion) + } + + return types.NewAppVersion(types.VersionPrefix, types.GenerateAddress(counterparty.PortId).String()), nil } diff --git a/modules/apps/27-interchain-accounts/module_test.go b/modules/apps/27-interchain-accounts/module_test.go new file mode 100644 index 00000000000..fbfea84a189 --- /dev/null +++ b/modules/apps/27-interchain-accounts/module_test.go @@ -0,0 +1,80 @@ +package interchain_accounts_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/suite" + + "github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" + ibctesting "github.com/cosmos/ibc-go/testing" +) + +var ( + // TestOwnerAddress defines a reusable bech32 address for testing purposes + TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + // TestPortID defines a resuable port identifier for testing purposes + TestPortID = fmt.Sprintf("ics-27-0-0-%s", TestOwnerAddress) + // TestVersion defines a resuable interchainaccounts version string for testing purposes + TestVersion = types.NewAppVersion(types.VersionPrefix, types.GenerateAddress(TestPortID).String()) +) + +type InterchainAccountsTestSuite struct { + suite.Suite + + coordinator *ibctesting.Coordinator + + // testing chains used for convenience and readability + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain + chainC *ibctesting.TestChain +} + +func (suite *InterchainAccountsTestSuite) SetupTest() { + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3) + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0)) + suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(2)) +} + +func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { + path := ibctesting.NewPath(chainA, chainB) + path.EndpointA.ChannelConfig.PortID = types.PortID + path.EndpointB.ChannelConfig.PortID = types.PortID + path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED + path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED + path.EndpointA.ChannelConfig.Version = types.VersionPrefix + path.EndpointB.ChannelConfig.Version = TestVersion + + return path +} + +func TestICATestSuite(t *testing.T) { + suite.Run(t, new(InterchainAccountsTestSuite)) +} + +func (suite *InterchainAccountsTestSuite) TestNegotiateAppVersion() { + suite.SetupTest() + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + module, _, err := suite.chainA.GetSimApp().GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), types.PortID) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.GetSimApp().GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + counterpartyPortID, err := types.GeneratePortID(TestOwnerAddress, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + suite.Require().NoError(err) + + counterparty := &channeltypes.Counterparty{ + PortId: counterpartyPortID, + ChannelId: path.EndpointB.ChannelID, + } + + version, err := cbs.NegotiateAppVersion(suite.chainA.GetContext(), channeltypes.ORDERED, path.EndpointA.ConnectionID, types.PortID, *counterparty, types.VersionPrefix) + suite.Require().NoError(err) + suite.Require().NoError(types.ValidateVersion(version)) + suite.Require().Equal(TestVersion, version) +} diff --git a/modules/apps/27-interchain-accounts/types/account.go b/modules/apps/27-interchain-accounts/types/account.go index 93f8b18e9a4..7948e2e6afa 100644 --- a/modules/apps/27-interchain-accounts/types/account.go +++ b/modules/apps/27-interchain-accounts/types/account.go @@ -5,13 +5,12 @@ import ( "fmt" "strings" - yaml "gopkg.in/yaml.v2" - crypto "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/tendermint/tendermint/crypto/tmhash" + yaml "gopkg.in/yaml.v2" connectiontypes "github.com/cosmos/ibc-go/modules/core/03-connection/types" ) @@ -20,9 +19,14 @@ const ( ICAPrefix string = "ics-27" ) -// GenerateAddress returns a truncated SHA256 hash using the provided port string -func GenerateAddress(port string) []byte { - return tmhash.SumTruncated([]byte(port)) +// GenerateAddress returns an sdk.AccAddress using the provided port identifier +func GenerateAddress(portID string) sdk.AccAddress { + return sdk.AccAddress(tmhash.SumTruncated([]byte(portID))) +} + +// ParseAddressFromVersion trims the interchainaccounts version prefix and returns the associated account address +func ParseAddressFromVersion(version string) string { + return strings.TrimPrefix(version, fmt.Sprint(VersionPrefix, Delimiter)) } // GeneratePortID generates the portID for a specific owner @@ -32,21 +36,21 @@ func GenerateAddress(port string) []byte { // 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) { - ownerID := strings.TrimSpace(owner) - if ownerID == "" { - return "", sdkerrors.Wrap(ErrInvalidOwnerAddress, "owner address cannot be empty") + 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") } - portID := fmt.Sprintf("%s-%d-%d-%s", ICAPrefix, connectionSeq, counterpartyConnectionSeq, ownerID) - return portID, nil + return fmt.Sprintf("%s-%d-%d-%s", ICAPrefix, connectionSeq, counterpartyConnectionSeq, owner), nil } type InterchainAccountI interface { diff --git a/modules/apps/27-interchain-accounts/types/account_test.go b/modules/apps/27-interchain-accounts/types/account_test.go index 15bc23eb3e8..8f3bbc1126b 100644 --- a/modules/apps/27-interchain-accounts/types/account_test.go +++ b/modules/apps/27-interchain-accounts/types/account_test.go @@ -1,12 +1,19 @@ package types_test import ( + "fmt" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + "github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types" - channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" - "github.com/stretchr/testify/suite" +) + +var ( + // TestOwnerAddress defines a reusable bech32 address for testing purposes + TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" ) type TypesTestSuite struct { @@ -25,59 +32,81 @@ func (suite *TypesTestSuite) SetupTest() { suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) } -func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { - path := ibctesting.NewPath(chainA, chainB) - path.EndpointA.ChannelConfig.PortID = types.PortID - path.EndpointB.ChannelConfig.PortID = types.PortID - path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED - path.EndpointA.ChannelConfig.Version = types.Version - path.EndpointB.ChannelConfig.Version = types.Version - - return path -} - func TestTypesTestSuite(t *testing.T) { suite.Run(t, new(TypesTestSuite)) } +func (suite *TypesTestSuite) TestGenerateAddress() { + addr := types.GenerateAddress("test-port-id") + accAddr, err := sdk.AccAddressFromBech32(addr.String()) + + suite.Require().NoError(err, "TestGenerateAddress failed") + suite.Require().NotEmpty(accAddr) +} + func (suite *TypesTestSuite) TestGeneratePortID() { var ( path *ibctesting.Path - owner string + owner = TestOwnerAddress ) - var testCases = []struct { + + testCases := []struct { name string malleate func() expValue string expPass bool }{ - {"success", func() {}, "ics-27-0-0-owner123", true}, - {"success with non matching connection sequences", func() { - path.EndpointA.ConnectionID = "connection-1" - }, "ics-27-1-0-owner123", true}, - {"invalid owner address", func() { - owner = " " - }, "", false}, - {"invalid connectionID", func() { - path.EndpointA.ConnectionID = "connection" - }, "", false}, - {"invalid counterparty connectionID", func() { - path.EndpointB.ConnectionID = "connection" - }, "", false}, + { + "success", + func() {}, + fmt.Sprintf("ics-27-0-0-%s", TestOwnerAddress), + true, + }, + { + "success with non matching connection sequences", + func() { + path.EndpointA.ConnectionID = "connection-1" + }, + fmt.Sprintf("ics-27-1-0-%s", TestOwnerAddress), + true, + }, + { + "invalid owner address", + func() { + owner = " " + }, + "", + false, + }, + { + "invalid connectionID", + func() { + path.EndpointA.ConnectionID = "connection" + }, + "", + false, + }, + { + "invalid counterparty connectionID", + func() { + path.EndpointB.ConnectionID = "connection" + }, + "", + false, + }, } for _, tc := range testCases { tc := tc suite.Run(tc.name, func() { suite.SetupTest() // reset - path = NewICAPath(suite.chainA, suite.chainB) + path = ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) - owner = "owner123" // must be explicitly changed tc.malleate() portID, err := types.GeneratePortID(owner, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + if tc.expPass { suite.Require().NoError(err, tc.name) suite.Require().Equal(tc.expValue, portID) diff --git a/modules/apps/27-interchain-accounts/types/errors.go b/modules/apps/27-interchain-accounts/types/errors.go index 32425a81071..5c5bf3fed44 100644 --- a/modules/apps/27-interchain-accounts/types/errors.go +++ b/modules/apps/27-interchain-accounts/types/errors.go @@ -11,9 +11,9 @@ var ( ErrUnsupportedChain = sdkerrors.Register(ModuleName, 5, "unsupported chain") ErrInvalidOutgoingData = sdkerrors.Register(ModuleName, 6, "invalid outgoing data") ErrInvalidRoute = sdkerrors.Register(ModuleName, 7, "invalid route") - ErrInterchainAccountNotFound = sdkerrors.Register(ModuleName, 8, "Interchain Account not found") - ErrInterchainAccountAlreadySet = sdkerrors.Register(ModuleName, 9, "Interchain Account is already set") + ErrInterchainAccountNotFound = sdkerrors.Register(ModuleName, 8, "interchain account not found") + ErrInterchainAccountAlreadySet = sdkerrors.Register(ModuleName, 9, "interchain account is already set") ErrActiveChannelNotFound = sdkerrors.Register(ModuleName, 10, "no active channel for this owner") ErrInvalidVersion = sdkerrors.Register(ModuleName, 11, "invalid interchain accounts version") - ErrInvalidOwnerAddress = sdkerrors.Register(ModuleName, 12, "invalid owner address") + ErrInvalidAccountAddress = sdkerrors.Register(ModuleName, 12, "invalid account address") ) diff --git a/modules/apps/27-interchain-accounts/types/keys.go b/modules/apps/27-interchain-accounts/types/keys.go index 909813ec46d..51841d0de0b 100644 --- a/modules/apps/27-interchain-accounts/types/keys.go +++ b/modules/apps/27-interchain-accounts/types/keys.go @@ -1,41 +1,48 @@ package types -import "fmt" +import ( + "fmt" +) const ( - // ModuleName defines the Interchain Account module name + // ModuleName defines the interchain accounts module name ModuleName = "interchainaccounts" - // Version defines the current version the IBC tranfer - // module supports - Version = "ics27-1" + // 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 = "ibcaccount" - StoreKey = ModuleName - RouterKey = ModuleName + // StoreKey is the store key string for interchain accounts + StoreKey = ModuleName - // Key to store portID in our store - PortKey = "portID" + // RouterKey is the message route for interchain accounts + RouterKey = ModuleName + // QuerierRoute is the querier route for interchain accounts QuerierRoute = ModuleName - // MemStoreKey defines the in-memory store key - MemStoreKey = "mem_capability" + // Delimiter is the delimiter used for the interchain accounts version string + Delimiter = "|" ) +var ( + // PortKey defines the key to store the port ID in store + PortKey = []byte{0x01} +) + +// NewVersion returns a complete version string in the format: VersionPrefix + Delimter + AccAddress +func NewAppVersion(versionPrefix, accAddr string) string { + return fmt.Sprint(versionPrefix, Delimiter, accAddr) +} + +// KeyActiveChannel creates and returns a new key used for active channels store operations func KeyActiveChannel(portId string) []byte { return []byte(fmt.Sprintf("activeChannel/%s", portId)) } +// KeyOwnerAccount creates and returns a new key used for owner account store operations func KeyOwnerAccount(portId string) []byte { return []byte(fmt.Sprintf("owner/%s", portId)) } - -var ( - KeyPrefixRegisteredAccount = []byte("register") -) - -func GetIdentifier(portID, channelID string) string { - return fmt.Sprintf("%s/%s/", portID, channelID) -} diff --git a/modules/apps/27-interchain-accounts/types/validate.go b/modules/apps/27-interchain-accounts/types/validate.go new file mode 100644 index 00000000000..691eb369e41 --- /dev/null +++ b/modules/apps/27-interchain-accounts/types/validate.go @@ -0,0 +1,44 @@ +package types + +import ( + "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 + +// 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 optional account address. +// When no delimiter is present, only the version prefix is validated +func ValidateVersion(version string) error { + s := strings.Split(version, Delimiter) + + if s[0] != VersionPrefix { + return sdkerrors.Wrapf(ErrInvalidVersion, "expected %s, got %s", VersionPrefix, s[0]) + } + + if len(s) > 1 { + if len(s) != 2 { + return sdkerrors.Wrap(ErrInvalidAccountAddress, "unexpected address format") + } + + if !IsValidAddr(s[1]) || len(s[1]) == 0 || len(s[1]) > 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/validate_test.go b/modules/apps/27-interchain-accounts/types/validate_test.go new file mode 100644 index 00000000000..aa1bb33c8ae --- /dev/null +++ b/modules/apps/27-interchain-accounts/types/validate_test.go @@ -0,0 +1,61 @@ +package types_test + +import ( + "github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types" +) + +func (suite *TypesTestSuite) TestValidateVersion() { + testCases := []struct { + name string + version string + expPass bool + }{ + { + "success", + types.NewAppVersion(types.VersionPrefix, TestOwnerAddress), + true, + }, + { + "success - version prefix only", + types.VersionPrefix, + true, + }, + { + "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, + }, + { + "invalid account address - address contains additional delimiter", + types.NewAppVersion(types.VersionPrefix, "cosmos17dtl0mjt3t77kpu|hg2edqzjpszulwhgzuj9ljs"), + 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) + } + }) + } +}