Standardize connection versioning + channel versioning docs

x/ibc-transfer/types/keys.go

@@ -14,7 +14,7 @@ const (
 
 	// Version defines the current version the IBC tranfer
 	// module supports
-	Version = "ics20-1"
+	Version = "(ics20-1,[])"
 
 	// PortID is the default port id that transfer module binds to
 	PortID = "transfer"
@@ -45,3 +45,8 @@ func GetEscrowAddress(portID, channelID string) sdk.AccAddress {
 func GetDenomPrefix(portID, channelID string) string {
 	return fmt.Sprintf("%s/%s/", portID, channelID)
 }
+
+// GetCompatibleVersions
+func GetCompatibleVersions() []string {
+	return []string{Version}
+}

x/ibc/03-connection/keeper/handshake.go

@@ -74,7 +74,7 @@ func (k Keeper) ConnOpenTry(
 
 	// chain B picks a version from Chain A's available versions that is compatible
 	// with the supported IBC versions
-	version, err := types.PickVersion(counterpartyVersions)
+	version, err := types.PickVersion(types.GetCompatibleVersions(), counterpartyVersions, types.AllowNilFeatureSetMap)
 	if err != nil {
 		return err
 	}
@@ -156,7 +156,7 @@ func (k Keeper) ConnOpenAck(
 		)
 	}
 
-	// Check that ChainB's proposed version number is supported by chainA
+	// Check that ChainB's proposed version identifier is supported by chainA
 	supportedVersion, found := types.FindSupportedVersion(version, types.GetCompatibleVersions())
 	if !found {
 		return sdkerrors.Wrapf(
@@ -166,11 +166,8 @@ func (k Keeper) ConnOpenAck(
 	}
 
 	// Check that ChainB's proposed feature set is supported by chainA
-	if !types.VerifyProposedFeatureSet(version, supportedVersion) {
-		return sdkerrors.Wrapf(
-			types.ErrVersionNegotiationFailed,
-			"connection version feature set provided (%s) is not supported (%s)", version, types.GetCompatibleVersions(),
-		)
+	if err := types.VerifyProposedVersion(version, supportedVersion, types.AllowNilFeatureSetMap); err != nil {
+		return err
 	}
 
 	// Retrieve chainA's consensus state at consensusheight

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

@@ -60,7 +60,7 @@ func (c ConnectionEnd) ValidateBasic() error {
 		return sdkerrors.Wrap(sdkerrors.ErrInvalidVersion, "missing connection versions")
 	}
 	for _, version := range c.Versions {
-		if err := host.ConnectionVersionValidator(version); err != nil {
+		if err := host.VersionValidator(version); err != nil {
 			return err
 		}
 	}

x/ibc/03-connection/types/errors.go

@@ -14,5 +14,5 @@ var (
 	ErrInvalidCounterparty           = sdkerrors.Register(SubModuleName, 7, "invalid counterparty connection")
 	ErrInvalidConnection             = sdkerrors.Register(SubModuleName, 8, "invalid connection")
 	ErrInvalidVersion                = sdkerrors.Register(SubModuleName, 9, "invalid connection version")
-	ErrVersionNegotiationFailed      = sdkerrors.Register(SubModuleName, 10, "connection version negotiation failed")
+	ErrVersionNegotiationFailed      = sdkerrors.Register(SubModuleName, 10, "version negotiation failed")
 )

x/ibc/03-connection/types/msgs.go

@@ -103,7 +103,7 @@ func (msg MsgConnectionOpenTry) ValidateBasic() error {
 		return sdkerrors.Wrap(sdkerrors.ErrInvalidVersion, "missing counterparty versions")
 	}
 	for _, version := range msg.CounterpartyVersions {
-		if err := host.ConnectionVersionValidator(version); err != nil {
+		if err := host.VersionValidator(version); err != nil {
 			return err
 		}
 	}
@@ -169,7 +169,7 @@ func (msg MsgConnectionOpenAck) ValidateBasic() error {
 	if err := host.ConnectionIdentifierValidator(msg.ConnectionID); err != nil {
 		return sdkerrors.Wrap(err, "invalid connection ID")
 	}
-	if err := host.ConnectionVersionValidator(msg.Version); err != nil {
+	if err := host.VersionValidator(msg.Version); err != nil {
 		return err
 	}
 	if len(msg.ProofTry) == 0 {

x/ibc/03-connection/types/version.go

@@ -9,9 +9,18 @@ import (
 )
 
 var (
-	// DefaultIBCVersion represents the latest supported version of IBC.
-	// The current version supports only ORDERED and UNORDERED channels.
+	// DefaultIBCVersion represents the latest supported version of IBC used
+	// in connection version negotiation. The current version supports only
+	// ORDERED and UNORDERED channels and requires at least one channel type
+	// to be agreed upon.
 	DefaultIBCVersion = CreateVersionString("1", []string{"ORDER_ORDERED", "ORDER_UNORDERED"})
+
+	// AllowNilFeatureSetMap is a helper map to indicate if a specified version
+	// is allowed to have a nil feature set. Any versions supported, but
+	// not included in the map default to not supporting nil feature sets.
+	AllowNilFeatureSetMap = map[string]bool{
+		DefaultIBCVersion: false,
+	}
 )
 
 // GetCompatibleVersions returns a descending ordered set of compatible IBC
@@ -39,7 +48,7 @@ func CreateVersionString(identifier string, featureSet []string) string {
 // feature set of a version. An error is returned if the version is not valid.
 func UnpackVersion(version string) (string, []string, error) {
 	// validate version so valid splitting assumptions can be made
-	if err := host.ConnectionVersionValidator(version); err != nil {
+	if err := host.VersionValidator(version); err != nil {
 		return "", nil, err
 	}
 
@@ -96,10 +105,8 @@ func FindSupportedVersion(version string, supportedVersions []string) (string, b
 // set with the intersection of the features supported by the source and
 // counterparty chains. This function is called in the ConnOpenTry handshake
 // procedure.
-func PickVersion(counterpartyVersions []string) (string, error) {
-	versions := GetCompatibleVersions()
-
-	for _, ver := range versions {
+func PickVersion(supportedVersions, counterpartyVersions []string, allowNilFeatureSet map[string]bool) (string, error) {
+	for _, ver := range supportedVersions {
 		// check if the source version is supported by the counterparty
 		if counterpartyVer, found := FindSupportedVersion(ver, counterpartyVersions); found {
 			sourceIdentifier, sourceFeatures, err := UnpackVersion(ver)
@@ -113,6 +120,9 @@ func PickVersion(counterpartyVersions []string) (string, error) {
 			}
 
 			featureSet := GetFeatureSetIntersection(sourceFeatures, counterpartyFeatures)
+			if len(featureSet) == 0 && !allowNilFeatureSet[ver] {
+				continue
+			}
 
 			version := CreateVersionString(sourceIdentifier, featureSet)
 			return version, nil
@@ -121,7 +131,7 @@ func PickVersion(counterpartyVersions []string) (string, error) {
 
 	return "", sdkerrors.Wrapf(
 		ErrVersionNegotiationFailed,
-		"failed to find a matching counterparty version (%s) from the supported version list (%s)", counterpartyVersions, versions,
+		"failed to find a matching counterparty version (%s) from the supported version list (%s)", counterpartyVersions, supportedVersions,
 	)
 }
 
@@ -139,26 +149,46 @@ func GetFeatureSetIntersection(sourceFeatureSet, counterpartyFeatureSet []string
 	return featureSet
 }
 
-// VerifyProposedFeatureSet verifies that the entire feature set in the
-// proposed version is supported by this chain.
-func VerifyProposedFeatureSet(proposedVersion, supportedVersion string) bool {
-	_, proposedFeatureSet, err := UnpackVersion(proposedVersion)
+// VerifyProposedVersion verifies that the entire feature set in the
+// proposed version is supported by this chain. If the feature set is
+// empty it verifies that this is allowed for the specified version
+// identifier. It also ensures that the supported version identifier
+// matches the proposed version identifier.
+func VerifyProposedVersion(proposedVersion, supportedVersion string, allowNilFeatureSet map[string]bool) error {
+	proposedIdentifier, proposedFeatureSet, err := UnpackVersion(proposedVersion)
 	if err != nil {
-		return false
+		return sdkerrors.Wrap(err, "could not unpack proposed version")
 	}
 
-	_, supportedFeatureSet, err := UnpackVersion(supportedVersion)
+	if len(proposedFeatureSet) == 0 && allowNilFeatureSet != nil && !allowNilFeatureSet[proposedIdentifier] {
+		return sdkerrors.Wrapf(
+			ErrVersionNegotiationFailed,
+			"nil feature sets are not supported for version identifier (%s)", proposedIdentifier,
+		)
+	}
+
+	supportedIdentifier, supportedFeatureSet, err := UnpackVersion(supportedVersion)
 	if err != nil {
-		return false
+		return sdkerrors.Wrap(err, "could not unpack supported version")
+	}
+
+	if supportedIdentifier != proposedIdentifier {
+		return sdkerrors.Wrapf(
+			ErrVersionNegotiationFailed,
+			"proposed version identifier by counterparty (%s) does not match supported version identifier (%s)", proposedIdentifier, supportedIdentifier,
+		)
 	}
 
 	for _, proposedFeature := range proposedFeatureSet {
 		if !contains(proposedFeature, supportedFeatureSet) {
-			return false
+			return sdkerrors.Wrapf(
+				ErrVersionNegotiationFailed,
+				"proposed feature set (%s) is not a supported feature set (%s)", proposedFeatureSet, supportedFeatureSet,
+			)
 		}
 	}
 
-	return true
+	return nil
 }
 
 // VerifySupportedFeature takes in a version string and feature string and returns

x/ibc/03-connection/types/version_test.go

@@ -74,13 +74,13 @@ func TestPickVersion(t *testing.T) {
 	}{
 		{"valid default ibc version", types.GetCompatibleVersions(), types.DefaultIBCVersion, true},
 		{"valid version in counterparty versions", []string{"(version1,[])", "(2.0.0,[DAG,ZK])", types.DefaultIBCVersion}, types.DefaultIBCVersion, true},
-		{"valid identifier match but empty feature set", []string{"(1,[DAG,ORDERED-ZK,UNORDERED-zk])"}, "(1,[])", true},
+		{"valid identifier match but empty feature set not allowed", []string{"(1,[DAG,ORDERED-ZK,UNORDERED-zk])"}, "(1,[])", false},
 		{"empty counterparty versions", []string{}, "", false},
 		{"non-matching counterparty versions", []string{"(2.0.0,[])"}, "", false},
 	}
 
 	for i, tc := range testCases {
-		version, err := types.PickVersion(tc.counterpartyVersions)
+		version, err := types.PickVersion(types.GetCompatibleVersions(), tc.counterpartyVersions, types.AllowNilFeatureSetMap)
 
 		if tc.expPass {
 			require.NoError(t, err, "valid test case %d failed: %s", i, tc.name)
@@ -99,16 +99,20 @@ func TestVerifyProposedFeatureSet(t *testing.T) {
 		supportedVersion string
 		expPass          bool
 	}{
-		{"entire feature set supported", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_ORDERED", "ORDER_UNORDERED", "ORDER_DAG"}), true},
-		{"empty feature sets", types.CreateVersionString("1", []string{}), types.DefaultIBCVersion, true},
+		{"entire feature set supported", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_ORDERED", "ORDER_UNORDERED", "DAG"}), true},
+		{"empty feature sets not supported", types.CreateVersionString("1", []string{}), types.DefaultIBCVersion, false},
 		{"one feature missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_UNORDERED", "ORDER_DAG"}), false},
 		{"both features missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_DAG"}), false},
 	}
 
 	for i, tc := range testCases {
-		supported := types.VerifyProposedFeatureSet(tc.proposedVersion, tc.supportedVersion)
+		err := types.VerifyProposedVersion(tc.proposedVersion, tc.supportedVersion, types.AllowNilFeatureSetMap)
 
-		require.Equal(t, tc.expPass, supported, "test case %d: %s", i, tc.name)
+		if tc.expPass {
+			require.NoError(t, err, "test case %d: %s", i, tc.name)
+		} else {
+			require.Error(t, err, "test case %d: %s", i, tc.name)
+		}
 	}
 
 }

x/ibc/04-channel/client/cli/tx.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/tx"
+	ibctransfertypes "github.com/cosmos/cosmos-sdk/x/ibc-transfer/types"
 	connectionutils "github.com/cosmos/cosmos-sdk/x/ibc/03-connection/client/utils"
 	"github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types"
 )
@@ -52,7 +53,7 @@ func NewChannelOpenInitCmd() *cobra.Command {
 		},
 	}
 	cmd.Flags().Bool(FlagOrdered, true, "Pass flag for opening ordered channels")
-	cmd.Flags().String(FlagIBCVersion, types.DefaultChannelVersion, "supported IBC version")
+	cmd.Flags().String(FlagIBCVersion, ibctransfertypes.Version, "IBC application version")
 
 	return cmd
 }
@@ -103,7 +104,7 @@ func NewChannelOpenTryCmd() *cobra.Command {
 		},
 	}
 	cmd.Flags().Bool(FlagOrdered, true, "Pass flag for opening ordered channels")
-	cmd.Flags().String(FlagIBCVersion, types.DefaultChannelVersion, "supported IBC version")
+	cmd.Flags().String(FlagIBCVersion, ibctransfertypes.Version, "IBC application version")
 
 	return cmd
 }
@@ -147,7 +148,7 @@ func NewChannelOpenAckCmd() *cobra.Command {
 			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
 		},
 	}
-	cmd.Flags().String(FlagIBCVersion, types.DefaultChannelVersion, "supported IBC version")
+	cmd.Flags().String(FlagIBCVersion, ibctransfertypes.Version, "IBC application version")
 
 	return cmd
 }

x/ibc/04-channel/keeper/handshake.go

@@ -238,6 +238,11 @@ func (k Keeper) ChanOpenAck(
 		panic("cannot find connection")
 	}
 
+	// verify that chainB's proposed version identifier is supported by chainA
+	if err := connectiontypes.VerifyProposedVersion(channel.Version, counterpartyVersion, nil); err != nil {
+		return err
+	}
+
 	// counterparty of the counterparty channel end (i.e self)
 	expectedCounterparty := types.NewCounterparty(portID, channelID)
 	expectedChannel := types.NewChannel(

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

@@ -13,9 +13,6 @@ var (
 	_ exported.CounterpartyI = (*Counterparty)(nil)
 )
 
-// DefaultChannelVersion defines the default channel version used during handshake.
-const DefaultChannelVersion = "1.0.0"
-
 // NewChannel creates a new Channel instance
 func NewChannel(
 	state State, ordering Order, counterparty Counterparty,

x/ibc/24-host/validate.go

@@ -14,14 +14,14 @@ import (
 // - `[`, `]`, `<`, `>`
 var IsValidID = regexp.MustCompile(`^[a-zA-Z0-9\.\_\+\-\#\[\]\<\>]+$`).MatchString
 
-// IsValidConnectionVersion defines the regular expression to check if the
-// string is in the form of a tuple consisting of a string identifier and
-// a set of features. The entire version tuple must be enclosed in parentheses.
-// The version identifier must not contain any commas. The set of features
-// must be enclosed in brackets and separated by commas.
+// IsValidnVersion defines the regular expression to check if the string is
+// in the form of a tuple consisting of a string identifier and a set of
+// features. The entire version tuple must be enclosed in parentheses. The
+// version identifier must not contain any commas. The set of features must
+// be enclosed in brackets and separated by commas.
 //
-// valid connection version = ([version_identifier], [feature_0, feature_1, etc])
-var IsValidConnectionVersion = regexp.MustCompile(`^\([^,]+\,\[([^,]+(\,[^,]+)*)?\]\)$`).MatchString
+// valid version = ([version_identifier], [feature_0, feature_1, etc])
+var IsValidVersion = regexp.MustCompile(`^\([^,]+\,\[([^,]+(\,[^,]+)*)?\]\)$`).MatchString
 
 // ICS 024 Identifier and Path Validation Implementation
 //
@@ -83,15 +83,15 @@ func PortIdentifierValidator(id string) error {
 	return defaultIdentifierValidator(id, 2, 20)
 }
 
-// ConnectionVersionValidator is the default validator function for Connection
-// versions. A valid version must be in semantic versioning form and contain
-// only non-negative integers.
-func ConnectionVersionValidator(version string) error {
+// VersionValidator is the default validator function for IBC connection and
+// channel versions. A valid version must follow the rules outlined in the
+// `IsValidVersion` regex expression.
+func VersionValidator(version string) error {
 	if strings.TrimSpace(version) == "" {
 		return sdkerrors.Wrap(ErrInvalidVersion, "version cannot be blank")
 	}
 
-	if !IsValidConnectionVersion(version) {
+	if !IsValidVersion(version) {
 		return sdkerrors.Wrapf(
 			ErrInvalidVersion,
 			"version '%s' must be in '(version_identifier,[feature_0, feature_1])' with no extra spacing", version,

x/ibc/24-host/validate_test.go

@@ -130,7 +130,7 @@ func TestConnectionVersionValidator(t *testing.T) {
 
 	for _, tc := range testCases {
 
-		err := ConnectionVersionValidator(tc.id)
+		err := VersionValidator(tc.id)
 
 		if tc.expPass {
 			require.NoError(t, err, tc.msg)

x/ibc/testing/chain.go

@@ -21,6 +21,7 @@ import (
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+	ibctransfertypes "github.com/cosmos/cosmos-sdk/x/ibc-transfer/types"
 	clientexported "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
 	connectiontypes "github.com/cosmos/cosmos-sdk/x/ibc/03-connection/types"
 	channelexported "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/exported"
@@ -38,7 +39,7 @@ const (
 	UnbondingPeriod time.Duration = time.Hour * 24 * 7 * 3
 	MaxClockDrift   time.Duration = time.Second * 10
 
-	ChannelVersion = "ics20-1"
+	ChannelVersion = ibctransfertypes.Version
 	InvalidID      = "IDisInvalid"
 
 	ConnectionIDPrefix = "connectionid"