Add verifiable upgrade type and validate basic functions

modules/core/03-connection/keeper/keeper.go

@@ -83,6 +83,18 @@ func (k Keeper) HasConnection(ctx sdk.Context, connectionID string) bool {
 	return store.Has(host.ConnectionKey(connectionID))
 }
 
+// CheckIsOpen returns nil if the connection with the given id is OPEN. Otherwise an error is returned.
+func (k Keeper) CheckIsOpen(ctx sdk.Context, connectionID string) error {
+	connection, found := k.GetConnection(ctx, connectionID)
+	if !found {
+		return errorsmod.Wrapf(types.ErrConnectionNotFound, "failed to retrieve connection: %s", connectionID)
+	}
+	if connection.State != types.OPEN {
+		return errorsmod.Wrapf(types.ErrInvalidConnectionState, "connectionID (%s) state is not OPEN (got %s)", connectionID, connection.State.String())
+	}
+	return nil
+}
+
 // SetConnection sets a connection to the store
 func (k Keeper) SetConnection(ctx sdk.Context, connectionID string, connection types.ConnectionEnd) {
 	store := ctx.KVStore(k.storeKey)

modules/core/03-connection/keeper/keeper_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
 	commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types"
+	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 )
@@ -176,3 +177,64 @@ func (suite *KeeperTestSuite) TestLocalhostConnectionEndCreation() {
 	expectedCounterParty := types.NewCounterparty(exported.LocalhostClientID, exported.LocalhostConnectionID, commitmenttypes.NewMerklePrefix(connectionKeeper.GetCommitmentPrefix().Bytes()))
 	suite.Require().Equal(expectedCounterParty, connectionEnd.Counterparty)
 }
+
+func (suite *KeeperTestSuite) TestCheckIsOpen() {
+	var path *ibctesting.Path
+
+	cases := []struct {
+		msg      string
+		malleate func()
+		expPass  bool
+	}{
+		{
+			msg:      "success",
+			malleate: func() {},
+			expPass:  true,
+		},
+		{
+			msg: "uninitialized connection",
+			malleate: func() {
+				connection := path.EndpointA.GetConnection()
+				connection.State = types.UNINITIALIZED
+				path.EndpointA.SetConnection(connection)
+			},
+			expPass: false,
+		},
+		{
+			msg: "init connection",
+			malleate: func() {
+				connection := path.EndpointA.GetConnection()
+				connection.State = types.INIT
+				path.EndpointA.SetConnection(connection)
+			},
+			expPass: false,
+		},
+		{
+			msg: "no connection",
+			malleate: func() {
+				storeKey := suite.chainA.GetSimApp().GetKey(exported.StoreKey)
+				kvStore := suite.chainA.GetContext().KVStore(storeKey)
+				kvStore.Delete(host.ConnectionKey(ibctesting.FirstConnectionID))
+			},
+			expPass: false,
+		},
+	}
+
+	for _, tc := range cases {
+		suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
+			suite.SetupTest() // reset
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+			suite.coordinator.Setup(path)
+
+			tc.malleate()
+
+			connectionKeeper := suite.chainA.GetSimApp().IBCKeeper.ConnectionKeeper
+
+			if tc.expPass {
+				suite.Require().NoError(connectionKeeper.CheckIsOpen(suite.chainA.GetContext(), ibctesting.FirstConnectionID))
+			} else {
+				suite.Require().Error(connectionKeeper.CheckIsOpen(suite.chainA.GetContext(), ibctesting.FirstConnectionID))
+			}
+		})
+	}
+}

modules/core/04-channel/keeper/keeper.go

@@ -1,15 +1,16 @@
 package keeper
 
 import (
+	"reflect"
 	"strconv"
 	"strings"
 
+	errorsmod "cosmossdk.io/errors"
 	db "github.com/cometbft/cometbft-db"
 	"github.com/cometbft/cometbft/libs/log"
 	"github.com/cosmos/cosmos-sdk/codec"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	errorsmod "github.com/cosmos/cosmos-sdk/types/errors"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
@@ -571,6 +572,30 @@ func (k Keeper) DeleteUpgradeTimeout(ctx sdk.Context, portID, channelID string)
 	store.Delete(host.ChannelUpgradeTimeoutKey(portID, channelID))
 }
 
+// ValidateProposedUpgradeFields validates the proposed upgrade fields against the existing channel.
+// It returns an error if the following constraints are not met:
+// - there exists at least one valid proposed change to the existing channel fields
+// - the proposed order is a subset of the existing order
+// - the proposed connection hops do not exist
+// - the proposed version is non-empty (checked in ModifiableUpgradeFields.ValidateBasic())
+// - the proposed connection hops are not open
+func (k Keeper) ValidateProposedUpgradeFields(ctx sdk.Context, proposedUpgrade types.ModifiableUpgradeFields, existingChannel types.Channel) error {
+	currentFields := types.ModifiableUpgradeFields{
+		Ordering:       existingChannel.Ordering,
+		ConnectionHops: existingChannel.ConnectionHops,
+		Version:        existingChannel.Version,
+	}
+	if reflect.DeepEqual(proposedUpgrade, currentFields) {
+		return errorsmod.Wrap(types.ErrChannelExists, "existing channel end is identical to proposed upgrade channel end")
+	}
+
+	if !currentFields.Ordering.SubsetOf(proposedUpgrade.Ordering) {
+		return errorsmod.Wrap(types.ErrInvalidChannelOrdering, "channel ordering must be a subset of the new ordering")
+	}
+
+	return k.connectionKeeper.CheckIsOpen(ctx, proposedUpgrade.ConnectionHops[0])
+}
+
 // common functionality for IteratePacketCommitment and IteratePacketAcknowledgement
 func (k Keeper) iterateHashes(ctx sdk.Context, iterator db.Iterator, cb func(portID, channelID string, sequence uint64, hash []byte) bool) {
 	defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() })

modules/core/04-channel/keeper/keeper_test.go

@@ -8,7 +8,10 @@ import (
 
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
+	connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
 	"github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
+	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
+	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 	ibcmock "github.com/cosmos/ibc-go/v7/testing/mock"
 )
@@ -557,3 +560,88 @@ func (suite *KeeperTestSuite) TestUpgradeTimeoutAccessors() {
 		suite.Require().False(found)
 	})
 }
+
+func (suite *KeeperTestSuite) TestValidateProposedUpgradeFields() {
+	var (
+		proposedUpgrade *types.ModifiableUpgradeFields
+		path            *ibctesting.Path
+	)
+
+	tests := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+		{
+			name: "modify ordering to invalid value",
+			malleate: func() {
+				proposedUpgrade.Ordering = types.ORDERED
+			},
+			expPass: false,
+		},
+		{
+			name:     "no modified fields in proposed upgrade",
+			malleate: func() {},
+			expPass:  false,
+		},
+		{
+			name: "change channel version",
+			malleate: func() {
+				proposedUpgrade.Version = "1.0.0"
+			},
+			expPass: true,
+		},
+		{
+			name: "change connection hops",
+			malleate: func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				suite.coordinator.Setup(path)
+				proposedUpgrade.ConnectionHops = []string{path.EndpointA.ConnectionID}
+			},
+			expPass: true,
+		},
+		{
+			name: "attempt upgrade when connection is not set",
+			malleate: func() {
+				storeKey := suite.chainA.GetSimApp().GetKey(exported.StoreKey)
+				kvStore := suite.chainA.GetContext().KVStore(storeKey)
+				kvStore.Delete(host.ConnectionKey(ibctesting.FirstConnectionID))
+			},
+			expPass: false,
+		},
+		{
+			name: "attempt upgrade when connection is not open",
+			malleate: func() {
+				connection := path.EndpointA.GetConnection()
+				connection.State = connectiontypes.UNINITIALIZED
+				path.EndpointA.SetConnection(connection)
+			},
+			expPass: false,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+			suite.coordinator.Setup(path)
+
+			existingChannel := path.EndpointA.GetChannel()
+			proposedUpgrade = &types.ModifiableUpgradeFields{
+				Ordering:       existingChannel.Ordering,
+				ConnectionHops: existingChannel.ConnectionHops,
+				Version:        existingChannel.Version,
+			}
+
+			tc.malleate()
+
+			err := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ValidateProposedUpgradeFields(suite.chainA.GetContext(), *proposedUpgrade, existingChannel)
+			if tc.expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().Error(err)
+			}
+		})
+	}
+}

modules/core/04-channel/types/errors.go

@@ -43,4 +43,5 @@ var (
 	ErrInvalidUpgradeTimeout   = errorsmod.Register(SubModuleName, 27, "either timeout height or timeout timestamp must be non-zero")
 	ErrUpgradeSequenceNotFound = errorsmod.Register(SubModuleName, 28, "upgrade sequence not found")
 	ErrUpgradeErrorNotFound    = errorsmod.Register(SubModuleName, 29, "upgrade error receipt not found")
+	ErrInvalidUpgrade          = errorsmod.Register(SubModuleName, 30, "invalid upgrade")
 )

modules/core/04-channel/types/expected_keepers.go

@@ -18,6 +18,8 @@ type ClientKeeper interface {
 
 // ConnectionKeeper expected account IBC connection keeper
 type ConnectionKeeper interface {
+	CheckIsOpen(ctx sdk.Context, connectionID string) error
+	HasConnection(ctx sdk.Context, connectionID string) bool
 	GetConnection(ctx sdk.Context, connectionID string) (connectiontypes.ConnectionEnd, bool)
 	GetTimestampAtHeight(
 		ctx sdk.Context,

modules/core/04-channel/types/upgrade.go

@@ -0,0 +1,43 @@
+package types
+
+import (
+	errorsmod "cosmossdk.io/errors"
+
+	"github.com/cosmos/ibc-go/v7/internal/collections"
+)
+
+// ValidateBasic performs a basic validation of the upgrade fields
+func (u Upgrade) ValidateBasic() error {
+	if err := u.UpgradeFields.ValidateBasic(); err != nil {
+		return errorsmod.Wrap(err, "proposed upgrade fields are invalid")
+	}
+
+	if !u.Timeout.IsValid() {
+		return errorsmod.Wrap(ErrInvalidUpgrade, "upgrade timeout cannot be empty")
+	}
+
+	// TODO: determine if last packet sequence sent can be 0?
+	return nil
+}
+
+// ValidateBasic performs a basic validation of the proposed upgrade fields
+func (muf ModifiableUpgradeFields) ValidateBasic() error {
+	if !collections.Contains(muf.Ordering, []Order{ORDERED, UNORDERED}) {
+		return errorsmod.Wrap(ErrInvalidChannelOrdering, muf.Ordering.String())
+	}
+
+	if len(muf.ConnectionHops) != 1 {
+		return errorsmod.Wrap(ErrTooManyConnectionHops, "current IBC version only supports one connection hop")
+	}
+
+	if muf.Version == "" {
+		return errorsmod.Wrap(ErrInvalidUpgrade, "proposed upgrade version cannot be empty")
+	}
+
+	return nil
+}
+
+// IsValid returns true if either the height or timestamp is non-zero
+func (ut UpgradeTimeout) IsValid() bool {
+	return !ut.TimeoutHeight.IsZero() || ut.TimeoutTimestamp != 0
+}