Remove IBC logic from x/upgrade

CHANGELOG.md

@@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/distribution) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if the distribution module account balance, coming from bank module state, does not match the one in distribution module state, the initialization will panic.
 * (client/keys) [\#8500](https://github.com/cosmos/cosmos-sdk/pull/8500) `InfoImporter` interface is removed from legacy keybase.
 * [\#8629](https://github.com/cosmos/cosmos-sdk/pull/8629) Deprecated `SetFullFundraiserPath` from `Config` in favor of `SetPurpose` and `SetCoinType`.
+* (x/upgrade) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) Remove x/ibc imports from x/upgrade by replacing plan.UpgradedClient with a `[]byte` instead of using an `Any`. IBC upgrade begin blocker logic moved to the IBC module.
 
 ### State Machine Breaking
 

docs/core/proto-docs.md

@@ -7507,7 +7507,7 @@ Plan specifies information about a planned upgrade and when it should occur.
 | `time` | [google.protobuf.Timestamp](#google.protobuf.Timestamp) |  | The time after which the upgrade must be performed. Leave set to its zero value to use a pre-defined Height instead. |
 | `height` | [int64](#int64) |  | The height at which the upgrade must be performed. Only used if Time is not set. |
 | `info` | [string](#string) |  | Any application specific upgrade info to be included on-chain such as a git commit that validators could automatically upgrade to |
-| `upgraded_client_state` | [google.protobuf.Any](#google.protobuf.Any) |  | 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 |
+| `upgraded_client_state` | [bytes](#bytes) |  | 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 |
 
 
 
@@ -7632,7 +7632,7 @@ RPC method.
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| `upgraded_consensus_state` | [google.protobuf.Any](#google.protobuf.Any) |  |  |
+| `upgraded_consensus_state` | [bytes](#bytes) |  |  |
 
 
 

proto/cosmos/upgrade/v1beta1/query.proto

@@ -64,5 +64,8 @@ message QueryUpgradedConsensusStateRequest {
 // QueryUpgradedConsensusStateResponse is the response type for the Query/UpgradedConsensusState
 // RPC method.
 message QueryUpgradedConsensusStateResponse {
-  google.protobuf.Any upgraded_consensus_state = 1;
+  reserved 1;
+  reserved "option";
+
+  bytes upgraded_consensus_state = 2;
 }

proto/cosmos/upgrade/v1beta1/upgrade.proto

@@ -34,12 +34,15 @@ message Plan {
   // such as a git commit that validators could automatically upgrade to
   string info = 4;
 
+  reserved 5;
+  reserved "option";
+
   // 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\""];
+  bytes upgraded_client_state = 6 [(gogoproto.moretags) = "yaml:\"upgraded_client_state\""];
 }
 
 // SoftwareUpgradeProposal is a gov Content type for initiating a software

simapp/app.go

@@ -299,7 +299,7 @@ func NewSimApp(
 
 	// Create IBC Keeper
 	app.IBCKeeper = ibckeeper.NewKeeper(
-		appCodec, keys[ibchost.StoreKey], app.GetSubspace(ibchost.ModuleName), app.StakingKeeper, scopedIBCKeeper,
+		appCodec, keys[ibchost.StoreKey], app.GetSubspace(ibchost.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper,
 	)
 
 	app.AuthzKeeper = authzkeeper.NewKeeper(keys[authztypes.StoreKey], appCodec, app.BaseApp.MsgServiceRouter())

x/ibc/core/02-client/abci.go

@@ -4,11 +4,31 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/keeper"
 	"github.com/cosmos/cosmos-sdk/x/ibc/core/exported"
+	ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types"
 )
 
 // BeginBlocker updates an existing localhost client with the latest block height.
 func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
-	_, found := k.GetClientState(ctx, exported.Localhost)
+	plan, found := k.GetUpgradePlan(ctx)
+	if found {
+		// Once we are at the last block this chain will commit, set the upgraded consensus state
+		// so that IBC clients can use the last NextValidatorsHash as a trusted kernel for verifying
+		// headers on the next version of the chain.
+		// Set the time to the last block time of the current chain.
+		// In order for a client to upgrade successfully, the first block of the new chain must be committed
+		// within the trusting period of the last block time on this chain.
+		if plan.IsIBCPlan() && ctx.BlockHeight() == plan.Height-1 {
+			upgradedConsState := &ibctmtypes.ConsensusState{
+				Timestamp:          ctx.BlockTime(),
+				NextValidatorsHash: ctx.BlockHeader().NextValidatorsHash,
+			}
+			bz := k.MustMarshalConsensusState(upgradedConsState)
+
+			k.SetUpgradedConsensusState(ctx, plan.Height, bz)
+		}
+	}
+
+	_, found = k.GetClientState(ctx, exported.Localhost)
 	if !found {
 		return
 	}

x/ibc/core/02-client/abci_test.go

@@ -4,12 +4,16 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/suite"
+	abci "github.com/tendermint/tendermint/abci/types"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 
 	client "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client"
 	"github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/core/exported"
+	ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types"
 	localhosttypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/09-localhost/types"
 	ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing"
+	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
 type ClientTestSuite struct {
@@ -58,3 +62,32 @@ func (suite *ClientTestSuite) TestBeginBlocker() {
 		prevHeight = localHostClient.GetLatestHeight().(types.Height)
 	}
 }
+
+func (suite *ClientTestSuite) TestBeginBlockerConsensusState() {
+	cs := []byte("IBC client state")
+	plan := &upgradetypes.Plan{
+		Name:                "test",
+		Height:              suite.chainA.GetContext().BlockHeight() + 1,
+		UpgradedClientState: cs,
+	}
+	store := suite.chainA.GetContext().KVStore(suite.chainA.App.GetKey(upgradetypes.StoreKey))
+	bz := suite.chainA.App.AppCodec().MustMarshalBinaryBare(plan)
+	store.Set(upgradetypes.PlanKey(), bz)
+
+	suite.T().Log("Verify that chain committed to consensus state on the last height it will commit")
+	nextValsHash := []byte("nextValsHash")
+	newCtx := suite.chainA.GetContext().WithBlockHeader(tmproto.Header{
+		Height:             suite.chainA.GetContext().BlockHeight(),
+		NextValidatorsHash: nextValsHash,
+	})
+
+	req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
+	suite.chainA.App.BeginBlock(req)
+
+	// plan Height is at ctx.BlockHeight+1
+	consState, err := suite.chainA.App.UpgradeKeeper.GetUpgradedConsensusState(newCtx, suite.chainA.GetContext().BlockHeight()+1)
+	suite.Require().NoError(err)
+	bz, err = types.MarshalConsensusState(suite.chainA.App.AppCodec(), &ibctmtypes.ConsensusState{Timestamp: newCtx.BlockTime(), NextValidatorsHash: nextValsHash})
+	suite.Require().NoError(err)
+	suite.Require().Equal(bz, consState)
+}

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

@@ -230,6 +230,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 		lastHeight                                  exported.Height
 		clientA                                     string
 		proofUpgradedClient, proofUpgradedConsState []byte
+		upgradedClientBz, upgradedConsStateBz       []byte
+		err                                         error
 	)
 
 	testCases := []struct {
@@ -240,18 +242,12 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 		{
 			name: "successful upgrade",
 			setup: func() {
-
-				upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
-				upgradedConsState = &ibctmtypes.ConsensusState{
-					NextValidatorsHash: []byte("nextValsHash"),
-				}
-
 				// last Height is at next block
 				lastHeight = clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1))
 
 				// zero custom fields and store in upgrade store
-				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClient)
-				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsState)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz)
 
 				// commit upgrade store changes and update clients
 
@@ -270,18 +266,12 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 		{
 			name: "client state not found",
 			setup: func() {
-
-				upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
-				upgradedConsState = &ibctmtypes.ConsensusState{
-					NextValidatorsHash: []byte("nextValsHash"),
-				}
-
 				// last Height is at next block
 				lastHeight = clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1))
 
 				// zero custom fields and store in upgrade store
-				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClient)
-				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsState)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz)
 
 				// commit upgrade store changes and update clients
 
@@ -302,18 +292,12 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 		{
 			name: "client state frozen",
 			setup: func() {
-
-				upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
-				upgradedConsState = &ibctmtypes.ConsensusState{
-					NextValidatorsHash: []byte("nextValsHash"),
-				}
-
 				// last Height is at next block
 				lastHeight = clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1))
 
 				// zero custom fields and store in upgrade store
-				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClient)
-				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsState)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz)
 
 				// commit upgrade store changes and update clients
 
@@ -338,18 +322,12 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 		{
 			name: "tendermint client VerifyUpgrade fails",
 			setup: func() {
-
-				upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
-				upgradedConsState = &ibctmtypes.ConsensusState{
-					NextValidatorsHash: []byte("nextValsHash"),
-				}
-
 				// last Height is at next block
 				lastHeight = clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1))
 
 				// zero custom fields and store in upgrade store
-				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClient)
-				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsState)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz)
+				suite.chainB.App.UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz)
 
 				// change upgradedClient client-specified parameters
 				upgradedClient = ibctmtypes.NewClientState("wrongchainID", ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, true, true)
@@ -371,13 +349,23 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 	for _, tc := range testCases {
 		tc := tc
 		clientA, _ = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint)
+		upgradedClient = ibctmtypes.NewClientState("newChainId", ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, newClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
+		upgradedClient = upgradedClient.ZeroCustomFields()
+		upgradedClientBz, err = types.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient)
+		suite.Require().NoError(err)
+
+		upgradedConsState = &ibctmtypes.ConsensusState{
+			NextValidatorsHash: []byte("nextValsHash"),
+		}
+		upgradedConsStateBz, err = types.MarshalConsensusState(suite.chainA.App.AppCodec(), upgradedConsState)
+		suite.Require().NoError(err)
 
 		tc.setup()
 
 		// Call ZeroCustomFields on upgraded clients to clear any client-chosen parameters in test-case upgradedClient
 		upgradedClient = upgradedClient.ZeroCustomFields()
 
-		err := suite.chainA.App.IBCKeeper.ClientKeeper.UpgradeClient(suite.chainA.GetContext(), clientA, upgradedClient, upgradedConsState, proofUpgradedClient, proofUpgradedConsState)
+		err = suite.chainA.App.IBCKeeper.ClientKeeper.UpgradeClient(suite.chainA.GetContext(), clientA, upgradedClient, upgradedConsState, proofUpgradedClient, proofUpgradedConsState)
 
 		if tc.expPass {
 			suite.Require().NoError(err, "verify upgrade failed on valid case: %s", tc.name)

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

@@ -28,10 +28,11 @@ type Keeper struct {
 	cdc           codec.BinaryMarshaler
 	paramSpace    paramtypes.Subspace
 	stakingKeeper types.StakingKeeper
+	upgradeKeeper types.UpgradeKeeper
 }
 
 // NewKeeper creates a new NewKeeper instance
-func NewKeeper(cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, sk types.StakingKeeper) Keeper {
+func NewKeeper(cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
 	// set KeyTable if it has not already been set
 	if !paramSpace.HasKeyTable() {
 		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
@@ -42,6 +43,7 @@ func NewKeeper(cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtype
 		cdc:           cdc,
 		paramSpace:    paramSpace,
 		stakingKeeper: sk,
+		upgradeKeeper: uk,
 	}
 }
 
@@ -327,6 +329,16 @@ func (k Keeper) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientS
 	return nil
 }
 
+// GetUpgradePlan executes the upgrade keeper GetUpgradePlan function.
+func (k Keeper) GetUpgradePlan(ctx sdk.Context) (plan upgradetypes.Plan, havePlan bool) {
+	return k.upgradeKeeper.GetUpgradePlan(ctx)
+}
+
+// GetUpgradedConsensusState executes the upgrade keeper SetUpgradedConsensusState function.
+func (k Keeper) SetUpgradedConsensusState(ctx sdk.Context, planHeight int64, bz []byte) error {
+	return k.upgradeKeeper.SetUpgradedConsensusState(ctx, planHeight, bz)
+}
+
 // IterateClients provides an iterator over all stored light client State
 // objects. For each State object, cb will be called. If the cb returns true,
 // the iterator will close and stop.

x/ibc/core/02-client/types/expected_keepers.go

@@ -5,10 +5,17 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
 // StakingKeeper expected staking keeper
 type StakingKeeper interface {
 	GetHistoricalInfo(ctx sdk.Context, height int64) (stakingtypes.HistoricalInfo, bool)
 	UnbondingTime(ctx sdk.Context) time.Duration
 }
+
+// UpgradeKeeper expected upgrade keeper
+type UpgradeKeeper interface {
+	GetUpgradePlan(ctx sdk.Context) (plan upgradetypes.Plan, havePlan bool)
+	SetUpgradedConsensusState(ctx sdk.Context, planHeight int64, bz []byte) error
+}

x/ibc/core/keeper/keeper.go

@@ -33,9 +33,10 @@ type Keeper struct {
 // NewKeeper creates a new ibc Keeper
 func NewKeeper(
 	cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace,
-	stakingKeeper clienttypes.StakingKeeper, scopedKeeper capabilitykeeper.ScopedKeeper,
+	stakingKeeper clienttypes.StakingKeeper, upgradeKeeper clienttypes.UpgradeKeeper,
+	scopedKeeper capabilitykeeper.ScopedKeeper,
 ) *Keeper {
-	clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper)
+	clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper, upgradeKeeper)
 	connectionKeeper := connectionkeeper.NewKeeper(cdc, key, clientKeeper)
 	portKeeper := portkeeper.NewKeeper(scopedKeeper)
 	channelKeeper := channelkeeper.NewKeeper(cdc, key, clientKeeper, connectionKeeper, portKeeper, scopedKeeper)