Add in-place and genesis migrations

modules/core/02-client/legacy/v100/genesis.go

@@ -110,15 +110,19 @@ func MigrateGenesis(cdc codec.BinaryCodec, clientGenState *types.GenesisState, g
 									// if we find the processed time metadata for an unexpired height, add the
 									// iteration key and processed height keys.
 									if bytes.Equal(metadata.Key, ibctmtypes.ProcessedTimeKey(height)) {
-										clientMetadata = append(clientMetadata, types.GenesisMetadata{
-											Key:   ibctmtypes.ProcessedHeightKey(height),
-											Value: []byte(selfHeight.String()),
-										})
-										clientMetadata = append(clientMetadata, metadata) // processed time
-										clientMetadata = append(clientMetadata, types.GenesisMetadata{
-											Key:   ibctmtypes.IterationKey(height),
-											Value: host.ConsensusStateKey(height),
-										})
+										clientMetadata = append(clientMetadata,
+											// set the processed height using the current self height
+											// this is safe, it may cause delays in packet processing if there
+											// is a non zero connection delay time
+											types.GenesisMetadata{
+												Key:   ibctmtypes.ProcessedHeightKey(height),
+												Value: []byte(selfHeight.String()),
+											},
+											metadata, // processed time
+											types.GenesisMetadata{
+												Key:   ibctmtypes.IterationKey(height),
+												Value: host.ConsensusStateKey(height),
+											})
 
 									}
 								}

modules/core/02-client/legacy/v100/genesis_test.go

@@ -3,7 +3,6 @@ package v100_test
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"time"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -167,8 +166,6 @@ func (suite *LegacyTestSuite) TestMigrateGenesisSolomachine() {
 	indentedBz, err := json.MarshalIndent(jsonObj, "", "\t")
 	suite.Require().NoError(err)
 
-	fmt.Println(string(indentedBz))
-
 	suite.Require().Equal(string(expectedIndentedBz), string(indentedBz))
 }
 
@@ -310,7 +307,5 @@ func (suite *LegacyTestSuite) TestMigrateGenesisTendermint() {
 	indentedBz, err := json.MarshalIndent(jsonObj, "", "\t")
 	suite.Require().NoError(err)
 
-	fmt.Println(string(indentedBz))
-
 	suite.Require().Equal(string(expectedIndentedBz), string(indentedBz))
 }

modules/core/02-client/legacy/v100/store.go

@@ -93,11 +93,12 @@ func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, cdc codec.BinaryCodec)
 				return sdkerrors.Wrap(types.ErrInvalidClient, "client state is not tendermint even though client id contains 07-tendermint")
 			}
 
-			if err = ibctmtypes.PruneAllExpiredConsensusStates(ctx, clientStore, cdc, tmClientState); err != nil {
+			// add iteration keys so pruning will be successful
+			if err = addConsensusMetadata(ctx, clientStore, cdc, tmClientState); err != nil {
 				return err
 			}
 
-			if err = addConsensusMetadata(ctx, clientStore, cdc, tmClientState); err != nil {
+			if err = ibctmtypes.PruneAllExpiredConsensusStates(ctx, clientStore, cdc, tmClientState); err != nil {
 				return err
 			}
 
@@ -150,27 +151,22 @@ func pruneSolomachineConsensusStates(clientStore sdk.KVStore) {
 	}
 }
 
-// addConsensusMetadata adds the iteration key and processed height for all unexpired tendermint consensus states
-// These keys were not included in the previous release of the IBC module.
+// addConsensusMetadata adds the iteration key and processed height for all tendermint consensus states
+// These keys were not included in the previous release of the IBC module. Adding the iteration keys allows
+// for pruning iteration.
 func addConsensusMetadata(ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, clientState *ibctmtypes.ClientState) error {
 	var heights []exported.Height
+	iterator := sdk.KVStorePrefixIterator(clientStore, []byte(host.KeyConsensusStatePrefix))
 
-	metadataCb := func(height exported.Height) bool {
-		consState, err := ibctmtypes.GetConsensusState(clientStore, cdc, height)
-		// this error should never occur
-		if err != nil {
-			return true
-		}
-
-		if !clientState.IsExpired(consState.Timestamp, ctx.BlockTime()) {
-			heights = append(heights, height)
+	defer iterator.Close()
+	for ; iterator.Valid(); iterator.Next() {
+		keySplit := strings.Split(string(iterator.Key()), "/")
+		// consensus key is in the format "consensusStates/<height>"
+		if len(keySplit) != 2 {
+			continue
 		}
 
-		return false
-	}
-
-	if err := ibctmtypes.IterateConsensusStateAscending(clientStore, metadataCb); err != nil {
-		return err
+		heights = append(heights, types.MustParseHeight(keySplit[1]))
 	}
 
 	for _, height := range heights {

modules/core/02-client/legacy/v100/store_test.go

@@ -118,88 +118,114 @@ func (suite *LegacyTestSuite) TestMigrateStoreSolomachine() {
 // ensure all expired consensus states are removed from tendermint client stores
 func (suite *LegacyTestSuite) TestMigrateStoreTendermint() {
 	// create path and setup clients
-	path := ibctesting.NewPath(suite.chainA, suite.chainB)
-	suite.coordinator.SetupClients(path)
+	path1 := ibctesting.NewPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupClients(path1)
 
-	// collect all heights expected to be pruned
-	var pruneHeights []exported.Height
-	pruneHeights = append(pruneHeights, path.EndpointA.GetClientState().GetLatestHeight())
+	path2 := ibctesting.NewPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupClients(path2)
+	pruneHeightMap := make(map[*ibctesting.Path][]exported.Height)
+	unexpiredHeightMap := make(map[*ibctesting.Path][]exported.Height)
 
-	// these heights will be expired and also pruned
-	for i := 0; i < 3; i++ {
-		path.EndpointA.UpdateClient()
+	for _, path := range []*ibctesting.Path{path1, path2} {
+		// collect all heights expected to be pruned
+		var pruneHeights []exported.Height
 		pruneHeights = append(pruneHeights, path.EndpointA.GetClientState().GetLatestHeight())
-	}
 
-	// double chedck all information is currently stored
-	for _, pruneHeight := range pruneHeights {
-		consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
-		suite.Require().True(ok)
-		suite.Require().NotNil(consState)
+		// these heights will be expired and also pruned
+		for i := 0; i < 3; i++ {
+			path.EndpointA.UpdateClient()
+			pruneHeights = append(pruneHeights, path.EndpointA.GetClientState().GetLatestHeight())
+		}
 
-		ctx := path.EndpointA.Chain.GetContext()
-		clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
+		// double chedck all information is currently stored
+		for _, pruneHeight := range pruneHeights {
+			consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
+			suite.Require().True(ok)
+			suite.Require().NotNil(consState)
 
-		processedTime, ok := ibctmtypes.GetProcessedTime(clientStore, pruneHeight)
-		suite.Require().True(ok)
-		suite.Require().NotNil(processedTime)
+			ctx := path.EndpointA.Chain.GetContext()
+			clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
 
-		processedHeight, ok := ibctmtypes.GetProcessedHeight(clientStore, pruneHeight)
-		suite.Require().True(ok)
-		suite.Require().NotNil(processedHeight)
+			processedTime, ok := ibctmtypes.GetProcessedTime(clientStore, pruneHeight)
+			suite.Require().True(ok)
+			suite.Require().NotNil(processedTime)
 
-		expectedConsKey := ibctmtypes.GetIterationKey(clientStore, pruneHeight)
-		suite.Require().NotNil(expectedConsKey)
+			processedHeight, ok := ibctmtypes.GetProcessedHeight(clientStore, pruneHeight)
+			suite.Require().True(ok)
+			suite.Require().NotNil(processedHeight)
+
+			expectedConsKey := ibctmtypes.GetIterationKey(clientStore, pruneHeight)
+			suite.Require().NotNil(expectedConsKey)
+		}
+		pruneHeightMap[path] = pruneHeights
 	}
 
 	// Increment the time by a week
 	suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)
 
-	// create the consensus state that can be used as trusted height for next update
-	path.EndpointA.UpdateClient()
+	for _, path := range []*ibctesting.Path{path1, path2} {
+		// create the consensus state that can be used as trusted height for next update
+		var unexpiredHeights []exported.Height
+		path.EndpointA.UpdateClient()
+		unexpiredHeights = append(unexpiredHeights, path.EndpointA.GetClientState().GetLatestHeight())
+		path.EndpointA.UpdateClient()
+		unexpiredHeights = append(unexpiredHeights, path.EndpointA.GetClientState().GetLatestHeight())
+
+		// remove processed height and iteration keys since these were missing from previous version of ibc module
+		clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(path.EndpointA.Chain.GetContext(), path.EndpointA.ClientID)
+		for _, height := range unexpiredHeights {
+			clientStore.Delete(ibctmtypes.ProcessedHeightKey(height))
+			clientStore.Delete(ibctmtypes.IterationKey(height))
+		}
+
+		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 := v100.MigrateStore(path.EndpointA.Chain.GetContext(), path.EndpointA.Chain.GetSimApp().GetKey(host.StoreKey), path.EndpointA.Chain.App.AppCodec())
+	err := v100.MigrateStore(path1.EndpointA.Chain.GetContext(), path1.EndpointA.Chain.GetSimApp().GetKey(host.StoreKey), path1.EndpointA.Chain.App.AppCodec())
 	suite.Require().NoError(err)
 
-	ctx := path.EndpointA.Chain.GetContext()
-	clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
+	for _, path := range []*ibctesting.Path{path1, path2} {
+		ctx := path.EndpointA.Chain.GetContext()
+		clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
 
-	// ensure everything has been pruned
-	for i, pruneHeight := range pruneHeights {
-		consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
-		suite.Require().False(ok, i)
-		suite.Require().Nil(consState, i)
+		// ensure everything has been pruned
+		for i, pruneHeight := range pruneHeightMap[path] {
+			consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
+			suite.Require().False(ok, i)
+			suite.Require().Nil(consState, i)
 
-		processedTime, ok := ibctmtypes.GetProcessedTime(clientStore, pruneHeight)
-		suite.Require().False(ok, i)
-		suite.Require().Equal(uint64(0), processedTime, i)
+			processedTime, ok := ibctmtypes.GetProcessedTime(clientStore, pruneHeight)
+			suite.Require().False(ok, i)
+			suite.Require().Equal(uint64(0), processedTime, i)
 
-		processedHeight, ok := ibctmtypes.GetProcessedHeight(clientStore, pruneHeight)
-		suite.Require().False(ok, i)
-		suite.Require().Nil(processedHeight, i)
+			processedHeight, ok := ibctmtypes.GetProcessedHeight(clientStore, pruneHeight)
+			suite.Require().False(ok, i)
+			suite.Require().Nil(processedHeight, i)
 
-		expectedConsKey := ibctmtypes.GetIterationKey(clientStore, pruneHeight)
-		suite.Require().Nil(expectedConsKey, i)
-	}
+			expectedConsKey := ibctmtypes.GetIterationKey(clientStore, pruneHeight)
+			suite.Require().Nil(expectedConsKey, i)
+		}
 
-	// ensure metadata is set for unexpired consensus state
-	height := path.EndpointA.GetClientState().GetLatestHeight()
-	consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, height)
-	suite.Require().True(ok)
-	suite.Require().NotNil(consState)
+		// ensure metadata is set for unexpired consensus state
+		for _, height := range unexpiredHeightMap[path] {
+			consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, height)
+			suite.Require().True(ok)
+			suite.Require().NotNil(consState)
 
-	processedTime, ok := ibctmtypes.GetProcessedTime(clientStore, height)
-	suite.Require().True(ok)
-	suite.Require().NotEqual(uint64(0), processedTime)
+			processedTime, ok := ibctmtypes.GetProcessedTime(clientStore, height)
+			suite.Require().True(ok)
+			suite.Require().NotEqual(uint64(0), processedTime)
 
-	processedHeight, ok := ibctmtypes.GetProcessedHeight(clientStore, height)
-	suite.Require().True(ok)
-	suite.Require().Equal(types.GetSelfHeight(path.EndpointA.Chain.GetContext()), processedHeight)
+			processedHeight, ok := ibctmtypes.GetProcessedHeight(clientStore, height)
+			suite.Require().True(ok)
+			suite.Require().Equal(types.GetSelfHeight(path.EndpointA.Chain.GetContext()), processedHeight)
 
-	consKey := ibctmtypes.GetIterationKey(clientStore, height)
-	suite.Require().Equal(host.ConsensusStateKey(height), consKey)
+			consKey := ibctmtypes.GetIterationKey(clientStore, height)
+			suite.Require().Equal(host.ConsensusStateKey(height), consKey)
+		}
+	}
 }