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 ValidatorQueue to use Collections #17562

Merged
merged 26 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
bc2da6f
wip: migrate ValidatorQueue to collections
likhita-809 Aug 28, 2023
51f9286
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 28, 2023
f6af9fc
add changelog
likhita-809 Aug 28, 2023
121c27c
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
34838c6
fix iterator and add diff test for migration
likhita-809 Aug 29, 2023
3afd3f1
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
b200277
remove ParseValidatorQueueKey
likhita-809 Aug 29, 2023
c346236
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
63db269
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 29, 2023
df6d113
use right hash in diff test
likhita-809 Aug 30, 2023
eff5ed6
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 30, 2023
a7cda07
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 30, 2023
d9987a2
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Aug 31, 2023
e037806
fix keeper test
likhita-809 Sep 1, 2023
d138a0a
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 1, 2023
18ad606
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 1, 2023
3e044c3
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 1, 2023
3f672aa
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 2, 2023
03c0359
add a note on using 3 keys
likhita-809 Sep 4, 2023
6e5a7cf
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 5, 2023
5848e81
remove FormatTimeBytes usage and apply frojdi's suggestion
likhita-809 Sep 5, 2023
3b7e386
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 5, 2023
26e2976
refactor(staking): cleanup UnbondAllMatureValidators (#17664)
testinginprod Sep 8, 2023
d9641a0
address nits
likhita-809 Sep 8, 2023
00af397
Merge branch 'likhita/valQueue' of https://github.com/cosmos/cosmos-s…
likhita-809 Sep 8, 2023
ff0261e
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 Sep 8, 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) [#17562](https://github.com/cosmos/cosmos-sdk/pull/17562) Use collections for `ValidatorQueue`
* remove from `types`: `GetValidatorQueueKey`, `ParseValidatorQueueKey`
* remove from `Keeper`: `ValidatorQueueIterator`
* (x/staking) [#17498](https://github.com/cosmos/cosmos-sdk/pull/17498) Use collections for `LastValidatorPower`:
* remove from `types`: `GetLastValidatorPowerKey`
* remove from `Keeper`: `LastValidatorsIterator`, `IterateLastValidators`
Expand Down
15 changes: 14 additions & 1 deletion x/staking/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,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.Triple[uint64, time.Time, uint64], types.ValAddresses]
LastValidatorPower collections.Map[[]byte, gogotypes.Int64Value]
}

Expand Down Expand Up @@ -170,7 +171,19 @@ func NewKeeper(
collections.BytesKey,
sdk.LengthPrefixedBytesKey, // sdk.LengthPrefixedBytesKey is needed to retain state compatibility
),
codec.CollValue[types.UnbondingDelegation](cdc)),
codec.CollValue[types.UnbondingDelegation](cdc),
),
// key format is: 67 | length(timestamp Bytes) | timestamp | height
ValidatorQueue: collections.NewMap(
sb, types.ValidatorQueueKey,
"validator_queue",
collections.TripleKeyCodec(
collections.Uint64Key,
sdk.TimeKey,
collections.Uint64Key,
),
codec.CollValue[types.ValAddresses](cdc),
),
}

schema, err := sb.Build()
Expand Down
65 changes: 65 additions & 0 deletions x/staking/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,33 @@ func getLastValidatorPowerKey(operator sdk.ValAddress) []byte {
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 {
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) TestLastTotalPowerMigrationToColls() {
s.SetupTest()

Expand Down Expand Up @@ -445,6 +472,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)
},
"8cf5fb4def683e83ea4cc4f14d8b2abc4c6af66709ad8af391dc749e63ef7524",
)
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)
},
"8cf5fb4def683e83ea4cc4f14d8b2abc4c6af66709ad8af391dc749e63ef7524",
)
s.Require().NoError(err)
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(KeeperTestSuite))
}
59 changes: 24 additions & 35 deletions x/staking/keeper/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,34 +403,26 @@ 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))
timeBz := sdk.FormatTimeBytes(endTime)
timeBzL := len(timeBz)
Copy link
Contributor

Choose a reason for hiding this comment

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

really unfortunate that we prefix time bytes with their length even when the size is constant and well-known. =(

Copy link
Contributor

Choose a reason for hiding this comment

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

you can use here if you want k.ValidatorsQueue.KeyCodec().Size(endTime), instead of using sdk.FormatTimeBytes... At least this way we don't need to rely on format time bytes and we keep it in between collections. I would even comment this in the Keeper why we use three keys instead of using simply two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we can use k.ValidatorQueue.KeyCodec().Size(endTime) because key here should be of format collections.Triple[uint64, time.Time, uint64], not just time(endTime). And it gives size of the combined length of fields in key.

Copy link
Member

Choose a reason for hiding this comment

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

I mean, it's a good workaround, unfortunate that we had to do it but it works lol. Once we finish up with all these migrations we'll most definitely do a lot of refactor so we can do proper migrations then

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, then @likhita-809 maybe we can use sdk.TimeKey.Size directly, sorry for being pedantic about this, but I'd rather we remove every form of bytes conversion in state from production code, testing is fine to have it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although, time key size is constant so in theory we could use the same number everywhere: https://github.com/cosmos/cosmos-sdk/blob/main/types/collections.go#L210

valAddrs, err := k.ValidatorQueue.Get(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight)))
if err != nil {
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
return nil, err
}

if bz == nil {
if !errors.Is(err, collections.ErrNotFound) {
return nil, err
}
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 {
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)
valAddrs := types.ValAddresses{Addresses: addrs}
timeBz := sdk.FormatTimeBytes(endTime)
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
timeBzL := len(timeBz)
return k.ValidatorQueue.Set(ctx, collections.Join3(uint64(timeBzL), endTime, uint64(endHeight)), valAddrs)
}

// InsertUnbondingValidatorQueue inserts a given unbonding validator address into
Expand All @@ -447,8 +439,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 {
store := k.storeService.OpenKVStore(ctx)
return store.Delete(types.GetValidatorQueueKey(endTime, endHeight))
timeBz := sdk.FormatTimeBytes(endTime)
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -485,13 +478,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 {
Expand All @@ -504,25 +490,28 @@ 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)

timeBz := sdk.FormatTimeBytes(blockTime)
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
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
}
defer unbondingValIterator.Close()

for ; unbondingValIterator.Valid(); unbondingValIterator.Next() {
key := unbondingValIterator.Key()
keyTime, keyHeight, err := types.ParseValidatorQueueKey(key)
key, err := unbondingValIterator.Key()
if err != nil {
return fmt.Errorf("failed to parse unbonding key: %w", err)
return err
}
_, 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)) {
addrs := types.ValAddresses{}
if err = k.cdc.Unmarshal(unbondingValIterator.Value(), &addrs); err != nil {
if keyHeight <= uint64(blockHeight) && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) {
addrs, err := unbondingValIterator.Value()
if err != nil {
return err
}

Expand Down
25 changes: 24 additions & 1 deletion x/staking/migrations/v2/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestStoreMigration(t *testing.T) {
{
"ValidatorQueueKey",
v1.GetValidatorQueueKey(now, 4),
types.GetValidatorQueueKey(now, 4),
getValidatorQueueKey(now, 4),
},
{
"HistoricalInfoKey",
Expand Down Expand Up @@ -161,3 +161,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
}
48 changes: 1 addition & 47 deletions x/staking/types/keys.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package types

import (
"bytes"
"encoding/binary"
"fmt"
"time"

"cosmossdk.io/collections"
Expand Down Expand Up @@ -53,7 +51,7 @@ var (

UnbondingQueueKey = collections.NewPrefix(65) // 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
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
Expand Down Expand Up @@ -142,50 +140,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) {
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 {
Expand Down
28 changes: 0 additions & 28 deletions x/staking/types/keys_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package types_test

import (
"bytes"
"encoding/hex"
"math/big"
"testing"
"time"

"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -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
}