feat: adding chanUpgradeConfirm implementation to 04-channel (#4373)

* chore: remove restore logic in try handler, fast forward upgrade sequence in try hander * chore: fix linter * disable acknowledmgent test where try upgrade is called * fixing failing msg server test for chanUpgradeTry * wip: update write upgrade try func to no longer write channel as TRYUPGRADE. no longer set flush status * refactor: remove channel flush status from ack msg and handler * refactor: remove channel flush status from ack msg and handler * fix: address test acknowledgement failing testcase, replace flush status with channel state assertion * fix: linter crying * adding back failure testcase for AcknowledgePacket * updating testcase name to be more reflective of channel state * refactor: update chanUpgradeAck as per spec changes * rm commented out lines of code in write try func * address todo for handling packet acks in correct channel state * chore: fixing ack tests * chore: remove unneeded comment * chore: removed previous state log entry * Update modules/core/04-channel/keeper/upgrade.go Co-authored-by: Damian Nolan <[email protected]> * block comment code to be moved and link issue, uncomment previously disabled tests * addding ChanUpgradeConfirm implementation with test suite --------- Co-authored-by: chatton <[email protected]> Co-authored-by: Cian Hatton <[email protected]>
cosmos · Aug 17, 2023 · 2e8b370 · 2e8b370
1 parent e2c8326
commit 2e8b370
Show file tree

Hide file tree

Showing 4 changed files with 278 additions and 39 deletions.
diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go
@@ -331,6 +331,80 @@ func (k Keeper) WriteUpgradeAckChannel(ctx sdk.Context, portID, channelID string
 	emitChannelUpgradeAckEvent(ctx, portID, channelID, channel, upgrade)
 }
 
+// ChanUpgradeConfirm is called on the chain which is on FLUSHING after chanUpgradeAck is called on the counterparty.
+// This will inform the TRY chain of the timeout set on ACK by the counterparty. If the timeout has already exceeded, we will write an error receipt and restore.
+func (k Keeper) ChanUpgradeConfirm(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+	counterpartyChannelState types.State,
+	counterpartyUpgrade types.Upgrade,
+	proofChannel,
+	proofUpgrade []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)
+	}
+
+	if channel.State != types.STATE_FLUSHING {
+		return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected %s, got %s", types.STATE_FLUSHING, channel.State)
+	}
+
+	if !collections.Contains(counterpartyChannelState, []types.State{types.STATE_FLUSHING, types.STATE_FLUSHCOMPLETE}) {
+		return errorsmod.Wrapf(types.ErrInvalidCounterparty, "expected one of [%s, %s], got %s", types.STATE_FLUSHING, types.STATE_FLUSHCOMPLETE, counterpartyChannelState)
+	}
+
+	connection, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0])
+	if !found {
+		return errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0])
+	}
+
+	if connection.GetState() != int32(connectiontypes.OPEN) {
+		return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectiontypes.State(connection.GetState()).String())
+	}
+
+	counterpartyHops := []string{connection.GetCounterparty().GetConnectionID()}
+	counterpartyChannel := types.Channel{
+		State:           counterpartyChannelState,
+		Ordering:        channel.Ordering,
+		ConnectionHops:  counterpartyHops,
+		Counterparty:    types.NewCounterparty(portID, channelID),
+		Version:         channel.Version,
+		UpgradeSequence: channel.UpgradeSequence,
+	}
+
+	if err := k.connectionKeeper.VerifyChannelState(
+		ctx,
+		connection,
+		proofHeight, proofChannel,
+		channel.Counterparty.PortId,
+		channel.Counterparty.ChannelId,
+		counterpartyChannel,
+	); err != nil {
+		return errorsmod.Wrap(err, "failed to verify counterparty channel state")
+	}
+
+	if err := k.connectionKeeper.VerifyChannelUpgrade(
+		ctx,
+		channel.Counterparty.PortId,
+		channel.Counterparty.ChannelId,
+		connection,
+		counterpartyUpgrade,
+		proofUpgrade, proofHeight,
+	); err != nil {
+		return errorsmod.Wrap(err, "failed to verify counterparty upgrade")
+	}
+
+	timeout := counterpartyUpgrade.Timeout
+	if hasPassed, err := timeout.HasPassed(ctx); hasPassed {
+		return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrap(err, "counterparty upgrade timeout has passed"))
+	}
+
+	return nil
+}
+
 // WriteUpgradeConfirmChannel writes a channel which has successfully passed the ChanUpgradeConfirm handshake step.
 // If the channel has no in-flight packets, its state is updated to indicate that flushing has completed. Otherwise, the counterparty upgrade is set
 // and the channel state is left unchanged.

diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go
@@ -672,6 +672,173 @@ func (suite *KeeperTestSuite) TestWriteChannelUpgradeAck() {
 	}
 }
 
+func (suite *KeeperTestSuite) TestChanUpgradeConfirm() {
+	var (
+		path                     *ibctesting.Path
+		counterpartyChannelState types.State
+		counterpartyUpgrade      types.Upgrade
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expError error
+	}{
+		{
+			"success",
+			func() {},
+			nil,
+		},
+		{
+			"success with later upgrade sequence",
+			func() {
+				channel := path.EndpointA.GetChannel()
+				channel.UpgradeSequence = 10
+				path.EndpointA.SetChannel(channel)
+
+				channel = path.EndpointB.GetChannel()
+				channel.UpgradeSequence = 10
+				path.EndpointB.SetChannel(channel)
+
+				suite.coordinator.CommitBlock(suite.chainA, suite.chainB)
+
+				err := path.EndpointB.UpdateClient()
+				suite.Require().NoError(err)
+			},
+			nil,
+		},
+		{
+			"channel not found",
+			func() {
+				path.EndpointB.ChannelID = ibctesting.InvalidID
+				path.EndpointB.ChannelConfig.PortID = ibctesting.InvalidID
+			},
+			types.ErrChannelNotFound,
+		},
+		{
+			"channel is not in FLUSHING state",
+			func() {
+				err := path.EndpointB.SetChannelState(types.CLOSED)
+				suite.Require().NoError(err)
+			},
+			types.ErrInvalidChannelState,
+		},
+		{
+			"invalid counterparty channel state",
+			func() {
+				counterpartyChannelState = types.CLOSED
+			},
+			types.ErrInvalidCounterparty,
+		},
+		{
+			"connection not found",
+			func() {
+				channel := path.EndpointB.GetChannel()
+				channel.ConnectionHops = []string{"connection-100"}
+				path.EndpointB.SetChannel(channel)
+			},
+			connectiontypes.ErrConnectionNotFound,
+		},
+		{
+			"invalid connection state",
+			func() {
+				connectionEnd := path.EndpointB.GetConnection()
+				connectionEnd.State = connectiontypes.UNINITIALIZED
+				path.EndpointB.SetConnection(connectionEnd)
+			},
+			connectiontypes.ErrInvalidConnectionState,
+		},
+		{
+			"fails due to proof verification failure, counterparty channel ordering does not match expected ordering",
+			func() {
+				channel := path.EndpointA.GetChannel()
+				channel.Ordering = types.ORDERED
+				path.EndpointA.SetChannel(channel)
+
+				suite.coordinator.CommitBlock(suite.chainA)
+
+				err := path.EndpointB.UpdateClient()
+				suite.Require().NoError(err)
+			},
+			commitmenttypes.ErrInvalidProof,
+		},
+		{
+			"fails due to mismatch in upgrade ordering",
+			func() {
+				upgrade := path.EndpointA.GetChannelUpgrade()
+				upgrade.Fields.Ordering = types.NONE
+
+				path.EndpointA.SetChannelUpgrade(upgrade)
+
+				suite.coordinator.CommitBlock(suite.chainA)
+
+				err := path.EndpointB.UpdateClient()
+				suite.Require().NoError(err)
+			},
+			commitmenttypes.ErrInvalidProof,
+		},
+		{
+			"counterparty timeout has elapsed",
+			func() {
+				// Need to set counterparty upgrade in state and update clients to ensure
+				// proofs submitted reflect the altered upgrade.
+				counterpartyUpgrade.Timeout = types.NewTimeout(clienttypes.NewHeight(0, 1), 0)
+				path.EndpointA.SetChannelUpgrade(counterpartyUpgrade)
+
+				suite.coordinator.CommitBlock(suite.chainA)
+
+				err := path.EndpointB.UpdateClient()
+				suite.Require().NoError(err)
+			},
+			types.NewUpgradeError(1, types.ErrInvalidUpgrade),
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+			suite.coordinator.Setup(path)
+
+			path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion
+			path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion
+
+			err := path.EndpointA.ChanUpgradeInit()
+			suite.Require().NoError(err)
+
+			err = path.EndpointB.ChanUpgradeTry()
+			suite.Require().NoError(err)
+
+			err = path.EndpointA.ChanUpgradeAck()
+			suite.Require().NoError(err)
+
+			err = path.EndpointB.UpdateClient()
+			suite.Require().NoError(err)
+
+			counterpartyChannelState = path.EndpointA.GetChannel().State
+			counterpartyUpgrade = path.EndpointA.GetChannelUpgrade()
+
+			tc.malleate()
+
+			proofChannel, proofUpgrade, proofHeight := path.EndpointB.QueryChannelUpgradeProof()
+
+			err = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.ChanUpgradeConfirm(
+				suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, counterpartyChannelState, counterpartyUpgrade,
+				proofChannel, proofUpgrade, proofHeight,
+			)
+
+			expPass := tc.expError == nil
+			if expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.assertUpgradeError(err, tc.expError)
+			}
+		})
+	}
+}
+
 // TODO: Uncomment and address testcases when appropriate, timeout logic currently causes failures
 // func (suite *KeeperTestSuite) TestChanUpgradeOpen() {
 // 	var path *ibctesting.Path

diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
@@ -841,15 +841,16 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh
 
 	ctx.Logger().Info("channel upgrade ack succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId)
 
-	// Move channel to OPEN state if both chains have finished flushing any in-flight packets. Counterparty flush status
-	// has been verified in ChanUpgradeAck.
-	if !k.ChannelKeeper.HasInflightPackets(ctx, msg.PortId, msg.ChannelId) {
-		cbs.OnChanUpgradeOpen(ctx, msg.PortId, msg.ChannelId)
+	// TODO: Move this block of code to the confirm handler (https://github.com/cosmos/ibc-go/issues/4268)
+	// // Move channel to OPEN state if both chains have finished flushing any in-flight packets. Counterparty flush status
+	// // has been verified in ChanUpgradeAck.
+	// if !k.ChannelKeeper.HasInflightPackets(ctx, msg.PortId, msg.ChannelId) {
+	// 	cbs.OnChanUpgradeOpen(ctx, msg.PortId, msg.ChannelId)
 
-		k.ChannelKeeper.WriteUpgradeOpenChannel(ctx, msg.PortId, msg.ChannelId)
+	// 	k.ChannelKeeper.WriteUpgradeOpenChannel(ctx, msg.PortId, msg.ChannelId)
 
-		ctx.Logger().Info("channel upgrade open succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId)
-	}
+	// 	ctx.Logger().Info("channel upgrade open succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId)
+	// }
 
 	return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.SUCCESS}, nil
 }

diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go
@@ -893,40 +893,37 @@ func (suite *KeeperTestSuite) TestChannelUpgradeAck() {
 		malleate  func()
 		expResult func(res *channeltypes.MsgChannelUpgradeAckResponse, err error)
 	}{
-		// TODO: uncomment and handle failing tests
-		// {
-		// 	"success, no pending in-flight packets",
-		// 	func() {},
-		// 	func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
-		// 		suite.Require().NoError(err)
-		// 		suite.Require().NotNil(res)
-		// 		suite.Require().Equal(channeltypes.SUCCESS, res.Result)
+		{
+			"success, no pending in-flight packets",
+			func() {},
+			func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
+				suite.Require().NoError(err)
+				suite.Require().NotNil(res)
+				suite.Require().Equal(channeltypes.SUCCESS, res.Result)
 
-		// 		channel := path.EndpointA.GetChannel()
-		// 		suite.Require().Equal(channeltypes.OPEN, channel.State)
-		// 		suite.Require().Equal(uint64(1), channel.UpgradeSequence)
-		// 		suite.Require().Equal(channeltypes.NOTINFLUSH, channel.FlushStatus)
-		// 	},
-		// },
-		// {
-		// 	"success, pending in-flight packets",
-		// 	func() {
-		// 		portID := path.EndpointA.ChannelConfig.PortID
-		// 		channelID := path.EndpointA.ChannelID
-		// 		// Set a dummy packet commitment to simulate in-flight packets
-		// 		suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), portID, channelID, 1, []byte("hash"))
-		// 	},
-		// 	func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
-		// 		suite.Require().NoError(err)
-		// 		suite.Require().NotNil(res)
-		// 		suite.Require().Equal(channeltypes.SUCCESS, res.Result)
+				channel := path.EndpointA.GetChannel()
+				suite.Require().Equal(channeltypes.STATE_FLUSHCOMPLETE, channel.State)
+				suite.Require().Equal(uint64(1), channel.UpgradeSequence)
+			},
+		},
+		{
+			"success, pending in-flight packets",
+			func() {
+				portID := path.EndpointA.ChannelConfig.PortID
+				channelID := path.EndpointA.ChannelID
+				// Set a dummy packet commitment to simulate in-flight packets
+				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), portID, channelID, 1, []byte("hash"))
+			},
+			func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
+				suite.Require().NoError(err)
+				suite.Require().NotNil(res)
+				suite.Require().Equal(channeltypes.SUCCESS, res.Result)
 
-		// 		channel := path.EndpointA.GetChannel()
-		// 		suite.Require().Equal(channeltypes.ACKUPGRADE, channel.State)
-		// 		suite.Require().Equal(uint64(1), channel.UpgradeSequence)
-		// 		suite.Require().Equal(channeltypes.FLUSHING, channel.FlushStatus)
-		// 	},
-		// },
+				channel := path.EndpointA.GetChannel()
+				suite.Require().Equal(channeltypes.STATE_FLUSHING, channel.State)
+				suite.Require().Equal(uint64(1), channel.UpgradeSequence)
+			},
+		},
 		{
 			"module capability not found",
 			func() {