feat: adding msg server implementation for ChannelUpgradeAck (#3849)

* adding boilerplate skeleton for chanUpgradeAck handler * updating msg servers args * adding test scaffolding and syncing latest changes of feat branch * configure both proposed upgrades to use mock.UpgradeVersion * updating chanUpgradeAck test cases * updating var naming for consistency, adding additional testcases * rm msg server implementation * adding invalid flush status err and rm lint ignore comment * adding test helpers to endpoint for get/set channel upgrade * lint it * adding initial msg server impl skeleton * pull in code for WriteUpgradeAckChannel * adding result to MsgChannelUpgradeAckResponse * add initial test cases * adding additional testcases * apply testcase naming review suggestions Co-authored-by: Carlos Rodriguez <carlos@interchain.io> * apply error return wrapping suggestions from review Co-authored-by: Carlos Rodriguez <carlos@interchain.io> * fix error to use Wrapf and correct channel id arg, adding success log * correct testing imports and satisy linter * apply self suggestions for testcase context with in-line comments * updating test func to use path.EndpointA and chainA sender acc --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
cosmos · Jun 21, 2023 · f734e49 · f734e49
1 parent ea0ded0
commit f734e49
Show file tree

Hide file tree

Showing 3 changed files with 231 additions and 27 deletions.
diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go
@@ -205,29 +205,6 @@ func (k Keeper) WriteUpgradeTryChannel(ctx sdk.Context, portID, channelID string
 	return channel, upgrade
 }
 
-// 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, newUpgradeSequence uint64) {
-	defer telemetry.IncrCounter(1, "ibc", "channel", "upgrade-cancel")
-
-	upgrade, found := k.GetUpgrade(ctx, portID, channelID)
-	if !found {
-		panic(fmt.Sprintf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID))
-	}
-
-	channel, found := k.GetChannel(ctx, portID, channelID)
-	if !found {
-		panic(fmt.Sprintf("could not find existing channel when updating channel state, channelID: %s, portID: %s", channelID, portID))
-	}
-
-	previousState := channel.State
-
-	k.restoreChannel(ctx, portID, channelID, newUpgradeSequence, channel)
-
-	k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", types.OPEN.String())
-	emitChannelUpgradeCancelEvent(ctx, portID, channelID, channel, upgrade)
-}
-
 // ChanUpgradeAck is called by a module to accept the ACKUPGRADE handshake step of the channel upgrade protocol.
 // This method should only be called by the IBC core msg server.
 // This method will verify that the counterparty has entered TRYUPGRADE
@@ -297,7 +274,7 @@ func (k Keeper) ChanUpgradeAck(
 func (k Keeper) WriteUpgradeAckChannel(
 	ctx sdk.Context,
 	portID, channelID string,
-	proposedUpgrade types.Upgrade,
+	upgradeVersion string,
 ) {
 	defer telemetry.IncrCounter(1, "ibc", "channel", "upgrade-ack")
 
@@ -311,10 +288,18 @@ func (k Keeper) WriteUpgradeAckChannel(
 	channel.FlushStatus = types.FLUSHING
 
 	k.SetChannel(ctx, portID, channelID, channel)
-	k.SetUpgrade(ctx, portID, channelID, proposedUpgrade)
+
+	upgrade, found := k.GetUpgrade(ctx, portID, channelID)
+	if !found {
+		panic(fmt.Sprintf("cound not find existing upgrade when updating channel state in successful ChanUpgradeAck step, channelID: %s, portID: %s", channelID, portID))
+	}
+
+	upgrade.Fields.Version = upgradeVersion
+
+	k.SetUpgrade(ctx, portID, channelID, upgrade)
 
 	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)
+	emitChannelUpgradeAckEvent(ctx, portID, channelID, channel, upgrade)
 }
 
 // ChanUpgradeCancel is called by a module to cancel a channel upgrade that is in progress.
@@ -357,6 +342,29 @@ func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, err
 	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, newUpgradeSequence uint64) {
+	defer telemetry.IncrCounter(1, "ibc", "channel", "upgrade-cancel")
+
+	upgrade, found := k.GetUpgrade(ctx, portID, channelID)
+	if !found {
+		panic(fmt.Sprintf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID))
+	}
+
+	channel, found := k.GetChannel(ctx, portID, channelID)
+	if !found {
+		panic(fmt.Sprintf("could not find existing channel when updating channel state, channelID: %s, portID: %s", channelID, portID))
+	}
+
+	previousState := channel.State
+
+	k.restoreChannel(ctx, portID, channelID, newUpgradeSequence, channel)
+
+	k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", types.OPEN.String())
+	emitChannelUpgradeCancelEvent(ctx, portID, channelID, channel, upgrade)
+}
+
 // 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(

diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
@@ -805,7 +805,55 @@ func (k Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgCh
 
 // ChannelUpgradeAck defines a rpc handler method for MsgChannelUpgradeAck.
 func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgChannelUpgradeAck) (*channeltypes.MsgChannelUpgradeAckResponse, error) {
-	return nil, nil
+	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	module, _, err := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.PortId, msg.ChannelId)
+	if err != nil {
+		ctx.Logger().Error("channel upgrade ack failed", "port-id", msg.PortId, "error", errorsmod.Wrap(err, "could not retrieve module from port-id"))
+		return nil, errorsmod.Wrap(err, "could not retrieve module from port-id")
+	}
+
+	cbs, ok := k.Router.GetRoute(module)
+	if !ok {
+		ctx.Logger().Error("channel upgrade ack failed", "port-id", msg.PortId, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module))
+		return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module)
+	}
+
+	err = k.ChannelKeeper.ChanUpgradeAck(ctx, msg.PortId, msg.ChannelId, msg.CounterpartyFlushStatus, msg.CounterpartyUpgrade, msg.ProofChannel, msg.ProofUpgrade, msg.ProofHeight)
+	if err != nil {
+		ctx.Logger().Error("channel upgrade ack failed", "error", errorsmod.Wrap(err, "channel handshake upgrade ack failed"))
+		if upgradeErr, ok := err.(*channeltypes.UpgradeError); ok {
+			if err := k.ChannelKeeper.AbortUpgrade(ctx, msg.PortId, msg.ChannelId, upgradeErr); err != nil {
+				return nil, errorsmod.Wrap(err, "channel upgrade ack (abort upgrade) failed")
+			}
+
+			// NOTE: a FAILURE result is returned to the client and an error receipt is written to state.
+			// This signals to the relayer to begin the cancel upgrade handshake subprotocol.
+			return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.FAILURE}, nil
+		}
+
+		// NOTE: an error is returned to baseapp and transaction state is not committed.
+		return nil, errorsmod.Wrap(err, "channel upgrade ack failed")
+	}
+
+	cacheCtx, writeFn := ctx.CacheContext()
+	err = cbs.OnChanUpgradeAck(cacheCtx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade.Fields.Version)
+	if err != nil {
+		ctx.Logger().Error("channel upgrade ack callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error())
+		if err := k.ChannelKeeper.AbortUpgrade(ctx, msg.PortId, msg.ChannelId, err); err != nil {
+			return nil, errorsmod.Wrapf(err, "channel upgrade ack callback (abort upgrade) failed for port ID: %s, channel ID: %s", msg.PortId, msg.ChannelId)
+		}
+
+		return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.FAILURE}, nil
+	}
+
+	writeFn()
+
+	k.ChannelKeeper.WriteUpgradeAckChannel(ctx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade.Fields.Version)
+
+	ctx.Logger().Info("channel upgrade ack succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId)
+
+	return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.SUCCESS}, nil
 }
 
 // ChannelUpgradeOpen defines a rpc handler method for MsgChannelUpgradeOpen.

diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go
@@ -925,6 +925,154 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTry() {
 	}
 }
 
+func (suite *KeeperTestSuite) TestChannelUpgradeAck() {
+	var (
+		path *ibctesting.Path
+		msg  *channeltypes.MsgChannelUpgradeAck
+	)
+
+	cases := []struct {
+		name      string
+		malleate  func()
+		expResult func(res *channeltypes.MsgChannelUpgradeAckResponse, err error)
+	}{
+		{
+			"success",
+			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.ACKUPGRADE, channel.State)
+				suite.Require().Equal(uint64(1), channel.UpgradeSequence)
+			},
+		},
+		{
+			"module capability not found",
+			func() {
+				msg.PortId = ibctesting.InvalidID
+				msg.ChannelId = ibctesting.InvalidID
+			},
+			func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
+				suite.Require().Error(err)
+				suite.Require().Nil(res)
+
+				suite.Require().ErrorIs(err, capabilitytypes.ErrCapabilityNotFound)
+			},
+		},
+		{
+			"core handler returns error and no upgrade error receipt is written",
+			func() {
+				// force an error by overriding the counterparty flush status to an invalid value
+				msg.CounterpartyFlushStatus = channeltypes.NOTINFLUSH
+			},
+			func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
+				suite.Require().Error(err)
+				suite.Require().Nil(res)
+				suite.Require().ErrorIs(err, channeltypes.ErrInvalidFlushStatus)
+
+				errorReceipt, found := suite.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				suite.Require().Empty(errorReceipt)
+				suite.Require().False(found)
+			},
+		},
+		{
+			"core handler returns error and writes upgrade error receipt",
+			func() {
+				// force an upgrade error by modifying the channel upgrade ordering to an incompatible value
+				upgrade := path.EndpointA.GetChannelUpgrade()
+				upgrade.Fields.Ordering = channeltypes.NONE
+
+				path.EndpointA.SetChannelUpgrade(upgrade)
+			},
+			func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
+				suite.Require().NoError(err)
+
+				suite.Require().NotNil(res)
+				suite.Require().Equal(channeltypes.FAILURE, res.Result)
+
+				errorReceipt, found := suite.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				suite.Require().True(found)
+				suite.Require().Equal(uint64(1), errorReceipt.Sequence)
+			},
+		},
+		{
+			"application callback returns error and error receipt is written",
+			func() {
+				suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeAck = func(
+					ctx sdk.Context, portID, channelID, counterpartyVersion string,
+				) error {
+					// set arbitrary value in store to mock application state changes
+					store := ctx.KVStore(suite.chainA.GetSimApp().GetKey(exported.ModuleName))
+					store.Set([]byte("foo"), []byte("bar"))
+					return fmt.Errorf("mock app callback failed")
+				}
+			},
+			func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) {
+				suite.Require().NoError(err)
+
+				suite.Require().NotNil(res)
+				suite.Require().Equal(channeltypes.FAILURE, res.Result)
+
+				errorReceipt, found := suite.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				suite.Require().True(found)
+				suite.Require().Equal(uint64(1), errorReceipt.Sequence)
+
+				// assert application state changes are not committed
+				store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(exported.ModuleName))
+				suite.Require().False(store.Has([]byte("foo")))
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+			suite.coordinator.Setup(path)
+
+			// configure the channel upgrade version on testing endpoints
+			path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion
+			path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion
+
+			err := path.EndpointA.ChanUpgradeInit()
+			suite.Require().NoError(err)
+
+			err = path.EndpointB.ChanUpgradeTry()
+			suite.Require().NoError(err)
+
+			err = path.EndpointA.UpdateClient()
+			suite.Require().NoError(err)
+
+			counterpartyChannel := path.EndpointB.GetChannel()
+			counterpartyUpgrade := path.EndpointB.GetChannelUpgrade()
+
+			proofChannel, proofUpgrade, proofHeight := path.EndpointA.QueryChannelUpgradeProof()
+
+			msg = &channeltypes.MsgChannelUpgradeAck{
+				PortId:                  path.EndpointA.ChannelConfig.PortID,
+				ChannelId:               path.EndpointA.ChannelID,
+				CounterpartyFlushStatus: counterpartyChannel.FlushStatus,
+				CounterpartyUpgrade:     counterpartyUpgrade,
+				ProofChannel:            proofChannel,
+				ProofUpgrade:            proofUpgrade,
+				ProofHeight:             proofHeight,
+				Signer:                  suite.chainA.SenderAccount.GetAddress().String(),
+			}
+
+			tc.malleate()
+
+			res, err := suite.chainA.GetSimApp().GetIBCKeeper().ChannelUpgradeAck(suite.chainA.GetContext(), msg)
+
+			tc.expResult(res, err)
+		})
+	}
+}
+
 func (suite *KeeperTestSuite) TestChannelUpgradeCancel() {
 	var (
 		path *ibctesting.Path