chore: refactor chanUpgradeAck tests to use expected errors

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

@@ -205,75 +205,6 @@ func (k Keeper) WriteUpgradeTryChannel(ctx sdk.Context, portID, channelID string
 	return channel, upgrade
 }
 
-// WriteUpgradeAckChannel writes a channel which has successfully passed the UpgradeAck handshake step as well as
-// setting the upgrade for that channel.
-// An event is emitted for the handshake step.
-func (k Keeper) WriteUpgradeAckChannel(
-	ctx sdk.Context,
-	portID, channelID string,
-	proposedUpgrade types.Upgrade,
-) {
-	defer telemetry.IncrCounter(1, "ibc", "channel", "upgrade-ack")
-
-	channel, found := k.GetChannel(ctx, portID, channelID)
-	if !found {
-		panic(fmt.Sprintf("could not find existing channel when updating channel state in successful ChanUpgradeAck step, channelID: %s, portID: %s", channelID, portID))
-	}
-
-	previousState := channel.State
-	channel.State = types.ACKUPGRADE
-	channel.FlushStatus = types.FLUSHING
-
-	k.SetChannel(ctx, portID, channelID, channel)
-	k.SetUpgrade(ctx, portID, channelID, proposedUpgrade)
-
-	k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", types.ACKUPGRADE.String())
-	emitChannelUpgradeAckEvent(ctx, portID, channelID, channel, proposedUpgrade)
-}
-
-// ChanUpgradeCancel is called by a module to cancel a channel upgrade that is in progress.
-func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt, errorReceiptProof []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)
-	}
-
-	// the channel state must be in INITUPGRADE or TRYUPGRADE
-	if !collections.Contains(channel.State, []types.State{types.INITUPGRADE, types.TRYUPGRADE}) {
-		return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected one of [%s, %s], got %s", types.INITUPGRADE, types.TRYUPGRADE, channel.State)
-	}
-
-	// get underlying connection for proof verification
-	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(),
-		)
-	}
-
-	if err := k.connectionKeeper.VerifyChannelUpgradeError(ctx, portID, channelID, connection, errorReceipt, errorReceiptProof, proofHeight); err != nil {
-		return errorsmod.Wrap(err, "failed to verify counterparty error receipt")
-	}
-
-	// If counterparty sequence is less than the current sequence, abort the transaction since this error receipt is from a previous upgrade.
-	// Otherwise, set our upgrade sequence to the counterparty's error sequence + 1 so that both sides start with a fresh sequence.
-	currentSequence := channel.UpgradeSequence
-	counterpartySequence := errorReceipt.Sequence
-	if counterpartySequence < currentSequence {
-		return errorsmod.Wrapf(types.ErrInvalidUpgradeSequence, "error receipt sequence (%d) must be greater than or equal to current sequence (%d)", counterpartySequence, currentSequence)
-	}
-
-	channel.UpgradeSequence = errorReceipt.Sequence + 1
-	k.SetChannel(ctx, portID, channelID, channel)
-
-	return nil
-}
-
 // WriteUpgradeCancelChannel writes a channel which has canceled the upgrade process.Auxiliary upgrade state is
 // also deleted.
 func (k Keeper) WriteUpgradeCancelChannel(ctx sdk.Context, portID, channelID string) {
@@ -360,6 +291,75 @@ func (k Keeper) ChanUpgradeAck(
 	return nil
 }
 
+// WriteUpgradeAckChannel writes a channel which has successfully passed the UpgradeAck handshake step as well as
+// setting the upgrade for that channel.
+// An event is emitted for the handshake step.
+func (k Keeper) WriteUpgradeAckChannel(
+	ctx sdk.Context,
+	portID, channelID string,
+	proposedUpgrade types.Upgrade,
+) {
+	defer telemetry.IncrCounter(1, "ibc", "channel", "upgrade-ack")
+
+	channel, found := k.GetChannel(ctx, portID, channelID)
+	if !found {
+		panic(fmt.Sprintf("could not find existing channel when updating channel state in successful ChanUpgradeAck step, channelID: %s, portID: %s", channelID, portID))
+	}
+
+	previousState := channel.State
+	channel.State = types.ACKUPGRADE
+	channel.FlushStatus = types.FLUSHING
+
+	k.SetChannel(ctx, portID, channelID, channel)
+	k.SetUpgrade(ctx, portID, channelID, proposedUpgrade)
+
+	k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", types.ACKUPGRADE.String())
+	emitChannelUpgradeAckEvent(ctx, portID, channelID, channel, proposedUpgrade)
+}
+
+// ChanUpgradeCancel is called by a module to cancel a channel upgrade that is in progress.
+func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt, errorReceiptProof []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)
+	}
+
+	// the channel state must be in INITUPGRADE or TRYUPGRADE
+	if !collections.Contains(channel.State, []types.State{types.INITUPGRADE, types.TRYUPGRADE}) {
+		return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected one of [%s, %s], got %s", types.INITUPGRADE, types.TRYUPGRADE, channel.State)
+	}
+
+	// get underlying connection for proof verification
+	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(),
+		)
+	}
+
+	if err := k.connectionKeeper.VerifyChannelUpgradeError(ctx, portID, channelID, connection, errorReceipt, errorReceiptProof, proofHeight); err != nil {
+		return errorsmod.Wrap(err, "failed to verify counterparty error receipt")
+	}
+
+	// If counterparty sequence is less than the current sequence, abort the transaction since this error receipt is from a previous upgrade.
+	// Otherwise, set our upgrade sequence to the counterparty's error sequence + 1 so that both sides start with a fresh sequence.
+	currentSequence := channel.UpgradeSequence
+	counterpartySequence := errorReceipt.Sequence
+	if counterpartySequence < currentSequence {
+		return errorsmod.Wrapf(types.ErrInvalidUpgradeSequence, "error receipt sequence (%d) must be greater than or equal to current sequence (%d)", counterpartySequence, currentSequence)
+	}
+
+	channel.UpgradeSequence = errorReceipt.Sequence + 1
+	k.SetChannel(ctx, portID, channelID, channel)
+
+	return nil
+}
+
 // ChanUpgradeTimeout times out an outstanding upgrade.
 // This should be used by the initialising chain when the counterparty chain has not responded to an upgrade proposal within the specified timeout period.
 func (k Keeper) ChanUpgradeTimeout(

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

@@ -336,12 +336,12 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
 	testCases := []struct {
 		name     string
 		malleate func()
-		expPass  bool
+		expError error
 	}{
 		{
 			"success",
 			func() {},
-			true,
+			nil,
 		},
 		{
 			"success with later upgrade sequence",
@@ -359,29 +359,29 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
 				err := path.EndpointA.UpdateClient()
 				suite.Require().NoError(err)
 			},
-			true,
+			nil,
 		},
 		{
 			"channel not found",
 			func() {
 				path.EndpointA.ChannelID = ibctesting.InvalidID
 				path.EndpointA.ChannelConfig.PortID = ibctesting.InvalidID
 			},
-			false,
+			types.ErrChannelNotFound,
 		},
 		{
 			"channel state is not in INITUPGRADE or TRYUPGRADE state",
 			func() {
 				suite.Require().NoError(path.EndpointA.SetChannelState(types.CLOSED))
 			},
-			false,
+			types.ErrInvalidChannelState,
 		},
 		{
 			"counterparty flush status is not in FLUSHING or FLUSHCOMPLETE",
 			func() {
 				counterpartyFlushStatus = types.NOTINFLUSH
 			},
-			false,
+			types.ErrInvalidFlushStatus,
 		},
 		{
 			"connection not found",
@@ -390,7 +390,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
 				channel.ConnectionHops = []string{"connection-100"}
 				path.EndpointA.SetChannel(channel)
 			},
-			false,
+			connectiontypes.ErrConnectionNotFound,
 		},
 		{
 			"invalid connection state",
@@ -399,46 +399,47 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
 				connectionEnd.State = connectiontypes.UNINITIALIZED
 				path.EndpointA.SetConnection(connectionEnd)
 			},
-			false,
+			connectiontypes.ErrInvalidConnectionState,
 		},
 		{
 			"upgrade not found",
 			func() {
 				store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(exported.ModuleName))
 				store.Delete(host.ChannelUpgradeKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
 			},
-			false,
+			types.ErrUpgradeNotFound,
 		},
 		{
-			"channel end version mismatch on crossing hellos",
+			"startFlushUpgradeHandshake fails due to proof verification failure, counterparty upgrade connection hops are tampered with",
 			func() {
-				channel := path.EndpointA.GetChannel()
-				channel.State = types.TRYUPGRADE
-
-				path.EndpointA.SetChannel(channel)
-
-				upgrade := path.EndpointA.GetChannelUpgrade()
-				upgrade.Fields.Version = "invalid-version"
-
-				path.EndpointA.SetChannelUpgrade(upgrade)
+				counterpartyUpgrade.Fields.ConnectionHops = []string{ibctesting.InvalidID}
 			},
-			false,
+			commitmenttypes.ErrInvalidProof,
 		},
 		{
-			"startFlushUpgradeHandshake fails due to proof verification failure, counterparty upgrade connection hops are tampered with",
+			"startFlushUpgradeHandshake fails due to mismatch in upgrade ordering",
 			func() {
-				counterpartyUpgrade.Fields.ConnectionHops = []string{ibctesting.InvalidID}
+				upgrade := path.EndpointA.GetChannelUpgrade()
+				upgrade.Fields.Ordering = types.NONE
+
+				path.EndpointA.SetChannelUpgrade(upgrade)
 			},
-			false,
+			types.NewUpgradeError(1, types.ErrIncompatibleCounterpartyUpgrade),
 		},
 		{
-			"startFlushUpgradeHandshake fails due to mismatch in upgrade sequences",
+			"channel end version mismatch on crossing hellos",
 			func() {
 				channel := path.EndpointA.GetChannel()
-				channel.UpgradeSequence = 5
+				channel.State = types.TRYUPGRADE
+
 				path.EndpointA.SetChannel(channel)
+
+				upgrade := path.EndpointA.GetChannelUpgrade()
+				upgrade.Fields.Version = "invalid-version"
+
+				path.EndpointA.SetChannelUpgrade(upgrade)
 			},
-			false,
+			types.NewUpgradeError(1, types.ErrIncompatibleCounterpartyUpgrade),
 		},
 	}
 
@@ -476,10 +477,11 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
 				proofChannel, proofUpgrade, proofHeight,
 			)
 
-			if tc.expPass {
+			expPass := tc.expError == nil
+			if expPass {
 				suite.Require().NoError(err)
 			} else {
-				suite.Require().Error(err)
+				suite.assertUpgradeError(err, tc.expError)
 			}
 		})
 	}