startFlushUpgradeHandshake (#3671)

* first draft startflushupgradehandshake * chore: adding check for upgrade sequence and additional TODOs * refactor: update startFlushUpgradeHandshake function to latest spec changes as well as with UpgradeError usage * test: add startFlushUpgradeHandshake unit tests * test: fix build and test cases for startFlushUpgradeHandshake * address pr comments --------- Co-authored-by: Cian Hatton <cian@interchain.io> Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>
cosmos · Jun 6, 2023 · 8334d3a · 8334d3a
1 parent ec1c1d8
commit 8334d3a
Show file tree

Hide file tree

Showing 5 changed files with 325 additions and 8 deletions.
diff --git a/modules/core/04-channel/keeper/export_test.go b/modules/core/04-channel/keeper/export_test.go
@@ -7,9 +7,25 @@ package keeper
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	"github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 )
 
+// StartFlushUpgradeHandshake is a wrapper around startFlushUpgradeHandshake to allow the function to be directly called in tests.
+func (k Keeper) StartFlushUpgradeHandshake(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+	proposedUpgradeFields types.UpgradeFields,
+	counterpartyChannel types.Channel,
+	counterpartyUpgrade types.Upgrade,
+	proofCounterpartyChannel,
+	proofCounterpartyUpgrade []byte,
+	proofHeight clienttypes.Height,
+) error {
+	return k.startFlushUpgradeHandshake(ctx, portID, channelID, proposedUpgradeFields, counterpartyChannel, counterpartyUpgrade, proofCounterpartyChannel, proofCounterpartyUpgrade, proofHeight)
+}
+
 // ValidateUpgradeFields is a wrapper around validateUpgradeFields to allow the function to be directly called in tests.
 func (k Keeper) ValidateUpgradeFields(ctx sdk.Context, proposedUpgrade types.UpgradeFields, currentChannel types.Channel) error {
 	return k.validateUpgradeFields(ctx, proposedUpgrade, currentChannel)

diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go
@@ -5,6 +5,7 @@ import (
 	"reflect"
 
 	errorsmod "cosmossdk.io/errors"
+	sdkmath "cosmossdk.io/math"
 	"github.com/cosmos/cosmos-sdk/telemetry"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
@@ -109,6 +110,110 @@ func (k Keeper) WriteUpgradeTryChannel(
 	emitChannelUpgradeTryEvent(ctx, portID, channelID, channel, proposedUpgrade)
 }
 
+// startFlushUpgradeHandshake will verify the counterparty proposed upgrade and the current channel state.
+// Once the counterparty information has been verified, it will be validated against the self proposed upgrade.
+// If any of the proposed upgrade fields are incompatible, an upgrade error will be returned resulting in an
+// aborted upgrade.
+//
+//lint:ignore U1000 Ignore unused function temporarily for debugging
+func (k Keeper) startFlushUpgradeHandshake(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+	proposedUpgradeFields types.UpgradeFields,
+	counterpartyChannel types.Channel,
+	counterpartyUpgrade types.Upgrade,
+	proofCounterpartyChannel,
+	proofCounterpartyUpgrade []byte,
+	proofHeight clienttypes.Height,
+) error {
+	channel, found := k.GetChannel(ctx, portID, channelID)
+	if !found {
+		return errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
+	}
+
+	connection, err := k.GetConnection(ctx, channel.ConnectionHops[0])
+	if err != nil {
+		return errorsmod.Wrap(err, "failed to retrieve connection using the channel connection hops")
+	}
+
+	if connection.GetState() != int32(connectiontypes.OPEN) {
+		return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectiontypes.State(connection.GetState()).String())
+	}
+
+	// verify the counterparty channel state containing the upgrade sequence
+	if err := k.connectionKeeper.VerifyChannelState(
+		ctx,
+		connection,
+		proofHeight, proofCounterpartyChannel,
+		channel.Counterparty.PortId,
+		channel.Counterparty.ChannelId,
+		counterpartyChannel,
+	); err != nil {
+		return errorsmod.Wrap(err, "failed to verify counterparty channel state")
+	}
+
+	// verifies the proof that a particular proposed upgrade has been stored in the upgrade path of the counterparty
+	if err := k.connectionKeeper.VerifyChannelUpgrade(
+		ctx,
+		connection,
+		proofHeight, proofCounterpartyUpgrade,
+		channel.Counterparty.PortId,
+		channel.Counterparty.ChannelId,
+		counterpartyUpgrade,
+	); err != nil {
+		return errorsmod.Wrap(err, "failed to verify counterparty upgrade")
+	}
+
+	// the current upgrade handshake must only continue if both channels are using the same upgrade sequence,
+	// otherwise an error receipt must be written so that the upgrade handshake may be attempted again with synchronized sequences
+	if counterpartyChannel.UpgradeSequence != channel.UpgradeSequence {
+		// save the previous upgrade sequence for the error message
+		prevUpgradeSequence := channel.UpgradeSequence
+
+		// error on the higher sequence so that both chains synchronize on a fresh sequence
+		channel.UpgradeSequence = sdkmath.Max(counterpartyChannel.UpgradeSequence, channel.UpgradeSequence)
+		k.SetChannel(ctx, portID, channelID, channel)
+
+		return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrapf(
+			types.ErrIncompatibleCounterpartyUpgrade, "expected upgrade sequence (%d) to match counterparty upgrade sequence (%d)", prevUpgradeSequence, counterpartyChannel.UpgradeSequence),
+		)
+	}
+
+	// assert that both sides propose the same channel ordering
+	if proposedUpgradeFields.Ordering != counterpartyUpgrade.Fields.Ordering {
+		return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrapf(
+			types.ErrIncompatibleCounterpartyUpgrade, "expected upgrade ordering (%s) to match counterparty upgrade ordering (%s)", proposedUpgradeFields.Ordering, counterpartyUpgrade.Fields.Ordering),
+		)
+	}
+
+	proposedConnection, err := k.GetConnection(ctx, proposedUpgradeFields.ConnectionHops[0])
+	if err != nil {
+		// NOTE: this error is expected to be unreachable as the proposed upgrade connectionID should have been
+		// validated in the upgrade INIT and TRY handlers
+		return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrap(
+			err, "expected proposed connection to be found"),
+		)
+	}
+
+	if proposedConnection.GetState() != int32(connectiontypes.OPEN) {
+		// NOTE: this error is expected to be unreachable as the proposed upgrade connectionID should have been
+		// validated in the upgrade INIT and TRY handlers
+		return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrapf(
+			connectiontypes.ErrInvalidConnectionState, "expected proposed connection to be OPEN (got %s)", connectiontypes.State(proposedConnection.GetState()).String()),
+		)
+	}
+
+	// connectionHops can change in a channelUpgrade, however both sides must still be each other's counterparty.
+	if counterpartyUpgrade.Fields.ConnectionHops[0] != proposedConnection.GetCounterparty().GetConnectionID() {
+		return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrapf(
+			types.ErrIncompatibleCounterpartyUpgrade, "counterparty upgrade connection end is not a counterparty of self proposed connection end (%s != %s)", counterpartyUpgrade.Fields.ConnectionHops[0], proposedConnection.GetCounterparty().GetConnectionID()),
+		)
+	}
+
+	return nil
+}
+
 // 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

diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go
@@ -3,8 +3,11 @@ package keeper_test
 import (
 	"fmt"
 
+	errorsmod "cosmossdk.io/errors"
+
 	connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
 	"github.com/cosmos/ibc-go/v7/modules/core/04-channel/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"
@@ -126,12 +129,183 @@ func (suite *KeeperTestSuite) TestChanUpgradeInit() {
 	}
 }
 
+// TestStartFlushUpgradeHandshake tests the startFlushUpgradeHandshake.
+// UpgradeInit will be run on chainA and startFlushUpgradeHandshake
+// will be called on chainB
+func (suite *KeeperTestSuite) TestStartFlushUpgradeHandshake() {
+	var (
+		path                *ibctesting.Path
+		upgrade             types.Upgrade
+		counterpartyChannel types.Channel
+		counterpartyUpgrade types.Upgrade
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expError error
+	}{
+		{
+			"success",
+			func() {},
+			nil,
+		},
+		{
+			"channel not found",
+			func() {
+				path.EndpointB.ChannelID = "invalid-channel"
+				path.EndpointB.ChannelConfig.PortID = "invalid-port"
+			},
+			types.ErrChannelNotFound,
+		},
+		{
+			"connection not found",
+			func() {
+				channel := path.EndpointB.GetChannel()
+				channel.ConnectionHops[0] = ibctesting.InvalidID
+				path.EndpointB.SetChannel(channel)
+			},
+			connectiontypes.ErrConnectionNotFound,
+		},
+		{
+			"connection state is not in OPEN state",
+			func() {
+				conn := path.EndpointB.GetConnection()
+				conn.State = connectiontypes.INIT
+				path.EndpointB.SetConnection(conn)
+			},
+			connectiontypes.ErrInvalidConnectionState,
+		},
+		{
+			"failed verification for counterparty channel state due to incorrectly constructed counterparty channel",
+			func() {
+				counterpartyChannel.State = types.CLOSED
+			},
+			commitmenttypes.ErrInvalidProof,
+		},
+		{
+			"failed verification for counterparty upgrade due to incorrectly constructed counterparty upgrade",
+			func() {
+				counterpartyUpgrade.LatestSequenceSend = 100
+			},
+			commitmenttypes.ErrInvalidProof,
+		},
+		{
+			"upgrade sequence mismatch, endpointB channel upgrade sequence is ahead",
+			func() {
+				channel := path.EndpointB.GetChannel()
+				channel.UpgradeSequence++
+				path.EndpointB.SetChannel(channel)
+			},
+			types.NewUpgradeError(2, types.ErrIncompatibleCounterpartyUpgrade), // max sequence will be returned
+		},
+		{
+			"upgrade ordering is not the same on both sides",
+			func() {
+				upgrade.Fields.Ordering = types.ORDERED
+			},
+			types.NewUpgradeError(1, types.ErrIncompatibleCounterpartyUpgrade),
+		},
+		{
+			"proposed connection is not found",
+			func() {
+				upgrade.Fields.ConnectionHops[0] = ibctesting.InvalidID
+			},
+			types.NewUpgradeError(1, connectiontypes.ErrConnectionNotFound),
+		},
+		{
+			"proposed connection is not in OPEN state",
+			func() {
+				// reuse existing connection to create a new connection in a non OPEN state
+				connectionEnd := path.EndpointB.GetConnection()
+				connectionEnd.State = connectiontypes.UNINITIALIZED
+				connectionEnd.Counterparty.ConnectionId = counterpartyUpgrade.Fields.ConnectionHops[0] // both sides must be each other's counterparty
+
+				// set proposed connection in state
+				proposedConnectionID := "connection-100"
+				suite.chainB.GetSimApp().GetIBCKeeper().ConnectionKeeper.SetConnection(suite.chainB.GetContext(), proposedConnectionID, connectionEnd)
+				upgrade.Fields.ConnectionHops[0] = proposedConnectionID
+			},
+			types.NewUpgradeError(1, connectiontypes.ErrInvalidConnectionState),
+		},
+		{
+			"proposed connection ends are not each other's counterparty",
+			func() {
+				// reuse existing connection to create a new connection in a non OPEN state
+				connectionEnd := path.EndpointB.GetConnection()
+				// ensure counterparty connectionID does not match connectionID set in counterparty proposed upgrade
+				connectionEnd.Counterparty.ConnectionId = "connection-50"
+
+				// set proposed connection in state
+				proposedConnectionID := "connection-100"
+				suite.chainB.GetSimApp().GetIBCKeeper().ConnectionKeeper.SetConnection(suite.chainB.GetContext(), proposedConnectionID, connectionEnd)
+				upgrade.Fields.ConnectionHops[0] = proposedConnectionID
+			},
+			types.NewUpgradeError(1, types.ErrIncompatibleCounterpartyUpgrade),
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+			suite.coordinator.Setup(path)
+
+			upgradeVersion := fmt.Sprintf("%s-v2", mock.Version)
+			path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = upgradeVersion
+			err := path.EndpointA.ChanUpgradeInit()
+			suite.Require().NoError(err)
+
+			// ensure proof verification succeeds
+			err = path.EndpointB.UpdateClient()
+			suite.Require().NoError(err)
+
+			proofChannel, proofUpgrade, proofHeight := path.EndpointB.QueryChannelUpgradeProof()
+			counterpartyChannel = path.EndpointA.GetChannel()
+
+			var found bool
+			counterpartyUpgrade, found = path.EndpointA.Chain.App.GetIBCKeeper().ChannelKeeper.GetUpgrade(path.EndpointA.Chain.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+			suite.Require().True(found)
+
+			// ensure that the channel has a valid upgrade sequence
+			channel := path.EndpointB.GetChannel()
+			channel.UpgradeSequence = 1
+			path.EndpointB.SetChannel(channel)
+
+			upgrade = types.Upgrade{
+				Fields: types.UpgradeFields{
+					Ordering:       types.UNORDERED,
+					ConnectionHops: []string{path.EndpointB.ConnectionID},
+					Version:        upgradeVersion,
+				},
+				Timeout:            types.NewTimeout(path.EndpointA.Chain.GetTimeoutHeight(), 0),
+				LatestSequenceSend: 1,
+			}
+
+			tc.malleate()
+
+			err = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.StartFlushUpgradeHandshake(
+				suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, upgrade.Fields,
+				counterpartyChannel, counterpartyUpgrade, proofChannel, proofUpgrade, proofHeight,
+			)
+
+			if tc.expError != nil {
+				suite.Require().Error(err)
+				suite.assertUpgradeError(err, tc.expError)
+			} else {
+				suite.Require().NoError(err)
+			}
+		})
+	}
+}
+
 func (suite *KeeperTestSuite) TestValidateProposedUpgradeFields() {
 	var (
 		proposedUpgrade *types.UpgradeFields
 		path            *ibctesting.Path
 	)
-
 	tests := []struct {
 		name     string
 		malleate func()
@@ -203,3 +377,15 @@ func (suite *KeeperTestSuite) TestValidateProposedUpgradeFields() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) assertUpgradeError(actualError, expError error) {
+	suite.Require().Error(actualError)
+
+	if expUpgradeError, ok := expError.(*types.UpgradeError); ok {
+		upgradeError, ok := actualError.(*types.UpgradeError)
+		suite.Require().True(ok)
+		suite.Require().Equal(expUpgradeError.GetErrorReceipt(), upgradeError.GetErrorReceipt())
+	}
+
+	suite.Require().True(errorsmod.IsOf(actualError, expError), actualError)
+}
diff --git a/modules/core/04-channel/types/errors.go b/modules/core/04-channel/types/errors.go
@@ -37,11 +37,12 @@ var (
 	// Perform a no-op on the current Msg
 	ErrNoOpMsg = errorsmod.Register(SubModuleName, 23, "message is redundant, no-op will be performed")
 
-	ErrInvalidChannelVersion  = errorsmod.Register(SubModuleName, 24, "invalid channel version")
-	ErrPacketNotSent          = errorsmod.Register(SubModuleName, 25, "packet has not been sent")
-	ErrInvalidTimeout         = errorsmod.Register(SubModuleName, 26, "invalid packet timeout")
-	ErrUpgradeErrorNotFound   = errorsmod.Register(SubModuleName, 27, "upgrade error receipt not found")
-	ErrInvalidUpgrade         = errorsmod.Register(SubModuleName, 28, "invalid upgrade")
-	ErrInvalidUpgradeSequence = errorsmod.Register(SubModuleName, 29, "invalid upgrade sequence")
-	ErrUpgradeNotFound        = errorsmod.Register(SubModuleName, 30, "upgrade not found")
+	ErrInvalidChannelVersion           = errorsmod.Register(SubModuleName, 24, "invalid channel version")
+	ErrPacketNotSent                   = errorsmod.Register(SubModuleName, 25, "packet has not been sent")
+	ErrInvalidTimeout                  = errorsmod.Register(SubModuleName, 26, "invalid packet timeout")
+	ErrUpgradeErrorNotFound            = errorsmod.Register(SubModuleName, 27, "upgrade error receipt not found")
+	ErrInvalidUpgrade                  = errorsmod.Register(SubModuleName, 28, "invalid upgrade")
+	ErrInvalidUpgradeSequence          = errorsmod.Register(SubModuleName, 29, "invalid upgrade sequence")
+	ErrUpgradeNotFound                 = errorsmod.Register(SubModuleName, 30, "upgrade not found")
+	ErrIncompatibleCounterpartyUpgrade = errorsmod.Register(SubModuleName, 31, "incompatible counterparty upgrade")
 )
diff --git a/modules/core/04-channel/types/expected_keepers.go b/modules/core/04-channel/types/expected_keepers.go
@@ -71,6 +71,15 @@ type ConnectionKeeper interface {
 		channelID string,
 		nextSequenceRecv uint64,
 	) error
+	VerifyChannelUpgrade(
+		ctx sdk.Context,
+		connection exported.ConnectionI,
+		height exported.Height,
+		proof []byte,
+		portID,
+		channelID string,
+		upgrade Upgrade,
+	) error
 }
 
 // PortKeeper expected account IBC port keeper