From 491cf3babe83d8ce9be2c0fe7813904730b265d5 Mon Sep 17 00:00:00 2001 From: Andrey Petrovich Date: Thu, 13 Apr 2023 09:21:54 +0200 Subject: [PATCH] refactor(staking)!: binary format for HistoricalInfoKey (#15701) ## Description Closes: #11463 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... * [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title * [ ] added `!` to the type prefix if API or client breaking change * [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) * [ ] provided a link to the relevant issue or specification * [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules) * [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) * [ ] added a changelog entry to `CHANGELOG.md` * [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) * [ ] updated the relevant documentation or specification * [ ] reviewed "Files changed" and left comments if necessary * [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... * [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title * [ ] confirmed `!` in the type prefix if API or client breaking change * [ ] confirmed all author checklist items have been addressed * [ ] reviewed state machine logic * [ ] reviewed API design and naming * [ ] reviewed documentation is accurate * [ ] reviewed tests and test coverage * [ ] manually tested (if applicable) --- CHANGELOG.md | 2 + .../staking/keeper/determinstic_test.go | 2 +- x/staking/keeper/historical_info_test.go | 8 +-- x/staking/keeper/migrations.go | 6 ++ x/staking/migrations/v2/keys.go | 9 +++ x/staking/migrations/v2/store_test.go | 2 +- x/staking/migrations/v5/keys.go | 12 ++++ x/staking/migrations/v5/migrations_test.go | 71 +++++++++++++++++++ x/staking/migrations/v5/store.go | 46 ++++++++++++ x/staking/module.go | 5 +- x/staking/types/keys.go | 5 +- x/staking/types/keys_test.go | 20 ++++++ 12 files changed, 179 insertions(+), 9 deletions(-) create mode 100644 x/staking/migrations/v5/keys.go create mode 100644 x/staking/migrations/v5/migrations_test.go create mode 100644 x/staking/migrations/v5/store.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e5273e6c44d..8ddf584967f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,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. * (x/slashing) [#15580](https://github.com/cosmos/cosmos-sdk/pull/15580) The validator slashing window now stores "chunked" bitmap entries for each validator's signing window instead of a single boolean entry per signing window index. * (x/feegrant) [#14294](https://github.com/cosmos/cosmos-sdk/pull/14294) Moved the logic of rejecting duplicate grant from `msg_server` to `keeper` method. * (x/staking) [#14590](https://github.com/cosmos/cosmos-sdk/pull/14590) `MsgUndelegateResponse` now includes undelegated amount. `x/staking` module's `keeper.Undelegate` now returns 3 values (completionTime,undelegateAmount,error) instead of 2. @@ -161,6 +162,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 diff --git a/tests/integration/staking/keeper/determinstic_test.go b/tests/integration/staking/keeper/determinstic_test.go index 042882bf421..ded82a8be2d 100644 --- a/tests/integration/staking/keeper/determinstic_test.go +++ b/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) { diff --git a/x/staking/keeper/historical_info_test.go b/x/staking/keeper/historical_info_test.go index 9709c5440b5..f3b68ceec48 100644 --- a/x/staking/keeper/historical_info_test.go +++ b/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) diff --git a/x/staking/keeper/migrations.go b/x/staking/keeper/migrations.go index b34a7993bfc..26dd511e219 100644 --- a/x/staking/keeper/migrations.go +++ b/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) +} diff --git a/x/staking/migrations/v2/keys.go b/x/staking/migrations/v2/keys.go index 439a823206a..57ccb0aebee 100644 --- a/x/staking/migrations/v2/keys.go +++ b/x/staking/migrations/v2/keys.go @@ -1,6 +1,15 @@ 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))...) +} diff --git a/x/staking/migrations/v2/store_test.go b/x/staking/migrations/v2/store_test.go index 31c8016b1bb..b9416ad4f80 100644 --- a/x/staking/migrations/v2/store_test.go +++ b/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), }, } diff --git a/x/staking/migrations/v5/keys.go b/x/staking/migrations/v5/keys.go new file mode 100644 index 00000000000..3725301f77d --- /dev/null +++ b/x/staking/migrations/v5/keys.go @@ -0,0 +1,12 @@ +package v5 + +import "encoding/binary" + +var HistoricalInfoKey = []byte{0x50} // prefix for the historical info + +// GetHistoricalInfoKey returns a key prefix for indexing HistoricalInfo objects. +func GetHistoricalInfoKey(height int64) []byte { + heightBytes := make([]byte, 8) + binary.BigEndian.PutUint64(heightBytes, uint64(height)) + return append(HistoricalInfoKey, heightBytes...) +} diff --git a/x/staking/migrations/v5/migrations_test.go b/x/staking/migrations/v5/migrations_test.go new file mode 100644 index 00000000000..67607ffa4e8 --- /dev/null +++ b/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: v5.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}} +} diff --git a/x/staking/migrations/v5/store.go b/x/staking/migrations/v5/store.go new file mode 100644 index 00000000000..4ab2eadece2 --- /dev/null +++ b/x/staking/migrations/v5/store.go @@ -0,0 +1,46 @@ +package v5 + +import ( + "fmt" + "strconv" + + "cosmossdk.io/log" + "cosmossdk.io/store/prefix" + storetypes "cosmossdk.io/store/types" + sdk "github.com/cosmos/cosmos-sdk/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, 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 := GetHistoricalInfoKey(intHeight) + + // Set new key on store. Values don't change. + store.Set(newStoreKey, oldStoreIter.Value()) + oldStore.Delete(oldStoreIter.Key()) + } + + return nil +} diff --git a/x/staking/module.go b/x/staking/module.go index b58a7ee191d..5e93989da5d 100644 --- a/x/staking/module.go +++ b/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. diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index f8f642af5c9..d197d58d1d3 100644 --- a/x/staking/types/keys.go +++ b/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...) } diff --git a/x/staking/types/keys_test.go b/x/staking/types/keys_test.go index e5b96e48444..ce3c55e4540 100644 --- a/x/staking/types/keys_test.go +++ b/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)) + }) + } +}