ibc: upgrade client (cosmos#7367)

* implement upgrade module changes

* implement client changes

* implement core tendermint logic

* remove assumption that new client state has same structure as old

* fix light client builds

* fix rest of build

* fix tendermint tests

* fix all tests except MarshalJSON

* fix, marshalUpgrade fails

* Apply suggestions from code review

* minor updates

* update proto and validate path

* fix MarshalJSON panic

* hack my way to first passing test case

* add rest of upgrade test cases

* fix plan tests

* add keeper tests

* fix upgrade tests

* add more tests

* add upgrade path validation to ValidateSelfClient

* validate upgradedClient

* fix upgrade handler tests

* appease linter

* Apply suggestions from code review

Co-authored-by: Federico Kunze <[email protected]>

* change signer to string in proto

* lint

* start address @colin-axner review

* improve test coverage

* fix abci stringer test

Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>

.gitignore

@@ -19,6 +19,7 @@ docs/modules
 dist
 tools-stamp
 proto-tools-stamp
+buf-stamp
 artifacts
 
 # Data - ideally these don't exist

proto/cosmos/upgrade/v1beta1/upgrade.proto

@@ -1,6 +1,7 @@
 syntax = "proto3";
 package cosmos.upgrade.v1beta1;
 
+import "google/protobuf/any.proto";
 import "gogoproto/gogo.proto";
 import "google/protobuf/timestamp.proto";
 
@@ -32,6 +33,13 @@ message Plan {
   // Any application specific upgrade info to be included on-chain
   // such as a git commit that validators could automatically upgrade to
   string info = 4;
+
+  // IBC-enabled chains can opt-in to including the upgraded client state in its upgrade plan
+  // This will make the chain commit to the correct upgraded (self) client state before the upgrade occurs,
+  // so that connecting chains can verify that the new upgraded client is valid by verifying a proof on the
+  // previous version of the chain.
+  // This will allow IBC connections to persist smoothly across planned chain upgrades
+  google.protobuf.Any upgraded_client_state = 5 [(gogoproto.moretags) = "yaml:\"upgraded_client_state\""];;
 }
 
 // SoftwareUpgradeProposal is a gov Content type for initiating a software

proto/ibc/client/client.proto

@@ -78,6 +78,18 @@ message MsgUpdateClient {
   string signer = 3;
 }
 
+// MsgUpgradeClient defines an sdk.Msg to upgrade an IBC client to a new client state
+message MsgUpgradeClient {
+  // client unique identifier
+  string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
+  // upgraded client state
+  google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""];
+  // proof that old chain committed to new client
+  bytes proof_upgrade = 3 [(gogoproto.moretags) = "yaml:\"proof_upgrade\""];
+  // signer address
+  string signer = 4;
+}
+
 // MsgSubmitMisbehaviour defines an sdk.Msg type that submits Evidence for
 // light client misbehaviour.
 message MsgSubmitMisbehaviour {

proto/ibc/commitment/commitment.proto

@@ -49,13 +49,12 @@ message KeyPath {
 message Key {
   option (gogoproto.goproto_getters) = false;
 
-  bytes       name = 1 [(gogoproto.customname) = "name"];
-  KeyEncoding enc  = 2 [(gogoproto.customname) = "enc"];
+  bytes       name = 1;
+  KeyEncoding enc  = 2;
 }
 
 // KeyEncoding defines the encoding format of a key's bytes.
 enum KeyEncoding {
-  option (gogoproto.goproto_enum_stringer) = false;
   option (gogoproto.goproto_enum_prefix)   = false;
 
   // URL encoding

proto/ibc/tendermint/tendermint.proto

@@ -39,12 +39,15 @@ message ClientState {
   // Proof specifications used in verifying counterparty state
   repeated ics23.ProofSpec proof_specs = 8 [(gogoproto.moretags) = "yaml:\"proof_specs\""];
 
+  // Path at which next upgraded client will be committed
+  ibc.commitment.MerklePath upgrade_path = 9 [(gogoproto.moretags) = "yaml:\"upgrade_path\""];
+
   // This flag, when set to true, will allow governance to recover a client
   // which has expired
-  bool allow_update_after_expiry = 9 [(gogoproto.moretags) = "yaml:\"allow_update_after_expiry\""];
+  bool allow_update_after_expiry = 10 [(gogoproto.moretags) = "yaml:\"allow_update_after_expiry\""];
   // This flag, when set to true, will allow governance to unfreeze a client
   // whose chain has experienced a misbehaviour event
-  bool allow_update_after_misbehaviour = 10 [(gogoproto.moretags) = "yaml:\"allow_update_after_misbehaviour\""];
+  bool allow_update_after_misbehaviour = 11 [(gogoproto.moretags) = "yaml:\"allow_update_after_misbehaviour\""];
 }
 
 // ConsensusState defines the consensus state from Tendermint.

x/ibc/02-client/handler.go

@@ -65,6 +65,33 @@ func HandleMsgUpdateClient(ctx sdk.Context, k keeper.Keeper, msg *types.MsgUpdat
 	}, nil
 }
 
+// HandleMsgUpgradeClient defines the sdk.Handler for MsgUpgradeClient
+func HandleMsgUpgradeClient(ctx sdk.Context, k keeper.Keeper, msg *types.MsgUpgradeClient) (*sdk.Result, error) {
+	upgradedClient, err := types.UnpackClientState(msg.ClientState)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := upgradedClient.Validate(); err != nil {
+		return nil, err
+	}
+
+	if err = k.UpgradeClient(ctx, msg.ClientId, upgradedClient, msg.ProofUpgrade); err != nil {
+		return nil, err
+	}
+
+	ctx.EventManager().EmitEvent(
+		sdk.NewEvent(
+			sdk.EventTypeMessage,
+			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
+		),
+	)
+
+	return &sdk.Result{
+		Events: ctx.EventManager().Events().ToABCIEvents(),
+	}, nil
+}
+
 // HandleMsgSubmitMisbehaviour defines the Evidence module handler for submitting a
 // light client misbehaviour.
 func HandleMsgSubmitMisbehaviour(ctx sdk.Context, k keeper.Keeper, msg *types.MsgSubmitMisbehaviour) (*sdk.Result, error) {

x/ibc/02-client/handler_test.go

@@ -1,13 +1,20 @@
 package client_test
 
 import (
+	"time"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 	client "github.com/cosmos/cosmos-sdk/x/ibc/02-client"
+	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 	clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 	ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types"
+	commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types"
+	"github.com/cosmos/cosmos-sdk/x/ibc/exported"
+	solomachinetypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/solomachine/types"
 	ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing"
+	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
 func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() {
@@ -73,3 +80,152 @@ func (suite *ClientTestSuite) TestNewClientUpdateProposalHandler() {
 	}
 
 }
+
+func (suite *ClientTestSuite) TestUpgradeClient() {
+	var (
+		clientA        string
+		upgradedClient exported.ClientState
+		msg            *clienttypes.MsgUpgradeClient
+	)
+
+	upgradeHeight := clienttypes.NewHeight(1, 1)
+
+	cases := []struct {
+		name    string
+		setup   func()
+		expPass bool
+	}{
+		{
+			name: "successful upgrade",
+			setup: func() {
+
+				upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, upgradeHeight, commitmenttypes.GetSDKSpecs(), &ibctesting.UpgradePath, false, false)
+				// zero custom fields and store in upgrade store
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), upgradedClient)
+
+				// commit upgrade store changes and update clients
+
+				suite.coordinator.CommitBlock(suite.chainB)
+				err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
+				suite.Require().NoError(err)
+
+				cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
+				suite.Require().True(found)
+
+				proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())
+
+				msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, proofUpgrade, suite.chainA.SenderAccount.GetAddress())
+				suite.Require().NoError(err)
+			},
+			expPass: true,
+		},
+		{
+			name: "successful upgrade to different client type",
+			setup: func() {
+
+				// previous chain committed to the change
+				upgradedClient = ibctesting.NewSolomachine(suite.T(), suite.chainA.App.AppCodec(), clientA, "diversifier", 1).ClientState()
+				soloClient, _ := upgradedClient.(*solomachinetypes.ClientState)
+				// change sequence to be higher height than latest current client height
+				soloClient.Sequence = 100000000000
+				// zero custom fields and store in upgrade store
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), soloClient)
+
+				// commit upgrade store changes and update clients
+
+				suite.coordinator.CommitBlock(suite.chainB)
+				err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
+				suite.Require().NoError(err)
+
+				cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
+				suite.Require().True(found)
+
+				proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())
+
+				msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, proofUpgrade, suite.chainA.SenderAccount.GetAddress())
+				suite.Require().NoError(err)
+			},
+			expPass: true,
+		},
+		{
+			name: "invalid upgrade: msg.ClientState does not contain valid clientstate",
+			setup: func() {
+
+				cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
+				suite.Require().True(found)
+
+				proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())
+
+				consState := ibctmtypes.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("app_hash")), []byte("next_vals_hash"))
+				consAny, err := clienttypes.PackConsensusState(consState)
+				suite.Require().NoError(err)
+
+				msg = &types.MsgUpgradeClient{ClientId: clientA, ClientState: consAny, ProofUpgrade: proofUpgrade, Signer: suite.chainA.SenderAccount.GetAddress().String()}
+			},
+			expPass: false,
+		},
+		{
+			name: "invalid clientstate",
+			setup: func() {
+
+				upgradedClient = ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, upgradeHeight, commitmenttypes.GetSDKSpecs(), &ibctesting.UpgradePath, false, false)
+				// zero custom fields and store in upgrade store
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), upgradedClient)
+
+				// commit upgrade store changes and update clients
+
+				suite.coordinator.CommitBlock(suite.chainB)
+				err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
+				suite.Require().NoError(err)
+
+				cs, found := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
+				suite.Require().True(found)
+
+				proofUpgrade, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(), cs.GetLatestHeight().GetEpochHeight())
+
+				msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, proofUpgrade, suite.chainA.SenderAccount.GetAddress())
+				suite.Require().NoError(err)
+			},
+			expPass: false,
+		},
+		{
+			name: "VerifyUpgrade fails",
+			setup: func() {
+
+				upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod+ibctesting.TrustingPeriod, ibctesting.MaxClockDrift, upgradeHeight, commitmenttypes.GetSDKSpecs(), &ibctesting.UpgradePath, false, false)
+				// zero custom fields and store in upgrade store
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), upgradedClient)
+
+				// commit upgrade store changes and update clients
+
+				suite.coordinator.CommitBlock(suite.chainB)
+				err := suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, ibctesting.Tendermint)
+				suite.Require().NoError(err)
+
+				msg, err = clienttypes.NewMsgUpgradeClient(clientA, upgradedClient, nil, suite.chainA.SenderAccount.GetAddress())
+				suite.Require().NoError(err)
+			},
+			expPass: false,
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		clientA, _ = suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint)
+
+		tc.setup()
+
+		_, err := client.HandleMsgUpgradeClient(
+			suite.chainA.GetContext(), suite.chainA.App.IBCKeeper.ClientKeeper, msg,
+		)
+
+		if tc.expPass {
+			suite.Require().NoError(err, "upgrade handler failed on valid case: %s", tc.name)
+			newClient, ok := suite.chainA.App.IBCKeeper.ClientKeeper.GetClientState(suite.chainA.GetContext(), clientA)
+			suite.Require().True(ok)
+			suite.Require().Equal(upgradedClient, newClient)
+		} else {
+			suite.Require().Error(err, "upgrade handler passed on invalid case: %s", tc.name)
+		}
+	}
+}

x/ibc/02-client/keeper/client.go

@@ -65,7 +65,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
 		consensusHeight = types.GetSelfHeight(ctx)
 	}
 
-	k.Logger(ctx).Info(fmt.Sprintf("client %s updated height %d", clientID, consensusHeight))
+	k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight)
 
 	// emitting events in the keeper emits for both begin block and handler client updates
 	ctx.EventManager().EmitEvent(
@@ -80,6 +80,41 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
 	return nil
 }
 
+// UpgradeClient upgrades the client to a new client state if this new client was committed to
+// by the old client at the specified upgrade height
+func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient exported.ClientState, proofUpgrade []byte) error {
+	clientState, found := k.GetClientState(ctx, clientID)
+	if !found {
+		return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot update client with ID %s", clientID)
+	}
+
+	// prevent upgrade if current client is frozen
+	if clientState.IsFrozen() {
+		return sdkerrors.Wrapf(types.ErrClientFrozen, "cannot update client with ID %s", clientID)
+	}
+
+	err := clientState.VerifyUpgrade(ctx, k.cdc, k.ClientStore(ctx, clientID), upgradedClient, proofUpgrade)
+	if err != nil {
+		return sdkerrors.Wrapf(err, "cannot upgrade client with ID: %s", clientID)
+	}
+
+	k.SetClientState(ctx, clientID, upgradedClient)
+
+	k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight())
+
+	// emitting events in the keeper emits for client upgrades
+	ctx.EventManager().EmitEvent(
+		sdk.NewEvent(
+			types.EventTypeUpgradeClient,
+			sdk.NewAttribute(types.AttributeKeyClientID, clientID),
+			sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()),
+			sdk.NewAttribute(types.AttributeKeyConsensusHeight, upgradedClient.GetLatestHeight().String()),
+		),
+	)
+
+	return nil
+}
+
 // CheckMisbehaviourAndUpdateState checks for client misbehaviour and freezes the
 // client if so.
 func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour exported.Misbehaviour) error {