Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

WIP: Queriers for staking to increase Gaia-lite performance #2139

Closed
wants to merge 37 commits into from
Closed

WIP: Queriers for staking to increase Gaia-lite performance #2139

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
2ca7e61
Cherry picked commits from prev branch
Aug 24, 2018
5e285e1
Added new keepers for querier functionalities
Aug 24, 2018
ec13fbe
Renaming
Aug 24, 2018
3eacdfa
Fixed gov errors and messages
Aug 24, 2018
682008f
Added Querier to stake and app
Aug 27, 2018
ceae374
Update delegation keepers
Aug 27, 2018
9ba98ab
REST Queriers not working
Aug 27, 2018
8bf6b2c
Fix marshalling error
Aug 27, 2018
cd0ca86
Querier tests working
Aug 28, 2018
01304c9
Pool and params working
Aug 28, 2018
7889374
sdk.NewCoin for test handler
Aug 28, 2018
d3b6c3e
Refactor and renaming
Aug 28, 2018
5fb1547
Update LCD queries and added more tests for queriers
Aug 29, 2018
8cfc507
use sdk.NewCoin
Aug 29, 2018
044b851
Delegator summary query and tests
Aug 29, 2018
685f05a
Added more tests for keeper
Aug 29, 2018
6d9c8b7
Update PENDING.md
Aug 29, 2018
3f5ad5f
Merge branch 'develop' into fedekunze/2009-queriers-staking
fedekunze Aug 29, 2018
f6aed4e
Update stake rest query
Aug 29, 2018
b9b04a0
Format and replaced panics for sdk.Error
Aug 30, 2018
2852b4a
Refactor and addressed comments from Sunny and Aleks
Aug 31, 2018
128deb5
Merge branch 'develop' into fedekunze/2009-queriers-staking
fedekunze Aug 31, 2018
e93a937
Fixed some of the errors produced by addr type change
Aug 31, 2018
80cd207
Fixed remaining errors
Aug 31, 2018
c83172d
Updated and fixed lite tests
Aug 31, 2018
7f3a42c
JSON Header and consistency on errors
Aug 31, 2018
87c1a0f
Increased cov for genesis
Aug 31, 2018
5ecedad
Added comment for maxRetrieve param in keepers
Aug 31, 2018
af757dd
Comment on DelegationWithoutDec
Aug 31, 2018
80db853
Bech32Validator Keepers
Aug 31, 2018
99acd57
Changed Bech validator
Sep 1, 2018
8abb44e
Updated remaining tests and bech32 validator
Sep 1, 2018
55ff4b7
Merge branch 'develop' into fedekunze/2009-queriers-staking
fedekunze Sep 2, 2018
450539b
Addressed most of Rigel's comments
Sep 3, 2018
61efdf6
Updated tests and types
Sep 6, 2018
0bd0417
Make codec to be unexported from keeper
Sep 6, 2018
8a2d144
Moved logic to query_utils and updated tests
Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 66 additions & 14 deletions x/stake/keeper/delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/stake/types"
)

// load a delegation
// return a specific delegation
func (k Keeper) GetDelegation(ctx sdk.Context,
delAddr sdk.AccAddress, valAddr sdk.ValAddress) (
delegation types.Delegation, found bool) {
Expand All @@ -23,7 +23,7 @@ func (k Keeper) GetDelegation(ctx sdk.Context,
return delegation, true
}

// load all delegations used during genesis dump
// return all delegations used during genesis dump
func (k Keeper) GetAllDelegations(ctx sdk.Context) (delegations []types.Delegation) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, DelegationKey)
Expand All @@ -36,7 +36,44 @@ func (k Keeper) GetAllDelegations(ctx sdk.Context) (delegations []types.Delegati
return delegations
}

// load all validators that a delegator is bonded to or retrieve a limited amount
// Return all validators that a delegator is bonded to. If maxRetrieve is supplied, the respective amount will be returned
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) GetDelegatorBechValidators(ctx sdk.Context, delegatorAddr sdk.AccAddress,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this function - for many reasons outlined already in my PR comments

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rigelrozanski I still need to iterate through the validators to change them to BechValidators. Do you prefer to do this iteration on the Querier or move this keeper to the query_utils.go file ?

maxRetrieve ...int16) (validators []types.BechValidator) {

retrieve := len(maxRetrieve) > 0
if retrieve {
validators = make([]types.BechValidator, maxRetrieve[0])
}
store := ctx.KVStore(k.storeKey)
delegatorPrefixKey := GetDelegationsKey(delegatorAddr)
iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) //smallest to largest

i := 0
for ; iterator.Valid() && (!retrieve || (retrieve && i < int(maxRetrieve[0]))); iterator.Next() {
addr := iterator.Key()
delegation := types.MustUnmarshalDelegation(k.cdc, addr, iterator.Value())
validator, found := k.GetValidator(ctx, delegation.ValidatorAddr)
if !found {
panic(types.ErrNoValidatorFound(types.DefaultCodespace))
}

bechValidator, err := validator.Bech32Validator()
if err != nil {
panic(err.Error())
}

if retrieve {
validators[i] = bechValidator
} else {
validators = append(validators, bechValidator)
}
i++
}
iterator.Close()
return validators[:i] // trim
}

// Return all validators that a delegator is bonded to. If maxRetrieve is supplied, the respective amount will be returned
func (k Keeper) GetDelegatorValidators(ctx sdk.Context, delegatorAddr sdk.AccAddress,
maxRetrieve ...int16) (validators []types.Validator) {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Split up into two functions as per my other comments - Actually I don't think GetAllDelegatorValidators should even exist I don't see a valid use of it - we should enforcing that a max value should be provided


Expand All @@ -56,6 +93,7 @@ func (k Keeper) GetDelegatorValidators(ctx sdk.Context, delegatorAddr sdk.AccAdd
if !found {
panic(types.ErrNoValidatorFound(types.DefaultCodespace))
}

if retrieve {
validators[i] = validator
} else {
Expand All @@ -68,22 +106,36 @@ func (k Keeper) GetDelegatorValidators(ctx sdk.Context, delegatorAddr sdk.AccAdd
return validators[:i] // trim
}

// load a validator that a delegator is bonded to
// return a validator that a delegator is bonded to
func (k Keeper) GetDelegatorBechValidator(ctx sdk.Context, delegatorAddr sdk.AccAddress,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
validatorAddr sdk.ValAddress) (bechValidator types.BechValidator) {

validator := k.GetDelegatorValidator(ctx, delegatorAddr, validatorAddr)
bechValidator, err := validator.Bech32Validator()
if err != nil {
panic(err.Error())
}

return
}

// return a validator that a delegator is bonded to
func (k Keeper) GetDelegatorValidator(ctx sdk.Context, delegatorAddr sdk.AccAddress,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
validatorAddr sdk.ValAddress) (validator types.Validator) {

delegation, found := k.GetDelegation(ctx, delegatorAddr, validatorAddr)
if !found {
panic(types.ErrNoDelegation(types.DefaultCodespace))
}

validator, found = k.GetValidator(ctx, delegation.ValidatorAddr)
if !found {
panic(types.ErrNoValidatorFound(types.DefaultCodespace))
}
return
}

// load all delegations for a delegator or retrieve a limited amount
// return all delegations for a delegator. If maxRetrieve is supplied, the respective amount will be returned
func (k Keeper) GetDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddress,
maxRetrieve ...int16) (delegations []types.Delegation) {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
retrieve := len(maxRetrieve) > 0
Expand All @@ -108,7 +160,7 @@ func (k Keeper) GetDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddres
return delegations[:i] // trim
}

// load all delegations for a delegator or retrieve a limited amount
// Return all delegations for a delegator. If maxRetrieve is supplied, the respective amount will be returned
func (k Keeper) GetDelegatorDelegationsWithoutRat(ctx sdk.Context, delegator sdk.AccAddress,
maxRetrieve ...int16) (delegations []types.DelegationWithoutDec) {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
retrieve := len(maxRetrieve) > 0
Expand Down Expand Up @@ -141,15 +193,15 @@ func (k Keeper) SetDelegation(ctx sdk.Context, delegation types.Delegation) {
store.Set(GetDelegationKey(delegation.DelegatorAddr, delegation.ValidatorAddr), b)
}

// remove the delegation
// remove a delegation from store
func (k Keeper) RemoveDelegation(ctx sdk.Context, delegation types.Delegation) {
store := ctx.KVStore(k.storeKey)
store.Delete(GetDelegationKey(delegation.DelegatorAddr, delegation.ValidatorAddr))
}

//_____________________________________________________________________________________

// load all unbonding-delegations for a delegator or retrieve a limited amount
// Return all unbonding delegations for a delegator. If maxRetrieve is supplied, the respective amount will be returned
func (k Keeper) GetDelegatorUnbondingDelegations(ctx sdk.Context, delegator sdk.AccAddress,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
maxRetrieve ...int16) (unbondingDelegations []types.UnbondingDelegation) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's enforce that there is a max retrieve here - I don't see a valid case for not requiring a maximum amount in this situation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

imo it's better to leave it this way. Otherwise how would you query all the UnbondingDelegations for example ?


Expand All @@ -175,7 +227,7 @@ func (k Keeper) GetDelegatorUnbondingDelegations(ctx sdk.Context, delegator sdk.
return unbondingDelegations[:i] // trim
}

// load a unbonding delegation
// return a unbonding delegation
func (k Keeper) GetUnbondingDelegation(ctx sdk.Context,
delAddr sdk.AccAddress, valAddr sdk.ValAddress) (ubd types.UnbondingDelegation, found bool) {

Expand All @@ -190,7 +242,7 @@ func (k Keeper) GetUnbondingDelegation(ctx sdk.Context,
return ubd, true
}

// load all unbonding delegations from a particular validator
// return all unbonding delegations from a particular validator
func (k Keeper) GetUnbondingDelegationsFromValidator(ctx sdk.Context, valAddr sdk.ValAddress) (ubds []types.UnbondingDelegation) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, GetUBDsByValIndexKey(valAddr))
Expand Down Expand Up @@ -241,7 +293,7 @@ func (k Keeper) RemoveUnbondingDelegation(ctx sdk.Context, ubd types.UnbondingDe

//_____________________________________________________________________________________

// load all redelegations for a delegator or retrieve a limited amount
// Return all redelegations for a delegator. If maxRetrieve is supplied, the respective amount will be returned
func (k Keeper) GetRedelegations(ctx sdk.Context, delegator sdk.AccAddress,
maxRetrieve ...int16) (redelegations []types.Redelegation) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's split this up into two functions GetRedelegations and GetAllRedelegations for reasons I've mentioned in other comments


Expand All @@ -267,7 +319,7 @@ func (k Keeper) GetRedelegations(ctx sdk.Context, delegator sdk.AccAddress,
return redelegations[:i] // trim
}

// load a redelegation
// return a redelegation
func (k Keeper) GetRedelegation(ctx sdk.Context,
delAddr sdk.AccAddress, valSrcAddr, valDstAddr sdk.ValAddress) (red types.Redelegation, found bool) {

Expand All @@ -282,7 +334,7 @@ func (k Keeper) GetRedelegation(ctx sdk.Context,
return red, true
}

// load all redelegations from a particular validator
// return all redelegations from a particular validator
func (k Keeper) GetRedelegationsFromValidator(ctx sdk.Context, valAddr sdk.ValAddress) (reds []types.Redelegation) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, GetREDsFromValSrcIndexKey(valAddr))
Expand All @@ -296,7 +348,7 @@ func (k Keeper) GetRedelegationsFromValidator(ctx sdk.Context, valAddr sdk.ValAd
return reds
}

// has a redelegation
// check if validator has an incoming redelegation
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) HasReceivingRedelegation(ctx sdk.Context,
delAddr sdk.AccAddress, valDstAddr sdk.ValAddress) bool {

Expand Down
7 changes: 7 additions & 0 deletions x/stake/keeper/delegation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,19 @@ func TestDelegation(t *testing.T) {
resVals = keeper.GetDelegatorValidators(ctx, addrDels[1], 4)
require.Equal(t, 3, len(resVals))

resBechVals := keeper.GetDelegatorBechValidators(ctx, addrDels[0])
require.Equal(t, 3, len(resBechVals))

for i := 0; i < 3; i++ {
resVal := keeper.GetDelegatorValidator(ctx, addrDels[0], addrVals[i])
require.Equal(t, addrVals[i], resVal.GetOperator())
resBechVal := keeper.GetDelegatorBechValidator(ctx, addrDels[0], addrVals[i])
require.Equal(t, addrVals[i], resBechVal.Operator)

resVal = keeper.GetDelegatorValidator(ctx, addrDels[1], addrVals[i])
require.Equal(t, addrVals[i], resVal.GetOperator())
resBechVal = keeper.GetDelegatorBechValidator(ctx, addrDels[1], addrVals[i])
require.Equal(t, addrVals[i], resBechVal.Operator)
}

// delete a record
Expand Down
59 changes: 59 additions & 0 deletions x/stake/keeper/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty
return validator, true
}

// get a single validator with Bech32 prefix
func (k Keeper) GetBechValidator(ctx sdk.Context, addr sdk.ValAddress) (bechValidator types.BechValidator, found bool) {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
validator, found := k.GetValidator(ctx, addr)
if !found {
return bechValidator, false
}

bechValidator, err := validator.Bech32Validator()
if err != nil {
panic(err.Error())
}
return bechValidator, true
}

// get a single validator by pubkey
func (k Keeper) GetValidatorByPubKey(ctx sdk.Context, pubkey crypto.PubKey) (validator types.Validator, found bool) {
store := ctx.KVStore(k.storeKey)
Expand All @@ -32,6 +46,20 @@ func (k Keeper) GetValidatorByPubKey(ctx sdk.Context, pubkey crypto.PubKey) (val
return k.GetValidator(ctx, addr)
}

// get a single validator by pubkey with Bech32 prefix
func (k Keeper) GetBechalidatorByPubKey(ctx sdk.Context, pubkey crypto.PubKey) (bechValidator types.BechValidator, found bool) {
validator, found := k.GetValidatorByPubKey(ctx, pubkey)
if !found {
return bechValidator, false
}

bechValidator, err := validator.Bech32Validator()
if err != nil {
panic(err.Error())
}
return bechValidator, true
}

// set the main record holding validator details
func (k Keeper) SetValidator(ctx sdk.Context, validator types.Validator) {
store := ctx.KVStore(k.storeKey)
Expand Down Expand Up @@ -88,6 +116,37 @@ func (k Keeper) GetValidators(ctx sdk.Context, maxRetrieve ...int16) (validators
return validators[:i] // trim
Copy link
Contributor

Choose a reason for hiding this comment

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

This only adds i items to the validators array, is there a reason to trim it?

}

// Get the set of all validators, retrieve an optional maxRetrieve number of records
func (k Keeper) GetBechValidators(ctx sdk.Context, maxRetrieve ...int16) (validators []types.BechValidator) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As per my previous comment we should rarely be using GetAllValidators so it should be its own edge case. But also, let's remove use of this function altogether, it bloats the keeper - what we should really be doing is just converting the validators to bech once we've retrieved them from GetValidators.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With that implementation we'd have to iterate the validator array twice, one to retrieve the validators and another to convert each of them into BechValidator ...

retrieve := len(maxRetrieve) > 0
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
if retrieve {
validators = make([]types.BechValidator, maxRetrieve[0])
}

store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, ValidatorsKey)

i := 0
for ; iterator.Valid() && (!retrieve || (retrieve && i < int(maxRetrieve[0]))); iterator.Next() {
addr := iterator.Key()[1:]
validator := types.MustUnmarshalValidator(k.cdc, addr, iterator.Value())

bechValidator, err := validator.Bech32Validator()
if err != nil {
panic(err.Error())
}

if retrieve {
validators[i] = bechValidator
} else {
validators = append(validators, bechValidator)
}
i++
}
iterator.Close()
return validators[:i] // trim
}

//___________________________________________________________________________

// get the group of the bonded validators
Expand Down
6 changes: 3 additions & 3 deletions x/stake/types/delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ type delegationValue struct {
Height int64
}

// TODO Remove DelegationWithoutDec. See \#2096

// defines a delegation without type Rat for shares
// DelegationWithoutDec defines a delegation without the type sdk.Dec for shares
//
// TODO: Remove this type (ref: #2096)
type DelegationWithoutDec struct {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
DelegatorAddr sdk.AccAddress `json:"delegator_addr"`
ValidatorAddr sdk.ValAddress `json:"validator_addr"`
Expand Down