diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index c2585b197..a7a5cd709 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -21,6 +21,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - sr25519: type `PubKeySr25519` is now `PubKey` - multisig: type `PubKeyMultisigThreshold` is now `PubKey` - [light] \#4946 Rename `lite2` pkg to `light`, the lite cmd has also been renamed to `light`. Remove `lite` implementation. + - [rpc] \#4937 Return an error when `page` pagination param is 0 in `/validators`, `tx_search` (@melekes) - Apps @@ -40,6 +41,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi Add `max_num` to consensus evidence parameters (default: 50 items). - [mempool] \#4940 Migrate mempool from amino binary encoding to Protobuf - [statesync] \#4943 Migrate statesync reactor from amino binary encoding to Protobuf + - [rpc/client] \#4947 `Validators`, `TxSearch` `page`/`per_page` params become pointers (@melekes) + `UnconfirmedTxs` `limit` param is a pointer ### FEATURES: diff --git a/light/provider/http/http.go b/light/provider/http/http.go index 5ca3e06b1..4db48a5ad 100644 --- a/light/provider/http/http.go +++ b/light/provider/http/http.go @@ -92,8 +92,8 @@ func (p *http) ValidatorSet(height int64) (*types.ValidatorSet, error) { return nil, err } - const maxPerPage = 100 - res, err := p.client.Validators(h, 0, maxPerPage) + maxPerPage := 100 + res, err := p.client.Validators(h, nil, &maxPerPage) if err != nil { // TODO: standartise errors on the RPC side if regexpMissingHeight.MatchString(err.Error()) { @@ -109,7 +109,7 @@ func (p *http) ValidatorSet(height int64) (*types.ValidatorSet, error) { // Check if there are more validators. for len(res.Validators) == maxPerPage { - res, err = p.client.Validators(h, page, maxPerPage) + res, err = p.client.Validators(h, &page, &maxPerPage) if err != nil { return nil, err } diff --git a/light/proxy/routes.go b/light/proxy/routes.go index cb2bc363c..843f32ee1 100644 --- a/light/proxy/routes.go +++ b/light/proxy/routes.go @@ -131,20 +131,20 @@ func makeTxFunc(c *lrpc.Client) rpcTxFunc { } type rpcTxSearchFunc func(ctx *rpctypes.Context, query string, prove bool, - page, perPage int, orderBy string) (*ctypes.ResultTxSearch, error) + page, perPage *int, orderBy string) (*ctypes.ResultTxSearch, error) func makeTxSearchFunc(c *lrpc.Client) rpcTxSearchFunc { - return func(ctx *rpctypes.Context, query string, prove bool, page, perPage int, orderBy string) ( + return func(ctx *rpctypes.Context, query string, prove bool, page, perPage *int, orderBy string) ( *ctypes.ResultTxSearch, error) { return c.TxSearch(query, prove, page, perPage, orderBy) } } type rpcValidatorsFunc func(ctx *rpctypes.Context, height *int64, - page, perPage int) (*ctypes.ResultValidators, error) + page, perPage *int) (*ctypes.ResultValidators, error) func makeValidatorsFunc(c *lrpc.Client) rpcValidatorsFunc { - return func(ctx *rpctypes.Context, height *int64, page, perPage int) (*ctypes.ResultValidators, error) { + return func(ctx *rpctypes.Context, height *int64, page, perPage *int) (*ctypes.ResultValidators, error) { return c.Validators(height, page, perPage) } } @@ -173,10 +173,10 @@ func makeConsensusParamsFunc(c *lrpc.Client) rpcConsensusParamsFunc { } } -type rpcUnconfirmedTxsFunc func(ctx *rpctypes.Context, limit int) (*ctypes.ResultUnconfirmedTxs, error) +type rpcUnconfirmedTxsFunc func(ctx *rpctypes.Context, limit *int) (*ctypes.ResultUnconfirmedTxs, error) func makeUnconfirmedTxsFunc(c *lrpc.Client) rpcUnconfirmedTxsFunc { - return func(ctx *rpctypes.Context, limit int) (*ctypes.ResultUnconfirmedTxs, error) { + return func(ctx *rpctypes.Context, limit *int) (*ctypes.ResultUnconfirmedTxs, error) { return c.UnconfirmedTxs(limit) } } diff --git a/light/rpc/client.go b/light/rpc/client.go index 9bf6590e8..b24e881de 100644 --- a/light/rpc/client.go +++ b/light/rpc/client.go @@ -136,7 +136,7 @@ func (c *Client) BroadcastTxSync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) return c.next.BroadcastTxSync(tx) } -func (c *Client) UnconfirmedTxs(limit int) (*ctypes.ResultUnconfirmedTxs, error) { +func (c *Client) UnconfirmedTxs(limit *int) (*ctypes.ResultUnconfirmedTxs, error) { return c.next.UnconfirmedTxs(limit) } @@ -396,7 +396,7 @@ func (c *Client) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return res, res.Proof.Validate(h.DataHash) } -func (c *Client) TxSearch(query string, prove bool, page, perPage int, orderBy string) ( +func (c *Client) TxSearch(query string, prove bool, page, perPage *int, orderBy string) ( *ctypes.ResultTxSearch, error) { return c.next.TxSearch(query, prove, page, perPage, orderBy) } @@ -405,7 +405,7 @@ func (c *Client) TxSearch(query string, prove bool, page, perPage int, orderBy s // // WARNING: only full validator sets are verified (when length of validators is // less than +perPage+. +perPage+ default is 30, max is 100). -func (c *Client) Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) { +func (c *Client) Validators(height *int64, page, perPage *int) (*ctypes.ResultValidators, error) { res, err := c.next.Validators(height, page, perPage) if err != nil { return nil, err diff --git a/rpc/client/http/http.go b/rpc/client/http/http.go index 7e0ac991f..45638f12f 100644 --- a/rpc/client/http/http.go +++ b/rpc/client/http/http.go @@ -267,9 +267,13 @@ func (c *baseRPCClient) broadcastTX(route string, tx types.Tx) (*ctypes.ResultBr return result, nil } -func (c *baseRPCClient) UnconfirmedTxs(limit int) (*ctypes.ResultUnconfirmedTxs, error) { +func (c *baseRPCClient) UnconfirmedTxs(limit *int) (*ctypes.ResultUnconfirmedTxs, error) { result := new(ctypes.ResultUnconfirmedTxs) - _, err := c.caller.Call("unconfirmed_txs", map[string]interface{}{"limit": limit}, result) + params := make(map[string]interface{}) + if limit != nil { + params["limit"] = limit + } + _, err := c.caller.Call("unconfirmed_txs", params, result) if err != nil { return nil, err } @@ -418,16 +422,20 @@ func (c *baseRPCClient) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return result, nil } -func (c *baseRPCClient) TxSearch(query string, prove bool, page, perPage int, orderBy string) ( +func (c *baseRPCClient) TxSearch(query string, prove bool, page, perPage *int, orderBy string) ( *ctypes.ResultTxSearch, error) { result := new(ctypes.ResultTxSearch) params := map[string]interface{}{ "query": query, "prove": prove, - "page": page, - "per_page": perPage, "order_by": orderBy, } + if page != nil { + params["page"] = page + } + if perPage != nil { + params["per_page"] = perPage + } _, err := c.caller.Call("tx_search", params, result) if err != nil { return nil, err @@ -435,11 +443,14 @@ func (c *baseRPCClient) TxSearch(query string, prove bool, page, perPage int, or return result, nil } -func (c *baseRPCClient) Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) { +func (c *baseRPCClient) Validators(height *int64, page, perPage *int) (*ctypes.ResultValidators, error) { result := new(ctypes.ResultValidators) - params := map[string]interface{}{ - "page": page, - "per_page": perPage, + params := make(map[string]interface{}) + if page != nil { + params["page"] = page + } + if perPage != nil { + params["per_page"] = perPage } if height != nil { params["height"] = height diff --git a/rpc/client/interface.go b/rpc/client/interface.go index 3821c1f0a..b3ce6492e 100644 --- a/rpc/client/interface.go +++ b/rpc/client/interface.go @@ -68,9 +68,9 @@ type SignClient interface { BlockByHash(hash []byte) (*ctypes.ResultBlock, error) BlockResults(height *int64) (*ctypes.ResultBlockResults, error) Commit(height *int64) (*ctypes.ResultCommit, error) - Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) + Validators(height *int64, page, perPage *int) (*ctypes.ResultValidators, error) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) - TxSearch(query string, prove bool, page, perPage int, orderBy string) (*ctypes.ResultTxSearch, error) + TxSearch(query string, prove bool, page, perPage *int, orderBy string) (*ctypes.ResultTxSearch, error) } // HistoryClient provides access to data from genesis to now in large chunks. @@ -113,7 +113,7 @@ type EventsClient interface { // MempoolClient shows us data about current mempool state. type MempoolClient interface { - UnconfirmedTxs(limit int) (*ctypes.ResultUnconfirmedTxs, error) + UnconfirmedTxs(limit *int) (*ctypes.ResultUnconfirmedTxs, error) NumUnconfirmedTxs() (*ctypes.ResultUnconfirmedTxs, error) } diff --git a/rpc/client/local/local.go b/rpc/client/local/local.go index e31e049a0..76209edcf 100644 --- a/rpc/client/local/local.go +++ b/rpc/client/local/local.go @@ -96,7 +96,7 @@ func (c *Local) BroadcastTxSync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) return core.BroadcastTxSync(c.ctx, tx) } -func (c *Local) UnconfirmedTxs(limit int) (*ctypes.ResultUnconfirmedTxs, error) { +func (c *Local) UnconfirmedTxs(limit *int) (*ctypes.ResultUnconfirmedTxs, error) { return core.UnconfirmedTxs(c.ctx, limit) } @@ -156,7 +156,7 @@ func (c *Local) Commit(height *int64) (*ctypes.ResultCommit, error) { return core.Commit(c.ctx, height) } -func (c *Local) Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) { +func (c *Local) Validators(height *int64, page, perPage *int) (*ctypes.ResultValidators, error) { return core.Validators(c.ctx, height, page, perPage) } @@ -164,7 +164,7 @@ func (c *Local) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return core.Tx(c.ctx, hash, prove) } -func (c *Local) TxSearch(query string, prove bool, page, perPage int, orderBy string) ( +func (c *Local) TxSearch(query string, prove bool, page, perPage *int, orderBy string) ( *ctypes.ResultTxSearch, error) { return core.TxSearch(c.ctx, query, prove, page, perPage, orderBy) } diff --git a/rpc/client/mock/client.go b/rpc/client/mock/client.go index 909916630..16ce0c889 100644 --- a/rpc/client/mock/client.go +++ b/rpc/client/mock/client.go @@ -158,7 +158,7 @@ func (c Client) Commit(height *int64) (*ctypes.ResultCommit, error) { return core.Commit(&rpctypes.Context{}, height) } -func (c Client) Validators(height *int64, page, perPage int) (*ctypes.ResultValidators, error) { +func (c Client) Validators(height *int64, page, perPage *int) (*ctypes.ResultValidators, error) { return core.Validators(&rpctypes.Context{}, height, page, perPage) } diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index ef92f1908..3420df667 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -169,7 +169,7 @@ func TestGenesisAndValidators(t *testing.T) { // get the current validators h := int64(1) - vals, err := c.Validators(&h, 0, 0) + vals, err := c.Validators(&h, nil, nil) require.Nil(t, err, "%d: %+v", i, err) require.Equal(t, 1, len(vals.Validators)) require.Equal(t, 1, vals.Count) @@ -350,7 +350,8 @@ func TestUnconfirmedTxs(t *testing.T) { for _, c := range GetClients() { mc := c.(client.MempoolClient) - res, err := mc.UnconfirmedTxs(1) + limit := 1 + res, err := mc.UnconfirmedTxs(&limit) require.NoError(t, err) assert.Equal(t, 1, res.Count) @@ -451,7 +452,7 @@ func TestTxSearchWithTimeout(t *testing.T) { timeoutClient := getHTTPClientWithTimeout(10) // query using a compositeKey (see kvstore application) - result, err := timeoutClient.TxSearch("app.creator='Cosmoshi Netowoko'", false, 1, 30, "asc") + result, err := timeoutClient.TxSearch("app.creator='Cosmoshi Netowoko'", false, nil, nil, "asc") require.Nil(t, err) if len(result.Txs) == 0 { t.Fatal("expected a lot of transactions") @@ -470,7 +471,7 @@ func TestTxSearch(t *testing.T) { // since we're not using an isolated test server, we'll have lingering transactions // from other tests as well - result, err := c.TxSearch("tx.height >= 0", true, 1, 100, "asc") + result, err := c.TxSearch("tx.height >= 0", true, nil, nil, "asc") require.NoError(t, err) txCount := len(result.Txs) @@ -482,7 +483,7 @@ func TestTxSearch(t *testing.T) { t.Logf("client %d", i) // now we query for the tx. - result, err := c.TxSearch(fmt.Sprintf("tx.hash='%v'", find.Hash), true, 1, 30, "asc") + result, err := c.TxSearch(fmt.Sprintf("tx.hash='%v'", find.Hash), true, nil, nil, "asc") require.Nil(t, err) require.Len(t, result.Txs, 1) require.Equal(t, find.Hash, result.Txs[0].Hash) @@ -500,57 +501,58 @@ func TestTxSearch(t *testing.T) { } // query by height - result, err = c.TxSearch(fmt.Sprintf("tx.height=%d", find.Height), true, 1, 30, "asc") + result, err = c.TxSearch(fmt.Sprintf("tx.height=%d", find.Height), true, nil, nil, "asc") require.Nil(t, err) require.Len(t, result.Txs, 1) // query for non existing tx - result, err = c.TxSearch(fmt.Sprintf("tx.hash='%X'", anotherTxHash), false, 1, 30, "asc") + result, err = c.TxSearch(fmt.Sprintf("tx.hash='%X'", anotherTxHash), false, nil, nil, "asc") require.Nil(t, err) require.Len(t, result.Txs, 0) // query using a compositeKey (see kvstore application) - result, err = c.TxSearch("app.creator='Cosmoshi Netowoko'", false, 1, 30, "asc") + result, err = c.TxSearch("app.creator='Cosmoshi Netowoko'", false, nil, nil, "asc") require.Nil(t, err) if len(result.Txs) == 0 { t.Fatal("expected a lot of transactions") } // query using an index key - result, err = c.TxSearch("app.index_key='index is working'", false, 1, 30, "asc") + result, err = c.TxSearch("app.index_key='index is working'", false, nil, nil, "asc") require.Nil(t, err) if len(result.Txs) == 0 { t.Fatal("expected a lot of transactions") } // query using an noindex key - result, err = c.TxSearch("app.noindex_key='index is working'", false, 1, 30, "asc") + result, err = c.TxSearch("app.noindex_key='index is working'", false, nil, nil, "asc") require.Nil(t, err) if len(result.Txs) != 0 { t.Fatal("expected no transaction") } // query using a compositeKey (see kvstore application) and height - result, err = c.TxSearch("app.creator='Cosmoshi Netowoko' AND tx.height<10000", true, 1, 30, "asc") + result, err = c.TxSearch("app.creator='Cosmoshi Netowoko' AND tx.height<10000", true, nil, nil, "asc") require.Nil(t, err) if len(result.Txs) == 0 { t.Fatal("expected a lot of transactions") } // query a non existing tx with page 1 and txsPerPage 1 - result, err = c.TxSearch("app.creator='Cosmoshi Neetowoko'", true, 1, 1, "asc") + perPage := 1 + result, err = c.TxSearch("app.creator='Cosmoshi Neetowoko'", true, nil, &perPage, "asc") require.Nil(t, err) require.Len(t, result.Txs, 0) // check sorting - result, err = c.TxSearch("tx.height >= 1", false, 1, 30, "asc") + result, err = c.TxSearch("tx.height >= 1", false, nil, nil, "asc") require.Nil(t, err) for k := 0; k < len(result.Txs)-1; k++ { require.LessOrEqual(t, result.Txs[k].Height, result.Txs[k+1].Height) require.LessOrEqual(t, result.Txs[k].Index, result.Txs[k+1].Index) } - result, err = c.TxSearch("tx.height >= 1", false, 1, 30, "desc") + result, err = c.TxSearch("tx.height >= 1", false, nil, nil, "desc") require.Nil(t, err) for k := 0; k < len(result.Txs)-1; k++ { require.GreaterOrEqual(t, result.Txs[k].Height, result.Txs[k+1].Height) @@ -558,14 +560,15 @@ func TestTxSearch(t *testing.T) { } // check pagination + perPage = 3 var ( seen = map[int64]bool{} maxHeight int64 - perPage = 3 pages = int(math.Ceil(float64(txCount) / float64(perPage))) ) for page := 1; page <= pages; page++ { - result, err = c.TxSearch("tx.height >= 1", false, page, perPage, "asc") + page := page + result, err = c.TxSearch("tx.height >= 1", false, &page, &perPage, "asc") require.NoError(t, err) if page < pages { require.Len(t, result.Txs, perPage) diff --git a/rpc/core/consensus.go b/rpc/core/consensus.go index 9f5a403bb..9da667b79 100644 --- a/rpc/core/consensus.go +++ b/rpc/core/consensus.go @@ -16,7 +16,7 @@ import ( // for the validators in the set as used in computing their Merkle root. // // More: https://docs.tendermint.com/master/rpc/#/Info/validators -func Validators(ctx *rpctypes.Context, heightPtr *int64, page, perPage int) (*ctypes.ResultValidators, error) { +func Validators(ctx *rpctypes.Context, heightPtr *int64, pagePtr, perPagePtr *int) (*ctypes.ResultValidators, error) { // The latest validator that we know is the NextValidator of the last block. height, err := getHeight(latestUncommittedHeight(), heightPtr) if err != nil { @@ -29,8 +29,8 @@ func Validators(ctx *rpctypes.Context, heightPtr *int64, page, perPage int) (*ct } totalCount := len(validators.Validators) - perPage = validatePerPage(perPage) - page, err = validatePage(page, perPage, totalCount) + perPage := validatePerPage(perPagePtr) + page, err := validatePage(pagePtr, perPage, totalCount) if err != nil { return nil, err } diff --git a/rpc/core/env.go b/rpc/core/env.go index bc7289561..7d19f145e 100644 --- a/rpc/core/env.go +++ b/rpc/core/env.go @@ -92,27 +92,33 @@ type Environment struct { //---------------------------------------------- -func validatePage(page, perPage, totalCount int) (int, error) { +func validatePage(pagePtr *int, perPage, totalCount int) (int, error) { if perPage < 1 { panic(fmt.Sprintf("zero or negative perPage: %d", perPage)) } - if page == 0 { - return 1, nil // default + if pagePtr == nil { // no page parameter + return 1, nil } pages := ((totalCount - 1) / perPage) + 1 if pages == 0 { pages = 1 // one page (even if it's empty) } - if page < 0 || page > pages { - return 1, fmt.Errorf("page should be within [0, %d] range, given %d", pages, page) + page := *pagePtr + if page <= 0 || page > pages { + return 1, fmt.Errorf("page should be within [1, %d] range, given %d", pages, page) } return page, nil } -func validatePerPage(perPage int) int { +func validatePerPage(perPagePtr *int) int { + if perPagePtr == nil { // no per_page parameter + return defaultPerPage + } + + perPage := *perPagePtr if perPage < 1 { return defaultPerPage } else if perPage > maxPerPage { diff --git a/rpc/core/env_test.go b/rpc/core/env_test.go index f9d408491..b44c21a4c 100644 --- a/rpc/core/env_test.go +++ b/rpc/core/env_test.go @@ -40,7 +40,7 @@ func TestPaginationPage(t *testing.T) { } for _, c := range cases { - p, err := validatePage(c.page, c.perPage, c.totalCount) + p, err := validatePage(&c.page, c.perPage, c.totalCount) if c.expErr { assert.Error(t, err) continue @@ -49,6 +49,11 @@ func TestPaginationPage(t *testing.T) { assert.Equal(t, c.newPage, p, fmt.Sprintf("%v", c)) } + // nil case + p, err := validatePage(nil, 1, 1) + if assert.NoError(t, err) { + assert.Equal(t, 1, p) + } } func TestPaginationPerPage(t *testing.T) { @@ -67,7 +72,11 @@ func TestPaginationPerPage(t *testing.T) { } for _, c := range cases { - p := validatePerPage(c.perPage) + p := validatePerPage(&c.perPage) assert.Equal(t, c.newPerPage, p, fmt.Sprintf("%v", c)) } + + // nil case + p := validatePerPage(nil) + assert.Equal(t, defaultPerPage, p) } diff --git a/rpc/core/mempool.go b/rpc/core/mempool.go index d8a72d6d5..11e72cd3d 100644 --- a/rpc/core/mempool.go +++ b/rpc/core/mempool.go @@ -130,9 +130,9 @@ func BroadcastTxCommit(ctx *rpctypes.Context, tx types.Tx) (*ctypes.ResultBroadc // UnconfirmedTxs gets unconfirmed transactions (maximum ?limit entries) // including their number. // More: https://docs.tendermint.com/master/rpc/#/Info/unconfirmed_txs -func UnconfirmedTxs(ctx *rpctypes.Context, limit int) (*ctypes.ResultUnconfirmedTxs, error) { +func UnconfirmedTxs(ctx *rpctypes.Context, limitPtr *int) (*ctypes.ResultUnconfirmedTxs, error) { // reuse per_page validator - limit = validatePerPage(limit) + limit := validatePerPage(limitPtr) txs := env.Mempool.ReapMaxTxs(limit) return &ctypes.ResultUnconfirmedTxs{ diff --git a/rpc/core/tx.go b/rpc/core/tx.go index 387de9e5e..63edbaa6b 100644 --- a/rpc/core/tx.go +++ b/rpc/core/tx.go @@ -54,7 +54,7 @@ func Tx(ctx *rpctypes.Context, hash []byte, prove bool) (*ctypes.ResultTx, error // TxSearch allows you to query for multiple transactions results. It returns a // list of transactions (maximum ?per_page entries) and the total count. // More: https://docs.tendermint.com/master/rpc/#/Info/tx_search -func TxSearch(ctx *rpctypes.Context, query string, prove bool, page, perPage int, orderBy string) ( +func TxSearch(ctx *rpctypes.Context, query string, prove bool, pagePtr, perPagePtr *int, orderBy string) ( *ctypes.ResultTxSearch, error) { // if index is disabled, return error if _, ok := env.TxIndexer.(*null.TxIndex); ok { @@ -93,8 +93,8 @@ func TxSearch(ctx *rpctypes.Context, query string, prove bool, page, perPage int // paginate results totalCount := len(results) - perPage = validatePerPage(perPage) - page, err = validatePage(page, perPage, totalCount) + perPage := validatePerPage(perPagePtr) + page, err := validatePage(pagePtr, perPage, totalCount) if err != nil { return nil, err } diff --git a/rpc/swagger/swagger.yaml b/rpc/swagger/swagger.yaml index 909fbed5c..a4428654f 100644 --- a/rpc/swagger/swagger.yaml +++ b/rpc/swagger/swagger.yaml @@ -656,7 +656,7 @@ paths: required: false schema: type: number - default: 0 + default: 1 example: 1 - in: query name: per_page @@ -786,9 +786,11 @@ paths: parameters: - in: query name: limit - description: Maximum number of unconfirmed transactions to return + description: Maximum number of unconfirmed transactions to return (max: 100) + required: false schema: type: number + default: 30 example: 1 tags: - Info