add key in ibc store tracking ibc upgrade heights

Add a new Key to the IBC client store to track the IBC Upgrade Height. This allows IBC upgrades to correctly remove old IBC upgrade states

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

@@ -117,6 +117,24 @@ func (k Keeper) SetNextClientSequence(ctx sdk.Context, sequence uint64) {
 	store.Set([]byte(types.KeyNextClientSequence), bz)
 }
 
+// GetUpgradePlanHeight gets the IBC upgrade plan height.
+func (k Keeper) GetUpgradePlanHeight(ctx sdk.Context) (uint64, bool) {
+	store := ctx.KVStore(k.storeKey)
+	bz := store.Get([]byte(types.KeyUpgradePlanHeight))
+	if bz == nil {
+		return 0, false
+	}
+
+	return sdk.BigEndianToUint64(bz), true
+}
+
+// SetUpgradePlanHeight sets the IBC upgrade plan height to the store.
+func (k Keeper) SetUpgradePlanHeight(ctx sdk.Context, height uint64) {
+	store := ctx.KVStore(k.storeKey)
+	bz := sdk.Uint64ToBigEndian(height)
+	store.Set([]byte(types.KeyUpgradePlanHeight), bz)
+}
+
 // IterateConsensusStates provides an iterator over all stored consensus states.
 // 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/keeper/proposal.go

@@ -77,9 +77,9 @@ func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdatePropo
 // store.
 func (k Keeper) HandleUpgradeProposal(ctx sdk.Context, p *types.UpgradeProposal) error {
 	// clear any old IBC state stored by previous plan
-	oldPlan, found := k.GetUpgradePlan(ctx)
+	planHeight, found := k.GetUpgradePlanHeight(ctx)
 	if found {
-		k.upgradeKeeper.ClearIBCState(ctx, oldPlan.Height)
+		k.upgradeKeeper.ClearIBCState(ctx, int64(planHeight))
 	}
 
 	clientState, err := types.UnpackClientState(p.UpgradedClientState)
@@ -98,6 +98,9 @@ func (k Keeper) HandleUpgradeProposal(ctx sdk.Context, p *types.UpgradeProposal)
 		return err
 	}
 
+	// set the new IBC upgrade plan height
+	k.SetUpgradePlanHeight(ctx, uint64(p.Plan.Height))
+
 	// sets the new upgraded client in last height committed on this chain is at plan.Height,
 	// since the chain will panic at plan.Height and new chain will resume at plan.Height
 	return k.upgradeKeeper.SetUpgradedClient(ctx, p.Plan.Height, bz)

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

@@ -211,6 +211,7 @@ func (suite *KeeperTestSuite) TestHandleUpgradeProposal() {
 				store := suite.chainA.GetContext().KVStore(suite.chainA.App.GetKey(upgradetypes.StoreKey))
 				bz := suite.chainA.App.AppCodec().MustMarshalBinaryBare(&oldPlan)
 				store.Set(upgradetypes.PlanKey(), bz)
+				suite.chainA.App.IBCKeeper.ClientKeeper.SetUpgradePlanHeight(suite.chainA.GetContext(), uint64(oldPlan.Height))
 
 				bz, err := types.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClientState)
 				suite.Require().NoError(err)

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

@@ -23,6 +23,10 @@ const (
 	// KeyNextClientSequence is the key used to store the next client sequence in
 	// the keeper.
 	KeyNextClientSequence = "nextClientSequence"
+
+	// KeyUpgradePlanHeight is the key used to store the IBC upgrade plan
+	// height in the keeper.
+	KeyUpgradePlanHeight = "upgradePlanHeight"
 )
 
 // FormatClientIdentifier returns the client identifier with the sequence appended.

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

@@ -111,6 +111,10 @@ func (up *UpgradeProposal) ValidateBasic() error {
 		return err
 	}
 
+	if up.Plan.Height <= 0 {
+		return sdkerrors.Wrap(ErrInvalidUpgradeProposal, "IBC chain upgrades must set a positive height")
+	}
+
 	if up.Plan.Time.Unix() > 0 {
 		return sdkerrors.Wrap(ErrInvalidUpgradeProposal, "IBC chain upgrades must only set height")
 	}