Add verifiable upgrade type and validate basic functions (#3451)

e2e/go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/cosmos/cosmos-sdk v0.47.0
 	github.com/cosmos/gogoproto v1.4.6
 	github.com/cosmos/ibc-go/v7 v7.0.0
-	github.com/cosmos/interchain-accounts v0.5.0
+	github.com/cosmos/interchain-accounts v0.5.1
 	github.com/docker/docker v20.10.19+incompatible
 	github.com/strangelove-ventures/interchaintest/v7 v7.0.0-20230322043324-cb6ba0947fff
 	github.com/stretchr/testify v1.8.2

e2e/go.sum

@@ -347,8 +347,8 @@ github.com/cosmos/iavl v0.20.0 h1:fTVznVlepH0KK8NyKq8w+U7c2L6jofa27aFX6YGlm38=
 github.com/cosmos/iavl v0.20.0/go.mod h1:WO7FyvaZJoH65+HFOsDir7xU9FWk2w9cHXNW1XHcl7A=
 github.com/cosmos/ics23/go v0.9.1-0.20221207100636-b1abd8678aab h1:I9ialKTQo7248V827Bba4OuKPmk+FPzmTVHsLXaIJWw=
 github.com/cosmos/ics23/go v0.9.1-0.20221207100636-b1abd8678aab/go.mod h1:2CwqasX5dSD7Hbp/9b6lhK6BwoBDCBldx7gPKRukR60=
-github.com/cosmos/interchain-accounts v0.5.0 h1:bD4U5PzaRbgsGCTwKEShPjKXFxSIp9D+kAqFutYKl1E=
-github.com/cosmos/interchain-accounts v0.5.0/go.mod h1:0Pzv5xHq+TVY7ExRIsrzJ33+t22TUoJ1k8UgSxvv1X4=
+github.com/cosmos/interchain-accounts v0.5.1 h1:J5ZaYsMc2u4DekKXbDPzv8nu4YD/RSmT0F8dmN7G1oM=
+github.com/cosmos/interchain-accounts v0.5.1/go.mod h1:JB3gKbX8geQhxEIrBQtpDco0cyKMUDpVhugb78e5z6U=
 github.com/cosmos/ledger-cosmos-go v0.13.0 h1:ex0CvCxToSR7j5WjrghPu2Bu9sSXKikjnVvUryNnx4s=
 github.com/cosmos/ledger-cosmos-go v0.13.0/go.mod h1:ZcqYgnfNJ6lAXe4HPtWgarNEY+B74i+2/8MhZw4ziiI=
 github.com/cosmos/rosetta-sdk-go v0.10.0 h1:E5RhTruuoA7KTIXUcMicL76cffyeoyvNybzUGSKFTcM=

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,49 @@ func (k Keeper) DeleteUpgradeTimeout(ctx sdk.Context, portID, channelID string)
 	store.Delete(host.ChannelUpgradeTimeoutKey(portID, channelID))
 }
 
+// ValidateUpgradeFields 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 UpgradeFields.ValidateBasic())
+// - the proposed connection hops are not open
+func (k Keeper) ValidateUpgradeFields(ctx sdk.Context, proposedUpgrade types.UpgradeFields, existingChannel types.Channel) error {
+	currentFields := extractUpgradeFields(existingChannel)
+
+	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")
+	}
+
+	connectionID := proposedUpgrade.ConnectionHops[0]
+	connection, err := k.GetConnection(ctx, connectionID)
+	if err != nil {
+		return errorsmod.Wrapf(connectiontypes.ErrConnectionNotFound, "failed to retrieve connection: %s", connectionID)
+	}
+
+	if connection.GetState() != int32(connectiontypes.OPEN) {
+		return errorsmod.Wrapf(
+			connectiontypes.ErrInvalidConnectionState,
+			"connection state is not OPEN (got %s)", connectiontypes.State(connection.GetState()).String(),
+		)
+	}
+
+	return nil
+}
+
+// extractUpgradeFields returns the upgrade fields from the provided channel.
+func extractUpgradeFields(channel types.Channel) types.UpgradeFields {
+	return types.UpgradeFields{
+		Ordering:       channel.Ordering,
+		ConnectionHops: channel.ConnectionHops,
+		Version:        channel.Version,
+	}
+}
+
 // 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.UpgradeFields
+		path            *ibctesting.Path
+	)
+
+	tests := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+		{
+			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: "fails with stricter ordering",
+			malleate: func() {
+				proposedUpgrade.Ordering = types.ORDERED
+			},
+			expPass: false,
+		},
+		{
+			name:     "fails with unmodified fields",
+			malleate: func() {},
+			expPass:  false,
+		},
+		{
+			name: "fails 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: "fails 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.UpgradeFields{
+				Ordering:       existingChannel.Ordering,
+				ConnectionHops: existingChannel.ConnectionHops,
+				Version:        existingChannel.Version,
+			}
+
+			tc.malleate()
+
+			err := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ValidateUpgradeFields(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/upgrade.go

@@ -0,0 +1,45 @@
+package types
+
+import (
+	"strings"
+
+	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.Fields.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 (uf UpgradeFields) ValidateBasic() error {
+	if !collections.Contains(uf.Ordering, []Order{ORDERED, UNORDERED}) {
+		return errorsmod.Wrap(ErrInvalidChannelOrdering, uf.Ordering.String())
+	}
+
+	if len(uf.ConnectionHops) != 1 {
+		return errorsmod.Wrap(ErrTooManyConnectionHops, "current IBC version only supports one connection hop")
+	}
+
+	if strings.TrimSpace(uf.Version) == "" {
+		return errorsmod.Wrap(ErrInvalidChannelVersion, "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
+}