feat: add optional migration pruning for tendermint consensus states

docs/migrations/v6-to-v7.md

@@ -13,7 +13,31 @@ There are four sections based on the four potential user groups of this document
 
 ## Chains
 
-- No relevant changes were made in this release.
+Chains will perform automatic migrations to remove existing localhost clients and to migrate the solomachine to the v3 of the protobuf definition. 
+
+An optional upgrade handler has been added to prune expired tendermint consensus states. It may be used during any upgrade (not just to v7)
+Add the following to the function call to the upgrade handler in `app/app.go`, to perform the optional state pruning.
+
+```go
+import (
+    // ...
+    ibcclientmigrations "github.com/cosmos/ibc-go/v6/modules/core/02-client/migrations"
+)
+
+// ...
+
+app.UpgradeKeeper.SetUpgradeHandler(
+    upgradeName,
+    func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {
+        // prune expired tendermint consensus states
+        ibcclientmigrations.PruneTendermintExpiredConsensusStates(ctx, app.Codec, appCodec, keys[ibchost.StoreKey])
+
+        return app.mm.RunMigrations(ctx, app.configurator, fromVM)
+    },
+)
+```
+
+Checkout the logs to see how many consensus states are pruned.
 
 ## IBC Apps
 
@@ -55,41 +79,6 @@ The function `GetTimestampAtHeight` has been added to the `ClientState` interfac
 
 The `GetRoot` function has been removed from consensus state interface since it was not used by core IBC.
 
-### Light client implementations
-
-The `09-localhost` light client implementation has been removed because it is currently non-functional.
-
-An upgrade handler has been added to supply chain developers with the logic needed to prune the ibc client store and successfully complete the removal of `09-localhost`.
-Add the following to the application upgrade handler in `app/app.go`, calling `MigrateToV6` to perform store migration logic.
-
-```go
-import (
-    // ...
-    ibcv6 "github.com/cosmos/ibc-go/v6/modules/core/migrations/v6"
-)
-
-// ...
-
-app.UpgradeKeeper.SetUpgradeHandler(
-    upgradeName,
-    func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {
-        // prune the 09-localhost client from the ibc client store
-        ibcv6.MigrateToV6(ctx, app.IBCKeeper.ClientKeeper)
-
-        return app.mm.RunMigrations(ctx, app.configurator, fromVM)
-    },
-)
-```
-
-Please note the above upgrade handler is optional and should only be run if chains have an existing `09-localhost` client stored in state.
-A simple query can be performed to check for a `09-localhost` client on chain.
-
-For example:
-
-```
-simd query ibc client states | grep 09-localhost
-```
-
 ### Client Keeper
 
 Keeper function `CheckMisbehaviourAndUpdateState` has been removed since function `UpdateClient` can now handle updating `ClientState` on `ClientMessage` type which can be any `Misbehaviour` implementations.  

modules/core/02-client/migrations/store.go

@@ -0,0 +1,76 @@
+package migrations
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/prefix"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	"github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
+	"github.com/cosmos/ibc-go/v6/modules/core/exported"
+	ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint"
+)
+
+// PruneTendermintConsensusStates prunes all expired tendermint consensus states. This function
+// may optionally be called during in-place store migrations. The ibc store key must be provided.
+func PruneTendermintConsensusStates(ctx sdk.Context, cdc codec.BinaryCodec, storeKey storetypes.StoreKey) error {
+	store := ctx.KVStore(storeKey)
+
+	// iterate over ibc store with prefix: host.ClientStoreKeyPrefix/07-tendermint,
+	tendermintClientPrefix := []byte(fmt.Sprintf("%s/%s", host.KeyClientStorePrefix, exported.Tendermint))
+	iterator := sdk.KVStorePrefixIterator(store, tendermintClientPrefix)
+
+	var clients []string
+
+	// collect all clients to avoid performing store state changes during iteration
+	defer iterator.Close()
+	for ; iterator.Valid(); iterator.Next() {
+		path := string(iterator.Key())
+		if !strings.Contains(path, host.KeyClientState) {
+			// skip non client state keys
+			continue
+		}
+
+		clientID, err := host.ParseClientStatePath(path)
+		if err != nil {
+			return err
+		}
+
+		clients = append(clients, clientID)
+	}
+
+	var totalPruned int
+
+	for _, clientID := range clients {
+		clientPrefix := []byte(fmt.Sprintf("%s/%s/", host.KeyClientStorePrefix, clientID))
+		clientStore := prefix.NewStore(ctx.KVStore(storeKey), clientPrefix)
+
+		bz := clientStore.Get(host.ClientStateKey())
+		if bz == nil {
+			return types.ErrClientNotFound
+		}
+
+		var clientState exported.ClientState
+		if err := cdc.UnmarshalInterface(bz, &clientState); err != nil {
+			return sdkerrors.Wrap(err, "failed to unmarshal client state bytes into tendermint client state")
+		}
+
+		tmClientState, ok := clientState.(*ibctm.ClientState)
+		if !ok {
+			return sdkerrors.Wrap(types.ErrInvalidClient, "client state is not tendermint even though client id contains 07-tendermint")
+		}
+
+		amtPruned := ibctm.PruneAllExpiredConsensusStates(ctx, clientStore, cdc, tmClientState)
+		totalPruned = totalPruned + amtPruned
+	}
+
+	clientLogger := ctx.Logger().With("module", "x/"+host.ModuleName+"/"+types.SubModuleName)
+	clientLogger.Info("pruned expired tendermint consensus states", "total", totalPruned)
+
+	return nil
+}

modules/core/02-client/migrations/store_test.go

@@ -0,0 +1,178 @@
+package migrations_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/suite"
+
+	"github.com/cosmos/ibc-go/v6/modules/core/02-client/migrations"
+	"github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
+	"github.com/cosmos/ibc-go/v6/modules/core/exported"
+	ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint"
+	ibctesting "github.com/cosmos/ibc-go/v6/testing"
+)
+
+type MigrationsTestSuite struct {
+	suite.Suite
+
+	coordinator *ibctesting.Coordinator
+
+	chainA *ibctesting.TestChain
+	chainB *ibctesting.TestChain
+}
+
+// TestMigrationsTestSuite runs all the tests within this package.
+func TestMigrationsTestSuite(t *testing.T) {
+	suite.Run(t, new(MigrationsTestSuite))
+}
+
+// SetupTest creates a coordinator with 2 test chains.
+func (suite *MigrationsTestSuite) SetupTest() {
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
+	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+}
+
+// test pruning of multiple expired tendermint consensus states
+func (suite *MigrationsTestSuite) TestPruneTendermintConsensusStates() {
+	// create multiple tendermint clients and a solo machine client
+	// the solo machine is used to verify this pruning function only modifies
+	// the tendermint store.
+
+	numTMClients := 3
+	paths := make([]*ibctesting.Path, numTMClients)
+
+	for i := 0; i < numTMClients; i++ {
+		path := ibctesting.NewPath(suite.chainA, suite.chainB)
+		suite.coordinator.SetupClients(path)
+
+		paths[i] = path
+	}
+
+	solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-0", "testing", 1)
+	smClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), solomachine.ClientID)
+
+	// set client state
+	bz, err := suite.chainA.App.AppCodec().MarshalInterface(solomachine.ClientState())
+	suite.Require().NoError(err)
+	smClientStore.Set(host.ClientStateKey(), bz)
+
+	bz, err = suite.chainA.App.AppCodec().MarshalInterface(solomachine.ConsensusState())
+	suite.Require().NoError(err)
+	smHeight := types.NewHeight(0, 1)
+	smClientStore.Set(host.ConsensusStateKey(smHeight), bz)
+
+	pruneHeightMap := make(map[*ibctesting.Path][]exported.Height)
+	unexpiredHeightMap := make(map[*ibctesting.Path][]exported.Height)
+
+	for _, path := range paths {
+		// collect all heights expected to be pruned
+		var pruneHeights []exported.Height
+		pruneHeights = append(pruneHeights, path.EndpointA.GetClientState().GetLatestHeight())
+
+		// these heights will be expired and also pruned
+		for i := 0; i < 3; i++ {
+			err := path.EndpointA.UpdateClient()
+			suite.Require().NoError(err)
+
+			pruneHeights = append(pruneHeights, path.EndpointA.GetClientState().GetLatestHeight())
+		}
+
+		// double chedck all information is currently stored
+		for _, pruneHeight := range pruneHeights {
+			consState, ok := suite.chainA.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
+			suite.Require().True(ok)
+			suite.Require().NotNil(consState)
+
+			ctx := suite.chainA.GetContext()
+			clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
+
+			processedTime, ok := ibctm.GetProcessedTime(clientStore, pruneHeight)
+			suite.Require().True(ok)
+			suite.Require().NotNil(processedTime)
+
+			processedHeight, ok := ibctm.GetProcessedHeight(clientStore, pruneHeight)
+			suite.Require().True(ok)
+			suite.Require().NotNil(processedHeight)
+
+			expectedConsKey := ibctm.GetIterationKey(clientStore, pruneHeight)
+			suite.Require().NotNil(expectedConsKey)
+		}
+		pruneHeightMap[path] = pruneHeights
+	}
+
+	// Increment the time by a week
+	suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)
+
+	for _, path := range paths {
+		// create the consensus state that can be used as trusted height for next update
+		var unexpiredHeights []exported.Height
+		err := path.EndpointA.UpdateClient()
+		suite.Require().NoError(err)
+		unexpiredHeights = append(unexpiredHeights, path.EndpointA.GetClientState().GetLatestHeight())
+
+		err = path.EndpointA.UpdateClient()
+		suite.Require().NoError(err)
+		unexpiredHeights = append(unexpiredHeights, path.EndpointA.GetClientState().GetLatestHeight())
+
+		unexpiredHeightMap[path] = unexpiredHeights
+	}
+
+	// Increment the time by another week, then update the client.
+	// This will cause the consensus states created before the first time increment
+	// to be expired
+	suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)
+	err = migrations.PruneTendermintConsensusStates(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().GetKey(host.StoreKey))
+	suite.Require().NoError(err)
+
+	for _, path := range paths {
+		ctx := suite.chainA.GetContext()
+		clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
+
+		// ensure everything has been pruned
+		for i, pruneHeight := range pruneHeightMap[path] {
+			consState, ok := suite.chainA.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
+			suite.Require().False(ok, i)
+			suite.Require().Nil(consState, i)
+
+			processedTime, ok := ibctm.GetProcessedTime(clientStore, pruneHeight)
+			suite.Require().False(ok, i)
+			suite.Require().Equal(uint64(0), processedTime, i)
+
+			processedHeight, ok := ibctm.GetProcessedHeight(clientStore, pruneHeight)
+			suite.Require().False(ok, i)
+			suite.Require().Nil(processedHeight, i)
+
+			expectedConsKey := ibctm.GetIterationKey(clientStore, pruneHeight)
+			suite.Require().Nil(expectedConsKey, i)
+		}
+
+		// ensure metadata is set for unexpired consensus state
+		for _, height := range unexpiredHeightMap[path] {
+			consState, ok := suite.chainA.GetConsensusState(path.EndpointA.ClientID, height)
+			suite.Require().True(ok)
+			suite.Require().NotNil(consState)
+
+			processedTime, ok := ibctm.GetProcessedTime(clientStore, height)
+			suite.Require().True(ok)
+			suite.Require().NotEqual(uint64(0), processedTime)
+
+			processedHeight, ok := ibctm.GetProcessedHeight(clientStore, height)
+			suite.Require().True(ok)
+			suite.Require().NotEqual(types.ZeroHeight(), processedHeight)
+
+			consKey := ibctm.GetIterationKey(clientStore, height)
+			suite.Require().Equal(host.ConsensusStateKey(height), consKey)
+		}
+	}
+
+	// verify that solomachine client and consensus state were not removed
+	smClientStore = suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), solomachine.ClientID)
+	bz = smClientStore.Get(host.ClientStateKey())
+	suite.Require().NotEmpty(bz)
+
+	bz = smClientStore.Get(host.ConsensusStateKey(smHeight))
+	suite.Require().NotEmpty(bz)
+}

modules/core/24-host/parse.go

@@ -32,6 +32,29 @@ func ParseIdentifier(identifier, prefix string) (uint64, error) {
 	return sequence, nil
 }
 
+// ParseClientStatePath returns the client ID from a client state path. It returns
+// an error if the provided path is invalid.
+func ParseClientStatePath(path string) (string, error) {
+	split := strings.Split(path, "/")
+	if len(split) != 3 {
+		return "", sdkerrors.Wrapf(ErrInvalidPath, "cannot parse client state path %s", path)
+	}
+
+	if split[0] != string(KeyClientStorePrefix) {
+		return "", sdkerrors.Wrapf(ErrInvalidPath, "path does not begin with client store prefix: expected %s, got %s", KeyClientStorePrefix, split[0])
+	}
+
+	if split[2] != KeyClientState {
+		return "", sdkerrors.Wrapf(ErrInvalidPath, "path does not end with client state key: expected %s, got %s", KeyClientState, split[2])
+	}
+
+	if strings.TrimSpace(split[1]) == "" {
+		return "", sdkerrors.Wrap(ErrInvalidPath, "clientID is empty")
+	}
+
+	return split[1], nil
+}
+
 // ParseConnectionPath returns the connection ID from a full path. It returns
 // an error if the provided path is invalid.
 func ParseConnectionPath(path string) (string, error) {