From 781d284bf457ef6e4203db9f1f708b8ada49e20c Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 26 Jul 2023 13:03:06 +0530 Subject: [PATCH 01/11] WIP: migrate delegation key to collections --- x/staking/keeper/delegation.go | 6 +++--- x/staking/keeper/keeper.go | 9 +++++++++ x/staking/types/keys.go | 12 ++++++------ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index e5849b8daa8e..d32f60f543b7 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "cosmossdk.io/collections" corestore "cosmossdk.io/core/store" errorsmod "cosmossdk.io/errors" "cosmossdk.io/math" @@ -134,14 +135,13 @@ func (k Keeper) SetDelegation(ctx context.Context, delegation types.Delegation) return err } - store := k.storeService.OpenKVStore(ctx) - b := types.MustMarshalDelegation(k.cdc, delegation) - err = store.Set(types.GetDelegationKey(delegatorAddress, valAddr), b) + err = k.Delegations.Set(ctx, collections.Join(sdk.AccAddress(delegatorAddress), sdk.ValAddress(valAddr)), delegation) if err != nil { return err } // set the delegation in validator delegator index + store := k.storeService.OpenKVStore(ctx) return store.Set(types.GetDelegationsByValKey(valAddr, delegatorAddress), []byte{}) } diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 8154b1b53264..d2cd33c9316b 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -36,6 +36,7 @@ type Keeper struct { HistoricalInfo collections.Map[uint64, types.HistoricalInfo] LastTotalPower collections.Item[math.Int] ValidatorUpdates collections.Item[types.ValidatorUpdates] + Delegations collections.Map[collections.Pair[sdk.AccAddress, sdk.ValAddress], types.Delegation] } // NewKeeper creates a new staking Keeper instance @@ -79,6 +80,14 @@ func NewKeeper( LastTotalPower: collections.NewItem(sb, types.LastTotalPowerKey, "last_total_power", sdk.IntValue), HistoricalInfo: collections.NewMap(sb, types.HistoricalInfoKey, "historical_info", collections.Uint64Key, codec.CollValue[types.HistoricalInfo](cdc)), ValidatorUpdates: collections.NewItem(sb, types.ValidatorUpdatesKey, "validator_updates", codec.CollValue[types.ValidatorUpdates](cdc)), + Delegations: collections.NewMap( + sb, types.DelegationKey, "delegations", + collections.PairKeyCodec( + sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), + sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), + ), + codec.CollValue[types.Delegation](cdc), + ), } schema, err := sb.Build() diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 8de7195ee6bf..433c1a9f9f9f 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 = collections.NewPrefix(49) // 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 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 From 31031ef02c79383a2e309995a8fb0ffe8b70e5f3 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 27 Jul 2023 19:18:08 +0530 Subject: [PATCH 02/11] fix lint --- x/staking/keeper/keeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index d2cd33c9316b..d89b4a15ed6f 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -83,8 +83,8 @@ func NewKeeper( Delegations: collections.NewMap( sb, types.DelegationKey, "delegations", collections.PairKeyCodec( - sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), - sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), + sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility + sdk.LengthPrefixedAddressKey(sdk.ValAddressKey), // nolint: staticcheck // sdk.LengthPrefixedAddressKey is needed to retain state compatibility ), codec.CollValue[types.Delegation](cdc), ), From 539cf24cc84f33bf01caa9c61951b5717e1abce2 Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 28 Jul 2023 20:26:54 +0530 Subject: [PATCH 03/11] delegations to collections --- x/staking/app_test.go | 7 +- x/staking/keeper/alias_functions.go | 30 +++-- x/staking/keeper/delegation.go | 125 +++++++-------------- x/staking/keeper/delegation_test.go | 23 ++-- x/staking/keeper/grpc_query.go | 62 ++++------ x/staking/keeper/msg_server_test.go | 7 +- x/staking/keeper/query_utils.go | 54 +++++---- x/staking/keeper/slash.go | 3 +- x/staking/migrations/v2/keys.go | 23 +++- x/staking/migrations/v2/store_test.go | 2 +- x/staking/migrations/v5/keys.go | 11 ++ x/staking/migrations/v5/migrations_test.go | 4 +- x/staking/simulation/decoder_test.go | 3 - x/staking/testutil/helpers.go | 3 +- x/staking/types/keys.go | 11 -- 15 files changed, 158 insertions(+), 210 deletions(-) diff --git a/x/staking/app_test.go b/x/staking/app_test.go index ae68287c40a7..b0ef33359b35 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -7,6 +7,7 @@ import ( cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" "github.com/stretchr/testify/require" + "cosmossdk.io/collections" "cosmossdk.io/depinject" "cosmossdk.io/log" "cosmossdk.io/math" @@ -115,7 +116,7 @@ func TestStakingMsgs(t *testing.T) { ctxCheck = app.BaseApp.NewContext(true) require.True(t, sdk.Coins{genCoin.Sub(bondCoin)}.Equal(bankKeeper.GetAllBalances(ctxCheck, addr2))) - _, err = stakingKeeper.GetDelegation(ctxCheck, addr2, sdk.ValAddress(addr1)) + _, err = stakingKeeper.Delegations.Get(ctxCheck, collections.Join(addr2, sdk.ValAddress(addr1))) require.NoError(t, err) // begin unbonding @@ -126,8 +127,8 @@ func TestStakingMsgs(t *testing.T) { // delegation should exist anymore ctxCheck = app.BaseApp.NewContext(true) - _, err = stakingKeeper.GetDelegation(ctxCheck, addr2, sdk.ValAddress(addr1)) - require.ErrorIs(t, err, types.ErrNoDelegation) + _, err = stakingKeeper.Delegations.Get(ctxCheck, collections.Join(addr2, sdk.ValAddress(addr1))) + require.ErrorIs(t, err, collections.ErrNotFound) // balance should be the same because bonding not yet complete require.True(t, sdk.Coins{genCoin.Sub(bondCoin)}.Equal(bankKeeper.GetAllBalances(ctxCheck, addr2))) diff --git a/x/staking/keeper/alias_functions.go b/x/staking/keeper/alias_functions.go index 4379043cc6d6..079dbed0ad2c 100644 --- a/x/staking/keeper/alias_functions.go +++ b/x/staking/keeper/alias_functions.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" @@ -115,7 +117,7 @@ func (k Keeper) GetValidatorSet() types.ValidatorSet { // Delegation gets the delegation interface for a particular set of delegator and validator addresses func (k Keeper) Delegation(ctx context.Context, addrDel sdk.AccAddress, addrVal sdk.ValAddress) (types.DelegationI, error) { - bond, err := k.GetDelegation(ctx, addrDel, addrVal) + bond, err := k.Delegations.Get(ctx, collections.Join(addrDel, addrVal)) if err != nil { return nil, err } @@ -127,25 +129,19 @@ func (k Keeper) Delegation(ctx context.Context, addrDel sdk.AccAddress, addrVal func (k Keeper) IterateDelegations(ctx context.Context, delAddr sdk.AccAddress, fn func(index int64, del types.DelegationI) (stop bool), ) error { - store := k.storeService.OpenKVStore(ctx) - delegatorPrefixKey := types.GetDelegationsKey(delAddr) - iterator, err := store.Iterator(delegatorPrefixKey, storetypes.PrefixEndBytes(delegatorPrefixKey)) - if err != nil { - return err - } - defer iterator.Close() - - for i := int64(0); iterator.Valid(); iterator.Next() { - del, err := types.UnmarshalDelegation(k.cdc, iterator.Value()) - if err != nil { - return err - } - - stop := fn(i, del) + var i int64 + rng := collections.NewPrefixedPairRange[sdk.AccAddress, sdk.ValAddress](delAddr) + err := k.Delegations.Walk(ctx, rng, func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (stop bool, err error) { + stop = fn(i, del) if stop { - break + return true, err } i++ + + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return err } return nil diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 93b7059fdd8c..ef51cecea990 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -18,48 +18,15 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking/types" ) -// GetDelegation returns a specific delegation. -func (k Keeper) GetDelegation(ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (types.Delegation, error) { - store := k.storeService.OpenKVStore(ctx) - key := types.GetDelegationKey(delAddr, valAddr) - - value, err := store.Get(key) - if err != nil { - return types.Delegation{}, err - } - - if value == nil { - return types.Delegation{}, types.ErrNoDelegation - } - - return types.UnmarshalDelegation(k.cdc, value) -} - -// IterateAllDelegations iterates through all of the delegations. -func (k Keeper) IterateAllDelegations(ctx context.Context, cb func(delegation types.Delegation) (stop bool)) error { - store := k.storeService.OpenKVStore(ctx) - iterator, err := store.Iterator(types.DelegationKey, storetypes.PrefixEndBytes(types.DelegationKey)) - if err != nil { - return err - } - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - delegation := types.MustUnmarshalDelegation(k.cdc, iterator.Value()) - if cb(delegation) { - break - } - } - - return nil -} - // GetAllDelegations returns all delegations used during genesis dump. -func (k Keeper) GetAllDelegations(ctx context.Context) (delegations []types.Delegation, err error) { - err = k.IterateAllDelegations(ctx, func(delegation types.Delegation) bool { - delegations = append(delegations, delegation) - return false - }) +func (k Keeper) GetAllDelegations(ctx context.Context) ([]types.Delegation, error) { + var delegations types.Delegations + err := k.Delegations.Walk(ctx, nil, + func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], delegation types.Delegation) (stop bool, err error) { + delegations = append(delegations, delegation) + return false, nil + }, + ) return delegations, err } @@ -67,22 +34,15 @@ func (k Keeper) GetAllDelegations(ctx context.Context) (delegations []types.Dele // GetValidatorDelegations returns all delegations to a specific validator. // Useful for querier. func (k Keeper) GetValidatorDelegations(ctx context.Context, valAddr sdk.ValAddress) ([]types.Delegation, error) { - store := k.storeService.OpenKVStore(ctx) - var delegations []types.Delegation rng := collections.NewPrefixedPairRange[sdk.ValAddress, sdk.AccAddress](valAddr) err := k.DelegationsByValidator.Walk(ctx, rng, func(key collections.Pair[sdk.ValAddress, sdk.AccAddress], _ []byte) (stop bool, err error) { - var delegation types.Delegation valAddr, delAddr := key.K1(), key.K2() - bz, err := store.Get(types.GetDelegationKey(delAddr, valAddr)) + delegation, err := k.Delegations.Get(ctx, collections.Join(delAddr, valAddr)) if err != nil { return true, err } - if err := k.cdc.Unmarshal(bz, &delegation); err != nil { - return true, err - } - delegations = append(delegations, delegation) return false, nil @@ -96,25 +56,22 @@ func (k Keeper) GetValidatorDelegations(ctx context.Context, valAddr sdk.ValAddr // GetDelegatorDelegations returns a given amount of all the delegations from a // delegator. -func (k Keeper) GetDelegatorDelegations(ctx context.Context, delegator sdk.AccAddress, maxRetrieve uint16) (delegations []types.Delegation, err error) { - delegations = make([]types.Delegation, maxRetrieve) - store := k.storeService.OpenKVStore(ctx) - delegatorPrefixKey := types.GetDelegationsKey(delegator) - - iterator, err := store.Iterator(delegatorPrefixKey, storetypes.PrefixEndBytes(delegatorPrefixKey)) - if err != nil { - return delegations, err - } - defer iterator.Close() - - i := 0 - for ; iterator.Valid() && i < int(maxRetrieve); iterator.Next() { - delegation, err := types.UnmarshalDelegation(k.cdc, iterator.Value()) - if err != nil { - return delegations, err +func (k Keeper) GetDelegatorDelegations(ctx context.Context, delegator sdk.AccAddress, maxRetrieve uint16) ([]types.Delegation, error) { + delegations := make([]types.Delegation, maxRetrieve) + + var i uint16 + rng := collections.NewPrefixedPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + err := k.Delegations.Walk(ctx, rng, func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (stop bool, err error) { + if i >= maxRetrieve { + return true, nil } - delegations[i] = delegation + delegations[i] = del i++ + + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return nil, err } return delegations[:i], nil // trim if the array length < maxRetrieve @@ -158,8 +115,7 @@ func (k Keeper) RemoveDelegation(ctx context.Context, delegation types.Delegatio return err } - store := k.storeService.OpenKVStore(ctx) - err = store.Delete(types.GetDelegationKey(delegatorAddress, valAddr)) + err = k.Delegations.Remove(ctx, collections.Join(sdk.AccAddress(delegatorAddress), sdk.ValAddress(valAddr))) if err != nil { return err } @@ -317,23 +273,18 @@ func (k Keeper) GetDelegatorBonded(ctx context.Context, delegator sdk.AccAddress // IterateDelegatorDelegations iterates through one delegator's delegations. func (k Keeper) IterateDelegatorDelegations(ctx context.Context, delegator sdk.AccAddress, cb func(delegation types.Delegation) (stop bool)) error { - store := k.storeService.OpenKVStore(ctx) - prefix := types.GetDelegationsKey(delegator) - iterator, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) - if err != nil { + rng := collections.NewPrefixedPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + err := k.Delegations.Walk(ctx, rng, func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (stop bool, err error) { + if cb(del) { + return true, nil + } + + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { return err } - defer iterator.Close() - for ; iterator.Valid(); iterator.Next() { - delegation, err := types.UnmarshalDelegation(k.cdc, iterator.Value()) - if err != nil { - return err - } - if cb(delegation) { - break - } - } return nil } @@ -868,11 +819,11 @@ func (k Keeper) Delegate( } // Get or create the delegation object and call the appropriate hook if present - delegation, err := k.GetDelegation(ctx, delAddr, validator.GetOperator()) + delegation, err := k.Delegations.Get(ctx, collections.Join(delAddr, validator.GetOperator())) if err == nil { // found err = k.Hooks().BeforeDelegationSharesModified(ctx, delAddr, validator.GetOperator()) - } else if errors.Is(err, types.ErrNoDelegation) { + } else if errors.Is(err, collections.ErrNotFound) { // not found delAddrStr, err1 := k.authKeeper.AddressCodec().BytesToString(delAddr) if err1 != nil { @@ -970,8 +921,8 @@ func (k Keeper) Unbond( ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress, shares math.LegacyDec, ) (amount math.Int, err error) { // check if a delegation object exists in the store - delegation, err := k.GetDelegation(ctx, delAddr, valAddr) - if errors.Is(err, types.ErrNoDelegation) { + delegation, err := k.Delegations.Get(ctx, collections.Join(delAddr, valAddr)) + if errors.Is(err, collections.ErrNotFound) { return amount, types.ErrNoDelegatorForAddress } else if err != nil { return amount, err @@ -1346,7 +1297,7 @@ func (k Keeper) ValidateUnbondAmount( return shares, err } - del, err := k.GetDelegation(ctx, delAddr, valAddr) + del, err := k.Delegations.Get(ctx, collections.Join(delAddr, valAddr)) if err != nil { return shares, err } diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 26fe6fbebb6b..8910e0b504a1 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -5,6 +5,7 @@ import ( "github.com/golang/mock/gomock" + "cosmossdk.io/collections" "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/codec/address" @@ -45,19 +46,19 @@ func (s *KeeperTestSuite) TestDelegation() { bond1to1 := stakingtypes.NewDelegation(addrDels[0].String(), valAddrs[0].String(), math.LegacyNewDec(9)) // check the empty keeper first - _, err := keeper.GetDelegation(ctx, addrDels[0], valAddrs[0]) - require.ErrorIs(err, stakingtypes.ErrNoDelegation) + _, err := keeper.Delegations.Get(ctx, collections.Join(addrDels[0], valAddrs[0])) + require.ErrorIs(err, collections.ErrNotFound) // set and retrieve a record require.NoError(keeper.SetDelegation(ctx, bond1to1)) - resBond, err := keeper.GetDelegation(ctx, addrDels[0], valAddrs[0]) + resBond, err := keeper.Delegations.Get(ctx, collections.Join(addrDels[0], valAddrs[0])) require.NoError(err) require.Equal(bond1to1, resBond) // modify a records, save, and retrieve bond1to1.Shares = math.LegacyNewDec(99) require.NoError(keeper.SetDelegation(ctx, bond1to1)) - resBond, err = keeper.GetDelegation(ctx, addrDels[0], valAddrs[0]) + resBond, err = keeper.Delegations.Get(ctx, collections.Join(addrDels[0], valAddrs[0])) require.NoError(err) require.Equal(bond1to1, resBond) @@ -131,8 +132,8 @@ func (s *KeeperTestSuite) TestDelegation() { // delete a record require.NoError(keeper.RemoveDelegation(ctx, bond2to3)) - _, err = keeper.GetDelegation(ctx, addrDels[1], valAddrs[2]) - require.ErrorIs(err, stakingtypes.ErrNoDelegation) + _, err = keeper.Delegations.Get(ctx, collections.Join(addrDels[1], valAddrs[2])) + require.ErrorIs(err, collections.ErrNotFound) resBonds, err = keeper.GetDelegatorDelegations(ctx, addrDels[1], 5) require.NoError(err) require.Equal(2, len(resBonds)) @@ -146,10 +147,10 @@ func (s *KeeperTestSuite) TestDelegation() { // delete all the records from delegator 2 require.NoError(keeper.RemoveDelegation(ctx, bond2to1)) require.NoError(keeper.RemoveDelegation(ctx, bond2to2)) - _, err = keeper.GetDelegation(ctx, addrDels[1], valAddrs[0]) - require.ErrorIs(err, stakingtypes.ErrNoDelegation) - _, err = keeper.GetDelegation(ctx, addrDels[1], valAddrs[1]) - require.ErrorIs(err, stakingtypes.ErrNoDelegation) + _, err = keeper.Delegations.Get(ctx, collections.Join(addrDels[1], valAddrs[0])) + require.ErrorIs(err, collections.ErrNotFound) + _, err = keeper.Delegations.Get(ctx, collections.Join(addrDels[1], valAddrs[1])) + require.ErrorIs(err, collections.ErrNotFound) resBonds, err = keeper.GetDelegatorDelegations(ctx, addrDels[1], 5) require.NoError(err) require.Equal(0, len(resBonds)) @@ -385,7 +386,7 @@ func (s *KeeperTestSuite) TestUnbondDelegation() { require.NoError(err) require.Equal(bondTokens, amount) // shares to be added to an unbonding delegation - delegation, err = keeper.GetDelegation(ctx, delAddrs[0], valAddrs[0]) + delegation, err = keeper.Delegations.Get(ctx, collections.Join(delAddrs[0], valAddrs[0])) require.NoError(err) validator, err = keeper.GetValidator(ctx, valAddrs[0]) require.NoError(err) diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index 39f628dee83a..f5078adc4726 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -101,8 +101,6 @@ func (k Querier) ValidatorDelegations(ctx context.Context, req *types.QueryValid return nil, err } - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - var ( dels types.Delegations pageRes *query.PageResponse @@ -111,16 +109,14 @@ func (k Querier) ValidatorDelegations(ctx context.Context, req *types.QueryValid dels, pageRes, err = query.CollectionPaginate(ctx, k.DelegationsByValidator, req.Pagination, func(key collections.Pair[sdk.ValAddress, sdk.AccAddress], _ []byte) (types.Delegation, error) { valAddr, delAddr := key.K1(), key.K2() - var delegation types.Delegation - - bz := store.Get(types.GetDelegationKey(delAddr, valAddr)) - err = k.cdc.Unmarshal(bz, &delegation) + delegation, err := k.Delegations.Get(ctx, collections.Join(delAddr, valAddr)) if err != nil { return types.Delegation{}, err } return delegation, nil - }, query.WithCollectionPaginationPairPrefix[sdk.ValAddress, sdk.AccAddress](valAddr)) + }, query.WithCollectionPaginationPairPrefix[sdk.ValAddress, sdk.AccAddress](valAddr), + ) if err != nil { delegations, pageResponse, err := k.getValidatorDelegationsLegacy(ctx, req) @@ -229,7 +225,7 @@ func (k Querier) Delegation(ctx context.Context, req *types.QueryDelegationReque return nil, err } - delegation, err := k.GetDelegation(ctx, delAddr, valAddr) + delegation, err := k.Delegations.Get(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) if err != nil { return nil, status.Errorf( codes.NotFound, @@ -288,23 +284,17 @@ func (k Querier) DelegatorDelegations(ctx context.Context, req *types.QueryDeleg if req.DelegatorAddr == "" { return nil, status.Error(codes.InvalidArgument, "delegator address cannot be empty") } - var delegations types.Delegations delAddr, err := k.authKeeper.AddressCodec().StringToBytes(req.DelegatorAddr) if err != nil { return nil, err } - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) - delStore := prefix.NewStore(store, types.GetDelegationsKey(delAddr)) - pageRes, err := query.Paginate(delStore, req.Pagination, func(key, value []byte) error { - delegation, err := types.UnmarshalDelegation(k.cdc, value) - if err != nil { - return err - } - delegations = append(delegations, delegation) - return nil - }) + delegations, pageRes, err := query.CollectionPaginate(ctx, k.Delegations, req.Pagination, + func(_ collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (types.Delegation, error) { + return del, nil + }, query.WithCollectionPaginationPairPrefix[sdk.AccAddress, sdk.ValAddress](delAddr), + ) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -442,32 +432,26 @@ func (k Querier) DelegatorValidators(ctx context.Context, req *types.QueryDelega } var validators types.Validators - store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) delAddr, err := k.authKeeper.AddressCodec().StringToBytes(req.DelegatorAddr) if err != nil { return nil, err } - delStore := prefix.NewStore(store, types.GetDelegationsKey(delAddr)) - pageRes, err := query.Paginate(delStore, req.Pagination, func(key, value []byte) error { - delegation, err := types.UnmarshalDelegation(k.cdc, value) - if err != nil { - return err - } - - valAddr, err := k.validatorAddressCodec.StringToBytes(delegation.GetValidatorAddr()) - if err != nil { - return err - } - - validator, err := k.GetValidator(ctx, valAddr) - if err != nil { - return err - } + _, pageRes, err := query.CollectionPaginate(ctx, k.Delegations, req.Pagination, + func(_ collections.Pair[sdk.AccAddress, sdk.ValAddress], delegation types.Delegation) (types.Delegation, error) { + valAddr, err := k.validatorAddressCodec.StringToBytes(delegation.GetValidatorAddr()) + if err != nil { + return types.Delegation{}, err + } + validator, err := k.GetValidator(ctx, valAddr) + if err != nil { + return types.Delegation{}, err + } - validators = append(validators, validator) - return nil - }) + validators = append(validators, validator) + return types.Delegation{}, nil + }, query.WithCollectionPaginationPairPrefix[sdk.AccAddress, sdk.ValAddress](delAddr), + ) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index df339ae371cb..bc3fa965e338 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -6,6 +6,7 @@ import ( "github.com/golang/mock/gomock" + "cosmossdk.io/collections" "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/codec/address" @@ -568,7 +569,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { shares := math.LegacyNewDec(100) del := stakingtypes.NewDelegation(Addr.String(), srcValAddr.String(), shares) require.NoError(keeper.SetDelegation(ctx, del)) - _, err = keeper.GetDelegation(ctx, Addr, srcValAddr) + _, err = keeper.Delegations.Get(ctx, collections.Join(Addr, srcValAddr)) require.NoError(err) testCases := []struct { @@ -722,7 +723,7 @@ func (s *KeeperTestSuite) TestMsgUndelegate() { shares := math.LegacyNewDec(100) del := stakingtypes.NewDelegation(Addr.String(), ValAddr.String(), shares) require.NoError(keeper.SetDelegation(ctx, del)) - _, err = keeper.GetDelegation(ctx, Addr, ValAddr) + _, err = keeper.Delegations.Get(ctx, collections.Join(Addr, ValAddr)) require.NoError(err) testCases := []struct { @@ -847,7 +848,7 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { shares := math.LegacyNewDec(100) del := stakingtypes.NewDelegation(Addr.String(), ValAddr.String(), shares) require.NoError(keeper.SetDelegation(ctx, del)) - resDel, err := keeper.GetDelegation(ctx, Addr, ValAddr) + resDel, err := keeper.Delegations.Get(ctx, collections.Join(Addr, ValAddr)) require.NoError(err) require.Equal(del, resDel) diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index 6bb16da3fcd0..c4a320c30536 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" @@ -14,31 +16,31 @@ func (k Keeper) GetDelegatorValidators( ctx context.Context, delegatorAddr sdk.AccAddress, maxRetrieve uint32, ) (types.Validators, error) { validators := make([]types.Validator, maxRetrieve) - store := k.storeService.OpenKVStore(ctx) - delegatorPrefixKey := types.GetDelegationsKey(delegatorAddr) - - iterator, err := store.Iterator(delegatorPrefixKey, storetypes.PrefixEndBytes(delegatorPrefixKey)) // smallest to largest - if err != nil { - return nil, err - } - defer iterator.Close() - i := 0 - for ; iterator.Valid() && i < int(maxRetrieve); iterator.Next() { - delegation := types.MustUnmarshalDelegation(k.cdc, iterator.Value()) + var i uint32 + rng := collections.NewPrefixedPairRange[sdk.AccAddress, sdk.ValAddress](delegatorAddr) + err := k.Delegations.Walk(ctx, rng, func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (stop bool, err error) { + if i >= maxRetrieve { + return true, nil + } - valAddr, err := k.validatorAddressCodec.StringToBytes(delegation.GetValidatorAddr()) + valAddr, err := k.validatorAddressCodec.StringToBytes(del.GetValidatorAddr()) if err != nil { - return nil, err + return true, err } validator, err := k.GetValidator(ctx, valAddr) if err != nil { - return nil, err + return true, err } validators[i] = validator i++ + + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return nil, err } return validators[:i], nil // trim @@ -48,7 +50,7 @@ func (k Keeper) GetDelegatorValidators( func (k Keeper) GetDelegatorValidator( ctx context.Context, delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress, ) (validator types.Validator, err error) { - delegation, err := k.GetDelegation(ctx, delegatorAddr, validatorAddr) + delegation, err := k.Delegations.Get(ctx, collections.Join(delegatorAddr, validatorAddr)) if err != nil { return validator, err } @@ -65,23 +67,17 @@ func (k Keeper) GetDelegatorValidator( func (k Keeper) GetAllDelegatorDelegations(ctx context.Context, delegator sdk.AccAddress) ([]types.Delegation, error) { delegations := make([]types.Delegation, 0) - store := k.storeService.OpenKVStore(ctx) - delegatorPrefixKey := types.GetDelegationsKey(delegator) + var i int64 + rng := collections.NewPrefixedPairRange[sdk.AccAddress, sdk.ValAddress](delegator) + err := k.Delegations.Walk(ctx, rng, func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (stop bool, err error) { + delegations = append(delegations, del) + i++ - iterator, err := store.Iterator(delegatorPrefixKey, storetypes.PrefixEndBytes(delegatorPrefixKey)) // smallest to largest - if err != nil { + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { return nil, err } - defer iterator.Close() - - for i := 0; iterator.Valid(); iterator.Next() { - delegation, err := types.UnmarshalDelegation(k.cdc, iterator.Value()) - if err != nil { - return nil, err - } - delegations = append(delegations, delegation) - i++ - } return delegations, nil } diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index d23b9b720a8e..06332a306d41 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "cosmossdk.io/collections" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" @@ -319,7 +320,7 @@ func (k Keeper) SlashRedelegation(ctx context.Context, srcValidator types.Valida panic(err) } - delegation, err := k.GetDelegation(ctx, delegatorAddress, valDstAddr) + delegation, err := k.Delegations.Get(ctx, collections.Join(sdk.AccAddress(delegatorAddress), sdk.ValAddress(valDstAddr))) if err != nil { // If deleted, delegation has zero shares, and we can't unbond any more continue diff --git a/x/staking/migrations/v2/keys.go b/x/staking/migrations/v2/keys.go index 57ccb0aebeea..a7a757c0bb8c 100644 --- a/x/staking/migrations/v2/keys.go +++ b/x/staking/migrations/v2/keys.go @@ -1,15 +1,34 @@ package v2 -import "strconv" +import ( + "strconv" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" +) const ( // ModuleName is the name of the module ModuleName = "staking" ) -var HistoricalInfoKey = []byte{0x50} // prefix for the historical info +var ( + DelegationKey = []byte{0x31} // prefix for the delegation + HistoricalInfoKey = []byte{0x50} // prefix for the historical info +) // GetHistoricalInfoKey returns a key prefix for indexing HistoricalInfo objects. func GetHistoricalInfoKey(height int64) []byte { return append(HistoricalInfoKey, []byte(strconv.FormatInt(height, 10))...) } + +// GetDelegationKey creates the key for delegator bond with validator +// VALUE: staking/Delegation +func GetDelegationKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { + return append(GetDelegationsKey(delAddr), address.MustLengthPrefix(valAddr)...) +} + +// gets the prefix for a delegator for all validators +func GetDelegationsKey(delAddr sdk.AccAddress) []byte { + return append(DelegationKey, address.MustLengthPrefix(delAddr.Bytes())...) +} diff --git a/x/staking/migrations/v2/store_test.go b/x/staking/migrations/v2/store_test.go index 20c55e5bfcbb..06e72b48116a 100644 --- a/x/staking/migrations/v2/store_test.go +++ b/x/staking/migrations/v2/store_test.go @@ -69,7 +69,7 @@ func TestStoreMigration(t *testing.T) { { "DelegationKey", v1.GetDelegationKey(addr4, valAddr1), - types.GetDelegationKey(addr4, valAddr1), + v2.GetDelegationKey(addr4, valAddr1), }, { "UnbondingDelegationKey", diff --git a/x/staking/migrations/v5/keys.go b/x/staking/migrations/v5/keys.go index 69e763ce61c8..44bc03998c56 100644 --- a/x/staking/migrations/v5/keys.go +++ b/x/staking/migrations/v5/keys.go @@ -101,3 +101,14 @@ func ParseDelegationsByValKey(bz []byte) (sdk.ValAddress, sdk.AccAddress, error) return val, del, nil } + +// GetDelegationKey creates the key for delegator bond with validator +// VALUE: staking/Delegation +func GetDelegationKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { + return append(GetDelegationsKey(delAddr), address.MustLengthPrefix(valAddr)...) +} + +// GetDelegationsKey creates the prefix for a delegator for all validators +func GetDelegationsKey(delAddr sdk.AccAddress) []byte { + return append(DelegationKey, address.MustLengthPrefix(delAddr)...) +} diff --git a/x/staking/migrations/v5/migrations_test.go b/x/staking/migrations/v5/migrations_test.go index c27180eb6517..846ad869822c 100644 --- a/x/staking/migrations/v5/migrations_test.go +++ b/x/staking/migrations/v5/migrations_test.go @@ -91,7 +91,7 @@ func TestDelegationsByValidatorMigrations(t *testing.T) { for i := 1; i < 11; i++ { del1 := stakingtypes.NewDelegation(accAddrs[i].String(), valAddrs[0].String(), sdkmath.LegacyNewDec(100)) - store.Set(stakingtypes.GetDelegationKey(accAddrs[i], valAddrs[0]), stakingtypes.MustMarshalDelegation(cdc, del1)) + store.Set(v5.GetDelegationKey(accAddrs[i], valAddrs[0]), stakingtypes.MustMarshalDelegation(cdc, del1)) addedDels = append(addedDels, del1) } @@ -120,7 +120,7 @@ func getValDelegations(ctx sdk.Context, cdc codec.Codec, storeKey storetypes.Sto panic(err) } - bz := store.Get(stakingtypes.GetDelegationKey(delAddr, valAddr)) + bz := store.Get(v5.GetDelegationKey(delAddr, valAddr)) cdc.MustUnmarshal(bz, &delegation) diff --git a/x/staking/simulation/decoder_test.go b/x/staking/simulation/decoder_test.go index 0179794929b1..b49b371d250c 100644 --- a/x/staking/simulation/decoder_test.go +++ b/x/staking/simulation/decoder_test.go @@ -31,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) - del := types.NewDelegation(delAddr1.String(), valAddr1.String(), math.LegacyOneDec()) 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() @@ -42,7 +41,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: types.GetDelegationKey(delAddr1, valAddr1), Value: cdc.MustMarshal(&del)}, {Key: types.GetUBDKey(delAddr1, valAddr1), Value: cdc.MustMarshal(&ubd)}, {Key: types.GetREDKey(delAddr1, valAddr1, valAddr1), Value: cdc.MustMarshal(&red)}, {Key: []byte{0x99}, Value: []byte{0x99}}, @@ -56,7 +54,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)}, - {"Delegation", fmt.Sprintf("%v\n%v", del, del)}, {"UnbondingDelegation", fmt.Sprintf("%v\n%v", ubd, ubd)}, {"Redelegation", fmt.Sprintf("%v\n%v", red, red)}, {"other", ""}, diff --git a/x/staking/testutil/helpers.go b/x/staking/testutil/helpers.go index 2387952b6172..8ffa2c47eb5d 100644 --- a/x/staking/testutil/helpers.go +++ b/x/staking/testutil/helpers.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" + "cosmossdk.io/collections" "cosmossdk.io/math" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -121,7 +122,7 @@ func (sh *Helper) CheckValidator(addr sdk.ValAddress, status stakingtypes.BondSt // CheckDelegator asserts that a delegator exists func (sh *Helper) CheckDelegator(delegator sdk.AccAddress, val sdk.ValAddress, found bool) { - _, ok := sh.k.GetDelegation(sh.Ctx, delegator, val) + _, ok := sh.k.Delegations.Get(sh.Ctx, collections.Join(delegator, val)) require.Equal(sh.t, ok, found) } diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 9fce1371602a..9e49a89af4b8 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -209,17 +209,6 @@ func ParseValidatorQueueKey(bz []byte) (time.Time, int64, error) { return ts, int64(height), nil } -// GetDelegationKey creates the key for delegator bond with validator -// VALUE: staking/Delegation -func GetDelegationKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { - return append(GetDelegationsKey(delAddr), address.MustLengthPrefix(valAddr)...) -} - -// GetDelegationsKey creates the prefix for a delegator for all validators -func GetDelegationsKey(delAddr sdk.AccAddress) []byte { - return append(DelegationKey, address.MustLengthPrefix(delAddr)...) -} - // GetUBDKey creates the key for an unbonding delegation by delegator and validator addr // VALUE: staking/UnbondingDelegation func GetUBDKey(delAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { From 3629a50d4b1a6c0c7ad7da40d0b1c99a8b8ad6fe Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 28 Jul 2023 21:21:49 +0530 Subject: [PATCH 04/11] fix tests --- .../distribution/keeper/msg_server_test.go | 2 +- tests/integration/evidence/keeper/infraction_test.go | 3 ++- tests/integration/staking/keeper/grpc_query_test.go | 7 ++++--- tests/integration/staking/keeper/slash_test.go | 3 ++- .../staking/keeper/validator_bench_test.go | 11 +++++------ x/staking/keeper/delegation.go | 3 +++ 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/integration/distribution/keeper/msg_server_test.go b/tests/integration/distribution/keeper/msg_server_test.go index dcce82ad1955..21a738bc20ac 100644 --- a/tests/integration/distribution/keeper/msg_server_test.go +++ b/tests/integration/distribution/keeper/msg_server_test.go @@ -253,7 +253,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) { ValidatorAddress: f.valAddr.String(), }, expErr: true, - expErrMsg: "no delegation for (address, validator) tuple", + expErrMsg: "not found", }, { name: "validator with no delegations", diff --git a/tests/integration/evidence/keeper/infraction_test.go b/tests/integration/evidence/keeper/infraction_test.go index f716cf4f0101..e250bfb38248 100644 --- a/tests/integration/evidence/keeper/infraction_test.go +++ b/tests/integration/evidence/keeper/infraction_test.go @@ -11,6 +11,7 @@ import ( cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" "gotest.tools/v3/assert" + "cosmossdk.io/collections" "cosmossdk.io/core/appmodule" "cosmossdk.io/core/comet" "cosmossdk.io/log" @@ -243,7 +244,7 @@ func TestHandleDoubleSign(t *testing.T) { // require we be able to unbond now ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) - del, _ := f.stakingKeeper.GetDelegation(ctx, sdk.AccAddress(operatorAddr), operatorAddr) + del, _ := f.stakingKeeper.Delegations.Get(ctx, collections.Join(sdk.AccAddress(operatorAddr), operatorAddr)) validator, _ := f.stakingKeeper.GetValidator(ctx, operatorAddr) totalBond := validator.TokensFromShares(del.GetShares()).TruncateInt() tstaking.Ctx = ctx diff --git a/tests/integration/staking/keeper/grpc_query_test.go b/tests/integration/staking/keeper/grpc_query_test.go index c5500f232b98..aaf68b9816b3 100644 --- a/tests/integration/staking/keeper/grpc_query_test.go +++ b/tests/integration/staking/keeper/grpc_query_test.go @@ -8,6 +8,7 @@ import ( cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" "gotest.tools/v3/assert" + "cosmossdk.io/collections" "cosmossdk.io/math" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" @@ -289,7 +290,7 @@ func TestGRPCQueryDelegation(t *testing.T) { addrVal := vals[0].OperatorAddress valAddr, err := sdk.ValAddressFromBech32(addrVal) assert.NilError(t, err) - delegation, found := f.stakingKeeper.GetDelegation(ctx, addrAcc, valAddr) + delegation, found := f.stakingKeeper.Delegations.Get(ctx, collections.Join(addrAcc, valAddr)) assert.Assert(t, found) var req *types.QueryDelegationRequest @@ -358,7 +359,7 @@ func TestGRPCQueryDelegatorDelegations(t *testing.T) { addrVal1 := vals[0].OperatorAddress valAddr, err := sdk.ValAddressFromBech32(addrVal1) assert.NilError(t, err) - delegation, found := f.stakingKeeper.GetDelegation(ctx, addrAcc, valAddr) + delegation, found := f.stakingKeeper.Delegations.Get(ctx, collections.Join(addrAcc, valAddr)) assert.Assert(t, found) var req *types.QueryDelegatorDelegationsRequest @@ -438,7 +439,7 @@ func TestGRPCQueryValidatorDelegations(t *testing.T) { addrVal2 := valAddrs[4] valAddr, err := sdk.ValAddressFromBech32(addrVal1) assert.NilError(t, err) - delegation, found := f.stakingKeeper.GetDelegation(ctx, addrAcc, valAddr) + delegation, found := f.stakingKeeper.Delegations.Get(ctx, collections.Join(addrAcc, valAddr)) assert.Assert(t, found) var req *types.QueryValidatorDelegationsRequest diff --git a/tests/integration/staking/keeper/slash_test.go b/tests/integration/staking/keeper/slash_test.go index 7595e7034e72..245eec0cf4e4 100644 --- a/tests/integration/staking/keeper/slash_test.go +++ b/tests/integration/staking/keeper/slash_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "gotest.tools/v3/assert" + "cosmossdk.io/collections" "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/codec/address" @@ -167,7 +168,7 @@ func TestSlashRedelegation(t *testing.T) { assert.DeepEqual(t, math.NewInt(10), rd.Entries[0].InitialBalance) // shares decreased - del, found = f.stakingKeeper.GetDelegation(f.sdkCtx, addrDels[0], addrVals[1]) + del, found = f.stakingKeeper.Delegations.Get(f.sdkCtx, collections.Join(addrDels[0], addrVals[1])) assert.Assert(t, found) assert.Equal(t, int64(5), del.Shares.RoundInt64()) diff --git a/tests/integration/staking/keeper/validator_bench_test.go b/tests/integration/staking/keeper/validator_bench_test.go index 70f38055f259..7984e4b0db01 100644 --- a/tests/integration/staking/keeper/validator_bench_test.go +++ b/tests/integration/staking/keeper/validator_bench_test.go @@ -152,17 +152,16 @@ func updateValidatorDelegationsLegacy(f *fixture, existingValAddr, newValAddr sd } func updateValidatorDelegations(f *fixture, existingValAddr, newValAddr sdk.ValAddress) { - storeKey := f.keys[types.StoreKey] - cdc, k := f.cdc, f.stakingKeeper - - store := f.sdkCtx.KVStore(storeKey) + k := f.stakingKeeper rng := collections.NewPrefixedPairRange[sdk.ValAddress, sdk.AccAddress](existingValAddr) err := k.DelegationsByValidator.Walk(f.sdkCtx, rng, func(key collections.Pair[sdk.ValAddress, sdk.AccAddress], _ []byte) (stop bool, err error) { valAddr, delAddr := key.K1(), key.K2() - bz := store.Get(types.GetDelegationKey(delAddr, valAddr)) - delegation := types.MustUnmarshalDelegation(cdc, bz) + delegation, err := k.Delegations.Get(f.sdkCtx, collections.Join(delAddr, valAddr)) + if err != nil { + return true, err + } // remove old operator addr from delegation if err := k.RemoveDelegation(f.sdkCtx, delegation); err != nil { diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index ef51cecea990..9ee201c967a7 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -27,6 +27,9 @@ func (k Keeper) GetAllDelegations(ctx context.Context) ([]types.Delegation, erro return false, nil }, ) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return nil, err + } return delegations, err } From a03fd544ce7d49b20ba5ad49f103cd26f933b02e Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 1 Aug 2023 12:05:23 +0530 Subject: [PATCH 05/11] fix tests --- tests/integration/staking/keeper/grpc_query_test.go | 2 +- x/staking/keeper/delegation.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/staking/keeper/grpc_query_test.go b/tests/integration/staking/keeper/grpc_query_test.go index aaf68b9816b3..fbb4f4e87f64 100644 --- a/tests/integration/staking/keeper/grpc_query_test.go +++ b/tests/integration/staking/keeper/grpc_query_test.go @@ -224,7 +224,7 @@ func TestGRPCQueryDelegatorValidator(t *testing.T) { } }, false, - "no delegation for (address, validator) tuple", + "not found", }, { "empty delegator address", diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 9ee201c967a7..da4ced71f25a 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -31,7 +31,7 @@ func (k Keeper) GetAllDelegations(ctx context.Context) ([]types.Delegation, erro return nil, err } - return delegations, err + return delegations, nil } // GetValidatorDelegations returns all delegations to a specific validator. From 2a21cc3ca4dab45eede276f6fe99bae8caad58e0 Mon Sep 17 00:00:00 2001 From: atheesh Date: Sat, 5 Aug 2023 18:04:16 +0530 Subject: [PATCH 06/11] review changes --- CHANGELOG.md | 4 +++- x/staking/keeper/grpc_query.go | 12 ++++++++---- x/staking/testutil/helpers.go | 7 ------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ca0b3f5c998..806fb2da5ea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `DelegationKey`: + * remove from `types`: `GetDelegationKey`, `GetDelegationsKey` * (x/staking) [#17256](https://github.com/cosmos/cosmos-sdk/pull/17256) Use collections for `UnbondingID`. * (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `ValidatorByConsAddr`: * remove from `types`: `GetValidatorByConsAddrKey` @@ -71,7 +73,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * remove from `Keeper`: `GetPreviousProposerConsAddr`, `SetPreviousProposerConsAddr`, `GetValidatorHistoricalReferenceCount`, `GetValidatorSlashEvent`, `SetValidatorSlashEvent`. * (x/feegrant) [#16535](https://github.com/cosmos/cosmos-sdk/pull/16535) Use collections for `FeeAllowance`, `FeeAllowanceQueue`. * (x/staking) [#17063](https://github.com/cosmos/cosmos-sdk/pull/17063) Use collections for `HistoricalInfo`: - * remove `Keeper`: `GetHistoricalInfo`, `SetHistoricalInfo`, + * remove `Keeper`: `GetHistoricalInfo`, `SetHistoricalInfo` * (x/staking) [#17062](https://github.com/cosmos/cosmos-sdk/pull/17062) Use collections for `ValidatorUpdates`: * remove `Keeper`: `SetValidatorUpdates`, `GetValidatorUpdates` * (x/slashing) [#17023](https://github.com/cosmos/cosmos-sdk/pull/17023) Use collections for `ValidatorSigningInfo`: diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index f5078adc4726..0fdc84be4fd1 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "errors" "strings" "google.golang.org/grpc/codes" @@ -227,10 +228,13 @@ func (k Querier) Delegation(ctx context.Context, req *types.QueryDelegationReque delegation, err := k.Delegations.Get(ctx, collections.Join(sdk.AccAddress(delAddr), sdk.ValAddress(valAddr))) if err != nil { - return nil, status.Errorf( - codes.NotFound, - "delegation with delegator %s not found for validator %s", - req.DelegatorAddr, req.ValidatorAddr) + if errors.Is(err, collections.ErrNotFound) { + return nil, status.Errorf( + codes.NotFound, + "delegation with delegator %s not found for validator %s", + req.DelegatorAddr, req.ValidatorAddr) + } + return nil, err } delResponse, err := delegationToDelegationResponse(ctx, k.Keeper, delegation) diff --git a/x/staking/testutil/helpers.go b/x/staking/testutil/helpers.go index 8ffa2c47eb5d..be09bc2625da 100644 --- a/x/staking/testutil/helpers.go +++ b/x/staking/testutil/helpers.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/require" - "cosmossdk.io/collections" "cosmossdk.io/math" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -120,12 +119,6 @@ func (sh *Helper) CheckValidator(addr sdk.ValAddress, status stakingtypes.BondSt return v } -// CheckDelegator asserts that a delegator exists -func (sh *Helper) CheckDelegator(delegator sdk.AccAddress, val sdk.ValAddress, found bool) { - _, ok := sh.k.Delegations.Get(sh.Ctx, collections.Join(delegator, val)) - require.Equal(sh.t, ok, found) -} - // TurnBlock calls EndBlocker and updates the block time func (sh *Helper) TurnBlock(newTime time.Time) sdk.Context { sh.Ctx = sh.Ctx.WithBlockTime(newTime) From 346d5af40d70380c7325762ac454151f0a345d83 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 7 Aug 2023 15:55:54 +0530 Subject: [PATCH 07/11] nit --- x/staking/keeper/grpc_query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index 0fdc84be4fd1..126a0026f37f 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -234,7 +234,7 @@ func (k Querier) Delegation(ctx context.Context, req *types.QueryDelegationReque "delegation with delegator %s not found for validator %s", req.DelegatorAddr, req.ValidatorAddr) } - return nil, err + return nil, status.Error(codes.Internal, err.Error()) } delResponse, err := delegationToDelegationResponse(ctx, k.Keeper, delegation) From 04718069e87ef42dc1c86ab05cd2d1ccfc289509 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 8 Aug 2023 19:15:43 +0530 Subject: [PATCH 08/11] review changes --- go.mod | 2 +- go.sum | 4 ++-- simapp/go.mod | 2 +- simapp/go.sum | 4 ++-- tests/go.mod | 2 +- tests/go.sum | 4 ++-- x/staking/keeper/alias_functions.go | 5 ++--- x/staking/keeper/delegation.go | 8 ++++---- x/staking/keeper/query_utils.go | 5 ++--- 9 files changed, 17 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index cb89588e9ca4..38c6a7ebdf03 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ module github.com/cosmos/cosmos-sdk require ( cosmossdk.io/api v0.7.0 - cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 + cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68 cosmossdk.io/core v0.9.0 cosmossdk.io/depinject v1.0.0-alpha.3 cosmossdk.io/errors v1.0.0 diff --git a/go.sum b/go.sum index b65c8bd609d7..8027963c63fc 100644 --- a/go.sum +++ b/go.sum @@ -37,8 +37,8 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= cosmossdk.io/api v0.7.0 h1:QsEMIWuv9xWDbF2HZnW4Lpu1/SejCztPu0LQx7t6MN4= cosmossdk.io/api v0.7.0/go.mod h1:kJFAEMLN57y0viszHDPLMmieF0471o5QAwwApa+270M= -cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 h1:4XhcAIVBXPwCFTT9abOzZZaEadbRaVws8A6UTr2KGIE= -cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7/go.mod h1:+KJND4gZHilxE3meopEl/Uet6IAw3PyiSbgeOrFzAZE= +cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68 h1:aFHpJtJgdqBH8kRvD86Rl92rvd1+JFpaUpj+5NZNodg= +cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68/go.mod h1:OK08xZu8fxXLoQcFIfkBDayo2aRokLfC3vVcXX0MB8E= cosmossdk.io/core v0.9.0 h1:30ScAOHDIUOCg1DKAwqkho9wuQJnu7GUrMcg0XLioic= cosmossdk.io/core v0.9.0/go.mod h1:NFgl5r41Q36+RixTvyrfsS6qQ65agCbZ1FTpnN7/G1Y= cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw= diff --git a/simapp/go.mod b/simapp/go.mod index e6be42126a6b..c61a6c713558 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -5,7 +5,7 @@ go 1.20 require ( cosmossdk.io/api v0.7.0 cosmossdk.io/client/v2 v2.0.0-20230630094428-02b760776860 - cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 + cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68 cosmossdk.io/core v0.9.0 cosmossdk.io/depinject v1.0.0-alpha.3 cosmossdk.io/log v1.2.0 diff --git a/simapp/go.sum b/simapp/go.sum index cb76a493e9b8..715ac0e110a4 100644 --- a/simapp/go.sum +++ b/simapp/go.sum @@ -189,8 +189,8 @@ cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1V cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M= cosmossdk.io/api v0.7.0 h1:QsEMIWuv9xWDbF2HZnW4Lpu1/SejCztPu0LQx7t6MN4= cosmossdk.io/api v0.7.0/go.mod h1:kJFAEMLN57y0viszHDPLMmieF0471o5QAwwApa+270M= -cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 h1:4XhcAIVBXPwCFTT9abOzZZaEadbRaVws8A6UTr2KGIE= -cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7/go.mod h1:+KJND4gZHilxE3meopEl/Uet6IAw3PyiSbgeOrFzAZE= +cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68 h1:aFHpJtJgdqBH8kRvD86Rl92rvd1+JFpaUpj+5NZNodg= +cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68/go.mod h1:OK08xZu8fxXLoQcFIfkBDayo2aRokLfC3vVcXX0MB8E= cosmossdk.io/core v0.9.0 h1:30ScAOHDIUOCg1DKAwqkho9wuQJnu7GUrMcg0XLioic= cosmossdk.io/core v0.9.0/go.mod h1:NFgl5r41Q36+RixTvyrfsS6qQ65agCbZ1FTpnN7/G1Y= cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw= diff --git a/tests/go.mod b/tests/go.mod index b360fcbd8a08..dfe035f43317 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -4,7 +4,7 @@ go 1.20 require ( cosmossdk.io/api v0.7.0 - cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 + cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68 cosmossdk.io/core v0.9.0 cosmossdk.io/depinject v1.0.0-alpha.3 cosmossdk.io/errors v1.0.0 diff --git a/tests/go.sum b/tests/go.sum index f9226ff6761a..6d0d1b9c8c65 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -189,8 +189,8 @@ cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1V cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M= cosmossdk.io/api v0.7.0 h1:QsEMIWuv9xWDbF2HZnW4Lpu1/SejCztPu0LQx7t6MN4= cosmossdk.io/api v0.7.0/go.mod h1:kJFAEMLN57y0viszHDPLMmieF0471o5QAwwApa+270M= -cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7 h1:4XhcAIVBXPwCFTT9abOzZZaEadbRaVws8A6UTr2KGIE= -cosmossdk.io/collections v0.3.1-0.20230727092431-f0f777fa3cb7/go.mod h1:+KJND4gZHilxE3meopEl/Uet6IAw3PyiSbgeOrFzAZE= +cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68 h1:aFHpJtJgdqBH8kRvD86Rl92rvd1+JFpaUpj+5NZNodg= +cosmossdk.io/collections v0.3.1-0.20230808102719-f04fefdc7a68/go.mod h1:OK08xZu8fxXLoQcFIfkBDayo2aRokLfC3vVcXX0MB8E= cosmossdk.io/core v0.9.0 h1:30ScAOHDIUOCg1DKAwqkho9wuQJnu7GUrMcg0XLioic= cosmossdk.io/core v0.9.0/go.mod h1:NFgl5r41Q36+RixTvyrfsS6qQ65agCbZ1FTpnN7/G1Y= cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw= diff --git a/x/staking/keeper/alias_functions.go b/x/staking/keeper/alias_functions.go index 079dbed0ad2c..d9790e784169 100644 --- a/x/staking/keeper/alias_functions.go +++ b/x/staking/keeper/alias_functions.go @@ -2,7 +2,6 @@ package keeper import ( "context" - "errors" "cosmossdk.io/collections" storetypes "cosmossdk.io/store/types" @@ -134,13 +133,13 @@ func (k Keeper) IterateDelegations(ctx context.Context, delAddr sdk.AccAddress, err := k.Delegations.Walk(ctx, rng, func(key collections.Pair[sdk.AccAddress, sdk.ValAddress], del types.Delegation) (stop bool, err error) { stop = fn(i, del) if stop { - return true, err + return true, nil } i++ return false, nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return err } diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index da4ced71f25a..111439d07838 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -27,7 +27,7 @@ func (k Keeper) GetAllDelegations(ctx context.Context) ([]types.Delegation, erro return false, nil }, ) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } @@ -50,7 +50,7 @@ func (k Keeper) GetValidatorDelegations(ctx context.Context, valAddr sdk.ValAddr return false, nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } @@ -73,7 +73,7 @@ func (k Keeper) GetDelegatorDelegations(ctx context.Context, delegator sdk.AccAd return false, nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } @@ -284,7 +284,7 @@ func (k Keeper) IterateDelegatorDelegations(ctx context.Context, delegator sdk.A return false, nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return err } diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index c4a320c30536..c4e19feb4c44 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" storetypes "cosmossdk.io/store/types" @@ -39,7 +38,7 @@ func (k Keeper) GetDelegatorValidators( return false, nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } @@ -75,7 +74,7 @@ func (k Keeper) GetAllDelegatorDelegations(ctx context.Context, delegator sdk.Ac return false, nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil { return nil, err } From 3028663523ee04f84ae9553b3c77665efe7a6554 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 8 Aug 2023 19:32:10 +0530 Subject: [PATCH 09/11] fix tests --- x/gov/abci_test.go | 80 ++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index 411e6cd7f88e..6b62d7ebbd91 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -35,7 +35,7 @@ func TestTickExpiredDepositPeriod(t *testing.T) { require.NoError(t, err) govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newProposalMsg, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, @@ -52,25 +52,25 @@ func TestTickExpiredDepositPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newHeader := ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) params, _ := suite.GovKeeper.Params.Get(ctx) newHeader = ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) err = gov.EndBlocker(ctx, suite.GovKeeper) require.NoError(t, err) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) } func TestTickMultipleExpiredDepositPeriod(t *testing.T) { @@ -86,7 +86,7 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { require.NoError(t, err) govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newProposalMsg, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, @@ -103,13 +103,13 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newHeader := ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(2) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newProposalMsg2, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, @@ -131,17 +131,17 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(-1) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper)) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newHeader = ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(5) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper)) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) } func TestTickPassedDepositPeriod(t *testing.T) { @@ -174,13 +174,13 @@ func TestTickPassedDepositPeriod(t *testing.T) { proposalID := res.ProposalId - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newHeader := ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) newDepositMsg := v1.NewMsgDeposit(addrs[1], proposalID, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)}) @@ -188,7 +188,7 @@ func TestTickPassedDepositPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res1) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) } func TestTickPassedVotingPeriod(t *testing.T) { @@ -222,8 +222,8 @@ func TestTickPassedVotingPeriod(t *testing.T) { require.NoError(t, err) govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposalCoins := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 5*depositMultiplier))} newProposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{mkTestLegacyContent(t)}, proposalCoins, addrs[0].String(), "", "Proposal", "description of proposal", tc.expedited) @@ -255,8 +255,8 @@ func TestTickPassedVotingPeriod(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*votingPeriod) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.NoError(t, err) @@ -266,12 +266,12 @@ func TestTickPassedVotingPeriod(t *testing.T) { require.NoError(t, err) if !tc.expedited { - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) return } // If expedited, it should be converted to a regular proposal instead. - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) @@ -467,8 +467,8 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { createValidators(t, stakingMsgSvr, ctx, []sdk.ValAddress{valAddr}, []int64{10}) _, err = suite.StakingKeeper.EndBlocker(ctx) require.NoError(t, err) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) macc := suite.GovKeeper.GetGovernanceAccount(ctx) require.NotNil(t, macc) @@ -501,8 +501,8 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.ExpeditedVotingPeriod) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) @@ -518,7 +518,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { err = gov.EndBlocker(ctx, suite.GovKeeper) require.NoError(t, err) if tc.expeditedPasses { - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) @@ -539,7 +539,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { } // Expedited proposal should be converted to a regular proposal instead. - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) @@ -560,8 +560,8 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod) ctx = ctx.WithBlockHeader(newHeader) - checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) if tc.regularEventuallyPassing { // Validator votes YES, letting the converted regular proposal pass. @@ -579,7 +579,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { submitterEventualBalance := suite.BankKeeper.GetAllBalances(ctx, addrs[0]) depositorEventualBalance := suite.BankKeeper.GetAllBalances(ctx, addrs[1]) - checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) @@ -632,26 +632,22 @@ func getDepositMultiplier(expedited bool) int64 { return 1 } -func checkActiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper, invalid bool) { +func checkActiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper) { t.Helper() err := k.ActiveProposalsQueue.Walk(ctx, collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()), func(key collections.Pair[time.Time, uint64], value uint64) (stop bool, err error) { return false, err }) - if invalid { - require.ErrorIs(t, err, collections.ErrInvalidIterator) - } else { - require.NoError(t, err) - } + + require.NoError(t, err) + } -func checkInactiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper, invalid bool) { +func checkInactiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper) { t.Helper() err := k.InactiveProposalsQueue.Walk(ctx, collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()), func(key collections.Pair[time.Time, uint64], value uint64) (stop bool, err error) { return false, err }) - if invalid { - require.ErrorIs(t, err, collections.ErrInvalidIterator) - } else { - require.NoError(t, err) - } + + require.NoError(t, err) + } From 73ed6f7fdfe1a564e0361719942d0d60e90f5ce0 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 8 Aug 2023 20:00:17 +0530 Subject: [PATCH 10/11] fix err --- x/gov/keeper/tally_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/keeper/tally_test.go b/x/gov/keeper/tally_test.go index bae984b75f7a..daff9f043df1 100644 --- a/x/gov/keeper/tally_test.go +++ b/x/gov/keeper/tally_test.go @@ -418,7 +418,7 @@ func TestTally(t *testing.T) { // Assert votes removal after tally rng := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposal.Id) _, err = suite.keeper.Votes.Iterate(suite.ctx, rng) - assert.ErrorIs(t, err, collections.ErrInvalidIterator, "votes must be removed after tally") + assert.Nil(t, err, collections.ErrInvalidIterator, "votes must be removed after tally") }) } } From 489a3382f6ffe79374b957aaf0332e2f9a9c26af Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 8 Aug 2023 20:01:05 +0530 Subject: [PATCH 11/11] fix tests --- x/gov/abci_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index 6b62d7ebbd91..c43d07a8a953 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -639,7 +639,6 @@ func checkActiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper) }) require.NoError(t, err) - } func checkInactiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper) { @@ -649,5 +648,4 @@ func checkInactiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper }) require.NoError(t, err) - }