Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(x/staking): Migrate LastValidatorPower to Collections #17498

Merged
merged 30 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a1fa4e3
wip: migrate LastValidatorPower to collections
likhita-809 Aug 22, 2023
3862dba
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 22, 2023
0d3368b
add diff tests
likhita-809 Aug 22, 2023
29fcab0
remove commented code
likhita-809 Aug 22, 2023
3c103ee
remove log
likhita-809 Aug 22, 2023
0cb3d26
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 23, 2023
3c3a788
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 23, 2023
7a6d3f3
wip: fix few tests
likhita-809 Aug 24, 2023
467e94d
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 24, 2023
f20a10f
try fixing test
likhita-809 Aug 24, 2023
a4371b9
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 24, 2023
c301ee3
fix lint
likhita-809 Aug 24, 2023
77f8538
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 28, 2023
63e1385
try fixing tests
likhita-809 Aug 28, 2023
ee4a932
cleanup
likhita-809 Aug 28, 2023
629deff
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 28, 2023
8ff06cd
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 28, 2023
c0b5325
add changelog
likhita-809 Aug 28, 2023
82a5e19
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 28, 2023
5c19c74
Merge branch 'main' into likhita/lastValPowerKey
likhita-809 Aug 28, 2023
8b5809a
Merge branch 'main' into likhita/lastValPowerKey
likhita-809 Aug 29, 2023
5644719
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
fd43c95
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
3aa588e
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
a9d51e4
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
44194e8
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 30, 2023
ef2a887
address review comments
likhita-809 Aug 30, 2023
54c7b14
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 30, 2023
92cb535
update changelog
likhita-809 Aug 30, 2023
633714c
fix lint
likhita-809 Aug 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,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`, `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`:
Expand Down
15 changes: 10 additions & 5 deletions x/staking/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -32,13 +37,13 @@ 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
}

return
return vals, returnErr
}

// ValidateGenesis validates the provided staking genesis state to ensure the
Expand Down
27 changes: 0 additions & 27 deletions x/staking/keeper/alias_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +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 {
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)
if err != nil {
return err
}

stop := fn(i, validator) // XXX is this safe will the validator unexposed fields be able to get written to?
if stop {
break
}
i++
}
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)
Expand Down
2 changes: 2 additions & 0 deletions x/staking/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type Keeper struct {
RedelegationsByValDst collections.Map[collections.Triple[[]byte, []byte, []byte], []byte]
RedelegationsByValSrc collections.Map[collections.Triple[[]byte, []byte, []byte], []byte]
UnbondingDelegationByValIndex collections.Map[collections.Pair[[]byte, []byte], []byte]
LastValidatorPower collections.Map[[]byte, []byte]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to say that I would find useful if we can add comments next to the field précising what is expected as key and as value. Right now, it is really not clear that we expect until I look deeper in the code.
I believe this would be a good practice to start doing when we simply use byte slices.

}

// NewKeeper creates a new staking Keeper instance
Expand Down Expand Up @@ -146,6 +147,7 @@ func NewKeeper(
),
collections.BytesValue,
),
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,
Expand Down
42 changes: 42 additions & 0 deletions x/staking/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,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"

Expand Down Expand Up @@ -195,6 +196,47 @@ 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)...)
}

func (s *KeeperTestSuite) TestLastTotalPowerMigrationToColls() {
s.SetupTest()

_, valAddrs := createValAddrs(100)

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)
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
},
"f28811f2b0a0ab9db60cdcae93680faff9dbadd4a3a8a2d088bb19b0428ad3a9",
)
s.Require().NoError(err)

err = testutil.DiffCollectionsMigration(
s.ctx,
s.key,
100,
func(i int64) {
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)
},
"f28811f2b0a0ab9db60cdcae93680faff9dbadd4a3a8a2d088bb19b0428ad3a9",
)
s.Require().NoError(err)
}

func (s *KeeperTestSuite) TestSrcRedelegationsMigrationToColls() {
s.SetupTest()

Expand Down
22 changes: 8 additions & 14 deletions x/staking/keeper/val_state_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,23 +463,17 @@ 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, value []byte) (bool, error) {
valAddrStr, err := k.validatorAddressCodec.BytesToString(key)
if err != nil {
return nil, err
return true, err
}

powerBytes := iterator.Value()
last[valAddrStr] = make([]byte, len(powerBytes))
copy(last[valAddrStr], powerBytes)
last[valAddrStr] = value
return false, nil
})
if err != nil {
return nil, err
}

return last, nil
Expand Down
53 changes: 19 additions & 34 deletions x/staking/keeper/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,7 @@ 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
}
Expand All @@ -358,81 +357,67 @@ 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))
}

// 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.Remove(ctx, operator)
}

// 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, 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
}

// 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 {
return nil, err
}
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, 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
Expand Down
3 changes: 2 additions & 1 deletion x/staking/keeper/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 5 additions & 1 deletion x/staking/migrations/v2/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestStoreMigration(t *testing.T) {
{
"LastValidatorPowerKey",
v1.GetLastValidatorPowerKey(valAddr1),
types.GetLastValidatorPowerKey(valAddr1),
getLastValidatorPowerKey(valAddr1),
},
{
"LastTotalPowerKey",
Expand Down Expand Up @@ -141,6 +141,10 @@ func TestStoreMigration(t *testing.T) {
}
}

func getLastValidatorPowerKey(operator sdk.ValAddress) []byte {
return append(types.LastValidatorPowerKey, sdkaddress.MustLengthPrefix(operator)...)
}

func getUBDByValIndexKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte {
return append(append(types.UnbondingDelegationByValIndexKey, sdkaddress.MustLengthPrefix(valAddr)...), sdkaddress.MustLengthPrefix(delAddr)...)
}
Expand Down
14 changes: 0 additions & 14 deletions x/staking/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions x/staking/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions x/staking/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = collections.NewPrefix(33) // prefix for each key to a validator
Expand Down Expand Up @@ -128,11 +128,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
Expand Down