From bc2da6f52a28d1943d76ad5a27f1e7e265b5cec8 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 22:17:06 +0530 Subject: [PATCH 01/10] wip: migrate ValidatorQueue to collections --- x/staking/keeper/keeper.go | 14 ++++- x/staking/keeper/validator.go | 74 +++++++++++++-------------- x/staking/migrations/v2/store_test.go | 25 ++++++++- x/staking/types/keys.go | 31 ++--------- x/staking/types/keys_test.go | 28 ---------- 5 files changed, 77 insertions(+), 95 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 792b447e51f4..eee40df67548 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -3,6 +3,7 @@ package keeper import ( "context" "fmt" + "time" "cosmossdk.io/collections" collcodec "cosmossdk.io/collections/codec" @@ -48,6 +49,7 @@ type Keeper struct { UnbondingDelegations collections.Map[collections.Pair[[]byte, []byte], types.UnbondingDelegation] RedelegationsByValDst collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] RedelegationsByValSrc collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] + ValidatorQueue collections.Map[collections.Pair[time.Time, int64], []byte] } // NewKeeper creates a new staking Keeper instance @@ -155,7 +157,17 @@ func NewKeeper( collections.BytesKey, sdk.LengthPrefixedBytesKey, // sdk.LengthPrefixedBytesKey is needed to retain state compatibility ), - codec.CollValue[types.UnbondingDelegation](cdc)), + codec.CollValue[types.UnbondingDelegation](cdc), + ), + ValidatorQueue: collections.NewMap( + sb, types.ValidatorQueueKey, + "validator_queue", + collections.PairKeyCodec( + sdk.TimeKey, + collections.Int64Key, + ), + collections.BytesValue, + ), } schema, err := sb.Build() diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 7d0c75ae54e7..a150bd16a00a 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -441,11 +441,12 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid // GetUnbondingValidators returns a slice of mature validator addresses that // complete their unbonding at a given time and height. func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, endHeight int64) ([]string, error) { - store := k.storeService.OpenKVStore(ctx) - - bz, err := store.Get(types.GetValidatorQueueKey(endTime, endHeight)) + bz, err := k.ValidatorQueue.Get(ctx, collections.Join(endTime, endHeight)) if err != nil { - return nil, err + if !errors.Is(err, collections.ErrNotFound) { + return nil, err + } + return []string{}, nil } if bz == nil { @@ -463,12 +464,11 @@ func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, e // SetUnbondingValidatorsQueue sets a given slice of validator addresses into // the unbonding validator queue by a given height and time. func (k Keeper) SetUnbondingValidatorsQueue(ctx context.Context, endTime time.Time, endHeight int64, addrs []string) error { - store := k.storeService.OpenKVStore(ctx) bz, err := k.cdc.Marshal(&types.ValAddresses{Addresses: addrs}) if err != nil { return err } - return store.Set(types.GetValidatorQueueKey(endTime, endHeight), bz) + return k.ValidatorQueue.Set(ctx, collections.Join(endTime, endHeight), bz) } // InsertUnbondingValidatorQueue inserts a given unbonding validator address into @@ -485,8 +485,7 @@ func (k Keeper) InsertUnbondingValidatorQueue(ctx context.Context, val types.Val // DeleteValidatorQueueTimeSlice deletes all entries in the queue indexed by a // given height and time. func (k Keeper) DeleteValidatorQueueTimeSlice(ctx context.Context, endTime time.Time, endHeight int64) error { - store := k.storeService.OpenKVStore(ctx) - return store.Delete(types.GetValidatorQueueKey(endTime, endHeight)) + return k.ValidatorQueue.Remove(ctx, collections.Join(endTime, endHeight)) } // DeleteValidatorQueue removes a validator by address from the unbonding queue @@ -523,13 +522,6 @@ func (k Keeper) DeleteValidatorQueue(ctx context.Context, val types.Validator) e return k.SetUnbondingValidatorsQueue(ctx, val.UnbondingTime, val.UnbondingHeight, newAddrs) } -// ValidatorQueueIterator returns an interator ranging over validators that are -// unbonding whose unbonding completion occurs at the given height and time. -func (k Keeper) ValidatorQueueIterator(ctx context.Context, endTime time.Time, endHeight int64) (corestore.Iterator, error) { - store := k.storeService.OpenKVStore(ctx) - return store.Iterator(types.ValidatorQueueKey, storetypes.InclusiveEndBytes(types.GetValidatorQueueKey(endTime, endHeight))) -} - // UnbondAllMatureValidators unbonds all the mature unbonding validators that // have finished their unbonding period. func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { @@ -542,61 +534,64 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { // ValidatorQueueKey | timeBzLen (8-byte big endian) | timeBz | heightBz (8-byte big endian), // so it may be possible that certain validator addresses that are iterated // over are not ready to unbond, so an explicit check is required. - unbondingValIterator, err := k.ValidatorQueueIterator(ctx, blockTime, blockHeight) - if err != nil { - return err - } - defer unbondingValIterator.Close() - - for ; unbondingValIterator.Valid(); unbondingValIterator.Next() { - key := unbondingValIterator.Key() - keyTime, keyHeight, err := types.ParseValidatorQueueKey(key) - if err != nil { - return fmt.Errorf("failed to parse unbonding key: %w", err) - } + // unbondingValIterator, err := k.ValidatorQueueIterator(ctx, blockTime, blockHeight) + // if err != nil { + // return err + // } + // defer unbondingValIterator.Close() + rng := collections.NewPrefixUntilPairRange[time.Time, int64](blockTime).EndInclusive(blockHeight) + err := k.ValidatorQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, int64], value []byte) (stop bool, err error) { + // for ; unbondingValIterator.Valid(); unbondingValIterator.Next() { + // key := unbondingValIterator.Key() + // keyTime, keyHeight, err := types.ParseValidatorQueueKey(key) + // if err != nil { + // return true, fmt.Errorf("failed to parse unbonding key: %w", err) + // } + keyTime := key.K1() + keyHeight := key.K2() // All addresses for the given key have the same unbonding height and time. // We only unbond if the height and time are less than the current height // and time. if keyHeight <= blockHeight && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) { addrs := types.ValAddresses{} - if err = k.cdc.Unmarshal(unbondingValIterator.Value(), &addrs); err != nil { - return err + if err = k.cdc.Unmarshal(value, &addrs); err != nil { + return true, err } for _, valAddr := range addrs.Addresses { addr, err := k.validatorAddressCodec.StringToBytes(valAddr) if err != nil { - return err + return true, err } val, err := k.GetValidator(ctx, addr) if err != nil { - return errorsmod.Wrap(err, "validator in the unbonding queue was not found") + return true, errorsmod.Wrap(err, "validator in the unbonding queue was not found") } if !val.IsUnbonding() { - return fmt.Errorf("unexpected validator in unbonding queue; status was not unbonding") + return true, fmt.Errorf("unexpected validator in unbonding queue; status was not unbonding") } if val.UnbondingOnHoldRefCount == 0 { for _, id := range val.UnbondingIds { if err = k.DeleteUnbondingIndex(ctx, id); err != nil { - return err + return true, err } } val, err = k.UnbondingToUnbonded(ctx, val) if err != nil { - return err + return true, err } if val.GetDelegatorShares().IsZero() { str, err := k.validatorAddressCodec.StringToBytes(val.GetOperator()) if err != nil { - return err + return true, err } if err = k.RemoveValidator(ctx, str); err != nil { - return err + return true, err } } else { // remove unbonding ids @@ -605,11 +600,16 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { // remove validator from queue if err = k.DeleteValidatorQueue(ctx, val); err != nil { - return err + return true, err } } } } + // } + return false, nil + }) + if err != nil { + return err } return nil } diff --git a/x/staking/migrations/v2/store_test.go b/x/staking/migrations/v2/store_test.go index c7a933c43036..0e313c2f16f2 100644 --- a/x/staking/migrations/v2/store_test.go +++ b/x/staking/migrations/v2/store_test.go @@ -111,7 +111,7 @@ func TestStoreMigration(t *testing.T) { { "ValidatorQueueKey", v1.GetValidatorQueueKey(now, 4), - types.GetValidatorQueueKey(now, 4), + getValidatorQueueKey(now, 4), }, { "HistoricalInfoKey", @@ -148,3 +148,26 @@ func getValidatorKey(operatorAddr sdk.ValAddress) []byte { func unbondingKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { return append(append(types.UnbondingDelegationKey, sdkaddress.MustLengthPrefix(delAddr)...), sdkaddress.MustLengthPrefix(valAddr)...) } + +func getValidatorQueueKey(timestamp time.Time, height int64) []byte { + heightBz := sdk.Uint64ToBigEndian(uint64(height)) + timeBz := sdk.FormatTimeBytes(timestamp) + timeBzL := len(timeBz) + prefixL := len(types.ValidatorQueueKey) + + bz := make([]byte, prefixL+8+timeBzL+8) + + // copy the prefix + copy(bz[:prefixL], types.ValidatorQueueKey) + + // copy the encoded time bytes length + copy(bz[prefixL:prefixL+8], sdk.Uint64ToBigEndian(uint64(timeBzL))) + + // copy the encoded time bytes + copy(bz[prefixL+8:prefixL+8+timeBzL], timeBz) + + // copy the encoded height + copy(bz[prefixL+8+timeBzL:], heightBz) + + return bz +} diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 3e4c08427bd3..046ef86455fd 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -50,9 +50,9 @@ var ( UnbondingIndexKey = collections.NewPrefix(56) // prefix for an index for looking up unbonding operations by their IDs UnbondingTypeKey = collections.NewPrefix(57) // prefix for an index containing the type of unbonding operations - UnbondingQueueKey = []byte{0x41} // prefix for the timestamps in unbonding queue - RedelegationQueueKey = []byte{0x42} // prefix for the timestamps in redelegations queue - ValidatorQueueKey = []byte{0x43} // prefix for the timestamps in validator queue + UnbondingQueueKey = []byte{0x41} // prefix for the timestamps in unbonding queue + RedelegationQueueKey = []byte{0x42} // prefix for the timestamps in redelegations queue + ValidatorQueueKey = collections.NewPrefix(67) // prefix for the timestamps in validator queue HistoricalInfoKey = collections.NewPrefix(80) // prefix for the historical info ValidatorUpdatesKey = collections.NewPrefix(97) // prefix for the end block validator updates key @@ -146,31 +146,6 @@ func ParseValidatorPowerRankKey(key []byte) (operAddr []byte) { return operAddr } -// GetValidatorQueueKey returns the prefix key used for getting a set of unbonding -// validators whose unbonding completion occurs at the given time and height. -func GetValidatorQueueKey(timestamp time.Time, height int64) []byte { - heightBz := sdk.Uint64ToBigEndian(uint64(height)) - timeBz := sdk.FormatTimeBytes(timestamp) - timeBzL := len(timeBz) - prefixL := len(ValidatorQueueKey) - - bz := make([]byte, prefixL+8+timeBzL+8) - - // copy the prefix - copy(bz[:prefixL], ValidatorQueueKey) - - // copy the encoded time bytes length - copy(bz[prefixL:prefixL+8], sdk.Uint64ToBigEndian(uint64(timeBzL))) - - // copy the encoded time bytes - copy(bz[prefixL+8:prefixL+8+timeBzL], timeBz) - - // copy the encoded height - copy(bz[prefixL+8+timeBzL:], heightBz) - - return bz -} - // ParseValidatorQueueKey returns the encoded time and height from a key created // from GetValidatorQueueKey. func ParseValidatorQueueKey(bz []byte) (time.Time, int64, error) { diff --git a/x/staking/types/keys_test.go b/x/staking/types/keys_test.go index e0ae6f66259e..93d381c28f11 100644 --- a/x/staking/types/keys_test.go +++ b/x/staking/types/keys_test.go @@ -1,11 +1,9 @@ package types_test import ( - "bytes" "encoding/hex" "math/big" "testing" - "time" "github.com/stretchr/testify/require" @@ -47,29 +45,3 @@ func TestGetValidatorPowerRank(t *testing.T) { require.Equal(t, tt.wantHex, got, "Keys did not match on test case %d", i) } } - -func TestGetValidatorQueueKey(t *testing.T) { - ts := time.Now() - height := int64(1024) - - bz := types.GetValidatorQueueKey(ts, height) - rTs, rHeight, err := types.ParseValidatorQueueKey(bz) - require.NoError(t, err) - require.Equal(t, ts.UTC(), rTs.UTC()) - require.Equal(t, rHeight, height) -} - -func TestTestGetValidatorQueueKeyOrder(t *testing.T) { - ts := time.Now().UTC() - height := int64(1000) - - endKey := types.GetValidatorQueueKey(ts, height) - - keyA := types.GetValidatorQueueKey(ts.Add(-10*time.Minute), height-10) - keyB := types.GetValidatorQueueKey(ts.Add(-5*time.Minute), height+50) - keyC := types.GetValidatorQueueKey(ts.Add(10*time.Minute), height+100) - - require.Equal(t, -1, bytes.Compare(keyA, endKey)) // keyA <= endKey - require.Equal(t, -1, bytes.Compare(keyB, endKey)) // keyB <= endKey - require.Equal(t, 1, bytes.Compare(keyC, endKey)) // keyB >= endKey -} From f6af9fc863da2dd1a26324242de983056126c6d1 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 22:20:28 +0530 Subject: [PATCH 02/10] add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cfb9864cb50..88144925f1ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/staking) [#17562](https://github.com/cosmos/cosmos-sdk/pull/17562) Use collections for `ValidatorQueue` + * remove from `types`: `GetValidatorQueueKey` + * remove from `Keeper`: `ValidatorQueueIterator` * (x/staking) [#17123](https://github.com/cosmos/cosmos-sdk/pull/17123) Use collections for `Validators` * (x/staking) [#17270](https://github.com/cosmos/cosmos-sdk/pull/17270) Use collections for `UnbondingDelegation`: * remove from `types`: `GetUBDsKey` From 34838c6b69d7a0235a247f68128ac9f1f831e03f Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 29 Aug 2023 14:39:51 +0530 Subject: [PATCH 03/10] fix iterator and add diff test for migration --- x/staking/keeper/keeper.go | 4 +- x/staking/keeper/keeper_test.go | 65 ++++++++++++++++++++++++++++ x/staking/keeper/validator.go | 75 ++++++++++++--------------------- 3 files changed, 95 insertions(+), 49 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index ea5fec5903ac..3b56ae72907c 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -50,7 +50,7 @@ type Keeper struct { UnbondingDelegations collections.Map[collections.Pair[[]byte, []byte], types.UnbondingDelegation] RedelegationsByValDst collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] RedelegationsByValSrc collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] - ValidatorQueue collections.Map[collections.Pair[time.Time, int64], []byte] + ValidatorQueue collections.Map[collections.Pair[time.Time, int64], types.ValAddresses] } // NewKeeper creates a new staking Keeper instance @@ -168,7 +168,7 @@ func NewKeeper( sdk.TimeKey, collections.Int64Key, ), - collections.BytesValue, + codec.CollValue[types.ValAddresses](cdc), ), } diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index bba448548031..7eb2477da786 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -188,6 +188,33 @@ func getValidatorKey(operatorAddr sdk.ValAddress) []byte { return append(validatorsKey, addresstypes.MustLengthPrefix(operatorAddr)...) } +// getValidatorQueueKey returns the prefix key used for getting a set of unbonding +// validators whose unbonding completion occurs at the given time and height. +func getValidatorQueueKey(timestamp time.Time, height int64) []byte { + validatorQueueKey := []byte{0x43} + + heightBz := sdk.Uint64ToBigEndian(uint64(height)) + timeBz := sdk.FormatTimeBytes(timestamp) + timeBzL := len(timeBz) + prefixL := len(validatorQueueKey) + + bz := make([]byte, prefixL+8+timeBzL+8) + + // copy the prefix + copy(bz[:prefixL], validatorQueueKey) + + // copy the encoded time bytes length + copy(bz[prefixL:prefixL+8], sdk.Uint64ToBigEndian(uint64(timeBzL))) + + // copy the encoded time bytes + copy(bz[prefixL+8:prefixL+8+timeBzL], timeBz) + + // copy the encoded height + copy(bz[prefixL+8+timeBzL:], heightBz) + + return bz +} + func (s *KeeperTestSuite) TestSrcRedelegationsMigrationToColls() { s.SetupTest() @@ -396,6 +423,44 @@ func (s *KeeperTestSuite) TestValidatorsMigrationToColls() { s.Require().NoError(err) } +func (s *KeeperTestSuite) TestValidatorQueueMigrationToColls() { + s.SetupTest() + _, valAddrs := createValAddrs(100) + endTime := time.Unix(0, 0).UTC() + endHeight := int64(10) + err := testutil.DiffCollectionsMigration( + s.ctx, + s.key, + 100, + func(i int64) { + var addrs []string + addrs = append(addrs, valAddrs[i].String()) + bz, err := s.cdc.Marshal(&stakingtypes.ValAddresses{Addresses: addrs}) + s.Require().NoError(err) + + // legacy Set method + s.ctx.KVStore(s.key).Set(getValidatorQueueKey(endTime, endHeight), bz) + }, + "bd39c68022fad4bd2d89b076a44ea6829a96f2668590582672b821794dc655a1", + ) + s.Require().NoError(err) + + err = testutil.DiffCollectionsMigration( + s.ctx, + s.key, + 100, + func(i int64) { + var addrs []string + addrs = append(addrs, valAddrs[i].String()) + + err := s.stakingKeeper.SetUnbondingValidatorsQueue(s.ctx, endTime, endHeight, addrs) + s.Require().NoError(err) + }, + "bd39c68022fad4bd2d89b076a44ea6829a96f2668590582672b821794dc655a1", + ) + s.Require().NoError(err) +} + func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index a150bd16a00a..ad64927d3fd6 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -441,7 +441,7 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid // GetUnbondingValidators returns a slice of mature validator addresses that // complete their unbonding at a given time and height. func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, endHeight int64) ([]string, error) { - bz, err := k.ValidatorQueue.Get(ctx, collections.Join(endTime, endHeight)) + valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join(endTime, endHeight)) if err != nil { if !errors.Is(err, collections.ErrNotFound) { return nil, err @@ -449,26 +449,14 @@ func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, e return []string{}, nil } - if bz == nil { - return []string{}, nil - } - - addrs := types.ValAddresses{} - if err = k.cdc.Unmarshal(bz, &addrs); err != nil { - return nil, err - } - - return addrs.Addresses, nil + return valAddrs.Addresses, nil } // SetUnbondingValidatorsQueue sets a given slice of validator addresses into // the unbonding validator queue by a given height and time. func (k Keeper) SetUnbondingValidatorsQueue(ctx context.Context, endTime time.Time, endHeight int64, addrs []string) error { - bz, err := k.cdc.Marshal(&types.ValAddresses{Addresses: addrs}) - if err != nil { - return err - } - return k.ValidatorQueue.Set(ctx, collections.Join(endTime, endHeight), bz) + valAddrs := types.ValAddresses{Addresses: addrs} + return k.ValidatorQueue.Set(ctx, collections.Join(endTime, endHeight), valAddrs) } // InsertUnbondingValidatorQueue inserts a given unbonding validator address into @@ -534,64 +522,62 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { // ValidatorQueueKey | timeBzLen (8-byte big endian) | timeBz | heightBz (8-byte big endian), // so it may be possible that certain validator addresses that are iterated // over are not ready to unbond, so an explicit check is required. - // unbondingValIterator, err := k.ValidatorQueueIterator(ctx, blockTime, blockHeight) - // if err != nil { - // return err - // } - // defer unbondingValIterator.Close() - rng := collections.NewPrefixUntilPairRange[time.Time, int64](blockTime).EndInclusive(blockHeight) - err := k.ValidatorQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, int64], value []byte) (stop bool, err error) { - // for ; unbondingValIterator.Valid(); unbondingValIterator.Next() { - // key := unbondingValIterator.Key() - // keyTime, keyHeight, err := types.ParseValidatorQueueKey(key) - // if err != nil { - // return true, fmt.Errorf("failed to parse unbonding key: %w", err) - // } - keyTime := key.K1() - keyHeight := key.K2() + + unbondingValIterator, err := k.ValidatorQueue.Iterate(ctx, (&collections.Range[collections.Pair[time.Time, int64]]{}).EndInclusive(collections.Join(blockTime, blockHeight))) + if err != nil { + return err + } + defer unbondingValIterator.Close() + + for ; unbondingValIterator.Valid(); unbondingValIterator.Next() { + key, err := unbondingValIterator.Key() + if err != nil { + return err + } + keyTime, keyHeight := key.K1(), key.K2() // All addresses for the given key have the same unbonding height and time. // We only unbond if the height and time are less than the current height // and time. if keyHeight <= blockHeight && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) { - addrs := types.ValAddresses{} - if err = k.cdc.Unmarshal(value, &addrs); err != nil { - return true, err + addrs, err := unbondingValIterator.Value() + if err != nil { + return err } for _, valAddr := range addrs.Addresses { addr, err := k.validatorAddressCodec.StringToBytes(valAddr) if err != nil { - return true, err + return err } val, err := k.GetValidator(ctx, addr) if err != nil { - return true, errorsmod.Wrap(err, "validator in the unbonding queue was not found") + return errorsmod.Wrap(err, "validator in the unbonding queue was not found") } if !val.IsUnbonding() { - return true, fmt.Errorf("unexpected validator in unbonding queue; status was not unbonding") + return fmt.Errorf("unexpected validator in unbonding queue; status was not unbonding") } if val.UnbondingOnHoldRefCount == 0 { for _, id := range val.UnbondingIds { if err = k.DeleteUnbondingIndex(ctx, id); err != nil { - return true, err + return err } } val, err = k.UnbondingToUnbonded(ctx, val) if err != nil { - return true, err + return err } if val.GetDelegatorShares().IsZero() { str, err := k.validatorAddressCodec.StringToBytes(val.GetOperator()) if err != nil { - return true, err + return err } if err = k.RemoveValidator(ctx, str); err != nil { - return true, err + return err } } else { // remove unbonding ids @@ -600,16 +586,11 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { // remove validator from queue if err = k.DeleteValidatorQueue(ctx, val); err != nil { - return true, err + return err } } } } - // } - return false, nil - }) - if err != nil { - return err } return nil } From b2002773bd4249d158e48507d1882effad7ca674 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 29 Aug 2023 14:44:56 +0530 Subject: [PATCH 04/10] remove ParseValidatorQueueKey --- CHANGELOG.md | 2 +- x/staking/types/keys.go | 21 --------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7cd891ef44b..f6118fadac4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,7 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes * (x/staking) [#17562](https://github.com/cosmos/cosmos-sdk/pull/17562) Use collections for `ValidatorQueue` - * remove from `types`: `GetValidatorQueueKey` + * remove from `types`: `GetValidatorQueueKey`, `ParseValidatorQueueKey` * remove from `Keeper`: `ValidatorQueueIterator` * (x/staking) [#17481](https://github.com/cosmos/cosmos-sdk/pull/17481) Use collections for `UnbondingQueue`: * remove from `Keeper`: `UBDQueueIterator` diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index f1a95e812241..d892cf465b8f 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -1,9 +1,7 @@ package types import ( - "bytes" "encoding/binary" - "fmt" "time" "cosmossdk.io/collections" @@ -146,25 +144,6 @@ func ParseValidatorPowerRankKey(key []byte) (operAddr []byte) { return operAddr } -// ParseValidatorQueueKey returns the encoded time and height from a key created -// from GetValidatorQueueKey. -func ParseValidatorQueueKey(bz []byte) (time.Time, int64, error) { - prefixL := len(ValidatorQueueKey) - if prefix := bz[:prefixL]; !bytes.Equal(prefix, ValidatorQueueKey) { - return time.Time{}, 0, fmt.Errorf("invalid prefix; expected: %X, got: %X", ValidatorQueueKey, prefix) - } - - timeBzL := sdk.BigEndianToUint64(bz[prefixL : prefixL+8]) - ts, err := sdk.ParseTimeBytes(bz[prefixL+8 : prefixL+8+int(timeBzL)]) - if err != nil { - return time.Time{}, 0, err - } - - height := sdk.BigEndianToUint64(bz[prefixL+8+int(timeBzL):]) - - return ts, int64(height), nil -} - // GetUBDKey creates the key for an unbonding delegation by delegator and validator addr // VALUE: staking/UnbondingDelegation func GetUBDKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { From df6d113a84de917db9b48ead953b3b5961b4b656 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 30 Aug 2023 09:27:07 +0530 Subject: [PATCH 05/10] use right hash in diff test --- x/staking/keeper/keeper_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index 7eb2477da786..e88f38e8d8a1 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -441,7 +441,7 @@ func (s *KeeperTestSuite) TestValidatorQueueMigrationToColls() { // legacy Set method s.ctx.KVStore(s.key).Set(getValidatorQueueKey(endTime, endHeight), bz) }, - "bd39c68022fad4bd2d89b076a44ea6829a96f2668590582672b821794dc655a1", + "8cf5fb4def683e83ea4cc4f14d8b2abc4c6af66709ad8af391dc749e63ef7524", ) s.Require().NoError(err) @@ -456,7 +456,7 @@ func (s *KeeperTestSuite) TestValidatorQueueMigrationToColls() { err := s.stakingKeeper.SetUnbondingValidatorsQueue(s.ctx, endTime, endHeight, addrs) s.Require().NoError(err) }, - "bd39c68022fad4bd2d89b076a44ea6829a96f2668590582672b821794dc655a1", + "8cf5fb4def683e83ea4cc4f14d8b2abc4c6af66709ad8af391dc749e63ef7524", ) s.Require().NoError(err) } From e0378064a78fb99db2f16e006318de39bfb28a09 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 1 Sep 2023 10:51:35 +0530 Subject: [PATCH 06/10] fix keeper test --- x/staking/keeper/keeper.go | 8 +++++--- x/staking/keeper/keeper_test.go | 12 ++++++------ x/staking/keeper/validator.go | 20 ++++++++++++++------ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index d29c9db29fe2..e19581800225 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -51,7 +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] - ValidatorQueue collections.Map[collections.Pair[time.Time, int64], types.ValAddresses] + ValidatorQueue collections.Map[collections.Triple[uint64, time.Time, uint64], types.ValAddresses] LastValidatorPower collections.Map[[]byte, []byte] } @@ -170,12 +170,14 @@ func NewKeeper( ), codec.CollValue[types.UnbondingDelegation](cdc), ), + // key format is: 67 | length(timestamp Bytes) | timestamp | height ValidatorQueue: collections.NewMap( sb, types.ValidatorQueueKey, "validator_queue", - collections.PairKeyCodec( + collections.TripleKeyCodec( + collections.Uint64Key, sdk.TimeKey, - collections.Int64Key, + collections.Uint64Key, ), codec.CollValue[types.ValAddresses](cdc), ), diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index 06757f0ac819..9933be4e12a0 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -196,6 +196,12 @@ 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)...) +} + // getValidatorQueueKey returns the prefix key used for getting a set of unbonding // validators whose unbonding completion occurs at the given time and height. func getValidatorQueueKey(timestamp time.Time, height int64) []byte { @@ -223,12 +229,6 @@ func getValidatorQueueKey(timestamp time.Time, height int64) []byte { return bz } -// 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() diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 436bf6f68d3f..0ad4f2fd4782 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -426,7 +426,9 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid // GetUnbondingValidators returns a slice of mature validator addresses that // complete their unbonding at a given time and height. func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, endHeight int64) ([]string, error) { - valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join(endTime, endHeight)) + timeBz := sdk.FormatTimeBytes(endTime) + timeBzL := len(timeBz) + valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight))) if err != nil { if !errors.Is(err, collections.ErrNotFound) { return nil, err @@ -441,7 +443,9 @@ func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, e // the unbonding validator queue by a given height and time. func (k Keeper) SetUnbondingValidatorsQueue(ctx context.Context, endTime time.Time, endHeight int64, addrs []string) error { valAddrs := types.ValAddresses{Addresses: addrs} - return k.ValidatorQueue.Set(ctx, collections.Join(endTime, endHeight), valAddrs) + timeBz := sdk.FormatTimeBytes(endTime) + timeBzL := len(timeBz) + return k.ValidatorQueue.Set(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight)), valAddrs) } // InsertUnbondingValidatorQueue inserts a given unbonding validator address into @@ -458,7 +462,9 @@ func (k Keeper) InsertUnbondingValidatorQueue(ctx context.Context, val types.Val // DeleteValidatorQueueTimeSlice deletes all entries in the queue indexed by a // given height and time. func (k Keeper) DeleteValidatorQueueTimeSlice(ctx context.Context, endTime time.Time, endHeight int64) error { - return k.ValidatorQueue.Remove(ctx, collections.Join(endTime, endHeight)) + timeBz := sdk.FormatTimeBytes(endTime) + timeBzL := len(timeBz) + return k.ValidatorQueue.Remove(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight))) } // DeleteValidatorQueue removes a validator by address from the unbonding queue @@ -508,7 +514,9 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { // so it may be possible that certain validator addresses that are iterated // over are not ready to unbond, so an explicit check is required. - unbondingValIterator, err := k.ValidatorQueue.Iterate(ctx, (&collections.Range[collections.Pair[time.Time, int64]]{}).EndInclusive(collections.Join(blockTime, blockHeight))) + timeBz := sdk.FormatTimeBytes(blockTime) + timeBzL := len(timeBz) + unbondingValIterator, err := k.ValidatorQueue.Iterate(ctx, (&collections.Range[collections.Triple[uint64, time.Time, uint64]]{}).EndInclusive(collections.Join3(uint64(timeBzL), blockTime, uint64(blockHeight)))) if err != nil { return err } @@ -519,12 +527,12 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { if err != nil { return err } - keyTime, keyHeight := key.K1(), key.K2() + _, keyTime, keyHeight := key.K1(), key.K2(), key.K3() // All addresses for the given key have the same unbonding height and time. // We only unbond if the height and time are less than the current height // and time. - if keyHeight <= blockHeight && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) { + if keyHeight <= uint64(blockHeight) && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) { addrs, err := unbondingValIterator.Value() if err != nil { return err From 03c0359e2ee9421ebc09de3bb2cda619ed3a3572 Mon Sep 17 00:00:00 2001 From: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com> Date: Mon, 4 Sep 2023 14:48:22 +0000 Subject: [PATCH 07/10] add a note on using 3 keys --- x/staking/keeper/keeper.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 8ecc2add2cd4..1f62d37f79a8 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -174,6 +174,7 @@ func NewKeeper( codec.CollValue[types.UnbondingDelegation](cdc), ), // key format is: 67 | length(timestamp Bytes) | timestamp | height + // Note: We use 3 keys here because we prefixed time bytes with its length previously and to retain state compatibility we remain to use the same ValidatorQueue: collections.NewMap( sb, types.ValidatorQueueKey, "validator_queue", From 5848e81a79cebfce4eedcfbc3959a1908d2032b6 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 5 Sep 2023 14:38:44 +0530 Subject: [PATCH 08/10] remove FormatTimeBytes usage and apply frojdi's suggestion --- x/staking/keeper/validator.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index f8a0feffe934..fb01def82908 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -403,9 +403,8 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid // GetUnbondingValidators returns a slice of mature validator addresses that // complete their unbonding at a given time and height. func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, endHeight int64) ([]string, error) { - timeBz := sdk.FormatTimeBytes(endTime) - timeBzL := len(timeBz) - valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight))) + timeSize := sdk.TimeKey.Size(endTime) + valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join3(uint64(timeSize), endTime, uint64(endHeight))) if err != nil { if !errors.Is(err, collections.ErrNotFound) { return nil, err From 26e2976847cb188964484154d02d8601f931c23b Mon Sep 17 00:00:00 2001 From: testinginprod <98415576+testinginprod@users.noreply.github.com> Date: Fri, 8 Sep 2023 14:23:16 +0200 Subject: [PATCH 09/10] refactor(staking): cleanup UnbondAllMatureValidators (#17664) Co-authored-by: unknown unknown --- x/staking/keeper/validator.go | 127 +++++++++++++++++----------------- 1 file changed, 62 insertions(+), 65 deletions(-) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index fb01def82908..4191cdfc11d2 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -482,83 +482,80 @@ func (k Keeper) DeleteValidatorQueue(ctx context.Context, val types.Validator) e func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { sdkCtx := sdk.UnwrapSDKContext(ctx) blockTime := sdkCtx.BlockTime() - blockHeight := sdkCtx.BlockHeight() + blockHeight := uint64(sdkCtx.BlockHeight()) - // unbondingValIterator will contains all validator addresses indexed under - // the ValidatorQueueKey prefix. Note, the entire index key is composed as - // ValidatorQueueKey | timeBzLen (8-byte big endian) | timeBz | heightBz (8-byte big endian), - // so it may be possible that certain validator addresses that are iterated - // over are not ready to unbond, so an explicit check is required. + rng := new(collections.Range[collections.Triple[uint64, time.Time, uint64]]). + EndInclusive(collections.Join3(uint64(29), blockTime, blockHeight)) - timeBz := sdk.FormatTimeBytes(blockTime) - timeBzL := len(timeBz) - unbondingValIterator, err := k.ValidatorQueue.Iterate(ctx, (&collections.Range[collections.Triple[uint64, time.Time, uint64]]{}).EndInclusive(collections.Join3(uint64(timeBzL), blockTime, uint64(blockHeight)))) - if err != nil { - return err + return k.ValidatorQueue.Walk(ctx, rng, func(key collections.Triple[uint64, time.Time, uint64], value types.ValAddresses) (stop bool, err error) { + return false, k.unbondMatureValidators(ctx, blockHeight, blockTime, key, value) + }) +} + +func (k Keeper) unbondMatureValidators( + ctx context.Context, + blockHeight uint64, + blockTime time.Time, + key collections.Triple[uint64, time.Time, uint64], + addrs types.ValAddresses, +) error { + keyTime, keyHeight := key.K2(), key.K3() + + // All addresses for the given key have the same unbonding height and time. + // We only unbond if the height and time are less than the current height + // and time. + if keyHeight > blockHeight || keyTime.After(blockTime) { + return nil } - defer unbondingValIterator.Close() - for ; unbondingValIterator.Valid(); unbondingValIterator.Next() { - key, err := unbondingValIterator.Key() + // finalize unbonding + for _, valAddr := range addrs.Addresses { + addr, err := k.validatorAddressCodec.StringToBytes(valAddr) if err != nil { return err } - _, keyTime, keyHeight := key.K1(), key.K2(), key.K3() + val, err := k.GetValidator(ctx, addr) + if err != nil { + return errorsmod.Wrap(err, "validator in the unbonding queue was not found") + } - // All addresses for the given key have the same unbonding height and time. - // We only unbond if the height and time are less than the current height - // and time. - if keyHeight <= uint64(blockHeight) && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) { - addrs, err := unbondingValIterator.Value() - if err != nil { + if !val.IsUnbonding() { + return fmt.Errorf("unexpected validator in unbonding queue; status was not unbonding") + } + + // if the ref count is not zero, early exit. + if val.UnbondingOnHoldRefCount != 0 { + return nil + } + + // otherwise do proper unbonding + for _, id := range val.UnbondingIds { + if err = k.DeleteUnbondingIndex(ctx, id); err != nil { return err } + } - for _, valAddr := range addrs.Addresses { - addr, err := k.validatorAddressCodec.StringToBytes(valAddr) - if err != nil { - return err - } - val, err := k.GetValidator(ctx, addr) - if err != nil { - return errorsmod.Wrap(err, "validator in the unbonding queue was not found") - } - - if !val.IsUnbonding() { - return fmt.Errorf("unexpected validator in unbonding queue; status was not unbonding") - } - - if val.UnbondingOnHoldRefCount == 0 { - for _, id := range val.UnbondingIds { - if err = k.DeleteUnbondingIndex(ctx, id); err != nil { - return err - } - } - - val, err = k.UnbondingToUnbonded(ctx, val) - if err != nil { - return err - } - - if val.GetDelegatorShares().IsZero() { - str, err := k.validatorAddressCodec.StringToBytes(val.GetOperator()) - if err != nil { - return err - } - if err = k.RemoveValidator(ctx, str); err != nil { - return err - } - } else { - // remove unbonding ids - val.UnbondingIds = []uint64{} - } - - // remove validator from queue - if err = k.DeleteValidatorQueue(ctx, val); err != nil { - return err - } - } + val, err = k.UnbondingToUnbonded(ctx, val) + if err != nil { + return err + } + + if val.GetDelegatorShares().IsZero() { + str, err := k.validatorAddressCodec.StringToBytes(val.GetOperator()) + if err != nil { + return err + } + if err = k.RemoveValidator(ctx, str); err != nil { + return err } + } else { + // remove unbonding ids + val.UnbondingIds = []uint64{} + } + + // remove validator from queue + if err = k.DeleteValidatorQueue(ctx, val); err != nil { + return err } } return nil From d9641a04299eeac57b8ea5ca4021b36ea58171d2 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 8 Sep 2023 18:05:42 +0530 Subject: [PATCH 10/10] address nits --- x/staking/keeper/validator.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index fb01def82908..172021fd3ef1 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -20,6 +20,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking/types" ) +var timeBzKeySize = uint64(29) // time bytes key size is 29 by default + // GetValidator gets a single validator func (k Keeper) GetValidator(ctx context.Context, addr sdk.ValAddress) (validator types.Validator, err error) { validator, err = k.Validators.Get(ctx, addr) @@ -405,11 +407,8 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, endHeight int64) ([]string, error) { timeSize := sdk.TimeKey.Size(endTime) valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join3(uint64(timeSize), endTime, uint64(endHeight))) - if err != nil { - if !errors.Is(err, collections.ErrNotFound) { - return nil, err - } - return []string{}, nil + if err != nil && !errors.Is(err, collections.ErrNotFound) { + return []string{}, err } return valAddrs.Addresses, nil @@ -419,9 +418,7 @@ func (k Keeper) GetUnbondingValidators(ctx context.Context, endTime time.Time, e // the unbonding validator queue by a given height and time. func (k Keeper) SetUnbondingValidatorsQueue(ctx context.Context, endTime time.Time, endHeight int64, addrs []string) error { valAddrs := types.ValAddresses{Addresses: addrs} - timeBz := sdk.FormatTimeBytes(endTime) - timeBzL := len(timeBz) - return k.ValidatorQueue.Set(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight)), valAddrs) + return k.ValidatorQueue.Set(ctx, collections.Join3(timeBzKeySize, endTime, uint64(endHeight)), valAddrs) } // InsertUnbondingValidatorQueue inserts a given unbonding validator address into @@ -438,9 +435,7 @@ func (k Keeper) InsertUnbondingValidatorQueue(ctx context.Context, val types.Val // DeleteValidatorQueueTimeSlice deletes all entries in the queue indexed by a // given height and time. func (k Keeper) DeleteValidatorQueueTimeSlice(ctx context.Context, endTime time.Time, endHeight int64) error { - timeBz := sdk.FormatTimeBytes(endTime) - timeBzL := len(timeBz) - return k.ValidatorQueue.Remove(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight))) + return k.ValidatorQueue.Remove(ctx, collections.Join3(timeBzKeySize, endTime, uint64(endHeight))) } // DeleteValidatorQueue removes a validator by address from the unbonding queue @@ -490,9 +485,7 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error { // so it may be possible that certain validator addresses that are iterated // over are not ready to unbond, so an explicit check is required. - timeBz := sdk.FormatTimeBytes(blockTime) - timeBzL := len(timeBz) - unbondingValIterator, err := k.ValidatorQueue.Iterate(ctx, (&collections.Range[collections.Triple[uint64, time.Time, uint64]]{}).EndInclusive(collections.Join3(uint64(timeBzL), blockTime, uint64(blockHeight)))) + unbondingValIterator, err := k.ValidatorQueue.Iterate(ctx, (&collections.Range[collections.Triple[uint64, time.Time, uint64]]{}).EndInclusive(collections.Join3(timeBzKeySize, blockTime, uint64(blockHeight)))) if err != nil { return err }