Use generated client identifiers

x/ibc/core/02-client/keeper/client_test.go

@@ -19,29 +19,23 @@ import (
 
 func (suite *KeeperTestSuite) TestCreateClient() {
 	cases := []struct {
-		msg      string
-		clientID string
-		expPass  bool
+		msg         string
+		clientState exported.ClientState
+		expPass     bool
 	}{
-		{"success", testClientID, true},
+		{"success", ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), true},
+		{"client type not supported", localhosttypes.NewClientState(testChainID, clienttypes.NewHeight(0, 1)), false},
 	}
 
 	for i, tc := range cases {
-		tc := tc
-		i := i
 
-		clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
-		if tc.expPass {
-			suite.Require().NotNil(clientState, "valid test case %d failed: %s", i, tc.msg)
-		}
-		// If we were able to NewClientState clientstate successfully, try persisting it to state
-		clientID, err := suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
+		clientID, err := suite.keeper.CreateClient(suite.ctx, tc.clientState, suite.consensusState)
 		if tc.expPass {
 			suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg)
 			suite.Require().NotNil(clientID, "valid test case %d failed: %s", i, tc.msg)
 		} else {
 			suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg)
-			suite.Require().Nil(clientID, "invalid test case %d passed: %s", i, tc.msg)
+			suite.Require().Equal("", clientID, "invalid test case %d passed: %s", i, tc.msg)
 		}
 	}
 }

x/ibc/core/02-client/types/height_test.go

@@ -112,6 +112,7 @@ func TestParseChainID(t *testing.T) {
 		{"gaiamainnet--4", 0, false},
 		{"gaiamainnet-3.4", 0, false},
 		{"gaiamainnet", 0, false},
+		{"a--1", 0, false},
 	}
 
 	for i, tc := range cases {

x/ibc/core/02-client/types/keys.go

@@ -25,18 +25,26 @@ const (
 	KeyNextClientSequence = "nextClientSequence"
 )
 
-// IsValidClientID checks if a clientID is in the format required for parsing client
-// identifier. The client identifier must be in the form: `{client-type}-{N}
-var IsValidClientID = regexp.MustCompile(`^.*[^-]-[0-9]{1,20}$`).MatchString
-
 // FormatClientIdentifier returns the client identifier with the sequence appended.
+// This is a SDK specific format.
 func FormatClientIdentifier(clientType string, sequence uint64) string {
 	return fmt.Sprintf("%s-%d", clientType, sequence)
 }
 
+// IsClientIDFormat checks if a clientID is in the format required on the SDK for
+// parsing client identifiers. The client identifier must be in the form: `{client-type}-{N}
+var IsClientIDFormat = regexp.MustCompile(`^.*[^-]-[0-9]{1,20}$`).MatchString
+
+// IsValidClientID checks if the clientID is valid and can be parsed into the client
+// identifier format.
+func IsValidClientID(clientID string) bool {
+	_, _, err := ParseClientIdentifier(clientID)
+	return err == nil
+}
+
 // ParseClientIdentifier parses the client type and sequence from the client identifier.
 func ParseClientIdentifier(clientID string) (string, uint64, error) {
-	if !IsValidClientID(clientID) {
+	if !IsClientIDFormat(clientID) {
 		return "", 0, sdkerrors.Wrapf(host.ErrInvalidID, "invalid client identifier %s is not in format: `{client-type}-{N}`", clientID)
 	}
 

x/ibc/core/02-client/types/keys_test.go

@@ -22,12 +22,16 @@ func TestParseClientIdentifier(t *testing.T) {
 		{"valid solemachine", "solomachine-v1-1", "solomachine-v1", 1, true},
 		{"valid large sequence", types.FormatClientIdentifier("tendermint", math.MaxUint64), "tendermint", math.MaxUint64, true},
 		{"valid short client type", "t-0", "t", 0, true},
+		// one above uint64 max
+		{"invalid uint64", "tendermint-18446744073709551616", "tendermint", 0, false},
 		// uint64 == 20 characters
 		{"invalid large sequence", "tendermint-2345682193567182931243", "tendermint", 0, false},
 		{"missing dash", "tendermint0", "tendermint", 0, false},
 		{"blank id", "               ", "    ", 0, false},
 		{"empty id", "", "", 0, false},
 		{"negative sequence", "tendermint--1", "tendermint", 0, false},
+		{"invalid format", "tendermint-tm", "tendermint", 0, false},
+		{"empty clientype", " -100", "tendermint", 0, false},
 	}
 
 	for _, tc := range testCases {
@@ -41,7 +45,7 @@ func TestParseClientIdentifier(t *testing.T) {
 			require.True(t, valid)
 			require.Equal(t, tc.clientType, clientType)
 		} else {
-			require.Error(t, err, tc.name)
+			require.Error(t, err, tc.name, tc.clientID)
 			require.False(t, valid)
 			require.Equal(t, "", clientType)
 		}

x/ibc/core/02-client/types/msgs_test.go

@@ -165,6 +165,16 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() {
 			},
 			false,
 		},
+		{
+			"invalid - client state and consensus state client types do not match",
+			func() {
+				tendermintClient := ibctmtypes.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
+				soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2)
+				msg, err = types.NewMsgCreateClient(tendermintClient, soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress())
+				suite.Require().NoError(err)
+			},
+			false,
+		},
 	}
 
 	for _, tc := range cases {

x/ibc/core/03-connection/types/keys.go

@@ -29,17 +29,29 @@ const (
 	ConnectionPrefix = "connection-"
 )
 
-// IsValidConnectionID checks if a connectionID is in the format required for parsing connection
-// identifier. The connection identifier must be in the form: `connection-{N}
-var IsValidConnectionID = regexp.MustCompile(`^connection-[0-9]{1,20}$`).MatchString
-
 // FormatConnectionIdentifier returns the connection identifier with the sequence appended.
+// This is a SDK specific format.
 func FormatConnectionIdentifier(sequence uint64) string {
 	return fmt.Sprintf("%s%d", ConnectionPrefix, sequence)
 }
 
+// IsConnectionIDFormat checks if a connectionID is in the format required on the SDK for
+// parsing connection identifiers. The connection identifier must be in the form: `connection-{N}
+var IsConnectionIDFormat = regexp.MustCompile(`^connection-[0-9]{1,20}$`).MatchString
+
+// IsValidConnectionID checks if the connection identifier is valid and can be parsed to
+// the connection identifier format.
+func IsValidConnectionID(connectionID string) bool {
+	_, err := ParseConnectionSequence(connectionID)
+	return err == nil
+}
+
 // ParseConnectionSequence parses the connection sequence from the connection identifier.
 func ParseConnectionSequence(connectionID string) (uint64, error) {
+	if !IsConnectionIDFormat(connectionID) {
+		return 0, sdkerrors.Wrap(host.ErrInvalidID, "connection identifier is not in the format: `connection-{N}`")
+	}
+
 	sequence, err := host.ParseIdentifier(connectionID, ConnectionPrefix)
 	if err != nil {
 		return 0, sdkerrors.Wrap(err, "invalid connection identifier")

x/ibc/core/03-connection/types/keys_test.go

@@ -19,6 +19,8 @@ func TestParseConnectionSequence(t *testing.T) {
 		{"valid 0", "connection-0", 0, true},
 		{"valid 1", "connection-1", 1, true},
 		{"valid large sequence", types.FormatConnectionIdentifier(math.MaxUint64), math.MaxUint64, true},
+		// one above uint64 max
+		{"invalid uint64", "connection-18446744073709551616", 0, false},
 		// uint64 == 20 characters
 		{"invalid large sequence", "connection-2345682193567182931243", 0, false},
 		{"capital prefix", "Connection-0", 0, false},

x/ibc/core/04-channel/types/keys.go

@@ -29,17 +29,29 @@ const (
 	ChannelPrefix = "channel-"
 )
 
-// IsValidChannelID checks if a channelID is in the format required for parsing channel
-// identifier. The channel identifier must be in the form: `connection-{N}
-var IsValidChannelID = regexp.MustCompile(`^channel-[0-9]{1,20}$`).MatchString
-
 // FormatChannelIdentifier returns the channel identifier with the sequence appended.
+// This is a SDK specific format.
 func FormatChannelIdentifier(sequence uint64) string {
 	return fmt.Sprintf("%s%d", ChannelPrefix, sequence)
 }
 
+// IsChannelIDFormat checks if a channelID is in the format required on the SDK for
+// parsing channel identifiers. The channel identifier must be in the form: `channel-{N}
+var IsChannelIDFormat = regexp.MustCompile(`^channel-[0-9]{1,20}$`).MatchString
+
+// IsValidChannelID checks if a channelID is valid and can be parsed to the channel
+// identifier format.
+func IsValidChannelID(channelID string) bool {
+	_, err := ParseChannelSequence(channelID)
+	return err == nil
+}
+
 // ParseChannelSequence parses the channel sequence from the channel identifier.
 func ParseChannelSequence(channelID string) (uint64, error) {
+	if !IsChannelIDFormat(channelID) {
+		return 0, sdkerrors.Wrap(host.ErrInvalidID, "channel identifier is not in the format: `channel-{N}`")
+	}
+
 	sequence, err := host.ParseIdentifier(channelID, ChannelPrefix)
 	if err != nil {
 		return 0, sdkerrors.Wrap(err, "invalid channel identifier")

x/ibc/core/04-channel/types/keys_test.go

@@ -18,6 +18,8 @@ func TestParseChannelSequence(t *testing.T) {
 		{"valid 0", "channel-0", 0, true},
 		{"valid 1", "channel-1", 1, true},
 		{"valid large sequence", "channel-234568219356718293", 234568219356718293, true},
+		// one above uint64 max
+		{"invalid uint64", "channel-18446744073709551616", 0, false},
 		// uint64 == 20 characters
 		{"invalid large sequence", "channel-2345682193567182931243", 0, false},
 		{"capital prefix", "Channel-0", 0, false},

x/ibc/core/24-host/parse.go

@@ -7,7 +7,9 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
-// ParseIdentifier parses the sequence from the identifier using the provided prefix.
+// ParseIdentifier parses the sequence from the identifier using the provided prefix. This function
+// does not need to be used by counterparty chains. SDK generated connection and channel identifiers
+// are required to use this format.
 func ParseIdentifier(identifier, prefix string) (uint64, error) {
 	if !strings.HasPrefix(identifier, prefix) {
 		return 0, sdkerrors.Wrapf(ErrInvalidID, "identifier doesn't contain prefix `%s`", prefix)

x/ibc/core/24-host/parse_test.go

@@ -0,0 +1,47 @@
+package host_test
+
+import (
+	"math"
+	"testing"
+
+	connectiontypes "github.com/cosmos/cosmos-sdk/x/ibc/core/03-connection/types"
+	host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseIdentifier(t *testing.T) {
+	testCases := []struct {
+		name       string
+		identifier string
+		prefix     string
+		expSeq     uint64
+		expPass    bool
+	}{
+		{"valid 0", "connection-0", "connection-", 0, true},
+		{"valid 1", "connection-1", "connection-", 1, true},
+		{"valid large sequence", connectiontypes.FormatConnectionIdentifier(math.MaxUint64), "connection-", math.MaxUint64, true},
+		// one above uint64 max
+		{"invalid uint64", "connection-18446744073709551616", "connection-", 0, false},
+		// uint64 == 20 characters
+		{"invalid large sequence", "connection-2345682193567182931243", "conenction-", 0, false},
+		{"capital prefix", "Connection-0", "connection-", 0, false},
+		{"double prefix", "connection-connection-0", "connection-", 0, false},
+		{"doesn't have prefix", "connection-0", "prefix", 0, false},
+		{"missing dash", "connection0", "connection-", 0, false},
+		{"blank id", "               ", "connection-", 0, false},
+		{"empty id", "", "connection-", 0, false},
+		{"negative sequence", "connection--1", "connection-", 0, false},
+	}
+
+	for _, tc := range testCases {
+
+		seq, err := host.ParseIdentifier(tc.identifier, tc.prefix)
+		require.Equal(t, tc.expSeq, seq)
+
+		if tc.expPass {
+			require.NoError(t, err, tc.name)
+		} else {
+			require.Error(t, err, tc.name)
+		}
+	}
+}

x/ibc/testing/chain.go

@@ -369,7 +369,7 @@ func (chain *TestChain) GetPrefix() commitmenttypes.MerklePrefix {
 
 // NewClientID appends a new clientID string in the format:
 // ClientFor<counterparty-chain-id><index>
-func (chain *TestChain) NewClientID(clientType, counterpartyChainID string) string {
+func (chain *TestChain) NewClientID(clientType string) string {
 	clientID := fmt.Sprintf("%s-%s", clientType, strconv.Itoa(len(chain.ClientIDs)))
 	chain.ClientIDs = append(chain.ClientIDs, clientID)
 	return clientID

x/ibc/testing/coordinator.go

@@ -95,7 +95,7 @@ func (coord *Coordinator) CreateClient(
 ) (clientID string, err error) {
 	coord.CommitBlock(source, counterparty)
 
-	clientID = source.NewClientID(clientType, counterparty.ChainID)
+	clientID = source.NewClientID(clientType)
 
 	switch clientType {
 	case exported.Tendermint: