From a1fa4e3e9e4781ce14c8c917dc3e872e690f756b Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Aug 2023 13:35:27 +0530 Subject: [PATCH 01/13] wip: migrate LastValidatorPower to collections --- x/staking/keeper/keeper.go | 4 +++- x/staking/keeper/validator.go | 20 ++++++++++++++------ x/staking/keeper/validator_test.go | 2 ++ x/staking/migrations/v2/store_test.go | 7 ++++++- x/staking/types/keys.go | 10 +++++----- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index a2d8f6ff3985..0d40c8491cf5 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -44,6 +44,7 @@ type Keeper struct { Redelegations collections.Map[collections.Triple[[]byte, []byte, []byte], types.Redelegation] Delegations collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], types.Delegation] UnbondingIndex collections.Map[uint64, []byte] + LastValidatorPower collections.Map[sdk.ValAddress, []byte] } // NewKeeper creates a new staking Keeper instance @@ -119,7 +120,8 @@ func NewKeeper( ), codec.CollValue[types.Redelegation](cdc), ), - UnbondingIndex: collections.NewMap(sb, types.UnbondingIndexKey, "unbonding_index", collections.Uint64Key, collections.BytesValue), + UnbondingIndex: collections.NewMap(sb, types.UnbondingIndexKey, "unbonding_index", collections.Uint64Key, collections.BytesValue), + LastValidatorPower: collections.NewMap(sb, types.LastValidatorPowerKey, "last_validator_power", sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility } schema, err := sb.Build() diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 1462b3e9a6fc..dd7b6c7751cf 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -342,8 +342,9 @@ func (k Keeper) ValidatorsPowerStoreIterator(ctx context.Context) (corestore.Ite // GetLastValidatorPower loads the last validator power. // Returns zero if the operator was not a validator last block. func (k Keeper) GetLastValidatorPower(ctx context.Context, operator sdk.ValAddress) (power int64, err error) { - store := k.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.GetLastValidatorPowerKey(operator)) + // store := k.storeService.OpenKVStore(ctx) + // bz, err := store.Get(types.GetLastValidatorPowerKey(operator)) + bz, err := k.LastValidatorPower.Get(ctx, operator) if err != nil { return 0, err } @@ -363,18 +364,25 @@ func (k Keeper) GetLastValidatorPower(ctx context.Context, operator sdk.ValAddre // SetLastValidatorPower sets the last validator power. func (k Keeper) SetLastValidatorPower(ctx context.Context, operator sdk.ValAddress, power int64) error { - store := k.storeService.OpenKVStore(ctx) + // store := k.storeService.OpenKVStore(ctx) bz, err := k.cdc.Marshal(&gogotypes.Int64Value{Value: power}) if err != nil { return err } - return store.Set(types.GetLastValidatorPowerKey(operator), bz) + // return store.Set(types.GetLastValidatorPowerKey(operator), bz) + return k.LastValidatorPower.Set(ctx, operator, bz) } // DeleteLastValidatorPower deletes the last validator power. func (k Keeper) DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAddress) error { - store := k.storeService.OpenKVStore(ctx) - return store.Delete(types.GetLastValidatorPowerKey(operator)) + // store := k.storeService.OpenKVStore(ctx) + // return store.Delete(types.GetLastValidatorPowerKey(operator)) + bz, err := k.cdc.Marshal(&gogotypes.Int64Value{Value: 0}) + if err != nil { + return err + } + return k.LastValidatorPower.Set(ctx, operator, bz) + // return k.LastValidatorPower.Remove(ctx, operator) } // lastValidatorsIterator returns an iterator for the consensus validators in the last block diff --git a/x/staking/keeper/validator_test.go b/x/staking/keeper/validator_test.go index f5015f7844a2..1e95acb6f251 100644 --- a/x/staking/keeper/validator_test.go +++ b/x/staking/keeper/validator_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "time" abci "github.com/cometbft/cometbft/abci/types" @@ -77,6 +78,7 @@ func (s *KeeperTestSuite) TestValidator() { // check the last validator power power := int64(100) require.NoError(keeper.SetLastValidatorPower(ctx, valAddr, power)) + fmt.Printf("valAddr: %v\n", valAddr.String()) resPower, err := keeper.GetLastValidatorPower(ctx, valAddr) require.NoError(err) require.Equal(power, resPower) diff --git a/x/staking/migrations/v2/store_test.go b/x/staking/migrations/v2/store_test.go index 7e8750877d0d..86ff7647b3c7 100644 --- a/x/staking/migrations/v2/store_test.go +++ b/x/staking/migrations/v2/store_test.go @@ -13,6 +13,7 @@ import ( sdktestuil "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + sdkaddress "github.com/cosmos/cosmos-sdk/types/address" v1 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v1" v2 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v2" "github.com/cosmos/cosmos-sdk/x/staking/testutil" @@ -45,7 +46,7 @@ func TestStoreMigration(t *testing.T) { { "LastValidatorPowerKey", v1.GetLastValidatorPowerKey(valAddr1), - types.GetLastValidatorPowerKey(valAddr1), + getLastValidatorPowerKey(valAddr1), }, { "LastTotalPowerKey", @@ -139,3 +140,7 @@ func TestStoreMigration(t *testing.T) { }) } } + +func getLastValidatorPowerKey(operator sdk.ValAddress) []byte { + return append(types.LastValidatorPowerKey, sdkaddress.MustLengthPrefix(operator)...) +} diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 961f9b33f3da..084855b797a5 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -32,7 +32,7 @@ const ( var ( // Keys for store prefixes // Last* values are constant during a block. - LastValidatorPowerKey = []byte{0x11} // prefix for each key to a validator index, for bonded validators + LastValidatorPowerKey = collections.NewPrefix(17) // prefix for each key to a validator index, for bonded validators LastTotalPowerKey = collections.NewPrefix(18) // prefix for the total power ValidatorsKey = []byte{0x21} // prefix for each key to a validator @@ -127,10 +127,10 @@ func GetValidatorsByPowerIndexKey(validator Validator, powerReduction math.Int, return key } -// GetLastValidatorPowerKey creates the bonded validator index key for an operator address -func GetLastValidatorPowerKey(operator sdk.ValAddress) []byte { - return append(LastValidatorPowerKey, address.MustLengthPrefix(operator)...) -} +// // GetLastValidatorPowerKey creates the bonded validator index key for an operator address +// func GetLastValidatorPowerKey(operator sdk.ValAddress) []byte { +// return append(LastValidatorPowerKey, address.MustLengthPrefix(operator)...) +// } // ParseValidatorPowerRankKey parses the validators operator address from power rank key func ParseValidatorPowerRankKey(key []byte) (operAddr []byte) { From 0d3368bdfaef9d38908dbcb8761259bb7c642a12 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Aug 2023 13:51:44 +0530 Subject: [PATCH 02/13] add diff tests --- x/staking/keeper/keeper_test.go | 42 +++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index f12a55fc04e4..a1ce85a788bf 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -12,7 +12,10 @@ import ( "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" + gogotypes "github.com/cosmos/gogoproto/types" + "github.com/cosmos/cosmos-sdk/baseapp" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/testutil" @@ -42,6 +45,7 @@ type KeeperTestSuite struct { queryClient stakingtypes.QueryClient msgServer stakingtypes.MsgServer key *storetypes.KVStoreKey + cdc codec.Codec } func (s *KeeperTestSuite) SetupTest() { @@ -52,6 +56,7 @@ func (s *KeeperTestSuite) SetupTest() { s.key = key ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) encCfg := moduletestutil.MakeTestEncodingConfig() + s.cdc = encCfg.Codec ctrl := gomock.NewController(s.T()) accountKeeper := stakingtestutil.NewMockAccountKeeper(ctrl) @@ -137,6 +142,43 @@ func getREDsFromValSrcIndexKey(valSrcAddr sdk.ValAddress) []byte { return append(redelegationByValSrcIndexKey, addresstypes.MustLengthPrefix(valSrcAddr)...) } +func getLastValidatorPowerKey(operator sdk.ValAddress) []byte { + lastValidatorPowerKey := []byte{0x11} + return append(lastValidatorPowerKey, addresstypes.MustLengthPrefix(operator)...) +} + +func (s *KeeperTestSuite) TestLastTotalPowerMigrationToColls() { + s.SetupTest() + + _, valAddrs := createValAddrs(101) + + bz, err := s.cdc.Marshal(&gogotypes.Int64Value{Value: 1}) + s.Require().NoError(err) + + err = testutil.DiffCollectionsMigration( + s.ctx, + s.key, + 100, + func(i int64) { + s.ctx.KVStore(s.key).Set(getLastValidatorPowerKey(valAddrs[i]), bz) + }, + "6cd9b908445fbe0b280b82cac51758cdb125882674a91d348b690dac4b7055cb", + ) + s.Require().NoError(err) + + err = testutil.DiffCollectionsMigration( + s.ctx, + s.key, + 100, + func(i int64) { + err := s.stakingKeeper.LastValidatorPower.Set(s.ctx, valAddrs[i], bz) + s.Require().NoError(err) + }, + "6cd9b908445fbe0b280b82cac51758cdb125882674a91d348b690dac4b7055cb", + ) + s.Require().NoError(err) +} + func (s *KeeperTestSuite) TestRedelegationsMigrationToColls() { s.SetupTest() From 29fcab0977c664e991d9df4ae123c6b4048653f8 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Aug 2023 14:30:09 +0530 Subject: [PATCH 03/13] remove commented code --- x/staking/keeper/validator.go | 7 ------- x/staking/types/keys.go | 5 ----- 2 files changed, 12 deletions(-) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index dd7b6c7751cf..bd4c3b4ed015 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -342,8 +342,6 @@ func (k Keeper) ValidatorsPowerStoreIterator(ctx context.Context) (corestore.Ite // GetLastValidatorPower loads the last validator power. // Returns zero if the operator was not a validator last block. func (k Keeper) GetLastValidatorPower(ctx context.Context, operator sdk.ValAddress) (power int64, err error) { - // store := k.storeService.OpenKVStore(ctx) - // bz, err := store.Get(types.GetLastValidatorPowerKey(operator)) bz, err := k.LastValidatorPower.Get(ctx, operator) if err != nil { return 0, err @@ -364,25 +362,20 @@ func (k Keeper) GetLastValidatorPower(ctx context.Context, operator sdk.ValAddre // SetLastValidatorPower sets the last validator power. func (k Keeper) SetLastValidatorPower(ctx context.Context, operator sdk.ValAddress, power int64) error { - // store := k.storeService.OpenKVStore(ctx) bz, err := k.cdc.Marshal(&gogotypes.Int64Value{Value: power}) if err != nil { return err } - // return store.Set(types.GetLastValidatorPowerKey(operator), bz) return k.LastValidatorPower.Set(ctx, operator, bz) } // DeleteLastValidatorPower deletes the last validator power. func (k Keeper) DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAddress) error { - // store := k.storeService.OpenKVStore(ctx) - // return store.Delete(types.GetLastValidatorPowerKey(operator)) bz, err := k.cdc.Marshal(&gogotypes.Int64Value{Value: 0}) if err != nil { return err } return k.LastValidatorPower.Set(ctx, operator, bz) - // return k.LastValidatorPower.Remove(ctx, operator) } // lastValidatorsIterator returns an iterator for the consensus validators in the last block diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 759ce9f57438..2a6e0e7f14fc 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -127,11 +127,6 @@ func GetValidatorsByPowerIndexKey(validator Validator, powerReduction math.Int, return key } -// // GetLastValidatorPowerKey creates the bonded validator index key for an operator address -// func GetLastValidatorPowerKey(operator sdk.ValAddress) []byte { -// return append(LastValidatorPowerKey, address.MustLengthPrefix(operator)...) -// } - // ParseValidatorPowerRankKey parses the validators operator address from power rank key func ParseValidatorPowerRankKey(key []byte) (operAddr []byte) { powerBytesLen := 8 From 3c103ee548dacb3d1db4ab9da8451f9514fefd3a Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Aug 2023 16:40:02 +0530 Subject: [PATCH 04/13] remove log --- x/staking/keeper/validator_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/staking/keeper/validator_test.go b/x/staking/keeper/validator_test.go index 1e95acb6f251..f5015f7844a2 100644 --- a/x/staking/keeper/validator_test.go +++ b/x/staking/keeper/validator_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "fmt" "time" abci "github.com/cometbft/cometbft/abci/types" @@ -78,7 +77,6 @@ func (s *KeeperTestSuite) TestValidator() { // check the last validator power power := int64(100) require.NoError(keeper.SetLastValidatorPower(ctx, valAddr, power)) - fmt.Printf("valAddr: %v\n", valAddr.String()) resPower, err := keeper.GetLastValidatorPower(ctx, valAddr) require.NoError(err) require.Equal(power, resPower) From 7a6d3f3b8245f69888a4c46cb6d635a4a940082b Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 24 Aug 2023 15:58:08 +0530 Subject: [PATCH 05/13] wip: fix few tests --- x/staking/keeper/alias_functions.go | 21 +++++------- x/staking/keeper/keeper.go | 4 +-- x/staking/keeper/val_state_change.go | 20 +++++------- x/staking/keeper/validator.go | 48 +++++++++++----------------- 4 files changed, 37 insertions(+), 56 deletions(-) diff --git a/x/staking/keeper/alias_functions.go b/x/staking/keeper/alias_functions.go index d9790e784169..dbab19a8d3ed 100644 --- a/x/staking/keeper/alias_functions.go +++ b/x/staking/keeper/alias_functions.go @@ -72,27 +72,22 @@ func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(inde // IterateLastValidators iterates through the active validator set and perform the provided function func (k Keeper) IterateLastValidators(ctx context.Context, fn func(index int64, validator types.ValidatorI) (stop bool)) error { - iterator, err := k.LastValidatorsIterator(ctx) - if err != nil { - return err - } - defer iterator.Close() - i := int64(0) - - for ; iterator.Valid(); iterator.Next() { - address := types.AddressFromLastValidatorPowerKey(iterator.Key()) - - validator, err := k.GetValidator(ctx, address) + err := k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + validator, err := k.GetValidator(ctx, key) if err != nil { - return err + return true, err } stop := fn(i, validator) // XXX is this safe will the validator unexposed fields be able to get written to? if stop { - break + return true, err } i++ + return false, nil + }) + if err != nil { + return err } return nil } diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 87e73cb6972a..e057e6daaa09 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -46,7 +46,7 @@ type Keeper struct { UnbondingIndex collections.Map[uint64, []byte] RedelegationsByValDst collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] RedelegationsByValSrc collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] - LastValidatorPower collections.Map[sdk.ValAddress, []byte] + LastValidatorPower collections.Map[[]byte, []byte] } // NewKeeper creates a new staking Keeper instance @@ -135,7 +135,7 @@ func NewKeeper( ), collections.BytesValue, ), - LastValidatorPower: collections.NewMap(sb, types.LastValidatorPowerKey, "last_validator_power", sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + LastValidatorPower: collections.NewMap(sb, types.LastValidatorPowerKey, "last_validator_power", sdk.LengthPrefixedBytesKey, collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedBytesKey is needed to retain state compatibility // key format is: 54 | lengthPrefixedBytes(DstValAddr) | lengthPrefixedBytes(AccAddr) | lengthPrefixedBytes(SrcValAddr) RedelegationsByValDst: collections.NewMap( sb, types.RedelegationByValDstIndexKey, diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index aee84f9c520e..b8007f283b5c 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -463,23 +463,19 @@ type validatorsByAddr map[string][]byte func (k Keeper) getLastValidatorsByAddr(ctx context.Context) (validatorsByAddr, error) { last := make(validatorsByAddr) - iterator, err := k.LastValidatorsIterator(ctx) - if err != nil { - return nil, err - } - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - // extract the validator address from the key (prefix is 1-byte, addrLen is 1-byte) - valAddr := types.AddressFromLastValidatorPowerKey(iterator.Key()) - valAddrStr, err := k.validatorAddressCodec.BytesToString(valAddr) + err := k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + valAddrStr, err := k.validatorAddressCodec.BytesToString(key) if err != nil { - return nil, err + return true, err } - powerBytes := iterator.Value() + powerBytes := value last[valAddrStr] = make([]byte, len(powerBytes)) copy(last[valAddrStr], powerBytes) + return false, nil + }) + if err != nil { + return nil, err } return last, nil diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index bd4c3b4ed015..289ccd37eb7d 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -358,6 +358,7 @@ func (k Keeper) GetLastValidatorPower(ctx context.Context, operator sdk.ValAddre } return intV.GetValue(), nil + // return k.LastValidatorPower.Get(ctx, operator.Bytes()) } // SetLastValidatorPower sets the last validator power. @@ -367,6 +368,7 @@ func (k Keeper) SetLastValidatorPower(ctx context.Context, operator sdk.ValAddre return err } return k.LastValidatorPower.Set(ctx, operator, bz) + // return k.LastValidatorPower.Set(ctx, operator.Bytes(), power) } // DeleteLastValidatorPower deletes the last validator power. @@ -375,33 +377,26 @@ func (k Keeper) DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAd if err != nil { return err } - return k.LastValidatorPower.Set(ctx, operator, bz) -} - -// lastValidatorsIterator returns an iterator for the consensus validators in the last block -func (k Keeper) LastValidatorsIterator(ctx context.Context) (corestore.Iterator, error) { - store := k.storeService.OpenKVStore(ctx) - return store.Iterator(types.LastValidatorPowerKey, storetypes.PrefixEndBytes(types.LastValidatorPowerKey)) + return k.LastValidatorPower.Set(ctx, operator, bz) // setting power to 0 on deletion } // IterateLastValidatorPowers iterates over last validator powers. func (k Keeper) IterateLastValidatorPowers(ctx context.Context, handler func(operator sdk.ValAddress, power int64) (stop bool)) error { - iter, err := k.LastValidatorsIterator(ctx) - if err != nil { - return err - } - - for ; iter.Valid(); iter.Next() { - addr := sdk.ValAddress(types.AddressFromLastValidatorPowerKey(iter.Key())) + err := k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + addr := sdk.ValAddress(key) intV := &gogotypes.Int64Value{} - if err = k.cdc.Unmarshal(iter.Value(), intV); err != nil { - return err + if err := k.cdc.Unmarshal(value, intV); err != nil { + return true, err } if handler(addr, intV.GetValue()) { - break + return true, nil } + return false, nil + }) + if err != nil { + return err } return nil @@ -409,8 +404,6 @@ func (k Keeper) IterateLastValidatorPowers(ctx context.Context, handler func(ope // GetLastValidators gets the group of the bonded validators func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Validator, err error) { - store := k.storeService.OpenKVStore(ctx) - // add the actual validator power sorted store maxValidators, err := k.MaxValidators(ctx) if err != nil { @@ -418,27 +411,24 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid } validators = make([]types.Validator, maxValidators) - iterator, err := store.Iterator(types.LastValidatorPowerKey, storetypes.PrefixEndBytes(types.LastValidatorPowerKey)) - if err != nil { - return nil, err - } - defer iterator.Close() - i := 0 - for ; iterator.Valid(); iterator.Next() { + err = k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { // sanity check if i >= int(maxValidators) { panic("more validators than maxValidators found") } - address := types.AddressFromLastValidatorPowerKey(iterator.Key()) - validator, err := k.GetValidator(ctx, address) + validator, err := k.GetValidator(ctx, key) if err != nil { - return nil, err + return true, err } validators[i] = validator i++ + return false, nil + }) + if err != nil { + return nil, err } return validators[:i], nil // trim From f20a10f37e48661234a3cab0f1b85b0c39d93956 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 24 Aug 2023 16:31:29 +0530 Subject: [PATCH 06/13] try fixing test --- x/staking/keeper/keeper.go | 2 +- x/staking/keeper/validator.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index e057e6daaa09..42d5f68cd63a 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -135,7 +135,7 @@ func NewKeeper( ), collections.BytesValue, ), - LastValidatorPower: collections.NewMap(sb, types.LastValidatorPowerKey, "last_validator_power", sdk.LengthPrefixedBytesKey, collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedBytesKey is needed to retain state compatibility + LastValidatorPower: collections.NewMap(sb, types.LastValidatorPowerKey, "last_validator_power", sdk.LengthPrefixedBytesKey, collections.BytesValue), // sdk.LengthPrefixedBytesKey is needed to retain state compatibility // key format is: 54 | lengthPrefixedBytes(DstValAddr) | lengthPrefixedBytes(AccAddr) | lengthPrefixedBytes(SrcValAddr) RedelegationsByValDst: collections.NewMap( sb, types.RedelegationByValDstIndexKey, diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 289ccd37eb7d..b27d00fdbd4a 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -415,7 +415,8 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid err = k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { // sanity check if i >= int(maxValidators) { - panic("more validators than maxValidators found") + // panic("more validators than maxValidators found") + return true, nil } validator, err := k.GetValidator(ctx, key) From c301ee34e42a85db133786ef950ef8237bea013e Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 24 Aug 2023 16:51:09 +0530 Subject: [PATCH 07/13] fix lint --- x/staking/keeper/alias_functions.go | 2 +- x/staking/keeper/keeper_test.go | 3 +-- x/staking/keeper/val_state_change.go | 2 +- x/staking/keeper/validator.go | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/x/staking/keeper/alias_functions.go b/x/staking/keeper/alias_functions.go index dbab19a8d3ed..46b46247be1e 100644 --- a/x/staking/keeper/alias_functions.go +++ b/x/staking/keeper/alias_functions.go @@ -73,7 +73,7 @@ func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(inde // IterateLastValidators iterates through the active validator set and perform the provided function func (k Keeper) IterateLastValidators(ctx context.Context, fn func(index int64, validator types.ValidatorI) (stop bool)) error { i := int64(0) - err := k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + err := k.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { validator, err := k.GetValidator(ctx, key) if err != nil { return true, err diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index cdd8d9a36bce..51beb1a5dfe4 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -5,6 +5,7 @@ import ( cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" cmttime "github.com/cometbft/cometbft/types/time" + gogotypes "github.com/cosmos/gogoproto/types" "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" @@ -12,8 +13,6 @@ import ( "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" - gogotypes "github.com/cosmos/gogoproto/types" - "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/address" diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index b8007f283b5c..edb4d32e43d8 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -463,7 +463,7 @@ type validatorsByAddr map[string][]byte func (k Keeper) getLastValidatorsByAddr(ctx context.Context) (validatorsByAddr, error) { last := make(validatorsByAddr) - err := k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + err := k.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { valAddrStr, err := k.validatorAddressCodec.BytesToString(key) if err != nil { return true, err diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index b27d00fdbd4a..2a3ad37c1227 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -382,7 +382,7 @@ func (k Keeper) DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAd // IterateLastValidatorPowers iterates over last validator powers. func (k Keeper) IterateLastValidatorPowers(ctx context.Context, handler func(operator sdk.ValAddress, power int64) (stop bool)) error { - err := k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + err := k.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { addr := sdk.ValAddress(key) intV := &gogotypes.Int64Value{} @@ -412,7 +412,7 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid validators = make([]types.Validator, maxValidators) i := 0 - err = k.LastValidatorPower.Walk(ctx, nil, func(key []byte, value []byte) (bool, error) { + err = k.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { // sanity check if i >= int(maxValidators) { // panic("more validators than maxValidators found") From 63e13853fdb9d2d1471e88575d8745afd7765191 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 13:31:26 +0530 Subject: [PATCH 08/13] try fixing tests --- x/staking/keeper/validator.go | 6 +----- x/staking/keeper/validator_test.go | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 3f00ba8ec256..534247f7d036 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -373,11 +373,7 @@ func (k Keeper) SetLastValidatorPower(ctx context.Context, operator sdk.ValAddre // DeleteLastValidatorPower deletes the last validator power. func (k Keeper) DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAddress) error { - bz, err := k.cdc.Marshal(&gogotypes.Int64Value{Value: 0}) - if err != nil { - return err - } - return k.LastValidatorPower.Set(ctx, operator, bz) // setting power to 0 on deletion + return k.LastValidatorPower.Remove(ctx, operator) // setting power to 0 on deletion } // IterateLastValidatorPowers iterates over last validator powers. diff --git a/x/staking/keeper/validator_test.go b/x/staking/keeper/validator_test.go index f5015f7844a2..f3afffa87bdd 100644 --- a/x/staking/keeper/validator_test.go +++ b/x/staking/keeper/validator_test.go @@ -6,6 +6,7 @@ import ( abci "github.com/cometbft/cometbft/abci/types" "github.com/golang/mock/gomock" + "cosmossdk.io/collections" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" @@ -82,7 +83,7 @@ func (s *KeeperTestSuite) TestValidator() { require.Equal(power, resPower) require.NoError(keeper.DeleteLastValidatorPower(ctx, valAddr)) resPower, err = keeper.GetLastValidatorPower(ctx, valAddr) - require.NoError(err) + require.Error(err, collections.ErrNotFound) require.Equal(int64(0), resPower) } From ee4a9320743316820ae8f31702482171d1cae3f7 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 13:56:07 +0530 Subject: [PATCH 09/13] cleanup --- x/staking/keeper/validator.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 534247f7d036..39bf25b3bf7c 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -358,7 +358,6 @@ func (k Keeper) GetLastValidatorPower(ctx context.Context, operator sdk.ValAddre } return intV.GetValue(), nil - // return k.LastValidatorPower.Get(ctx, operator.Bytes()) } // SetLastValidatorPower sets the last validator power. @@ -368,12 +367,11 @@ func (k Keeper) SetLastValidatorPower(ctx context.Context, operator sdk.ValAddre return err } return k.LastValidatorPower.Set(ctx, operator, bz) - // return k.LastValidatorPower.Set(ctx, operator.Bytes(), power) } // DeleteLastValidatorPower deletes the last validator power. func (k Keeper) DeleteLastValidatorPower(ctx context.Context, operator sdk.ValAddress) error { - return k.LastValidatorPower.Remove(ctx, operator) // setting power to 0 on deletion + return k.LastValidatorPower.Remove(ctx, operator) } // IterateLastValidatorPowers iterates over last validator powers. @@ -411,8 +409,7 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid err = k.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { // sanity check if i >= int(maxValidators) { - // panic("more validators than maxValidators found") - return true, nil + panic("more validators than maxValidators found") } validator, err := k.GetValidator(ctx, key) From c0b5325d8ff0edeccca8d7c9e3c94c3425309198 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 15:51:14 +0530 Subject: [PATCH 10/13] add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 734ee100dbfe..200d5161ff70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/staking) [#17498](https://github.com/cosmos/cosmos-sdk/pull/17498) Use collections for `LastValidatorPower`: + * remove from `types`: `GetLastValidatorPowerKey` + * remove from `Keeper`: `LastValidatorsIterator` * (x/staking) [#17270](https://github.com/cosmos/cosmos-sdk/pull/17270) Use collections for `UnbondingDelegation`: * remove from `types`: `GetUBDsKey` * remove from `Keeper`: `IterateUnbondingDelegations`, `IterateDelegatorUnbondingDelegations` From ef2a887a77eb6be3ee597bd3c61ee2d0c8845c6d Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 30 Aug 2023 15:05:02 +0530 Subject: [PATCH 11/13] address review comments --- x/staking/genesis.go | 13 ++++++++---- x/staking/keeper/alias_functions.go | 22 -------------------- x/staking/keeper/keeper_test.go | 20 +++++++++++------- x/staking/keeper/val_state_change.go | 4 +--- x/staking/testutil/expected_keepers_mocks.go | 14 ------------- x/staking/types/expected_keepers.go | 4 ---- 6 files changed, 22 insertions(+), 55 deletions(-) diff --git a/x/staking/genesis.go b/x/staking/genesis.go index 1d4cd64e4bba..6b249b370b9c 100644 --- a/x/staking/genesis.go +++ b/x/staking/genesis.go @@ -13,16 +13,21 @@ import ( // WriteValidators returns a slice of bonded genesis validators. func WriteValidators(ctx sdk.Context, keeper *keeper.Keeper) (vals []cmttypes.GenesisValidator, returnErr error) { - err := keeper.IterateLastValidators(ctx, func(_ int64, validator types.ValidatorI) (stop bool) { + err := keeper.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { + validator, err := keeper.GetValidator(ctx, key) + if err != nil { + return true, err + } + pk, err := validator.ConsPubKey() if err != nil { returnErr = err - return true + return true, err } cmtPk, err := cryptocodec.ToCmtPubKeyInterface(pk) if err != nil { returnErr = err - return true + return true, err } vals = append(vals, cmttypes.GenesisValidator{ @@ -32,7 +37,7 @@ func WriteValidators(ctx sdk.Context, keeper *keeper.Keeper) (vals []cmttypes.Ge Name: validator.GetMoniker(), }) - return false + return false, nil }) if err != nil { return nil, err diff --git a/x/staking/keeper/alias_functions.go b/x/staking/keeper/alias_functions.go index 46b46247be1e..1a00b71bc00a 100644 --- a/x/staking/keeper/alias_functions.go +++ b/x/staking/keeper/alias_functions.go @@ -70,28 +70,6 @@ func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(inde return nil } -// IterateLastValidators iterates through the active validator set and perform the provided function -func (k Keeper) IterateLastValidators(ctx context.Context, fn func(index int64, validator types.ValidatorI) (stop bool)) error { - i := int64(0) - err := k.LastValidatorPower.Walk(ctx, nil, func(key, value []byte) (bool, error) { - validator, err := k.GetValidator(ctx, key) - if err != nil { - return true, err - } - - stop := fn(i, validator) // XXX is this safe will the validator unexposed fields be able to get written to? - if stop { - return true, err - } - i++ - return false, nil - }) - if err != nil { - return err - } - return nil -} - // Validator gets the Validator interface for a particular address func (k Keeper) Validator(ctx context.Context, address sdk.ValAddress) (types.ValidatorI, error) { return k.GetValidator(ctx, address) diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index a68a51e4be5b..998a6a00eb5f 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -189,6 +189,7 @@ func getValidatorKey(operatorAddr sdk.ValAddress) []byte { return append(validatorsKey, addresstypes.MustLengthPrefix(operatorAddr)...) } +// getLastValidatorPowerKey creates the bonded validator index key for an operator address func getLastValidatorPowerKey(operator sdk.ValAddress) []byte { lastValidatorPowerKey := []byte{0x11} return append(lastValidatorPowerKey, addresstypes.MustLengthPrefix(operator)...) @@ -197,19 +198,19 @@ func getLastValidatorPowerKey(operator sdk.ValAddress) []byte { func (s *KeeperTestSuite) TestLastTotalPowerMigrationToColls() { s.SetupTest() - _, valAddrs := createValAddrs(101) - - bz, err := s.cdc.Marshal(&gogotypes.Int64Value{Value: 1}) - s.Require().NoError(err) + _, valAddrs := createValAddrs(100) - err = testutil.DiffCollectionsMigration( + err := testutil.DiffCollectionsMigration( s.ctx, s.key, 100, func(i int64) { + bz, err := s.cdc.Marshal(&gogotypes.Int64Value{Value: i}) + s.Require().NoError(err) + s.ctx.KVStore(s.key).Set(getLastValidatorPowerKey(valAddrs[i]), bz) }, - "6cd9b908445fbe0b280b82cac51758cdb125882674a91d348b690dac4b7055cb", + "f28811f2b0a0ab9db60cdcae93680faff9dbadd4a3a8a2d088bb19b0428ad3a9", ) s.Require().NoError(err) @@ -218,10 +219,13 @@ func (s *KeeperTestSuite) TestLastTotalPowerMigrationToColls() { s.key, 100, func(i int64) { - err := s.stakingKeeper.LastValidatorPower.Set(s.ctx, valAddrs[i], bz) + bz, err := s.cdc.Marshal(&gogotypes.Int64Value{Value: i}) + s.Require().NoError(err) + + err = s.stakingKeeper.LastValidatorPower.Set(s.ctx, valAddrs[i], bz) s.Require().NoError(err) }, - "6cd9b908445fbe0b280b82cac51758cdb125882674a91d348b690dac4b7055cb", + "f28811f2b0a0ab9db60cdcae93680faff9dbadd4a3a8a2d088bb19b0428ad3a9", ) s.Require().NoError(err) } diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index edb4d32e43d8..c0c996a28edb 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -469,9 +469,7 @@ func (k Keeper) getLastValidatorsByAddr(ctx context.Context) (validatorsByAddr, return true, err } - powerBytes := value - last[valAddrStr] = make([]byte, len(powerBytes)) - copy(last[valAddrStr], powerBytes) + last[valAddrStr] = value return false, nil }) if err != nil { diff --git a/x/staking/testutil/expected_keepers_mocks.go b/x/staking/testutil/expected_keepers_mocks.go index 1bdee0360354..470f112cfc84 100644 --- a/x/staking/testutil/expected_keepers_mocks.go +++ b/x/staking/testutil/expected_keepers_mocks.go @@ -336,20 +336,6 @@ func (mr *MockValidatorSetMockRecorder) IterateBondedValidatorsByPower(arg0, arg return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IterateBondedValidatorsByPower", reflect.TypeOf((*MockValidatorSet)(nil).IterateBondedValidatorsByPower), arg0, arg1) } -// IterateLastValidators mocks base method. -func (m *MockValidatorSet) IterateLastValidators(arg0 context.Context, arg1 func(int64, types0.ValidatorI) bool) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IterateLastValidators", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 -} - -// IterateLastValidators indicates an expected call of IterateLastValidators. -func (mr *MockValidatorSetMockRecorder) IterateLastValidators(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IterateLastValidators", reflect.TypeOf((*MockValidatorSet)(nil).IterateLastValidators), arg0, arg1) -} - // IterateValidators mocks base method. func (m *MockValidatorSet) IterateValidators(arg0 context.Context, arg1 func(int64, types0.ValidatorI) bool) error { m.ctrl.T.Helper() diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go index 3c3ae276ed16..0cd5f453f512 100644 --- a/x/staking/types/expected_keepers.go +++ b/x/staking/types/expected_keepers.go @@ -52,10 +52,6 @@ type ValidatorSet interface { IterateBondedValidatorsByPower(context.Context, func(index int64, validator ValidatorI) (stop bool)) error - // iterate through the consensus validator set of the last block by operator address, execute func for each validator - IterateLastValidators(context.Context, - func(index int64, validator ValidatorI) (stop bool)) error - Validator(context.Context, sdk.ValAddress) (ValidatorI, error) // get a particular validator by operator address ValidatorByConsAddr(context.Context, sdk.ConsAddress) (ValidatorI, error) // get a particular validator by consensus address TotalBondedTokens(context.Context) (math.Int, error) // total bonded tokens within the validator set From 92cb5351b909b2c5350ec57c4eaa5e0afb9587cd Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 30 Aug 2023 15:07:44 +0530 Subject: [PATCH 12/13] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be83413766f8..d58b5d9add4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,7 +61,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [#17498](https://github.com/cosmos/cosmos-sdk/pull/17498) Use collections for `LastValidatorPower`: * remove from `types`: `GetLastValidatorPowerKey` - * remove from `Keeper`: `LastValidatorsIterator` + * remove from `Keeper`: `LastValidatorsIterator`, `IterateLastValidators` * (x/staking) [#17291](https://github.com/cosmos/cosmos-sdk/pull/17291) Use collections for `UnbondingDelegationByValIndex`: * remove from `types`: `GetUBDKeyFromValIndexKey`, `GetUBDsByValIndexKey`, `GetUBDByValIndexKey` * (x/slashing) [#17568](https://github.com/cosmos/cosmos-sdk/pull/17568) Use collections for `ValidatorMissedBlockBitmap`: From 633714caf258b529fb33df4401d18732fe3eb922 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 30 Aug 2023 15:34:43 +0530 Subject: [PATCH 13/13] fix lint --- x/staking/genesis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/staking/genesis.go b/x/staking/genesis.go index 6b249b370b9c..005ec107a5b4 100644 --- a/x/staking/genesis.go +++ b/x/staking/genesis.go @@ -43,7 +43,7 @@ func WriteValidators(ctx sdk.Context, keeper *keeper.Keeper) (vals []cmttypes.Ge return nil, err } - return + return vals, returnErr } // ValidateGenesis validates the provided staking genesis state to ensure the