diff --git a/.pending/bugfixes/sdk/4194-Fix-pagination- b/.pending/bugfixes/sdk/4194-Fix-pagination- new file mode 100644 index 0000000000..64edb2d426 --- /dev/null +++ b/.pending/bugfixes/sdk/4194-Fix-pagination- @@ -0,0 +1 @@ +#4194 Fix pagination and results returned from /slashing/signing_infos diff --git a/.pending/improvements/sdk/4194-ValidatorSignin b/.pending/improvements/sdk/4194-ValidatorSignin new file mode 100644 index 0000000000..48627f3687 --- /dev/null +++ b/.pending/improvements/sdk/4194-ValidatorSignin @@ -0,0 +1 @@ +#4194 ValidatorSigningInfo now includes the validator's consensus address. diff --git a/types/rest/rest.go b/types/rest/rest.go index 71549584da..556821dcf2 100644 --- a/types/rest/rest.go +++ b/types/rest/rest.go @@ -221,9 +221,10 @@ func PostProcessResponse(w http.ResponseWriter, cdc *codec.Codec, response inter _, _ = w.Write(output) } -// ParseHTTPArgs parses the request's URL and returns a slice containing all arguments pairs. -// It separates page and limit used for pagination -func ParseHTTPArgs(r *http.Request) (tags []string, page, limit int, err error) { +// ParseHTTPArgsWithLimit parses the request's URL and returns a slice containing +// all arguments pairs. It separates page and limit used for pagination where a +// default limit can be provided. +func ParseHTTPArgsWithLimit(r *http.Request, defaultLimit int) (tags []string, page, limit int, err error) { tags = make([]string, 0, len(r.Form)) for key, values := range r.Form { if key == "page" || key == "limit" { @@ -258,7 +259,7 @@ func ParseHTTPArgs(r *http.Request) (tags []string, page, limit int, err error) limitStr := r.FormValue("limit") if limitStr == "" { - limit = DefaultLimit + limit = defaultLimit } else { limit, err = strconv.Atoi(limitStr) if err != nil { @@ -270,3 +271,9 @@ func ParseHTTPArgs(r *http.Request) (tags []string, page, limit int, err error) return tags, page, limit, nil } + +// ParseHTTPArgs parses the request's URL and returns a slice containing all +// arguments pairs. It separates page and limit used for pagination. +func ParseHTTPArgs(r *http.Request) (tags []string, page, limit int, err error) { + return ParseHTTPArgsWithLimit(r, DefaultLimit) +} diff --git a/types/staking.go b/types/staking.go index 54f28f9727..3ff844b8c9 100644 --- a/types/staking.go +++ b/types/staking.go @@ -119,6 +119,9 @@ type ValidatorSet interface { // Delegation allows for getting a particular delegation for a given validator // and delegator outside the scope of the staking module. Delegation(Context, AccAddress, ValAddress) Delegation + + // MaxValidators returns the maximum amount of bonded validators + MaxValidators(Context) uint16 } //_______________________________________________________________________________ diff --git a/x/slashing/client/rest/query.go b/x/slashing/client/rest/query.go index 5da8ad825d..c17b61053e 100644 --- a/x/slashing/client/rest/query.go +++ b/x/slashing/client/rest/query.go @@ -7,7 +7,6 @@ import ( "github.com/gorilla/mux" "github.com/cosmos/cosmos-sdk/client/context" - "github.com/cosmos/cosmos-sdk/client/rpc" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/rest" @@ -17,13 +16,13 @@ import ( func registerQueryRoutes(cliCtx context.CLIContext, r *mux.Router, cdc *codec.Codec) { r.HandleFunc( "/slashing/validators/{validatorPubKey}/signing_info", - signingInfoHandlerFn(cliCtx, slashing.StoreKey, cdc), + signingInfoHandlerFn(cliCtx, cdc), ).Methods("GET") r.HandleFunc( "/slashing/signing_infos", - signingInfoHandlerListFn(cliCtx, slashing.StoreKey, cdc), - ).Methods("GET").Queries("page", "{page}", "limit", "{limit}") + signingInfoHandlerListFn(cliCtx, cdc), + ).Methods("GET") r.HandleFunc( "/slashing/parameters", @@ -32,7 +31,7 @@ func registerQueryRoutes(cliCtx context.CLIContext, r *mux.Router, cdc *codec.Co } // http request handler to query signing info -func signingInfoHandlerFn(cliCtx context.CLIContext, storeName string, cdc *codec.Codec) http.HandlerFunc { +func signingInfoHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) pk, err := sdk.GetConsPubKeyBech32(vars["validatorPubKey"]) @@ -41,65 +40,49 @@ func signingInfoHandlerFn(cliCtx context.CLIContext, storeName string, cdc *code return } - signingInfo, code, err := getSigningInfo(cliCtx, storeName, cdc, pk.Address()) + params := slashing.NewQuerySigningInfoParams(sdk.ConsAddress(pk.Address())) - if err != nil { - rest.WriteErrorResponse(w, code, err.Error()) - return - } - - rest.PostProcessResponse(w, cdc, signingInfo, cliCtx.Indent) - } -} - -// http request handler to query signing info -func signingInfoHandlerListFn(cliCtx context.CLIContext, storeName string, cdc *codec.Codec) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - var signingInfoList []slashing.ValidatorSigningInfo - - _, page, limit, err := rest.ParseHTTPArgs(r) + bz, err := cdc.MarshalJSON(params) if err != nil { rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } - height, err := rpc.GetChainHeight(cliCtx) + route := fmt.Sprintf("custom/%s/%s", slashing.QuerierRoute, slashing.QuerySigningInfo) + res, err := cliCtx.QueryWithData(route, bz) if err != nil { rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } - validators, err := rpc.GetValidators(cliCtx, &height) + rest.PostProcessResponse(w, cdc, res, cliCtx.Indent) + } +} + +// http request handler to query signing info +func signingInfoHandlerListFn(cliCtx context.CLIContext, cdc *codec.Codec) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + _, page, limit, err := rest.ParseHTTPArgsWithLimit(r, 0) + if err != nil { + rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + return + } + + params := slashing.NewQuerySigningInfosParams(page, limit) + bz, err := cdc.MarshalJSON(params) if err != nil { rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } - if len(validators.Validators) == 0 { - rest.PostProcessResponse(w, cdc, signingInfoList, cliCtx.Indent) + route := fmt.Sprintf("custom/%s/%s", slashing.QuerierRoute, slashing.QuerySigningInfos) + res, err := cliCtx.QueryWithData(route, bz) + if err != nil { + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } - // TODO: this should happen when querying Validators from RPC, - // as soon as it's available this is not needed anymore - // parameter page is (page-1) because ParseHTTPArgs starts with page 1, where our array start with 0 - start, end := adjustPagination(uint(len(validators.Validators)), uint(page)-1, uint(limit)) - for _, validator := range validators.Validators[start:end] { - address := validator.Address - signingInfo, code, err := getSigningInfo(cliCtx, storeName, cdc, address) - if err != nil { - rest.WriteErrorResponse(w, code, err.Error()) - return - } - signingInfoList = append(signingInfoList, signingInfo) - } - - if len(signingInfoList) == 0 { - rest.PostProcessResponse(w, cdc, signingInfoList, cliCtx.Indent) - return - } - - rest.PostProcessResponse(w, cdc, signingInfoList, cliCtx.Indent) + rest.PostProcessResponse(w, cdc, res, cliCtx.Indent) } } @@ -116,48 +99,3 @@ func queryParamsHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Hand rest.PostProcessResponse(w, cdc, res, cliCtx.Indent) } } - -func getSigningInfo(cliCtx context.CLIContext, storeName string, cdc *codec.Codec, address []byte) (signingInfo slashing.ValidatorSigningInfo, code int, err error) { - key := slashing.GetValidatorSigningInfoKey(sdk.ConsAddress(address)) - - res, err := cliCtx.QueryStore(key, storeName) - if err != nil { - code = http.StatusInternalServerError - return - } - - if len(res) == 0 { - code = http.StatusOK - return - } - - err = cdc.UnmarshalBinaryLengthPrefixed(res, &signingInfo) - if err != nil { - code = http.StatusInternalServerError - return - } - - return -} - -// Adjust pagination with page starting from 0 -func adjustPagination(size, page, limit uint) (start uint, end uint) { - // If someone asks for pages bigger than our dataset, just return everything - if limit > size { - return 0, size - } - - // Do pagination when healthy, fallback to 0 - start = 0 - if page*limit < size { - start = page * limit - } - - // Do pagination only when healthy, fallback to len(dataset) - end = size - if start+limit <= size { - end = start + limit - } - - return start, end -} diff --git a/x/slashing/client/rest/query_test.go b/x/slashing/client/rest/query_test.go deleted file mode 100644 index 62afaef0de..0000000000 --- a/x/slashing/client/rest/query_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package rest - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestAdjustPagination(t *testing.T) { - type args struct { - s string - } - tests := []struct { - name string - size uint - page uint - limit uint - start uint - end uint - }{ - {"Ok", 3, 0, 1, 0, 1}, - {"Limit too big", 3, 1, 5, 0, 3}, - {"Page over limit", 3, 2, 3, 0, 3}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - start, end := adjustPagination(tt.size, tt.page, tt.limit) - require.Equal(t, tt.start, start) - require.Equal(t, tt.end, end) - }) - } -} diff --git a/x/slashing/errors.go b/x/slashing/errors.go index 94997026df..47a5215921 100644 --- a/x/slashing/errors.go +++ b/x/slashing/errors.go @@ -2,6 +2,8 @@ package slashing import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -17,6 +19,7 @@ const ( CodeValidatorNotJailed CodeType = 103 CodeMissingSelfDelegation CodeType = 104 CodeSelfDelegationTooLow CodeType = 105 + CodeMissingSigningInfo CodeType = 106 ) func ErrNoValidatorForAddress(codespace sdk.CodespaceType) sdk.Error { @@ -42,3 +45,7 @@ func ErrMissingSelfDelegation(codespace sdk.CodespaceType) sdk.Error { func ErrSelfDelegationTooLowToUnjail(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeValidatorNotJailed, "validator's self delegation less than MinSelfDelegation, cannot be unjailed") } + +func ErrNoSigningInfoFound(codespace sdk.CodespaceType, consAddr sdk.ConsAddress) sdk.Error { + return sdk.NewError(codespace, CodeMissingSigningInfo, fmt.Sprintf("no signing info found for address: %s", consAddr)) +} diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 93a7057fb5..95a559e8f4 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -85,7 +85,7 @@ func TestJailedValidatorDelegations(t *testing.T) { staking.EndBlocker(ctx, stakingKeeper) // set dummy signing info - newInfo := NewValidatorSigningInfo(0, 0, time.Unix(0, 0), false, 0) + newInfo := NewValidatorSigningInfo(consAddr, 0, 0, time.Unix(0, 0), false, 0) slashingKeeper.SetValidatorSigningInfo(ctx, consAddr, newInfo) // delegate tokens to the validator diff --git a/x/slashing/hooks.go b/x/slashing/hooks.go index 9908e8d7ff..5083ca273d 100644 --- a/x/slashing/hooks.go +++ b/x/slashing/hooks.go @@ -13,13 +13,14 @@ func (k Keeper) AfterValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, _ // Update the signing info start height or create a new signing info _, found := k.getValidatorSigningInfo(ctx, address) if !found { - signingInfo := ValidatorSigningInfo{ - StartHeight: ctx.BlockHeight(), - IndexOffset: 0, - JailedUntil: time.Unix(0, 0), - Tombstoned: false, - MissedBlocksCounter: 0, - } + signingInfo := NewValidatorSigningInfo( + address, + ctx.BlockHeight(), + 0, + time.Unix(0, 0), + false, + 0, + ) k.SetValidatorSigningInfo(ctx, address, signingInfo) } } diff --git a/x/slashing/querier.go b/x/slashing/querier.go index a0faaeb509..291030f3db 100644 --- a/x/slashing/querier.go +++ b/x/slashing/querier.go @@ -1,6 +1,8 @@ package slashing import ( + "fmt" + abci "github.com/tendermint/tendermint/abci/types" "github.com/cosmos/cosmos-sdk/codec" @@ -9,7 +11,9 @@ import ( // Query endpoints supported by the slashing querier const ( - QueryParameters = "parameters" + QueryParameters = "parameters" + QuerySigningInfo = "signingInfo" + QuerySigningInfos = "signingInfos" ) // NewQuerier creates a new querier for slashing clients. @@ -18,6 +22,13 @@ func NewQuerier(k Keeper, cdc *codec.Codec) sdk.Querier { switch path[0] { case QueryParameters: return queryParams(ctx, cdc, k) + + case QuerySigningInfo: + return querySigningInfo(ctx, cdc, req, k) + + case QuerySigningInfos: + return querySigningInfos(ctx, cdc, req, k) + default: return nil, sdk.ErrUnknownRequest("unknown staking query endpoint") } @@ -34,3 +45,86 @@ func queryParams(ctx sdk.Context, cdc *codec.Codec, k Keeper) ([]byte, sdk.Error return res, nil } + +// QuerySigningInfoParams defines the params for the following queries: +// - 'custom/slashing/signingInfo' +type QuerySigningInfoParams struct { + ConsAddress sdk.ConsAddress +} + +func NewQuerySigningInfoParams(consAddr sdk.ConsAddress) QuerySigningInfoParams { + return QuerySigningInfoParams{consAddr} +} + +func querySigningInfo(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k Keeper) ([]byte, sdk.Error) { + var params QuerySigningInfoParams + + err := cdc.UnmarshalJSON(req.Data, ¶ms) + if err != nil { + return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) + } + + signingInfo, found := k.getValidatorSigningInfo(ctx, params.ConsAddress) + if !found { + return nil, ErrNoSigningInfoFound(DefaultCodespace, params.ConsAddress) + } + + res, err := codec.MarshalJSONIndent(cdc, signingInfo) + if err != nil { + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("failed to JSON marshal result: %s", err.Error())) + } + + return res, nil +} + +// QuerySigningInfosParams defines the params for the following queries: +// - 'custom/slashing/signingInfos' +type QuerySigningInfosParams struct { + Page, Limit int +} + +func NewQuerySigningInfosParams(page, limit int) QuerySigningInfosParams { + return QuerySigningInfosParams{page, limit} +} + +func querySigningInfos(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k Keeper) ([]byte, sdk.Error) { + var params QuerySigningInfosParams + + err := cdc.UnmarshalJSON(req.Data, ¶ms) + if err != nil { + return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) + } + + if params.Limit == 0 { + // set the default limit to max bonded if no limit was provided + params.Limit = int(k.validatorSet.MaxValidators(ctx)) + } + + var signingInfos []ValidatorSigningInfo + + k.IterateValidatorSigningInfos(ctx, func(consAddr sdk.ConsAddress, info ValidatorSigningInfo) (stop bool) { + signingInfos = append(signingInfos, info) + return false + }) + + // get pagination bounds + start := (params.Page - 1) * params.Limit + end := params.Limit + start + if end >= len(signingInfos) { + end = len(signingInfos) + } + + if start >= len(signingInfos) { + // page is out of bounds + signingInfos = []ValidatorSigningInfo{} + } else { + signingInfos = signingInfos[start:end] + } + + res, err := codec.MarshalJSONIndent(cdc, signingInfos) + if err != nil { + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("failed to JSON marshal result: %s", err.Error())) + } + + return res, nil +} diff --git a/x/slashing/signing_info.go b/x/slashing/signing_info.go index 517304f1ae..01f967fd60 100644 --- a/x/slashing/signing_info.go +++ b/x/slashing/signing_info.go @@ -92,18 +92,22 @@ func (k Keeper) clearValidatorMissedBlockBitArray(ctx sdk.Context, address sdk.C // Signing info for a validator type ValidatorSigningInfo struct { - StartHeight int64 `json:"start_height"` // height at which validator was first a candidate OR was unjailed - IndexOffset int64 `json:"index_offset"` // index offset into signed block bit array - JailedUntil time.Time `json:"jailed_until"` // timestamp validator cannot be unjailed until - Tombstoned bool `json:"tombstoned"` // whether or not a validator has been tombstoned (killed out of validator set) - MissedBlocksCounter int64 `json:"missed_blocks_counter"` // missed blocks counter (to avoid scanning the array every time) + Address sdk.ConsAddress `json:"address"` // validator consensus address + StartHeight int64 `json:"start_height"` // height at which validator was first a candidate OR was unjailed + IndexOffset int64 `json:"index_offset"` // index offset into signed block bit array + JailedUntil time.Time `json:"jailed_until"` // timestamp validator cannot be unjailed until + Tombstoned bool `json:"tombstoned"` // whether or not a validator has been tombstoned (killed out of validator set) + MissedBlocksCounter int64 `json:"missed_blocks_counter"` // missed blocks counter (to avoid scanning the array every time) } // Construct a new `ValidatorSigningInfo` struct -func NewValidatorSigningInfo(startHeight, indexOffset int64, jailedUntil time.Time, - tombstoned bool, missedBlocksCounter int64) ValidatorSigningInfo { +func NewValidatorSigningInfo( + condAddr sdk.ConsAddress, startHeight, indexOffset int64, + jailedUntil time.Time, tombstoned bool, missedBlocksCounter int64, +) ValidatorSigningInfo { return ValidatorSigningInfo{ + Address: condAddr, StartHeight: startHeight, IndexOffset: indexOffset, JailedUntil: jailedUntil, @@ -114,11 +118,13 @@ func NewValidatorSigningInfo(startHeight, indexOffset int64, jailedUntil time.Ti // Return human readable signing info func (i ValidatorSigningInfo) String() string { - return fmt.Sprintf(`Start Height: %d -Index Offset: %d -Jailed Until: %v -Tombstoned: %t -Missed Blocks Counter: %d`, - i.StartHeight, i.IndexOffset, i.JailedUntil, + return fmt.Sprintf(`Validator Signing Info: + Address: %s + Start Height: %d + Index Offset: %d + Jailed Until: %v + Tombstoned: %t + Missed Blocks Counter: %d`, + i.Address, i.StartHeight, i.IndexOffset, i.JailedUntil, i.Tombstoned, i.MissedBlocksCounter) } diff --git a/x/slashing/signing_info_test.go b/x/slashing/signing_info_test.go index d340eaf7a5..d6803af8da 100644 --- a/x/slashing/signing_info_test.go +++ b/x/slashing/signing_info_test.go @@ -13,12 +13,14 @@ func TestGetSetValidatorSigningInfo(t *testing.T) { ctx, _, _, _, keeper := createTestInput(t, DefaultParams()) info, found := keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(addrs[0])) require.False(t, found) - newInfo := ValidatorSigningInfo{ - StartHeight: int64(4), - IndexOffset: int64(3), - JailedUntil: time.Unix(2, 0), - MissedBlocksCounter: int64(10), - } + newInfo := NewValidatorSigningInfo( + sdk.ConsAddress(addrs[0]), + int64(4), + int64(3), + time.Unix(2, 0), + false, + int64(10), + ) keeper.SetValidatorSigningInfo(ctx, sdk.ConsAddress(addrs[0]), newInfo) info, found = keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(addrs[0])) require.True(t, found) diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index 886e120e21..cfc0297ff7 100644 --- a/x/slashing/test_common.go +++ b/x/slashing/test_common.go @@ -2,7 +2,6 @@ package slashing import ( "encoding/hex" - "os" "testing" "time" @@ -66,7 +65,7 @@ func createTestInput(t *testing.T, defaults Params) (sdk.Context, bank.Keeper, s ms.MountStoreWithDB(tkeyParams, sdk.StoreTypeTransient, db) err := ms.LoadLatestVersion() require.Nil(t, err) - ctx := sdk.NewContext(ms, abci.Header{Time: time.Unix(0, 0)}, false, log.NewTMLogger(os.Stdout)) + ctx := sdk.NewContext(ms, abci.Header{Time: time.Unix(0, 0)}, false, log.NewNopLogger()) cdc := createTestCodec() paramsKeeper := params.NewKeeper(cdc, keyParams, tkeyParams, params.DefaultCodespace) accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, paramsKeeper.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount) diff --git a/x/staking/client/rest/query.go b/x/staking/client/rest/query.go index 8b74b92f16..38129101d8 100644 --- a/x/staking/client/rest/query.go +++ b/x/staking/client/rest/query.go @@ -244,17 +244,12 @@ func delegatorValidatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) ht // HTTP request handler to query list of validators func validatorsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - _, page, limit, err := rest.ParseHTTPArgs(r) + _, page, limit, err := rest.ParseHTTPArgsWithLimit(r, 0) if err != nil { rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } - // override default limit if it wasn't provided - if l := r.FormValue("limit"); l == "" { - limit = 0 - } - status := r.FormValue("status") if status == "" { status = sdk.BondStatusBonded