refactor(staking)!: binary format for HistoricalInfoKey

CHANGELOG.md

@@ -158,6 +158,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Client Breaking Changes
 
+* (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) `HistoricalInfoKey` now has a binary format.
 * (grpc-web) [#14652](https://github.com/cosmos/cosmos-sdk/pull/14652) Use same port for gRPC-Web and the API server.
 
 ### CLI Breaking Changes
@@ -314,6 +315,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### State Machine Breaking
 
+* (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) The `HistoricalInfoKey` has been updated to use a binary format.
 * (baseapp, x/auth/posthandler) [#13940](https://github.com/cosmos/cosmos-sdk/pull/13940) Update `PostHandler` to receive the `runTx` success boolean.
 * (store) [#14378](https://github.com/cosmos/cosmos-sdk/pull/14378) The `CacheKV` store is thread-safe again, which includes improved iteration and deletion logic. Iteration is on a strictly isolated view now, which is breaking from previous behavior.
 * (x/bank) [#14538](https://github.com/cosmos/cosmos-sdk/pull/14538) Validate denom in bank balances GRPC queries.

tests/integration/staking/keeper/determinstic_test.go

@@ -605,7 +605,7 @@ func TestGRPCHistoricalInfo(t *testing.T) {
 		Height: height,
 	}
 
-	testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.HistoricalInfo, 1930, false)
+	testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.HistoricalInfo, 1945, false)
 }
 
 func TestGRPCDelegatorValidators(t *testing.T) {

x/staking/keeper/historical_info_test.go

@@ -136,9 +136,9 @@ func (s *KeeperTestSuite) TestGetAllHistoricalInfo() {
 		testutil.NewValidator(s.T(), addrVals[1], PKs[1]),
 	}
 
-	header1 := cmtproto.Header{ChainID: "HelloChain", Height: 10}
-	header2 := cmtproto.Header{ChainID: "HelloChain", Height: 11}
-	header3 := cmtproto.Header{ChainID: "HelloChain", Height: 12}
+	header1 := cmtproto.Header{ChainID: "HelloChain", Height: 9}
+	header2 := cmtproto.Header{ChainID: "HelloChain", Height: 10}
+	header3 := cmtproto.Header{ChainID: "HelloChain", Height: 11}
 
 	hist1 := stakingtypes.HistoricalInfo{Header: header1, Valset: valSet}
 	hist2 := stakingtypes.HistoricalInfo{Header: header2, Valset: valSet}
@@ -147,7 +147,7 @@ func (s *KeeperTestSuite) TestGetAllHistoricalInfo() {
 	expHistInfos := []stakingtypes.HistoricalInfo{hist1, hist2, hist3}
 
 	for i, hi := range expHistInfos {
-		keeper.SetHistoricalInfo(ctx, int64(10+i), &hi) //nolint:gosec // G601: Implicit memory aliasing in for loop.
+		keeper.SetHistoricalInfo(ctx, int64(9+i), &hi) //nolint:gosec // G601: Implicit memory aliasing in for loop.
 	}
 
 	infos := keeper.GetAllHistoricalInfo(ctx)

x/staking/keeper/migrations.go

@@ -6,6 +6,7 @@ import (
 	v2 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v2"
 	v3 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v3"
 	v4 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v4"
+	v5 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v5"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -36,3 +37,8 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error {
 func (m Migrator) Migrate3to4(ctx sdk.Context) error {
 	return v4.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.legacySubspace)
 }
+
+// Migrate4to5 migrates x/staking state from consensus version 4 to 5.
+func (m Migrator) Migrate4to5(ctx sdk.Context) error {
+	return v5.MigrateStore(ctx, m.keeper.storeKey)
+}

x/staking/migrations/v2/keys.go

@@ -1,6 +1,17 @@
 package v2
 
+import "strconv"
+
 const (
 	// ModuleName is the name of the module
 	ModuleName = "staking"
 )
+
+var (
+	HistoricalInfoKey = []byte{0x50} // prefix for the historical info
+)
+
+// GetHistoricalInfoKey returns a key prefix for indexing HistoricalInfo objects.
+func GetHistoricalInfoKey(height int64) []byte {
+	return append(HistoricalInfoKey, []byte(strconv.FormatInt(height, 10))...)
+}

x/staking/migrations/v2/store_test.go

@@ -114,7 +114,7 @@ func TestStoreMigration(t *testing.T) {
 		{
 			"HistoricalInfoKey",
 			v1.GetHistoricalInfoKey(4),
-			types.GetHistoricalInfoKey(4),
+			v2.GetHistoricalInfoKey(4),
 		},
 	}
 

x/staking/migrations/v5/migrations_test.go

@@ -0,0 +1,71 @@
+package v5_test
+
+import (
+	"math"
+	"math/rand"
+	"testing"
+	"time"
+
+	storetypes "cosmossdk.io/store/types"
+	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	"github.com/cosmos/cosmos-sdk/testutil"
+	moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
+	v2 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v2"
+	v5 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v5"
+	stackingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestHistoricalKeysMigration(t *testing.T) {
+	storeKey := storetypes.NewKVStoreKey("staking")
+	tKey := storetypes.NewTransientStoreKey("transient_test")
+	ctx := testutil.DefaultContext(storeKey, tKey)
+	store := ctx.KVStore(storeKey)
+
+	type testCase struct {
+		oldKey, newKey []byte
+		historicalInfo []byte
+	}
+
+	testCases := make(map[int64]testCase)
+
+	// edge cases
+	testCases[0], testCases[1], testCases[math.MaxInt32] = testCase{}, testCase{}, testCase{}
+
+	// random cases
+	seed := time.Now().UnixNano()
+	r := rand.New(rand.NewSource(seed))
+	for i := 0; i < 10; i++ {
+		height := r.Intn(math.MaxInt32-2) + 2
+
+		testCases[int64(height)] = testCase{}
+	}
+
+	cdc := moduletestutil.MakeTestEncodingConfig().Codec
+	for height := range testCases {
+		testCases[height] = testCase{
+			oldKey:         v2.GetHistoricalInfoKey(height),
+			newKey:         stackingtypes.GetHistoricalInfoKey(height),
+			historicalInfo: cdc.MustMarshal(createHistoricalInfo(height, "testChainID")),
+		}
+	}
+
+	// populate store using old key format
+	for _, tc := range testCases {
+		store.Set(tc.oldKey, tc.historicalInfo)
+	}
+
+	// migrate store to new key format
+	require.NoErrorf(t, v5.MigrateStore(ctx, storeKey), "v5.MigrateStore failed, seed: %d", seed)
+
+	// check results
+	for _, tc := range testCases {
+		require.Nilf(t, store.Get(tc.oldKey), "old key should be deleted, seed: %d", seed)
+		require.NotNilf(t, store.Get(tc.newKey), "new key should be created, seed: %d", seed)
+		require.Equalf(t, tc.historicalInfo, store.Get(tc.newKey), "seed: %d", seed)
+	}
+}
+
+func createHistoricalInfo(height int64, chainID string) *stackingtypes.HistoricalInfo {
+	return &stackingtypes.HistoricalInfo{Header: cmtproto.Header{ChainID: chainID, Height: height}}
+}

x/staking/migrations/v5/store.go

@@ -0,0 +1,47 @@
+package v5
+
+import (
+	"fmt"
+	"strconv"
+
+	"cosmossdk.io/log"
+	"cosmossdk.io/store/prefix"
+	storetypes "cosmossdk.io/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/staking/types"
+)
+
+// MigrateStore performs in-place store migrations from v4 to v5.
+func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey) error {
+	store := ctx.KVStore(storeKey)
+	return migrateHistoricalInfoKeys(store, ctx.Logger())
+}
+
+// migrateHistoricalInfoKeys migrate HistoricalInfo keys to binary format
+func migrateHistoricalInfoKeys(store storetypes.KVStore, logger log.Logger) error {
+	// old key is of format:
+	// prefix (0x50) || heightBytes (string representation of height in 10 base)
+	// new key is of format:
+	// prefix (0x50) || heightBytes (byte array representation using big-endian byte order)
+	oldStore := prefix.NewStore(store, types.HistoricalInfoKey)
+
+	oldStoreIter := oldStore.Iterator(nil, nil)
+	defer sdk.LogDeferred(logger, func() error { return oldStoreIter.Close() })
+
+	for ; oldStoreIter.Valid(); oldStoreIter.Next() {
+		strHeight := oldStoreIter.Key()
+
+		intHeight, err := strconv.ParseInt(string(strHeight), 10, 64)
+		if err != nil {
+			return fmt.Errorf("can't parse height from key %q to int64: %v", strHeight, err)
+		}
+
+		newStoreKey := types.GetHistoricalInfoKey(intHeight)
+
+		// Set new key on store. Values don't change.
+		store.Set(newStoreKey, oldStoreIter.Value())
+		oldStore.Delete(oldStoreIter.Key())
+	}
+
+	return nil
+}

x/staking/module.go

@@ -34,7 +34,7 @@ import (
 )
 
 const (
-	consensusVersion uint64 = 4
+	consensusVersion uint64 = 5
 )
 
 var (
@@ -165,6 +165,9 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
 	if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4); err != nil {
 		panic(fmt.Sprintf("failed to migrate x/%s from version 3 to 4: %v", types.ModuleName, err))
 	}
+	if err := cfg.RegisterMigration(types.ModuleName, 4, m.Migrate4to5); err != nil {
+		panic(fmt.Sprintf("failed to migrate x/%s from version 4 to 5: %v", types.ModuleName, err))
+	}
 }
 
 // InitGenesis performs genesis initialization for the staking module.

x/staking/types/keys.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
-	"strconv"
 	"time"
 
 	"cosmossdk.io/math"
@@ -376,5 +375,7 @@ func GetREDsByDelToValDstIndexKey(delAddr sdk.AccAddress, valDstAddr sdk.ValAddr
 
 // GetHistoricalInfoKey returns a key prefix for indexing HistoricalInfo objects.
 func GetHistoricalInfoKey(height int64) []byte {
-	return append(HistoricalInfoKey, []byte(strconv.FormatInt(height, 10))...)
+	heightBytes := make([]byte, 8)
+	binary.BigEndian.PutUint64(heightBytes, uint64(height))
+	return append(HistoricalInfoKey, heightBytes...)
 }

x/staking/types/keys_test.go

@@ -3,7 +3,9 @@ package types_test
 import (
 	"bytes"
 	"encoding/hex"
+	math2 "math"
 	"math/big"
+	"strconv"
 	"testing"
 	"time"
 
@@ -129,3 +131,21 @@ func TestTestGetValidatorQueueKeyOrder(t *testing.T) {
 	require.Equal(t, -1, bytes.Compare(keyB, endKey)) // keyB <= endKey
 	require.Equal(t, 1, bytes.Compare(keyC, endKey))  // keyB >= endKey
 }
+
+func TestGetHistoricalInfoKey(t *testing.T) {
+	tests := []struct {
+		height int64
+		want   []byte
+	}{
+		{0, append(types.HistoricalInfoKey, []byte{0, 0, 0, 0, 0, 0, 0, 0}...)},
+		{1, append(types.HistoricalInfoKey, []byte{0, 0, 0, 0, 0, 0, 0, 1}...)},
+		{2, append(types.HistoricalInfoKey, []byte{0, 0, 0, 0, 0, 0, 0, 2}...)},
+		{514, append(types.HistoricalInfoKey, []byte{0, 0, 0, 0, 0, 0, 2, 2}...)},
+		{math2.MaxInt64, append(types.HistoricalInfoKey, []byte{127, 255, 255, 255, 255, 255, 255, 255}...)},
+	}
+	for _, tt := range tests {
+		t.Run(strconv.FormatInt(tt.height, 10), func(t *testing.T) {
+			require.Equal(t, tt.want, types.GetHistoricalInfoKey(tt.height))
+		})
+	}
+}