updating Version -> VersionPrefix, error msgs, validation tests

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

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

@@ -1,8 +1,6 @@
 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"
@@ -91,8 +89,8 @@ func (k Keeper) OnChanOpenTry(
 
 	accAddr := types.GenerateAddress(counterparty.PortId)
 	parsedAddr := types.ParseAddressFromVersion(version)
-	if strings.Compare(parsedAddr, accAddr.String()) != 0 {
-		return sdkerrors.Wrapf(types.ErrInvalidAccountAddress, "invalid account address: %s, expected %s", parsedAddr, accAddr)
+	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

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

@@ -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))
@@ -149,7 +149,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
 			path = NewICAPath(suite.chainA, suite.chainB)
-			counterpartyVersion = types.Version
+			counterpartyVersion = types.VersionPrefix
 			suite.coordinator.SetupConnections(path)
 
 			err := InitInterchainAccount(path.EndpointA, TestOwnerAddress)

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

@@ -15,8 +15,10 @@ import (
 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 = fmt.Sprintf("%s|%s", types.Version, types.GenerateAddress("ics-27-0-0-"+TestOwnerAddress))
+	TestVersion = fmt.Sprintf("%s|%s", types.VersionPrefix, types.GenerateAddress(TestPortID))
 )
 
 type KeeperTestSuite struct {
@@ -43,7 +45,7 @@ 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.EndpointA.ChannelConfig.Version = types.VersionPrefix
 	path.EndpointB.ChannelConfig.Version = TestVersion
 
 	return path

modules/apps/27-interchain-accounts/module.go

@@ -264,9 +264,9 @@ func (am AppModule) NegotiateAppVersion(
 	counterparty channeltypes.Counterparty,
 	proposedVersion string,
 ) (string, error) {
-	if proposedVersion != types.Version {
-		return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "failed to negotiate app version: expected %s, got %s", types.Version, proposedVersion)
+	if proposedVersion != types.VersionPrefix {
+		return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "failed to negotiate app version: expected %s, got %s", types.VersionPrefix, proposedVersion)
 	}
 
-	return fmt.Sprint(types.Version, types.Delimiter, types.GenerateAddress(counterparty.PortId)), nil
+	return types.NewAppVersion(types.GenerateAddress(counterparty.PortId).String()), nil
 }

modules/apps/27-interchain-accounts/module_test.go

@@ -14,8 +14,10 @@ import (
 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 = fmt.Sprintf("%s|%s", types.Version, types.GenerateAddress("ics-27-0-0-"+TestOwnerAddress))
+	TestVersion = fmt.Sprintf("%s|%s", types.VersionPrefix, types.GenerateAddress(TestPortID))
 )
 
 type InterchainAccountsTestSuite struct {
@@ -42,7 +44,7 @@ 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.EndpointA.ChannelConfig.Version = types.VersionPrefix
 	path.EndpointB.ChannelConfig.Version = TestVersion
 
 	return path
@@ -71,7 +73,8 @@ func (suite *InterchainAccountsTestSuite) TestNegotiateAppVersion() {
 		ChannelId: path.EndpointB.ChannelID,
 	}
 
-	version, err := cbs.NegotiateAppVersion(suite.chainA.GetContext(), channeltypes.ORDERED, path.EndpointA.ConnectionID, types.PortID, *counterparty, types.Version)
+	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)
 }

modules/apps/27-interchain-accounts/types/account.go

@@ -26,7 +26,7 @@ func GenerateAddress(portID string) sdk.AccAddress {
 
 // ParseAddressFromVersion trims the interchainaccounts version prefix and returns the associated account address
 func ParseAddressFromVersion(version string) string {
-	return strings.TrimPrefix(version, fmt.Sprint(Version, Delimiter))
+	return strings.TrimPrefix(version, fmt.Sprint(VersionPrefix, Delimiter))
 }
 
 // GeneratePortID generates the portID for a specific owner

modules/apps/27-interchain-accounts/types/keys.go

@@ -5,42 +5,44 @@ import (
 )
 
 const (
-	// ModuleName defines the Interchain Account module name
+	// ModuleName defines the interchain accounts module name
 	ModuleName = "interchainaccounts"
 
-	// Version defines the current version the IBC interchainaccounts
-	// module supports
-	Version = "ics27-1"
+	// VersionPrefix defines the current version for interchain accounts
+	VersionPrefix = "ics27-1"
 
-	// PortID is the default port id that the interchainaccounts module binds to
+	// PortID is the default port id that the interchain accounts module binds to
 	PortID = "ibcaccount"
 
-	// StoreKey is the store key string for IBC interchainaccounts
+	// StoreKey is the store key string for interchain accounts
 	StoreKey = ModuleName
 
-	// RouterKey is the message route for IBC interchainaccounts
+	// RouterKey is the message route for interchain accounts
 	RouterKey = ModuleName
 
-	// QuerierRoute is the querier route for IBC interchainaccounts
+	// QuerierRoute is the querier route for interchain accounts
 	QuerierRoute = ModuleName
 
-	// Delimiter is the delimiter used for the interchainaccounts version string
+	// 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 = "portID"
+	PortKey = []byte{0x01}
 )
 
+// NewVersion returns a complete version string in the format: VersionPrefix + Delimter + AccAddress
+func NewAppVersion(addr string) string {
+	return fmt.Sprint(VersionPrefix, Delimiter, addr)
+}
+
+// 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))
 }
-
-func GetIdentifier(portID, channelID string) string {
-	return fmt.Sprintf("%s/%s/", portID, channelID)
-}

modules/apps/27-interchain-accounts/types/validate.go

@@ -8,10 +8,7 @@ import (
 )
 
 // DefaultMaxAddrLength defines the default maximum character length used in validation of addresses
-var DefaultMaxAddrLength = 64
-
-// DefaultMinAddrLength defines the default minimum character length used in validation of addresses
-var DefaultMinAddrLength = 32
+var DefaultMaxAddrLength = 128
 
 // IsValidAddr defines a regular expression to check if the provided string consists of
 // strictly alphanumeric characters
@@ -22,16 +19,19 @@ var IsValidAddr = regexp.MustCompile("^[a-zA-Z0-9]*$").MatchString
 func ValidateVersion(version string) error {
 	s := strings.Split(version, Delimiter)
 
-	if s[0] != Version {
-		return sdkerrors.Wrapf(ErrInvalidVersion, "invalid version: expected %s, got %s", Version, s[0])
+	if s[0] != VersionPrefix {
+		return sdkerrors.Wrapf(ErrInvalidVersion, "expected %s, got %s", VersionPrefix, s[0])
 	}
 
 	if len(s) > 1 {
-		if !IsValidAddr(s[1]) || len(s[1]) > DefaultMaxAddrLength || len(s[1]) < DefaultMinAddrLength {
+		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, %d-%d characters in length",
-				DefaultMinAddrLength,
+				"address must contain strictly alphanumeric characters, not exceeding %d characters in length",
 				DefaultMaxAddrLength,
 			)
 		}

modules/apps/27-interchain-accounts/types/validate_test.go

@@ -14,12 +14,12 @@ func (suite *TypesTestSuite) TestValidateVersion() {
 	}{
 		{
 			"success",
-			fmt.Sprint(types.Version, types.Delimiter, TestOwnerAddress),
+			fmt.Sprint(types.VersionPrefix, types.Delimiter, TestOwnerAddress),
 			true,
 		},
 		{
 			"success - version only",
-			fmt.Sprint(types.Version),
+			fmt.Sprint(types.VersionPrefix),
 			true,
 		},
 		{
@@ -28,18 +28,23 @@ func (suite *TypesTestSuite) TestValidateVersion() {
 			false,
 		},
 		{
-			"invalid account address - 31 chars",
-			fmt.Sprint(types.Version, types.Delimiter, "xtignpvthxbwxtmnzyfwhhywobaatlt"),
+			"invalid account address - empty",
+			fmt.Sprint(types.VersionPrefix, types.Delimiter, ""),
 			false,
 		},
 		{
-			"invalid account address - 65 chars",
-			fmt.Sprint(types.Version, types.Delimiter, "ofwafxhdmqcdbpzvrccxkidbunrwyyoboyctignpvthxbwxtmnzyfwhhywobaatlt"),
+			"invalid account address - exceeded character length",
+			fmt.Sprint(types.VersionPrefix, types.Delimiter, "ofwafxhdmqcdbpzvrccxkidbunrwyyoboyctignpvthxbwxtmnzyfwhhywobaatltfwafxhdmqcdbpzvrccxkidbunrwyyoboyctignpvthxbwxtmnzyfwhhywobaatlt"),
 			false,
 		},
 		{
 			"invalid account address - non alphanumeric characters",
-			fmt.Sprint(types.Version, types.Delimiter, "-_-"),
+			fmt.Sprint(types.VersionPrefix, types.Delimiter, "-_-"),
+			false,
+		},
+		{
+			"invalid account address - address contains additional delimiter",
+			fmt.Sprint(types.VersionPrefix, types.Delimiter, "cosmos17dtl0mjt3t77kpu|hg2edqzjpszulwhgzuj9ljs"),
 			false,
 		},
 	}