From 498e4d1d2c72d2ccc73f75afb965c3d3bfa8ba7d Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 3 Aug 2023 10:37:53 +0530 Subject: [PATCH 01/23] migrate DelegationKey to collections --- x/staking/keeper/delegation.go | 73 ++++++++++----------------- x/staking/keeper/grpc_query.go | 24 ++++----- x/staking/keeper/keeper.go | 2 + x/staking/keeper/query_utils.go | 27 ++++------ x/staking/migrations/v2/store_test.go | 7 ++- x/staking/types/keys.go | 14 ++--- 6 files changed, 66 insertions(+), 81 deletions(-) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index f7761c9f9189..f5f75e3cacbe 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -173,23 +173,22 @@ func (k Keeper) RemoveDelegation(ctx context.Context, delegation types.Delegatio func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAddress, maxRetrieve uint16) (unbondingDelegations []types.UnbondingDelegation, err error) { unbondingDelegations = make([]types.UnbondingDelegation, maxRetrieve) - store := k.storeService.OpenKVStore(ctx) - delegatorPrefixKey := types.GetUBDsKey(delegator) - - iterator, err := store.Iterator(delegatorPrefixKey, storetypes.PrefixEndBytes(delegatorPrefixKey)) - if err != nil { - return unbondingDelegations, err - } - defer iterator.Close() - i := 0 - for ; iterator.Valid() && i < int(maxRetrieve); iterator.Next() { - unbondingDelegation, err := types.UnmarshalUBD(k.cdc, iterator.Value()) - if err != nil { - return unbondingDelegations, err - } - unbondingDelegations[i] = unbondingDelegation - i++ + rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + err = k.UnbondingDelegation.Walk( + ctx, + rng, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + i++ + unbondingDelegations = append(unbondingDelegations, value) + if i >= int(maxRetrieve) { + return false, err + } + return false, err + }, + ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return nil, err } return unbondingDelegations[:i], nil // trim if the array length < maxRetrieve @@ -265,36 +264,20 @@ func (k Keeper) IterateUnbondingDelegations(ctx context.Context, fn func(index i // GetDelegatorUnbonding returns the total amount a delegator has unbonding. func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddress) (math.Int, error) { unbonding := math.ZeroInt() - err := k.IterateDelegatorUnbondingDelegations(ctx, delegator, func(ubd types.UnbondingDelegation) bool { - for _, entry := range ubd.Entries { - unbonding = unbonding.Add(entry.Balance) - } - return false - }) - return unbonding, err -} - -// IterateDelegatorUnbondingDelegations iterates through a delegator's unbonding delegations. -func (k Keeper) IterateDelegatorUnbondingDelegations(ctx context.Context, delegator sdk.AccAddress, cb func(ubd types.UnbondingDelegation) (stop bool)) error { - store := k.storeService.OpenKVStore(ctx) - prefix := types.GetUBDsKey(delegator) - iterator, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) - if err != nil { - return err - } - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - ubd, err := types.UnmarshalUBD(k.cdc, iterator.Value()) - if err != nil { - return err - } - if cb(ubd) { - break - } + err := k.UnbondingDelegation.Walk( + ctx, + nil, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + for _, entry := range ubd.Entries { + unbonding = unbonding.Add(entry.Balance) + } + return false, nil + }, + ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return unbonding, err } - - return nil + return unbonding, err } // GetDelegatorBonded returs the total amount a delegator has bonded. diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index 39f628dee83a..245821652199 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -359,23 +359,23 @@ func (k Querier) DelegatorUnbondingDelegations(ctx context.Context, req *types.Q } var unbondingDelegations types.UnbondingDelegations - delAddr, err := k.authKeeper.AddressCodec().StringToBytes(req.DelegatorAddr) + delAddr, err := sdk.AccAddressFromBech32(req.DelegatorAddr) if err != nil { return nil, err } - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - unbStore := prefix.NewStore(store, types.GetUBDsKey(delAddr)) - pageRes, err := query.Paginate(unbStore, req.Pagination, func(key, value []byte) error { - unbond, err := types.UnmarshalUBD(k.cdc, value) - if err != nil { - return err - } - unbondingDelegations = append(unbondingDelegations, unbond) - return nil - }) + unbondingDelegations, pageRes, err := query.CollectionPaginate( + ctx, + k.Keeper.UnbondingDelegation, + req.Pagination, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (ubd types.UnbondingDelegation, err error) { + unbondingDelegations = append(unbondingDelegations, value) + return value, err + }, + query.WithCollectionPaginationPairPrefix[sdk.AccAddress, sdk.ValAddress](delAddr), + ) if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + return nil, err } return &types.QueryDelegatorUnbondingDelegationsResponse{ diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 64ad687d1f6f..d0f64aa0d6a4 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -37,6 +37,7 @@ type Keeper struct { LastTotalPower collections.Item[math.Int] ValidatorUpdates collections.Item[types.ValidatorUpdates] DelegationsByValidator collections.Map[collections.Pair[sdk.ValAddress, sdk.AccAddress], []byte] + UnbondingDelegation collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], types.UnbondingDelegation] } // NewKeeper creates a new staking Keeper instance @@ -86,6 +87,7 @@ func NewKeeper( collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), sdk.AccAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility collections.BytesValue, ), + UnbondingDelegation: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.ValAddressKey), codec.CollValue[types.UnbondingDelegation](cdc)), } schema, err := sb.Build() diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index 6bb16da3fcd0..4888e7b854f2 100644 --- a/x/staking/keeper/query_utils.go +++ b/x/staking/keeper/query_utils.go @@ -2,7 +2,9 @@ package keeper import ( "context" + "errors" + "cosmossdk.io/collections" storetypes "cosmossdk.io/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -90,24 +92,17 @@ func (k Keeper) GetAllDelegatorDelegations(ctx context.Context, delegator sdk.Ac func (k Keeper) GetAllUnbondingDelegations(ctx context.Context, delegator sdk.AccAddress) ([]types.UnbondingDelegation, error) { unbondingDelegations := make([]types.UnbondingDelegation, 0) - store := k.storeService.OpenKVStore(ctx) - delegatorPrefixKey := types.GetUBDsKey(delegator) - - iterator, err := store.Iterator(delegatorPrefixKey, storetypes.PrefixEndBytes(delegatorPrefixKey)) // smallest to largest - if err != nil { + err := k.UnbondingDelegation.Walk( + ctx, + collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator), + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + unbondingDelegations = append(unbondingDelegations, value) + return false, err + }, + ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { return nil, err } - defer iterator.Close() - - for i := 0; iterator.Valid(); iterator.Next() { - unbondingDelegation, err := types.UnmarshalUBD(k.cdc, iterator.Value()) - if err != nil { - return nil, err - } - unbondingDelegations = append(unbondingDelegations, unbondingDelegation) - i++ - } - return unbondingDelegations, nil } diff --git a/x/staking/migrations/v2/store_test.go b/x/staking/migrations/v2/store_test.go index 20c55e5bfcbb..6c7d86399edd 100644 --- a/x/staking/migrations/v2/store_test.go +++ b/x/staking/migrations/v2/store_test.go @@ -12,6 +12,7 @@ import ( sdktestuil "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + "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" @@ -74,7 +75,7 @@ func TestStoreMigration(t *testing.T) { { "UnbondingDelegationKey", v1.GetUBDKey(addr4, valAddr1), - types.GetUBDKey(addr4, valAddr1), + unbondingKey(addr4, valAddr1), }, { "UnbondingDelegationByValIndexKey", @@ -138,3 +139,7 @@ func TestStoreMigration(t *testing.T) { }) } } + +func unbondingKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { + return append(append(types.UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), address.MustLengthPrefix(valAddr)...) +} diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 067073c1bdff..1570e2328117 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -38,12 +38,12 @@ var ( ValidatorsByConsAddrKey = []byte{0x22} // prefix for each key to a validator index, by pubkey ValidatorsByPowerIndexKey = []byte{0x23} // prefix for each key to a validator index, sorted by power - DelegationKey = []byte{0x31} // key for a delegation - UnbondingDelegationKey = []byte{0x32} // key for an unbonding-delegation - UnbondingDelegationByValIndexKey = []byte{0x33} // prefix for each key for an unbonding-delegation, by validator operator - RedelegationKey = []byte{0x34} // key for a redelegation - RedelegationByValSrcIndexKey = []byte{0x35} // prefix for each key for an redelegation, by source validator operator - RedelegationByValDstIndexKey = []byte{0x36} // prefix for each key for an redelegation, by destination validator operator + DelegationKey = []byte{0x31} // key for a delegation + UnbondingDelegationKey = collections.NewPrefix(50) // key for an unbonding-delegation + UnbondingDelegationByValIndexKey = []byte{0x33} // prefix for each key for an unbonding-delegation, by validator operator + RedelegationKey = []byte{0x34} // key for a redelegation + RedelegationByValSrcIndexKey = []byte{0x35} // prefix for each key for an redelegation, by source validator operator + RedelegationByValDstIndexKey = []byte{0x36} // prefix for each key for an redelegation, by destination validator operator UnbondingIDKey = []byte{0x37} // key for the counter for the incrementing id for UnbondingOperations UnbondingIndexKey = []byte{0x38} // prefix for an index for looking up unbonding operations by their IDs @@ -223,7 +223,7 @@ func GetDelegationsKey(delAddr sdk.AccAddress) []byte { // 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 { - return append(GetUBDsKey(delAddr.Bytes()), address.MustLengthPrefix(valAddr)...) + return append(append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), valAddr...) } // GetUBDByValIndexKey creates the index-key for an unbonding delegation, stored by validator-index From 4baf88e0ecee63d90620938dd7ef8f8ddbb38ff5 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 3 Aug 2023 10:49:25 +0530 Subject: [PATCH 02/23] remove GetUBDsKey --- x/staking/types/keys.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 1570e2328117..cdfbd8e42681 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -243,12 +243,7 @@ func GetUBDKeyFromValIndexKey(indexKey []byte) []byte { kv.AssertKeyAtLeastLength(addrs, 3+int(valAddrLen)) delAddr := addrs[valAddrLen+2:] - return GetUBDKey(delAddr, valAddr) -} - -// GetUBDsKey creates the prefix for all unbonding delegations from a delegator -func GetUBDsKey(delAddr sdk.AccAddress) []byte { - return append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...) + return append(append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), valAddr...) } // GetUBDsByValIndexKey creates the prefix keyspace for the indexes of unbonding delegations for a validator From 7b4a54f25ac196f1f8dcc52390285c035079fc67 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 3 Aug 2023 10:51:56 +0530 Subject: [PATCH 03/23] add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9cd618671e8..66968db54f8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/staking) [#17270](https://github.com/cosmos/cosmos-sdk/pull/17270) Use collections for `UnbondingDelegation`: + * remove from `types`: `GetUBDsKey` * (x/distribution) [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Use collections for `PreviousProposer` and `ValidatorSlashEvents`: * remove from `Keeper`: `GetPreviousProposerConsAddr`, `SetPreviousProposerConsAddr`, `GetValidatorHistoricalReferenceCount`, `GetValidatorSlashEvent`, `SetValidatorSlashEvent`. * (x/slashing) [17063](https://github.com/cosmos/cosmos-sdk/pull/17063) Use collections for `HistoricalInfo`: From 0781b598e07aa2fbfe4e03101a26399d1989d664 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 3 Aug 2023 12:31:47 +0530 Subject: [PATCH 04/23] iterator changes --- simapp/export.go | 26 +++++++++++++++----------- x/staking/keeper/delegation.go | 24 ------------------------ x/staking/keeper/genesis.go | 16 +++++++++++----- x/staking/keeper/invariants.go | 20 +++++++++++++------- x/staking/types/keys.go | 2 +- 5 files changed, 40 insertions(+), 48 deletions(-) diff --git a/simapp/export.go b/simapp/export.go index 55335c0a45bc..adc731feab3d 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -185,17 +185,21 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] } // iterate through unbonding delegations, reset creation height - err = app.StakingKeeper.IterateUnbondingDelegations(ctx, func(_ int64, ubd stakingtypes.UnbondingDelegation) (stop bool) { - for i := range ubd.Entries { - ubd.Entries[i].CreationHeight = 0 - } - err = app.StakingKeeper.SetUnbondingDelegation(ctx, ubd) - if err != nil { - panic(err) - } - return false - }) - if err != nil { + err = app.StakingKeeper.UnbondingDelegation.Walk( + ctx, + nil, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) { + for i := range ubd.Entries { + ubd.Entries[i].CreationHeight = 0 + } + err = app.StakingKeeper.SetUnbondingDelegation(ctx, ubd) + if err != nil { + panic(err) + } + return false, err + }, + ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { panic(err) } // Iterate through validators by power descending, reset bond heights, and diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index f5f75e3cacbe..68eeec68a405 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -237,30 +237,6 @@ func (k Keeper) GetUnbondingDelegationsFromValidator(ctx context.Context, valAdd return ubds, nil } -// IterateUnbondingDelegations iterates through all of the unbonding delegations. -func (k Keeper) IterateUnbondingDelegations(ctx context.Context, fn func(index int64, ubd types.UnbondingDelegation) (stop bool)) error { - store := k.storeService.OpenKVStore(ctx) - prefix := types.UnbondingDelegationKey - iterator, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) - if err != nil { - return err - } - defer iterator.Close() - - for i := int64(0); iterator.Valid(); iterator.Next() { - ubd, err := types.UnmarshalUBD(k.cdc, iterator.Value()) - if err != nil { - return err - } - if stop := fn(i, ubd); stop { - break - } - i++ - } - - return nil -} - // GetDelegatorUnbonding returns the total amount a delegator has unbonding. func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddress) (math.Int, error) { unbonding := math.ZeroInt() diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index ca544e7d26aa..ed63cad10aa1 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -2,10 +2,12 @@ package keeper import ( "context" + "errors" "fmt" abci "github.com/cometbft/cometbft/abci/types" + "cosmossdk.io/collections" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" @@ -210,11 +212,15 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { var unbondingDelegations []types.UnbondingDelegation - err := k.IterateUnbondingDelegations(ctx, func(_ int64, ubd types.UnbondingDelegation) (stop bool) { - unbondingDelegations = append(unbondingDelegations, ubd) - return false - }) - if err != nil { + err := k.UnbondingDelegation.Walk( + ctx, + nil, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + unbondingDelegations = append(unbondingDelegations, ubd) + return false, err + }, + ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { panic(err) } diff --git a/x/staking/keeper/invariants.go b/x/staking/keeper/invariants.go index 3f73f8a9bbc0..df24c5ac06c3 100644 --- a/x/staking/keeper/invariants.go +++ b/x/staking/keeper/invariants.go @@ -2,8 +2,10 @@ package keeper import ( "bytes" + "errors" "fmt" + "cosmossdk.io/collections" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" @@ -72,13 +74,17 @@ func ModuleAccountInvariants(k *Keeper) sdk.Invariant { panic(err) } - err = k.IterateUnbondingDelegations(ctx, func(_ int64, ubd types.UnbondingDelegation) bool { - for _, entry := range ubd.Entries { - notBonded = notBonded.Add(entry.Balance) - } - return false - }) - if err != nil { + err = k.UnbondingDelegation.Walk( + ctx, + nil, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + for _, entry := range ubd.Entries { + notBonded = notBonded.Add(entry.Balance) + } + return false, err + }, + ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { panic(err) } diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index cdfbd8e42681..b6439077810a 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -223,7 +223,7 @@ func GetDelegationsKey(delAddr sdk.AccAddress) []byte { // 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 { - return append(append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), valAddr...) + return append(append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), address.MustLengthPrefix(valAddr)...) } // GetUBDByValIndexKey creates the index-key for an unbonding delegation, stored by validator-index From a3394854d36d55161602c7be7ac8dbb5b6856794 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 4 Aug 2023 16:52:37 +0530 Subject: [PATCH 05/23] fix all tests --- simapp/export.go | 8 ++++++-- x/staking/keeper/delegation.go | 26 ++++++++++++++++++-------- x/staking/keeper/genesis.go | 12 ++++++++---- x/staking/keeper/grpc_query.go | 18 +++++++++++------- x/staking/keeper/invariants.go | 10 +++++++--- x/staking/keeper/keeper.go | 8 ++++---- x/staking/keeper/query_utils.go | 15 ++++++++++----- x/staking/types/keys.go | 2 +- 8 files changed, 65 insertions(+), 34 deletions(-) diff --git a/simapp/export.go b/simapp/export.go index adc731feab3d..d6fb24378252 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -185,10 +185,14 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] } // iterate through unbonding delegations, reset creation height - err = app.StakingKeeper.UnbondingDelegation.Walk( + err = app.StakingKeeper.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { + ubd, err := stakingtypes.UnmarshalUBD(app.appCodec, value) + if err != nil { + return true, err + } for i := range ubd.Entries { ubd.Entries[i].CreationHeight = 0 } diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 68eeec68a405..2718a9e6fde1 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -175,16 +175,21 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd i := 0 rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) - err = k.UnbondingDelegation.Walk( + err = k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { + unbondingDelegation, err := types.UnmarshalUBD(k.cdc, value) + if err != nil { + return true, err + } + unbondingDelegations = append(unbondingDelegations, unbondingDelegation) i++ - unbondingDelegations = append(unbondingDelegations, value) + if i >= int(maxRetrieve) { - return false, err + return true, nil } - return false, err + return false, nil }, ) if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { @@ -240,10 +245,15 @@ func (k Keeper) GetUnbondingDelegationsFromValidator(ctx context.Context, valAdd // GetDelegatorUnbonding returns the total amount a delegator has unbonding. func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddress) (math.Int, error) { unbonding := math.ZeroInt() - err := k.UnbondingDelegation.Walk( + rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + err := k.UnbondingDelegations.Walk( ctx, - nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + rng, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { + ubd, err := types.UnmarshalUBD(k.cdc, value) + if err != nil { + return true, err + } for _, entry := range ubd.Entries { unbonding = unbonding.Add(entry.Balance) } diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index ed63cad10aa1..b47b229fbd01 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -212,12 +212,16 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { var unbondingDelegations []types.UnbondingDelegation - err := k.UnbondingDelegation.Walk( + err := k.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { - unbondingDelegations = append(unbondingDelegations, ubd) - return false, err + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { + unbondingDelegation, err := types.UnmarshalUBD(k.cdc, value) + if err != nil { + return true, err + } + unbondingDelegations = append(unbondingDelegations, unbondingDelegation) + return false, nil }, ) if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index 245821652199..c987def01f1c 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -359,23 +359,27 @@ func (k Querier) DelegatorUnbondingDelegations(ctx context.Context, req *types.Q } var unbondingDelegations types.UnbondingDelegations - delAddr, err := sdk.AccAddressFromBech32(req.DelegatorAddr) + delAddr, err := k.authKeeper.AddressCodec().StringToBytes(req.DelegatorAddr) if err != nil { return nil, err } - unbondingDelegations, pageRes, err := query.CollectionPaginate( + _, pageRes, err := query.CollectionPaginate( ctx, - k.Keeper.UnbondingDelegation, + k.UnbondingDelegations, req.Pagination, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (ubd types.UnbondingDelegation, err error) { - unbondingDelegations = append(unbondingDelegations, value) - return value, err + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (unbond types.UnbondingDelegation, err error) { + unbond, err = types.UnmarshalUBD(k.cdc, value) + if err != nil { + return unbond, err + } + unbondingDelegations = append(unbondingDelegations, unbond) + return unbond, nil }, query.WithCollectionPaginationPairPrefix[sdk.AccAddress, sdk.ValAddress](delAddr), ) if err != nil { - return nil, err + return nil, status.Error(codes.Internal, err.Error()) } return &types.QueryDelegatorUnbondingDelegationsResponse{ diff --git a/x/staking/keeper/invariants.go b/x/staking/keeper/invariants.go index df24c5ac06c3..baff0b00e477 100644 --- a/x/staking/keeper/invariants.go +++ b/x/staking/keeper/invariants.go @@ -74,14 +74,18 @@ func ModuleAccountInvariants(k *Keeper) sdk.Invariant { panic(err) } - err = k.UnbondingDelegation.Walk( + err = k.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { + ubd, err := types.UnmarshalUBD(k.cdc, value) + if err != nil { + return true, err + } for _, entry := range ubd.Entries { notBonded = notBonded.Add(entry.Balance) } - return false, err + return false, nil }, ) if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index b30e887f1f93..d6f0f600b468 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -38,10 +38,10 @@ type Keeper struct { LastTotalPower collections.Item[math.Int] ValidatorUpdates collections.Item[types.ValidatorUpdates] DelegationsByValidator collections.Map[collections.Pair[sdk.ValAddress, sdk.AccAddress], []byte] - UnbondingDelegation collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], types.UnbondingDelegation] UnbondingID collections.Sequence ValidatorByConsensusAddress collections.Map[sdk.ConsAddress, sdk.ValAddress] UnbondingType collections.Map[uint64, uint64] + UnbondingDelegations collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], []byte] } // NewKeeper creates a new staking Keeper instance @@ -91,15 +91,15 @@ func NewKeeper( collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), sdk.AccAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility collections.BytesValue, ), - UnbondingDelegation: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.ValAddressKey), codec.CollValue[types.UnbondingDelegation](cdc)), - UnbondingID: collections.NewSequence(sb, types.UnbondingIDKey, "unbonding_id"), + UnbondingID: collections.NewSequence(sb, types.UnbondingIDKey, "unbonding_id"), ValidatorByConsensusAddress: collections.NewMap( sb, types.ValidatorsByConsAddrKey, "validator_by_cons_addr", sdk.LengthPrefixedAddressKey(sdk.ConsAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility collcodec.KeyToValueCodec(sdk.ValAddressKey), ), - UnbondingType: collections.NewMap(sb, types.UnbondingTypeKey, "unbonding_type", collections.Uint64Key, collections.Uint64Value), + UnbondingType: collections.NewMap(sb, types.UnbondingTypeKey, "unbonding_type", collections.Uint64Key, collections.Uint64Value), + UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.ValAddressKey), collections.BytesValue), } schema, err := sb.Build() diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index 4888e7b854f2..c4db7de6b176 100644 --- a/x/staking/keeper/query_utils.go +++ b/x/staking/keeper/query_utils.go @@ -92,12 +92,17 @@ func (k Keeper) GetAllDelegatorDelegations(ctx context.Context, delegator sdk.Ac func (k Keeper) GetAllUnbondingDelegations(ctx context.Context, delegator sdk.AccAddress) ([]types.UnbondingDelegation, error) { unbondingDelegations := make([]types.UnbondingDelegation, 0) - err := k.UnbondingDelegation.Walk( + rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + err := k.UnbondingDelegations.Walk( ctx, - collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator), - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { - unbondingDelegations = append(unbondingDelegations, value) - return false, err + rng, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { + unbondingDelegation, err := types.UnmarshalUBD(k.cdc, value) + if err != nil { + return true, err + } + unbondingDelegations = append(unbondingDelegations, unbondingDelegation) + return false, nil }, ) if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 1b731207d1a1..f2d249a699b4 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -230,7 +230,7 @@ func GetUBDKeyFromValIndexKey(indexKey []byte) []byte { kv.AssertKeyAtLeastLength(addrs, 3+int(valAddrLen)) delAddr := addrs[valAddrLen+2:] - return append(append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), valAddr...) + return append(append(UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), address.MustLengthPrefix(valAddr)...) } // GetUBDsByValIndexKey creates the prefix keyspace for the indexes of unbonding delegations for a validator From f2dc4f5a3e726dced2135ce028bfab2c5059b441 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 8 Aug 2023 15:58:11 +0530 Subject: [PATCH 06/23] remove store usage from ubd set, get, remove funcs --- x/staking/keeper/delegation.go | 31 ++++++++++++---------------- x/staking/simulation/decoder_test.go | 3 ++- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 2718a9e6fde1..524df60e7e27 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -201,10 +201,8 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd // GetUnbondingDelegation returns a unbonding delegation. func (k Keeper) GetUnbondingDelegation(ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (ubd types.UnbondingDelegation, err error) { - store := k.storeService.OpenKVStore(ctx) - key := types.GetUBDKey(delAddr, valAddr) - value, err := store.Get(key) - if err != nil { + value, err := k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr)) + if err != nil && !errors.Is(err, collections.ErrNotFound) { return ubd, err } @@ -345,19 +343,18 @@ func (k Keeper) HasMaxUnbondingDelegationEntries(ctx context.Context, delegatorA // SetUnbondingDelegation sets the unbonding delegation and associated index. func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingDelegation) error { - delAddr, err := k.authKeeper.AddressCodec().StringToBytes(ubd.DelegatorAddress) + store := k.storeService.OpenKVStore(ctx) + bz := types.MustMarshalUBD(k.cdc, ubd) + + delAddr, err := sdk.AccAddressFromBech32(ubd.DelegatorAddress) if err != nil { return err } - - store := k.storeService.OpenKVStore(ctx) - bz := types.MustMarshalUBD(k.cdc, ubd) - valAddr, err := k.validatorAddressCodec.StringToBytes(ubd.ValidatorAddress) + valAddr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) if err != nil { return err } - key := types.GetUBDKey(delAddr, valAddr) - err = store.Set(key, bz) + err = k.UnbondingDelegations.Set(ctx, collections.Join(delAddr, valAddr), bz) if err != nil { return err } @@ -367,23 +364,21 @@ func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingD // RemoveUnbondingDelegation removes the unbonding delegation object and associated index. func (k Keeper) RemoveUnbondingDelegation(ctx context.Context, ubd types.UnbondingDelegation) error { - delegatorAddress, err := k.authKeeper.AddressCodec().StringToBytes(ubd.DelegatorAddress) + store := k.storeService.OpenKVStore(ctx) + delAddr, err := sdk.AccAddressFromBech32(ubd.DelegatorAddress) if err != nil { return err } - - store := k.storeService.OpenKVStore(ctx) - addr, err := k.validatorAddressCodec.StringToBytes(ubd.ValidatorAddress) + valAddr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) if err != nil { return err } - key := types.GetUBDKey(delegatorAddress, addr) - err = store.Delete(key) + err = k.UnbondingDelegations.Remove(ctx, collections.Join(delAddr, valAddr)) if err != nil { return err } - return store.Delete(types.GetUBDByValIndexKey(delegatorAddress, addr)) + return store.Delete(types.GetUBDByValIndexKey(delAddr, valAddr)) } // SetUnbondingDelegationEntry adds an entry to the unbonding delegation at diff --git a/x/staking/simulation/decoder_test.go b/x/staking/simulation/decoder_test.go index 0179794929b1..140bbec91da0 100644 --- a/x/staking/simulation/decoder_test.go +++ b/x/staking/simulation/decoder_test.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" sdk "github.com/cosmos/cosmos-sdk/types" + sdkaddress "github.com/cosmos/cosmos-sdk/types/address" "github.com/cosmos/cosmos-sdk/types/kv" "github.com/cosmos/cosmos-sdk/types/module/testutil" "github.com/cosmos/cosmos-sdk/x/staking/simulation" @@ -43,7 +44,7 @@ func TestDecodeStore(t *testing.T) { {Key: types.GetValidatorKey(valAddr1), Value: cdc.MustMarshal(&val)}, {Key: types.LastValidatorPowerKey, Value: valAddr1.Bytes()}, {Key: types.GetDelegationKey(delAddr1, valAddr1), Value: cdc.MustMarshal(&del)}, - {Key: types.GetUBDKey(delAddr1, valAddr1), Value: cdc.MustMarshal(&ubd)}, + {Key: append(append(types.UnbondingDelegationKey, sdkaddress.MustLengthPrefix(delAddr1)...), sdkaddress.MustLengthPrefix(valAddr1)...), Value: cdc.MustMarshal(&ubd)}, {Key: types.GetREDKey(delAddr1, valAddr1, valAddr1), Value: cdc.MustMarshal(&red)}, {Key: []byte{0x99}, Value: []byte{0x99}}, }, From 6fc003c2688ec0b0d8f165a50fd5817bc6e7b249 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 9 Aug 2023 14:58:28 +0530 Subject: [PATCH 07/23] wip: try fixing keeper tests --- x/staking/keeper/unbonding.go | 9 +++++---- x/staking/simulation/decoder_test.go | 4 ---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/x/staking/keeper/unbonding.go b/x/staking/keeper/unbonding.go index bd874392b86e..70ce4afa887d 100644 --- a/x/staking/keeper/unbonding.go +++ b/x/staking/keeper/unbonding.go @@ -45,8 +45,6 @@ func (k Keeper) SetUnbondingType(ctx context.Context, id uint64, unbondingType t // GetUnbondingDelegationByUnbondingID returns a unbonding delegation that has an unbonding delegation entry with a certain ID func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint64) (ubd types.UnbondingDelegation, err error) { - store := k.storeService.OpenKVStore(ctx) - ubdKey, err := k.UnbondingIndex.Get(ctx, id) if err != nil { if errors.Is(err, collections.ErrNotFound) { @@ -59,8 +57,11 @@ func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint return types.UnbondingDelegation{}, types.ErrNoUnbondingDelegation } - value, err := store.Get(ubdKey) - if err != nil { + delAddr := ubdKey[2 : (len(ubdKey)/2)+1] + valAddr := ubdKey[2+len(ubdKey)/2:] + + value, err := k.UnbondingDelegations.Get(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) + if err != nil && !errors.Is(err, collections.ErrNotFound) { return types.UnbondingDelegation{}, err } diff --git a/x/staking/simulation/decoder_test.go b/x/staking/simulation/decoder_test.go index 888ccb426ff3..ac8af1122066 100644 --- a/x/staking/simulation/decoder_test.go +++ b/x/staking/simulation/decoder_test.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" sdk "github.com/cosmos/cosmos-sdk/types" - sdkaddress "github.com/cosmos/cosmos-sdk/types/address" "github.com/cosmos/cosmos-sdk/types/kv" "github.com/cosmos/cosmos-sdk/types/module/testutil" "github.com/cosmos/cosmos-sdk/x/staking/simulation" @@ -32,7 +31,6 @@ func TestDecodeStore(t *testing.T) { val, err := types.NewValidator(valAddr1.String(), delPk1, types.NewDescription("test", "test", "test", "test", "test")) require.NoError(t, err) - ubd := types.NewUnbondingDelegation(delAddr1, valAddr1, 15, bondTime, math.OneInt(), 1, address.NewBech32Codec("cosmosvaloper"), address.NewBech32Codec("cosmos")) red := types.NewRedelegation(delAddr1, valAddr1, valAddr1, 12, bondTime, math.OneInt(), math.LegacyOneDec(), 0, address.NewBech32Codec("cosmosvaloper"), address.NewBech32Codec("cosmos")) oneIntBz, err := math.OneInt().Marshal() require.NoError(t, err) @@ -42,7 +40,6 @@ func TestDecodeStore(t *testing.T) { {Key: types.LastTotalPowerKey, Value: oneIntBz}, {Key: types.GetValidatorKey(valAddr1), Value: cdc.MustMarshal(&val)}, {Key: types.LastValidatorPowerKey, Value: valAddr1.Bytes()}, - {Key: append(append(types.UnbondingDelegationKey, sdkaddress.MustLengthPrefix(delAddr1)...), sdkaddress.MustLengthPrefix(valAddr1)...), Value: cdc.MustMarshal(&ubd)}, {Key: types.GetREDKey(delAddr1, valAddr1, valAddr1), Value: cdc.MustMarshal(&red)}, {Key: []byte{0x99}, Value: []byte{0x99}}, }, @@ -55,7 +52,6 @@ func TestDecodeStore(t *testing.T) { {"LastTotalPower", fmt.Sprintf("%v\n%v", math.OneInt(), math.OneInt())}, {"Validator", fmt.Sprintf("%v\n%v", val, val)}, {"LastValidatorPower/ValidatorsByConsAddr/ValidatorsByPowerIndex", fmt.Sprintf("%v\n%v", valAddr1, valAddr1)}, - {"UnbondingDelegation", fmt.Sprintf("%v\n%v", ubd, ubd)}, {"Redelegation", fmt.Sprintf("%v\n%v", red, red)}, {"other", ""}, } From b92f8a807e79b7e6463efaf581ca736fd932bbcc Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 16 Aug 2023 12:30:45 +0530 Subject: [PATCH 08/23] address review comments and fix tests --- x/staking/keeper/delegation.go | 12 ++++++------ x/staking/migrations/v2/store_test.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 43ba8db8c2da..8bb50882fc7d 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -298,15 +298,15 @@ func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingD store := k.storeService.OpenKVStore(ctx) bz := types.MustMarshalUBD(k.cdc, ubd) - delAddr, err := sdk.AccAddressFromBech32(ubd.DelegatorAddress) + delAddr, err := k.authKeeper.AddressCodec().StringToBytes(ubd.DelegatorAddress) if err != nil { return err } - valAddr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) + valAddr, err := k.validatorAddressCodec.StringToBytes(ubd.ValidatorAddress) if err != nil { return err } - err = k.UnbondingDelegations.Set(ctx, collections.Join(delAddr, valAddr), bz) + err = k.UnbondingDelegations.Set(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr)), bz) if err != nil { return err } @@ -317,15 +317,15 @@ func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingD // RemoveUnbondingDelegation removes the unbonding delegation object and associated index. func (k Keeper) RemoveUnbondingDelegation(ctx context.Context, ubd types.UnbondingDelegation) error { store := k.storeService.OpenKVStore(ctx) - delAddr, err := sdk.AccAddressFromBech32(ubd.DelegatorAddress) + delAddr, err := k.authKeeper.AddressCodec().StringToBytes(ubd.DelegatorAddress) if err != nil { return err } - valAddr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) + valAddr, err := k.validatorAddressCodec.StringToBytes(ubd.ValidatorAddress) if err != nil { return err } - err = k.UnbondingDelegations.Remove(ctx, collections.Join(delAddr, valAddr)) + err = k.UnbondingDelegations.Remove(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) if err != nil { return err } diff --git a/x/staking/migrations/v2/store_test.go b/x/staking/migrations/v2/store_test.go index 1b7cecdf5618..fd285d661124 100644 --- a/x/staking/migrations/v2/store_test.go +++ b/x/staking/migrations/v2/store_test.go @@ -13,7 +13,7 @@ import ( sdktestuil "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" + 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" @@ -142,5 +142,5 @@ func TestStoreMigration(t *testing.T) { } func unbondingKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { - return append(append(types.UnbondingDelegationKey, address.MustLengthPrefix(delAddr)...), address.MustLengthPrefix(valAddr)...) + return append(append(types.UnbondingDelegationKey, sdkaddress.MustLengthPrefix(delAddr)...), sdkaddress.MustLengthPrefix(valAddr)...) } From a2b3f5900e715d7081554efbb39cc893cb56da95 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 16 Aug 2023 12:36:56 +0530 Subject: [PATCH 09/23] fix tests --- simapp/export.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/simapp/export.go b/simapp/export.go index cdd20a593e64..39297bbcb0b0 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -2,11 +2,13 @@ package simapp import ( "encoding/json" + "errors" "fmt" "log" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + "cosmossdk.io/collections" storetypes "cosmossdk.io/store/types" servertypes "github.com/cosmos/cosmos-sdk/server/types" From fd0a9f8ea2c7056bd735af7ca459eaeb06e01020 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 16 Aug 2023 13:38:48 +0530 Subject: [PATCH 10/23] fix few tests --- tests/integration/staking/keeper/deterministic_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/staking/keeper/deterministic_test.go b/tests/integration/staking/keeper/deterministic_test.go index fb27be24ea96..a475165e891d 100644 --- a/tests/integration/staking/keeper/deterministic_test.go +++ b/tests/integration/staking/keeper/deterministic_test.go @@ -471,7 +471,7 @@ func TestGRPCValidatorUnbondingDelegations(t *testing.T) { ValidatorAddr: validator.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorUnbondingDelegations, 3719, false) + testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorUnbondingDelegations, 2735, false) } func TestGRPCDelegation(t *testing.T) { @@ -543,7 +543,7 @@ func TestGRPCUnbondingDelegation(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.UnbondingDelegation, 1621, false) + testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.UnbondingDelegation, 1618, false) } func TestGRPCDelegatorDelegations(t *testing.T) { @@ -654,7 +654,7 @@ func TestGRPCDelegatorUnbondingDelegations(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorUnbondingDelegations, 1302, false) + testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorUnbondingDelegations, 1296, false) } func TestGRPCHistoricalInfo(t *testing.T) { From 751dcafdf4910e5252a6be104b605f1514435580 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 18 Aug 2023 12:44:53 +0530 Subject: [PATCH 11/23] fix tests --- simapp/export.go | 3 +-- x/staking/keeper/keeper.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/simapp/export.go b/simapp/export.go index 39297bbcb0b0..277f790204e2 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -2,7 +2,6 @@ package simapp import ( "encoding/json" - "errors" "fmt" "log" @@ -211,7 +210,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] return false, err }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { panic(err) } // Iterate through validators by power descending, reset bond heights, and diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index f0d7e9cccaf9..dde65cd2c81b 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -110,7 +110,7 @@ func NewKeeper( ), UnbondingType: collections.NewMap(sb, types.UnbondingTypeKey, "unbonding_type", collections.Uint64Key, collections.Uint64Value), UnbondingIndex: collections.NewMap(sb, types.UnbondingIndexKey, "unbonding_index", collections.Uint64Key, collections.BytesValue), - UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.ValAddressKey), collections.BytesValue), + UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), sdk.ValAddressKey), collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility } schema, err := sb.Build() From d9b5cb5b9a1d7e5a0edf82f29fe9d50243e082b7 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 18 Aug 2023 15:19:24 +0530 Subject: [PATCH 12/23] fix usage of length prefixed addr key --- x/staking/keeper/keeper.go | 2 +- x/staking/simulation/decoder_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 7dd7e9fde5a9..3818bd556040 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -121,7 +121,7 @@ func NewKeeper( codec.CollValue[types.Redelegation](cdc), ), UnbondingIndex: collections.NewMap(sb, types.UnbondingIndexKey, "unbonding_index", collections.Uint64Key, collections.BytesValue), - UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), sdk.ValAddressKey), collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, 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/simulation/decoder_test.go b/x/staking/simulation/decoder_test.go index bcb2bae94a3d..771a47a7318d 100644 --- a/x/staking/simulation/decoder_test.go +++ b/x/staking/simulation/decoder_test.go @@ -18,7 +18,6 @@ import ( var ( delPk1 = ed25519.GenPrivKey().PubKey() - delAddr1 = sdk.AccAddress(delPk1.Address()) valAddr1 = sdk.ValAddress(delPk1.Address()) ) From d27e4dade2bc79d06084d7e8d2bcf5e34f56f682 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 18 Aug 2023 15:28:12 +0530 Subject: [PATCH 13/23] fix integration tests --- tests/integration/staking/keeper/deterministic_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/staking/keeper/deterministic_test.go b/tests/integration/staking/keeper/deterministic_test.go index a475165e891d..fb27be24ea96 100644 --- a/tests/integration/staking/keeper/deterministic_test.go +++ b/tests/integration/staking/keeper/deterministic_test.go @@ -471,7 +471,7 @@ func TestGRPCValidatorUnbondingDelegations(t *testing.T) { ValidatorAddr: validator.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorUnbondingDelegations, 2735, false) + testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorUnbondingDelegations, 3719, false) } func TestGRPCDelegation(t *testing.T) { @@ -543,7 +543,7 @@ func TestGRPCUnbondingDelegation(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.UnbondingDelegation, 1618, false) + testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.UnbondingDelegation, 1621, false) } func TestGRPCDelegatorDelegations(t *testing.T) { @@ -654,7 +654,7 @@ func TestGRPCDelegatorUnbondingDelegations(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorUnbondingDelegations, 1296, false) + testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorUnbondingDelegations, 1302, false) } func TestGRPCHistoricalInfo(t *testing.T) { From 1726250acc9556dfe0648de214369626fdbe108a Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 21 Aug 2023 14:04:04 +0530 Subject: [PATCH 14/23] add diff test for unbonding delegation --- x/staking/keeper/keeper_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index 46aabdcabb4c..0b291d43c497 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "testing" + "time" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" cmttime "github.com/cometbft/cometbft/types/time" @@ -39,11 +40,13 @@ type KeeperTestSuite struct { accountKeeper *stakingtestutil.MockAccountKeeper queryClient stakingtypes.QueryClient msgServer stakingtypes.MsgServer + key *storetypes.KVStoreKey } func (s *KeeperTestSuite) SetupTest() { require := s.Require() key := storetypes.NewKVStoreKey(stakingtypes.StoreKey) + s.key = key storeService := runtime.NewKVStoreService(key) testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) @@ -112,3 +115,33 @@ func (s *KeeperTestSuite) TestLastTotalPower() { func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } + +func (s *KeeperTestSuite) TestDiffCollsMigration() { + s.SetupTest() + + delAddrs, valAddrs := createValAddrs(100) + err := testutil.DiffCollectionsMigration( + s.ctx, + s.key, + 100, + func(i int64) { + ubd := stakingtypes.UnbondingDelegation{ + DelegatorAddress: delAddrs[i].String(), + ValidatorAddress: valAddrs[i].String(), + Entries: []stakingtypes.UnbondingDelegationEntry{ + { + CreationHeight: i, + CompletionTime: time.Unix(i, 0).UTC(), + Balance: math.NewInt(i), + UnbondingId: uint64(i), + }, + }, + } + err := s.stakingKeeper.SetUnbondingDelegation(s.ctx, ubd) + s.Require().NoError(err) + }, + "d03ca412f3f6849b5148a2ca49ac2555f65f90b7fab6a289575ed337f15c0f4b", + ) + + s.Require().NoError(err) +} From ad6b1d059dc113328e927572285cc93707ded8bb Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Aug 2023 11:54:14 +0530 Subject: [PATCH 15/23] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cf85e08ea7b..5706b1fee522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [#17270](https://github.com/cosmos/cosmos-sdk/pull/17270) Use collections for `UnbondingDelegation`: * remove from `types`: `GetUBDsKey` + * remove from `Keeper`: `IterateUnbondingDelegations`, `IterateDelegatorUnbondingDelegations` * (x/staking) [#17315](https://github.com/cosmos/cosmos-sdk/pull/17044) Use collections for `RedelegationKey`: * remove from `keeper`: `GetRedelegation` * (types) `module.BeginBlockAppModule` has been replaced by Core API `appmodule.HasBeginBlocker`. From ffd1912d66f1e8e767c5e0c9afa64dec045b94d4 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Aug 2023 16:42:59 +0530 Subject: [PATCH 16/23] address nits --- x/staking/keeper/delegation.go | 4 ++-- x/staking/keeper/genesis.go | 3 +-- x/staking/keeper/invariants.go | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index eb97df9f0426..a64756632aa3 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -149,7 +149,7 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd return false, nil }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } @@ -215,7 +215,7 @@ func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddr return false, nil }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return unbonding, err } return unbonding, err diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index 93bc475d1e57..b86fa0c276f5 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -2,7 +2,6 @@ package keeper import ( "context" - "errors" "fmt" abci "github.com/cometbft/cometbft/abci/types" @@ -228,7 +227,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { return false, nil }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { panic(err) } diff --git a/x/staking/keeper/invariants.go b/x/staking/keeper/invariants.go index 891864ed1523..ed72273f7058 100644 --- a/x/staking/keeper/invariants.go +++ b/x/staking/keeper/invariants.go @@ -2,7 +2,6 @@ package keeper import ( "bytes" - "errors" "fmt" "cosmossdk.io/collections" @@ -88,7 +87,7 @@ func ModuleAccountInvariants(k *Keeper) sdk.Invariant { return false, nil }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { panic(err) } From ef9178e9e218a27d99297b2639d85816805fe19e Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 23 Aug 2023 17:39:22 +0530 Subject: [PATCH 17/23] wip: use UnbondingDelegations type instead of bytes --- simapp/export.go | 6 +----- x/staking/keeper/delegation.go | 31 +++++++++------------------- x/staking/keeper/genesis.go | 8 ++------ x/staking/keeper/grpc_query.go | 10 +++------ x/staking/keeper/invariants.go | 6 +----- x/staking/keeper/keeper.go | 4 ++-- x/staking/keeper/keeper_test.go | 36 ++++++++++++++++++++++++++++++++- x/staking/keeper/query_utils.go | 8 ++------ x/staking/keeper/unbonding.go | 15 ++++---------- 9 files changed, 60 insertions(+), 64 deletions(-) diff --git a/simapp/export.go b/simapp/export.go index 277f790204e2..30a0b96016d6 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -195,11 +195,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] err = app.StakingKeeper.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { - ubd, err := stakingtypes.UnmarshalUBD(app.appCodec, value) - if err != nil { - return true, err - } + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) { for i := range ubd.Entries { ubd.Entries[i].CreationHeight = 0 } diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index e771631d53d3..d030ee0dd95b 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -135,12 +135,8 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd err = k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { - unbondingDelegation, err := types.UnmarshalUBD(k.cdc, value) - if err != nil { - return true, err - } - unbondingDelegations = append(unbondingDelegations, unbondingDelegation) + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + unbondingDelegations = append(unbondingDelegations, value) i++ if i >= int(maxRetrieve) { @@ -158,16 +154,14 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd // GetUnbondingDelegation returns a unbonding delegation. func (k Keeper) GetUnbondingDelegation(ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (ubd types.UnbondingDelegation, err error) { - value, err := k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr)) - if err != nil && !errors.Is(err, collections.ErrNotFound) { + ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr)) + if err != nil { + if errors.Is(err, collections.ErrNotFound) { + return ubd, types.ErrNoUnbondingDelegation + } return ubd, err } - - if value == nil { - return ubd, types.ErrNoUnbondingDelegation - } - - return types.UnmarshalUBD(k.cdc, value) + return ubd, nil } // GetUnbondingDelegationsFromValidator returns all unbonding delegations from a @@ -204,11 +198,7 @@ func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddr err := k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { - ubd, err := types.UnmarshalUBD(k.cdc, value) - if err != nil { - return true, err - } + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { for _, entry := range ubd.Entries { unbonding = unbonding.Add(entry.Balance) } @@ -275,7 +265,6 @@ func (k Keeper) HasMaxUnbondingDelegationEntries(ctx context.Context, delegatorA // SetUnbondingDelegation sets the unbonding delegation and associated index. func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingDelegation) error { store := k.storeService.OpenKVStore(ctx) - bz := types.MustMarshalUBD(k.cdc, ubd) delAddr, err := k.authKeeper.AddressCodec().StringToBytes(ubd.DelegatorAddress) if err != nil { @@ -285,7 +274,7 @@ func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingD if err != nil { return err } - err = k.UnbondingDelegations.Set(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr)), bz) + err = k.UnbondingDelegations.Set(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr)), ubd) if err != nil { return err } diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index b86fa0c276f5..ea1d8a93261e 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -218,12 +218,8 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { err := k.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { - unbondingDelegation, err := types.UnmarshalUBD(k.cdc, value) - if err != nil { - return true, err - } - unbondingDelegations = append(unbondingDelegations, unbondingDelegation) + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + unbondingDelegations = append(unbondingDelegations, value) return false, nil }, ) diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index ad1f1f993ff1..6708451382fd 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -362,13 +362,9 @@ func (k Querier) DelegatorUnbondingDelegations(ctx context.Context, req *types.Q ctx, k.UnbondingDelegations, req.Pagination, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (unbond types.UnbondingDelegation, err error) { - unbond, err = types.UnmarshalUBD(k.cdc, value) - if err != nil { - return unbond, err - } - unbondingDelegations = append(unbondingDelegations, unbond) - return unbond, nil + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (types.UnbondingDelegation, error) { + unbondingDelegations = append(unbondingDelegations, value) + return value, nil }, query.WithCollectionPaginationPairPrefix[sdk.AccAddress, sdk.ValAddress](delAddr), ) diff --git a/x/staking/keeper/invariants.go b/x/staking/keeper/invariants.go index ed72273f7058..7bc5f3e0a6c7 100644 --- a/x/staking/keeper/invariants.go +++ b/x/staking/keeper/invariants.go @@ -76,11 +76,7 @@ func ModuleAccountInvariants(k *Keeper) sdk.Invariant { err = k.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { - ubd, err := types.UnmarshalUBD(k.cdc, value) - if err != nil { - return true, err - } + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { for _, entry := range ubd.Entries { notBonded = notBonded.Add(entry.Balance) } diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 3eb6b559dbd8..7b4e7411ddbd 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -44,7 +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] - UnbondingDelegations collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], []byte] + UnbondingDelegations collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], types.UnbondingDelegation] RedelegationsByValDst collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] RedelegationsByValSrc collections.Map[collections.Triple[[]byte, []byte, []byte], []byte] } @@ -146,7 +146,7 @@ func NewKeeper( ), collections.BytesValue, ), - UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.LengthPrefixedAddressKey(sdk.ValAddressKey)), collections.BytesValue), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.LengthPrefixedAddressKey(sdk.ValAddressKey)), codec.CollValue[types.UnbondingDelegation](cdc)), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility } schema, err := sb.Build() diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index c68602c57901..88b255f05bbe 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -14,6 +14,7 @@ import ( storetypes "cosmossdk.io/store/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" @@ -43,6 +44,7 @@ type KeeperTestSuite struct { queryClient stakingtypes.QueryClient msgServer stakingtypes.MsgServer key *storetypes.KVStoreKey + cdc codec.Codec } func (s *KeeperTestSuite) SetupTest() { @@ -54,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) @@ -231,11 +234,42 @@ func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } -func (s *KeeperTestSuite) TestDiffCollsMigration() { +// 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 { + unbondingDelegationKey := []byte{0x32} + return append(append(unbondingDelegationKey, addresstypes.MustLengthPrefix(delAddr)...), addresstypes.MustLengthPrefix(valAddr)...) +} + +func (s *KeeperTestSuite) TestUnbondingDelegationsMigrationToColls() { s.SetupTest() delAddrs, valAddrs := createValAddrs(100) err := testutil.DiffCollectionsMigration( + s.ctx, + s.key, + 100, + func(i int64) { + ubd := stakingtypes.UnbondingDelegation{ + DelegatorAddress: delAddrs[i].String(), + ValidatorAddress: valAddrs[i].String(), + Entries: []stakingtypes.UnbondingDelegationEntry{ + { + CreationHeight: i, + CompletionTime: time.Unix(i, 0).UTC(), + Balance: math.NewInt(i), + UnbondingId: uint64(i), + }, + }, + } + bz := stakingtypes.MustMarshalUBD(s.cdc, ubd) + s.ctx.KVStore(s.key).Set(getUBDKey(delAddrs[i], valAddrs[i]), bz) + }, + "a428c023032bfc71a954f8a696e33ecc0806ae77021bd17fc369ba203a96681f", + ) + s.Require().NoError(err) + + err = testutil.DiffCollectionsMigration( s.ctx, s.key, 100, diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index 7c01f51f7101..b4736d873063 100644 --- a/x/staking/keeper/query_utils.go +++ b/x/staking/keeper/query_utils.go @@ -89,12 +89,8 @@ func (k Keeper) GetAllUnbondingDelegations(ctx context.Context, delegator sdk.Ac err := k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value []byte) (stop bool, err error) { - unbondingDelegation, err := types.UnmarshalUBD(k.cdc, value) - if err != nil { - return true, err - } - unbondingDelegations = append(unbondingDelegations, unbondingDelegation) + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + unbondingDelegations = append(unbondingDelegations, value) return false, nil }, ) diff --git a/x/staking/keeper/unbonding.go b/x/staking/keeper/unbonding.go index 70ce4afa887d..6c6c28c1850b 100644 --- a/x/staking/keeper/unbonding.go +++ b/x/staking/keeper/unbonding.go @@ -60,18 +60,11 @@ func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint delAddr := ubdKey[2 : (len(ubdKey)/2)+1] valAddr := ubdKey[2+len(ubdKey)/2:] - value, err := k.UnbondingDelegations.Get(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) - if err != nil && !errors.Is(err, collections.ErrNotFound) { - return types.UnbondingDelegation{}, err - } - - if value == nil { - return types.UnbondingDelegation{}, types.ErrNoUnbondingDelegation - } - - ubd, err = types.UnmarshalUBD(k.cdc, value) - // An error here means that what we got wasn't the right type + ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) if err != nil { + if errors.Is(err, collections.ErrNotFound) { + return types.UnbondingDelegation{}, types.ErrNoUnbondingDelegation + } return types.UnbondingDelegation{}, err } From d3dbaba39010eb41c4a5fe79f6f4b7c254b3eb93 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 23 Aug 2023 17:53:12 +0530 Subject: [PATCH 18/23] fix keeper test --- x/staking/keeper/keeper_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index 88b255f05bbe..8edb42ef5a6b 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -264,8 +264,9 @@ func (s *KeeperTestSuite) TestUnbondingDelegationsMigrationToColls() { } bz := stakingtypes.MustMarshalUBD(s.cdc, ubd) s.ctx.KVStore(s.key).Set(getUBDKey(delAddrs[i], valAddrs[i]), bz) + s.ctx.KVStore(s.key).Set(stakingtypes.GetUBDByValIndexKey(delAddrs[i], valAddrs[i]), []byte{}) }, - "a428c023032bfc71a954f8a696e33ecc0806ae77021bd17fc369ba203a96681f", + "d03ca412f3f6849b5148a2ca49ac2555f65f90b7fab6a289575ed337f15c0f4b", ) s.Require().NoError(err) From 68de5c5cf504d8b6fc3c91e8346fd590cf88738a Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 24 Aug 2023 12:37:34 +0530 Subject: [PATCH 19/23] address review comments --- simapp/export.go | 2 +- x/staking/keeper/delegation.go | 14 +++++++------- x/staking/keeper/genesis.go | 2 +- x/staking/keeper/grpc_query.go | 4 ++-- x/staking/keeper/invariants.go | 2 +- x/staking/keeper/keeper.go | 11 +++++++++-- x/staking/keeper/query_utils.go | 4 ++-- x/staking/keeper/unbonding.go | 2 +- 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/simapp/export.go b/simapp/export.go index 30a0b96016d6..f1b69ba357ca 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -201,7 +201,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] } err = app.StakingKeeper.SetUnbondingDelegation(ctx, ubd) if err != nil { - panic(err) + return true, err } return false, err }, diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index d030ee0dd95b..d5824cedcf89 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -131,11 +131,11 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd unbondingDelegations = make([]types.UnbondingDelegation, maxRetrieve) i := 0 - rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + rng := collections.NewPrefixedPairRange[[]byte, []byte](delegator) err = k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[[]byte, []byte], value types.UnbondingDelegation) (stop bool, err error) { unbondingDelegations = append(unbondingDelegations, value) i++ @@ -154,7 +154,7 @@ func (k Keeper) GetUnbondingDelegations(ctx context.Context, delegator sdk.AccAd // GetUnbondingDelegation returns a unbonding delegation. func (k Keeper) GetUnbondingDelegation(ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (ubd types.UnbondingDelegation, err error) { - ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr)) + ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(delAddr.Bytes(), valAddr.Bytes())) if err != nil { if errors.Is(err, collections.ErrNotFound) { return ubd, types.ErrNoUnbondingDelegation @@ -194,11 +194,11 @@ func (k Keeper) GetUnbondingDelegationsFromValidator(ctx context.Context, valAdd // GetDelegatorUnbonding returns the total amount a delegator has unbonding. func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddress) (math.Int, error) { unbonding := math.ZeroInt() - rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + rng := collections.NewPrefixedPairRange[[]byte, []byte](delegator) err := k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[[]byte, []byte], ubd types.UnbondingDelegation) (stop bool, err error) { for _, entry := range ubd.Entries { unbonding = unbonding.Add(entry.Balance) } @@ -274,7 +274,7 @@ func (k Keeper) SetUnbondingDelegation(ctx context.Context, ubd types.UnbondingD if err != nil { return err } - err = k.UnbondingDelegations.Set(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr)), ubd) + err = k.UnbondingDelegations.Set(ctx, collections.Join(delAddr, valAddr), ubd) if err != nil { return err } @@ -293,7 +293,7 @@ func (k Keeper) RemoveUnbondingDelegation(ctx context.Context, ubd types.Unbondi if err != nil { return err } - err = k.UnbondingDelegations.Remove(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) + err = k.UnbondingDelegations.Remove(ctx, collections.Join(delAddr, valAddr)) if err != nil { return err } diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index ea1d8a93261e..4cd505ba6d1d 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -218,7 +218,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { err := k.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[[]byte, []byte], value types.UnbondingDelegation) (stop bool, err error) { unbondingDelegations = append(unbondingDelegations, value) return false, nil }, diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index 6708451382fd..ca87fcacc743 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -362,11 +362,11 @@ func (k Querier) DelegatorUnbondingDelegations(ctx context.Context, req *types.Q ctx, k.UnbondingDelegations, req.Pagination, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (types.UnbondingDelegation, error) { + func(key collections.Pair[[]byte, []byte], value types.UnbondingDelegation) (types.UnbondingDelegation, error) { unbondingDelegations = append(unbondingDelegations, value) return value, nil }, - query.WithCollectionPaginationPairPrefix[sdk.AccAddress, sdk.ValAddress](delAddr), + query.WithCollectionPaginationPairPrefix[[]byte, []byte](delAddr), ) if err != nil { return nil, status.Error(codes.Internal, err.Error()) diff --git a/x/staking/keeper/invariants.go b/x/staking/keeper/invariants.go index 7bc5f3e0a6c7..855f32ed8942 100644 --- a/x/staking/keeper/invariants.go +++ b/x/staking/keeper/invariants.go @@ -76,7 +76,7 @@ func ModuleAccountInvariants(k *Keeper) sdk.Invariant { err = k.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[[]byte, []byte], ubd types.UnbondingDelegation) (stop bool, err error) { for _, entry := range ubd.Entries { notBonded = notBonded.Add(entry.Balance) } diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 7b4e7411ddbd..6ff107627187 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -44,7 +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] - UnbondingDelegations collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], types.UnbondingDelegation] + 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] } @@ -146,7 +146,14 @@ func NewKeeper( ), collections.BytesValue, ), - UnbondingDelegations: collections.NewMap(sb, types.UnbondingDelegationKey, "unbonding_delegation", collections.PairKeyCodec(sdk.AccAddressKey, sdk.LengthPrefixedAddressKey(sdk.ValAddressKey)), codec.CollValue[types.UnbondingDelegation](cdc)), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + UnbondingDelegations: collections.NewMap( + sb, types.UnbondingDelegationKey, + "unbonding_delegation", + collections.PairKeyCodec( + collections.BytesKey, + sdk.LengthPrefixedBytesKey, // sdk.LengthPrefixedBytesKey is needed to retain state compatibility + ), + codec.CollValue[types.UnbondingDelegation](cdc)), } schema, err := sb.Build() diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index b4736d873063..8cdbfc227734 100644 --- a/x/staking/keeper/query_utils.go +++ b/x/staking/keeper/query_utils.go @@ -85,11 +85,11 @@ func (k Keeper) GetAllDelegatorDelegations(ctx context.Context, delegator sdk.Ac func (k Keeper) GetAllUnbondingDelegations(ctx context.Context, delegator sdk.AccAddress) ([]types.UnbondingDelegation, error) { unbondingDelegations := make([]types.UnbondingDelegation, 0) - rng := collections.NewPrefixUntilPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + rng := collections.NewPrefixUntilPairRange[[]byte, []byte](delegator) err := k.UnbondingDelegations.Walk( ctx, rng, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], value types.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[[]byte, []byte], value types.UnbondingDelegation) (stop bool, err error) { unbondingDelegations = append(unbondingDelegations, value) return false, nil }, diff --git a/x/staking/keeper/unbonding.go b/x/staking/keeper/unbonding.go index 6c6c28c1850b..0dba8dc1e584 100644 --- a/x/staking/keeper/unbonding.go +++ b/x/staking/keeper/unbonding.go @@ -60,7 +60,7 @@ func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint delAddr := ubdKey[2 : (len(ubdKey)/2)+1] valAddr := ubdKey[2+len(ubdKey)/2:] - ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) + ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr)) if err != nil { if errors.Is(err, collections.ErrNotFound) { return types.UnbondingDelegation{}, types.ErrNoUnbondingDelegation From 64a41da09979eeb88c91a10e431c827fe6ad39b5 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 24 Aug 2023 12:40:49 +0530 Subject: [PATCH 20/23] fix simapp test --- simapp/export.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simapp/export.go b/simapp/export.go index f1b69ba357ca..6676c644f699 100644 --- a/simapp/export.go +++ b/simapp/export.go @@ -195,7 +195,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [] err = app.StakingKeeper.UnbondingDelegations.Walk( ctx, nil, - func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) { + func(key collections.Pair[[]byte, []byte], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) { for i := range ubd.Entries { ubd.Entries[i].CreationHeight = 0 } From 9b9cc7469817fe37802afac4a116d5186e18be53 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 25 Aug 2023 09:55:58 +0530 Subject: [PATCH 21/23] remove unused error --- x/staking/keeper/query_utils.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index 8cdbfc227734..ff0499d28783 100644 --- a/x/staking/keeper/query_utils.go +++ b/x/staking/keeper/query_utils.go @@ -2,7 +2,6 @@ package keeper import ( "context" - "errors" "cosmossdk.io/collections" @@ -94,7 +93,7 @@ func (k Keeper) GetAllUnbondingDelegations(ctx context.Context, delegator sdk.Ac return false, nil }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } return unbondingDelegations, nil From 08ca9076ae5bc7a65afa3e893588271df1ac85f9 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 13:45:12 +0530 Subject: [PATCH 22/23] nit: add godoc for key splits --- x/staking/keeper/unbonding.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/staking/keeper/unbonding.go b/x/staking/keeper/unbonding.go index 0dba8dc1e584..55c6b1ecec95 100644 --- a/x/staking/keeper/unbonding.go +++ b/x/staking/keeper/unbonding.go @@ -45,7 +45,7 @@ func (k Keeper) SetUnbondingType(ctx context.Context, id uint64, unbondingType t // GetUnbondingDelegationByUnbondingID returns a unbonding delegation that has an unbonding delegation entry with a certain ID func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint64) (ubd types.UnbondingDelegation, err error) { - ubdKey, err := k.UnbondingIndex.Get(ctx, id) + ubdKey, err := k.UnbondingIndex.Get(ctx, id) // ubdKey => [UnbondingDelegationKey(Prefix)+len(delAddr)+delAddr+len(valAddr)+valAddr] if err != nil { if errors.Is(err, collections.ErrNotFound) { return types.UnbondingDelegation{}, types.ErrNoUnbondingDelegation @@ -57,7 +57,9 @@ func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint return types.UnbondingDelegation{}, types.ErrNoUnbondingDelegation } + // remove prefix bytes and length bytes (since ubdKey obtained is prefixed by UnbondingDelegationKey prefix and length of the address) delAddr := ubdKey[2 : (len(ubdKey)/2)+1] + // remvoe prefix length bytes valAddr := ubdKey[2+len(ubdKey)/2:] ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr)) From ccc4a1b9d06500166cf085f2e2149880f223b04e Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 28 Aug 2023 13:46:09 +0530 Subject: [PATCH 23/23] spell correct --- x/staking/keeper/unbonding.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/staking/keeper/unbonding.go b/x/staking/keeper/unbonding.go index 55c6b1ecec95..b3507526b207 100644 --- a/x/staking/keeper/unbonding.go +++ b/x/staking/keeper/unbonding.go @@ -59,7 +59,7 @@ func (k Keeper) GetUnbondingDelegationByUnbondingID(ctx context.Context, id uint // remove prefix bytes and length bytes (since ubdKey obtained is prefixed by UnbondingDelegationKey prefix and length of the address) delAddr := ubdKey[2 : (len(ubdKey)/2)+1] - // remvoe prefix length bytes + // remove prefix length bytes valAddr := ubdKey[2+len(ubdKey)/2:] ubd, err = k.UnbondingDelegations.Get(ctx, collections.Join(delAddr, valAddr))